Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from django.core.asgi import get_asgi_application
from sentry_sdk.integrations.asgi import SentryAsgiMiddleware

# Initialize Django settings
# Initialize Django settings to ensure models and routing can be imported without issues
os.environ.setdefault("DJANGO_SETTINGS_MODULE", "web.settings")
django.setup()

Expand Down
193 changes: 193 additions & 0 deletions web/assignment_reminders.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
"""
Assignment deadline reminder system for Alpha One Labs.

Handles automatic sending of reminder emails to students when assignments
are due soon. Uses Django signals to automatically create and manage reminders.
"""

import logging
from datetime import timedelta

from django.conf import settings
from django.core.mail import send_mail
from django.db.models.signals import post_save
from django.dispatch import receiver
from django.template.loader import render_to_string
from django.utils import timezone

from .models import CourseMaterial, Enrollment

logger = logging.getLogger(__name__)


class ReminderScheduler:
"""Manages scheduling and sending of assignment deadline reminders."""

@staticmethod
def should_send_early_reminder(course_material):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

πŸ› οΈ 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.

"""
Check if early reminder (24 hours before) should be sent.

Returns True if:
- Material has a due_date
- Reminder hasn't been sent yet
- Deadline is within 24-48 hours from now
"""
if not course_material.due_date or course_material.reminder_sent:
return False

now = timezone.now()
time_until_due = course_material.due_date - now


return timedelta(hours=24) <= time_until_due <= timedelta(hours=48)

@staticmethod
def should_send_final_reminder(course_material):
"""
Check if final reminder (on deadline day) should be sent.

Returns True if:
- Material has a due_date
- Final reminder hasn't been sent yet
- Deadline is within next 24 hours
"""
if not course_material.due_date or course_material.final_reminder_sent:
return False

now = timezone.now()
time_until_due = course_material.due_date - now

# Send final reminder if deadline is within 24 hours
return timedelta(0) <= time_until_due <= timedelta(hours=24)

@staticmethod
def send_early_reminder(course_material):
"""Send 24-hour before deadline reminder to all enrolled students."""
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)}"
)

Comment on lines +67 to +114
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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).

@staticmethod
def send_final_reminder(course_material):
"""Send final reminder on deadline day to all enrolled students."""
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,
"is_final": True,
}

subject = f"FINAL REMINDER: '{course_material.title}' is due today!"
html_message = render_to_string(
"emails/assignment_reminder_final.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"Final reminder sent to {student.username} for "
f"assignment '{course_material.title}'"
)

# Mark final reminder as sent
course_material.final_reminder_sent = True
course_material.save(update_fields=["final_reminder_sent"])
logger.info(f"Final reminder sent for {course_material.title}")

except Exception as e:
logger.error(
f"Error sending final reminder for {course_material.title}: {str(e)}"
)


@receiver(post_save, sender=CourseMaterial)
def check_and_send_assignment_reminders(sender, instance, created, **kwargs):
Comment on lines +167 to +168
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "assignment_reminders.py" | head -5

Repository: alphaonelabs/website

Length of output: 93


🏁 Script executed:

git ls-files | grep -E "(assignment_reminder|signal)" | head -20

Repository: alphaonelabs/website

Length of output: 390


🏁 Script executed:

fd "assignment_reminder" --type f

Repository: alphaonelabs/website

Length of output: 375


🏁 Script executed:

wc -l ./web/assignment_reminders.py

Repository: 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 -50

Repository: 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 -20

Repository: 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 -50

Repository: 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.

"""
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)
Comment on lines +167 to +186
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, find the ReminderScheduler class definition
rg "class ReminderScheduler" -A 10

Repository: 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 5

Repository: 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 -20

Repository: alphaonelabs/website

Length of output: 46


🏁 Script executed:

# Find imports in assignment_reminders.py to understand dependencies
fd "assignment_reminders.py" | xargs head -30

Repository: 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 -100

Repository: 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 30

Repository: alphaonelabs/website

Length of output: 2280


🏁 Script executed:

# See full context of send_assignment_reminders in notifications.py
cat web/notifications.py | head -80

Repository: 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 -20

Repository: alphaonelabs/website

Length of output: 46


🏁 Script executed:

# Check CourseMaterial model for the reminder_sent and final_reminder_sent fields
rg "class CourseMaterial" -A 30

Repository: 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.

Comment on lines +181 to +186
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.



def strip_tags(html):
"""Utility function to strip HTML tags for plain text emails."""
import re
clean = re.compile('<.*?>')
return re.sub(clean, '', html)
Comment on lines +189 to +193
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | πŸ”΅ Trivial

Use Django’s built-in strip_tags instead of regex HTML stripping.

The regex approach is brittle for real HTML and is less maintainable than django.utils.html.strip_tags.

βœ… Minimal fix
+from django.utils.html import strip_tags
...
-def strip_tags(html):
-    """Utility function to strip HTML tags for plain text emails."""
-    import re
-    clean = re.compile('<.*?>')
-    return re.sub(clean, '', html)
πŸ“ Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def strip_tags(html):
"""Utility function to strip HTML tags for plain text emails."""
import re
clean = re.compile('<.*?>')
return re.sub(clean, '', html)
πŸ€– Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/assignment_reminders.py` around lines 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.

4 changes: 4 additions & 0 deletions web/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,7 @@ def invalidate_session_cache(sender, instance, **kwargs):
enrollments = Enrollment.objects.filter(course=instance.course)
for enrollment in enrollments:
invalidate_progress_cache(enrollment.student)


# Import assignment reminder signal handlers
from .assignment_reminders import check_and_send_assignment_reminders # noqa
89 changes: 89 additions & 0 deletions web/templates/emails/assignment_reminder_24h.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<!DOCTYPE html>
<html>
<head>
Comment on lines +2 to +3
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟑 Minor

Add required document metadata for accessibility.

<html> is missing lang, and <head> is missing <title>.

βœ… Minimal fix
-<html>
+<html lang="en">
 <head>
+    <title>24-Hour Assignment Reminder</title>

As per coding guidelines "Ensure proper HTML structure and accessibility in templates".

Also applies to: 56-56

🧰 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_24h.html` around lines 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).

<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 #FFA500;
}
.header {
color: #FFA500;
font-size: 24px;
font-weight: bold;
margin-bottom: 20px;
}
.course-name {
background-color: #fff3cd;
border-left: 4px solid #FFA500;
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: #FFA500;
color: white;
padding: 12px 24px;
text-decoration: none;
border-radius: 4px;
margin: 20px 0;
font-weight: bold;
}
</style>
Comment on lines +4 to +55
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Template styling does not follow project HTML/Tailwind standards.

The template relies on inline CSS, custom classes, and hex color values rather than required Tailwind utilities, project 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_24h.html` around lines 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.

</head>
<body>
<div class="container">
<div class="header">⏰ Assignment Due in 24 Hours</div>

<p>Hi {{ student_name }},</p>

<p>This is a reminder that you have an assignment due soon in <strong>{{ course_title }}</strong>.</p>

<div class="course-name">
<strong>Assignment:</strong> {{ assignment_title }}<br>
<div class="due-date">Due: {{ due_date|date:"F j, Y \a\t g:i A" }}</div>
</div>

{% if description %}
<p><strong>Details:</strong></p>
<p>{{ description }}</p>
{% endif %}

<p>Make sure to submit your work before the deadline to avoid any late submissions.</p>

<p>Need help? Visit the course materials or reach out to your instructor.</p>

<p>
<a href="{{ site_url }}" class="button">View Assignment</a>
</p>

<div class="footer">
<p>Alpha One Labs Education Platform<br>
This is an automated reminder. Please do not reply to this email.</p>
</div>
</div>
</body>
</html>
Loading
Loading