feat: ensure lms api synced with latest value in config

[ghassanmas]

This change make it possible if LMS url to be changed, that
the last value will be picked.

This is simlair openedx/frontend-app-authoring/pull/389
which issue overhangio/tutor-mfe/issues/86, the fixes is needed
so that dynamic config would work with tutor:
overhangio/tutor-mfe/pull/69

TL;DR - [ A short summary of what this PR does and why ]

• This change is need to so that when LMS URL value is change at runtime. The case of which case happen is when that URL is change through dynamic config. If the LMS url is deifned only at build time this change shouldn't affect it.

• The course authroing mfe has simlair issue with CMS url ref: fix: force studio url to reload if changed frontend-app-authoring#389

What changed?

• [ More in depth breakdown of changes ]

  • Converting all the use of urls from value to a fucntion, by mkaing the urls obejct return a function than a static object, you will be able to get the latest value of LMS url.

  Here is a simple example of our case here:

  The situation before

  const LMS  = getConfing().LMS  // assume LMS_1 is example.com
  
  setConfig({LMS: openedx.com})  
  
   LMS === getConfig().LMS  // returs false now its a porblem when any function/ is using LMS_1 varaible

  The situation after

  const getLMS = ()=> `${getConfing().LMS}`  // assume LMS is example.com
  
  setConfig({LMS: openedx.com})  
  
   getLMS() === getConfig().LMS  // returs true now its not a porblem when any function/ is using LMS_1 varaible 

Developer Checklist

• Test suites passing I tested with tutor
• Documentation and test plan updated, if applicable
• Received code-owner approving review
• Bumped version number package.json

Testing Instructions

• Since this change would fix the following case:

  • Dynamic API config is enabled by the LMS
  • The gradebook MFE rely on LMS url to be set at runtime
    • For this to be true then Dynamic config API for this MFE should be relative and a proxy need to be set to forward for the lms, check this PR for more context MFE dynamic config overhangio/tutor-mfe#69

• In case dynamic config is off, this change doesn't suppose to affect anything,

Reviewer Checklist

Collectively, these should be completed by reviewers of this PR:

• I've done a visual code review
• I've tested the new functionality

FYI: @openedx/content-aurora

[openedx-webhooks]

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

[codecov]

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (912ff8b) compared to base (4f43e65).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #289   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          109       109           
  Lines         1264      1263    -1     
  Branches       248       248           
=========================================
- Hits          1264      1263    -1     

Impacted Files	Coverage Δ
src/data/services/lms/api.js	100.00% <100.00%> (ø)
src/data/services/lms/urls.js	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[arbrandes]

I have yet to test this, but the code looks good.

Why are we updating package-lock in the same commit, though?

[ghassanmas]

The change in pacakge-lock.json is realted just to version number which @kdmccormick changed in the previous change in pacakge.json #286.
So in summary my npm updated the pacakge-lock.json to match the version in pacakge.json. anyway I can remove that change if necessary

[kdmccormick]

FYI @openedx/content-aurora : The team ping in the PR template didn't work, probably because Ghassan doesn't have direct write access to this repo.

[arbrandes]

@muselesscreator, is this something you or somebody on your team could review in the next couple of days? It's pretty important for the Tutor/Olive release, which is happening next Monday (the 11th) at the latest.

[nsprenkle]

@ghassanmas, can you update PR description template fields (lots of stuff left black which should be filled or deleted) and provide testing instructions? Thanks you.

[ghassanmas]

@nsprenkle to be able to test this you would need to use enable dynamic config api, plus you would need to rely on the LMS URL, and to do that second you have to set a proxy for the mfe config url... This assuming you don't use tutor.

Again the thing with testing this, I am not sure what is your dev env, anyway as @kdmccormick in simlair PR you can follow the same steps.

Essetionaly what this change does, is that it enforce that the call of getConfig to be called as from a function not to be called at the file level i.e. Hoisting. It the exact same issue we had with course authoring in this openedx/frontend-app-authoring/pull/389 . It's just in this case it's the LMS url instead of CMS for the course authroing.

And again to not confuse you @kdmccormick change was necessary but it's not enough to make an MFE use dyanmic config, an MFE would need to load the config from frontend-platform, and it also needs to ensure that configuration are loaded from a function because otherwise the variabale would be defined once, as mentioned before Hoisting

[kdmccormick]

@ghassanmas -- Thanks for that info, but please fill it into the PR template.

@nsprenkle -- MFE runtime config will be enabled in Tutor and disabled-by-default for the Devstack/edx.org installation. So:

• If you have time in the next couple of days day to provision Tutor and test this change out using runtime config, you could do that. I'm free most of today and would be happy to help you with any provisioning issues.
• Alternatively, you could just smoke test the edx.org static-config use case by running Gradebook normally with devstack (you'd just be confirming that nothing is broken), and then delegate the Tutor runtime-config use case testing to @arbrandes and I.

[ghassanmas]

@nsprenkle @ghassanmas I have added more context and info in the template. Hope that is suffiecnt, in any case I am happy to assist if needed.

[kdmccormick]

@nsprenkle Since we haven't heard from you, Adolfo and/or I are going to move forward with testing, reviewing, and merging this.

[arbrandes]

I tested this on both a devstack and Tutor. As expected, there were no changes detectable on the first, and on the latter the MFE now works.

[openedx-webhooks]

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