Skip to content
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

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

dyudyunov
Copy link
Contributor

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!

  1. Set up a program
  2. Go to Profile and click on "View My Records" button
    Screenshot 2024-03-28 at 17 48 13
  3. Click on "View Program Record" button
    Screenshot 2024-03-28 at 17 49 02

Actual result

PUBLIC_PATH for Learner Record MFE is ignored, and the user is redirected to the wrong URL
image

The same result on the Program Records page for the "Back To My Records" link
Screenshot 2024-03-28 at 17 55 21

Expected result

Correct internal routing is performed

Notes

I'll create a backport PR for Quince soon

Add a new util function for constructing the correct
internal route URL.
@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 28, 2024
@openedx-webhooks
Copy link

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:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

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.

Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.73%. Comparing base (9efeceb) to head (10c950f).

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.
📢 Have feedback on the report? Share it here.

@justinhynes
Copy link
Contributor

@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 😬.

@mphilbrick211 mphilbrick211 requested a review from a team March 28, 2024 21:27
@dyudyunov
Copy link
Contributor Author

dyudyunov commented Mar 29, 2024

@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 createCorrectInternalRoute helper function to the common dependency such as frontend-platform in the future to get rid of duplicated functionality.

@ihor-romaniuk FYI

@dyudyunov
Copy link
Contributor Author

@justinhynes @mphilbrick211 It would be really great to have these changes in open-release/quince.3 🙏

I've created a backport PR: #313

@justinhynes justinhynes merged commit 6f70f40 into openedx:master Mar 29, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants