Bug 32468

Summary: Vendors select only allows selecting from first 20 vendors by default
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: ERMAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: major    
Priority: P5 - low CC: jonathan.druart, jonathan.field, lucas, martin.renvoize, pedro.amorim
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32807
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.01
Bug Depends on: 32030    
Bug Blocks: 32474, 36392    
Attachments: Bug 32468: ERM - Populate dropdown list with all entries
Bug 32468: Use fetchLocalTitleCount to know if local titles exist
Bug 32468: Remove unecessary fetch titles JS functions
Bug 32468: ERM - Populate dropdown list with all entries
Bug 32468: Use fetchLocalTitleCount to know if local titles exist
Bug 32468: Remove unecessary fetch titles JS functions
Bug 32468: ERM - Populate dropdown list with all entries
Bug 32468: Use fetchLocalTitleCount to know if local titles exist
Bug 32468: Remove unecessary fetch titles JS functions

Description Martin Renvoize 2022-12-14 16:22:25 UTC
It looks like we forgot about API paging when implementing the vendors select box on ERM.. as such, by default, we can only access the first 20 vendors created on the system.

Is there perhaps a Select2 style vue component we could lean on for this.. both to improve consistency and to resolve this paging issue too.?
Comment 1 Jonathan Druart 2022-12-14 16:32:25 UTC
We need that: https://select2.org/data-sources/ajax#pagination
Comment 2 Martin Renvoize 2022-12-14 19:53:57 UTC
Are we using vue-select?  I should look at the code, that component appears inspired by select2, with many similar options.
Comment 3 Jonathan Druart 2022-12-15 07:31:52 UTC
Created attachment 144601 [details] [review]
Bug 32468: ERM - Populate dropdown list with all entries

The REST API routes is configured to return the 20 first results if
_per_page is not provided. Here we want to display all the
vendors/agreements/licenses/packages in the dropdown lists.

Ideally we should implement an infinite scroll to not load all the data
at once (https://vue-select.org/guide/infinite-scroll.html)

Test plan:
Create more than 20 vendors and confirm that, with this patch, all the
vendors are display in the dropdown list displayed on the add agreement
form
Same for agreements, licenses and local packages
Comment 4 Jonathan Druart 2022-12-15 07:31:57 UTC
Created attachment 144602 [details] [review]
Bug 32468: Use fetchLocalTitleCount to know if local titles exist

We don't need to fetch 20 titles to know if there is at least one. We
should call fetchLocalTitleCount.

Note that this should be implemented for all 'List' views.
Comment 5 Jonathan Druart 2022-12-15 07:32:01 UTC
Created attachment 144603 [details] [review]
Bug 32468: Remove unecessary fetch titles JS functions

Those JS functions are no longer in use and can be removed.
Comment 6 Lucas Gass 2022-12-15 15:38:30 UTC
Created attachment 144620 [details] [review]
Bug 32468: ERM - Populate dropdown list with all entries

The REST API routes is configured to return the 20 first results if
_per_page is not provided. Here we want to display all the
vendors/agreements/licenses/packages in the dropdown lists.

Ideally we should implement an infinite scroll to not load all the data
at once (https://vue-select.org/guide/infinite-scroll.html)

Test plan:
Create more than 20 vendors and confirm that, with this patch, all the
vendors are display in the dropdown list displayed on the add agreement
form
Same for agreements, licenses and local packages

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 7 Lucas Gass 2022-12-15 15:38:34 UTC
Created attachment 144621 [details] [review]
Bug 32468: Use fetchLocalTitleCount to know if local titles exist

We don't need to fetch 20 titles to know if there is at least one. We
should call fetchLocalTitleCount.

Note that this should be implemented for all 'List' views.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 8 Lucas Gass 2022-12-15 15:38:39 UTC
Created attachment 144622 [details] [review]
Bug 32468: Remove unecessary fetch titles JS functions

Those JS functions are no longer in use and can be removed.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 9 Lucas Gass 2022-12-15 15:43:49 UTC
Just a couple notes to myself or anyone else testing ERM/vue stuff:

-Apply patch
-run yarn build_js
-clear browser cache
-test patch 

Like with SCSS toss out the compiled changes ( koha-tmpl/intranet-tmpl/prog/js/vue/dist/main.js ) and leave that to the RM/RMaints
Comment 10 Martin Renvoize 2022-12-19 14:42:36 UTC
Created attachment 144714 [details] [review]
Bug 32468: ERM - Populate dropdown list with all entries

The REST API routes is configured to return the 20 first results if
_per_page is not provided. Here we want to display all the
vendors/agreements/licenses/packages in the dropdown lists.

Ideally we should implement an infinite scroll to not load all the data
at once (https://vue-select.org/guide/infinite-scroll.html)

Test plan:
Create more than 20 vendors and confirm that, with this patch, all the
vendors are display in the dropdown list displayed on the add agreement
form
Same for agreements, licenses and local packages

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2022-12-19 14:42:41 UTC
Created attachment 144715 [details] [review]
Bug 32468: Use fetchLocalTitleCount to know if local titles exist

We don't need to fetch 20 titles to know if there is at least one. We
should call fetchLocalTitleCount.

Note that this should be implemented for all 'List' views.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2022-12-19 14:42:45 UTC
Created attachment 144716 [details] [review]
Bug 32468: Remove unecessary fetch titles JS functions

Those JS functions are no longer in use and can be removed.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2022-12-19 14:43:24 UTC
Thanks for the quick fix Jonathan and the quick testing Lucas.

All looks good and works for me.. Passing QA
Comment 14 Tomás Cohen Arazi 2022-12-19 14:48:08 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 15 Martin Renvoize 2022-12-19 17:06:42 UTC
Nice work everyone!

Pushed to 22.11.x for the next release