refactor: call skills api only when required

The skills API was being called for each xblock causing performance issue. Now the API is only called if we cross the probablity check.

CHANGELOG.rst

@@ -16,6 +16,15 @@ Unreleased
 
 *
 
+[0.1.3] - 2023-09-27
+************************************************
+
+Changed
+=======
+
+* Gate skills API call behind probablity check to reduce traffic.
+
+
 [0.1.2] - 2023-08-18
 ************************************************
 

skill_tagging/__init__.py

@@ -2,7 +2,7 @@
 Django app plugin for fetching and verifying tags for xblock skills.
 """
 
-__version__ = '0.1.2'
+__version__ = '0.1.3'
 
 # pylint: disable=invalid-name
 default_app_config = 'skill_tagging.apps.SkillTaggingConfig'

skill_tagging/pipeline.py

@@ -82,8 +82,11 @@ class AddVerticalBlockSkillVerificationSection(VerificationPipelineBase, Pipelin
     def run_filter(self, block, fragment, context, view):  # pylint: disable=arguments-differ
         """Pipeline Step implementing the Filter"""
 
+        # Check whether we need to run this filter and only call the API.
+        if not self.should_run_filter():
+            return {"block": block, "fragment": fragment, "context": context, "view": view}
         skills = self.fetch_related_skills(block)
-        if not skills or not self.should_run_filter():
+        if not skills:
             return {"block": block, "fragment": fragment, "context": context, "view": view}
         usage_id = block.scope_ids.usage_id
         data = self.get_skill_context(usage_id, block, skills)
@@ -116,11 +119,11 @@ class AddVideoBlockSkillVerificationComponent(VerificationPipelineBase, Pipeline
     def run_filter(self, block, context):  # pylint: disable=arguments-differ
         """Pipeline Step implementing the Filter"""
         usage_id = block.scope_ids.usage_id
-        if usage_id.block_type != "video":
+        if usage_id.block_type != "video" or not self.should_run_filter():
             # avoid fetching skills for other xblocks
             return {"block": block, "context": context}
         skills = self.fetch_related_skills(block)
-        if not skills or not self.should_run_filter():
+        if not skills:
             return {"block": block, "context": context}
         data = self.get_skill_context(usage_id, block, skills)
 

skill_tagging/skill_tagging_mixin.py

@@ -71,7 +71,7 @@ def fetch_skill_tags(self):
         usage_id_str = str(self.scope_ids.usage_id)
         XBLOCK_SKILL_TAGS_API = urljoin(
             settings.TAXONOMY_API_BASE_URL,
-            '/taxonomy/api/v1/xblocks'
+            '/taxonomy/api/v1/xblocks/'
         )
         response = api_client.get(
             XBLOCK_SKILL_TAGS_API,

tests/test_pipeline.py

@@ -35,14 +35,12 @@ def setUp(self) -> None:
         SHOW_SKILL_VERIFICATION_PROBABILITY=0,
     )
     def test_pipeline_does_nothing_when_probability_set_to_zero(self, mock_get_api_client):
-        """
-        Check that the input fragment is unchanged when there is no
-        configuration for a course.
-        """
+        """Check that pipeline is not executed if probability is set to zero"""
         mock_get_api_client.return_value = self.get_mock_api_response()
         _, fragment, _, _ = VerticalBlockRenderCompleted.run_filter(
             block=self.block, context={}, fragment=self.original_fragement, view={}
         )
+        mock_get_api_client.assert_not_called()
         self.assertEqual(fragment.content, self.original_fragement.content)
         self.assertNotIn("SKILL-0", fragment.content)
 
@@ -84,6 +82,19 @@ def setUp(self) -> None:
         self.original_fragement = Fragment(content="<div class='video-player'></div>")
         self.block.fragment = self.original_fragement
 
+    @override_settings(
+        SHOW_SKILL_VERIFICATION_PROBABILITY=0,
+    )
+    def test_pipeline_does_nothing_when_probability_set_to_zero(self, mock_get_api_client):
+        """Check that pipeline is not executed if probability is set to zero"""
+        mock_get_api_client.return_value = self.get_mock_api_response()
+        _, fragment, _, _ = VerticalBlockRenderCompleted.run_filter(
+            block=self.block, context={}, fragment=self.original_fragement, view={}
+        )
+        mock_get_api_client.assert_not_called()
+        self.assertEqual(fragment.content, self.original_fragement.content)
+        self.assertNotIn("SKILL-0", fragment.content)
+
     def test_pipeline_adds_required_resources(self, mock_get_api_client):
         """Check that pipeline adds required resources."""
         mock_get_api_client.return_value = self.get_mock_api_response()