refactor: code cleanup & docs rewording

credentials/apps/badges/exceptions.py

@@ -13,9 +13,3 @@ class BadgesProcessingError(BadgesError):
     """
     Exception raised for errors that occur during badge processing.
     """
-
-
-class StopEventProcessing(BadgesProcessingError):
-    """
-    Exception raised to stop processing an event due to a specific condition.
-    """

credentials/apps/badges/models.py

@@ -5,6 +5,7 @@
 import logging
 import operator
 import uuid
+from urllib.parse import urljoin
 
 from django.conf import settings
 from django.db import models
@@ -157,7 +158,9 @@ def management_url(self):
         Build external Credly dashboard URL.
         """
         credly_host_base_url = get_credly_base_url(settings)
-        return f"{credly_host_base_url}mgmt/organizations/{self.organization.uuid}/badges/templates/{self.uuid}/details"
+        return urljoin(
+            credly_host_base_url, f"mgmt/organizations/{self.organization.uuid}/badges/templates/{self.uuid}/details"
+        )
 
 
 class BadgeRequirement(models.Model):
@@ -237,7 +240,7 @@ def reset(self, username: str):
             requirement=self,
             progress__username=username,
         ).first()
-        deleted, __ = fulfillment.delete() if fulfillment else (False, 0)
+        deleted = self._delete_fulfillment_if_exists(fulfillment)
         if deleted:
             notify_requirement_regressed(
                 sender=self,
@@ -253,6 +256,17 @@ def is_fulfilled(self, username: str) -> bool:
 
         return self.fulfillments.filter(progress__username=username, progress__template=self.template).exists()
 
+    def _delete_fulfillment_if_exists(self, fulfillment):
+        """
+        Deletes the fulfillment if it exists.
+        """
+
+        if not fulfillment:
+            return False
+
+        fulfillment.delete()
+        return True
+
     @classmethod
     def is_group_fulfilled(cls, *, group: str, template: BadgeTemplate, username: str) -> bool:
         """
@@ -291,8 +305,6 @@ class AbstractDataRule(models.Model):
     OPERATORS = Choices(
         ("eq", "="),
         ("ne", "!="),
-        # ('lt', '<'),
-        # ('gt', '>'),
     )
 
     TRUE_VALUES = ["True", "true", "Yes", "yes", "+"]
@@ -481,7 +493,7 @@ class BadgeProgress(models.Model):
     - user-centric;
     """
 
-    username = models.CharField(max_length=255)  # index
+    username = models.CharField(max_length=255)
     template = models.ForeignKey(
         BadgeTemplate,
         models.SET_NULL,
@@ -513,8 +525,8 @@ def ratio(self) -> float:
         if not self.groups:
             return 0.00
 
-        true_values = len(list(filter(lambda x: x, self.groups.values())))
-        return round(true_values / len(self.groups.keys()), 2)
+        true_values_count = self._get_groups_true_values_count()
+        return round(true_values_count / len(self.groups.keys()), 2)
 
     @property
     def groups(self):
@@ -556,6 +568,17 @@ def reset(self):
 
         Fulfillment.objects.filter(progress=self).delete()
 
+    def _get_groups_true_values_count(self):
+        """
+        Returns the count of groups with fulfilled requirements.
+        """
+
+        result = 0
+        for fulfilled in self.groups.values():
+            if fulfilled:
+                result += 1
+        return result
+
 
 class Fulfillment(models.Model):
     """

credentials/apps/badges/processing/generic.py

@@ -4,7 +4,7 @@
 
 import logging
 
-from credentials.apps.badges.exceptions import BadgesProcessingError, StopEventProcessing
+from credentials.apps.badges.exceptions import BadgesProcessingError
 from credentials.apps.badges.processing.progression import process_requirements
 from credentials.apps.badges.processing.regression import process_penalties
 from credentials.apps.badges.utils import extract_payload, get_user_data
@@ -36,10 +36,6 @@ def process_event(sender, **kwargs):
         # penalties processing
         process_penalties(event_type, username, extract_payload(kwargs))
 
-    except StopEventProcessing:
-        # controlled processing dropping
-        return
-
     except BadgesProcessingError as error:
         logger.error(f"Badges processing error: {error}")
         return
@@ -53,7 +49,8 @@ def identify_user(*, event_type, event_payload):
     or creates a new user based on this data, and then returns the username.
 
     Args:
-        **kwargs: public event keyword arguments containing user identification data.
+        event_type (str): The type of the event.
+        event_payload (dict): The payload of the event.
 
     Returns:
         str: The username of the identified (and created if needed) user.

credentials/apps/badges/processing/progression.py

@@ -39,12 +39,10 @@ def process_requirements(event_type, username, payload):
 
         # drop early: if the badge template is already "done"
         if requirement.template_id in completed_templates:
-            # TODO: move this check to `discover_requirements` query
             continue
 
         # drop early: if the requirement is already "done"
         if requirement.is_fulfilled(username):
-            # TODO: move this check to `discover_requirements` query
             continue
 
         # process: payload rules

credentials/apps/badges/tests/test_services.py

@@ -391,6 +391,9 @@ def setUp(self):
         # disconnect BADGE_PROGRESS_COMPLETE signal
         BADGE_PROGRESS_COMPLETE.disconnect(handle_badge_completion)
 
+    def tearDown(self):
+        BADGE_PROGRESS_COMPLETE.connect(handle_badge_completion)
+
     # test cases
     #     A course completion - course A w/o a group;
     #     A or B course completion - courses A, B have the same group value;
@@ -400,6 +403,12 @@ def setUp(self):
     #     (A or B) and (C or D) - courses A, B have the same group value; courses C, D have the same group value;
 
     def test_course_a_completion(self):
+        """
+        Test course A completion.
+
+        A course completion - course A w/o a group.
+        """
+
         requirement = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -416,6 +425,12 @@ def test_course_a_completion(self):
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
     def test_course_a_or_b_completion(self):
+        """
+        Test course A or B completion.
+
+        A or B course completion - courses A, B have the same group value.
+        """
+
         requirement_a = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -446,6 +461,12 @@ def test_course_a_or_b_completion(self):
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
     def test_course_a_or_b_or_c_completion(self):
+        """
+        Test course A or B or C completion.
+
+        A or B or C course completion - courses A, B, C have the same group value.
+        """
+
         requirement_a = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -489,6 +510,12 @@ def test_course_a_or_b_or_c_completion(self):
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
     def test_course_a_or_completion(self):
+        """
+        Test course A or completion.
+
+        A or - courses A is the only course in the group.
+        """
+
         requirement = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -506,6 +533,12 @@ def test_course_a_or_completion(self):
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
     def test_course_a_or_b_and_c_completion(self):
+        """
+        Test course A or B and C completion.
+
+        (A or B) and C - A, B have the same group value; course C w/o a group.
+        """
+
         requirement_a = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -558,6 +591,12 @@ def test_course_a_or_b_and_c_completion(self):
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
     def test_course_a_or_b_and_c_or_d_completion(self):
+        """
+        Test course A or B and C or D completion.
+
+        (A or B) and (C or D) - courses A, B have the same group value; courses C, D have the same group value.
+        """
+
         requirement_a = BadgeRequirement.objects.create(
             template=self.badge_template,
             event_type=COURSE_PASSING_EVENT,
@@ -625,9 +664,6 @@ def test_course_a_or_b_and_c_or_d_completion(self):
         self.assertEqual(Fulfillment.objects.filter(requirement=requirement_d).count(), 0)
         self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed)
 
-    def tearDown(self):
-        BADGE_PROGRESS_COMPLETE.connect(handle_badge_completion)
-
 
 class TestIdentifyUser(TestCase):
     def test_identify_user(self):

docs/badges/configuration.rst

@@ -20,11 +20,11 @@ Multiple Credly Organizations can be configured.
 Go to the Credly Organization section in the admin panel and create a new item:
 
 1. to set the UUID use your Credly Organization identifier;
-2. to set the authorization token issue one in the Credly Organization's dashboard.
+2. to set the authorization token, used to sync the Credly Organization.
 
 Check: the system pulls the Organization's details and updates its name.
 
-In case of errors check the used credentials for the Organization.
+In case of errors, check the credentials used for the Organization
 
 Badge templates
 ---------------
@@ -40,14 +40,9 @@ Webhooks
 
 Webhooks are set up on Credly management dashboard as Credly is a main initiator of the syncronization.
 
-You should be able to select an action from the list so that whenever this action happens internally, external platform will be alerted and updated.
+You should be able to select an action from the list so that whenever the specified action occurs internally, the external system is alerted.
 
-Without the syncronization, actions such as:
-
-- Update of the badge template
-- Archivation of the badge template
-
-external platform will not be aware of any changes so it might continue issuing outdated badges.
+Without this synchronization, the external system will not be notified of any significant changes (e.g. a badge template update, or a badge template has been archived) and may incorrectly issue erroneous or outdated badges.
 
 Synchronization
 ~~~~~~~~~~~~~~~
@@ -69,7 +64,7 @@ On success, the system will update the list of Credly badge templates for the Or
 .. image:: ../_static/images/badges/badges-admin-credly-templates-list.png
         :alt: Credly badge templates list
 
-For the usage a badge template must be configured and activated first.
+For a badge template to be considered during the processing it must be configured (to have at least 1 requirement) and activated (enabled) first.
 
 Badge Requirements
 ------------------
@@ -84,7 +79,7 @@ Badge Requirements are listed inline on a badge template detail page.
 .. image:: ../_static/images/badges/badges-admin-template-requirements.png
         :alt: Credly badge template requirements
 
-A badge template can can have multiple requirements. All badge requirements must be *fulfilled* to earn a badge.
+A badge template can have multiple requirements. All badge requirements must be *fulfilled* before the system will issue a badge to a learner.
 
 Event type
 ~~~~~~~~~~
@@ -171,7 +166,7 @@ There could be 0 or more badge penalties configured for a badge template.
 Each badge penalty is *targeted* to 1 or more badge requirements.
 A penalty setup is similar to a badge requirement, but has different effect: it decreases badge progress for a user.
 
-When a penalty has worked all linked badge requirements are "rolled back" (user's progress for such requirements is reset).
+When all penalty rules have been applied, a learner's progress towards a badge is reset.
 
 .. image:: ../_static/images/badges/badges-admin-penalty-rules.png
         :alt: Badge penalty rules edit

docs/badges/index.rst

@@ -10,7 +10,7 @@ Current Badges version is highly integrated with the `Credly (by Pearson)`_ serv
 
     What is Credly?
 
-    **Credly** is the end-to-end solution for creating, issuing, and managing digital credentials. Thousands of organizations use **Credly** to recognize achievement. 
+    **Credly** is a end-to-end solution for creating, issuing, and managing digital Credentials. Organizations use **Credly** to recognize their learners' achievements.
     Users use Credly as a professional board to store badges to visualize their professional success – which courses were completed and when. Badges provide employers and peers concrete evidence of what you had to do to earn your credential and what you're now capable of. Credly also offers labor market insights based on users’ skills. Users can search and apply for job opportunities right through Credly.
 
 

docs/badges/processing.rst

@@ -78,8 +78,6 @@ On badge progress completion, the system:
 
     The Badges application implements its extended ``UserCredential`` version (the CredlyBadge) to track external issuing state. Once the Credly badge is successfully issued the **CredlyBadge is updated with its UUID and state**.
 
-Badge revoking is optional. Business needs. Configuration. And why.
-
 .. _event types: https://docs.openedx.org/projects/openedx-events/en/latest/
 .. _openedx-events: https://github.com/openedx/openedx-events
 .. _default settings: settings.html#default-settings
@@ -101,4 +99,4 @@ Badges can also be revoked. Its a separete set of rules that need to be set up.
 
 .. _Configuration: configuration.html
 
-When system revokes a badge, the status of a badge will change from awarded to revoked in the admin panel (admim/badges/credly_badges) as the revoke process is synced with external platform. 
+When a learner's badge is revoked by Credly, the Credentials IDA will be notified and will update it's internal records. The status of the badge will change from `awarded` to `revoked` upon successful revocation.