Skip to content

Issue 1520 - Body → Chapter (required) Arabic numeral label descending...

Duncan Bennett requested to merge 1520-order-by-chapter-descending into develop

feat(server): order by chapter number desc

#1520 (closed)

I'm creating this merge request now so that it can be reviewed and tested while I am working on the unit tests.

Adds some comments on the expected behaviour of "orderModel.js".

Adds "chapterNumberDesc" option when sorting "body" division. Adds matching "number_desc" select value which is passed to "sorting callback". Updates "BookComponent" and "Division" so that their callbacks check for "number_desc" in addition to "number".

Fixes what appears to be a small bug. 'asc' and 'desc' order seem to be reversed. If this fix is valid, then we need QA to check the sort results for all entities which use "OrderModel". If this fix is not valid, I will roll it back and switch around the "order" values for "BookComponent" and "Division".

Updates the front end logic which checks whether 'metadata.chapter_number' contains sortable data. 'ui/Pages/Bookmanager/Book/Row.js' needs to do more than test that 'metadata.chapter_number' is populated; it needs to check that the value ends with a number (is sortable).

I still have to write unit tests for the following:

  • Confirm that valid chapter numbers are sorted in descending order
  • Confirm that invalid chapter numbers are ignored
  • Confirm that sorting an empty list does not cause errors
  • If there is time, write tests for the "OrderModel" component of "OrderService"

Things to test:

  • 'asc' and 'desc' order were inverted and this should have been fixed. Has this fix resulted in any unwanted changes to the way chapters are ordered? Eg: are there now elements at the bottom of the list of chapters which should be at the top?
  • When the chapter number us not set, a red TOC tag should appear for that row
  • When the chapter number us not sortable, because it does not end with a number, a red TOC tag should appear for that row

Merge request reports