Summary: | Fix missing array reference in provider rollup reports | ||
---|---|---|---|
Product: | Koha | Reporter: | Matt Blenkinsop <matt.blenkinsop> |
Component: | ERM | Assignee: | Matt Blenkinsop <matt.blenkinsop> |
Status: | RESOLVED FIXED | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | trivial | ||
Priority: | P5 - low | CC: | fridolin.somers, jonathan.druart, jonathan.field, lucas, m.de.rooy, martin.renvoize, matt.blenkinsop, pedro.amorim |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
24.05.00,23.11.06
|
|
Circulation function: | |||
Attachments: |
Bug 36093: Fix missing array reference
Bug 36093: Fix missing array reference Bug 36093: Add unit test and fix QA tests Bug 36093: (QA follow-up) Fix unit tests Bug 36093: Fix missing array reference Bug 36093: Add unit test and fix QA tests Bug 36093: (QA follow-up) Fix unit tests |
Description
Matt Blenkinsop
2024-02-14 12:28:25 UTC
Created attachment 162145 [details] [review] Bug 36093: Fix missing array reference This patch adds an array reference where it was previously missed. Test plan: Review patch diff and observe that the array reference is now correctly added Created attachment 162199 [details] [review] Bug 36093: Fix missing array reference This patch adds an array reference where it was previously missed. Test plan: Review patch diff and observe that the array reference is now correctly added Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Can you provide a test to cover this change please? Created attachment 162655 [details] [review] Bug 36093: Add unit test and fix QA tests prove t/db_dependent/api/v1/erm_custom_reports.t 1. You are not supposed to tidy the whole pm to make the QA tests pass 2. Why do you set date_uploaded in the tests, that's not needed. 3. + Koha::ERM::EUsage::MonthlyUsages->search->delete; You should not need to remove data before tests. You can count how many are there before the tests then do a sum. (In reply to Jonathan Druart from comment #5) > 1. You are not supposed to tidy the whole pm to make the QA tests pass Don't consider it a blocker, but keep in mind for next time. If you want to tidy the whole ERM namespace please do it on a separate bug. Completely missed this QA review! (In reply to Jonathan Druart from comment #5) > 1. You are not supposed to tidy the whole pm to make the QA tests pass Noted, will bear in mind for future patches > 2. Why do you set date_uploaded in the tests, that's not needed. This can be removed > 3. + Koha::ERM::EUsage::MonthlyUsages->search->delete; > You should not need to remove data before tests. You can count how many are > there before the tests then do a sum. I think probably when I wrote this test I had some data in the system already that was corrupting the test figures? I'll remove these lines and re-test Created attachment 165156 [details] [review] Bug 36093: (QA follow-up) Fix unit tests This patch removes some unnecessary lines from the test file Created attachment 165159 [details] [review] Bug 36093: Fix missing array reference This patch adds an array reference where it was previously missed. Test plan: Review patch diff and observe that the array reference is now correctly added Signed-off-by: Jonathan Field <jonathan.field@ptfs-europe.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 165160 [details] [review] Bug 36093: Add unit test and fix QA tests prove t/db_dependent/api/v1/erm_custom_reports.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 165161 [details] [review] Bug 36093: (QA follow-up) Fix unit tests This patch removes some unnecessary lines from the test file Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Pushed for 24.05! Well done everyone, thank you! Pushed to 23.11.x for 23.11.06 Doesn't apply cleanly in 23.05.x, no backport. |