refactor: handle relative proctoring link - #974
Conversation
|
Thanks for the pull request, @navinkarkera! 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. |
1 similar comment
|
Thanks for the pull request, @navinkarkera! 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. |
| if (proctoringExamConfigurationLink && !( | ||
| proctoringExamConfigurationLink.indexOf('http://') === 0 || proctoringExamConfigurationLink.indexOf('https://') === 0 | ||
| )) { | ||
| fullProctoringExamConfigurationLink = new URL(proctoringExamConfigurationLink, getConfig().STUDIO_BASE_URL).href; |
There was a problem hiding this comment.
| fullProctoringExamConfigurationLink = new URL(proctoringExamConfigurationLink, getConfig().STUDIO_BASE_URL).href; | |
| fullProctoringExamConfigurationLink = () => new URL(proctoringExamConfigurationLink, getConfig().STUDIO_BASE_URL).href; |
There was a problem hiding this comment.
There are known issues with Tutor where the variable will not generate the link unless new URL is called as the return of a function.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #974 +/- ##
=======================================
Coverage 92.20% 92.20%
=======================================
Files 703 703
Lines 12334 12336 +2
Branches 2671 2665 -6
=======================================
+ Hits 11372 11374 +2
Misses 926 926
Partials 36 36 ☔ View full report in Codecov by Sentry. |
03f362a to
c0368d2
Compare
| let fullProctoringExamConfigurationLink = () => proctoringExamConfigurationLink; | ||
|
|
||
| // check if proctoringExamConfigurationLink is a relative link | ||
| if (proctoringExamConfigurationLink && !( | ||
| proctoringExamConfigurationLink.indexOf('http://') === 0 || proctoringExamConfigurationLink.indexOf('https://') === 0 | ||
| )) { | ||
| // new URL needs to called inside a function for tutor | ||
| fullProctoringExamConfigurationLink = () => new URL( | ||
| proctoringExamConfigurationLink, | ||
| getConfig().STUDIO_BASE_URL, | ||
| ).href; | ||
| } |
There was a problem hiding this comment.
I this this code is more complex than it needs to be.
| let fullProctoringExamConfigurationLink = () => proctoringExamConfigurationLink; | |
| // check if proctoringExamConfigurationLink is a relative link | |
| if (proctoringExamConfigurationLink && !( | |
| proctoringExamConfigurationLink.indexOf('http://') === 0 || proctoringExamConfigurationLink.indexOf('https://') === 0 | |
| )) { | |
| // new URL needs to called inside a function for tutor | |
| fullProctoringExamConfigurationLink = () => new URL( | |
| proctoringExamConfigurationLink, | |
| getConfig().STUDIO_BASE_URL, | |
| ).href; | |
| } | |
| const fullProctoringExamConfigurationLink = () => ( | |
| proctoringExamConfigurationLink && new URL(proctoringExamConfigurationLink, getConfig().STUDIO_BASE_URL.href; | |
| ); |
If the link is defined, and is relative then this will make the url relative to the studio base. If it's not a relative url it will override the base url.
There was a problem hiding this comment.
I'm also not sure why this needs to be a function, nothing that runs in it should change between the component being mounted and the function being called.
There was a problem hiding this comment.
@xitij2000 Thanks! updated.
I'm also not sure why this needs to be a function, nothing that runs in it should change between the component being mounted and the function being called.
Same here. @KristinAoki Do we have a link or a PR which explains this tutor related bug?
c0368d2 to
3e74e63
Compare
xitij2000
left a comment
There was a problem hiding this comment.
👍
- I tested this: tested on tutor devstack
- I read through the code
- I checked for accessibility issues
- Includes documentation
|
@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future. |
Description
If proctoring link is not an absolute url, update it to use studio as base url.
Supporting information
Private-ref: BB-8832