If bug 9809 and bug 14702 make their way into Koha, time comes to remove the bibitems parameter from AddReserve. In that case it is no longer used (code for reserve fee calculation being replaced). Note that this parameter was not supplied in the same form consistently thru Koha. The old code could handle that; the new code does not need it.
Created attachment 42198 [details] [review] Bug 14711: We do no longer need bibitems in AddReserve Removes the unnecessary bibitems parameter from all the calls to AddReserve function and also from the function itself. Test plan: 1. Try some (if not all) of the pages' functionalities where the code was changed. 2. Make sure they work like before and there is no errors in the koha-error log. 3. If you didn't try all the pages make sure the code changes in the other pages are exactly like they were in the pages you tested. 4. Run all the test and look if they pass
Bug 9809 already updated all calls to AddReserve, to prevent a future change again it could worth to change the prototype of this subroutine to make it get a hashref.
(In reply to Jonathan Druart from comment #2) > Bug 9809 already updated all calls to AddReserve, to prevent a future change > again it could worth to change the prototype of this subroutine to make it > get a hashref. Attachment 42198 [details] is now obsolete, right? Shall we create a new bug report for the hashref change?
no, just submit another patch with the call updated.
I'm not working on this anymore, somebody else feel free to take this further.
Created attachment 77940 [details] [review] Bug 14711: Change prototype for AddReserve - pass a hashref The number of parameters of AddReserve makes it hard to read and maintain. This patch replace it with a hashref, which will make the calls more readable. Moreover the bibitems has been removed as it was not used by the subroutine. Test plan: - Make sure the tests pass - Read the diff and search for typos - Place a hold on few items Note for QA: reservation_date and expiration_date do not match the DB column's names, should we?
Path no longer applies 8-(
Created attachment 98708 [details] [review] Bug 14711: Change prototype for AddReserve - pass a hashref The number of parameters of AddReserve makes it hard to read and maintain. This patch replace it with a hashref, which will make the calls more readable. Moreover the bibitems has been removed as it was not used by the subroutine. Test plan: - Make sure the tests pass - Read the diff and search for typos - Place a hold on few items Note for QA: reservation_date and expiration_date do not match the DB column's names, should we?
Created attachment 98710 [details] [review] Bug 14711: Change prototype for AddReserve - pass a hashref The number of parameters of AddReserve makes it hard to read and maintain. This patch replace it with a hashref, which will make the calls more readable. Moreover the bibitems has been removed as it was not used by the subroutine. Test plan: - Make sure the tests pass - Read the diff and search for typos - Place a hold on few items Note for QA: reservation_date and expiration_date do not match the DB column's names, should we? Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Works as expected.. a look through the code doesn't highlight any issues regressions.. I'm tempted to PQA and push and let Jenkins do it's thing here rather than let it languish and require another rebase. As for the note.. I think we're safe with the signature.. the db column name mismatch isn't the first and should likely get cleaned up as a distinct issue upon more to Koha:: PQA
Nice work everyone! Pushed to master for 20.05
Enhancement not pushed to 19.11.x