Update handler_url support to match the latest in the xblock repo, for both js and python runtimes #1692
Conversation
|
At a first pass, test coverage is fairly low: https://jenkins.testeng.edx.org/job/edx-platform-report/735/Diff_Coverage_Report/? |
There was a problem hiding this comment.
Just want to point out in workbench/runtime.py this condition isn't checked for (if suffix='' there is still a /). I'm not clear on how important it is to have or remove this trailing slash.
There was a problem hiding this comment.
XModules append to it, so I wanted to make sure it was a clean prefix (w/o the trailing slash). For pure xblocks, it doesn't matter as much.
There was a problem hiding this comment.
Can we make any guarantees about slashlessness about these URLs? Certainly if there's a query, you can't just append "/foo" to the URL. The caller would be in control of the query I guess....
|
The code is good but would like to see some test coverage. |
|
I don't have anything to add beyond my note in 👍 |
There was a problem hiding this comment.
Is it customary in coffeescript to have no documentation or comments on functions? (This sounds sarcastic, but only half-so. Maybe there's something about coffeescript I don't understand.)
…r both js and python runtimes [LMS-1613]
|
👍 |
|
Only test failure is known studio acceptance test flakiness. Merging. |
Update handler_url support to match the latest in the xblock repo, for both js and python runtimes
3 participants
No description provided.