Skip to content

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

Open
rahulkanneri-2u wants to merge 2 commits into
release-ulmofrom
lp-1177-address-release-retired-learner-email-review-feedback
Open

fix: address release-retired-learner-email PR review feedback#463
rahulkanneri-2u wants to merge 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

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.

2 participants