fix: decodeURIComponent for locationId

[vunguyen-dmt]

The current code is wrong for this case (localtion id contains characters like: Đ) :
https://apps.demo.openedx.overhang.io/ora-grading/block-v1:ORG+Đ001+2023+type@openassessment+block@14709d5612014c57befb3c278b3b9b4f

Current code:

window.location.pathname.replace(getConfig().PUBLIC_PATH, '') = 
block-v1:ORG+%C4%90+2023+type@openassessment+block@14709d5612014c57befb3c278b3b9b4f

This PR code

decodeURIComponent(window.location.pathname).replace('/ora-grading/', '') =
block-v1:ORG+Đ+2023+type@openassessment+block@14709d5612014c57befb3c278b3b9b4f

The localtionId is used in initialize request
{LMS_BASE_URL}/api/ora_staff_grader/initialize?oraLocation={locationId}

[openedx-webhooks]

Thanks for the pull request, @vunguyen-dmt! 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.

[vunguyen-dmt]

Signed CLA for github username: vunguyen-dmt.

[codecov]

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (cc11ce0) 100.00% compared to head (1f4d50b) 100.00%.

❗ Current head 1f4d50b differs from pull request most recent head 86cce77. Consider uploading reports for the commit 86cce77 to get more accurate results

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #189   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          110       110           
  Lines         1079      1079           
  Branches       159       159           
=========================================
  Hits          1079      1079           

Files	Coverage Δ
src/data/constants/app.js	100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mphilbrick211]

Hi @openedx/content-aurora! Would some be able to review/merge this for us? Thanks!

[mphilbrick211]

Hi @openedx/content-aurora, just checking in on this :)

[leangseu-edx]

I am confuse for what this pr suppose to do?

const url = 'https://apps.demo.openedx.overhang.io/ora-grading/block-v1:ORG+Đ001+2023+type@openassessment+block@14709d5612014c57befb3c278b3b9b4f'
decodeURIComponent(url) === url

[vunguyen-dmt]

Hi @leangseu-edx.
If you paste the URL to a web browser and try window.location.pathname and decodeURIComponent(window.location.pathname) they're two different results. The one without decodeURIComponent caused me errors in my course.

My courses were created back in Maple or Lilac through course_run API so some of them may have characters not from [A-z-az0-9-_.].

I believe in the current version (Palm) if courses were created in the web UI, they should not have any problems with decoding URLs.

If this app was created with an assumption that course id only contains [A-z-az0-9_-.] characters then this PR is not needed.
But if somewhere in the past or in the future this assumption was not true then it's safer to decodeURIComponent.

[leangseu-edx]

Can you update src/data/constants/app.test.js?

import * as platform from '@edx/frontend-platform';
import * as constants from './app';

jest.unmock('./app');

jest.mock('@edx/frontend-platform', () => {
  const PUBLIC_PATH = '/test-public-path/';
  return {
    getConfig: () => ({ PUBLIC_PATH }),
    PUBLIC_PATH,
  };
});

describe('app constants', () => {
  test('route path draws from public path and adds courseId', () => {
    expect(constants.routePath()).toEqual(`${platform.PUBLIC_PATH}:courseId`);
  });
  describe('locationId', () => { 
    const domain = 'https://example.com';

    test('returns trimmed pathname', () => {
      const old = window.location;
      Object.defineProperty(window, "location", {
        value: new URL(`${domain}${platform.PUBLIC_PATH}somePath.jpg`),
        writable: true,
      });
      expect(constants.locationId()).toEqual(window.location.pathname.replace(platform.PUBLIC_PATH, ''));
      window.location = old;
    });
    test('handle none-standard characters pathName', () => {
      const old = window.location;
      const noneStandardPath = `${platform.PUBLIC_PATH}ORG+%C4%90+2023`;
      const expectedPath = `${platform.PUBLIC_PATH}ORG+Đ+2023`;
      Object.defineProperty(window, "location", {
        value: new URL(`${domain}${noneStandardPath}`),
        writable: true,
      });
      expect(noneStandardPath).not.toEqual(expectedPath);
      expect(constants.locationId()).toEqual(expectedPath.replace(platform.PUBLIC_PATH, ''));
      window.location = old;
    });
  });
});

[vunguyen-dmt]

Thanks! I updated it.

[leangseu-edx]

Please update the entire file. I added another test specifically for this one and the previous test wasn't working properly.

[mphilbrick211]

Hi @leangseu-edx! Would you mind enabling tests again for this PR?

[mphilbrick211]

Hi @vunguyen-dmt! It looks like there is a failing test - would you mind having a look? Thanks!

[mphilbrick211]

Hi @vunguyen-dmt! It looks like there is a failing test - would you mind having a look? Thanks!

Hi @vunguyen-dmt! Just following up on this :)

[vunguyen-dmt]

I can do some JavaScript, but I don't know much about writing Reactjs unit tests.
Will try to see if I can fix it when I have time!

[vunguyen-dmt]

Can you update src/data/constants/app.test.js?

import * as platform from '@edx/frontend-platform';
import * as constants from './app';

jest.unmock('./app');

jest.mock('@edx/frontend-platform', () => {
  const PUBLIC_PATH = '/test-public-path/';
  return {
    getConfig: () => ({ PUBLIC_PATH }),
    PUBLIC_PATH,
  };
});

describe('app constants', () => {
  test('route path draws from public path and adds courseId', () => {
    expect(constants.routePath()).toEqual(`${platform.PUBLIC_PATH}:courseId`);
  });
  describe('locationId', () => { 
    const domain = 'https://example.com';

    test('returns trimmed pathname', () => {
      const old = window.location;
      Object.defineProperty(window, "location", {
        value: new URL(`${domain}${platform.PUBLIC_PATH}somePath.jpg`),
        writable: true,
      });
      expect(constants.locationId()).toEqual(window.location.pathname.replace(platform.PUBLIC_PATH, ''));
      window.location = old;
    });
    test('handle none-standard characters pathName', () => {
      const old = window.location;
      const noneStandardPath = `${platform.PUBLIC_PATH}ORG+%C4%90+2023`;
      const expectedPath = `${platform.PUBLIC_PATH}ORG+Đ+2023`;
      Object.defineProperty(window, "location", {
        value: new URL(`${domain}${noneStandardPath}`),
        writable: true,
      });
      expect(noneStandardPath).not.toEqual(expectedPath);
      expect(constants.locationId()).toEqual(expectedPath.replace(platform.PUBLIC_PATH, ''));
      window.location = old;
    });
  });
});

I've copied these code to apps.test.js. Is there anything else?
Sorry for the long delay!

[mphilbrick211]

Hi @leangseu-edx! Would you be able to help with the author's question, and re-enable checks to run?

[openedx-webhooks]

@vunguyen-dmt 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.