add a /jump_to_id/ shortcut for producing more durable links between cou... #452
Conversation
|
@ormsbee @dmitchell can you review? See motivations up above. This solves a big pain for course authors. @talbs can you check the edit unit page. I have a visual change that you should sign off on. @TeppoJouttenus can I tag you to review from the course author's point of view. The links between courseware can now be more easily - and robustly - constructed like:
|
|
Need to look at build instability, but I can't get to Jenkins from home. Tests pass locally, so it must be some violations. |
| @@ -171,7 +171,7 @@ <h4 class="header">${_("Unit Settings")}</h4> | |||
| <div class="window unit-location"> | |||
| <h4 class="header">${_("Unit Location")}</h4> | |||
| <div class="window-contents"> | |||
| <div><input type="text" class="url" value="/courseware/${section.url_name}/${subsection.url_name}" disabled /></div> | |||
| <div>${_("Unit Identifier:")} <input type="text" class="url" value="${unit.location.name}" disabled /></div> | |||
There was a problem hiding this comment.
Split mongo ensures that the block id is unique w/in the course, but I don't believe old mongo does. It just requires uniqueness of the (category, name) tuple within the course I believe. As to whether this is a real problem, idk.
|
Thanks for sharing @chrisndodge. I took a look and made some quick HTML/styling changes to this branch. This is good to go from my point of view. 👍 |
|
Presuming this is adequately tested, the only concern I have is whether the name is unique w/in the course, but I think the url_name would have the same problem; so, I doubt it's a regression. Split mongo ensures the block ids are unique w/in course. Old mongo only ensures they're unique w/in category afaik. |
|
@dmitchell there's no hard uniqueness constraint, but - in effect - since Studio generates guid's for the location.name field for all elements (outside of the Course module), uniqueness across all content can be reasonably safely assumed. The only caveat could be for importing old XML courses, but I'd hope - as best practice - those authors would also try to make them unique. |
|
Now that I can look at Jenkins, seems like I have poor diff-coverage numbers. I'll increase tests although I would have thought these substitutions would have been covered through other tests which enumerates and renders modules. |
|
@chrisndodge Looks great to me, I agree that this will be very helpful for course authors. As a separate feature request (I submitted this at some point), we could make it easier for course authors to find the ID's by creating a labeled box etc from which to copy paste. |
|
@TeppoJouttenus thanks for the thumbs up. This PR indeed exposes a new "ID" field in the Unit page, so authors will be able to easily know what the ID for their jump_to_id targets are. |
|
@ormsbee can I ping you on a code review while I figure out the diff-coverage disparity? |
|
@chrisndodge: looking now |
| <div class="row wrapper-unit-id"> | ||
| <p class="unit-id"> | ||
| <span class="label">${_("Unit Identifier:")}</span> | ||
| <input type="text" class="url value" value="${unit.location.name}" disabled /> |
There was a problem hiding this comment.
No need for escaping? What kinds of things can be in a name?
There was a problem hiding this comment.
location.name here is always a GUID when it is created in Studio. If it's imported from an XML course, then it will be the 'url_name', which - I presume - is appropriate for URL purposes.
|
@ormsbee I addressed most of your comments - otherwise replied to two other comments |
| self.mock_user, | ||
| mock_request, | ||
| ['i4x', 'edX', 'toy', 'html', 'toyjumpto'], | ||
| model_data_cache, self.course_id |
There was a problem hiding this comment.
Sorry, I realize this is pedantic, but new line for self.course_id please?
|
After merge conflicts resolved, 👍 |
…courseware in Studio
|
OK, rebased and resolve conflicts (on toy course content). Waiting for build to pass.... |
add a /jump_to_id/ shortcut for producing more durable links between cou...
Feature/bridger/new wiki
style(login & reigster): inputbox vertical align
* README Updates
...rseware in Studio
NOTE TO REVIEWERS:
This work is motivated by:
the existing /course/ shortcut does not work well for Studio authored course since the author would need to know the URL sub-path that the target would have in the LMS. The sub-path also ends with an ordinal (index into the sequence). This is nearly imposible for the author to know when building a course
/course/ shortcut is also not robust/durable when items are moved around in the course tree
the jump_to redirect URL had been recomended to authors but it is very, very awkward to have the author determine what the 'i4x' location should be
the jump_to redirect is not durable when "re-namespacing" a course (e.g. export and re-import into a different course) as the org and course are 'burned' into content
The solution is to create a new shortcut that authors can use:
<a href='/jump_to_id/{id}' > link is here </a>where {id} is the location.name. In Studio location.name is always a GUID and therefore can be assumed to be unique.
This shortcut get's rewritten at LMS rendering time, using the same get_html() wrappers that /static/ and /course/ shortcuts use.
Also, there is a small change in the edit Unit page where we removed that useless display of the /course/aaa/bbb. There we now just display the Id of the Unit (aka Vertical)