Skip to content

[BTR-53] [BB-3502] Misuse of urljoin in Gradebook URL from Instructor Panel - #312

Closed
spokerman12 wants to merge 2 commits into
opencraft-release/koa.1from
danielf/BTR-53-BB-3502
Closed

[BTR-53] [BB-3502] Misuse of urljoin in Gradebook URL from Instructor Panel#312
spokerman12 wants to merge 2 commits into
opencraft-release/koa.1from
danielf/BTR-53-BB-3502

Conversation

@spokerman12

Copy link
Copy Markdown

When the Gradebook MFE is deployed on a subdirectory, urljoin returns the wrong link on Instructor Dashboard.

JIRA tickets: OSPR-5358 BTR-53

Discussions: Background: https://github.com/edx/configuration/pull/6171#discussion_r549190558

The frontend-app-gradebook doesn't use the /gradebook/ path out of the box (or any other). The present issue is specified as "the Gradebook link [stops] working if Gradebooks are deployed using the subdirectory mechanism," based on the linked discussion. From this method's code and its unit test, the frontend-app-gradebook should be hosted at WRITABLE_GRADEBOOK_URL. I believe these test cases (based on the mfe_deployer role in configuration/master) should pass


    WRITABLE_GRADEBOOK_URL = https://MFE_BASE/gradebook
    WRITABLE_GRADEBOOK_URL = https://MFE_BASE
    WRITABLE_GRADEBOOK_URL = https://any.domain.really

Using .format instead of urljoin works nicely, and it's also applied on some important edx-platform code with similar behavior.

Dependencies: None

Screenshots: See "Testing instructions"

Sandbox URL: Sandbox. You can use staff@example.com to access the Demo Course's Instructor panel. This instance was deployed with configuration's master branch

Merge deadline: None

Testing instructions:

  • Deploy a devstack using master
  • Set up a course and an admin account if there isn't one.
  • Enable Gradebook MFE using the instructions here.
  • This will have you set WRITABLE_GRADEBOOK_URL: 'http://localhost:1994' on /edx/etc/lms.yml, but because we want the MFE on a subdirectory, let's set it to http://localhost:1800/gradebooks (this is based on how the MFE deploy role on configuration does it).
  • Go to your course's instructor panel as staff@example.com or similar.
  • Check the link. Note the URL, it's missing /gradebooks/

broken

After applying the fix:

fixed

Author notes and concerns: None.

Reviewers

@spokerman12 spokerman12 closed this Feb 5, 2021
@xitij2000
xitij2000 deleted the danielf/BTR-53-BB-3502 branch March 8, 2021 06:21
nizarmah pushed a commit that referenced this pull request Dec 14, 2021
from the edx-sga release notes at https://github.com/mitodl/edx-sga/blob/master/RELEASE.rst

- Update license classifier (#312)
- Update license text to match pypi's acceptable list (#310)
- Upgrade django to 3.2 LTS (#304)
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.

1 participant