Skip to content

Commit d11dbec

Browse files
committed
Fix code review issues and improve robustness
1 parent 5292f3b commit d11dbec

File tree

3 files changed

+68
-31
lines changed

3 files changed

+68
-31
lines changed

backend/apps/nest/management/commands/nest_update_badges.py

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,14 @@ def update_owasp_staff_badge(self):
4343
self.stdout.write(f"Created badge: {badge.name}")
4444

4545
# Assign badge to employees who don't have it.
46-
employees_without_badge = User.objects.filter(is_owasp_staff=True).exclude(
47-
badges__badge=badge
46+
employees_missing_or_inactive = User.objects.filter(is_owasp_staff=True).exclude(
47+
badges__badge=badge, badges__is_active=True
4848
)
49-
count = employees_without_badge.count()
49+
count = employees_missing_or_inactive.count()
5050

5151
if count:
52-
for user in employees_without_badge:
53-
user_badge, created = UserBadge.objects.get_or_create(user=user, badge=badge)
52+
for user in employees_missing_or_inactive:
53+
user_badge, _ = UserBadge.objects.get_or_create(user=user, badge=badge)
5454
if not user_badge.is_active:
5555
user_badge.is_active = True
5656
user_badge.save(update_fields=["is_active"])
@@ -96,12 +96,14 @@ def update_waspy_award_badge(self):
9696
).distinct()
9797

9898
# Assign badge to WASPY award winners who don't have it
99-
users_without_badge = waspy_award_users.exclude(badges__badge=badge)
100-
count = users_without_badge.count()
99+
users_missing_or_inactive = waspy_award_users.exclude(
100+
badges__badge=badge, badges__is_active=True
101+
)
102+
count = users_missing_or_inactive.count()
101103

102104
if count:
103-
for user in users_without_badge:
104-
user_badge, created = UserBadge.objects.get_or_create(user=user, badge=badge)
105+
for user in users_missing_or_inactive:
106+
user_badge, _ = UserBadge.objects.get_or_create(user=user, badge=badge)
105107
if not user_badge.is_active:
106108
user_badge.is_active = True
107109
user_badge.save(update_fields=["is_active"])

backend/apps/owasp/management/commands/owasp_sync_awards.py

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,21 @@ def handle(self, *args, **kwargs) -> None:
1919
"""Handle the command execution."""
2020
self.stdout.write("Syncing OWASP awards...")
2121

22-
data = yaml.safe_load(
23-
get_repository_file_content(
24-
"https://raw.githubusercontent.com/OWASP/owasp.github.io/main/_data/awards.yml"
22+
url = "https://raw.githubusercontent.com/OWASP/owasp.github.io/main/_data/awards.yml"
23+
raw = get_repository_file_content(url)
24+
if not raw:
25+
self.stderr.write(self.style.WARNING("No awards data fetched; aborting."))
26+
return
27+
try:
28+
data = yaml.safe_load(raw) or []
29+
except yaml.YAMLError as e:
30+
self.stderr.write(self.style.ERROR(f"Failed to parse awards YAML: {e}"))
31+
return
32+
if not isinstance(data, list):
33+
self.stderr.write(
34+
self.style.WARNING("Unexpected awards YAML structure; expected a list.")
2535
)
26-
)
36+
return
2737

2838
awards_to_save = []
2939
for item in data:
@@ -34,40 +44,59 @@ def handle(self, *args, **kwargs) -> None:
3444
if award:
3545
awards_to_save.append(award)
3646

37-
Award.bulk_save(awards_to_save)
47+
Award.bulk_save(awards_to_save, fields=("category", "description", "year", "user"))
3848
self.stdout.write(self.style.SUCCESS(f"Successfully synced {len(awards_to_save)} awards"))
3949

4050
def _create_or_update_award(self, award_data, winner_data):
4151
"""Create or update award instance."""
42-
name = f"{award_data['title']} - {winner_data['name']} ({award_data['year']})"
52+
# Safely extract values with defaults
53+
title = award_data.get("title", "")
54+
year = award_data.get("year", 0)
55+
category = award_data.get("category", "")
56+
57+
# Handle winner_data being string or dict
58+
if isinstance(winner_data, str):
59+
winner_name = winner_data
60+
winner_info = ""
61+
else:
62+
winner_name = winner_data.get("name", "")
63+
winner_info = winner_data.get("info", "")
64+
65+
name = f"{title} - {winner_name} ({year})"
4366

4467
try:
4568
award = Award.objects.get(name=name)
4669
except Award.DoesNotExist:
4770
award = Award(name=name)
4871

49-
award.category = award_data.get("category", "")
50-
award.description = winner_data.get("info", "")
51-
award.year = award_data.get("year", 0)
72+
award.category = category
73+
award.description = winner_info
74+
award.year = year
5275

53-
# Try to match user by name
54-
user = self._match_user(winner_data["name"])
55-
if user:
56-
award.user = user
57-
else:
58-
logger.warning("Could not match user for award winner: %s", winner_data["name"])
76+
# Only set user if not already reviewed
77+
if not (award.user and award.is_reviewed):
78+
user = self._match_user(winner_name)
79+
if user:
80+
award.user = user
81+
else:
82+
logger.warning("Could not match user for award winner: %s", winner_name)
5983

6084
return award
6185

6286
def _match_user(self, winner_name):
6387
"""Try to match award winner with existing user."""
64-
# Try exact name match first
65-
user = User.objects.filter(name__iexact=winner_name).first()
66-
if user:
67-
return user
88+
winner_name = winner_name.strip()
89+
90+
# Check if it looks like a GitHub handle
91+
if winner_name.startswith("@") or (" " not in winner_name and winner_name):
92+
# Strip leading @ and try login match first
93+
login_name = winner_name.lstrip("@")
94+
user = User.objects.filter(login__iexact=login_name).first()
95+
if user:
96+
return user
6897

69-
# Try login match (GitHub username)
70-
user = User.objects.filter(login__iexact=winner_name).first()
98+
# Try exact name match
99+
user = User.objects.filter(name__iexact=winner_name).first()
71100
if user:
72101
return user
73102

backend/apps/owasp/models/award.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,16 @@ class Meta:
1515
indexes = [
1616
models.Index(fields=["-year"], name="award_year_desc_idx"),
1717
]
18+
constraints = [
19+
models.UniqueConstraint(
20+
fields=["category", "name", "year"],
21+
name="uniq_award_cat_name_year",
22+
),
23+
]
1824
verbose_name_plural = "Awards"
1925

2026
category = models.CharField(verbose_name="Category", max_length=100)
21-
name = models.CharField(verbose_name="Name", max_length=255, unique=True)
27+
name = models.CharField(verbose_name="Name", max_length=255)
2228
description = models.TextField(verbose_name="Description", blank=True, default="")
2329
year = models.IntegerField(verbose_name="Year")
2430
is_reviewed = models.BooleanField(

0 commit comments

Comments
 (0)