From 0cd1d8a226e0be4e47d7c3dd8d8ab0bcb2535950 Mon Sep 17 00:00:00 2001 From: luisfelipec95 Date: Mon, 1 Jun 2026 11:15:59 -0500 Subject: [PATCH 1/4] refactor(badges): prevent orphaned BadgeProgress creation --- credentials/apps/badges/models.py | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/credentials/apps/badges/models.py b/credentials/apps/badges/models.py index 5c21fb128..50dbaccf2 100644 --- a/credentials/apps/badges/models.py +++ b/credentials/apps/badges/models.py @@ -126,7 +126,15 @@ def user_progress(self, username: str) -> float: """ Determines a completion progress for user. """ - progress = BadgeProgress.for_user(username=username, template_id=self.id) + progress = BadgeProgress.for_user( + username=username, + template_id=self.id, + create=False + ) + + if not progress: + return 0.00 + return progress.ratio def is_completed(self, username: str) -> bool: @@ -274,7 +282,15 @@ def is_group_fulfilled(cls, *, group: str, template: BadgeTemplate, username: st Checks if the group is fulfilled. """ - progress = BadgeProgress.for_user(username=username, template_id=template.id) + progress = BadgeProgress.for_user( + username=username, + template_id=template.id, + create=False + ) + + if not progress: + return False + requirements = cls.objects.filter(template=template, blend=group) fulfilled_requirements = requirements.filter(fulfillments__progress=progress).count() @@ -509,13 +525,16 @@ def __str__(self): return f"BadgeProgress:{self.username}" @classmethod - def for_user(cls, *, username, template_id): + def for_user(cls, *, username, template_id, create=True): """ Service shortcut. """ - progress, __ = cls.objects.get_or_create(username=username, template_id=template_id) - return progress + if create: + progress, __ = cls.objects.get_or_create(username=username, template_id=template_id) + return progress + else: + return cls.objects.filter(username=username, template_id=template_id).first() @property def ratio(self) -> float: From b37e9fc819d17efff3bacee2c78cebfd1de7b481 Mon Sep 17 00:00:00 2001 From: luisfelipec95 Date: Fri, 19 Jun 2026 09:08:06 -0500 Subject: [PATCH 2/4] chore: improvements for the for_user method --- credentials/apps/badges/models.py | 35 +++++++++++---------- credentials/apps/badges/signals/handlers.py | 4 +-- 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/credentials/apps/badges/models.py b/credentials/apps/badges/models.py index 50dbaccf2..860549b84 100644 --- a/credentials/apps/badges/models.py +++ b/credentials/apps/badges/models.py @@ -126,11 +126,7 @@ def user_progress(self, username: str) -> float: """ Determines a completion progress for user. """ - progress = BadgeProgress.for_user( - username=username, - template_id=self.id, - create=False - ) + progress = BadgeProgress.for_user(username=username, template_id=self.id) if not progress: return 0.00 @@ -223,7 +219,7 @@ def fulfill(self, username: str): Returns: (bool) if progression happened """ template_id = self.template.id - progress = BadgeProgress.for_user(username=username, template_id=template_id) + progress = BadgeProgress.for_user(username=username, template_id=template_id, create_if_absent=True) fulfillment, created = Fulfillment.objects.get_or_create(progress=progress, requirement=self, blend=self.blend) if created: @@ -282,11 +278,7 @@ def is_group_fulfilled(cls, *, group: str, template: BadgeTemplate, username: st Checks if the group is fulfilled. """ - progress = BadgeProgress.for_user( - username=username, - template_id=template.id, - create=False - ) + progress = BadgeProgress.for_user(username=username, template_id=template.id) if not progress: return False @@ -525,16 +517,27 @@ def __str__(self): return f"BadgeProgress:{self.username}" @classmethod - def for_user(cls, *, username, template_id, create=True): + def for_user(cls, *, username, template_id, create_if_absent=False): """ - Service shortcut. + Retrieve or create a BadgeProgress record for a user and template. + + This method follows a lazy-load pattern to control when BadgeProgress records + are created. Use create_if_absent=False for read-only operations to avoid + creating orphaned records. + + Args: + username: The username of the user to get or create progress for. + template_id: The ID of the BadgeTemplate to track progress for. + create_if_absent: Whether to create a new record if one doesn't exist. + - If True: Creates a new BadgeProgress record if needed + - If False: Returns None if the record doesn't exist, without creating """ - if create: + if create_if_absent: progress, __ = cls.objects.get_or_create(username=username, template_id=template_id) return progress - else: - return cls.objects.filter(username=username, template_id=template_id).first() + + return cls.objects.filter(username=username, template_id=template_id).first() @property def ratio(self) -> float: diff --git a/credentials/apps/badges/signals/handlers.py b/credentials/apps/badges/signals/handlers.py index 70418c405..644079c6d 100644 --- a/credentials/apps/badges/signals/handlers.py +++ b/credentials/apps/badges/signals/handlers.py @@ -50,7 +50,7 @@ def handle_requirement_fulfilled(sender, username, **kwargs): """ On user's Badge progression (completion). """ - BadgeProgress.for_user(username=username, template_id=sender.template.id).progress() + BadgeProgress.for_user(username=username, template_id=sender.template.id, create_if_absent=True).progress() @receiver(BADGE_REQUIREMENT_REGRESSED) @@ -58,7 +58,7 @@ def handle_requirement_regressed(sender, username, **kwargs): """ On user's Badge regression (incompletion). """ - BadgeProgress.for_user(username=username, template_id=sender.template.id).regress() + BadgeProgress.for_user(username=username, template_id=sender.template.id, create_if_absent=True).regress() @receiver(BADGE_PROGRESS_COMPLETE) From b9aa5070f22cd0240c86ab3f4cd3006d5187ba05 Mon Sep 17 00:00:00 2001 From: luisfelipec95 Date: Fri, 19 Jun 2026 09:18:39 -0500 Subject: [PATCH 3/4] chore: update test cases --- .../apps/badges/tests/test_services.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/credentials/apps/badges/tests/test_services.py b/credentials/apps/badges/tests/test_services.py index b6f91e8c8..5646ca0d7 100644 --- a/credentials/apps/badges/tests/test_services.py +++ b/credentials/apps/badges/tests/test_services.py @@ -421,7 +421,7 @@ def test_course_a_completion(self): ) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_course_a_or_b_completion(self): """ @@ -457,7 +457,7 @@ def test_course_a_or_b_completion(self): process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_course_a_or_b_or_c_completion(self): """ @@ -506,7 +506,7 @@ def test_course_a_or_b_or_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 0) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_course_a_or_completion(self): """ @@ -529,7 +529,7 @@ def test_course_a_or_completion(self): ) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_course_a_or_b_and_c_completion(self): """ @@ -572,7 +572,7 @@ def test_course_a_or_b_and_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) DataRule.objects.create( requirement=requirement_b, @@ -587,7 +587,7 @@ def test_course_a_or_b_and_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_course_a_or_b_and_c_or_d_completion(self): """ @@ -639,7 +639,7 @@ def test_course_a_or_b_and_c_or_d_completion(self): value="D", ) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) @@ -647,7 +647,7 @@ def test_course_a_or_b_and_c_or_d_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_d).count(), 0) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) DataRule.objects.create( requirement=requirement_c, @@ -661,7 +661,7 @@ def test_course_a_or_b_and_c_or_d_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) 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) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) class TestIdentifyUser(TestCase): @@ -720,7 +720,7 @@ def setUp(self): def test_process_event_passing(self): event_payload = COURSE_PASSING_DATA process_event(sender=self.sender, kwargs=event_payload) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) def test_process_event_not_passing(self): event_payload = CoursePassingStatusData( @@ -733,7 +733,7 @@ def test_process_event_not_passing(self): ), ) process_event(sender=self.sender, kwargs=event_payload) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id).completed) + self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) @patch.object(BadgeProgress, "regress", mock_progress_regress) def test_process_event_not_found(self): From b038cec417d5d73f75ff5473d7a37f9891359019 Mon Sep 17 00:00:00 2001 From: luisfelipec95 Date: Fri, 19 Jun 2026 09:40:14 -0500 Subject: [PATCH 4/4] chore: fix CI lint issues --- credentials/apps/badges/models.py | 10 +-- .../apps/badges/tests/test_services.py | 66 +++++++++++++++---- 2 files changed, 60 insertions(+), 16 deletions(-) diff --git a/credentials/apps/badges/models.py b/credentials/apps/badges/models.py index 860549b84..5fd76577d 100644 --- a/credentials/apps/badges/models.py +++ b/credentials/apps/badges/models.py @@ -127,7 +127,7 @@ def user_progress(self, username: str) -> float: Determines a completion progress for user. """ progress = BadgeProgress.for_user(username=username, template_id=self.id) - + if not progress: return 0.00 @@ -279,7 +279,7 @@ def is_group_fulfilled(cls, *, group: str, template: BadgeTemplate, username: st """ progress = BadgeProgress.for_user(username=username, template_id=template.id) - + if not progress: return False @@ -520,11 +520,11 @@ def __str__(self): def for_user(cls, *, username, template_id, create_if_absent=False): """ Retrieve or create a BadgeProgress record for a user and template. - + This method follows a lazy-load pattern to control when BadgeProgress records are created. Use create_if_absent=False for read-only operations to avoid creating orphaned records. - + Args: username: The username of the user to get or create progress for. template_id: The ID of the BadgeTemplate to track progress for. @@ -536,7 +536,7 @@ def for_user(cls, *, username, template_id, create_if_absent=False): if create_if_absent: progress, __ = cls.objects.get_or_create(username=username, template_id=template_id) return progress - + return cls.objects.filter(username=username, template_id=template_id).first() @property diff --git a/credentials/apps/badges/tests/test_services.py b/credentials/apps/badges/tests/test_services.py index 5646ca0d7..ac4bd82f9 100644 --- a/credentials/apps/badges/tests/test_services.py +++ b/credentials/apps/badges/tests/test_services.py @@ -421,7 +421,11 @@ def test_course_a_completion(self): ) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_course_a_or_b_completion(self): """ @@ -457,7 +461,11 @@ def test_course_a_or_b_completion(self): process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_course_a_or_b_or_c_completion(self): """ @@ -506,7 +514,11 @@ def test_course_a_or_b_or_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 0) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_course_a_or_completion(self): """ @@ -529,7 +541,11 @@ def test_course_a_or_completion(self): ) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) self.assertEqual(Fulfillment.objects.filter(requirement=requirement).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_course_a_or_b_and_c_completion(self): """ @@ -572,7 +588,11 @@ def test_course_a_or_b_and_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_a).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertFalse( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) DataRule.objects.create( requirement=requirement_b, @@ -587,7 +607,11 @@ def test_course_a_or_b_and_c_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_course_a_or_b_and_c_or_d_completion(self): """ @@ -639,7 +663,11 @@ def test_course_a_or_b_and_c_or_d_completion(self): value="D", ) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertFalse( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) process_requirements(COURSE_PASSING_EVENT, "test_username", COURSE_PASSING_DATA) @@ -647,7 +675,11 @@ def test_course_a_or_b_and_c_or_d_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_d).count(), 0) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertFalse( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) DataRule.objects.create( requirement=requirement_c, @@ -661,7 +693,11 @@ def test_course_a_or_b_and_c_or_d_completion(self): self.assertEqual(Fulfillment.objects.filter(requirement=requirement_b).count(), 0) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_c).count(), 1) self.assertEqual(Fulfillment.objects.filter(requirement=requirement_d).count(), 0) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) class TestIdentifyUser(TestCase): @@ -720,7 +756,11 @@ def setUp(self): def test_process_event_passing(self): event_payload = COURSE_PASSING_DATA process_event(sender=self.sender, kwargs=event_payload) - self.assertTrue(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertTrue( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) def test_process_event_not_passing(self): event_payload = CoursePassingStatusData( @@ -733,7 +773,11 @@ def test_process_event_not_passing(self): ), ) process_event(sender=self.sender, kwargs=event_payload) - self.assertFalse(BadgeProgress.for_user(username="test_username", template_id=self.badge_template.id, create_if_absent=True).completed) + self.assertFalse( + BadgeProgress.for_user( + username="test_username", template_id=self.badge_template.id, create_if_absent=True + ).completed + ) @patch.object(BadgeProgress, "regress", mock_progress_regress) def test_process_event_not_found(self):