In catalogue/detail.pl page the "Serial enumeration / chronology" column sorting is broken after the changes in bug 27980. To reproduce problem add 2 items, one with "A" and one with "B" value in "h - Serial Enumeration / chronology item" field. Then go to catalogue/detail.pl and try to sort by clicking on the "Serial enumeration / chronology" column.
Created attachment 119869 [details] [review] Bug 28171: Serial enumeration / chronology sorting is broken in biblio page This patch updates the bibliographic detail page so that the "Serial enumeration/chronology" column is sorted by published date only if a published date is present. To test, apply the patch and locate a bibliograhpic record for a subscription that has multiple items. On the bibliographic detail page, confirm that the "Serial enumeration/chronology" column sorts correctly.
Created attachment 119896 [details] [review] Bug 28171: Serial enumeration / chronology sorting is broken in biblio page This patch updates the bibliographic detail page so that the "Serial enumeration/chronology" column is sorted by published date only if a published date is present. To test, apply the patch and locate a bibliograhpic record for a subscription that has multiple items. On the bibliographic detail page, confirm that the "Serial enumeration/chronology" column sorts correctly. Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Hmm, the proposed solution here is not like it was before bug 27980. Before we sorted either with publisheddate alone if one of the items had it at least, but now with this we would sort both of the values. This seems okay because how this seems to work now is that the ones with the data-order attribute set (so publisheddate) are sorted as one group and then the ones without data-order are sorted as another group. So both get sorted, but separately. Owen, do you think this change in sorting order is okay? Previously the ones without publisheddate didn't get sorted so this might actually be improvement but want to hear your opinion as well. Also I didn't find any documentation for this in DataTables manual so we might also want to consider if we should depend on this and whether the sorting order will possibly change in future if DataTables changes some code because it seems undocumented feature.
I'm glad to have your detailed feedback, because I don't have good serials data to test with and I wasn't sure I was mocking it up correctly. I don't know the answer to whether the grouped sorting is better, so I defer to your opinion. What feature do you mean when you say it isn't documented? The data-order sorting option is documented here: https://datatables.net/examples/advanced_init/html5-data-attributes.html
(In reply to Owen Leonard from comment #4) > I'm glad to have your detailed feedback, because I don't have good serials > data to test with and I wasn't sure I was mocking it up correctly. I don't > know the answer to whether the grouped sorting is better, so I defer to your > opinion. > > What feature do you mean when you say it isn't documented? The data-order > sorting option is documented here: > https://datatables.net/examples/advanced_init/html5-data-attributes.html The feature of Datatables sorting separately the rows with order-by attribute and the ones without and then concatenating them. There is no mention of that in the documentation so it probably could have been implemented so that it would sort everything as one group. I think the undocumented sorting functionality here is good but only doubt is that if it breaks in the future and our users will expect us to fix that it might not be possible.
In the previous message with "order-by" I wanted to say instead "data-order"
From the DataTables website I found this: "In order for the HTML 5 data-* attribute detection and processing to work correctly, all cells in a column must have the same attribute available. Without this, DataTables will give a warning." (https://datatables.net/manual/data/orthogonal-data) So unless I'm misunderstanding we probably shouldn't even depend on this, but go back to the old behaviour where we had all the columns having the data-order value set even though for some it was empty. However, I didn't get this warning message when testing.
Owen, so my recommendation would be that in this bug we restore the old behaviour before bug 27980 and if somebody wants to modify that let's open a new bug for this. The "Bug 27980: Remove itemdata_publisheddate from controller" needs to be reverted to restore the old behaviour.
Created attachment 119960 [details] [review] Bug 28171: Revert "Bug 27980: Remove itemdata_publisheddate from controller" This reverts commit c7fef23763754ad65c5a59438ed370d53aeec4b6. The code is needed back to fix broken sorting.
Created attachment 119961 [details] [review] Bug 28171: Restore previous sorting behaviour for Serial enumchron column Before Bug 27980 the sorting was done either by item's publisheddate if it existed in at least one item. If it didn't exist then the sorting was done using "h - Serial Enumeration / chronology" field. This restores this sorting functionality in catalogue/detail.pl page. To test: 1. Create a serial subscription for a biblio, make sure to enable the option "Create an item record when receiving this serial" 2. Receive two serials, so that they have different received dates and check that the items are now created to the biblio 3. Add now two additional items with one having "A" and other having "B" in the "h - Serial Enumeration / chronology" field. 4. Try sorting and notice the items with publisheddate are being sorted and ones without stay in the order they happened to be 5. Remove the items with publisheddate and notice that now items with "A" and "B" get sorted correctly
Attached solution that should restore exactly the previous behaviour. Let me know what you think.
Hi Joonas, Your latest patch fixes the issue, so I am wondering if it is still necessary to revert c7fef23763754ad65c5a59438ed370d53aeec4b6 ?
(In reply to Lucas Gass from comment #12) > Hi Joonas, > > Your latest patch fixes the issue, so I am wondering if it is still > necessary to revert c7fef23763754ad65c5a59438ed370d53aeec4b6 ? It is necessary, my code uses the code that c7fef23763754ad65c5a59438ed370d53aeec4b6 removed so it needs to be restored by reverting the commit.
Created attachment 121210 [details] [review] Bug 28171: Revert "Bug 27980: Remove itemdata_publisheddate from controller" This reverts commit c7fef23763754ad65c5a59438ed370d53aeec4b6. The code is needed back to fix broken sorting. Signed-off-by: cnelson <christian.nelson@tritonia.fi>
Created attachment 121211 [details] [review] Bug 28171: Restore previous sorting behaviour for Serial enumchron column Before Bug 27980 the sorting was done either by item's publisheddate if it existed in at least one item. If it didn't exist then the sorting was done using "h - Serial Enumeration / chronology" field. This restores this sorting functionality in catalogue/detail.pl page. To test: 1. Create a serial subscription for a biblio, make sure to enable the option "Create an item record when receiving this serial" 2. Receive two serials, so that they have different received dates and check that the items are now created to the biblio 3. Add now two additional items with one having "A" and other having "B" in the "h - Serial Enumeration / chronology" field. 4. Try sorting and notice the items with publisheddate are being sorted and ones without stay in the order they happened to be 5. Remove the items with publisheddate and notice that now items with "A" and "B" get sorted correctly Signed-off-by: cnelson <christian.nelson@tritonia.fi>
Created attachment 121212 [details] [review] Bug 28171: Revert "Bug 27980: Remove itemdata_publisheddate from controller" This reverts commit c7fef23763754ad65c5a59438ed370d53aeec4b6. The code is needed back to fix broken sorting. Signed-off-by: cnelson <christian.nelson@tritonia.fi> Signed-off-by: cnelson <christian.nelson@tritonia.fi>
Created attachment 121213 [details] [review] Bug 28171: Restore previous sorting behaviour for Serial enumchron column Before Bug 27980 the sorting was done either by item's publisheddate if it existed in at least one item. If it didn't exist then the sorting was done using "h - Serial Enumeration / chronology" field. This restores this sorting functionality in catalogue/detail.pl page. To test: 1. Create a serial subscription for a biblio, make sure to enable the option "Create an item record when receiving this serial" 2. Receive two serials, so that they have different received dates and check that the items are now created to the biblio 3. Add now two additional items with one having "A" and other having "B" in the "h - Serial Enumeration / chronology" field. 4. Try sorting and notice the items with publisheddate are being sorted and ones without stay in the order they happened to be 5. Remove the items with publisheddate and notice that now items with "A" and "B" get sorted correctly Signed-off-by: cnelson <christian.nelson@tritonia.fi> Signed-off-by: cnelson <christian.nelson@tritonia.fi>
Created attachment 121214 [details] [review] Bug 28171: Revert "Bug 27980: Remove itemdata_publisheddate from controller" This reverts commit c7fef23763754ad65c5a59438ed370d53aeec4b6. The code is needed back to fix broken sorting. Signed-off-by: Christian Nelson <christian.nelson@tritonia.fi>
Created attachment 121215 [details] [review] Bug 28171: Restore previous sorting behaviour for Serial enumchron column Before Bug 27980 the sorting was done either by item's publisheddate if it existed in at least one item. If it didn't exist then the sorting was done using "h - Serial Enumeration / chronology" field. This restores this sorting functionality in catalogue/detail.pl page. To test: 1. Create a serial subscription for a biblio, make sure to enable the option "Create an item record when receiving this serial" 2. Receive two serials, so that they have different received dates and check that the items are now created to the biblio 3. Add now two additional items with one having "A" and other having "B" in the "h - Serial Enumeration / chronology" field. 4. Try sorting and notice the items with publisheddate are being sorted and ones without stay in the order they happened to be 5. Remove the items with publisheddate and notice that now items with "A" and "B" get sorted correctly Signed-off-by: Christian Nelson <christian.nelson@tritonia.fi>
As Christian was signing-off the patches via Koha Sandbox the signed-off line didn't update to the correct name. Fixed it now on behalf of Christian in the latest patches attached. Ready for QA.
Created attachment 121483 [details] [review] Bug 28171: Revert "Bug 27980: Remove itemdata_publisheddate from controller" This reverts commit c7fef23763754ad65c5a59438ed370d53aeec4b6. The code is needed back to fix broken sorting. Signed-off-by: Christian Nelson <christian.nelson@tritonia.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 121484 [details] [review] Bug 28171: Restore previous sorting behaviour for Serial enumchron column Before Bug 27980 the sorting was done either by item's publisheddate if it existed in at least one item. If it didn't exist then the sorting was done using "h - Serial Enumeration / chronology" field. This restores this sorting functionality in catalogue/detail.pl page. To test: 1. Create a serial subscription for a biblio, make sure to enable the option "Create an item record when receiving this serial" 2. Receive two serials, so that they have different received dates and check that the items are now created to the biblio 3. Add now two additional items with one having "A" and other having "B" in the "h - Serial Enumeration / chronology" field. 4. Try sorting and notice the items with publisheddate are being sorted and ones without stay in the order they happened to be 5. Remove the items with publisheddate and notice that now items with "A" and "B" get sorted correctly Signed-off-by: Christian Nelson <christian.nelson@tritonia.fi> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
(In reply to Joonas Kylmälä from comment #11) > Attached solution that should restore exactly the previous behaviour. Let me > know what you think. Regarding bug 13420 it seems that we need to restore the previous behaviour, and not pick Owen's patch (what I was thinking first). commit 0ecf7694eac802f29ea4786502b7bffaaad3a6bf Bug 13420: Fallback to the previous behavior if published date is not used [% IF volinfo %] [% IF itemdate_publisheddate #If there is at least one published date, use it for sorting%] <th class="title-string">Serial enumeration / chronology</th> [% ELSE %] <th>Serial enumeration / chronology</th> [% END %] [% END %] Maybe we should restore this comment.
Created attachment 121593 [details] [review] Bug 28171: Restore comment to remove ambiguity
(In reply to Jonathan Druart from comment #23) > Maybe we should restore this comment. Yeah, good idea. :)
Pushed to master for 21.05, thanks to everybody involved!
Pushed to master for 21.11, thanks to everybody involved!
Pushed to 21.05.x for 21.05.01
Depends on Bug 27980 not in 20.11.x
It looks like we have the "clever" sorting only on staff side, any reason why we shoudl not do the same for the OPAC?