feat: preserve the activation next URL for all learners - #456
Merged
Conversation
pwnage101
force-pushed
the
pwnage101/ENT-11816-account-activation
branch
from
September 3, 2026 22:07
0a42578 to
47e0e43
Compare
Rework activate_account() so the post-activation destination is no longer sensitive to enterprise membership, and consistently redirects unauthenticated learners to the login page in a way that preserves the `next` URL. Furthermore, this commit expands the cases where the activation CTA cookie is cleared, so that it's *always* cleared after successful activation. Prior to this commit, there were three user stories which seemed wrong and had possibly misleading behavior: 1. Already-logged-in, non-enterprise learners trying to click an "activate account" link containing a `next` URL would get dropped into the learner dashboard despite a `next` URL being specified. 2. Logged-out learners on a platform with the AuthN MFE disabled (still a supported configuration) trying to click an "activate account" link containing a `next` URL would get dropped into the learner dashboard despite a `next` URL being specified. 3. Logged-out learners on a platform with the AuthN MFE enabled may see an activation CTA (reminder message to activate) even after successful activation and login. In user stories 1-2, the `next` URL has been carefully supplied by the client, but subsequently ignored. If the intended behavior is to terminate on the dashboard, clients should NOT specify a `next` query parameter on the activate_account link. In user story 3, the stray activation CTA may seem nagging and annoying since activation was already attempted and succeeded. ENT-11816
pwnage101
force-pushed
the
pwnage101/ENT-11816-account-activation
branch
from
September 8, 2026 15:46
47e0e43 to
24c7351
Compare
pwnage101
marked this pull request as ready for review
September 8, 2026 22:04
There was a problem hiding this comment.
🟡 Changes recommended
Explicit /dashboard destinations remain lost when a custom post-login default is configured.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Preserves activation destinations across authentication flows and clears stale activation CTA cookies.
Changes:
- Routes unauthenticated learners through login while retaining safe
nextURLs. - Removes enterprise-only redirect behavior.
- Adds redirect and cookie-clearing tests.
File summaries
| File | Description |
|---|---|
common/djangoapps/student/views/management.py |
Updates post-activation redirects and cookie cleanup. |
common/djangoapps/student/tests/test_activate_account.py |
Expands coverage across authentication configurations. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
subhashree-sahu31
approved these changes
Sep 9, 2026
subhashree-sahu31
left a comment
subhashree-sahu31
left a comment
There was a problem hiding this comment.
Code changes LGTM 🚀
please resolve the open Copilot comment (unauthenticated next=/dashboard being dropped) with either a fix or a reply before merging.
brobro10000
approved these changes
Sep 9, 2026
Member
Author
|
Since I neither altered nor introduced the /dashboard dropping behavior, I'm going to opt to leave its "fix" for a separate PR and consider it out-of-scope for my current task. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rework activate_account() so the post-activation destination is no longer sensitive to enterprise membership, and consistently redirects unauthenticated learners to the login page in a way that preserves the
nextURL. Furthermore, this commit expands the cases where the activation CTA cookie is cleared, so that it's always cleared after successful activation.Prior to this commit, there were three user stories which seemed wrong and had possibly misleading behavior:
nextURL would get dropped into the learner dashboard despite anextURL being specified.nextURL would get dropped into the learner dashboard despite anextURL being specified.In user stories 1-2, the
nextURL has been carefully supplied by the client, but subsequently ignored. If the intended behavior is to terminate on the dashboard, clients should NOT specify anextquery parameter on the activate_account link.In user story 3, the stray activation CTA may seem nagging and annoying since activation was already attempted and succeeded.
ENT-11816