Skip to content

Commit

Permalink
refactor: [ACI-997] refactor for quality checks pass
Browse files Browse the repository at this point in the history
  • Loading branch information
Andrii committed May 31, 2024
1 parent 61c8082 commit 5a4911d
Show file tree
Hide file tree
Showing 27 changed files with 874 additions and 823 deletions.
27 changes: 9 additions & 18 deletions credentials/apps/badges/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!
"""
),
Expand Down Expand Up @@ -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.
"""
Expand All @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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]))
Expand Down
21 changes: 12 additions & 9 deletions credentials/apps/badges/admin_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -64,7 +64,7 @@ def save(self, commit=True):

return instance

Check warning on line 65 in credentials/apps/badges/admin_forms.py

View check run for this annotation

Codecov / codecov/patch

credentials/apps/badges/admin_forms.py#L65

Added line #L65 was not covered by tests

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.
"""
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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)

Check warning on line 175 in credentials/apps/badges/admin_forms.py

View check run for this annotation

Codecov / codecov/patch

credentials/apps/badges/admin_forms.py#L174-L175

Added lines #L174 - L175 were not covered by tests

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())

Check warning on line 178 in credentials/apps/badges/admin_forms.py

View check run for this annotation

Codecov / codecov/patch

credentials/apps/badges/admin_forms.py#L178

Added line #L178 was not covered by tests


class PenaltyDataRuleFormSet(ParentMixin, forms.BaseInlineFormSet): ...
class PenaltyDataRuleFormSet(ParentMixin, forms.BaseInlineFormSet):
pass


class PenaltyDataRuleForm(DataRuleExtensionsMixin, forms.ModelForm):
Expand Down
12 changes: 8 additions & 4 deletions credentials/apps/badges/apps.py
Original file line number Diff line number Diff line change
@@ -1,13 +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.
"""

plugin_label = "badges"

@property
def verbose_name(self):
return f"Badges: {self.plugin_label}"

Check warning on line 15 in credentials/apps/badges/apps.py

View check run for this annotation

Codecov / codecov/patch

credentials/apps/badges/apps.py#L15

Added line #L15 was not covered by tests
Expand All @@ -29,9 +31,11 @@ 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 listen_to_badging_events # pylint: import-outside-toplevel

listen_to_badging_events()

Expand Down
2 changes: 1 addition & 1 deletion credentials/apps/badges/credly/api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down
4 changes: 0 additions & 4 deletions credentials/apps/badges/credly/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,8 @@ class CredlyError(BadgesError):
Credly backend generic error.
"""

pass


class CredlyAPIError(CredlyError):
"""
Credly API errors.
"""

pass
6 changes: 0 additions & 6 deletions credentials/apps/badges/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion credentials/apps/badges/issuers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = []
Expand All @@ -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:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Generated by Django 4.1 on 2024-05-31 13:46

from django.db import migrations, models
import model_utils.fields


class Migration(migrations.Migration):

dependencies = [
("badges", "0021_alter_credlyorganization_api_key"),
]

operations = [
migrations.RenameField(
model_name="fulfillment",
old_name="group",
new_name="blend",
),
migrations.RemoveField(
model_name="badgerequirement",
name="group",
),
migrations.AddField(
model_name="badgerequirement",
name="blend",
field=models.CharField(
blank=True,
help_text="Optional. Group requirements together using the same Group ID for interchangeable (OR processing logic).",
max_length=255,
null=True,
),
),
migrations.AlterField(
model_name="badgetemplate",
name="state",
field=model_utils.fields.StatusField(
choices=[("draft", "draft"), ("active", "active"), ("archived", "archived")],
default="draft",
help_text="Credly badge template state (auto-managed).",
max_length=100,
no_check_for_status=True,
),
),
migrations.AlterField(
model_name="credlybadge",
name="state",
field=model_utils.fields.StatusField(
choices=[
("created", "created"),
("no_response", "no_response"),
("error", "error"),
("pending", "pending"),
("accepted", "accepted"),
("rejected", "rejected"),
("revoked", "revoked"),
("expired", "expired"),
],
default="created",
help_text="Credly badge issuing state",
max_length=100,
no_check_for_status=True,
),
),
]
Loading

0 comments on commit 5a4911d

Please sign in to comment.