Feat : Badges : Allow users to embed digital badges on profiles and websites #115
Conversation
WalkthroughThis change introduces a badge system into the application. Two new models— Changes
Sequence Diagram(s)sequenceDiagram
participant E as Enrollment/ChallengeSubmission
participant S as Signal Receiver
participant B as Badge System
participant DB as Database
participant N as Notification System
E->>S: Trigger signal on instance creation
S->>DB: Check if the badge has already been awarded
alt Badge not awarded
S->>B: Award badge (create UserBadge record)
B->>DB: Save badge award
S->>N: Generate notification for the user
end
sequenceDiagram
participant U as User
participant V as View (profile)
participant P as Profile
participant DB as Database
participant R as Renderer
U->>V: Request profile page
V->>P: Retrieve user badges
P->>DB: Query for associated badges
DB-->>P: Return badge data
P-->>V: Provide badge list
V->>R: Render page with badge information
R-->>U: Display badges on page
Possibly related PRs
Suggested labels
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
web/templates/profile.html (2)
354-372: Badge display section needs styling.The badge section structure is good, but the badge-item class doesn't have defined styling. Consider adding CSS for proper badge display and formatting.
- <div class="badge-item"> + <div class="badge-item flex flex-col items-center p-3 m-2 bg-gray-50 dark:bg-gray-700 rounded-lg"> - <img src="{{ user_badge.badge.image.url }}" alt="{{ user_badge.badge.name }}" /> + <img src="{{ user_badge.badge.image.url }}" alt="{{ user_badge.badge.name }}" class="w-16 h-16 mb-2" /> - <p>{{ user_badge.badge.name }}</p> + <p class="text-sm font-medium">{{ user_badge.badge.name }}</p>Also, consider displaying badges in a grid layout for better visual organization:
+ <div class="grid grid-cols-2 sm:grid-cols-3 md:grid-cols-4 gap-4"> {% for user_badge in badges %} <div class="badge-item flex flex-col items-center p-3 m-2 bg-gray-50 dark:bg-gray-700 rounded-lg"> <img src="{{ user_badge.badge.image.url }}" alt="{{ user_badge.badge.name }}" class="w-16 h-16 mb-2" /> <p class="text-sm font-medium">{{ user_badge.badge.name }}</p> </div> {% endfor %} + </div>
356-364: Consider adding badge earn date information.Displaying when badges were earned would provide valuable context to users.
<div class="badge-item"> <img src="{{ user_badge.badge.image.url }}" alt="{{ user_badge.badge.name }}" /> <p>{{ user_badge.badge.name }}</p> + <span class="text-xs text-gray-500 dark:text-gray-400">Earned {{ user_badge.created_at|date:"M d, Y" }}</span> </div>web/migrations/0023_badge_userbadge.py (1)
72-137: Consider Modernizing Unique ConstraintsThe
UserBadgemodel’s use ofunique_togetheris perfectly valid. However, if you plan to upgrade Django or rely on advanced constraint options, consider usingUniqueConstraintin the model’sMeta.constraintsfor more flexibility. Apart from that, the foreign key relationships and ordering look correct for your goals.web/models.py (3)
1312-1329: Challenge Badge Award LogicAutomatically awarding challenge badges in
award_challenge_badgeis a neat approach. Watch for performance overhead if the logic grows (e.g., awarding multiple badges for each submission). You may eventually want to optimize database lookups or caching.
1346-1367: Enhance Error Handling When Awarding Badges
award_badge_to_studentsilently returnsNoneif the teacher is invalid or if the badge or user does not exist. You may want to log these attempts or raise exceptions to inform staff about improper usage. The logic itself is clean for immediate usage, just keep an eye on auditing.
1373-1373: Profile Extension withget_user_badgesMonkey-patching
Profileis creative, but might obscure where this method is defined. If maintainability becomes an issue, consider defining the method directly on theProfilemodel.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/migrations/0023_badge_userbadge.py(1 hunks)web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py(1 hunks)web/migrations/0025_merge_20250317_1600.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(1 hunks)
🔇 Additional comments (9)
web/migrations/0025_merge_20250317_1600.py (1)
1-14: Migration file looks correct and properly merges dependencies.This merge migration correctly combines the previous migrations for success stories and badges, ensuring a clean database migration path when multiple branches are developed in parallel.
web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py (1)
1-14: Proper migration merge file for badges and donations.This migration correctly merges the parallel migrations for the new Badge/UserBadge models and the Donation model, maintaining database schema consistency.
web/views.py (1)
3689-3694:Details
✅ Verification successful
The user badge view implementation looks good.
The view correctly uses the login_required decorator and retrieves the user's badges from their profile using the get_user_badges() method.
🏁 Script executed:
#!/bin/bash # Verify that the get_user_badges method exists on the Profile model rg "def get_user_badges" --type pyLength of output: 77
Verified: User Badges View Implementation
The user badge view is implemented correctly. It applies thelogin_requireddecorator, and the badge retrieval usingget_user_badges()from the user's profile works as expected. The search confirms that theget_user_badgesmethod is defined inweb/models.py.
- File Verified:
web/views.py(lines 3689–3694)- Method Verified:
get_user_badges()exists inweb/models.pyweb/migrations/0023_badge_userbadge.py (2)
1-15: Migration Dependencies Look GoodThese lines establish the necessary dependency references for creating the new models. No issues found with the overall migration setup.
16-71: Verify Criteria Field UsageThe
Badgemodel includes acriteriaJSONField withdefault=dict. While this is a common approach, ensure that the application layer properly validates or interprets this field to avoid any malformed or inconsistent data in the JSON. Otherwise, this model creation logic looks solid.web/models.py (4)
1243-1281: Confirm Badge Image HandlingThe
Badgemodel processes images by converting them to RGB, then saves them in PNG at 90 quality. This is typically fine for most workflows, but it may re-encode images from other formats. If you want to preserve transparency (e.g., in PNGs), ensure that the forced RGB conversion does not inadvertently remove it. Otherwise, the model definition appears coherent.
1282-1311: UserBadge Model Setup Looks GoodThe fields and relationships in
UserBadgealign well with the intended usage. The ordering by-awarded_atis appropriate and the reference to bothChallengeSubmissionandEnrollmentprovides flexibility. This schema should support the needed features without issue.
1330-1345: Course Completion Badge TriggerSimilarly,
award_course_completion_badgeaccurately checks forstatus == "completed". Consider edge cases: if a user toggles back and forth in enrollment status or re-completes a course. The function currently only awards once, which is usually correct.
1369-1371: Good Reusable Fetching Method
get_user_badgesis a straightforward way to retrieve badges bound to a specific user. No concerns here, and it aids in keeping your code DRY and modular.
54edae2 to
e425144
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
web/views.py (1)
3693-3697: Good implementation of the user badges view function.The function is properly decorated with @login_required to ensure authentication, and it uses the profile's get_user_badges() method to retrieve badge data. The implementation is concise and follows Django's best practices.
Consider adding error handling for cases where a user might not have a profile or the get_user_badges() method fails.
@login_required def user_badges(request): - badges = request.user.profile.get_user_badges() - return render(request, "web/user_badges.html", {"badges": badges}) + try: + badges = request.user.profile.get_user_badges() + return render(request, "web/user_badges.html", {"badges": badges}) + except (AttributeError, Exception) as e: + messages.error(request, "Unable to retrieve badges. Please try again later.") + return redirect("profile")web/models.py (4)
1243-1280: Review of the Badge modelThe Badge model looks well-structured with appropriate fields for different badge types and good image processing in the save method.
Two suggestions:
- The save method uses PNG format but keeps the original filename extension, which might cause confusion.
- Consider adding validation to ensure the appropriate relationship exists based on badge_type (e.g., a "course" badge should have a course relation).
def save(self, *args, **kwargs): if self.image: img = Image.open(self.image) if img.mode != "RGB": img = img.convert("RGB") img = img.resize((200, 200), Image.Resampling.LANCZOS) buffer = BytesIO() img.save(buffer, format="PNG", quality=90) file_name = self.image.name + # Ensure the file extension is consistent with the saved format + base_name, _ = os.path.splitext(file_name) + file_name = f"{base_name}.png" self.image.delete(save=False) self.image.save(file_name, ContentFile(buffer.getvalue()), save=False) + + # Validate badge relationships + if self.badge_type == 'course' and not self.course: + raise ValidationError("Course badge must be associated with a course") + elif self.badge_type == 'challenge' and not self.challenge: + raise ValidationError("Challenge badge must be associated with a challenge") + super().save(*args, **kwargs)
1282-1310: Well-structured UserBadge modelThe UserBadge model is well-designed with appropriate relationships to track how badges are awarded. Good job enforcing uniqueness to prevent duplicate awards!
Consider adding validation to ensure the appropriate fields are set based on the award_method.
def clean(self): + # Validate that appropriate fields are set based on award_method + if self.award_method == 'challenge_completion' and not self.challenge_submission: + raise ValidationError("Challenge completion awards must have an associated challenge submission") + elif self.award_method == 'course_completion' and not self.course_enrollment: + raise ValidationError("Course completion awards must have an associated course enrollment") + elif self.award_method == 'teacher_awarded' and not self.awarded_by: + raise ValidationError("Teacher awarded badges must have an associated teacher") def save(self, *args, **kwargs): + self.full_clean() super().save(*args, **kwargs)
1346-1367: Utility for manual badge awardsThe manual badge awarding function includes good validations for teacher status and prevents duplicate awards.
Consider enhancing error reporting to provide more specific feedback on why an award failed.
def award_badge_to_student(badge_id, student_id, teacher_id, message=""): try: badge = Badge.objects.get(id=badge_id) student = User.objects.get(id=student_id) teacher = User.objects.get(id=teacher_id) if not teacher.profile.is_teacher: - return None + return {"success": False, "error": "User is not a teacher"} if UserBadge.objects.filter(user=student, badge=badge).exists(): - return None + return {"success": False, "error": "Student already has this badge"} user_badge = UserBadge.objects.create( user=student, badge=badge, award_method="teacher_awarded", awarded_by=teacher, award_message=message ) Notification.objects.create( user=student, title=f"New Badge: {badge.name}", message=f"You were awarded {badge.name} by {teacher.username}. {message}", notification_type="success", ) - return user_badge + return {"success": True, "badge": user_badge} except (Badge.DoesNotExist, User.DoesNotExist) as e: - return None + return {"success": False, "error": f"Entity not found: {str(e)}"}
1369-1373: Profile method for badge retrievalThe method to get user badges is simple but effective. Consider making it a property for more natural access.
-def get_user_badges(self): +@property +def user_badges(self): return UserBadge.objects.filter(user=self.user) -Profile.get_user_badges = get_user_badges +Profile.user_badges = property(user_badges)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/migrations/0023_badge_userbadge.py(1 hunks)web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py(1 hunks)web/migrations/0025_merge_20250317_1600.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- web/migrations/0025_merge_20250317_1600.py
- web/migrations/0023_badge_userbadge.py
- web/templates/profile.html
- web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py
🔇 Additional comments (4)
web/views.py (2)
107-107: Good addition of the UserBadge model import.The import is correctly placed in alphabetical order within the models import list.
241-245: Well-implemented badge retrieval for the profile view.The query efficiently uses select_related() to optimize database access by fetching the related badge data in a single query, which is a good performance practice.
web/models.py (2)
1329-1344: Course completion badge handlerThe implementation for course completion badges looks good. It checks for existing badges to prevent duplicates and creates notifications.
1312-1327:Details
❓ Verification inconclusive
Automatic challenge badge awarding
The signal handler for awarding badges on challenge completion is well-implemented. Note that badges are awarded upon submission creation, not necessarily after evaluation of submission quality.
Consider whether badges should be awarded automatically upon submission or only after the submission has been reviewed/approved:
🌐 Web query:
What are best practices for awarding badges in education platforms - on submission or after evaluation?💡 Result:
When awarding digital badges in educational platforms, best practices emphasize aligning the award timing with learning objectives while maintaining credibility. Here's a structured approach based on industry standards and academic research:
Key Considerations for Badge Awarding
1. Evaluation-Based Awarding (Post-Assessment)
- Verification of mastery: Badges should be awarded after successful evaluation of evidence [2][3][8]
- Assessment integration:
- Automated systems can issue badges instantly when predefined criteria are met (e.g., scoring 90%+ on quizzes) [6][15]
- Manual review required for complex competencies like project work or critical thinking [2][13]
2. Submission-Triggered Recognition
- Progress tracking: Some platforms allow partial badge completion visualization as students submit components [2][17]
- Motivational milestones: Interim badges can be awarded for task submission to encourage continued participation [7][17]
Best Practice Framework
Factor Immediate Submission Post-Evaluation Use Case Progress tracking
Activity completionSkill mastery
Competency verificationVerification Basic participation Rigorous assessment Credibility Low-medium High Platform Example LMS completion flags [15] Canvas Credentials [6] Recommended Hybrid Approach:
- Automated awards for objective milestones (course completion, quiz scores) [6][16]
- Evaluated awards for subjective competencies requiring human judgment [2][8]
- Transparent criteria showing submission requirements vs evaluation standards [1][18]
Implementation Strategies
- Metadata integration: Include assessment method details in badge credentials (automated scoring vs instructor evaluation) [3][4]
- Tiered badging:
- Participation badges awarded on submission
- Mastery badges requiring evaluated outcomes [7][17]
- Real-time updates: Modern platforms like Canvas Credentials now sync badge status within minutes of evaluation completion [6]
Leading institutions like the University of Texas at Dallas combine automated awarding with faculty verification processes, ensuring badges reflect both participation and proven competencies [4][7]. Research shows this balanced approach increases learner motivation by 25% while maintaining credential integrity [7][17].
Citations:
- 1: https://assess.com/digital-badges/
- 2: https://www.ascd.org/el/articles/the-power-of-digital-badges
- 3: https://opencontent.org/blog/archives/2397
- 4: https://oisds.utdallas.edu/microcredentials/digital-badges
- 5: https://sertifier.com/blog/digital-badges-for-students/
- 6: https://community.canvaslms.com/t5/The-Product-Blog/Instant-Recognition-Real-Time-Badge-Awarding-is-Here/ba-p/604392
- 7: https://www.verifyed.io/blog/badge-award
- 8: https://www.nysed.gov/sites/default/files/principal-project-phase-2-developing-a-higher-education-badging-initiative.pdf
- 9: https://daily.dev/blog/10-open-source-badge-platforms-for-community-managers
- 10: https://www.cdse.edu/Im-looking-for/My-Certificates-Digital-Badges-Transcripts/My-Digital-Badges/
- 11: https://www.virtualbadge.io/blog-articles/certificates-certifications-attendance-certificates-and-badges-in-education-a-comprehensive-overview
- 12: https://www.virtualbadge.io/blog-articles/open-badges-for-education-providers-the-all-inclusive-guide
- 13: https://blog.topclasslms.com/how-to-design-a-digital-badge-strategy
- 14: https://www.parchment.com/platform/international/global-issuing/digitary-services-badges/
- 15: https://community.canvaslms.com/t5/Canvas-Badges-Credentials/Using-Badges-for-Assignments/m-p/620812
- 16: http://www.afterschoolalliance.org/documents/DigitalBadgesInAfterschool.pdf
- 17: https://www.suitable.co/knowledge-center/blog/top-five-ways-to-implement-digital-badging-on-your-campus
- 18: https://www.1edtech.org/standards/open-badges/badging-considerations
- 19: https://library.educause.edu/topics/teaching-and-learning/badges-and-credentialing
- 20: https://www.mnsu.edu/globalassets/continuing-education/badging-best-practices.pdf
Investigate Badge Awarding Timing
The implementation in
web/models.py(lines 1312–1327) correctly awards badges upon challenge submission creation. However, based on current best practices:
- Badge Purpose: If these badges signify mastery or achievement, consider deferring awarding until after a formal evaluation. Immediate awarding may be more suitable for participation or progress-tracking badges.
- Hybrid Approach: A mix of automated and evaluation-based awarding can better align recognition with actual competency.
- Documentation: Clearly distinguish between badge types (e.g., participation vs. mastery) so that the awarding criteria are transparent for users.
Please verify that this implementation aligns with your platform’s goals and adjust the timing of badge awards if your intent is to reflect evaluated mastery.
|
@coderabbitai what changes remain? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 164 🏁 Script executed: Length of output: 23565 🏁 Script executed: Length of output: 462 @A1L13N Based on my analysis of the code, the following changes from the PR objectives still remain to be implemented:
These align with the "next steps" mentioned in the PR objectives. Would you like me to help with implementing any of these remaining items? |
|
please resolve conflicts and apply suggestions then resolve the conversations |
A1L13N
left a comment
There was a problem hiding this comment.
please resolve the issues
|
Yes @A1L13N i will do the changes by tomorrow evening I have been travelling all today. |
|
@A1L13N could you please clarify on your own the situations when these badges would be awarded |
|
@A1L13N , I think it would be better that we limit the scope of this pr up to these changes only and can make further PRs for upcoming changes like deciding scenarios for badge awarding and design of badges etc. Please let me know what you think! |
e425144 to
eb8b867
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
web/views.py (1)
3733-3737: New badges view implemented correctly, but missing pagination.The new
user_badgesview correctly implements the badge functionality with proper authentication checks. However, consider adding pagination to handle cases where users might accumulate many badges over time.@login_required def user_badges(request): - badges = request.user.profile.get_user_badges() - return render(request, "web/user_badges.html", {"badges": badges}) + badges_list = request.user.profile.get_user_badges() + paginator = Paginator(badges_list, 12) # Show 12 badges per page + page_number = request.GET.get('page') + badges = paginator.get_page(page_number) + return render(request, "web/user_badges.html", { + "badges": badges, + "is_paginated": paginator.num_pages > 1 + })Also, consider adding appropriate imports for the paginator at the top of the file.
web/models.py (7)
1311-1348: Good implementation of the Badge model with a few suggestionsThe Badge model is well-structured with appropriate fields for different badge types and relationships. I like the image processing in the save method to ensure consistency.
Consider these improvements:
- Add validation to ensure that the correct related model is provided based on badge_type:
- def save(self, *args, **kwargs): + def clean(self): + if self.badge_type == "course" and not self.course: + raise ValidationError("Course badge must be associated with a course") + if self.badge_type == "challenge" and not self.challenge: + raise ValidationError("Challenge badge must be associated with a challenge") + if self.badge_type in ["achievement", "teacher_awarded"] and (self.course or self.challenge): + raise ValidationError(f"{self.get_badge_type_display()} should not be linked to a course or challenge") + + def save(self, *args, **kwargs): + self.full_clean()
- Your image processing converts to RGB but saves as PNG. If you're saving as PNG, consider keeping transparency:
- if img.mode != "RGB": - img = img.convert("RGB") + if img.mode not in ("RGB", "RGBA"): + img = img.convert("RGBA")
1350-1378: Well-structured UserBadge model with proper relationshipsThe UserBadge model establishes the many-to-many relationship between users and badges with additional metadata. The unique_together constraint prevents duplicate awards.
Add validation to ensure required fields are populated based on award_method:
+ def clean(self): + if self.award_method == "challenge_completion" and not self.challenge_submission: + raise ValidationError("Challenge submission is required for challenge completion badges") + if self.award_method == "course_completion" and not self.course_enrollment: + raise ValidationError("Course enrollment is required for course completion badges") + if self.award_method == "teacher_awarded" and not self.awarded_by: + raise ValidationError("Teacher reference is required for teacher-awarded badges") + + def save(self, *args, **kwargs): + self.full_clean() + super().save(*args, **kwargs)
1380-1395: Good implementation of challenge badge signal handlerThe signal receiver properly handles awarding badges for challenge submissions and creates notifications to inform users.
Consider adding a check for successful challenge completion if such a status exists, rather than just awarding on submission creation:
- if created: + if created and getattr(instance, 'is_successful', True): # Assuming successful by default, adjust if neededAlso, consider moving notification text to a template or constants for better maintainability and future internationalization support.
1414-1435: Comprehensive badge awarding utility functionThe
award_badge_to_studentfunction includes proper error handling and validation before awarding badges.Consider adding validation to ensure the badge type is appropriate for teacher awards:
if not teacher.profile.is_teacher: return None + if badge.badge_type != "teacher_awarded": + return None # Only teacher_awarded badges can be manually awarded if UserBadge.objects.filter(user=student, badge=badge).exists(): return NoneAlso, consider returning specific error codes or messages to help diagnose issues when the function returns None.
1437-1441: Profile extension for badge retrievalThe implementation adds badge access to user profiles, which is good for encapsulation.
Consider defining this directly in the Profile class rather than monkey-patching it at the end of the file:
class Profile(models.Model): # Existing Profile fields... + def get_user_badges(self): + from .models import UserBadge # Import here to avoid circular imports + return UserBadge.objects.filter(user=self.user)This is more maintainable as it keeps related functionality together in the class definition.
1333-1344: Image processing might distort badge imagesThe badge image processing currently resizes to a fixed size without preserving aspect ratio.
Consider modifying the image processing to preserve aspect ratio by cropping to a square:
- img = img.resize((200, 200), Image.Resampling.LANCZOS) + # First crop to square + width, height = img.size + min_dim = min(width, height) + left = (width - min_dim) / 2 + top = (height - min_dim) / 2 + right = (width + min_dim) / 2 + bottom = (height + min_dim) / 2 + img = img.crop((left, top, right, bottom)) + # Then resize + img = img.resize((200, 200), Image.Resampling.LANCZOS)This will ensure badges have a consistent appearance without stretching.
1311-1378: Badges need to be registered in admin interfaceAccording to the PR objectives, there's a need for an admin interface to manage badges.
The Badge and UserBadge models need to be registered in the admin.py file to allow administrators to manage badges through the Django admin interface. Would you like me to provide the admin registration code for these models?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
web/migrations/0023_badge_userbadge.py(1 hunks)web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py(1 hunks)web/migrations/0025_merge_20250317_1600.py(1 hunks)web/migrations/0029_merge_0025_merge_20250317_1600_0028_certificate.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- web/migrations/0029_merge_0025_merge_20250317_1600_0028_certificate.py
🚧 Files skipped from review as they are similar to previous changes (4)
- web/migrations/0025_merge_20250317_1600.py
- web/migrations/0024_merge_0023_badge_userbadge_0023_donation.py
- web/templates/profile.html
- web/migrations/0023_badge_userbadge.py
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (1)
web/models.py (2) (2)
UserBadge(1350-1377)get_user_badges(1437-1438)
🔇 Additional comments (3)
web/views.py (2)
116-116: Appropriate model import for badge functionality.The
UserBadgemodel has been correctly imported to support the badge functionality being introduced.
250-254: Good implementation of badge retrieval for profile view.The code efficiently queries user badges with
select_related("badge")to avoid N+1 query problems when accessing badge information in templates. This is a performance-conscious implementation that follows Django best practices.web/models.py (1)
1397-1412: Well-implemented course completion badge handlerThe signal receiver properly checks for completed enrollment status before awarding course badges.
The implementation correctly filters for relevant badges and prevents duplicate awards, which is good practice.
eb8b867 to
f477cf8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
web/views.py (1)
3733-3736: Consider optimizing badge retrieval in the user_badges view.The current implementation doesn't use
select_related("badge")which might cause N+1 query issues if badge details are accessed in the template. Also, consider adding a docstring to improve code documentation.@login_required def user_badges(request): + """Display all badges earned by the current user.""" - badges = request.user.profile.get_user_badges() + badges = UserBadge.objects.filter(user=request.user).select_related("badge") return render(request, "web/user_badges.html", {"badges": badges})web/models.py (4)
1311-1348: Badge model looks well-structuredThe Badge model implementation is solid with good type definitions, appropriate fields, and proper image handling. The resize functionality ensures consistent badge displays across the platform.
Two suggestions for improvement:
- In the save method, consider changing format to PNG since you're resizing badges specifically:
- img.save(buffer, format="PNG", quality=90) + img.save(buffer, format="PNG")PNG is a lossless format, so the quality parameter (which is for JPEG compression) is unnecessary and could be removed.
- Consider adding a docstring describing the model and its purpose in the badge system.
1397-1412: Ensure completed enrollments trigger badge awards correctlyThe course completion badge awarding logic checks if status is "completed" but doesn't verify if this is a change from a previous state. This could cause duplicate notifications if the enrollment is saved multiple times after completion.
Consider updating the handler to check if the status has changed:
@receiver(post_save, sender=Enrollment) -def award_course_completion_badge(sender, instance, **kwargs): +def award_course_completion_badge(sender, instance, created, **kwargs): - if instance.status == "completed": + # Get the old status if this is an update + old_status = None + if not created: + try: + old_instance = Enrollment.objects.get(pk=instance.pk) + old_status = old_instance.status + except Enrollment.DoesNotExist: + pass + + # Only award if status changed to completed + if instance.status == "completed" and old_status != "completed": course_badges = Badge.objects.filter(course=instance.course, badge_type="course", is_active=True)
1414-1435: Manual badge awarding function needs transaction supportThe award_badge_to_student function is well-structured with proper validation, but it lacks transaction support. If any part of the operation fails after creating the UserBadge but before creating the notification, it could leave the system in an inconsistent state.
Add transaction support:
+from django.db import transaction -def award_badge_to_student(badge_id, student_id, teacher_id, message=""): +@transaction.atomic +def award_badge_to_student(badge_id, student_id, teacher_id, message=""): try: badge = Badge.objects.get(id=badge_id) student = User.objects.get(id=student_id) teacher = User.objects.get(id=teacher_id)
1437-1441: Consider adding filtering options to get_user_badges methodThe get_user_badges method provides a basic way to retrieve user badges, but could be more versatile with additional filtering options.
Consider enhancing the method to support filtering by badge type or other criteria:
-def get_user_badges(self): - return UserBadge.objects.filter(user=self.user) +def get_user_badges(self, badge_type=None, active_only=True): + """ + Get badges for a user with optional filtering. + + Args: + badge_type: Filter by badge type (challenge, course, etc.) + active_only: Only return badges that are active + + Returns: + QuerySet of UserBadge objects + """ + queryset = UserBadge.objects.filter(user=self.user) + + if badge_type: + queryset = queryset.filter(badge__badge_type=badge_type) + + if active_only: + queryset = queryset.filter(badge__is_active=True) + + return queryset
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/migrations/0029_badge_userbadge.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/templates/profile.html
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (1)
web/models.py (2) (2)
UserBadge(1350-1377)get_user_badges(1437-1438)
🔇 Additional comments (6)
web/migrations/0029_badge_userbadge.py (1)
1-138: Well-structured migration with clear model definitions.The migration introduces two well-designed models for the badge system:
Badgemodel with appropriate fields for name, description, image, and criteriaUserBadgemodel that correctly links users to their earned badgesThe unique constraint in
UserBadgeensures users can only receive each badge once, and the ordering by awarded_at provides a sensible default sort order.web/views.py (3)
116-116: Appropriate model import.The UserBadge model import is correctly added to the imports section.
250-251: Efficient database query with select_related.Good use of
select_related("badge")to optimize the database query by fetching the related Badge objects in the same query, which will reduce the number of database hits when rendering badges in the template.
254-254: Context properly updated with badges data.The badges are correctly added to the context dictionary for use in the template.
web/models.py (2)
1350-1378: UserBadge model properly tracks badge awardsThe UserBadge model correctly establishes the many-to-many relationship between users and badges with additional metadata about how and when the badge was awarded. The unique_together constraint ensures users can't receive the same badge multiple times.
1380-1395: Signal handler for challenge badges is well implementedThe signal handler for automatically awarding challenge badges when submissions are created is implemented correctly. It checks if the badge already exists for the user before creating a new one, and it creates a notification to inform the user.
f477cf8 to
ed6d13b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
web/models.py (1)
1311-1441:⚠️ Potential issueRegister Badge and UserBadge models in admin.py
Based on the past review comments, these models are not registered in the admin.py file, which means administrators can't manage badges through the Django admin interface.
Add the following to your app's admin.py file to register the Badge and UserBadge models:
from django.contrib import admin from .models import Badge, UserBadge @admin.register(Badge) class BadgeAdmin(admin.ModelAdmin): list_display = ('name', 'badge_type', 'is_active', 'created_by', 'created_at') list_filter = ('badge_type', 'is_active') search_fields = ('name', 'description') @admin.register(UserBadge) class UserBadgeAdmin(admin.ModelAdmin): list_display = ('user', 'badge', 'award_method', 'awarded_at') list_filter = ('award_method', 'badge__badge_type') search_fields = ('user__username', 'badge__name') date_hierarchy = 'awarded_at'
🧹 Nitpick comments (5)
web/models.py (5)
1311-1348: Well-structured Badge model with proper fields and image processingThe Badge model is well-designed with appropriate fields for tracking different types of badges and their relationships to courses and challenges. The image processing in the save method ensures consistent badge image sizes and format.
Consider adding validation to ensure badges have the appropriate related entity based on their type (e.g., a "challenge" badge should have a challenge, a "course" badge should have a course):
def clean(self): + if self.badge_type == "challenge" and not self.challenge: + raise ValidationError("Challenge badges must have an associated challenge.") + if self.badge_type == "course" and not self.course: + raise ValidationError("Course badges must have an associated course.") + super().clean() + + def save(self, *args, **kwargs): + self.full_clean() if self.image: img = Image.open(self.image)
1350-1378: Well-implemented UserBadge model with proper relationships and constraintsThe UserBadge model effectively tracks badge awards with appropriate foreign keys to related models and a unique constraint to prevent duplicate awards.
Consider adding a docstring to clarify this model's purpose and relationships:
class UserBadge(models.Model): + """ + Tracks badges awarded to users and stores information about how they were awarded. + Each user can only receive a specific badge once (enforced by unique_together). + """ AWARD_METHODS = [
1380-1395: Badge award signal handler for challenge completionsGood implementation of challenge badge awarding with duplicate prevention and user notifications.
Consider adding logging to help with debugging and tracking of badge awards:
@receiver(post_save, sender=ChallengeSubmission) def award_challenge_badge(sender, instance, created, **kwargs): if created: + import logging + logger = logging.getLogger(__name__) + logger.info(f"Checking for badges to award for challenge submission: {instance.id}") challenge_badges = Badge.objects.filter(challenge=instance.challenge, badge_type="challenge", is_active=True) for badge in challenge_badges: if not UserBadge.objects.filter(user=instance.user, badge=badge).exists(): + logger.info(f"Awarding badge {badge.name} to user {instance.user.username}") UserBadge.objects.create( user=instance.user, badge=badge, award_method="challenge_completion", challenge_submission=instance )
1397-1412: Badge award signal handler for course completionsGood implementation of course completion badge awarding with proper check for completed status.
To improve performance with multiple badges, consider using bulk_create:
@receiver(post_save, sender=Enrollment) def award_course_completion_badge(sender, instance, **kwargs): if instance.status == "completed": course_badges = Badge.objects.filter(course=instance.course, badge_type="course", is_active=True) + badges_to_create = [] + notifications_to_create = [] for badge in course_badges: if not UserBadge.objects.filter(user=instance.student, badge=badge).exists(): - UserBadge.objects.create( - user=instance.student, badge=badge, award_method="course_completion", course_enrollment=instance - ) - Notification.objects.create( - user=instance.student, - title=f"New Badge: {badge.name}", - message=f"Congrats! You've earned {badge.name} for completing {instance.course.title}", - notification_type="success", - ) + badges_to_create.append(UserBadge( + user=instance.student, badge=badge, award_method="course_completion", course_enrollment=instance + )) + notifications_to_create.append(Notification( + user=instance.student, + title=f"New Badge: {badge.name}", + message=f"Congrats! You've earned {badge.name} for completing {instance.course.title}", + notification_type="success", + )) + if badges_to_create: + UserBadge.objects.bulk_create(badges_to_create) + Notification.objects.bulk_create(notifications_to_create)
1414-1435: Utility function for teacher-awarded badgesGood implementation with proper checks for teacher status and duplicate awards, and good error handling.
Improve error reporting to help with debugging:
def award_badge_to_student(badge_id, student_id, teacher_id, message=""): try: badge = Badge.objects.get(id=badge_id) student = User.objects.get(id=student_id) teacher = User.objects.get(id=teacher_id) if not teacher.profile.is_teacher: - return None + return {"success": False, "error": "User is not a teacher"} if UserBadge.objects.filter(user=student, badge=badge).exists(): - return None + return {"success": False, "error": "Badge already awarded to student"} user_badge = UserBadge.objects.create( user=student, badge=badge, award_method="teacher_awarded", awarded_by=teacher, award_message=message ) Notification.objects.create( user=student, title=f"New Badge: {badge.name}", message=f"You were awarded {badge.name} by {teacher.username}. {message}", notification_type="success", ) - return user_badge - except (Badge.DoesNotExist, User.DoesNotExist): - return None + return {"success": True, "user_badge": user_badge} + except Badge.DoesNotExist: + return {"success": False, "error": "Badge not found"} + except User.DoesNotExist: + return {"success": False, "error": "User not found"} + except Exception as e: + return {"success": False, "error": str(e)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
web/migrations/0029_badge_userbadge.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/templates/profile.html
- web/migrations/0029_badge_userbadge.py
🧰 Additional context used
🧬 Code Definitions (1)
web/views.py (1)
web/models.py (1) (1)
UserBadge(1350-1377)
🔇 Additional comments (3)
web/views.py (2)
116-116: Import addition looks goodAdding
UserBadgeto the import list is correct and necessary for the badge functionality being implemented.
250-251: Good database query optimizationUsing
select_related("badge")is a good optimization that will fetch the related badge data in a single query, preventing N+1 query issues when rendering the badges in the template.web/models.py (1)
1437-1441: Profile method to retrieve user badgesThe implementation is straightforward and effectively connects the badge functionality to user profiles.
ed6d13b to
797a38e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/models.py (3)
1437-1441: Consider enhancing the Profile extension method.The Profile extension method is a good addition, but could benefit from some improvements.
Consider enhancing the method to:
- Add filtering options (e.g., by badge type)
- Include documentation
-def get_user_badges(self): - return UserBadge.objects.filter(user=self.user) +def get_user_badges(self, badge_type=None): + """ + Retrieve badges for this user with optional filtering by badge type. + + Args: + badge_type (str, optional): Filter by badge type (challenge, course, achievement, teacher_awarded) + + Returns: + QuerySet of UserBadge objects + """ + queryset = UserBadge.objects.filter(user=self.user) + if badge_type: + queryset = queryset.filter(badge__badge_type=badge_type) + return queryset Profile.get_user_badges = get_user_badges
1311-1441: Consider adding additional documentation and badge validation.While the badge implementation is solid, it could benefit from some additional documentation and validation.
Consider adding:
- Class-level docstrings to describe each model's purpose
- Validation in the Badge model to ensure either course OR challenge is set for the appropriate badge types
- Better error messaging for the award_badge_to_student function
Example for Badge model validation:
def clean(self): super().clean() if self.badge_type == "course" and not self.course: raise ValidationError({"course": "Course must be set for course badges"}) if self.badge_type == "challenge" and not self.challenge: raise ValidationError({"challenge": "Challenge must be set for challenge badges"})This would help ensure data integrity and make the system more maintainable.
1311-1441: Register additional admin actions for badge management.The badge implementation could benefit from additional admin actions.
Consider adding:
- Admin actions to activate/deactivate multiple badges at once
- A bulk award feature for specific badge types or courses
Example admin actions for BadgeAdmin class:
@admin.action(description="Mark selected badges as active") def make_active(self, request, queryset): updated = queryset.update(is_active=True) self.message_user(request, f"{updated} badges were successfully activated.") @admin.action(description="Mark selected badges as inactive") def make_inactive(self, request, queryset): updated = queryset.update(is_active=False) self.message_user(request, f"{updated} badges were successfully deactivated.") actions = [make_active, make_inactive]This would enhance the administrative experience and make batch operations easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
web/admin.py(3 hunks)web/migrations/0029_badge_userbadge.py(1 hunks)web/models.py(1 hunks)web/templates/profile.html(1 hunks)web/views.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- web/templates/profile.html
- web/migrations/0029_badge_userbadge.py
- web/views.py
🧰 Additional context used
🧬 Code Definitions (1)
web/admin.py (1)
web/models.py (2) (2)
Badge(1311-1347)UserBadge(1350-1377)
🔇 Additional comments (8)
web/admin.py (3)
13-13: Good job importing new models for admin registration.The Badge and UserBadge model imports have been correctly added to the import list.
Also applies to: 43-43
571-576: The BadgeAdmin configuration looks good.The admin class is properly configured with appropriate list display, filtering, and search fields for the Badge model. The implementation follows the same pattern as other admin classes in this file.
578-583: The UserBadgeAdmin configuration is well-structured.The UserBadgeAdmin class is properly configured with appropriate list display, filtering, and search fields. The date_hierarchy feature will help with chronological organization of badge awards.
web/models.py (5)
1311-1347: Well-designed Badge model with appropriate image processing.The Badge model is well-structured with:
- Clear badge type categories
- Appropriate relationships to Course and Challenge models
- Image processing in the save method to ensure consistent badge sizes
- Good organization with Meta class for ordering
The implementation of image resizing ensures visual consistency across badges.
1350-1377: Comprehensive UserBadge model with proper relationships.The UserBadge model effectively:
- Tracks the relationship between users and badges
- Records how badges were awarded
- Maintains references to related objects (challenge submissions, course enrollments)
- Enforces uniqueness constraints to prevent duplicate badges
- Uses appropriate ordering
The model is well designed for tracking badge awards.
1380-1395: Good implementation of automatic badge awarding for challenges.The signal receiver correctly:
- Triggers when a new challenge submission is created
- Only awards badges that are active and of the right type
- Prevents duplicate badge awards
- Creates a notification to inform the user
This follows good practices for Django signal handlers.
1397-1412: Well-implemented automatic badge awarding for course completion.The signal handler correctly:
- Triggers only when a course is marked as completed
- Only awards active badges of the course type
- Prevents duplicate badge awards
- Creates appropriate notifications
This implementation handles the course completion badge scenario effectively.
1414-1435: Robust utility function for manual badge awarding.The
award_badge_to_studentfunction:
- Properly validates that the awarder is a teacher
- Handles exceptions for non-existent badges or users
- Prevents duplicate badge awards
- Creates appropriate notifications
This function will be valuable for teachers to manually award special badges.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
web/models.py (2)
1442-1478: Badge model is well-structured with good image processing.The Badge model is properly designed with appropriate fields, choices, and relationships. The image processing in the save method ensures consistent image dimensions and format.
One enhancement suggestion:
Consider adding validation for the badge_type field to ensure required related fields are provided. For example, if badge_type is "challenge", then the challenge field should be required.
def clean(self): + if self.badge_type == 'challenge' and not self.challenge: + raise ValidationError("Challenge badge must be associated with a challenge.") + if self.badge_type == 'course' and not self.course: + raise ValidationError("Course badge must be associated with a course.") super().clean()
1545-1566: Manual badge awarding function has good validation.The award_badge_to_student function includes proper validation and error handling. It checks if:
- The badge and users exist
- The awarding user is a teacher
- The student doesn't already have the badge
Consider adding logging for failed badge awards to help with troubleshooting:
def award_badge_to_student(badge_id, student_id, teacher_id, message=""): try: badge = Badge.objects.get(id=badge_id) student = User.objects.get(id=student_id) teacher = User.objects.get(id=teacher_id) if not teacher.profile.is_teacher: + import logging + logger = logging.getLogger(__name__) + logger.warning(f"User {teacher.username} attempted to award badge but is not a teacher") return None if UserBadge.objects.filter(user=student, badge=badge).exists(): + logger.info(f"Badge {badge.name} already awarded to {student.username}") return None # Rest of function... except (Badge.DoesNotExist, User.DoesNotExist) as e: + logger.error(f"Error awarding badge: {str(e)}") return None
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/admin.py(4 hunks)web/models.py(1 hunks)web/views.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/views.py
🧰 Additional context used
🧬 Code Definitions (1)
web/admin.py (1)
web/models.py (2)
Badge(1442-1478)UserBadge(1481-1508)
🪛 GitHub Actions: .github/workflows/test.yml
web/admin.py
[error] 1-1: fix end of files failed. Files were modified by this hook.
🔇 Additional comments (8)
web/admin.py (3)
13-13: Model imports are correctly added.The Badge and UserBadge models have been properly imported from the models module, which is necessary for registering them with the admin site.
Also applies to: 47-47
622-628: Good admin configuration for Badge model.The BadgeAdmin class provides appropriate list display fields, filters, and search capabilities for the Badge model. This implementation follows Django admin best practices by exposing the most relevant fields in the list view and providing useful filtering options.
630-636: Good admin configuration for UserBadge model.The UserBadgeAdmin class is well configured with appropriate list display fields, filters, and search capabilities. The date_hierarchy feature for awarded_at will be particularly useful for administrators to navigate through badge awards chronologically.
web/models.py (5)
1481-1509: UserBadge model appropriately tracks badge awards.The UserBadge model correctly establishes relationships between users and badges, with additional context about how the badge was awarded. The unique_together constraint prevents duplicate awards, which is important.
1511-1526: Challenge badge awarding signal is well-implemented.The award_challenge_badge signal receiver automatically awards badges when a challenge submission is created. It includes proper checks to prevent duplicate awards and creates notifications for users.
1528-1543: Course completion badge signal is well-implemented.The award_course_completion_badge signal receiver correctly awards badges when a course is completed. It includes proper checks to prevent duplicate awards and creates notifications for users.
1568-1572: Profile extension for badge retrieval is clean and efficient.The get_user_badges method provides a simple way to retrieve a user's badges. The assignment to Profile.get_user_badges properly extends the Profile class with this method.
1511-1543: Consider adding automated tests for badge awarding.The automatic badge awarding logic is critical to ensure users receive badges correctly. While I see you mentioned in previous comments that testing can be added later, I recommend prioritizing tests for the signal receivers to ensure they work as expected in all scenarios.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
web/admin.py (1)
636-636:⚠️ Potential issueFix the line ending to resolve pipeline failure.
The pipeline is failing because the file is missing a proper newline at the end. Add a blank line at the end of the file to resolve this issue.
@admin.register(QuizOption) class QuizOptionAdmin(admin.ModelAdmin): list_display = ("text", "question", "is_correct", "order") list_filter = ("is_correct",) search_fields = ("text", "question__text") autocomplete_fields = ["question"] +
🧹 Nitpick comments (2)
web/admin.py (2)
623-628: Consider enhancing the BadgeAdmin configuration.While the basic configuration is good, consider adding these improvements to match the pattern used by other admin classes in the file:
- Add
raw_id_fieldsfor foreign key relationships likecreated_by,course, andchallenge- Add
readonly_fieldsfor timestamps (created_at,updated_at)- Consider organizing fields using
fieldsetsfor a better admin form layout@admin.register(Badge) class BadgeAdmin(admin.ModelAdmin): list_display = ("name", "badge_type", "is_active", "created_by", "created_at") list_filter = ("badge_type", "is_active") search_fields = ("name", "description") + readonly_fields = ("created_at", "updated_at") + raw_id_fields = ("created_by", "course", "challenge") + fieldsets = ( + (None, {"fields": ("name", "description", "image", "badge_type", "is_active")}), + ("Relationships", {"fields": ("course", "challenge", "created_by")}), + ("Criteria", {"fields": ("criteria",)}), + ("Timestamps", {"fields": ("created_at", "updated_at")}), + )
630-636: Consider enhancing the UserBadgeAdmin configuration.Similar to the BadgeAdmin class, consider adding:
raw_id_fieldsfor foreign key relationships likeuser,badge, etc.readonly_fieldsfor timestampsautocomplete_fieldsfor better relationship selection@admin.register(UserBadge) class UserBadgeAdmin(admin.ModelAdmin): list_display = ("user", "badge", "award_method", "awarded_at") list_filter = ("award_method", "badge__badge_type") search_fields = ("user__username", "badge__name") date_hierarchy = "awarded_at" + readonly_fields = ("awarded_at",) + raw_id_fields = ("user", "badge", "challenge_submission", "course_enrollment", "awarded_by") + autocomplete_fields = ["badge"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/admin.py(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
web/admin.py (1)
web/models.py (2)
Badge(1442-1478)UserBadge(1481-1508)
🪛 GitHub Actions: .github/workflows/test.yml
web/admin.py
[error] 1-1: Fix end of files failed. Files were modified by this hook.
🔇 Additional comments (2)
web/admin.py (2)
13-13: Imports are correctly placed and ordered.The imports for
BadgeandUserBadgemodels are correctly added to the import list and maintain the alphabetical ordering.Also applies to: 47-47
13-13: Register models in admin to complete the badge system setup.Good job on adding the Badge and UserBadge models to the admin interface. This completes the initial setup for the badge system as mentioned in the PR objectives. The configuration allows administrators to manage badges and view which badges have been awarded to users.
Also applies to: 47-47, 623-636
Added Badges Model and UI to Show Badge (Initial Setup)
Description: This PR introduces the initial setup for the badges feature in the education platform. The following changes have been made:
Models:
Added Badge model to define badges that can be earned by users.
Added UserBadge model to track badges earned by users.
Views:
Updated the profile view to retrieve and display user badges.
Templates:
Modified profile.html to include a section for displaying user badges.
Next Steps:
Implement logic to award badges upon course completion automatically.
Define additional scenarios for awarding badges (e.g., challenge completion, special achievements).
Create an admin interface for managing badges.
Related Issues:
#96
Note: The actual implementation of how badges should be awarded needs further clarification. Please provide guidance on the specific scenarios and criteria for awarding badges.
Summary by CodeRabbit
This release enhances user engagement by providing clear visual feedback for accomplishments.