Shareable Certificates: Allows users to share certificates or badges for course completions on LinkedIn, Twitter, and Facebook. #14New shareable certificates#128
Conversation
WalkthroughThis pull request introduces a new Certificate model to the Django application. A migration file is added to define the database schema, and a corresponding model is created with fields for UUID, completion date, timestamps, and foreign key relationships to a Course and the user model. In addition, the pull request adds two new view functions and URL endpoints for displaying certificate details and generating certificates when course completions are confirmed. New templates are also added to show certificate details and to list certificates on the student dashboard, including options to generate certificates for completed enrollments. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as GenerateCertificateView
participant DB as Database
participant C as CertificateModel
U->>V: Request certificate generation (enrollment_id)
V->>DB: Retrieve enrollment for current user
alt Enrollment not completed
V-->>U: Return error & redirect to dashboard
else Enrollment completed
V->>DB: Check if certificate already exists
alt Certificate exists
V-->>U: Inform certificate exists & redirect to certificate detail
else Certificate does not exist
V->>C: Create new certificate record
C-->>V: Certificate created
V-->>U: Return success message & redirect to certificate detail
end
end
sequenceDiagram
participant U as User
participant V as CertificateDetailView
participant DB as Database
participant T as Template
U->>V: Request certificate detail (certificate_id)
V->>DB: Retrieve certificate by certificate_id
alt Unauthorized access
V-->>U: Return forbidden response
else Authorized
V->>T: Render certificate detail template with certificate data
T-->>U: Display certificate details
end
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (8)
web/models.py (1)
1311-1327: Model implementation looks clean, but consider adding social sharing metadata fields.The
Certificatemodel provides a solid foundation for the certificate feature with appropriate use of UUID for the certificate ID and proper timestamp fields. The relationships to user and course are well-defined.However, considering the PR objective of enabling social sharing on platforms like LinkedIn, Twitter, and Facebook, consider adding metadata fields to optimize the sharing experience:
class Certificate(models.Model): # Certificate Model certificate_id = models.UUIDField(default=uuid.uuid4, editable=False, unique=True) completion_date = models.DateField(auto_now_add=True) course = models.ForeignKey( "web.Course", on_delete=models.CASCADE, related_name="certificates", null=True, blank=True ) user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="certificates") # New fields added as per feedback created_at = models.DateTimeField(auto_now_add=True) modified_at = models.DateTimeField(auto_now=True) + # Social sharing metadata + share_title = models.CharField(max_length=200, blank=True, help_text="Custom title for social media sharing") + share_description = models.TextField(blank=True, help_text="Custom description for social media sharing") + share_image = models.ImageField(upload_to="certificate_shares/", blank=True, help_text="Custom image for social media sharing") def __str__(self): course_title = self.course.title if self.course else "No Course" return f"Certificate for {self.user.username} - {course_title}" + + def get_absolute_url(self): + """Returns the URL for viewing this certificate.""" + from django.urls import reverse + return reverse("certificate_detail", kwargs={"certificate_id": self.certificate_id}) + + def get_social_share_urls(self): + """Returns a dictionary of social media sharing URLs.""" + absolute_url = self.get_absolute_url() + base_url = "https://example.com" # Should be replaced with settings.SITE_URL in real code + full_url = f"{base_url}{absolute_url}" + title = self.share_title or f"I completed {self.course.title}" if self.course else "My Certificate" + + return { + "linkedin": f"https://www.linkedin.com/sharing/share-offsite/?url={full_url}", + "twitter": f"https://twitter.com/intent/tweet?url={full_url}&text={title}", + "facebook": f"https://www.facebook.com/sharer/sharer.php?u={full_url}", + }web/views.py (2)
3790-3797: Certificate view implementation is proper but could use more context in the error message.The view correctly retrieves the certificate by UUID and checks user permissions before rendering the detail template. Permission check is properly implemented to allow both the certificate owner and staff members to view certificates.
Consider enhancing the forbidden message to provide more context:
- return HttpResponseForbidden("You don't have permission to view this certificate") + return HttpResponseForbidden("You don't have permission to view this certificate. Only the certificate owner and staff members can access it.")
3801-3818: Certificate generation logic is correct but could benefit from transaction wrapping.The implementation handles all expected cases: checking if the course is completed, verifying if a certificate already exists, and creating a new one if needed. Good use of messaging to communicate the status to the user.
Consider wrapping the certificate creation in a transaction to ensure database consistency:
- # Create a new certificate record manually - certificate = Certificate.objects.create(user=request.user, course=enrollment.course) + # Create a new certificate record with transaction protection + from django.db import transaction + with transaction.atomic(): + certificate = Certificate.objects.create(user=request.user, course=enrollment.course)web/templates/courses/certificate_detail.html (1)
30-44: Social share URLs could be improved for better sharing experience.While the social share buttons are functional, the sharing URLs could be optimized to provide more context when shared.
Enhance the social sharing URLs with more descriptive content:
- <a href="https://www.linkedin.com/sharing/share-offsite/?url={{ request.build_absolute_uri }}" + <a href="https://www.linkedin.com/sharing/share-offsite/?url={{ request.build_absolute_uri }}&title=Certificate%20of%20Completion%20-%20{{ certificate.course.title|urlencode }}&summary=I%20completed%20{{ certificate.course.title|urlencode }}%20on%20{{ certificate.completion_date|date:'F d, Y'|urlencode }}" target="_blank" class="px-6 py-3 bg-blue-700 hover:bg-blue-800 text-white rounded-lg"> Share on LinkedIn </a> - <a href="https://twitter.com/intent/tweet?url={{ request.build_absolute_uri }}&text=I%20just%20completed%20{{ certificate.course.title }}!" + <a href="https://twitter.com/intent/tweet?url={{ request.build_absolute_uri }}&text=I%20just%20earned%20a%20certificate%20for%20completing%20{{ certificate.course.title|urlencode }}!%20%23learning%20%23achievement" target="_blank" class="px-6 py-3 bg-blue-500 hover:bg-blue-600 text-white rounded-lg"> Share on Twitter </a>Also, consider using Django's
urlencodefilter for all variables in the URLs to ensure special characters are properly encoded.web/templates/dashboard/student.html (1)
141-142: Empty paragraph tag in the "no certificates" case.There's an empty paragraph tag when no certificates are available.
Replace the empty paragraph with a more informative message:
- <p class="text-gray-500 dark:text-gray-400"></p> + <p class="text-gray-500 dark:text-gray-400">You haven't earned any certificates yet. Complete a course to receive a certificate.</p>web/tests/certificate_tests.py (3)
5-6: Remove unused imports.The static analysis tool has identified unused imports:
CourseProgressandtimezone. These should be removed to keep the code clean.-from web.models import Certificate, Course, Enrollment, Subject, CourseProgress -from django.utils import timezone +from web.models import Certificate, Course, Enrollment, Subject🧰 Tools
🪛 Ruff (0.8.2)
5-5:
web.models.CourseProgressimported but unusedRemove unused import:
web.models.CourseProgress(F401)
6-6:
django.utils.timezoneimported but unusedRemove unused import:
django.utils.timezone(F401)
43-46: Consider adding a comment explaining the hard-coded UUID.You're using a specific UUID for testing purposes. Consider adding a comment explaining why this particular UUID is used to improve code readability.
- self.certificate_uuid = uuid.UUID("9775783c-9f74-4aeb-8a47-5128a3b7b73a") + # Using a fixed UUID for predictable test results + self.certificate_uuid = uuid.UUID("9775783c-9f74-4aeb-8a47-5128a3b7b73a")
85-87: Use consistent string quotes to improve readability.The escaped quotes in the string make it harder to read. Consider using single quotes for the outer string to avoid escaping the inner quotes.
- expected_course_text = f"Course: <span class=\"font-bold\">{self.course.title}</span>" + expected_course_text = f'Course: <span class="font-bold">{self.course.title}</span>'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
web/migrations/0028_certificate.py(1 hunks)web/models.py(1 hunks)web/templates/courses/certificate_detail.html(1 hunks)web/templates/dashboard/student.html(1 hunks)web/tests/certificate_tests.py(1 hunks)web/urls.py(1 hunks)web/views.py(3 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
web/tests/certificate_tests.py (1)
web/models.py (7) (7)
Certificate(1311-1326)Course(168-262)Enrollment(452-471)Subject(132-150)CourseProgress(496-523)price(1018-1025)delete(369-375)
web/urls.py (1)
web/views.py (2) (2)
certificate_detail(3790-3797)generate_certificate(3801-3818)
web/views.py (1)
web/models.py (2) (2)
Certificate(1311-1326)Enrollment(452-471)
🪛 Ruff (0.8.2)
web/tests/certificate_tests.py
5-5: web.models.CourseProgress imported but unused
Remove unused import: web.models.CourseProgress
(F401)
6-6: django.utils.timezone imported but unused
Remove unused import: django.utils.timezone
(F401)
🪛 GitHub Actions: Tests
web/tests/certificate_tests.py
[error] 1-1: trailing-whitespace hook failed. Files were modified by this hook.
🔇 Additional comments (12)
web/models.py (1)
1315-1317: Consider whether certificates should always be tied to a course.The
coursefield is currently optional (null=True, blank=True), which means certificates can exist without being associated with a course. Given that the purpose of these certificates is to recognize course completion, it might be more appropriate to make this field required.Is there a specific use case where a certificate wouldn't be associated with a course? If not, consider making the course field required.
web/views.py (1)
85-85: Addition of Certificate import looks correct.This import for the Certificate model aligns with the new certificate functionality being implemented.
web/templates/courses/certificate_detail.html (1)
1-48: Well-structured certificate template with social sharing functionality.The certificate template is well-organized with clean HTML structure and appropriate styling classes. The social sharing buttons are properly implemented with the correct sharing URLs for LinkedIn, Twitter, and Facebook.
The template extends the base template correctly and uses appropriate font sizes and spacing for a certificate design. The social sharing capabilities align well with the PR objective of allowing users to share their achievements.
web/urls.py (1)
15-16: New URL patterns correctly configured.The certificate URL patterns are properly defined to handle both viewing certificate details and generating new certificates, with appropriate parameter types.
Good choice to add these URLs outside the language-prefixed patterns to ensure certificates can be shared without language barriers.
web/templates/dashboard/student.html (1)
124-158: Certificate section integrates well with the existing dashboard design.The new "Your Certificates" section follows the established design patterns and provides a clean interface for viewing and generating certificates.
The implementation correctly handles both displaying existing certificates and providing generation buttons for completed courses. The conditional logic is well-structured.
web/migrations/0028_certificate.py (3)
20-24: LGTM! Solid timestamps implementation.The model includes both creation and modification timestamps, which is a good practice for tracking when certificates are generated and updated.
8-44: Migration looks well-structured and follows Django conventions.The Certificate model has appropriate relationships and fields for the certificate sharing functionality. The UUID field for certificate_id provides a secure and unique identifier that can be safely shared in URLs.
27-33:Details
❓ Verification inconclusive
Consider if course relationship should be optional.
The
coursefield is defined as optional (null=True, blank=True), which means certificates can exist without being associated with a course. Based on the PR objectives, certificates are meant to be generated when students complete a course, so this design choice should be reviewed.While this might be intentional to support certificates for achievements beyond courses, it's worth confirming this is the desired behavior.
🏁 Script executed:
#!/bin/bash # Check if the Certificate model is used without a course elsewhere in the codebase echo "Searching for certificate creations without a course..." rg -A 2 "Certificate.objects.create" | grep -v "course="Length of output: 628
Action Required: Confirm Optional Course Association in Certificate Model
The migration snippet below defines the course relationship as optional:
models.ForeignKey( blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name="certificates", to="web.course", ),However, our search indicates that certificates are being created in both tests (
web/tests/certificate_tests.py) and in views (web/views.py) without a specified course. Since the PR objectives indicate that certificates should be generated upon course completion, please verify whether allowing certificates to exist without an associated course is intentional. If the design needs to support non-course-related certificates, consider adding documentation to clarify this behavior.web/tests/certificate_tests.py (4)
69-86: LGTM! Comprehensive test of the dashboard certificates section.The test properly simulates the scenario where a certificate hasn't been generated yet for a completed enrollment, ensuring the "Generate" button appears on the dashboard. The detailed comments also clearly explain the test's intention.
50-59: LGTM! Good test for certificate creation.The test properly verifies both the automatic UUID generation and the string representation of the certificate.
60-68: LGTM! Thorough testing of certificate detail view.The test ensures the certificate detail view renders correctly by checking both the HTTP status code and the presence of key content elements.
8-49: Test setup is comprehensive and well-structured.The test setup creates all the necessary objects for testing certificate functionality, including a user, subject, course, enrollment, and certificate. The setup also properly logs in the user, which is essential for testing authenticated views.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
web/tests/certificate_tests.py (1)
69-86:⚠️ Potential issueFix trailing whitespace on line 77.
There's a trailing whitespace after the certificate deletion that needs to be removed.
- self.certificate.delete() - + self.certificate.delete()Otherwise, the dashboard certificates section test is well-structured and checks for all key elements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/models.py(1 hunks)web/templates/dashboard/student.html(1 hunks)web/tests/certificate_tests.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/templates/dashboard/student.html
🧰 Additional context used
🧬 Code Definitions (1)
web/tests/certificate_tests.py (1)
web/models.py (6) (6)
Certificate(1311-1345)Course(168-262)Enrollment(452-471)Subject(132-150)price(1018-1025)delete(369-375)
🪛 GitHub Actions: Tests
web/models.py
[error] 1-1: Trailing whitespace found. The hook 'trailing-whitespace' failed and modified the file.
🪛 Ruff (0.8.2)
web/tests/certificate_tests.py
5-5: web.models.CourseProgress imported but unused
Remove unused import: web.models.CourseProgress
(F401)
6-6: django.utils.timezone imported but unused
Remove unused import: django.utils.timezone
(F401)
🔇 Additional comments (4)
web/models.py (1)
1311-1346: Implementation looks good with clean validation logic.The Certificate model has been properly implemented with all necessary fields and a validation mechanism that ensures certificates are only created for completed courses. The model includes a clean method that verifies the user has actually completed the course by checking for an enrollment with 'completed' status before allowing a certificate to be created.
web/tests/certificate_tests.py (3)
8-49: Well-structured test setup.The test setup is thorough and creates all necessary test objects including user, subject, course, enrollment, and certificate. Good job using a predefined UUID for consistent testing.
50-59: Good certificate creation test.This test effectively verifies both the certificate creation and its string representation.
60-68: Comprehensive certificate detail view test.This test properly verifies that the certificate detail view displays all the essential certificate information.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/models.py(1 hunks)web/tests/certificate_tests.py(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
web/tests/certificate_tests.py (1)
web/models.py (6) (6)
Certificate(1311-1370)Course(168-262)Enrollment(452-471)Subject(132-150)price(1018-1025)delete(369-375)
web/models.py (1)
web/forms.py (10) (10)
clean(337-359)clean(560-575)clean(784-789)clean(1016-1038)clean(1140-1148)save(153-176)save(421-446)save(505-515)save(1040-1053)save(1150-1165)
🪛 Ruff (0.8.2)
web/models.py
1330-1330: django.core.exceptions.ValidationError imported but unused
Remove unused import: django.core.exceptions.ValidationError
(F401)
1331-1331: Redefinition of unused clean from line 1328
(F811)
1354-1354: Redefinition of unused clean from line 1331
(F811)
🪛 GitHub Actions: Tests
web/models.py
[error] 1-1: Trailing whitespace found. The hook 'trailing-whitespace' modified the file.
🔇 Additional comments (5)
web/models.py (1)
1311-1327: LGTM: Certificate model structure is well-designed.The Certificate model is well structured with:
- A unique UUID field for certificate identification
- Proper foreign key relationships to Course and User
- Appropriate timestamp fields
- A clear string representation
web/tests/certificate_tests.py (4)
6-47: LGTM: Test setup is comprehensive and well-structured.The test setup creates all necessary objects (user, subject, course, enrollment, certificate) with appropriate relationships, making it easy to test certificate functionality. Using a fixed UUID for the test certificate is a good practice for predictable testing.
48-57: LGTM: Certificate creation test is effective.This test properly checks two important aspects:
- Certificate creation with auto-generated UUID
- The string representation of the certificate
58-66: LGTM: Certificate detail view test is comprehensive.This test effectively verifies:
- The response status code is correct
- The view contains essential certificate elements (title, user name, course title)
67-84: LGTM: Dashboard integration test is thorough.This test correctly:
- Simulates the scenario where a certificate needs to be generated
- Verifies the "Generate" button appears for completed enrollments
- Checks that course information is properly displayed
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/models.py (1)
1313-1318: Consider adding a field for certificate template or styling.While the basic structure is in place, adding a field to store template information or styling preferences would enhance the flexibility of certificate generation. This would allow for different certificate designs for different courses or achievement levels.
# Certificate Model certificate_id = models.UUIDField(default=uuid.uuid4, editable=False, unique=True) completion_date = models.DateField(auto_now_add=True) course = models.ForeignKey( "web.Course", on_delete=models.CASCADE, related_name="certificates", null=True, blank=True ) user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, related_name="certificates") + template = models.CharField( + max_length=50, + default="default", + choices=[ + ("default", "Default Template"), + ("premium", "Premium Template"), + ("advanced", "Advanced Template"), + ], + help_text="Certificate template design" + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/migrations/0028_certificate.py(1 hunks)web/models.py(1 hunks)web/templates/courses/certificate_detail.html(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/templates/courses/certificate_detail.html
- web/migrations/0028_certificate.py
🧰 Additional context used
🧬 Code Definitions (1)
web/models.py (1)
web/forms.py (10) (10)
clean(337-359)clean(560-575)clean(784-789)clean(1016-1038)clean(1140-1148)save(153-176)save(421-446)save(505-515)save(1040-1053)save(1150-1165)
🔇 Additional comments (4)
web/models.py (4)
1311-1342: Good implementation of the Certificate model with validation logic in place.The Certificate model is well-structured with appropriate fields and validation to ensure certificates are only created for completed courses. The model includes a UUID for unique identification, timestamps, and proper string representation.
1328-1338: Great validation implementation to ensure certificate integrity.The validation logic correctly ensures that certificates can only be created for users who have completed the associated course, maintaining data integrity and preventing unauthorized certificate generation.
1339-1341: Excellent use of full_clean() before saving.Using
full_clean()in the save method ensures that validation is always performed before saving the model, which helps maintain data integrity.
1316-1317:Details
❓ Verification inconclusive
Verify the need for null/blank in the course field.
The
coursefield allows null and blank values, which means certificates can be created without being associated with a course. Given that the PR is specifically about course completion certificates, consider whether this flexibility is intentionally needed.
🏁 Script executed:
#!/bin/bash # Check if there are any view functions that create certificates without courses rg -A 10 "Certificate\(" --type pyLength of output: 736
Action Required: Reevaluate Course Field Optionality
Based on our examination, the search only shows the Certificate model definition and no view functions creating certificates without a course. This indicates that, in practice, the course field is likely always provided. Given that the PR focuses on course completion certificates, please confirm whether allowing
nullandblankfor thecoursefield is intentional or if it should be made mandatory.
- File:
web/models.py(lines 1316-1317)- Suggestion: If a certificate must always be associated with a course, consider removing
null=Trueandblank=Trueto enforce data integrity.
2 participants
I implemented a shareable certificates feature that automatically generates a certificate when a student completes a course. The certificate is rendered as a beautifully styled HTML page—with bold, modern typography and vibrant colors—and includes social share buttons for LinkedIn, Twitter, and Facebook. I also updated the student dashboard to include a "Your Certificates" section so students can easily view and share their achievements.
Summary by CodeRabbit