Some libraries may have hundreds of open orders for a vendor, depending on acqisitions workflow. At that number of open orders, the tables on parcel.pl take 10 to 20 seconds to load. To speed up the loading of this page and the data, we should convert the order tables from pre-loading all order data to ajax tables that load order data on an as needed basis. That way we are only pulling the data for the orders that are currently visible.
Was a bit confused by the bug title - hope I made it a bit more clear.
Created attachment 69184 [details] [review] Bug 18731: WIP /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. It uses the new pagination and query parsing plugins. Sponsored-by: Camden County
Created attachment 69185 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It adds query parameters definitions for the reserved params: - _match - _order_by - _page - _per_page
Created attachment 69309 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. It uses the new pagination, query parsing and object searching plugins. Sponsored-by: Camden County
Created attachment 69310 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It adds query parameters definitions for the reserved params: - _match - _order_by - _page - _per_page
Created attachment 69544 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint.
I'm writting a new RFC spec to be voted on a proper dev meeting. I will re-do this work based on that vote.
Created attachment 73959 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. It uses the new pagination, query parsing and object searching plugins. The attribute names are consistent with the voted RFC Sponsored-by: Camden County
Created attachment 73960 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It adds query parameters definitions for the reserved params: - _match - _order_by - _page - _per_page
Created attachment 78435 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. It uses the new pagination, query parsing and object searching plugins. The attribute names are consistent with the voted RFC Sponsored-by: Camden County
Created attachment 78436 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It adds query parameters definitions for the reserved params: - _match - _order_by - _page - _per_page
Created attachment 84133 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. It uses the new pagination, query parsing and object searching plugins. The attribute names are consistent with the voted RFC Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 84134 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It adds query parameters definitions for the reserved params: - _match - _order_by - _page - _per_page Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
As far as I'm concerned, it works as expected.
started testing: 1) we should generalize API controller to not repeat almost same code againg, this is nice prototype, but out of the scope of this report 2) basket_id description: "Basket this order is linked to. If ommited, a new basket is automatically created" It is not true, basket is not created 3) Tests are missing 4) Where mandatory parameter in body is missing on creation of object, api reports 500 "Something went wrong, check the logs". But the api client has no chance to know what is wrong here. This is probably true on more places in api, but i not tested it all now. anyway good start, thanks Tomas for working on that!
And last thing for now, qa tools are complaining: FAIL Koha/REST/V1/Acquisitions/Orders.pm FAIL pod =head3 without preceding higher level in file Koha/REST/V1/Acquisitions/Orders.pm =head2 without preceding higher level *** WARNING: FAIL pod coverage POD is missing for add POD is missing for delete POD is missing for get POD is missing for list POD is missing for update
Created attachment 95054 [details] [review] Bug 18731: Unit tests This patch implements unit tests for the orders endpoint. Sponsored-by: Camden County
Created attachment 95055 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It matches the voted RFC. Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 95056 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. The attribute names are consistent with the voted RFC To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => SUCCESS: Tests pass! 3. Test the API with your favourite tool (Postman?) => SUCCESS: It works as expected! 4 Sign off :-D Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 96853 [details] [review] Bug 18731: Make authenticate_api_request stash the embed structure
Created attachment 96854 [details] [review] Bug 18731: Unit tests This patch implements unit tests for the orders endpoint. Sponsored-by: Camden County
Created attachment 96855 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It matches the voted RFC. Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 96856 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. The attribute names are consistent with the voted RFC To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => SUCCESS: Tests pass! 3. Test the API with your favourite tool (Postman?) => SUCCESS: It works as expected! 4 Sign off :-D Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 96857 [details] [review] Bug 18731: Add API mappings to K::A::{Basket,Invoice} This patch adds to_api_mapping definitions to the following classes: - Koha::Acquisition::Basket - Koha::Acquisition::Invoice They are implemented following the proposed RFCs: https://wiki.koha-community.org/wiki/Acquisitions_baskets_endpoint_RFC https://wiki.koha-community.org/wiki/Acquisitions_invoices_endpoint_RFC
Created attachment 96858 [details] [review] Bug 18731: Use the stashed koha.embed in objects.search This patch makes the objects.search helper use the koha.embed structure that is embedded in the authenticate_api_request step. This way, any controller using it will benefit from automatic embed handling. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D
I've set this back to NSO as I introduced some important changes.
Created attachment 96916 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. The attribute names are consistent with the voted RFC To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => SUCCESS: Tests pass! 3. Test the API with your favourite tool (Postman?) => SUCCESS: It works as expected! 4 Sign off :-D Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 96917 [details] [review] Bug 18731: Add API mappings to K::A::{Basket,Invoice} This patch adds to_api_mapping definitions to the following classes: - Koha::Acquisition::Basket - Koha::Acquisition::Invoice They are implemented following the proposed RFCs: https://wiki.koha-community.org/wiki/Acquisitions_baskets_endpoint_RFC https://wiki.koha-community.org/wiki/Acquisitions_invoices_endpoint_RFC
Created attachment 96918 [details] [review] Bug 18731: Use the stashed koha.embed in objects.search This patch makes the objects.search helper use the koha.embed structure that is embedded in the authenticate_api_request step. This way, any controller using it will benefit from automatic embed handling. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D
Created attachment 96938 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It matches the voted RFC. Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Created attachment 96961 [details] [review] Bug 18731: Make authenticate_api_request stash the embed structure Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 96962 [details] [review] Bug 18731: Unit tests This patch implements unit tests for the orders endpoint. Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 96963 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. The attribute names are consistent with the voted RFC To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => SUCCESS: Tests pass! 3. Test the API with your favourite tool (Postman?) => SUCCESS: It works as expected! 4 Sign off :-D Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 96964 [details] [review] Bug 18731: Add API mappings to K::A::{Basket,Invoice} This patch adds to_api_mapping definitions to the following classes: - Koha::Acquisition::Basket - Koha::Acquisition::Invoice They are implemented following the proposed RFCs: https://wiki.koha-community.org/wiki/Acquisitions_baskets_endpoint_RFC https://wiki.koha-community.org/wiki/Acquisitions_invoices_endpoint_RFC Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 96965 [details] [review] Bug 18731: Use the stashed koha.embed in objects.search This patch makes the objects.search helper use the koha.embed structure that is embedded in the authenticate_api_request step. This way, any controller using it will benefit from automatic embed handling. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 96966 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It matches the voted RFC. Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is an impressive culmination of work. There's little chance of introducing regressions here and in my testing everything appears to work as expected. Passing QA and Pushing.. lets see if anything drops out of it when we start to use the endpoints in earnest :) Well done everyone!
Created attachment 97107 [details] [review] Bug 18731: Make authenticate_api_request stash the embed structure Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97108 [details] [review] Bug 18731: Unit tests This patch implements unit tests for the orders endpoint. Sponsored-by: Camden County Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97109 [details] [review] Bug 18731: /acquisitions/orders endpoint This patches implement the /acquisitions/orders endpoint. The attribute names are consistent with the voted RFC To test: 1. Apply this patchset 2. Run: $ kshell k$ prove t/db_dependent/api/v1/acquisitions_orders.t => SUCCESS: Tests pass! 3. Test the API with your favourite tool (Postman?) => SUCCESS: It works as expected! 4 Sign off :-D Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97110 [details] [review] Bug 18731: Add API mappings to K::A::{Basket,Invoice} This patch adds to_api_mapping definitions to the following classes: - Koha::Acquisition::Basket - Koha::Acquisition::Invoice They are implemented following the proposed RFCs: https://wiki.koha-community.org/wiki/Acquisitions_baskets_endpoint_RFC https://wiki.koha-community.org/wiki/Acquisitions_invoices_endpoint_RFC Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97111 [details] [review] Bug 18731: Use the stashed koha.embed in objects.search This patch makes the objects.search helper use the koha.embed structure that is embedded in the authenticate_api_request step. This way, any controller using it will benefit from automatic embed handling. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97112 [details] [review] Bug 18731: OpenAPI definitions This patch introduces the OpenAPI definitions required for the orders endpoint. It matches the voted RFC. Sponsored-by: Camden County Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 97113 [details] [review] Bug 18731: (QA follow-up) Spelling and POD corrections Just a coulpe of very minor tweaks to keep the QA scripts happy Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice work everyone! Pushed to master for 20.05
This is really great to see! Is it able to be backported to 19.05?
The new test is failing (randomly?) t/db_dependent/Koha/REST/Plugin/Objects.t .. 1/3 # Failed test 'exact match for JSON Pointer "/0"' # at t/db_dependent/Koha/REST/Plugin/Objects.t line 174. # Structures begin differing at: # $got->{fund}{budget_id} = '534289848' # $expected->{fund}{budget_id} = Does not exist Koha_Master_D9_My8 run 76 It's not related to My8, I recreate locally with MariaDB latest.
(In reply to Jonathan Druart from comment #47) > The new test is failing (randomly?) > > t/db_dependent/Koha/REST/Plugin/Objects.t .. 1/3 > # Failed test 'exact match for JSON Pointer "/0"' > # at t/db_dependent/Koha/REST/Plugin/Objects.t line 174. > # Structures begin differing at: > # $got->{fund}{budget_id} = '534289848' > # $expected->{fund}{budget_id} = Does not exist > > Koha_Master_D9_My8 run 76 > > It's not related to My8, I recreate locally with MariaDB latest. I really like it when tests serve the purpose :-D
Created attachment 97331 [details] [review] Bug 18731: Overload K::A::Fund->to_api to avoid conflict This patch overloads the to_api methods on the Fund class, so conflicting (on mapping) attribute names are not a problem. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Fund.t \ t/db_dependent/Koha/REST/Plugin/Objects.t => SUCCESS: Tests pass! 3. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Followup pushed, thanks Tomas
Enhancement not backported to 19.11.x