Skip to content

Commit 46ab72e

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

File tree

3 files changed

+100
-31
lines changed

3 files changed

+100
-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: 82 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@
1111

1212
logger = logging.getLogger(__name__)
1313

14+
# Year validation constants
15+
MIN_VALID_YEAR = 1900
16+
MAX_VALID_YEAR = 2100
17+
1418

1519
class Command(BaseCommand):
1620
help = "Import awards from the OWASP awards YAML file"
@@ -19,55 +23,112 @@ def handle(self, *args, **kwargs) -> None:
1923
"""Handle the command execution."""
2024
self.stdout.write("Syncing OWASP awards...")
2125

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

2842
awards_to_save = []
43+
skipped_count = 0
2944
for item in data:
3045
if item.get("type") == "award":
3146
winners = item.get("winners", [])
3247
for winner in winners:
3348
award = self._create_or_update_award(item, winner)
3449
if award:
3550
awards_to_save.append(award)
51+
else:
52+
skipped_count += 1
3653

37-
Award.bulk_save(awards_to_save)
54+
Award.bulk_save(awards_to_save, fields=("category", "description", "year", "user"))
3855
self.stdout.write(self.style.SUCCESS(f"Successfully synced {len(awards_to_save)} awards"))
56+
if skipped_count:
57+
self.stdout.write(
58+
self.style.WARNING(f"Skipped {skipped_count} awards due to invalid data")
59+
)
3960

4061
def _create_or_update_award(self, award_data, winner_data):
4162
"""Create or update award instance."""
42-
name = f"{award_data['title']} - {winner_data['name']} ({award_data['year']})"
63+
# Safely extract values with defaults
64+
title = award_data.get("title", "")
65+
category = award_data.get("category", "")
66+
67+
# Validate and parse year
68+
try:
69+
year = int(award_data.get("year", 0))
70+
if year <= 0 or year < MIN_VALID_YEAR or year > MAX_VALID_YEAR:
71+
logger.warning("Invalid year %s for award %s, skipping", year, title)
72+
return None
73+
except (ValueError, TypeError):
74+
logger.warning(
75+
"Could not parse year %s for award %s, skipping", award_data.get("year"), title
76+
)
77+
return None
78+
79+
# Handle winner_data being string or dict
80+
if isinstance(winner_data, str):
81+
winner_name = winner_data
82+
winner_info = ""
83+
else:
84+
# Prefer explicit GitHub login over name
85+
login = winner_data.get("login") or winner_data.get("github")
86+
if login:
87+
login = login.lstrip("@")
88+
# Skip bot accounts
89+
if "bot" in login.lower() or login.lower().endswith("[bot]"):
90+
logger.warning("Skipping bot account: %s", login)
91+
return None
92+
winner_name = login
93+
else:
94+
winner_name = winner_data.get("name", "")
95+
winner_info = winner_data.get("info", "")
96+
97+
name = f"{title} - {winner_name} ({year})"
4398

4499
try:
45100
award = Award.objects.get(name=name)
46101
except Award.DoesNotExist:
47102
award = Award(name=name)
48103

49-
award.category = award_data.get("category", "")
50-
award.description = winner_data.get("info", "")
51-
award.year = award_data.get("year", 0)
104+
award.category = category
105+
award.description = winner_info
106+
award.year = year
52107

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"])
108+
# Only set user if not already reviewed
109+
if not (award.user and award.is_reviewed):
110+
user = self._match_user(winner_name)
111+
if user:
112+
award.user = user
113+
else:
114+
logger.warning("Could not match user for award winner: %s", winner_name)
59115

60116
return award
61117

62118
def _match_user(self, winner_name):
63119
"""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
120+
winner_name = winner_name.strip()
68121

69-
# Try login match (GitHub username)
70-
user = User.objects.filter(login__iexact=winner_name).first()
122+
# Check if it looks like a GitHub handle
123+
if winner_name.startswith("@") or (" " not in winner_name and winner_name):
124+
# Strip leading @ and try login match first
125+
login_name = winner_name.lstrip("@")
126+
user = User.objects.filter(login__iexact=login_name).first()
127+
if user:
128+
return user
129+
130+
# Try exact name match
131+
user = User.objects.filter(name__iexact=winner_name).first()
71132
if user:
72133
return user
73134

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)