Bug 34107

Summary: Sorting agreements by Name actually sorts by ID
Product: Koha Reporter: Katrin Fischer <katrin.fischer>
Component: ERMAssignee: Pedro Amorim <pedro.amorim>
Status: CLOSED FIXED QA Contact:
Severity: minor    
Priority: P5 - low CC: black23, jonathan.druart, jonathan.field, martin.renvoize, matt.blenkinsop, pedro.amorim
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.02,22.11.08
Circulation function:
Bug Depends on: 33973    
Bug Blocks:    
Attachments: Bug 34107: Remove id from name column in erm list tables
Bug 34028: Show the secret when registering from the auth screen
Bug 34107: Switch order for id and name feeding the first column in datatables
Bug 34107: Switch order for id and name feeding the first column in datatables
Bug 34107: Switch order for id and name feeding the first column in datatables

Description Katrin Fischer 2023-06-22 22:01:09 UTC
There is still something wrong when searching by Name. Add agreements in this sequence:
t
b
z

They get sequential IDs (1 2 3 or similar).

Now when you sort on the Name column, they won't sort by the description, but by their ID: zbt or tbz
Comment 1 Pedro Amorim 2023-06-23 08:28:45 UTC
Created attachment 152622 [details] [review]
Bug 34107: Remove id from name column in erm list tables
Comment 2 Michal Denar 2023-06-30 20:57:24 UTC
Any test plan?
Comment 3 Pedro Amorim 2023-07-04 07:57:25 UTC
(In reply to Michal Denar from comment #2)
> Any test plan?

* Create 3 agreements, agreement #1 named 'a', agreement #2 named 'c' and agreement #3 named 'b'.
* Go to agreements list, click the "Name" column header, notice how the agreements get sorted by id #, not by first char in name. Expected order would be abc or cba, but it's acb or bca.
* Apply patch, on k-t-d, run the following if you're not using 'yarn js:watch':
yarn js:build
* Sort the list again on the 'name' column, notice how it now sorts alphabetically as expected, either abc or cba.
Comment 4 Jonathan Druart 2023-07-04 09:20:54 UTC
Then you are removing the ability to filter by id on the column (where we show it).
The correct fix I think is to put the id after the name:

-                    data: "me.agreement_id:me.name",
+                    data: "me.name:me.agreement_id",
Comment 5 Jonathan Druart 2023-07-04 10:05:15 UTC Comment hidden (obsolete)
Comment 6 Pedro Amorim 2023-07-04 15:13:41 UTC
Created attachment 153019 [details] [review]
Bug 34107: Switch order for id and name feeding the first column in datatables

Test plan:
* Create 3 agreements, agreement #1 named 'a', agreement #2 named 'c' and agreement #3 named 'b'.
* Go to agreements list, click the Name column header, notice how the agreements get sorted by id #, not by first char in name. Expected order would be abc or cba, but it's acb or bca.
* Apply patch, on k-t-d, run the following if you're not using 'yarn js:watch':
yarn js:build
* Sort the list again on the 'name' column, notice how it now sorts alphabetically as expected, either abc or cba.
Comment 7 Pedro Amorim 2023-07-04 15:15:41 UTC
(In reply to Jonathan Druart from comment #4)
> Then you are removing the ability to filter by id on the column (where we
> show it).
> The correct fix I think is to put the id after the name:
> 
> -                    data: "me.agreement_id:me.name",
> +                    data: "me.name:me.agreement_id",

You're right, thank you!

I'm not sure we need the change on EHoldingsEBSCOPackagesList.vue though, it's not searchable or orderable.
Comment 8 Pedro Amorim 2023-07-04 15:27:06 UTC
Created attachment 153020 [details] [review]
Bug 34107: Switch order for id and name feeding the first column in datatables

Test plan:
* Create 3 agreements, agreement #1 named 'a', agreement #2 named 'c' and agreement #3 named 'b'.
* Go to agreements list, click the Name column header, notice how the agreements get sorted by id #, not by first char in name. Expected order would be abc or cba, but it's acb or bca.
* Apply patch, on k-t-d, run the following if you're not using 'yarn js:watch':
    yarn js:build
* Sort the list again on the 'name' column, notice how it now sorts alphabetically as expected, either abc or cba.
Comment 9 Pedro Amorim 2023-07-04 15:28:54 UTC
This last patch removes the change from EHoldingsEBSCOPackagesList.vue (not needed, that column is not searchable or sortable).
And also added the same behavior to EHoldingsLocalTitlesList.vue for consistency (was not possible to filter by id in titles list before, even though it shows, like other tables).
Comment 10 Jonathan Druart 2023-07-05 11:37:45 UTC
Created attachment 153052 [details] [review]
Bug 34107: Switch order for id and name feeding the first column in datatables

Test plan:
* Create 3 agreements, agreement #1 named 'a', agreement #2 named 'c' and agreement #3 named 'b'.
* Go to agreements list, click the Name column header, notice how the agreements get sorted by id #, not by first char in name. Expected order would be abc or cba, but it's acb or bca.
* Apply patch, on k-t-d, run the following if you're not using 'yarn js:watch':
    yarn js:build
* Sort the list again on the 'name' column, notice how it now sorts alphabetically as expected, either abc or cba.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Tomás Cohen Arazi (tcohen) 2023-07-06 15:25:19 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Martin Renvoize (ashimema) 2023-07-18 06:04:44 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 13 Matt Blenkinsop 2023-07-18 13:33:16 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x