Bug 36392

Summary: Only 20 vendors in ERM dropdown
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: ERMAssignee: Jonathan Druart <jonathan.druart>
Status: Pushed to oldstable --- QA Contact:
Severity: normal    
Priority: P5 - low CC: david, fridolin.somers, jonathan.druart, jonathan.field, lucas, matt.blenkinsop, matthias.le-gac, pedro.amorim, philippe.blouin
Version: MainKeywords: regression
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36956
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the listing of vendors when adding a new agreement in the electronic resources (ERM) module. Previously only the first 20 vendors were displayed, now all vendors are displayed.
Version(s) released in:
24.05.00,23.11.06,23.05.12
Circulation function:
Bug Depends on: 32468, 34219    
Bug Blocks:    
Attachments: Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM

Description Caroline Cyr La Rose 2024-03-21 19:55:49 UTC
When there are more than 20 vendors in acquisitions, only 20 appear in the New agreement dropdown list.

To recreate:
1. Enable the ERMModule system preference
2. In Acquisitions, create more than 20 vendors (see below for an sql insert that I used)
3. Go to E-resource management > Agreements > New agreement
4. Open the vendor dropdown
   --> It stops at Sierra (or the 20th vendor if you created your own)
5. Type "Tango" or "Zulu" (or one of the names of your 21st+ vendor)
   --> No results

This is what I used to create all the vendors

INSERT INTO aqbooksellers (name,active) VALUES ('Alpha','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Bravo','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Charlie','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Delta','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Echo','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Foxtrot','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Golf','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Hotel','1');
INSERT INTO aqbooksellers (name,active) VALUES ('India','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Juliet','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Kilo','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Lima','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Mike','1');
INSERT INTO aqbooksellers (name,active) VALUES ('November','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Oscar','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Papa','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Quebec','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Romeo','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Sierra','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Tango','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Uniform','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Victor','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Whiskey','1');
INSERT INTO aqbooksellers (name,active) VALUES ('X-Ray','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Yankee','1');
INSERT INTO aqbooksellers (name,active) VALUES ('Zulu','1');

Paul D. pointed me to the RESTdefaultPageSize system preference which does affect the dropdown. If I change the syspref to 50, I can see all the vendors in the dropdown. The problem with this is that I'm not sure what else this syspref affects. I have a client with 674 vendors, I don't want to put 1000 in the syspref and then somewhere else something takes forever to load because of this.

I'm not sure what is happening here. I found bug 32474 and bug 32468 that seem to deal with this and that are marked as fixed, but I don't see the correct behaviour in ktd with master. I hope someone else can confirm or invalidate this...

The client who pointed this out to me (the one with 674 vendors) is on 23.05.05, which already has the patches from bug 32468.
Comment 1 Caroline Cyr La Rose 2024-03-21 20:54:56 UTC
I bisected and found that it was due to bug 34219 which was added to 23.05.04 which makes sense because my client's 23.05.05 has the wrong behaviour.
Comment 2 Jonathan Druart 2024-03-22 07:43:45 UTC
Created attachment 163660 [details] [review]
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM

In commit f1078daf8f496e1b4234acf766578c511ca8aad1
   Bug 34219: Allow getAll to receive additional URL parameters

We needed to call getAll instead of get to fetch all the vendors

Test plan:
Have more than 20 vendors and create/edit an agreement.
Notice that the vendor list contains all the vendors
Comment 3 David Nind 2024-03-22 10:03:37 UTC
Created attachment 163671 [details] [review]
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM

In commit f1078daf8f496e1b4234acf766578c511ca8aad1
   Bug 34219: Allow getAll to receive additional URL parameters

We needed to call getAll instead of get to fetch all the vendors

Test plan:
Have more than 20 vendors and create/edit an agreement.
Notice that the vendor list contains all the vendors

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 Matthias Le Gac 2024-03-22 18:50:55 UTC
I applied the patch to master, checking that I was up to date. I checked that the code had changed. Then I created more than 20 suppliers and when I go to E-resource management > Agreements > New agreement and click on the vendor dropdown. I still only get 20 results. If I change the RESTdefaultPageSize preference as Caroline says, then I can see all the vendors.

Did I miss a step because I see that David signed Off?
Comment 5 David Nind 2024-03-22 20:13:11 UTC
Hi Matthias.

A 'yarn build' is required after applying the patch for view-related changes (and I normally do a flush_memcached and restart_all) (this was using koha-testing-docker).

David
Comment 6 Matthias Le Gac 2024-03-22 20:21:44 UTC
Created attachment 163741 [details] [review]
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM

In commit f1078daf8f496e1b4234acf766578c511ca8aad1
   Bug 34219: Allow getAll to receive additional URL parameters

We needed to call getAll instead of get to fetch all the vendors

Test plan:
Have more than 20 vendors and create/edit an agreement.
Notice that the vendor list contains all the vendors

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Comment 7 Matthias Le Gac 2024-03-22 20:22:44 UTC
Thanks after I yarn build and reload apache2 all works good.
Comment 8 Pedro Amorim 2024-03-25 10:08:54 UTC
Created attachment 163775 [details] [review]
Bug 36392: (bug 34219 follow-up) Fix getAll for vendors - ERM

In commit f1078daf8f496e1b4234acf766578c511ca8aad1
   Bug 34219: Allow getAll to receive additional URL parameters

We needed to call getAll instead of get to fetch all the vendors

Test plan:
Have more than 20 vendors and create/edit an agreement.
Notice that the vendor list contains all the vendors

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Matthias Le Gac <matthias.le-gac@inlibro.com>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 9 Katrin Fischer 2024-03-25 16:34:48 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 10 Fridolin Somers 2024-05-23 08:41:04 UTC
Pushed to 23.11.x for 23.11.06
Comment 11 Lucas Gass (lukeg) 2024-05-28 16:14:58 UTC
Backported to 23.05.x for upcoming 23.05.12