feat: Add automatic assignment deadline reminder system#1044
feat: Add automatic assignment deadline reminder system#1044nourzakhama2003 wants to merge 2 commits intoalphaonelabs:mainfrom
Conversation
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Reminder Core Logic web/assignment_reminders.py |
New module with ReminderScheduler class implementing due-date windowing logic for early (24–48h) and final (<24h) reminders, email rendering via Django templates with context data, and signal-driven triggering. Includes exception handling, logging, and HTML-to-text conversion utility. |
Signal Registration web/signals.py |
Added import of check_and_send_assignment_reminders to integrate the new reminder signal handler into Django's signal dispatcher. |
Email Templates web/templates/emails/assignment_reminder_24h.html, web/templates/emails/assignment_reminder_final.html |
Two new styled HTML email templates with Django template variables for student name, course/assignment titles, formatted due dates, optional description, and action links. The final reminder template includes urgency callouts and late-work warnings. |
Test Coverage web/tests/test_assignment_reminders.py |
New test module with three test classes validating: reminder decision logic for date windows and state flags, signal triggering and email delivery to approved enrollments only, and reminder flag state management. |
Configuration web/asgi.py |
Updated comment clarifying Django initialization ensures models and routing can be imported without issues. |
Sequence Diagram(s)
sequenceDiagram
participant App as Application
participant Signal as Django Signals
participant Scheduler as ReminderScheduler
participant DB as Database
participant Email as Email Service
App->>Signal: Save CourseMaterial (assignment with due_date)
Signal->>Scheduler: Trigger post_save signal handler
Scheduler->>Scheduler: Check should_send_early_reminder()
alt Early reminder due (24-48h away)
Scheduler->>DB: Query approved Enrollments for course
Scheduler->>Email: Render & send early reminder emails
Email-->>Scheduler: Success/Failure logged
Scheduler->>DB: Update CourseMaterial.reminder_sent = True
end
Scheduler->>Scheduler: Check should_send_final_reminder()
alt Final reminder due (<24h away)
Scheduler->>DB: Query approved Enrollments for course
Scheduler->>Email: Render & send final reminder emails
Email-->>Scheduler: Success/Failure logged
Scheduler->>DB: Update CourseMaterial.final_reminder_sent = True
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main feature: an automatic assignment deadline reminder system is the primary change across all modified files. |
| Docstring Coverage | ✅ Passed | Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
|
You can't take this task yet. You still have uncompleted issues: #1040. Please complete them before requesting another. |
👀 Peer Review RequiredHi @nourzakhama2003! This pull request does not yet have a peer review. Before this PR can be merged, please request a review from one of your peers:
Thank you for contributing! 🎉 |
|
Hi team, This PR is ready for review. Could someone please take a look? @ayesha1145 @omsherikar (or any other active contributor you see) Thank you! |
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/assignment_reminders.py`:
- Around line 67-114: The current try/except around the whole send flow causes
one failed send to abort the loop; instead wrap the per-recipient send and its
success log in a per-enrollment try/except (around send_mail and the subsequent
logger.info) inside the for enrollment in enrollments loop so a single failure
is caught, logged, and the loop continues; track failures (e.g., a failed_sends
list or counter) while leaving Enrollment, course_material, send_mail, and
logger references intact, and only set course_material.reminder_sent = True and
call course_material.save(update_fields=["reminder_sent"]) if failed_sends is
empty (otherwise log the failed recipients and do not mark the reminder as
sent).
- Line 27: Add explicit type hints for should_send_early_reminder and every
other function in this module that lacks annotations (the functions referenced
at the comment: lines 46, 65, 116, 168, 189). For should_send_early_reminder,
annotate the parameter (e.g., course_material: dict | CourseMaterial |
typing.Mapping[str, typing.Any]) and the return type (-> bool) to reflect it
returns a boolean. For the other functions, add appropriate parameter and return
annotations using concrete types, typing primitives (str, int, bool),
typing.Dict/Mapping, Optional[...] or custom dataclasses/TypedDicts where the
structure is known, and use -> None for procedures; import typing names as
needed. Ensure all function defs in this module include these annotations so
linters accept them and maintainers understand expected types.
- Around line 189-193: Replace the custom regex-based strip_tags function with
Django's built-in implementation: remove the defined function strip_tags(html)
and add an import from django.utils.html import strip_tags (or import as
django_strip_tags and update call sites if you want to avoid shadowing) so all
places using strip_tags in this module use django.utils.html.strip_tags instead
of the brittle regex version.
- Around line 167-186: The post_save handler check_and_send_assignment_reminders
currently calls ReminderScheduler.send_early_reminder and
ReminderScheduler.send_final_reminder synchronously; change it to defer actual
email sending until after the DB transaction commits (or push to the task queue)
by wrapping the send calls in Django's transaction.on_commit (or instead enqueue
a Celery/Huey task that invokes
send_assignment_reminders/send_early_reminder/send_final_reminder); keep the
initial eligibility checks (ReminderScheduler.should_send_early_reminder and
should_send_final_reminder) in the signal but register a lambda or task that
calls the corresponding ReminderScheduler send methods after commit to avoid
blocking saves and transaction rollback issues.
- Around line 167-168: The post_save receiver
check_and_send_assignment_reminders is being registered via side-effect imports
and can be added multiple times; make its registration idempotent by adding a
unique dispatch_uid to the `@receiver` decorator for post_save
(sender=CourseMaterial) so duplicate handlers aren't registered across
imports/tests; use a clear unique string like
"check_and_send_assignment_reminders.CourseMaterial" as the dispatch_uid in the
decorator for the check_and_send_assignment_reminders function.
In `@web/templates/emails/assignment_reminder_24h.html`:
- Around line 2-3: The template's root <html> element is missing a lang
attribute and the <head> is missing a <title>, so update the template by adding
an appropriate lang="en" (or project locale) to the <html> tag and include a
descriptive <title> element inside the <head> (e.g., "Assignment Reminder — 24
hours") to satisfy accessibility and HTML metadata requirements; also apply the
same changes to the other occurrence referenced (the second <html>/<head>
block).
- Around line 4-55: The template uses a <style> block, inline CSS and hex colors
for selectors like .container, .header, .course-name, .due-date, .footer and
.button; replace the entire CSS with Tailwind utility classes on the HTML
elements (remove the <style> block), swap hex values for the project's color
token classes (e.g., bg-primary / text-accent / text-danger or whatever token
names your design system provides), and add dark: variants for each visual class
(e.g., dark:bg-... , dark:text-...) so the container, header, course-name,
due-date, footer and call-to-action button use Tailwind utilities and project
tokens instead of custom CSS.
In `@web/templates/emails/assignment_reminder_final.html`:
- Around line 2-3: Add required HTML metadata for accessibility by setting the
lang attribute on the root html tag and adding a meaningful title element inside
the head; update the template's <html> element to include lang="en" (or the
appropriate locale variable) and add a <title> (e.g., using the assignment or
site name template variable) within the <head> of assignment_reminder_final.html
so screen readers and browsers have proper language and document title
information.
- Around line 4-64: Remove the inline <style> block and replace the custom CSS
classes (body, .container, .header, .urgent-badge, .course-name, .due-date,
.footer, .button) with Tailwind utility classes and project color tokens; update
each element to use Tailwind utilities for spacing, typography, borders and
background instead of hex values, and add dark: variants for equivalent
dark-mode styles (e.g., dark:bg-..., dark:text-..., dark:border-...); ensure no
inline styles remain and that color/border choices map to the project's
prescribed tokens while preserving the same visual hierarchy and accessibility.
In `@web/tests/test_assignment_reminders.py`:
- Line 295: Rename the test class CourseMaterialRemindeFlagTests to
CourseMaterialReminderFlagTests across the file to fix the typo; update the
class definition name and any references (e.g., test discovery or imports) that
mention CourseMaterialRemindeFlagTests so tests continue to run correctly.
- Around line 225-227: Replace the weak assertion on mock_send_mail.call_count
with an exact equality check against the expected number of recipients: compute
the expected count from the test's approved enrollments collection (e.g.,
len(approved_enrollments) or ApprovedEnrollment queryset count used in this
test) and assert mock_send_mail.call_count == expected_count (use assertEqual).
Update the assertion that currently references mock_send_mail.call_count so it
compares to that expected_count to ensure every approved enrollment receives an
email.
- Around line 63-147: Tests in ReminderSchedulerTests are creating
CourseMaterial via CourseMaterial.objects.create(), which triggers post_save
signals and side effects; replace those DB saves with in-memory CourseMaterial
instances (instantiate CourseMaterial directly and set course, title,
material_type, due_date, reminder_sent, final_reminder_sent as needed) when
calling ReminderScheduler.should_send_early_reminder and
ReminderScheduler.should_send_final_reminder so tests don't invoke the signal
handlers; update all test methods that currently call
CourseMaterial.objects.create() in the block covering the early/final reminder
tests to construct plain instances instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: alphaonelabs/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fef062f8-b0d8-427c-9568-fbdb1c9efa82
📒 Files selected for processing (6)
web/asgi.pyweb/assignment_reminders.pyweb/signals.pyweb/templates/emails/assignment_reminder_24h.htmlweb/templates/emails/assignment_reminder_final.htmlweb/tests/test_assignment_reminders.py
| """Manages scheduling and sending of assignment deadline reminders.""" | ||
|
|
||
| @staticmethod | ||
| def should_send_early_reminder(course_material): |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add function/method type hints for this module.
Multiple methods and functions still miss parameter/return annotations; adding them will improve linting and maintainability.
As per coding guidelines "**/*.py: Use type hints in Python where appropriate".
Also applies to: 46-46, 65-65, 116-116, 168-168, 189-189
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 27-27: Missing return type annotation for staticmethod should_send_early_reminder
(ANN205)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/assignment_reminders.py` at line 27, Add explicit type hints for
should_send_early_reminder and every other function in this module that lacks
annotations (the functions referenced at the comment: lines 46, 65, 116, 168,
189). For should_send_early_reminder, annotate the parameter (e.g.,
course_material: dict | CourseMaterial | typing.Mapping[str, typing.Any]) and
the return type (-> bool) to reflect it returns a boolean. For the other
functions, add appropriate parameter and return annotations using concrete
types, typing primitives (str, int, bool), typing.Dict/Mapping, Optional[...] or
custom dataclasses/TypedDicts where the structure is known, and use -> None for
procedures; import typing names as needed. Ensure all function defs in this
module include these annotations so linters accept them and maintainers
understand expected types.
| try: | ||
| course = course_material.course | ||
| enrollments = Enrollment.objects.filter( | ||
| course=course, | ||
| status="approved" | ||
| ).select_related("student", "student__profile") | ||
|
|
||
| for enrollment in enrollments: | ||
| student = enrollment.student | ||
| context = { | ||
| "student_name": student.first_name or student.username, | ||
| "assignment_title": course_material.title, | ||
| "course_title": course.title, | ||
| "due_date": course_material.due_date, | ||
| "description": course_material.description, | ||
| "hours_remaining": 24, | ||
| } | ||
|
|
||
| subject = f"Reminder: '{course_material.title}' is due in 24 hours" | ||
| html_message = render_to_string( | ||
| "emails/assignment_reminder_24h.html", | ||
| context | ||
| ) | ||
|
|
||
| send_mail( | ||
| subject=subject, | ||
| message=strip_tags(html_message), | ||
| from_email=settings.DEFAULT_FROM_EMAIL, | ||
| recipient_list=[student.email], | ||
| html_message=html_message, | ||
| fail_silently=False, | ||
| ) | ||
|
|
||
| logger.info( | ||
| f"24-hour reminder sent to {student.username} for " | ||
| f"assignment '{course_material.title}'" | ||
| ) | ||
|
|
||
| # Mark reminder as sent | ||
| course_material.reminder_sent = True | ||
| course_material.save(update_fields=["reminder_sent"]) | ||
| logger.info(f"Early reminder sent for {course_material.title}") | ||
|
|
||
| except Exception as e: | ||
| logger.error( | ||
| f"Error sending early reminder for {course_material.title}: {str(e)}" | ||
| ) | ||
|
|
There was a problem hiding this comment.
A single email failure currently aborts the entire recipient batch.
The try wraps the whole send flow, so one bad recipient/send error stops remaining students and can cause repeated duplicates later on retry.
🔧 Suggested structure
- try:
- ...
- for enrollment in enrollments:
- ...
- send_mail(...)
- course_material.reminder_sent = True
- course_material.save(update_fields=["reminder_sent"])
- except Exception as e:
- logger.error(...)
+ sent_any = False
+ for enrollment in enrollments:
+ try:
+ ...
+ send_mail(...)
+ sent_any = True
+ except Exception:
+ logger.exception(
+ "Failed sending reminder for material_id=%s to user_id=%s",
+ course_material.id,
+ enrollment.student_id,
+ )
+ continue
+
+ if sent_any:
+ course_material.reminder_sent = True
+ course_material.save(update_fields=["reminder_sent"])Also applies to: 118-164
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 101-102: Logging statement uses f-string
(G004)
[warning] 108-108: Logging statement uses f-string
(G004)
[warning] 110-110: Do not catch blind exception: Exception
(BLE001)
[warning] 111-113: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
[warning] 112-112: Logging statement uses f-string
(G004)
[warning] 112-112: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/assignment_reminders.py` around lines 67 - 114, The current try/except
around the whole send flow causes one failed send to abort the loop; instead
wrap the per-recipient send and its success log in a per-enrollment try/except
(around send_mail and the subsequent logger.info) inside the for enrollment in
enrollments loop so a single failure is caught, logged, and the loop continues;
track failures (e.g., a failed_sends list or counter) while leaving Enrollment,
course_material, send_mail, and logger references intact, and only set
course_material.reminder_sent = True and call
course_material.save(update_fields=["reminder_sent"]) if failed_sends is empty
(otherwise log the failed recipients and do not mark the reminder as sent).
| @receiver(post_save, sender=CourseMaterial) | ||
| def check_and_send_assignment_reminders(sender, instance, created, **kwargs): | ||
| """ | ||
| Signal handler to check and send assignment reminders when CourseMaterial is saved. | ||
|
|
||
| This runs after every save of a CourseMaterial object and: | ||
| 1. Sends 24-hour early reminder if deadline is 24-48 hours away | ||
| 2. Sends final reminder if deadline is within 24 hours | ||
| """ | ||
| # Only process if material is an assignment with a due date | ||
| if instance.material_type != "assignment" or not instance.due_date: | ||
| return | ||
|
|
||
| # Check and send early reminder | ||
| if ReminderScheduler.should_send_early_reminder(instance): | ||
| ReminderScheduler.send_early_reminder(instance) | ||
|
|
||
| # Check and send final reminder | ||
| if ReminderScheduler.should_send_final_reminder(instance): | ||
| ReminderScheduler.send_final_reminder(instance) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ReminderScheduler class definition
rg "class ReminderScheduler" -A 10Repository: alphaonelabs/website
Length of output: 1493
🏁 Script executed:
# Search for send_early_reminder and send_final_reminder implementations
rg "def send_early_reminder|def send_final_reminder" -A 5Repository: alphaonelabs/website
Length of output: 930
🏁 Script executed:
# Check for async task definitions and queue setup
rg "celery|@task|@shared_task|background_task|defer" --type py | head -20Repository: alphaonelabs/website
Length of output: 46
🏁 Script executed:
# Find imports in assignment_reminders.py to understand dependencies
fd "assignment_reminders.py" | xargs head -30Repository: alphaonelabs/website
Length of output: 3758
🏁 Script executed:
# Get the complete implementation of send_early_reminder and send_final_reminder
fd "assignment_reminders.py" | xargs cat -n | sed -n '1,200p'Repository: alphaonelabs/website
Length of output: 8748
🏁 Script executed:
# Check the web/notifications.py file to see the deferred approach
fd "notifications.py" -path "*/web/*" | xargs cat -n | head -100Repository: alphaonelabs/website
Length of output: 232
🏁 Script executed:
# Search for notifications.py
find . -name "notifications.py" -path "*/web/*"Repository: alphaonelabs/website
Length of output: 86
🏁 Script executed:
# Get the send_assignment_reminders function from notifications.py
rg "def send_assignment_reminders" -A 30Repository: alphaonelabs/website
Length of output: 2280
🏁 Script executed:
# See full context of send_assignment_reminders in notifications.py
cat web/notifications.py | head -80Repository: alphaonelabs/website
Length of output: 2582
🏁 Script executed:
# Check if Celery or any task queue is configured
rg "celery|CELERY|@task|@shared_task" --type py | head -20Repository: alphaonelabs/website
Length of output: 46
🏁 Script executed:
# Check CourseMaterial model for the reminder_sent and final_reminder_sent fields
rg "class CourseMaterial" -A 30Repository: alphaonelabs/website
Length of output: 7303
Move email sending outside the post_save signal using transaction.on_commit() or a task queue to avoid blocking saves and transaction rollback issues.
Sending emails synchronously in a post_save signal handler has two problems: it blocks the save operation (users experience slower responses when multiple students need reminders), and it creates a consistency risk where emails are sent but the database transaction later rolls back, leaving the reminder state inconsistent.
For this pattern, either queue the reminder sending using Celery/Huey, or use Django's transaction.on_commit() callback to defer sending until after the transaction commits. Consider leveraging the deferred pattern already used in send_assignment_reminders() from the management command approach—it handles transaction safety and can batch-process multiple assignments efficiently.
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 168-168: Unused function argument: sender
(ARG001)
[warning] 168-168: Unused function argument: created
(ARG001)
[warning] 168-168: Missing type annotation for **kwargs
(ANN003)
[warning] 168-168: Unused function argument: kwargs
(ARG001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/assignment_reminders.py` around lines 167 - 186, The post_save handler
check_and_send_assignment_reminders currently calls
ReminderScheduler.send_early_reminder and ReminderScheduler.send_final_reminder
synchronously; change it to defer actual email sending until after the DB
transaction commits (or push to the task queue) by wrapping the send calls in
Django's transaction.on_commit (or instead enqueue a Celery/Huey task that
invokes send_assignment_reminders/send_early_reminder/send_final_reminder); keep
the initial eligibility checks (ReminderScheduler.should_send_early_reminder and
should_send_final_reminder) in the signal but register a lambda or task that
calls the corresponding ReminderScheduler send methods after commit to avoid
blocking saves and transaction rollback issues.
| @receiver(post_save, sender=CourseMaterial) | ||
| def check_and_send_assignment_reminders(sender, instance, created, **kwargs): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "assignment_reminders.py" | head -5Repository: alphaonelabs/website
Length of output: 93
🏁 Script executed:
git ls-files | grep -E "(assignment_reminder|signal)" | head -20Repository: alphaonelabs/website
Length of output: 390
🏁 Script executed:
fd "assignment_reminder" --type fRepository: alphaonelabs/website
Length of output: 375
🏁 Script executed:
wc -l ./web/assignment_reminders.pyRepository: alphaonelabs/website
Length of output: 97
🏁 Script executed:
cat -n ./web/assignment_reminders.py | sed -n '160,175p'Repository: alphaonelabs/website
Length of output: 768
🏁 Script executed:
cat -n ./web/assignment_reminders.py | head -50Repository: alphaonelabs/website
Length of output: 1893
🏁 Script executed:
cat -n ./web/assignment_reminders.py | sed -n '165,193p'Repository: alphaonelabs/website
Length of output: 1326
🏁 Script executed:
grep -r "assignment_reminders" --include="*.py" | grep -E "(import|from)" | head -20Repository: alphaonelabs/website
Length of output: 459
🏁 Script executed:
cat ./web/signals.py 2>/dev/null || echo "File not found"Repository: alphaonelabs/website
Length of output: 3000
🏁 Script executed:
cat ./web/apps.py 2>/dev/null | head -50Repository: alphaonelabs/website
Length of output: 309
Add dispatch_uid to make receiver registration idempotent.
This receiver is imported via side effects in web/signals.py and without dispatch_uid, repeated imports can register it multiple times. This causes duplicate reminders to be sent when the signal fires. Adding a dispatch_uid ensures the registration is idempotent and safe across multiple imports (common in tests, management commands, and other entry points).
Minimal fix
-@receiver(post_save, sender=CourseMaterial)
+@receiver(
+ post_save,
+ sender=CourseMaterial,
+ dispatch_uid="web.assignment_reminders.check_and_send_assignment_reminders",
+)
def check_and_send_assignment_reminders(sender, instance, created, **kwargs):🧰 Tools
🪛 Ruff (0.15.6)
[warning] 168-168: Unused function argument: sender
(ARG001)
[warning] 168-168: Unused function argument: created
(ARG001)
[warning] 168-168: Missing type annotation for **kwargs
(ANN003)
[warning] 168-168: Unused function argument: kwargs
(ARG001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/assignment_reminders.py` around lines 167 - 168, The post_save receiver
check_and_send_assignment_reminders is being registered via side-effect imports
and can be added multiple times; make its registration idempotent by adding a
unique dispatch_uid to the `@receiver` decorator for post_save
(sender=CourseMaterial) so duplicate handlers aren't registered across
imports/tests; use a clear unique string like
"check_and_send_assignment_reminders.CourseMaterial" as the dispatch_uid in the
decorator for the check_and_send_assignment_reminders function.
| if ReminderScheduler.should_send_early_reminder(instance): | ||
| ReminderScheduler.send_early_reminder(instance) | ||
|
|
||
| # Check and send final reminder | ||
| if ReminderScheduler.should_send_final_reminder(instance): | ||
| ReminderScheduler.send_final_reminder(instance) |
There was a problem hiding this comment.
Reminder deduplication is non-atomic under concurrency.
should_send_* checks and *_sent updates are separate operations; concurrent saves/workers can both pass checks and send duplicates. Use an atomic claim step (conditional DB update / lock) before dispatch.
| <html> | ||
| <head> |
There was a problem hiding this comment.
Add required document metadata for accessibility.
<html> is missing lang, and <head> is missing <title>.
✅ Minimal fix
-<html>
+<html lang="en">
<head>
+ <title>Final Assignment Deadline Reminder</title>As per coding guidelines "Ensure proper HTML structure and accessibility in templates".
Also applies to: 65-65
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 2-2: An lang attribute must be present on elements.
(html-lang-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/templates/emails/assignment_reminder_final.html` around lines 2 - 3, Add
required HTML metadata for accessibility by setting the lang attribute on the
root html tag and adding a meaningful title element inside the head; update the
template's <html> element to include lang="en" (or the appropriate locale
variable) and add a <title> (e.g., using the assignment or site name template
variable) within the <head> of assignment_reminder_final.html so screen readers
and browsers have proper language and document title information.
| <style> | ||
| body { | ||
| font-family: Arial, sans-serif; | ||
| line-height: 1.6; | ||
| color: #333; | ||
| max-width: 600px; | ||
| margin: 0 auto; | ||
| padding: 20px; | ||
| } | ||
| .container { | ||
| background-color: #f8f9fa; | ||
| border-radius: 8px; | ||
| padding: 30px; | ||
| border-left: 4px solid #d9534f; | ||
| } | ||
| .header { | ||
| color: #d9534f; | ||
| font-size: 24px; | ||
| font-weight: bold; | ||
| margin-bottom: 20px; | ||
| } | ||
| .urgent-badge { | ||
| display: inline-block; | ||
| background-color: #d9534f; | ||
| color: white; | ||
| padding: 8px 16px; | ||
| border-radius: 4px; | ||
| font-weight: bold; | ||
| margin-bottom: 20px; | ||
| } | ||
| .course-name { | ||
| background-color: #f2dede; | ||
| border-left: 4px solid #d9534f; | ||
| padding: 15px; | ||
| margin: 20px 0; | ||
| border-radius: 4px; | ||
| } | ||
| .due-date { | ||
| font-size: 18px; | ||
| color: #d9534f; | ||
| font-weight: bold; | ||
| margin: 10px 0; | ||
| } | ||
| .footer { | ||
| margin-top: 30px; | ||
| padding-top: 20px; | ||
| border-top: 1px solid #ddd; | ||
| font-size: 12px; | ||
| color: #666; | ||
| } | ||
| .button { | ||
| display: inline-block; | ||
| background-color: #d9534f; | ||
| color: white; | ||
| padding: 12px 24px; | ||
| text-decoration: none; | ||
| border-radius: 4px; | ||
| margin: 20px 0; | ||
| font-weight: bold; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
Template styling does not follow project HTML/Tailwind standards.
This template uses inline <style>, custom classes, and hex colors instead of required Tailwind utility classes, prescribed color tokens, and dark-mode variants.
As per coding guidelines "Always use Tailwind CSS classes for styling HTML elements", "Never use inline styles", and "Always include dark mode alternatives using dark: prefix".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/templates/emails/assignment_reminder_final.html` around lines 4 - 64,
Remove the inline <style> block and replace the custom CSS classes (body,
.container, .header, .urgent-badge, .course-name, .due-date, .footer, .button)
with Tailwind utility classes and project color tokens; update each element to
use Tailwind utilities for spacing, typography, borders and background instead
of hex values, and add dark: variants for equivalent dark-mode styles (e.g.,
dark:bg-..., dark:text-..., dark:border-...); ensure no inline styles remain and
that color/border choices map to the project's prescribed tokens while
preserving the same visual hierarchy and accessibility.
| def test_should_send_early_reminder_when_due_in_24_hours(self): | ||
| """Test early reminder should be sent when deadline is 24-48 hours away.""" | ||
| due_date = timezone.now() + timedelta(hours=36) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date | ||
| ) | ||
|
|
||
| self.assertTrue(ReminderScheduler.should_send_early_reminder(material)) | ||
|
|
||
| def test_should_not_send_early_reminder_when_already_sent(self): | ||
| """Test early reminder not sent if already marked as sent.""" | ||
| due_date = timezone.now() + timedelta(hours=36) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date, | ||
| reminder_sent=True # Already sent | ||
| ) | ||
|
|
||
| self.assertFalse(ReminderScheduler.should_send_early_reminder(material)) | ||
|
|
||
| def test_should_not_send_early_reminder_without_due_date(self): | ||
| """Test early reminder not sent if no due date set.""" | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment" | ||
| # No due_date | ||
| ) | ||
|
|
||
| self.assertFalse(ReminderScheduler.should_send_early_reminder(material)) | ||
|
|
||
| def test_should_not_send_early_reminder_when_too_far_away(self): | ||
| """Test early reminder not sent when deadline is more than 48 hours away.""" | ||
| due_date = timezone.now() + timedelta(hours=60) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date | ||
| ) | ||
|
|
||
| self.assertFalse(ReminderScheduler.should_send_early_reminder(material)) | ||
|
|
||
| def test_should_send_final_reminder_when_due_today(self): | ||
| """Test final reminder should be sent when deadline is within 24 hours.""" | ||
| due_date = timezone.now() + timedelta(hours=12) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date | ||
| ) | ||
|
|
||
| self.assertTrue(ReminderScheduler.should_send_final_reminder(material)) | ||
|
|
||
| def test_should_not_send_final_reminder_when_already_sent(self): | ||
| """Test final reminder not sent if already marked as sent.""" | ||
| due_date = timezone.now() + timedelta(hours=12) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date, | ||
| final_reminder_sent=True # Already sent | ||
| ) | ||
|
|
||
| self.assertFalse(ReminderScheduler.should_send_final_reminder(material)) | ||
|
|
||
| def test_should_not_send_final_reminder_when_far_away(self): | ||
| """Test final reminder not sent when deadline is more than 24 hours away.""" | ||
| due_date = timezone.now() + timedelta(hours=48) | ||
| material = CourseMaterial.objects.create( | ||
| course=self.course, | ||
| title="Assignment 1", | ||
| material_type="assignment", | ||
| due_date=due_date | ||
| ) | ||
|
|
||
| self.assertFalse(ReminderScheduler.should_send_final_reminder(material)) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "*.py" | grep -E "(assignment_reminder|reminder)" | head -20Repository: alphaonelabs/website
Length of output: 344
🏁 Script executed:
find . -type f -name "*.py" | grep -E "models\.py" | head -10Repository: alphaonelabs/website
Length of output: 181
🏁 Script executed:
rg "post_save.*CourseMaterial|signal.*reminder" --type py -A 5 -B 5Repository: alphaonelabs/website
Length of output: 2096
🏁 Script executed:
cat web/tests/test_assignment_reminders.py | head -200Repository: alphaonelabs/website
Length of output: 6996
🏁 Script executed:
cat web/assignment_reminders.py | head -100Repository: alphaonelabs/website
Length of output: 3497
🏁 Script executed:
cat web/assignment_reminders.py | tail -100Repository: alphaonelabs/website
Length of output: 3968
🏁 Script executed:
rg "check_and_send_assignment_reminders" -A 30 --type pyRepository: alphaonelabs/website
Length of output: 1901
🏁 Script executed:
rg "disable_signals|disconnect|mock|patch" web/tests/test_assignment_reminders.py -B 3 -A 3Repository: alphaonelabs/website
Length of output: 2983
🏁 Script executed:
rg "setUp|tearDown" web/tests/test_assignment_reminders.py -A 10 | head -60Repository: alphaonelabs/website
Length of output: 1076
🏁 Script executed:
rg "ReminderSchedulerTests" web/tests/test_assignment_reminders.py -A 3Repository: alphaonelabs/website
Length of output: 175
🏁 Script executed:
rg "class ReminderSchedulerTests" web/tests/test_assignment_reminders.py -B 5 -A 80 | grep -E "def test_|@patch|@|objects.create"Repository: alphaonelabs/website
Length of output: 862
🏁 Script executed:
cat web/tests/test_assignment_reminders.py | sed -n '20,80p'Repository: alphaonelabs/website
Length of output: 2058
🏁 Script executed:
grep -r "EMAIL_BACKEND\|EMAIL_HOST\|CELERY_TASK_ALWAYS_EAGER" --include="*.py" web/tests/ tests/Repository: alphaonelabs/website
Length of output: 46
🏁 Script executed:
rg "class.*TestCase|django.test" web/tests/test_assignment_reminders.py | head -5Repository: alphaonelabs/website
Length of output: 231
🏁 Script executed:
cat web/tests/test_assignment_reminders.py | sed -n '1,100p' | tail -50Repository: alphaonelabs/website
Length of output: 1958
Remove ORM database saves from ReminderScheduler unit tests to ensure test isolation.
ReminderSchedulerTests uses CourseMaterial.objects.create() which triggers the post_save signal registered in web/assignment_reminders.py. This signal attempts to send reminder emails (unmocked) and modify the reminder_sent and final_reminder_sent flags during test setup, causing test pollution and coupling test logic to side effects.
Since ReminderScheduler.should_send_early_reminder() and should_send_final_reminder() are static methods that only read instance attributes (due date, flag values, and time deltas), construct instances in memory instead:
🔧 Refactored pattern for isolated scheduler tests
- material = CourseMaterial.objects.create(
+ material = CourseMaterial(
course=self.course,
title="Assignment 1",
material_type="assignment",
- due_date=due_date
+ )Update all test methods in ReminderSchedulerTests (lines 63–147) to use this pattern. This preserves test intent while isolating the scheduler logic from signal side effects, improving reliability and test clarity.
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 73-73: Use a regular assert instead of unittest-style assertTrue
Replace assertTrue(...) with assert ...
(PT009)
[warning] 86-86: Use a regular assert instead of unittest-style assertFalse
Replace assertFalse(...) with assert ...
(PT009)
[warning] 97-97: Use a regular assert instead of unittest-style assertFalse
Replace assertFalse(...) with assert ...
(PT009)
[warning] 109-109: Use a regular assert instead of unittest-style assertFalse
Replace assertFalse(...) with assert ...
(PT009)
[warning] 121-121: Use a regular assert instead of unittest-style assertTrue
Replace assertTrue(...) with assert ...
(PT009)
[warning] 134-134: Use a regular assert instead of unittest-style assertFalse
Replace assertFalse(...) with assert ...
(PT009)
[warning] 146-146: Use a regular assert instead of unittest-style assertFalse
Replace assertFalse(...) with assert ...
(PT009)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/test_assignment_reminders.py` around lines 63 - 147, Tests in
ReminderSchedulerTests are creating CourseMaterial via
CourseMaterial.objects.create(), which triggers post_save signals and side
effects; replace those DB saves with in-memory CourseMaterial instances
(instantiate CourseMaterial directly and set course, title, material_type,
due_date, reminder_sent, final_reminder_sent as needed) when calling
ReminderScheduler.should_send_early_reminder and
ReminderScheduler.should_send_final_reminder so tests don't invoke the signal
handlers; update all test methods that currently call
CourseMaterial.objects.create() in the block covering the early/final reminder
tests to construct plain instances instead.
| # Check that emails were sent | ||
| self.assertGreater(mock_send_mail.call_count, 0) | ||
|
|
There was a problem hiding this comment.
Use an exact recipient-count assertion for stronger coverage.
assertGreater(call_count, 0) can pass even if only one student receives email; assert the exact approved enrollment count instead.
As per coding guidelines "**/tests/**: Ensure tests are descriptive ... and meaningful assertions."
🧰 Tools
🪛 Ruff (0.15.6)
[warning] 226-226: Use a regular assert instead of unittest-style assertGreater
Replace assertGreater(...) with assert ...
(PT009)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/test_assignment_reminders.py` around lines 225 - 227, Replace the
weak assertion on mock_send_mail.call_count with an exact equality check against
the expected number of recipients: compute the expected count from the test's
approved enrollments collection (e.g., len(approved_enrollments) or
ApprovedEnrollment queryset count used in this test) and assert
mock_send_mail.call_count == expected_count (use assertEqual). Update the
assertion that currently references mock_send_mail.call_count so it compares to
that expected_count to ensure every approved enrollment receives an email.
| self.assertIn('Test Assignment', email_body) | ||
|
|
||
|
|
||
| class CourseMaterialRemindeFlagTests(TestCase): |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor typo in test class name.
Consider renaming CourseMaterialRemindeFlagTests to CourseMaterialReminderFlagTests for clarity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/tests/test_assignment_reminders.py` at line 295, Rename the test class
CourseMaterialRemindeFlagTests to CourseMaterialReminderFlagTests across the
file to fix the typo; update the class definition name and any references (e.g.,
test discovery or imports) that mention CourseMaterialRemindeFlagTests so tests
continue to run correctly.
Add Automatic Assignment Deadline Reminder System
Problem
Students often miss assignment deadlines, leading to incomplete coursework and poor engagement. There's no automated system to remind them when assignments are due soon.
Solution
Implemented an automated reminder system that:
Implementation Details
New Module:
web/assignment_reminders.pyReminderSchedulerclass with two configurable time windowsshould_send_early_reminder()- Checks if 24-hour early warning should sendshould_send_final_reminder()- Checks if deadline day final reminder should sendsend_early_reminder()- Sends to all approved enrolled studentssend_final_reminder()- Sends final urgent reminderEmail Templates
Signal Integration
web/signals.pyto import assignment reminderspost_savesignal for CourseMaterialFeatures
reminder_sentandfinal_reminder_sentflagsTesting
13 comprehensive test cases covering:
Benefits
Database & Migrations
CourseMaterialfields:due_date(existing)reminder_sent(existing)final_reminder_sent(existing)Configuration
Breaking Changes
Type
New feature
Improves user experience
Bug fix
Summary
This pull request implements an automated assignment deadline reminder system that proactively notifies students about upcoming and same-day assignment deadlines.
Purpose
To reduce missed assignment submissions by sending two-tier reminder notifications: an early warning 24–48 hours before the deadline and a final urgent reminder on the due date itself.
Key Changes
Core Reminder System (
web/assignment_reminders.py):ReminderSchedulerclass with decision logic for two reminder windows:reminder_sentandfinal_reminder_sentflags onCourseMaterialapprovedenrollment statusSignal Integration (
web/signals.py):CourseMaterialcreation/updateEmail Templates:
assignment_reminder_24h.html: Orange-accented warning with 24-hour countdownassignment_reminder_final.html: Red-accented urgent notice emphasizing same-day deadline and potential late-work restrictionsTest Coverage (
web/tests/test_assignment_reminders.py):Impact