| Summary: | Add useful methods to Koha::Acquisition::Orders | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Component: | Architecture, internals, and plumbing | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED FIXED | QA Contact: | Katrin Fischer <katrin.fischer> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | caroline.cyr-la-rose, fridolin.somers, jonathan.druart, kyle, martin.renvoize, nick, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| Change sponsored?: | --- | Patch complexity: | Small patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This architectural enhancement adds helpful filter methods to the Koha::Acquisitions:: modules to ease the development of Acquisitions API's and future acquisitions related enhancements.
|
Version(s) released in: |
21.05.00
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 30092, 20212, 28509 | ||
| Attachments: |
Bug 25670: Unit tests
Bug 25670: Add Koha::Acquisition::Orders->filter_by_id_including_transfers Bug 25670: Unit tests Bug 25670: Add filter_by_active and filter_by_id_including_transfers Bug 25670: Unit tests Bug 25670: Add filter_by_active and filter_by_id_including_transfers Bug 25670: Unit tests Bug 25670: Add filter_by_active and filter_by_id_including_transfers Bug 25670: Regression tests Bug 25670: New orders included only if standing Bug 25670: Fix random failure on Orders.t |
||
|
Description
Tomás Cohen Arazi (tcohen)
2020-06-03 19:54:37 UTC
Created attachment 105534 [details] [review] Bug 25670: Unit tests Created attachment 105535 [details] [review] Bug 25670: Add Koha::Acquisition::Orders->filter_by_id_including_transfers This patch introduces a method to filter on the ordernumber, looking for it on the aqorders_transfers table ('ordernumber_from' field). To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Orders.t => SUCCESS: Tests pass! 3. Sign off :-D Created attachment 109859 [details] [review] Bug 25670: Unit tests Created attachment 109860 [details] [review] Bug 25670: Add filter_by_active and filter_by_id_including_transfers This patch introduces a method to filter on the ordernumber, looking for it on the aqorders_transfers table ('ordernumber_from' field). It also adds a method for filtering on active orders. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Orders.t => SUCCESS: Tests pass! 3. Sign off :-D lib Created attachment 110643 [details] [review] Bug 25670: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 110644 [details] [review] Bug 25670: Add filter_by_active and filter_by_id_including_transfers This patch introduces a method to filter on the ordernumber, looking for it on the aqorders_transfers table ('ordernumber_from' field). It also adds a method for filtering on active orders. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Orders.t => SUCCESS: Tests pass! 3. Sign off :-D lib Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Code reads well, tests are good.. Signing off -=head2 Class Methods +=head2 Class methods Thank you! I wonder if $orders->filter_by_active is a good name. Maybe filter_by_pending ? Created attachment 110700 [details] [review] Bug 25670: Unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 110701 [details] [review] Bug 25670: Add filter_by_active and filter_by_id_including_transfers This patch introduces a method to filter on the ordernumber, looking for it on the aqorders_transfers table ('ordernumber_from' field). It also adds a method for filtering on active orders. To test: 1. Apply this patches 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Orders.t => SUCCESS: Tests pass! 3. Sign off :-D lib Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 114940 [details] [review] Bug 25670: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 114941 [details] [review] Bug 25670: New orders included only if standing The original implementation included new orders regardless they were standing orders. This patch makes the query respect that conditions found on C4::Acquisitions::SearchOrders. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Koha/Acquisition/Orders.t => FAIL: More orders than expected are returned 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! New orders are only considered if the basket they are attached to, is marked as standing! 5. Verify the tests make sense 6. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 21.05, thanks to everybody involved! Created attachment 116141 [details] [review] Bug 25670: Fix random failure on Orders.t It has failed at least 3 times: # Failed test 'The right order is returned' # at t/db_dependent/Koha/Acquisition/Orders.t line 153. # got: '50' # expected: '49' # Failed test 'The right order is returned' # at t/db_dependent/Koha/Acquisition/Orders.t line 154. # got: '49' # expected: '50' # Looks like you failed 2 tests of 6. I cannot manage to make it fail locally Follow-up pushed to master. Enhancement not pushed to 20.11.x (In reply to Jonathan Druart from comment #14) > Created attachment 116141 [details] [review] [review] > Bug 25670: Fix random failure on Orders.t > > It has failed at least 3 times: > > # Failed test 'The right order is returned' > # at t/db_dependent/Koha/Acquisition/Orders.t line 153. > # got: '50' > # expected: '49' > > # Failed test 'The right order is returned' > # at t/db_dependent/Koha/Acquisition/Orders.t line 154. > # got: '49' > # expected: '50' > # Looks like you failed 2 tests of 6. > > I cannot manage to make it fail locally Again today Koha_Master_D9 #1564: 16:34:46 koha_1 | # Failed test 'The 2 orders are returned' 16:34:46 koha_1 | # at t/db_dependent/Koha/Acquisition/Orders.t line 152. 16:34:46 koha_1 | # Structures begin differing at: 16:34:46 koha_1 | # $got->[0] = '100' 16:34:46 koha_1 | # $expected->[0] = '99' 16:34:46 koha_1 | # Looks like you failed 1 test of 4. 16:34:46 koha_1 | 16:34:46 koha_1 | # Failed test 'filter_by_id_including_transfers() tests' 16:34:46 koha_1 | # at t/db_dependent/Koha/Acquisition/Orders.t line 160. Master_D10 (#484) 14:03:26 koha_1 | # Failed test 'The 2 orders are returned' 14:03:26 koha_1 | # at t/db_dependent/Koha/Acquisition/Orders.t line 152. 14:03:26 koha_1 | # Structures begin differing at: 14:03:26 koha_1 | # $got->[0] = '10' 14:03:26 koha_1 | # $expected->[0] = '9' 14:03:26 koha_1 | # Looks like you failed 1 test of 4. 14:03:26 koha_1 | 14:03:26 koha_1 | # Failed test 'filter_by_id_including_transfers() tests' 14:03:26 koha_1 | # at t/db_dependent/Koha/Acquisition/Orders.t line 160. Still additional_work_needed ? Looks like there has been : Bug 20212: Fix test after 25670 follow-up (In reply to Fridolin Somers from comment #19) > Still additional_work_needed ? > > Looks like there has been : > Bug 20212: Fix test after 25670 follow-up CommitDate: Thu Jan 28 16:44:12 2021 +0100 Last comment is from March. Tomas opened bug 30092 to track this random failure. |