Bug 32807

Summary: No need to fetch all if we need to know if one exist
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: ERMAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P5 - low CC: dcook, jonathan.druart, jonathan.field, lucas, martin.renvoize, matt.blenkinsop, pedro.amorim, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32468
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32808
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.05
Bug Depends on: 32030, 32991    
Bug Blocks:    
Attachments: Bug 32807: Fetch count instead of all entries for agreements and licenses
Bug 32807: Fetch count instead of all entries for agreements and licenses
Bug 32807: Fetch count instead of all entries for agreements and licenses
Bug 32807: Fix cypress tests
Bug 32807: Fix cypress tests

Description Jonathan Druart 2023-02-02 20:42:10 UTC
In (at least) AgreementsList we are calling fetchAgreements which fetch all the agreements, to know if we display the table (that will fetch again the first X agreements).
We need a fetchCountAgreements (similar to fetchCountLocalPackages) instead.

I think we discussed that already somewhere else.

Anyone willing to provide a patch to make their grip on Vue and the ERM module?
Comment 1 David Cook 2023-02-02 22:43:21 UTC
(In reply to Jonathan Druart from comment #0)
> Anyone willing to provide a patch to make their grip on Vue and the ERM
> module?

I don't really know anything about the ERM, but one of these days I do plan to get to grips with Vue. Probably not this patch though 😅
Comment 2 Jonathan Druart 2023-02-22 07:51:53 UTC
Created attachment 147111 [details] [review]
Bug 32807: Fetch count instead of all entries for agreements and licenses

To list agreements and licenses we are retrieving the whole list to
simply know if at least one exists (and display the table that will
fetch the X first elements to display).

We should call count instead.

Test plan:
List agreements and licenses.
If none exists, the table is not displayed but a "There are no...
defined" message instead.
If at least one exists the table must be there
Comment 3 Matt Blenkinsop 2023-02-22 10:00:38 UTC
Created attachment 147115 [details] [review]
Bug 32807: Fetch count instead of all entries for agreements and licenses

To list agreements and licenses we are retrieving the whole list to
simply know if at least one exists (and display the table that will
fetch the X first elements to display).

We should call count instead.

Test plan:
List agreements and licenses.
If none exists, the table is not displayed but a "There are no...
defined" message instead.
If at least one exists the table must be there

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 4 Kyle M Hall 2023-03-24 11:04:29 UTC
Created attachment 148654 [details] [review]
Bug 32807: Fetch count instead of all entries for agreements and licenses

To list agreements and licenses we are retrieving the whole list to
simply know if at least one exists (and display the table that will
fetch the X first elements to display).

We should call count instead.

Test plan:
List agreements and licenses.
If none exists, the table is not displayed but a "There are no...
defined" message instead.
If at least one exists the table must be there

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 5 Tomás Cohen Arazi 2023-03-27 10:47:33 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 6 Jonathan Druart 2023-03-29 07:40:20 UTC
Created attachment 148851 [details] [review]
Bug 32807: Fix cypress tests

Cypress tests were failing, not sure I understand why exactly as I don't
recreate the failure when using the interface. We could simplify the if
logic anyway.
Comment 7 Pedro Amorim 2023-03-29 09:28:35 UTC
Created attachment 148894 [details] [review]
Bug 32807: Fix cypress tests

Cypress tests were failing, not sure I understand why exactly as I don't
recreate the failure when using the interface. We could simplify the if
logic anyway.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 8 Tomás Cohen Arazi 2023-03-29 10:33:19 UTC
Follow-up pushed to master. Thanks for the quick fix, guys!
Comment 9 Matt Blenkinsop 2023-04-11 10:55:08 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 10 Lucas Gass 2023-04-24 17:06:43 UTC
Missing dependencies for 22.05.x, no backport
Comment 11 Jonathan Druart 2023-10-03 06:56:06 UTC
*** Bug 32494 has been marked as a duplicate of this bug. ***