test: increase test coverage

.annotation_safe_list.yml

@@ -13,6 +13,20 @@ auth.Group:
   ".. no_pii:": "This model has no PII"
 auth.Permission:
   ".. no_pii:": "This model has no PII"
+badges.BadgePenalty:
+  ".. no_pii:": "This model has no PII"
+badges.BadgeProgress:
+  ".. no_pii:": "This model has no PII"
+badges.BadgeRequirement:
+  ".. no_pii:": "This model has no PII"
+badges.CredlyOrganization:
+  ".. no_pii:": "This model has no PII"
+badges.DataRule:
+  ".. no_pii:": "This model has no PII"
+badges.Fulfillment:
+  ".. no_pii:": "This model has no PII"
+badges.PenaltyDataRule:
+  ".. no_pii:": "This model has no PII"
 credentials.HistoricalProgramCompletionEmailConfiguration:
     ".. no_pii:": "This model has no PII"
 contenttypes.ContentType:

credentials/apps/badges/admin.py

@@ -47,10 +47,10 @@ class BadgeRequirementInline(admin.TabularInline):
         "event_type",
         "rules",
         "description",
-        "group",
+        "blend",
     )
     readonly_fields = ("rules",)
-    ordering = ("group",)
+    ordering = ("blend",)
     form = BadgeRequirementForm
     formset = BadgeRequirementFormSet
 
@@ -194,7 +194,7 @@ def get_fields(self, request, obj=None):
         return fields
 
     def get_readonly_fields(self, request, obj=None):
-        readonly_fields = super().get_readonly_fields(request, obj)
+        readonly_fields = list(super().get_readonly_fields(request, obj))
 
         if not obj:
             return readonly_fields
@@ -247,7 +247,7 @@ class CredlyBadgeTemplateAdmin(admin.ModelAdmin):
                 "description": _(
                     """
                     WARNING: avoid configuration updates on activated badges.
-                    Active badge templates are continuously processed and learners may already have partial progress on them.
+                    Active badge templates are continuously processed and learners may already have progress on them.
                     Any changes in badge template requirements (including data rules) will affect learners' experience!
                     """
                 ),
@@ -318,10 +318,10 @@ def image(self, obj):
 
     image.short_description = _("icon")
 
-    def save_model(self, request, obj, form, change):  # pylint: disable=unused-argument
+    def save_model(self, request, obj, form, change):
         pass
 
-    def save_formset(self, request, form, formset, change):  # pylint: disable=unused-argument
+    def save_formset(self, request, form, formset, change):
         """
         Check if template is active and has requirements.
         """
@@ -331,7 +331,7 @@ def save_formset(self, request, form, formset, change):  # pylint: disable=unuse
             messages.set_level(request, messages.ERROR)
             messages.error(request, _("Active badge template must have at least one requirement."))
             return HttpResponseRedirect(request.path)
-        form.instance.save()
+        return form.instance.save()
 
 
 class DataRulePenaltyInline(admin.TabularInline):
@@ -368,14 +368,14 @@ class BadgeRequirementAdmin(admin.ModelAdmin):
         "template",
         "event_type",
         "template_link",
-        "group",
+        "blend",
     ]
 
     fields = [
         "template_link",
         "event_type",
         "description",
-        "group",
+        "blend",
     ]
 
     def has_add_permission(self, request):
@@ -455,15 +455,6 @@ def formfield_for_manytomany(self, db_field, request, **kwargs):
                 kwargs["queryset"] = BadgeRequirement.objects.filter(template_id=template_id)
         return super().formfield_for_manytomany(db_field, request, **kwargs)
 
-    def template_link(self, instance):
-        """
-        Interactive link to parent (badge template).
-        """
-        url = reverse("admin:badges_credlybadgetemplate_change", args=[instance.template.pk])
-        return format_html('<a href="{}">{}</a>', url, instance.template)
-
-    template_link.short_description = _("badge template")
-
     def response_change(self, request, obj):
         if "_save" in request.POST:
             return HttpResponseRedirect(reverse("admin:badges_credlybadgetemplate_change", args=[obj.template.pk]))

credentials/apps/badges/admin_forms.py

@@ -50,7 +50,7 @@ def clean(self):
             api_key = settings.BADGES_CONFIG["credly"]["ORGANIZATIONS"][str(uuid)]
 
         credly_api_client = CredlyAPIClient(uuid, api_key)
-        self._ensure_organization_exists(credly_api_client)
+        self.ensure_organization_exists(credly_api_client)
 
         return cleaned_data
 
@@ -64,7 +64,7 @@ def save(self, commit=True):
 
         return instance
 
-    def _ensure_organization_exists(self, api_client):
+    def ensure_organization_exists(self, api_client):
         """
         Try to fetch organization data by the configured Credly Organization ID.
         """
@@ -93,7 +93,7 @@ def clean(self):
         requirements = cleaned_data.get("requirements")
 
         if requirements and not all(
-            [requirement.template.id == cleaned_data.get("template").id for requirement in requirements]
+            requirement.template.id == cleaned_data.get("template").id for requirement in requirements
         ):
             raise forms.ValidationError(_("All requirements must belong to the same template."))
         return cleaned_data
@@ -143,7 +143,8 @@ def clean(self):
         return cleaned_data
 
 
-class DataRuleFormSet(ParentMixin, forms.BaseInlineFormSet): ...
+class DataRuleFormSet(ParentMixin, forms.BaseInlineFormSet):
+    pass
 
 
 class DataRuleForm(DataRuleExtensionsMixin, forms.ModelForm):
@@ -158,25 +159,27 @@ class Meta:
     data_path = forms.ChoiceField()
 
 
-class BadgeRequirementFormSet(ParentMixin, forms.BaseInlineFormSet): ...
+class BadgeRequirementFormSet(ParentMixin, forms.BaseInlineFormSet):
+    pass
 
 
 class BadgeRequirementForm(forms.ModelForm):
     class Meta:
         model = BadgeRequirement
         fields = "__all__"
 
-    group = forms.ChoiceField()
+    blend = forms.ChoiceField()
 
     def __init__(self, *args, parent_instance=None, **kwargs):
         self.template = parent_instance
         super().__init__(*args, **kwargs)
 
-        self.fields["group"].choices = Choices(*[(chr(i), chr(i)) for i in range(65, 91)])
-        self.fields["group"].initial = chr(65 + self.template.requirements.count())
+        self.fields["blend"].choices = Choices(*[(chr(i), chr(i)) for i in range(65, 91)])
+        self.fields["blend"].initial = chr(65 + self.template.requirements.count())
 
 
-class PenaltyDataRuleFormSet(ParentMixin, forms.BaseInlineFormSet): ...
+class PenaltyDataRuleFormSet(ParentMixin, forms.BaseInlineFormSet):
+    pass
 
 
 class PenaltyDataRuleForm(DataRuleExtensionsMixin, forms.ModelForm):

credentials/apps/badges/apps.py

@@ -1,25 +1,15 @@
 from django.apps import AppConfig
 
-from .toggles import check_badges_enabled
+from credentials.apps.badges.toggles import check_badges_enabled
 
 
-class BadgesAppConfig(AppConfig):
-    """
-    Extended application config with additional Badges-specific logic.
-    """
-
-    @property
-    def verbose_name(self):
-        return f"Badges: {self.plugin_label}"
-
-
-class BadgesConfig(BadgesAppConfig):
+class BadgesConfig(AppConfig):
     """
     Core badges application configuration.
     """
 
-    default = True
     name = "credentials.apps.badges"
+    plugin_label = "badges"
     verbose_name = "Badges"
 
     @check_badges_enabled
@@ -29,9 +19,13 @@ def ready(self):
 
         Performs initial registrations for checks, signals, etc.
         """
-        from . import signals  # pylint: disable=unused-import,import-outside-toplevel
-        from .checks import badges_checks  # pylint: disable=unused-import,import-outside-toplevel
-        from .signals.handlers import listen_to_badging_events
+        from credentials.apps.badges import signals  # pylint: disable=unused-import,import-outside-toplevel
+        from credentials.apps.badges.checks import (  # pylint: disable=unused-import,import-outside-toplevel
+            badges_checks,
+        )
+        from credentials.apps.badges.signals.handlers import (  # pylint: disable=import-outside-toplevel
+            listen_to_badging_events,
+        )
 
         listen_to_badging_events()
 

credentials/apps/badges/checks.py

@@ -14,6 +14,7 @@ def badges_checks(*args, **kwargs):
 
     Raises compatibility Errors upon:
         - BADGES_CONFIG['events'] is empty
+        - Credly settings are not properly configured
 
     Returns:
         List of any Errors.

credentials/apps/badges/credly/api_client.py

@@ -70,7 +70,7 @@ def perform_request(self, method, url_suffix, data=None):
         """
         url = urljoin(self.base_api_url, url_suffix)
         logger.debug(f"Credly API: {method.upper()} {url}")
-        response = requests.request(method.upper(), url, headers=self._get_headers(), json=data)
+        response = requests.request(method.upper(), url, headers=self._get_headers(), json=data, timeout=10)
         self._raise_for_error(response)
         return response.json()
 

credentials/apps/badges/credly/exceptions.py

@@ -10,12 +10,8 @@ class CredlyError(BadgesError):
     Credly backend generic error.
     """
 
-    pass
-
 
 class CredlyAPIError(CredlyError):
     """
     Credly API errors.
     """
-
-    pass

credentials/apps/badges/credly/webhooks.py

@@ -59,14 +59,19 @@ def post(self, request):
 
         return Response(status=status.HTTP_204_NO_CONTENT)
 
+    @staticmethod
+    def _get_badge_template_from_data(data):
+        badge_template = data.get("data", {}).get("badge_template", {})
+        return badge_template
+
     @staticmethod
     def handle_badge_template_created_event(request, data):
         """
         Create a new badge template.
         """
-        # TODO: dry it
-        badge_template = data.get("data", {}).get("badge_template", {})
-        owner = data.get("data", {}).get("badge_template", {}).get("owner", {})
+
+        badge_template = CredlyWebhook._get_badge_template_from_data(data)
+        owner = badge_template.get("owner", {})
 
         organization = get_object_or_404(CredlyOrganization, uuid=owner.get("id"))
 
@@ -87,9 +92,9 @@ def handle_badge_template_changed_event(request, data):
         """
         Change the badge template.
         """
-        # TODO: dry it
-        badge_template = data.get("data", {}).get("badge_template", {})
-        owner = data.get("data", {}).get("badge_template", {}).get("owner", {})
+
+        badge_template = CredlyWebhook._get_badge_template_from_data(data)
+        owner = badge_template.get("owner", {})
 
         organization = get_object_or_404(CredlyOrganization, uuid=owner.get("id"))
 
@@ -117,6 +122,6 @@ def handle_badge_template_deleted_event(request, data):
         Deletes the badge template by provided uuid.
         """
         CredlyBadgeTemplate.objects.filter(
-            uuid=data.get("data", {}).get("badge_template", {}).get("id"),
+            uuid=CredlyWebhook._get_badge_template_from_data(data).get("id"),
             site=get_current_site(request),
         ).delete()

credentials/apps/badges/exceptions.py

@@ -8,20 +8,14 @@ class BadgesError(Exception):
     Badges generic exception.
     """
 
-    pass
-
 
 class BadgesProcessingError(BadgesError):
     """
     Exception raised for errors that occur during badge processing.
     """
 
-    pass
-
 
 class StopEventProcessing(BadgesProcessingError):
     """
     Exception raised to stop processing an event due to a specific condition.
     """
-
-    pass

credentials/apps/badges/issuers.py

@@ -36,7 +36,7 @@ def issue_credential(
         attributes=None,
         date_override=None,
         request=None,
-        lms_user_id=None,  # pylint: disable=unused-argument
+        lms_user_id=None,
     ):
         """
         Issue a credential to the user.
@@ -86,6 +86,15 @@ def award(self, *, username, credential_id):
         return user_credential
 
     def revoke(self, credential_id, username):
+        """
+        Revokes a badge.
+
+        Changes user credential status to REVOKED, for a given user.
+        Notifies about the revoked badge (public signal).
+
+        Returns: UserCredential
+        """
+
         credential = self.get_credential(credential_id)
         user_credential = self.issue_credential(credential, username, status=UserCredentialStatus.REVOKED)
 
@@ -130,6 +139,10 @@ def issue_credly_badge(self, *, user_credential):
         user_credential.save()
 
     def revoke_credly_badge(self, credential_id, user_credential):
+        """
+        Requests Credly service for external badge revoking based on internal user credential (CredlyBadge).
+        """
+
         credential = self.get_credential(credential_id)
         credly_api = CredlyAPIClient(credential.organization.uuid)
         revoke_data = {
@@ -165,6 +178,16 @@ def award(self, *, username, credential_id):
         return credly_badge
 
     def revoke(self, credential_id, username):
+        """
+        Revokes a Credly badge.
+
+        - Changes user credential status to REVOKED, for a given user;
+        - Notifies about the revoked badge (public signal);
+        - Revokes external Credly badge (Credly API);
+
+        Returns: (CredlyBadge) user credential
+        """
+
         user_credential = super().revoke(credential_id, username)
         if user_credential.propagated:
             self.revoke_credly_badge(credential_id, user_credential)

credentials/apps/badges/management/commands/sync_organization_badge_templates.py

@@ -21,8 +21,11 @@ def handle(self, *args, **options):
         Sync badge templates for a specific organization or all organizations.
 
         Usage:
-            ./manage.py sync_organization_badge_templates --site_id 1
-            ./manage.py sync_organization_badge_templates --site_id 1 --organization_id c117c179-81b1-4f7e-a3a1-e6ae30568c13
+            site_id=1
+            org_id=c117c179-81b1-4f7e-a3a1-e6ae30568c13
+
+            ./manage.py sync_organization_badge_templates --site_id $site_id
+            ./manage.py sync_organization_badge_templates --site_id $site_id --organization_id $org_id
         """
         DEFAULT_SITE_ID = 1
         organizations_to_sync = []
@@ -40,7 +43,8 @@ def handle(self, *args, **options):
         else:
             organizations_to_sync = CredlyOrganization.get_all_organization_ids()
             logger.info(
-                f"Organization ID wasn't provided: syncing badge templates for all organizations - {organizations_to_sync}"
+                "Organization ID wasn't provided: syncing badge templates for all organizations - "
+                f"{organizations_to_sync}",
             )
 
         for organization_id in organizations_to_sync: