| Summary: | C4::Reserves::AddReserves should take a hashref in parameters | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
| Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
| Status: | CLOSED FIXED | QA Contact: | Martin Renvoize (ashimema) <martin.renvoize> |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | 1joynelson, david, gmcharlt, jonathan.druart, joonas.kylmala, joonas.kylmala, martin.renvoize |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14702 | ||
| Change sponsored?: | --- | Patch complexity: | Medium patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
20.05.00
|
|
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 24769 | ||
| Attachments: |
Bug 14711: We do no longer need bibitems in AddReserve
Bug 14711: Change prototype for AddReserve - pass a hashref Bug 14711: Change prototype for AddReserve - pass a hashref Bug 14711: Change prototype for AddReserve - pass a hashref |
||
|
Description
Marcel de Rooy
2015-08-24 13:15:32 UTC
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 |