Skip to content

Commit

Permalink
Ensure badge/parent-child consistency on badge edit without need to s…
Browse files Browse the repository at this point in the history
…ync (barring adding/deleting badges implied by non-child descendent relationship)
  • Loading branch information
julianweng committed Feb 19, 2025
1 parent d4fe362 commit 7134faf
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 35 deletions.
83 changes: 51 additions & 32 deletions backend/clubs/management/commands/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class Command(BaseCommand):
"There should be no issues with repeatedly running this script. "
)
web_execute = True
dry_run = False # Initialize for usage as standalone utility

def add_arguments(self, parser):
parser.add_argument(
Expand Down Expand Up @@ -95,43 +96,61 @@ def sync_club_fairs(self):
else:
self.stdout.write(f"Would have deleted {len(dups)} duplicate entries!")

def sync_badges(self):
def sync_badge(self, badge: Badge):
"""
Synchronizes badges based on parent child relationships.
Tends to favor adding objects to fix relationships instead of removing them.
Synchronizes a badge with its parent child relationships.
If a club has one of them (badge, parent), add the other.
"""
# add badges to parent child relationships
count = 0
for badge in Badge.objects.all():
if badge.org is not None:
self._visited = set()
count += self.recursively_add_badge(badge.org, badge)
self.stdout.write(
self.style.SUCCESS(f"Modified {count} club badge relationships.")
)
if badge.org is None:
return 0, 0

Check warning on line 105 in backend/clubs/management/commands/sync.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/management/commands/sync.py#L105

Added line #L105 was not covered by tests

# add badge to parent child relationships
badge_add_count = 0
parent_child_add_count = 0
self._visited = set()
badge_add_count += self.recursively_add_badge(badge.org, badge)

# if badge exist on child, link it to the parent directly
# unless it is already indirectly linked
count = 0
for club in badge.club_set.all():
if club.pk == badge.org.pk:
continue

parent_club_codes = self.get_parent_club_codes(club)
if badge.org.code not in parent_club_codes:
if not self.dry_run:
self.stdout.write(
f"Adding {badge.org.name} as parent for {club.name}."
)
club.parent_orgs.add(badge.org)
else:
self.stdout.write(

Check warning on line 127 in backend/clubs/management/commands/sync.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/management/commands/sync.py#L127

Added line #L127 was not covered by tests
f"Would have added {badge.org.name} "
f"as a parent for {club.name}."
)
parent_child_add_count += 1
return badge_add_count, parent_child_add_count

def sync_badges(self):
"""
Synchronizes badges based on parent child relationships.
Uses parent child relationships as source of truth.
"""
# remove badges with orgs from all clubs
for club in Club.objects.all():
club.badges = club.badges.filter(org__isnull=False)
club.save()

Check warning on line 142 in backend/clubs/management/commands/sync.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/management/commands/sync.py#L140-L142

Added lines #L140 - L142 were not covered by tests

badge_add_count = 0
parent_child_add_count = 0

Check warning on line 145 in backend/clubs/management/commands/sync.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/management/commands/sync.py#L144-L145

Added lines #L144 - L145 were not covered by tests
for badge in Badge.objects.all():
if badge.org is not None:
for club in badge.club_set.all():
if club.pk == badge.org.pk:
continue

parent_club_codes = self.get_parent_club_codes(club)
if badge.org.code not in parent_club_codes:
if not self.dry_run:
self.stdout.write(
f"Adding {badge.org.name} as parent for {club.name}."
)
club.parent_orgs.add(badge.org)
else:
self.stdout.write(
f"Would have added {badge.org.name} "
f"as a parent for {club.name}."
)
count += 1
b_count, pc_count = self.sync_badge(badge)
badge_add_count += b_count
parent_child_add_count += pc_count

Check warning on line 149 in backend/clubs/management/commands/sync.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/management/commands/sync.py#L147-L149

Added lines #L147 - L149 were not covered by tests

self.stdout.write(
self.style.SUCCESS(f"Modified {count} parent child relationships.")
self.style.SUCCESS(
f"Modified {badge_add_count} club badge relationships."
f"Modified {parent_child_add_count} parent child relationships."
)
)
6 changes: 5 additions & 1 deletion backend/clubs/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1489,7 +1489,11 @@ class Badge(models.Model):

# The organization that this badge represents (If this is the "SAC Funded" badge,
# then this would link to SAC)
org = models.ForeignKey(Club, on_delete=models.CASCADE, blank=True, null=True)
# TODO: Editability can be loosened if correspondance betweent parent_orgs, badges
# of clubs with the badge is maintained when modifying this field
org = models.ForeignKey(
Club, on_delete=models.CASCADE, blank=True, null=True, editable=False
)

# The fair that this badge is related to
fair = models.ForeignKey(ClubFair, on_delete=models.CASCADE, blank=True, null=True)
Expand Down
4 changes: 4 additions & 0 deletions backend/clubs/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
from tatsu.exceptions import FailedParse

from clubs.filters import RandomOrderingFilter, RandomPageNumberPagination
from clubs.management.commands.sync import Command as SyncCommand
from clubs.mixins import XLSXFormatterMixin
from clubs.models import (
AdminNote,
Expand Down Expand Up @@ -7539,12 +7540,15 @@ def create(self, request, *args, **kwargs):
badge = get_object_or_404(Badge, pk=self.kwargs["badge_pk"])
club = get_object_or_404(Club, code=request.data["club"])
club.badges.add(badge)
SyncCommand().sync_badge(badge)
return Response({"success": True})

def destroy(self, request, *args, **kwargs):
club = self.get_object()
badge = get_object_or_404(Badge, pk=self.kwargs["badge_pk"])
club.badges.remove(badge)
club.parent_orgs.remove(badge.org)
SyncCommand().sync_badge(badge)

Check warning on line 7551 in backend/clubs/views.py

View check run for this annotation

Codecov / codecov/patch

backend/clubs/views.py#L7550-L7551

Added lines #L7550 - L7551 were not covered by tests
return Response({"success": True})

def get_queryset(self):
Expand Down
16 changes: 14 additions & 2 deletions backend/tests/clubs/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -1617,8 +1617,20 @@ def test_club_modify_child(self):
self.assertIn(resp.status_code, [400, 403], resp.content)

# assert that we can modify a child club
child_club.badges.add(self.wc_badge)
call_command("sync")
self.client.login(username=self.user5.username, password="test")
resp = self.client.post(
reverse("badge-clubs-list", args=(self.wc_badge.id,)),
{"club": child_club.code},
content_type="application/json",
)
self.assertIn(resp.status_code, [200, 201], resp.content)
child_club.refresh_from_db()
self.assertEqual(child_club.badges.count(), 1)
self.assertEqual(child_club.badges.all()[0], self.wc_badge)
self.assertEqual(child_club.parent_orgs.count(), 1)
self.assertEqual(child_club.parent_orgs.all()[0], self.wc)

self.client.login(username=self.user4.username, password="test")
resp = self.client.patch(
reverse("clubs-detail", args=(child_club.code,)),
{"description": "We hate Wharton"},
Expand Down

0 comments on commit 7134faf

Please sign in to comment.