search

Simon-Initiative / oli-torus

Next Generation OLI Authoring and Delivery Platform
https://proton.oli.cmu.edu
MIT License
83 stars 35 forks source link

[ENHANCEMENT] [MER-3125] sort line item creation by hierarchy ordering #4873

Closed eliknebel closed 3 months ago

eliknebel commented 3 months ago

https://eliterate.atlassian.net/browse/MER-3125

This PR changes the "Update LMS Line Items" function to create and update graded line items in the order they appear in the hierarchy, and append all non-hierarchical pages to the end.

This is a stop-gap implementation in order to fix the issue short term of gradebook columns appearing in a seemingly random order. Other improvements to be made here in the future are:

  1. Store the page ordering at the database level or in a reachability data structure so that the hierarchy doesn't need to be built in memory for pages to be sorted
  2. Use a reachability data structure to properly sort non-hierarchical pages, instead of just appending them to the end

This was tested using a Canvas instance on the Chem course and behaved as expected, but it would be good to test on the LMS' referenced in the ticket, Brightspace and D2L.

Screenshot 2024-06-12 at 5 00 16 PM
darrensiegel commented 3 months ago

Also, I see task 2 of "Call line items creation function automatically during LMS section creation". I do not believe that we should do this. Imagine an instructor that is creating a course section with then the intention of remixing to remove some units or modules that they do not plan to cover. A feature where these line items were created automatically at section creation time would populate the LMS gradebook with graded pages that the instructor isn't going to use.

I could be onboard with "Adding an optional feature of course section creation, where we let LMS course section instructors choose if they want to have the system populate the LMS gradebook line items". But that seems like too much work given this stage of development.

eliknebel commented 3 months ago

Couldn't the "PreviousNextIndex", which is precomputed and already stored in the section record be traversed to determine the page ordering?

I don't initially see any prior art here to traverse this cached index, I suppose we could write a map_reduce function that performs an in-order traversal over this index to build the list of ordered graded pages but this might take a bit more work to optimize this "Update LMS Line Items" that theoretically should only need to be run once per course section