Align badge css classes and auto-sync badges during user update#3725
Align badge css classes and auto-sync badges during user update#3725Isha-upadhyay wants to merge 6 commits intoOWASP:mainfrom
Conversation
Summary by CodeRabbit
WalkthroughAdds post-sync badge synchronization calls to the GitHub user update flow, changes several badge CSS class values and an enum choice (with migration), refactors frontend badge icon resolution to use camelCase keys, updates tests to match, and adds comprehensive badge documentation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/apps/nest/models/badge.py`:
- Line 15: Create a Django data migration to rename persisted css_class values
so existing Badge rows keep matching the updated TextChoices: add a migration
that uses apps.get_model('nest', 'Badge') and a forward function (e.g.,
update_bug_slash_css_class) to filter
Badge.objects.filter(css_class='bug_slash').update(css_class='bugSlash') and a
reverse function (e.g., reverse_bug_slash_css_class) to change 'bugSlash' back
to 'bug_slash'; add this migration to the nest app dependencies immediately
after the previous migration so the stored values align with the new BUG_SLASH
enum constant.
🧹 Nitpick comments (2)
backend/apps/github/management/commands/github_update_users.py (1)
61-66: Consider error handling for badge sync commands.If either badge sync command fails, the entire
github_update_userscommand will fail after user contributions have already been saved. This could leave the system in a partially updated state. Consider wrapping the calls in try/except to log errors without failing the entire update:🛡️ Suggested error handling
# Sync badges after user data refresh self.stdout.write("Syncing badges...") - call_command("nest_update_staff_badges") - call_command("nest_update_project_leader_badges") + try: + call_command("nest_update_staff_badges", stdout=self.stdout) + call_command("nest_update_project_leader_badges", stdout=self.stdout) + except Exception as e: + logger.exception("Badge sync failed") + self.stderr.write(self.style.ERROR(f"Badge sync failed: {e}"))Note: Passing
stdout=self.stdoutalso ensures the nested commands' output is captured consistently.backend/tests/apps/github/management/commands/github_update_users_test.py (1)
240-262: Tighten badge-sync call assertions to prevent false positives.
assert_any_callpasses even if extra commands are invoked or order changes unintentionally. Consider asserting exact calls + count for stronger regression protection.✅ Suggested tightening
-from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, call- mock_call_command.assert_any_call("nest_update_staff_badges") - mock_call_command.assert_any_call("nest_update_project_leader_badges") + mock_call_command.assert_has_calls( + [ + call("nest_update_staff_badges"), + call("nest_update_project_leader_badges"), + ], + any_order=False, + ) + assert mock_call_command.call_count == 2
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/apps/owasp/migrations/0073_rename_bug_slash_css_class.py`:
- Around line 16-19: The migration class (Migration) and its dependency tuple
referencing ("nest", "0072") are in the wrong app; move this data migration so
it belongs to the nest app (the migration that manipulates the Badge model) and
update the Migration.dependencies to point to the correct preceding nest
migration (i.e., the immediate prior nest migration name/number) so Django
registers it under the nest app migration history and preserves ordering and
model access to Badge.
🧹 Nitpick comments (1)
backend/apps/github/management/commands/github_update_users.py (1)
65-70: Consider isolating exception handling for each badge command.Both badge commands are wrapped in a single try block. If
nest_update_staff_badgesfails,nest_update_project_leader_badgeswill be skipped entirely, even though they are independent operations. Separating them ensures both commands get a chance to run.Also, the command will exit with success (exit code 0) even when badge sync fails since the exception is caught but not re-raised. If this is intentional (to prioritize user contribution updates), consider documenting that decision. Otherwise, you may want to track the failure and raise at the end.
♻️ Proposed refactor to isolate exception handling
# Sync badges after user data refresh self.stdout.write("Syncing badges...") + badge_sync_failed = False + try: call_command("nest_update_staff_badges", stdout=self.stdout) + except Exception as e: + logger.exception("Staff badge sync failed") + self.stderr.write(self.style.ERROR(f"Staff badge sync failed: {e}")) + badge_sync_failed = True + + try: call_command("nest_update_project_leader_badges", stdout=self.stdout) except Exception as e: - logger.exception("Badge sync failed") - self.stderr.write(self.style.ERROR(f"Badge sync failed: {e}")) + logger.exception("Project leader badge sync failed") + self.stderr.write(self.style.ERROR(f"Project leader badge sync failed: {e}")) + badge_sync_failed = True + + if badge_sync_failed: + self.stderr.write(self.style.WARNING("User update completed but badge sync had errors"))Based on learnings: In Django management commands, use
self.stdout.write()andself.stderr.write()for output (which this code correctly does).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@backend/docs/Badges.md`:
- Line 200: The fenced code block that currently starts with ``` and contains
the lines "Syncing badges...", "Syncing OWASP Staff...", "Syncing Project
Leaders..." needs a language specifier for proper highlighting and linting;
change the opening fence to include a language (e.g., use ```text or ```bash) so
the block becomes ```text (or another appropriate language) followed by the
existing lines and the closing ```.
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
1 participant
Proposed change
Resolves #3419
This PR cleans up and stabilizes how badges are handled across backend and frontend.
What was happening
css_classvalues in a different format than what the frontend expects.What I changed
BadgeCssClassvalues directly with frontend icon keys (removed normalization dependency).github_update_userssync flow so badges refresh automatically.Screenshots:
.
Result
Checklist
make check-testlocally: all warnings addressed, tests passed