Bug 32468 - Vendors select only allows selecting from first 20 vendors by default
Summary: Vendors select only allows selecting from first 20 vendors by default
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 32030
Blocks: 32474 36392
  Show dependency treegraph
 
Reported: 2022-12-14 16:22 UTC by Martin Renvoize
Modified: 2024-03-21 20:06 UTC (History)
5 users (show)

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


Attachments
Bug 32468: ERM - Populate dropdown list with all entries (2.39 KB, patch)
2022-12-15 07:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32468: Use fetchLocalTitleCount to know if local titles exist (6.85 KB, patch)
2022-12-15 07:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32468: Remove unecessary fetch titles JS functions (1.47 KB, patch)
2022-12-15 07:32 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32468: ERM - Populate dropdown list with all entries (2.45 KB, patch)
2022-12-15 15:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32468: Use fetchLocalTitleCount to know if local titles exist (6.91 KB, patch)
2022-12-15 15:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32468: Remove unecessary fetch titles JS functions (1.53 KB, patch)
2022-12-15 15:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32468: ERM - Populate dropdown list with all entries (2.51 KB, patch)
2022-12-19 14:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32468: Use fetchLocalTitleCount to know if local titles exist (6.97 KB, patch)
2022-12-19 14:42 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32468: Remove unecessary fetch titles JS functions (1.59 KB, patch)
2022-12-19 14:42 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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