-
-
Notifications
You must be signed in to change notification settings - Fork 114
Fix #621: Add clickable links to teacher message email notifications #766
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix #621: Add clickable links to teacher message email notifications #766
Conversation
…tifications
- Add inbox_url and messaging_dashboard_url to email context in views.py
- Update both teacher_message.html templates with action buttons
- Use Tailwind utilities for professional styling
- Add accessibility labels and guards for better UX
- Include helpful footer text explaining options
- Improve user experience by reducing friction in message access
Features:
- Primary button: 'View in Inbox' with teal-300 styling
- Secondary button: 'Messaging Dashboard' with gray styling
- Accessibility: aria-labels for screen readers
- Guards: {% if %} conditions to prevent broken links
- Dark mode: Full dark: variant support
- Focus states: Proper keyboard navigation support
Users can now click directly to inbox or messaging dashboard from emails.
Summary of ChangesHello @omsherikar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the user experience for teachers receiving message notifications by embedding direct, clickable links within the email. This change allows recipients to navigate quickly to their inbox or messaging dashboard, reducing friction and improving accessibility, while also ensuring robust styling and link integrity. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two absolute URLs ( Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant View as message_teacher view
participant URLBuilder as reverse + build_absolute_uri
participant Template as teacher_message email template
participant Recipient as Email client / User
View->>URLBuilder: reverse("inbox"), reverse("messaging_dashboard")
URLBuilder-->>View: inbox_url, messaging_dashboard_url
View->>Template: render(email with inbox_url & messaging_dashboard_url)
Template-->>Recipient: send email (conditional buttons)
alt Clicks "View in Inbox"
Recipient->>URLBuilder: GET inbox_url
URLBuilder-->>Recipient: Inbox page
else Clicks "Messaging Dashboard"
Recipient->>URLBuilder: GET messaging_dashboard_url
URLBuilder-->>Recipient: Messaging dashboard
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (5)web/templates/**/*.html📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{html,htm}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{html,htm,css}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.py📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
web/views.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (8)📓 Common learnings📚 Learning: 2025-12-15T00:22:20.785ZApplied to files:
📚 Learning: 2025-12-15T00:22:20.785ZApplied to files:
📚 Learning: 2025-12-15T00:22:20.785ZApplied to files:
📚 Learning: 2025-12-15T00:22:20.785ZApplied to files:
📚 Learning: 2025-12-15T00:22:20.785ZApplied to files:
📚 Learning: 2025-03-26T01:18:47.700ZApplied to files:
📚 Learning: 2025-03-26T01:18:47.700ZApplied to files:
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request is a great enhancement for user experience, making it easier for teachers to access their messages directly from email notifications. The implementation is straightforward and correct. My main feedback is regarding code duplication in the two updated HTML templates. I've added a specific comment suggesting the use of a Django template include to centralize the new button logic, which will improve maintainability. Additionally, it would be beneficial to add a unit test to verify that the new URLs are correctly passed to the email template context, ensuring the robustness of this new feature.
|
@coderabbitai review this pr |
|
@omsherikar: I'll review this PR for you. Let me analyze the changes related to adding clickable links to teacher message email notifications. ✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
web/templates/emails/teacher_message.html(1 hunks)web/templates/web/emails/teacher_message.html(1 hunks)web/views.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.html
📄 CodeRabbit inference engine (.cursorrules)
**/*.html: Always use Tailwind CSS utility classes for styling HTML elements
Never use inline styles on elements (no style="...")
Use the project color scheme via Tailwind classes: teal-300 (primary), gray-600 (secondary), green-600 (success), yellow-600 (warning), red-600 (danger), with dark: variants
Containers should use: container mx-auto px-4
Cards should use: bg-white dark:bg-gray-800 rounded-lg shadow-lg
Form inputs should use: w-full px-4 py-2 border border-gray-300 dark:border-gray-600 rounded-lg focus:ring-2 focus:ring-blue-500
Form labels should use: block text-sm font-medium text-gray-700 dark:text-gray-300
Primary buttons should use: bg-teal-300 hover:bg-teal-400 text-white px-6 py-2 rounded-lg transition duration-200
Secondary buttons should use: bg-gray-100 hover:bg-gray-200 dark:bg-gray-700 text-gray-700 dark:text-gray-300
Headings use Tailwind typography: text-2xl font-bold (adjust size as needed)
Body text should use: text-gray-600 dark:text-gray-300
Links should use: text-blue-600 hover:text-blue-800 dark:text-blue-400
Use flex and grid utilities for layouts instead of custom CSS
Use consistent spacing with gap-{size} and space-{x|y}-{size} utilities
Ensure responsive design using Tailwind responsive prefixes (sm:, md:, lg:, xl:)
Always include dark mode alternatives using the dark: prefix where appropriate
Use dark:bg-gray-800 for card backgrounds in dark mode
Use dark:text-gray-300 for text in dark mode
Maintain proper heading hierarchy (h1 > h2 > h3...)
Include appropriate ARIA labels and roles where needed
Ensure sufficient color contrast between text and background
Add visible hover and focus states for all interactive elements
Files:
web/templates/emails/teacher_message.htmlweb/templates/web/emails/teacher_message.html
web/templates/**/*.html
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/templates/**/*.html: Format HTML/Django templates with djlint using 120-character line length
Use Tailwind CSS classes only in templates (no custom CSS or inline styles)
Include dark mode variants using the "dark:" prefix in templates
Ensure proper HTML structure and accessibility in templates
Follow project color scheme in templates (teal-300, gray-600, green-600, yellow-600, red-600)
Files:
web/templates/emails/teacher_message.htmlweb/templates/web/emails/teacher_message.html
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Format Python code with Black using 120-character line length
Sort Python imports with isort using the Black profile
Python code must pass flake8 with max line length 120
Follow PEP 8 guidelines for Python code
Maximum Python line length is 120 characters
Use type hints in Python where appropriate
Add docstrings to Python functions, classes, and modules
Files:
web/views.py
🔇 Additional comments (1)
web/templates/emails/teacher_message.html (1)
38-49: Approve template guards and accessibility features.The conditional rendering with
{% if inbox_url %}and{% if messaging_dashboard_url %}guards are well-implemented and prevent broken links. Aria-labels on both anchors support screen reader accessibility. This aligns well with the PR objectives.
- Replace Tailwind classes with custom CSS styles in <style> blocks - Add .button, .button-primary, .button-secondary classes - Use teal-300 (#2dd4bf) and teal-400 (#14b8a6) colors for primary button - Use gray colors for secondary button styling - Add proper focus states and hover effects - Ensure buttons render correctly in email clients without Tailwind Email clients don't load Tailwind CSS, so explicit CSS is required for proper button styling. This ensures the clickable buttons appear as professional action buttons rather than plain blue links.
105ac12 to
c2f4af4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/templates/web/emails/teacher_message.html (1)
47-114:⚠️ Major code duplication: button markup and styling identical in both email templates.The button group markup (lines 101–114) and button CSS styling (lines 47–82) in this file are identical to those in
web/templates/emails/teacher_message.html. This duplication makes maintenance harder—any future button updates must be made in two places, increasing the risk of inconsistencies.Extract the shared button styles and markup into a reusable template partial at
web/templates/emails/_action_buttons.html:New partial file:
web/templates/emails/_action_buttons.html<div class="button-group"> {% if inbox_url %} <a href="{{ inbox_url }}" aria-label="View this message in your inbox" class="button button-primary"> View in Inbox </a> {% endif %} {% if messaging_dashboard_url %} <a href="{{ messaging_dashboard_url }}" aria-label="Open messaging dashboard" class="button button-secondary"> Messaging Dashboard </a> {% endif %} </div>Shared CSS partial:
web/templates/emails/_button_styles.html<style> .button-group { text-align: center; margin: 24px 0; } .button { display: inline-block; padding: 10px 24px; border-radius: 8px; font-weight: 600; text-decoration: none; margin: 0 8px; } .button:focus { outline: 2px solid #2563eb; outline-offset: 2px; } .button-primary { background-color: #2dd4bf; color: #ffffff; } .button-primary:hover { background-color: #14b8a6; } .button-secondary { background-color: #f3f4f6; color: #374151; } .button-secondary:hover { background-color: #e5e7eb; } </style>Then in both templates, replace the inline styles and button markup with:
{% include "emails/_button_styles.html" %} ... {% include "emails/_action_buttons.html" %}
♻️ Duplicate comments (2)
web/templates/web/emails/teacher_message.html (1)
98-98: Inconsistent message formatting: missinglinebreaksfilter.Line 98 renders
{{ message }}without thelinebreaksfilter, while the other email template (web/templates/emails/teacher_message.htmlline 72) uses{{ message|linebreaks }}. Without the filter, newlines in the message body are not preserved in HTML output, resulting in mangled formatting.Apply the linebreaks filter to ensure proper text wrapping:
- <div class="message-content">{{ message }}</div> + <div class="message-content">{{ message|linebreaks }}</div>web/templates/emails/teacher_message.html (1)
46-62: Fix color contrast issue while addressing guideline conflict: email templates using inline styles should apply accessible colors.The review comment is correct—the contrast ratio of 1.86:1 fails WCAG AA standards (requires 4.5:1 for normal text, 3:1 for large text). However, this reveals a conflict: the project's
.cursorrulesguidelines recommend exactly this color combination (bg-teal-300 hover:bg-teal-400 text-white), which is inaccessible.The file currently violates styling rules by using inline
<style>tags with custom CSS classes instead of Tailwind. For this email template, the fix should:
- Change
.button-primarytext color to#134e4a(teal-900) or#000000for sufficient contrast- Consider whether email templates should be exempt from Tailwind-only requirements due to email client CSS limitations
- Address the guideline flaw: the project standards recommend colors that fail accessibility
.button-primary { background-color: #2dd4bf; /* teal-300 */ - color: #ffffff; + color: #134e4a; /* teal-900 for WCAG AA compliance */ } .button-primary:hover { background-color: #14b8a6; /* teal-400 */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/templates/emails/teacher_message.html(2 hunks)web/templates/web/emails/teacher_message.html(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.html
📄 CodeRabbit inference engine (.cursorrules)
**/*.html: Always use Tailwind CSS utility classes for styling HTML elements
Never use inline styles on elements (no style="...")
Use the project color scheme via Tailwind classes: teal-300 (primary), gray-600 (secondary), green-600 (success), yellow-600 (warning), red-600 (danger), with dark: variants
Containers should use: container mx-auto px-4
Cards should use: bg-white dark:bg-gray-800 rounded-lg shadow-lg
Form inputs should use: w-full px-4 py-2 border border-gray-300 dark:border-gray-600 rounded-lg focus:ring-2 focus:ring-blue-500
Form labels should use: block text-sm font-medium text-gray-700 dark:text-gray-300
Primary buttons should use: bg-teal-300 hover:bg-teal-400 text-white px-6 py-2 rounded-lg transition duration-200
Secondary buttons should use: bg-gray-100 hover:bg-gray-200 dark:bg-gray-700 text-gray-700 dark:text-gray-300
Headings use Tailwind typography: text-2xl font-bold (adjust size as needed)
Body text should use: text-gray-600 dark:text-gray-300
Links should use: text-blue-600 hover:text-blue-800 dark:text-blue-400
Use flex and grid utilities for layouts instead of custom CSS
Use consistent spacing with gap-{size} and space-{x|y}-{size} utilities
Ensure responsive design using Tailwind responsive prefixes (sm:, md:, lg:, xl:)
Always include dark mode alternatives using the dark: prefix where appropriate
Use dark:bg-gray-800 for card backgrounds in dark mode
Use dark:text-gray-300 for text in dark mode
Maintain proper heading hierarchy (h1 > h2 > h3...)
Include appropriate ARIA labels and roles where needed
Ensure sufficient color contrast between text and background
Add visible hover and focus states for all interactive elements
Files:
web/templates/emails/teacher_message.htmlweb/templates/web/emails/teacher_message.html
web/templates/**/*.html
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
web/templates/**/*.html: Format HTML/Django templates with djlint using 120-character line length
Use Tailwind CSS classes only in templates (no custom CSS or inline styles)
Include dark mode variants using the "dark:" prefix in templates
Ensure proper HTML structure and accessibility in templates
Follow project color scheme in templates (teal-300, gray-600, green-600, yellow-600, red-600)
Files:
web/templates/emails/teacher_message.htmlweb/templates/web/emails/teacher_message.html
🧠 Learnings (1)
📚 Learning: 2025-08-25T00:43:03.755Z
Learnt from: CR
PR: alphaonelabs/alphaonelabs-education-website#0
File: .cursorrules:0-0
Timestamp: 2025-08-25T00:43:03.755Z
Learning: Applies to **/*.html : Primary buttons should use: bg-teal-300 hover:bg-teal-400 text-white px-6 py-2 rounded-lg transition duration-200
Applied to files:
web/templates/emails/teacher_message.html
🔇 Additional comments (3)
web/templates/emails/teacher_message.html (2)
27-62: ✅ Button styling correctly uses inline CSS for email compatibility.The new CSS rules for buttons use email-safe inline styles instead of Tailwind utilities, which will render properly in email clients. The class-based approach makes the markup cleaner and more maintainable.
89-91: 🔴 Critical: Footer contains Tailwind utility classes that won't render in email clients.Lines 89–91 use Tailwind CSS utility classes (
mt-8,pt-5,border-t,border-gray-200,dark:border-gray-700,text-gray-600,dark:text-gray-300,text-sm). Email clients (Gmail, Outlook, Apple Mail) do not support external stylesheets or Tailwind utilities. These classes will be stripped and the footer will render unstyled. Dark mode variants are especially unsupported.Replace all Tailwind classes with inline CSS in a
<style>block. Add this rule near the existing button styles:.button-secondary:hover { background-color: #e5e7eb; } + + .footer-note { + margin-top: 32px; + padding-top: 20px; + border-top: 1px solid #e5e7eb; + color: #666; + font-size: 14px; + }Then update the footer markup:
- <div class="mt-8 pt-5 border-t border-gray-200 dark:border-gray-700"> - <p class="text-gray-600 dark:text-gray-300 text-sm">You can reply to this message by directly responding to this email, or click the buttons above to view it in your account.</p> + <div class="footer-note"> + <p>You can reply to this message by directly responding to this email, or click the buttons above to view it in your account.</p>Likely an incorrect or invalid review comment.
web/templates/web/emails/teacher_message.html (1)
47-82: ✅ Button styling correctly uses inline CSS for email compatibility.The new CSS rules for buttons use email-safe inline styles instead of Tailwind utilities, which will render properly in email clients. The class-based approach makes the markup cleaner and more maintainable.
|
@A1L13N please take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Features:
Users can now click directly to inbox or messaging dashboard from emails.
Related issues
Fixes #621
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.