| Summary: | getAll not allowing additional parameters | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
| Component: | ERM | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | RESOLVED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, jonathan.field, martin.renvoize, matt.blenkinsop, pedro.amorim, victor |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.04,22.11.10
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 36392, 32474 | ||
| Attachments: |
Bug 34219: Allow getAll to receive additional URL parameters
Bug 34219: Allow getAll to receive additional URL parameters Bug 34219: Allow getAll to receive additional URL parameters |
||
|
Description
Jonathan Druart
2023-07-06 14:09:31 UTC
Created attachment 153097 [details] [review] Bug 34219: Allow getAll to receive additional URL parameters It's not possible to pass additional URL parameters. We need it at least for bug 32474. Pedro, this is one for you ;) getAll allows for additional parameters, e.g. query (see AgreementRelationships.vue), problem is it doesn't allow for additional URL parameters! We have to: - Add URLparams as a parameter to getAll - Have a clear distinction between query parameters and URL parameters in http-client.js (and maybe also somewhere else) - Rename getAll to search - Handle default _per_page value - Likely rename this bug I hope to revisit this soon. (In reply to Pedro Amorim from comment #3) > - Add URLparams as a parameter to getAll Do you mean we should rename params.params to params.URLparams? > - Have a clear distinction between query parameters and URL parameters in > http-client.js (and maybe also somewhere else) 2 separate parameters, not params.URLparams and params.query, is that what you mean? > - Rename getAll to search On a separate bug please. get => find getAll => search > - Handle default _per_page value This is done already. > - Likely rename this bug It depends if we let it sit down or move on fast. We need it quite quickly in master I think, we will certainly need it soon (we need already on bug 32474). Created attachment 153631 [details] [review] Bug 34219: Allow getAll to receive additional URL parameters It's not possible to pass additional URL parameters. We need it at least for bug 32474. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Please add a test plan to the commit message. (In reply to Katrin Fischer from comment #6) > Please add a test plan to the commit message. I believe this bug only serves as preparation to 32474, it doesn't implement anything, hence not having a test plan (sorry!). A couple things that can be done though: 1) Run cypress tests - make sure all pass 2) Verify for yourself that everywhere getAll is used, is still working fine, those are: -- Agreements form - Related licenses -- Agreements form - Related agreements -- Local packages form - Related agreements -- Local titles form - Related packages -- Titles - Import from list I should probably not admit that, but how do I run the Cypress tests? (In reply to Katrin Fischer from comment #8) > I should probably not admit that, but how do I run the Cypress tests? The easiest is: % perl /kohadevbox/misc4dev/run_tests.pl --run-cypress-tests-only Confirmed that tests pass after patch :) To run one test (in case of flaky tests): yarn cypress run --spec t/cypress/integration/ERM/Agreements_spec.ts Created attachment 154517 [details] [review] Bug 34219: Allow getAll to receive additional URL parameters It's not possible to pass additional URL parameters. We need it at least for bug 32474. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Code makes sense and all works as expected here.. unit tests are passing still and qa script is happy. Passing QA Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.04 Nice work everyone! Pushed to oldstable for 22.11.x |