From 310d3418148d461572650de3cfd7ce35aee1f03a Mon Sep 17 00:00:00 2001 From: Volodymyr Bergman Date: Tue, 16 Apr 2024 12:28:16 +0300 Subject: [PATCH] refactor: [ACI-249] extract and unify (#142) * refactor: [ACI-249] extract and unify * refactor: extract signal emitters * fix: progress method API * fix: "model.check" is an internal Django method --- credentials/apps/badges/issuers.py | 3 +- .../0013_alter_datarule_requirement.py | 19 +++ credentials/apps/badges/models.py | 149 +++++++++++++----- credentials/apps/badges/processing/generic.py | 1 + .../apps/badges/processing/progression.py | 20 +-- .../apps/badges/processing/regression.py | 25 ++- credentials/apps/badges/signals/handlers.py | 24 +-- credentials/apps/badges/signals/signals.py | 62 ++++++++ 8 files changed, 212 insertions(+), 91 deletions(-) create mode 100644 credentials/apps/badges/migrations/0013_alter_datarule_requirement.py diff --git a/credentials/apps/badges/issuers.py b/credentials/apps/badges/issuers.py index afbdca875..12af5bebb 100644 --- a/credentials/apps/badges/issuers.py +++ b/credentials/apps/badges/issuers.py @@ -10,8 +10,7 @@ from credentials.apps.badges.credly.data import IssueBadgeData from credentials.apps.badges.credly.exceptions import CredlyAPIError from credentials.apps.badges.models import BadgeTemplate, CredlyBadge, CredlyBadgeTemplate, UserCredential -from credentials.apps.badges.processing.progression import notify_badge_awarded -from credentials.apps.badges.processing.regression import notify_badge_revoked +from credentials.apps.badges.signals.signals import notify_badge_awarded, notify_badge_revoked from credentials.apps.core.api import get_user_by_username from credentials.apps.credentials.constants import UserCredentialStatus from credentials.apps.credentials.issuers import AbstractCredentialIssuer diff --git a/credentials/apps/badges/migrations/0013_alter_datarule_requirement.py b/credentials/apps/badges/migrations/0013_alter_datarule_requirement.py new file mode 100644 index 000000000..7caf76874 --- /dev/null +++ b/credentials/apps/badges/migrations/0013_alter_datarule_requirement.py @@ -0,0 +1,19 @@ +# Generated by Django 3.2.20 on 2024-04-15 14:28 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('badges', '0012_auto_20240412_1707'), + ] + + operations = [ + migrations.AlterField( + model_name='datarule', + name='requirement', + field=models.ForeignKey(help_text='Parent requirement for this data rule.', on_delete=django.db.models.deletion.CASCADE, related_name='rules', to='badges.badgerequirement'), + ), + ] diff --git a/credentials/apps/badges/models.py b/credentials/apps/badges/models.py index 573792670..b00d6db3a 100644 --- a/credentials/apps/badges/models.py +++ b/credentials/apps/badges/models.py @@ -9,6 +9,12 @@ from django.core.exceptions import ValidationError from django.db import models from django.utils.translation import gettext_lazy as _ +from credentials.apps.badges.signals.signals import ( + notify_progress_complete, + notify_progress_incomplete, + notify_requirement_fulfilled, + notify_requirement_regressed, +) from django_extensions.db.models import TimeStampedModel from model_utils import Choices from model_utils.fields import StatusField @@ -93,7 +99,7 @@ def user_progress(self, username: str) -> float: """ Determines a completion progress for user. """ - progress = BadgeProgress.objects.filter(username=username, template=self).first() + progress = BadgeProgress.for_user(username=username, template_id=self.id) if progress is None: return 0.00 return progress.ratio @@ -136,7 +142,7 @@ class BadgeRequirement(models.Model): Describes what must happen for badge template to progress. - what unique event is expected to happen; - - what exact conditions such event must carry in its payload; + - what exact conditions the expected event must carry in its payload; NOTE: all attached to a badge template requirements must be fulfilled by default; to achieve "OR" processing logic for 2 attached requirements just group them (put identical group ID). @@ -157,9 +163,7 @@ class BadgeRequirement(models.Model): 'Public signal type. Available events are configured in "BADGES_CONFIG" setting. The crucial aspect for event to carry UserData in its payload.' ), ) - description = models.TextField(null=True, blank=True, help_text=_("Provide more details if needed.")) - group = models.CharField( max_length=255, null=True, @@ -172,31 +176,58 @@ class BadgeRequirement(models.Model): def __str__(self): return f"BadgeRequirement:{self.id}:{self.template.uuid}" + def fulfill(self, username: str): + """ + Marks itself as "done" for the user. + + - notifies about the progression if any; + + Returns: (bool) if progression happened + """ + template_id = self.template.id + progress = BadgeProgress.for_user(username=username, template_id=template_id) + fulfillment, created = Fulfillment.objects.get_or_create(progress=progress, requirement=self) + if created: + notify_requirement_fulfilled( + sender=self, + username=username, + badge_template_id=template_id, + fulfillment_id=fulfillment.id, + ) + return created + def reset(self, username: str): - fulfillments = Fulfillment.objects.filter( + """ + Marks itself as "undone" for the user. + + - removes user progress for the requirement if any; + - notifies about the regression if any; + + Returns: (bool) if any progress existed. + """ + template_id = self.template.id + fulfillment = Fulfillment.objects.filter( requirement=self, progress__username=username, - ) - fulfillments.delete() - BADGE_REQUIREMENT_REGRESSED.send(sender=None, username=username, fulfillments=fulfillments) + ).first() + deleted, __ = fulfillment.delete() + if deleted: + notify_requirement_regressed( + sender=self, + username=username, + badge_template_id=template_id, + fulfillment_id=fulfillment.id, + ) + return bool(deleted) def is_fulfilled(self, username: str) -> bool: return self.fulfillment_set.filter(progress__username=username, progress__template=self.template).exists() - def fulfill(self, username: str): - progress, _ = BadgeProgress.objects.get_or_create(template=self.template, username=username) - fulfillment, _ = Fulfillment.objects.get_or_create(progress=progress, requirement=self) - BADGE_REQUIREMENT_FULFILLED.send(sender=None, username=username, fulfillment=fulfillment) - def apply_rules(self, data: dict) -> bool: - for rule in self.datarule_set.all(): - comparison_func = getattr(operator, rule.operator, None) - if comparison_func: - data_value = str(keypath(data, rule.data_path)) - result = comparison_func(data_value, rule.value) - if not result: - return False - return True + """ + Evaluates payload rules. + """ + return all(rule.apply(data) for rule in self.rules.all()) @property def is_active(self): @@ -237,6 +268,36 @@ class AbstractDataRule(models.Model): class Meta: abstract = True + def apply(self, data: dict) -> bool: + """ + Evaluates itself on the input data (event payload). + + This method retrieves a value specified by a data path within a given dictionary, + converts that value to a string, and then applies a comparison operation against + a predefined value. The comparison operation is determined by the `self.operator` + attribute, which should match the name of an operator function in the `operator` + module. + + Parameters: + - data (dict): A dictionary containing data against which the comparison operation + will be applied. The specific value to be compared is determined by + the `self.data_path` attribute, which specifies the path to the value + within the dictionary. + + Returns: + - bool: True if the rule "worked". + + Example: + Assuming `self.operator` is set to "eq", `self.data_path` is set to "user.age", + and `self.value` is "30", then calling `apply({"user": {"age": 30}})` will return True + because the age matches the specified value. + """ + comparison_func = getattr(operator, self.operator, None) + if comparison_func: + data_value = str(keypath(data, self.data_path)) + return comparison_func(data_value, self.value) + return False + class DataRule(AbstractDataRule): """ @@ -248,6 +309,7 @@ class DataRule(AbstractDataRule): BadgeRequirement, on_delete=models.CASCADE, help_text=_("Parent requirement for this data rule."), + related_name="rules", ) class Meta: @@ -293,15 +355,15 @@ class BadgePenalty(models.Model): description = models.TextField(null=True, blank=True, help_text=_("Provide more details if needed.")) class Meta: - verbose_name_plural = "Badge penalties" + verbose_name_plural = _("Badge penalties") def __str__(self): return f"BadgePenalty:{self.id}:{self.template.uuid}" - class Meta: - verbose_name_plural = "Badge penalties" - def apply_rules(self, data: dict) -> bool: + """ + Evaluates payload rules. + """ return all(rule.apply(data) for rule in self.rules.all()) def reset_requirements(self, username: str): @@ -329,25 +391,15 @@ class PenaltyDataRule(AbstractDataRule): class Meta: unique_together = ("penalty", "data_path", "operator", "value") + def __str__(self): + return f"{self.penalty.template.uuid}:{self.data_path}:{self.operator}:{self.value}" + def save(self, *args, **kwargs): if not is_datapath_valid(self.data_path, self.penalty.event_type): raise ValidationError("Invalid data path for event type") super().save(*args, **kwargs) - def __str__(self): - return f"{self.penalty.template.uuid}:{self.data_path}:{self.operator}:{self.value}" - - def apply(self, data: dict) -> bool: - comparison_func = getattr(operator, self.operator, None) - if comparison_func: - data_value = str(keypath(data, self.data_path)) - return comparison_func(data_value, self.value) - return False - - class Meta: - unique_together = ("penalty", "data_path", "operator", "value") - @property def is_active(self): return self.penalty.template.is_active @@ -358,6 +410,7 @@ class BadgeProgress(models.Model): Tracks a single badge template progress for user. - allows multiple requirements status tracking; + - user-centric; """ credential = models.OneToOneField( @@ -380,10 +433,18 @@ class Meta: def __str__(self): return f"BadgeProgress:{self.username}" + @classmethod + def for_user(cls, *, username, template_id): + """ + Service shortcut. + """ + progress, __ = cls.objects.get_or_create(username=username, template_id=template_id) + return progress + @property def ratio(self) -> float: """ - Calculate badge template progress ratio. + Calculates badge template progress ratio. """ requirements = BadgeRequirement.objects.filter(template=self.template) @@ -407,6 +468,16 @@ def ratio(self) -> float: return 0.00 return round(fulfilled_requirements_count / requirements_count, 2) + def validate(self): + """ + Performs self-check and notifies about the current status. + """ + if self.completed(): + notify_progress_complete(self, self.username, self.template.id) + + if not self.completed(): + notify_progress_incomplete(self, self.username, self.template.id) + def reset(self): Fulfillment.objects.filter(progress=self).delete() diff --git a/credentials/apps/badges/processing/generic.py b/credentials/apps/badges/processing/generic.py index c11b3faf2..0337527ab 100644 --- a/credentials/apps/badges/processing/generic.py +++ b/credentials/apps/badges/processing/generic.py @@ -10,6 +10,7 @@ from credentials.apps.badges.utils import extract_payload, get_user_data from credentials.apps.core.api import get_or_create_user_from_event_data + logger = logging.getLogger(__name__) diff --git a/credentials/apps/badges/processing/progression.py b/credentials/apps/badges/processing/progression.py index 0cd6cfeb1..498e1d7a2 100644 --- a/credentials/apps/badges/processing/progression.py +++ b/credentials/apps/badges/processing/progression.py @@ -2,13 +2,15 @@ Awarding pipeline - badge progression. """ +import logging from typing import List -from openedx_events.learning.signals import BADGE_AWARDED - from credentials.apps.badges.models import BadgeRequirement +logger = logging.getLogger(__name__) + + def discover_requirements(event_type: str) -> List[BadgeRequirement]: """ Picks all relevant requirements based on the event type. @@ -24,6 +26,8 @@ def process_requirements(event_type, username, payload_dict): requirements = discover_requirements(event_type=event_type) completed_templates = set() + logger.debug("BADGES: found %s requirements to process.", len(requirements)) + for requirement in requirements: # ignore: if the badge template wasn't activated yet @@ -45,15 +49,3 @@ def process_requirements(event_type, username, payload_dict): # process: payload rules if requirement.apply_rules(payload_dict): requirement.fulfill(username) - - -def notify_badge_awarded(user_credential): # pylint: disable=unused-argument - """ - Emit public event about badge template completion. - - - username - - badge template ID - """ - - badge_data = user_credential.as_badge_data() - BADGE_AWARDED.send_event(badge=badge_data) diff --git a/credentials/apps/badges/processing/regression.py b/credentials/apps/badges/processing/regression.py index 2e8f5e641..7b8deaac7 100644 --- a/credentials/apps/badges/processing/regression.py +++ b/credentials/apps/badges/processing/regression.py @@ -2,21 +2,23 @@ Revocation pipeline - badge regression. """ -import uuid +import logging from typing import List from openedx_events.learning.data import ( BadgeData, BadgeTemplateData, + CoursePassingStatusData, UserData, UserPersonalData, ) -from openedx_events.learning.data import CoursePassingStatusData -from openedx_events.learning.signals import BADGE_REVOKED +from credentials.apps.badges.models import BadgePenalty, CredlyBadgeTemplate, UserCredential from credentials.apps.badges.signals.signals import BADGE_PROGRESS_INCOMPLETE from credentials.apps.badges.utils import keypath -from credentials.apps.badges.models import BadgePenalty, CredlyBadgeTemplate, UserCredential + + +logger = logging.getLogger(__name__) def discover_penalties(event_type: str) -> List[BadgePenalty]: @@ -40,20 +42,11 @@ def process_penalties(event_type, username, payload_dict): """ penalties = discover_penalties(event_type=event_type) + + logger.debug("BADGES: found %s penalties to process.", len(penalties)) + for penalty in penalties: if not penalty.is_active: continue if penalty.apply_rules(payload_dict): penalty.reset_requirements(username) - - -def notify_badge_revoked(user_credential): # pylint: disable=unused-argument - """ - Emit public event about badge template regression. - - - username - - badge template ID - """ - - badge_data = user_credential.as_badge_data() - BADGE_REVOKED.send_event(badge=badge_data) diff --git a/credentials/apps/badges/signals/handlers.py b/credentials/apps/badges/signals/handlers.py index 0d3867dc7..7d5a151be 100644 --- a/credentials/apps/badges/signals/handlers.py +++ b/credentials/apps/badges/signals/handlers.py @@ -10,6 +10,7 @@ from openedx_events.tooling import OpenEdxPublicSignal, load_all_signals from credentials.apps.badges.issuers import CredlyBadgeTemplateIssuer +from credentials.apps.badges.models import BadgeProgress from credentials.apps.badges.processing.generic import process_event from credentials.apps.badges.signals import ( BADGE_PROGRESS_COMPLETE, @@ -46,29 +47,12 @@ def handle_badging_event(sender, signal, **kwargs): @receiver(BADGE_REQUIREMENT_FULFILLED) -def handle_requirement_fulfilled(sender, username, fulfillment, **kwargs): # pylint: disable=unused-argument - """ - Fires once a single requirement was marked as "done". - """ - if not fulfillment.progress.completed(): - BADGE_PROGRESS_COMPLETE.send( - sender=None, - username=username, - badge_template_id=fulfillment.progress.template.id, - ) - - @receiver(BADGE_REQUIREMENT_REGRESSED) -def handle_requirement_regressed(sender, username, fulfillments, **kwargs): # pylint: disable=unused-argument +def handle_requirement_regressed(sender, username, **kwargs): # pylint: disable=unused-argument """ - Fires once a single requirement for a badge template was marked as "done". + Revises user's progress status each time relevant requirement's progress was updated (+/-). """ - for fulfillment in fulfillments: - BADGE_PROGRESS_INCOMPLETE.send( - sender=None, - username=username, - badge_template_id=fulfillment.progress.template.id, - ) + BadgeProgress.for_user(username=username, template_id=sender.template.id).validate() @receiver(BADGE_PROGRESS_COMPLETE) diff --git a/credentials/apps/badges/signals/signals.py b/credentials/apps/badges/signals/signals.py index 8eb21b258..6494d05cc 100644 --- a/credentials/apps/badges/signals/signals.py +++ b/credentials/apps/badges/signals/signals.py @@ -3,6 +3,7 @@ """ from django.dispatch import Signal +from openedx_events.learning.signals import BADGE_AWARDED, BADGE_REVOKED # a single requirements for a badge template was finished @@ -16,3 +17,64 @@ # badge template penalty reset some of fulfilled requirements, so badge template became incomplete BADGE_PROGRESS_INCOMPLETE = Signal() + + +def notify_requirement_fulfilled(*, sender, username, badge_template_id, **kwargs): + """ + FIXME: + """ + BADGE_REQUIREMENT_FULFILLED.send( + sender=sender, + username=username, + badge_template_id=badge_template_id, + ) + + +def notify_requirement_regressed(*, sender, username, badge_template_id): + """ + FIXME: implement/use + """ + + +def notify_progress_complete(sender, username, badge_template_id): + """ + FIXME: + """ + BADGE_PROGRESS_COMPLETE.send( + sender=sender, + username=username, + badge_template_id=badge_template_id, + ) + + +def notify_progress_incomplete(sender, username, badge_template_id): + """ + FIXME: implement/use + """ + BADGE_PROGRESS_INCOMPLETE.send( + sender=sender, + username=username, + badge_template_id=badge_template_id, + ) + + +def notify_badge_awarded(user_credential): # pylint: disable=unused-argument + """ + Emit public event about badge template completion. + + - username + - badge template ID + """ + + BADGE_AWARDED.send_event(badge=user_credential.as_badge_data()) + + +def notify_badge_revoked(user_credential): # pylint: disable=unused-argument + """ + Emit public event about badge template regression. + + - username + - badge template ID + """ + + BADGE_REVOKED.send_event(badge=user_credential.as_badge_data())