Skip to content

Fix GradeChangeRequest sender address missing email - #6141

Merged
willgearty merged 2 commits into
learning-unlimited:mainfrom
Shresth-14:fix/grade-change-sender-address
Oct 20, 2026
Merged

willgearty merged 2 commits into
learning-unlimited:mainfrom
Shresth-14:fix/grade-change-sender-address

Conversation

@Shresth-14

Copy link
Copy Markdown
Contributor

Description

GradeChangeRequest.send_confirmation_email() constructed the from_email argument using an or expression followed by string concatenation:

Tag.getTag('full_group_name') or f'{settings.INSTITUTION_NAME} {settings.ORGANIZATION_SHORT_NAME}' + ' <info@' + settings.SITE_INFO[1] + '>'

In Python, the + operator has higher precedence than or. Consequently, when full_group_name is set, the expression evaluates to the group name alone, without the sender's email address.

The project's send_mail validator rejects this value with Invalid 'From' email address, preventing the grade change confirmation email from being sent.

This PR fixes the issue by computing the display name and sender address separately, ensuring that the email address is always included:

from_name = Tag.getTag('full_group_name') or f'{settings.INSTITUTION_NAME} {settings.ORGANIZATION_SHORT_NAME}'
from_email = f'{from_name} <info@{settings.SITE_INFO[1]}>'

Changes Made

  • esp/esp/users/models/__init__.py: Separate the sender display name from the complete from_email address and pass the correctly formatted sender to send_mail. The subject, message, and recipient remain unchanged.
  • esp/esp/users/tests.py: Add test_confirmation_email_sender_with_full_group_name to verify that when full_group_name is configured, the sender address is correctly formatted as MIT Educational Studies Program <info@{SITE_INFO[1]}>.

Related Issue

Closes #6139

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactor / cleanup
  • CI/CD or build change

Testing

  • Existing tests pass
  • New regression test added
  • Sender-address logic manually verified

Test results:

  • esp/esp/users/tests.py: 117 passed, 0 failed, including the new regression test.
  • Regression verification: The new test passes with the fix applied and fails when the original implementation is restored, producing ESPError_Log: Invalid 'From' email address (MIT Educational Studies Program).
  • Linting: flake8 reports no warnings in either changed file.

Verification scope: The regression was verified locally using the failing and passing test cases. A live email was not sent from a deployed instance.

AI Disclosure

AI was used to review the codebase and test coverage. Implementation, testing, and debugging were done manually.

Checklist

  • Self-reviewed the code
  • [] Updated documentation, if needed (no documentation changes required)
  • No new warnings or errors introduced

…ed#6139)

Signed-off-by: Shresth Juyal <shresthajuyal96@gmail.com>
Copilot AI lite review requested due to automatic review settings October 19, 2026 05:07

Copilot AI left a comment

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.

🟢 Approval recommended

The sender-address fix is covered by a focused regression test, with no unresolved blocking issues.

No open issues 🎉

What changed in this PR

Fixes grade-change confirmation emails so the configured group name includes a valid sender address.

Changes:

  • Corrects from_email construction.
  • Adds regression coverage for configured full_group_name.
File Description
esp/​esp/​users/​tests.py Verifies the corrected sender format.
esp/​esp/​users/​models/​__init__.py Corrects sender address construction.

Review effort: Lite

@willgearty willgearty left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@willgearty
willgearty merged commit 4294860 into learning-unlimited:main Oct 20, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Grade change confirmation emails use a sender with no email address

3 participants