-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: internal routing ignores public path #312
Conversation
Add a new util function for constructing the correct internal route URL.
Thanks for the pull request, @dyudyunov! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #312 +/- ##
==========================================
+ Coverage 70.22% 70.73% +0.50%
==========================================
Files 27 28 +1
Lines 403 410 +7
Branches 85 87 +2
==========================================
+ Hits 283 290 +7
Misses 119 119
Partials 1 1 ☔ View full report in Codecov by Sentry. |
@dyudyunov -- thanks for calling out that this can't be reproduced in devstack 😅. I was sitting wracking my brain trying to figure out how we could have missed this, but we have missed all of Tutor thus far 😬. |
@justinhynes I would also mention that a similar fix is present in the Course Authoring MFE (both master and quince.master branches) It could be helpful to move the @ihor-romaniuk FYI |
@justinhynes @mphilbrick211 It would be really great to have these changes in open-release/quince.3 🙏 I've created a backport PR: #313 |
Description
Internal routes don't respect the PUBLIC_PATH which leads to redirecting to incorrect URLs.
Add a new util function for constructing the correct internal route URL.
Steps to reproduce
You'll need an instance deployed with the common domain for MFEs (each MFE should have the PUBLIC_PATH set).
You can reproduce it using Tutor.
Devstack uses separate domains for each MFE so you won't reproduce the issue there!
Actual result
PUBLIC_PATH for Learner Record MFE is ignored, and the user is redirected to the wrong URL
The same result on the Program Records page for the "Back To My Records" link
Expected result
Correct internal routing is performed
Notes
I'll create a backport PR for Quince soon