Bug 34107 - Sorting agreements by Name actually sorts by ID
Summary: Sorting agreements by Name actually sorts by ID
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Pedro Amorim
QA Contact:
URL:
Keywords:
Depends on: 33973
Blocks:
  Show dependency treegraph
 
Reported: 2023-06-22 22:01 UTC by Katrin Fischer
Modified: 2023-12-28 20:47 UTC (History)
6 users (show)

See Also:
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


Attachments
Bug 34107: Remove id from name column in erm list tables (3.37 KB, patch)
2023-06-23 08:28 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34028: Show the secret when registering from the auth screen (999 bytes, patch)
2023-07-04 10:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 34107: Switch order for id and name feeding the first column in datatables (3.94 KB, patch)
2023-07-04 15:13 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34107: Switch order for id and name feeding the first column in datatables (3.94 KB, patch)
2023-07-04 15:27 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 34107: Switch order for id and name feeding the first column in datatables (4.02 KB, patch)
2023-07-05 11:37 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 2023-07-06 15:25:19 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Martin Renvoize 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