An array has not been correctly referenced when passed into a function when creating a provider rollup report
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.