Skip to content

fix: address release-retired-learner-email PR review feedback - #463

Merged
rahulkanneri-2u merged 2 commits into
release-ulmofrom
lp-1177-address-release-retired-learner-email-review-feedback
Sep 11, 2026
Merged

fix: address release-retired-learner-email PR review feedback#463
rahulkanneri-2u merged 2 commits into
release-ulmofrom
lp-1177-address-release-retired-learner-email-review-feedback

Conversation

@rahulkanneri-2u

Copy link
Copy Markdown

Description

Follow-up to #460 addressing the review comments that were
still open when that PR merged into release-ulmo. No new behavior is introduced release_retired_learner_email()
itself is unchanged. This is cleanup of the surrounding code and tests, plus one CLI interface change:

  • Removed --username from the release_retired_user_email management command. Per review feedback, a
    tool built for retired learners shouldn't accept a username (PII-shaped input) at all --user_id is now the
    only supported identifier. This is a breaking change for anyone already scripting the old --username flag.
  • Dropped the lint-amnesty tag on the User import in accounts/utils.py and in the management command —
    it's slated for removal from openedx altogether, so it shouldn't be copied into new files.
  • Test setup emails now use settings.RETIRED_EMAIL_DOMAIN instead of a hardcoded retired.invalid, matching
    the assertions in the same tests and removing the inconsistency flagged in review.
  • Switched the command's two "success message" tests from asserting against captured stdout (capsys) to
    mocking the logger (mock_logger.info.assert_called_with(...)), matching the existing pattern in
    common/djangoapps/student/management/tests/test_change_enrollment.py.

Supporting information

Testing instructions

  1. Retire a test user to the COMPLETE state.
  2. is_email_retired("learner@example.com") => True.
  3. ./manage.py lms release_retired_user_email --user_id <retired_user_id>
  4. is_email_retired("learner@example.com") => False; confirm re-registration succeeds.
  5. Confirm the old --username <retired_username> form now fails with an "unrecognized arguments" error.
  6. pytest openedx/core/djangoapps/user_api/accounts/tests/test_utils.py openedx/core/djangoapps/user_api/management/tests/test_release_retired_user_email.py

Deadline

None.

Other information

@rahulkanneri-2u
rahulkanneri-2u merged commit 9427ef6 into release-ulmo Sep 11, 2026
64 checks passed
@rahulkanneri-2u
rahulkanneri-2u deleted the lp-1177-address-release-retired-learner-email-review-feedback branch September 11, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants