Summary: | Improved sorting in checkouts table | ||
---|---|---|---|
Product: | Koha | Reporter: | Martin Renvoize (ashimema) <martin.renvoize> |
Component: | Circulation | Assignee: | Martin Renvoize (ashimema) <martin.renvoize> |
Status: | In Discussion --- | QA Contact: | Testopia <testopia> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | amit.gupta, amitddng135, andrew, bolshawh, gmcharlt, koha, kyle.m.hall, lucas, marjorie.barry-vila, oleonard, patrick.robitaille |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24456 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 23493, 25890 | ||
Bug Blocks: | |||
Attachments: |
Bug 24533: Switch to aDataSort for multi-column sorting
sorting screenshot Bug 24533: Switch to aDataSort for multi-column sorting Bug 24533: Switch to aDataSort for multi-column sorting |
Description
Martin Renvoize (ashimema)
2020-01-29 16:22:53 UTC
Created attachment 98088 [details] [review] Bug 24533: Switch to aDataSort for multi-column sorting This patch adds aDataSort configurations for each sortable column such that we always maintain the group sorting primarily and apply a secondary sort using the column in question. When sorting by due date you are much more likely to want the true date sorting without forcing a primary sort on the grouping column. We achieve that by removing the primary sort on checkout date and disable rowGrouping in such a case. Sorry but with this patch applied the sorting is completely broken! Could you elaborate a bit for me Owen.. broken in what way.. for me sorting is working on all fields with this patch. Created attachment 99765 [details]
sorting screenshot
This did get a little weird on me. I checked out four things and then updated issuedate and date_due for some of them, ending up with these values:
select issue_id, issuedate, date_due from issues order by issuedate;
+----------+---------------------+---------------------+
| issue_id | issuedate | date_due |
+----------+---------------------+---------------------+
| 4 | 2020-02-19 14:17:00 | 2020-03-05 23:59:00 |
| 1 | 2020-02-21 14:17:00 | 2020-03-04 23:59:00 |
| 2 | 2020-02-28 14:17:44 | 2020-03-24 23:59:00 |
| 3 | 2020-02-28 14:17:50 | 2020-04-29 23:59:00 |
+----------+---------------------+---------------------+
Sorting looked ok to me except for when I tried to sort by Checked Out On (see attached screenshot). It looks like it might just be sorting on issue_id rather than date?
+----------+---------------------+---------------------+
| issue_id | issuedate | date_due |
+----------+---------------------+---------------------+
| 1 | 2020-02-21 14:17:00 | 2020-03-04 23:59:00 |
| 2 | 2020-02-28 14:17:44 | 2020-03-24 23:59:00 |
| 3 | 2020-02-28 14:17:50 | 2020-04-29 23:59:00 |
| 4 | 2020-02-19 14:17:00 | 2020-03-05 23:59:00 |
+----------+---------------------+---------------------+
I suppose in the real world sort by issue_id should be a reliable way to sort on checkout order, but it does open up some possible confusion if libraries are out there updating things from the database.
Created attachment 108836 [details] [review] Bug 24533: Switch to aDataSort for multi-column sorting This patch adds aDataSort configurations for each sortable column such that we always maintain the group sorting primarily and apply a secondary sort using the column in question. When sorting by due date you are much more likely to want the true date sorting without forcing a primary sort on the grouping column. We achieve that by removing the primary sort on checkout date and disable rowGrouping in such a case. Rebased and was about to dig into the errors you reported Andrew when I realised that I think they're already fixed by the bug that conflicted with this one causing the rebase requirement: bug 25890 Could you test again and let me know if you find any further issues.. all seems well to me now. Created attachment 113120 [details] [review] Bug 24533: Switch to aDataSort for multi-column sorting This patch adds aDataSort configurations for each sortable column such that we always maintain the group sorting primarily and apply a secondary sort using the column in question. When sorting by due date you are much more likely to want the true date sorting without forcing a primary sort on the grouping column. We achieve that by removing the primary sort on checkout date and disable rowGrouping in such a case. Rebased. The patch works as expected as far as I can tell, but I preferred the original behaviour prior to bug 23493: "* Initial table load would group the results by "Today's checkouts" and "Previous checkouts". * Any subsequent column sort would disable the grouping entirely." I like the way the "Today's checkouts" and "Previous checkouts" groupings are removed when sorting by date and would prefer the table to do the same when sorting by Title, branch etc. I can see that library staff might want to see the "Today's checkouts" and "Previous checkouts" groupings. However, if people wanted to keep the today/previous grouping, I think it would make most sense to keep this grouping when sorting by date and remove it for all other columns i.e. the opposite of what this patch does. (In reply to Henry Bolshaw from comment #9) > The patch works as expected as far as I can tell, but I preferred the > original behaviour prior to bug 23493: > > "* Initial table load would group the results by "Today's checkouts" and > "Previous checkouts". > * Any subsequent column sort would disable the grouping entirely." > > I like the way the "Today's checkouts" and "Previous checkouts" groupings > are removed when sorting by date and would prefer the table to do the same > when sorting by Title, branch etc. > > I can see that library staff might want to see the "Today's checkouts" and > "Previous checkouts" groupings. However, if people wanted to keep the > today/previous grouping, I think it would make most sense to keep this > grouping when sorting by date and remove it for all other columns i.e. the > opposite of what this patch does. +1 +1 |