Skip to content

Conversation

@krrish-sehgal
Copy link
Contributor

@krrish-sehgal krrish-sehgal commented Jul 21, 2025

fixes #4334

With integration of bacon in challenges and also changed the UI for single user challenges to be more consistent with team challenges.

Screenshot 2025-07-22 at 2 52 42 AM Screenshot 2025-07-22 at 2 53 56 AM

Summary by CodeRabbit

  • New Features

    • Introduced BACON tokens as an additional reward for completing individual and team challenges.
    • Challenge details now display both point and BACON token rewards, including new icons and labels in the interface.
  • Enhancements

    • Redesigned the user challenges page for improved structure, interactivity, and visual feedback, including animated progress circles and clearer completion indicators.
    • Team and individual challenge progress now features visually consistent circular progress indicators.
  • Chores

    • Updated settings to disable mandatory email verification for user accounts.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 21, 2025

Walkthrough

The changes introduce BACON tokens as an additional reward for completing challenges, alongside existing points. This involves updating the Challenge model, UI templates for both team and user challenges, and backend logic to award BACON tokens upon challenge completion. Supporting settings and a database migration are also included.

Changes

File(s) Change Summary
website/challenge_signals.py Added logic to award BACON tokens to users/teams upon challenge completion in relevant functions.
website/models.py
website/migrations/0243_add_bacon_rewards_to_challenges.py
Added bacon_reward field to Challenge model and migration to set initial and specific values.
website/templates/team_challenges.html Updated UI to display BACON token rewards for team challenges.
website/templates/user_challenges.html Redesigned user challenges template: Alpine.js-driven UI, progress circles, BACON rewards display.
website/views/user.py Added calculation of progress circle properties for rendering in user challenges UI.
blt/settings.py Set ACCOUNT_EMAIL_VERIFICATION to "none" (was "mandatory"); added explanatory comment.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ChallengeSystem
    participant giveBacon

    User->>ChallengeSystem: Complete Challenge (Individual or Team)
    ChallengeSystem->>giveBacon: Award BACON tokens (per user/team member)
    giveBacon-->>ChallengeSystem: BACON tokens granted
    ChallengeSystem-->>User: Challenge completion confirmed, rewards updated
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 25cff1f and 1811763.

📒 Files selected for processing (3)
  • blt/settings.py (1 hunks)
  • website/migrations/0243_add_bacon_rewards_to_challenges.py (1 hunks)
  • website/models.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • website/models.py
🔇 Additional comments (5)
blt/settings.py (1)

348-348: Confirm intentional removal of mandatory email verification
I see you’ve set ACCOUNT_EMAIL_VERIFICATION = "none" in blt/settings.py, which disables email checks at signup. Meanwhile:

  • website/views/issue.py still blocks unverified users from accessing issues.
  • website/tests.py and website/test_api.py have been updated to manually mark emails as verified.

Disabling verification can introduce risks—account takeover, invalid or disposable emails, and degraded data quality. Please confirm this change is intentional (e.g., only for development/testing) and ensure that production environments require "mandatory" verification to safeguard user accounts.

website/migrations/0243_add_bacon_rewards_to_challenges.py (4)

6-29: LGTM! Well-structured data migration with proper error handling.

The migration function correctly:

  • Uses apps.get_model() for proper migration isolation
  • Implements appropriate error handling for missing challenges
  • Provides clear logging for debugging
  • Maps challenge titles to bacon reward values systematically

The hardcoded challenge titles are appropriate for a data migration as they represent the intended initial state.


31-35: LGTM! Proper reverse migration implementation.

The reverse function correctly resets all bacon_reward values to the default, ensuring the migration can be safely rolled back.


43-47: LGTM! Appropriate field definition with sensible defaults.

The bacon_reward field is well-defined with:

  • Appropriate data type (IntegerField)
  • Reasonable default value (5)
  • Clear help text explaining the field's purpose

48-51: LGTM! Correct RunPython operation structure.

The migration properly uses RunPython with both forward and reverse operations, enabling proper rollback functionality.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
website/migrations/0178_add_bacon_reward_to_challenge.py (1)

1-17: Critical: This migration duplicates migration 0243

This migration adds the same bacon_reward field that migration 0243_add_bacon_reward_to_challenge.py also adds. This duplication will cause migration conflicts.

The comment indicates this was "Generated manually" while 0243 was auto-generated. Please coordinate to ensure only one of these migrations exists in the final codebase.

🧹 Nitpick comments (1)
website/templates/user_challenges.html (1)

104-105: Consider moving Alpine.js to base template for consistency.

Loading Alpine.js directly in this template works, but consider moving it to the base template if other pages also use Alpine.js to avoid duplicate script loading and ensure consistent versions across the application.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 37f4c66 and 5b873be.

📒 Files selected for processing (9)
  • website/challenge_signals.py (4 hunks)
  • website/migrations/0178_add_bacon_reward_to_challenge.py (1 hunks)
  • website/migrations/0179_update_challenge_bacon_rewards.py (1 hunks)
  • website/migrations/0243_add_bacon_reward_to_challenge.py (1 hunks)
  • website/migrations/0244_update_existing_challenges_bacon_rewards.py (1 hunks)
  • website/models.py (1 hunks)
  • website/templates/team_challenges.html (2 hunks)
  • website/templates/user_challenges.html (1 hunks)
  • website/views/user.py (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: validate_pr_closing_issues
  • GitHub Check: Run Tests
  • GitHub Check: docker-test
🔇 Additional comments (15)
website/models.py (1)

1605-1605: LGTM: Clean field addition for BACON token rewards

The bacon_reward field addition is well-implemented with an appropriate default value and clear naming convention.

website/templates/team_challenges.html (2)

14-16: Good: Clear messaging about BACON token rewards

The updated header text effectively communicates the new BACON token reward system alongside existing points and leaderboard functionality.


34-39: LGTM: Well-integrated BACON token display

The BACON token display is well-implemented with:

  • Consistent styling matching the points display
  • Appropriate icon usage (fas fa-bacon)
  • Clear labeling with token amount
  • Proper spacing and visual hierarchy
website/views/user.py (1)

977-980: LGTM: Progress circle calculations are mathematically correct.

The SVG progress circle calculations properly implement a circular progress indicator where the stroke dash offset creates a visual progress fill effect. The circumference value (125.6) matches 2πr for r=20, and the offset formula correctly calculates the visible portion based on progress percentage.

website/challenge_signals.py (4)

42-47: LGTM: Team BACON token rewards properly implemented.

The BACON token awarding logic correctly iterates through all team members and awards tokens upon challenge completion. The local import of giveBacon is appropriate to avoid potential circular imports, and the null check for team_member.user prevents errors.


66-69: LGTM: Individual BACON token rewards consistently implemented.

The individual challenge BACON token awarding follows the same pattern as team challenges, maintaining consistency across the codebase. The placement after point awarding ensures proper reward sequencing.


170-173: LGTM: Sign-in challenge BACON tokens follow consistent pattern.

The BACON token implementation for sign-in challenges maintains the same structure as other challenge types, ensuring consistency across the reward system.


215-220: LGTM: Team sign-in challenge BACON tokens implemented consistently.

The team sign-in challenge BACON token implementation mirrors the pattern used in other team challenges, including proper iteration through team members and null safety checks.

website/migrations/0244_update_existing_challenges_bacon_rewards.py (2)

6-29: LGTM: Migration safely updates BACON rewards with appropriate values.

The migration properly handles updating existing challenges with BACON rewards. The reward values are well-balanced (higher for team challenges), and the try/catch pattern gracefully handles missing challenges. The print statements provide useful feedback during migration execution.


31-34: LGTM: Reverse migration provides clean rollback.

The reverse migration efficiently resets all BACON rewards to the default value using a bulk update, providing a clean way to rollback the changes if needed.

website/migrations/0179_update_challenge_bacon_rewards.py (1)

6-35: Verify migration sequence and BACON reward consistency

We’ve confirmed that:

  • 0179_update_challenge_bacon_rewards (depends on 0178) sets:
    • Team challenges: “Report 10 IPs”=10, “Report 10 Issues”=10, “All Members Sign in for 5 Days”=15
    • Single challenges: “Report 5 IPs”=5, “Report 5 Issues”=5, “Sign in for 5 Days”=8
  • 0244_update_existing_challenges_bacon_rewards (depends on 0243) then sets:
    • “Report 5 IPs”=10, “Report 5 Issues”=15, “Sign in for 5 Days”=5
    • “Report 10 IPs”=20, “Report 10 Issues”=25, “All Members Sign in for 5 Days”=10

Because migration 0243 re-adds or alters the bacon_reward field, 0179’s updates may be overwritten before 0244 runs. Please verify that applying all migrations from a fresh database produces the intended final BACON rewards. In particular:

  • Confirm the dependency chain: 0178_add_bacon_reward0179_update_* → … → 0243_add_bacon_reward0244_update_*.
  • Ensure no intermediate drop of the bacon_reward column discards 0179’s work.
  • Consider consolidating or removing the now-superseded 0179 migration if it only affects data that will be reset later.
website/templates/user_challenges.html (4)

4-17: LGTM: Header section provides clear context and modern design.

The new header section effectively communicates the purpose of individual challenges with a clean, professional design. The icon and descriptive text help users understand they can earn both points and BACON tokens.


32-43: LGTM: BACON token integration is prominent and well-designed.

The BACON token display alongside points provides clear visibility of the new reward system. The bacon icon and completion badge enhance the user experience and make the reward structure immediately apparent.


48-57: LGTM: Circular progress indicator properly utilizes backend calculations.

The SVG circular progress indicator correctly uses the stroke_dasharray and stroke_dashoffset values calculated in the UserChallengeListView, creating a visually appealing progress representation that's consistent with the team challenges interface.


64-88: LGTM: Interactive details section with appropriate completion states.

The Alpine.js-powered collapsible details provide smooth animations and clear action states. The conditional rendering between "Start Challenge" and "Challenge Completed!" buttons appropriately reflects the user's progress status.

coderabbitai[bot]
coderabbitai bot previously approved these changes Jul 21, 2025
@DonnieBLT DonnieBLT enabled auto-merge July 25, 2025 11:20
@krrish-sehgal
Copy link
Contributor Author

@CodeRabbit , review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 25, 2025

Tip

For best results, initiate chat on the files or code changes.

@krrish-sehgal Thank you for requesting a review! I've analyzed your BACON token integration PR and here's my comprehensive assessment:

🎯 Overall Assessment

This is a well-structured implementation that successfully integrates BACON tokens as an additional reward mechanism for challenges. The changes are comprehensive and touch all the necessary components.

✅ Strengths

1. Comprehensive Integration

  • Clean addition of bacon_reward field to the Challenge model with proper defaults
  • Consistent BACON token awarding across all challenge completion paths
  • Well-structured migration with proper error handling and rollback support

2. UI/UX Improvements

  • The user challenges template redesign with Alpine.js is a significant improvement
  • Consistent progress circle visualization between team and user challenges
  • Clear visual representation of BACON rewards alongside points

3. Code Quality

  • Proper separation of concerns in signal handlers
  • Good use of local imports for giveBacon function
  • Appropriate default values and help text in the model

⚠️ Areas of Concern

1. Email Verification Setting (Critical)
The change in blt/settings.py from ACCOUNT_EMAIL_VERIFICATION = "mandatory" to "none" is concerning:

# This was changed from "mandatory" to "none"
ACCOUNT_EMAIL_VERIFICATION = "none"  # Set to "mandatory" to enforce email verification

Security Implications:

  • Allows unverified email addresses to create accounts
  • Potential for account takeover attacks
  • Risk of spam/disposable email addresses
  • Inconsistent with existing code that checks for verified emails

Recommendation: Please clarify if this change is:

  • Intentional for development/testing only
  • A permanent change (needs security review)
  • Should be reverted before production deployment

2. Migration Data Integrity
While the migration is well-structured, consider adding validation for the bacon reward values to ensure they're reasonable (e.g., positive integers within expected ranges).

🔍 Technical Review

Database Migration: ✅ Excellent

  • Proper use of apps.get_model() for migration isolation
  • Good error handling for missing challenges
  • Proper reverse migration implementation

Signal Integration: ✅ Good

  • Consistent pattern across all completion handlers
  • Proper handling of both individual and team challenges
  • Clean local imports

Template Updates: ✅ Excellent

  • Alpine.js integration improves interactivity
  • Consistent styling with existing team challenges
  • Proper accessibility considerations

🚀 Recommendations

  1. Address the email verification setting - This needs immediate attention
  2. Consider adding validation for bacon reward values in the model
  3. Test edge cases like challenges with 0 bacon rewards
  4. Documentation - Consider updating any relevant documentation about the reward system

📊 Code Quality Score: 8.5/10

The implementation is solid with good architectural decisions. The main concern is the email verification setting change, which needs clarification and potentially reversal.

Would you like me to help create a follow-up issue for any of these recommendations, or do you need clarification on any specific aspect of the review?

@DonnieBLT
Copy link
Collaborator

@CodeRabbit approve

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 25, 2025

✅ Actions performed

Comments resolved and changes approved.

@DonnieBLT DonnieBLT added this pull request to the merge queue Jul 25, 2025
Merged via the queue into OWASP-BLT:main with commit 1d942ad Jul 25, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate bacon reward system in Team/User Challenges

2 participants