Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [ACI-975] enable + upgrade penalties processing #168

Merged
merged 1 commit into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions credentials/apps/badges/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
CredlyOrganizationAdminForm,
DataRuleForm,
DataRuleFormSet,
PenaltyDataRuleForm,
PenaltyDataRuleFormSet,
)

from credentials.apps.badges.models import (
Expand Down Expand Up @@ -217,8 +219,7 @@ class CredlyBadgeTemplateAdmin(admin.ModelAdmin):
)
inlines = [
BadgeRequirementInline,
# FIXME: disable until "Release V"
# BadgePenaltyInline,
BadgePenaltyInline,
]

def has_add_permission(self, request):
Expand Down Expand Up @@ -277,6 +278,8 @@ def save_formset(self, request, form, formset, change): # pylint: disable=unuse
class DataRulePenaltyInline(admin.TabularInline):
model = PenaltyDataRule
extra = 0
form = PenaltyDataRuleForm
formset = PenaltyDataRuleFormSet


class BadgeRequirementAdmin(admin.ModelAdmin):
Expand Down Expand Up @@ -362,7 +365,8 @@ def has_add_permission(self, request):

def formfield_for_manytomany(self, db_field, request, **kwargs):
if db_field.name == "requirements":
template_id = request.resolver_match.kwargs.get("object_id")
object_id = request.resolver_match.kwargs.get("object_id")
template_id = self.get_object(request, object_id).template_id
if template_id:
kwargs["queryset"] = BadgeRequirement.objects.filter(template_id=template_id)
return super().formfield_for_manytomany(db_field, request, **kwargs)
Expand Down Expand Up @@ -463,6 +467,5 @@ def has_add_permission(self, request):
admin.site.register(CredlyBadgeTemplate, CredlyBadgeTemplateAdmin)
admin.site.register(CredlyBadge, CredlyBadgeAdmin)
admin.site.register(BadgeRequirement, BadgeRequirementAdmin)
# FIXME: disable until "Release V"
# admin.site.register(BadgePenalty, BadgePenaltyAdmin)
admin.site.register(BadgePenalty, BadgePenaltyAdmin)
admin.site.register(BadgeProgress, BadgeProgressAdmin)
41 changes: 39 additions & 2 deletions credentials/apps/badges/admin_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from credentials.apps.badges.credly.api_client import CredlyAPIClient
from credentials.apps.badges.credly.exceptions import CredlyAPIError
from credentials.apps.badges.models import BadgePenalty, BadgeRequirement, CredlyOrganization, DataRule
from credentials.apps.badges.models import BadgePenalty, BadgeRequirement, CredlyOrganization, DataRule, PenaltyDataRule
from credentials.apps.badges.utils import get_event_type_keypaths


Expand Down Expand Up @@ -141,4 +141,41 @@ def __init__(self, *args, parent_instance=None, **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["group"].initial = chr(65 + self.template.requirements.count())


class PenaltyDataRuleFormSet(forms.BaseInlineFormSet):
"""
Formset for PenaltyDataRule model.
"""
def get_form_kwargs(self, index):
"""
Pass parent instance to the form.
"""

kwargs = super().get_form_kwargs(index)
kwargs["parent_instance"] = self.instance
return kwargs


class PenaltyDataRuleForm(forms.ModelForm):
"""
Form for PenaltyDataRule model.
"""

class Meta:
model = PenaltyDataRule
fields = "__all__"

data_path = forms.ChoiceField()

def __init__(self, *args, parent_instance=None, **kwargs):
"""
Load data paths based on the parent instance event type.
"""
self.parent_instance = parent_instance
super().__init__(*args, **kwargs)

if self.parent_instance:
event_type = self.parent_instance.event_type
self.fields["data_path"].choices = Choices(*get_event_type_keypaths(event_type=event_type))
2 changes: 1 addition & 1 deletion credentials/apps/badges/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ def apply_rules(self, data: dict) -> bool:
Evaluates payload rules.
"""

return all(rule.apply(data) for rule in self.rules.all())
return all(rule.apply(data) for rule in self.rules.all()) if self.rules.exists() else False

def reset_requirements(self, username: str):
"""
Expand Down
3 changes: 1 addition & 2 deletions credentials/apps/badges/processing/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,7 @@ def process_event(sender, **kwargs):
process_requirements(event_type, username, extract_payload(kwargs))

# penalties processing
# FIXME: disable until "Release V"
# process_penalties(event_type, username, extract_payload(kwargs))
process_penalties(event_type, username, extract_payload(kwargs))

except StopEventProcessing:
# controlled processing dropping
Expand Down
142 changes: 131 additions & 11 deletions credentials/apps/badges/tests/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

COURSE_PASSING_EVENT = "org.openedx.learning.course.passing.status.updated.v1"
COURSE_PASSING_DATA = CoursePassingStatusData(
status="passing",
is_passing=True,
course=CourseData(course_key=CourseKey.from_string("course-v1:edX+DemoX.1+2014"), display_name="A"),
user=UserData(
id=1,
Expand Down Expand Up @@ -133,7 +133,7 @@ def setUp(self):
)
self.site = Site.objects.create(domain="test_domain", name="test_name")
self.badge_template = BadgeTemplate.objects.create(
uuid=uuid.uuid4(), name="test_template", state="draft", site=self.site
uuid=uuid.uuid4(), name="test_template", state="draft", site=self.site, is_active=True
)

self.CCX_COURSE_PASSING_EVENT = "org.openedx.learning.ccx.course.passing.status.updated.v1"
Expand All @@ -151,15 +151,15 @@ def test_process_penalties_all_datarules_success(self):
)
DataRule.objects.create(
requirement=requirement1,
data_path="course_passing_status.user.pii.username",
data_path="course.display_name",
operator="eq",
value="test_username",
value="A",
)
DataRule.objects.create(
requirement=requirement2,
data_path="course_passing_status.user.pii.email",
operator="eq",
value="test_email",
data_path="course.display_name",
operator="ne",
value="B",
)

progress = BadgeProgress.objects.create(username="test_username", template=self.badge_template)
Expand Down Expand Up @@ -199,15 +199,15 @@ def test_process_penalties_one_datarule_fail(self):
)
DataRule.objects.create(
requirement=requirement1,
data_path="course_passing_status.user.pii.username",
data_path="course.display_name",
operator="eq",
value="test_username",
value="A",
)
DataRule.objects.create(
requirement=requirement2,
data_path="course_passing_status.user.pii.email",
data_path="course.display_name",
operator="eq",
value="test_email",
value="B",
)

progress = BadgeProgress.objects.create(username="test_username")
Expand Down Expand Up @@ -239,6 +239,126 @@ def test_process_penalties_one_datarule_fail(self):
process_penalties(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 2)

def test_process_single_requirement_penalty(self):
requirement = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
)
DataRule.objects.create(
requirement=requirement,
data_path="course.display_name",
operator="ne",
value="B",
)
progress = BadgeProgress.objects.create(username="test_username", template=self.badge_template)
Fulfillment.objects.create(progress=progress, requirement=requirement)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 1)

penalty = BadgePenalty.objects.create(template=self.badge_template, event_type=COURSE_PASSING_EVENT)
penalty.requirements.add(requirement)
PenaltyDataRule.objects.create(
penalty=penalty,
data_path="course.display_name",
operator="eq",
value="A",
)
process_penalties(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 0)

def test_process_one_of_grouped_requirements_penalty(self):
requirement_a = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
group="a_or_b",
)
requirement_b = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
group="a_or_b",
)
DataRule.objects.create(
requirement=requirement_a,
data_path="course.display_name",
wowkalucky marked this conversation as resolved.
Show resolved Hide resolved
operator="eq",
value="A",
)
DataRule.objects.create(
requirement=requirement_b,
data_path="course.display_name",
operator="eq",
value="A",
)
progress = BadgeProgress.objects.create(username="test_username", template=self.badge_template)
Fulfillment.objects.create(progress=progress, requirement=requirement_a)
Fulfillment.objects.create(progress=progress, requirement=requirement_b)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 2)

penalty = BadgePenalty.objects.create(template=self.badge_template, event_type=COURSE_PASSING_EVENT)
penalty.requirements.add(requirement_b)
PenaltyDataRule.objects.create(
penalty=penalty,
data_path="course.display_name",
operator="ne",
value="B",
)
process_penalties(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 1)

def test_process_mixed_penalty(self):
requirement_a = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
group="a_or_b",
)
requirement_b = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
group="a_or_b",
)
requirement_c = BadgeRequirement.objects.create(
template=self.badge_template,
event_type=COURSE_PASSING_EVENT,
description="Test course passing award description",
)
DataRule.objects.create(
requirement=requirement_a,
data_path="course.display_name",
operator="eq",
value="A",
)
DataRule.objects.create(
requirement=requirement_b,
data_path="course.display_name",
operator="eq",
value="B",
)
DataRule.objects.create(
requirement=requirement_c,
data_path="course.display_name",
operator="ne",
value="C",
)
progress = BadgeProgress.objects.create(username="test_username", template=self.badge_template)
Fulfillment.objects.create(progress=progress, requirement=requirement_a)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fulfillment now expects group attribute.

Fulfillment.objects.create(progress=progress, requirement=requirement_c)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 2)

penalty = BadgePenalty.objects.create(template=self.badge_template, event_type=COURSE_PASSING_EVENT)
penalty.requirements.add(requirement_a, requirement_c)
PenaltyDataRule.objects.create(
penalty=penalty,
data_path="course.display_name",
operator="ne",
value="B",
)
process_penalties(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA)
self.assertEqual(Fulfillment.objects.filter(progress=progress).count(), 0)


class TestProcessRequirements(TestCase):
def setUp(self):
Expand Down
4 changes: 2 additions & 2 deletions credentials/apps/badges/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def setUp(self):

self.passing_status_1 = {
"course_passing_status": CoursePassingStatusData(
status=CoursePassingStatusData.PASSING, course=self.course_data_1, user=self.user_data_1
is_passing=True, course=self.course_data_1, user=self.user_data_1
)
}

Expand Down Expand Up @@ -99,7 +99,7 @@ def test_extract_payload(self):
id=1, is_active=True, pii=UserPersonalData(username="user1", email="[email protected] ", name="John Doe")
)
course_passing_status = CoursePassingStatusData(
status=CoursePassingStatusData.PASSING, course=self.course_data, user=user_data
is_passing=True, course=self.course_data, user=user_data
)
public_signal_kwargs = {"course_passing_status": course_passing_status}
result = extract_payload(public_signal_kwargs)
Expand Down
Loading