feat: waffle flag for redirecting to courseware MFE #23016
Conversation
davidjoy
force-pushed
the
djoy/learning_mfe_waffle_flag
branch
3 times, most recently
from
3590d4b to
87500e6
Compare
|
jenkins run python |
|
jenkins run quality |
|
jenkins run bokchoy |
davidjoy
force-pushed
the
djoy/learning_mfe_waffle_flag
branch
2 times, most recently
from
4d5f557 to
b548d92
Compare
|
jenkins run quality |
1. This is a partial check-in. It causes jump_to links in the header user menu to work, but doesn’t address any other dashboard links. 2. I also need to figure out the best way to test this, having not tested a toggle like this before.
davidjoy
force-pushed
the
djoy/learning_mfe_waffle_flag
branch
from
b548d92 to
fe19425
Compare
We decided this approach - of redirecting to the new courseware MFE when users hit the page - won’t be feasible because there are still circumstances when we want that page to load. For instance, exams, or if an instructor wants to go back to the old experience. Instead of doing this, we need to track down all the links to the courseware page and put the waffle flag check on those instead.
davidjoy
force-pushed
the
djoy/learning_mfe_waffle_flag
branch
from
2a14a36 to
18e0020
Compare
|
jenkins run bokchoy |
This will ultmately be put back in as part of TNL-6982, but for now we don’t want this code to be associated with the waffle flag.
davidjoy
force-pushed
the
djoy/learning_mfe_waffle_flag
branch
from
18e0020 to
e427fad
Compare
|
Your PR has finished running tests. There were no failures. |
ormsbee
left a comment
ormsbee
left a comment
There was a problem hiding this comment.
Quick paranoia question.
| return redirect_url | ||
|
|
||
|
|
||
| def get_microfrontend_redirect_url(course_key, path=None): |
There was a problem hiding this comment.
Who is the caller of this? Is path always derived from sanitized information that we have (i.e. real blocks), or does it sometimes get provided by the browser calling?
There was a problem hiding this comment.
It may be that I should just remove this method from the PR. It was being called by redirects which I removed from this PR, and will be again when we put those back in. It's not currently in use, which now that I say it out loud, probably means I should just pull it for now. 😄
The path is derived from path_to_location when it's used, so yes, I believe it's always sanitized.
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
|
EdX Release Notice: This PR has been deployed to the production environment. |
|
EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production. |
JIRA: https://openedx.atlassian.net/browse/TNL-6982
Please consider the following when opening a pull request:
In each commit, include description that could help a developer
several months from now.
make upgrade, run as close to the time of merging as possibleto avoid accidentally downgrading someone else's package.
Put the output of
make upgradein its own separate commit,decoupled from other code changes.
automated testing isn't a substitute for manual verification.
Code that is amenable to refactoring and improvement benefits all platform developers,
especially given the size and scope of edx-platform.
Consult existing Architectural Decision Records (ADRs),
including those concerning the app(s) you are changing and
those concerning edx-platform as a whole.