If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical.
Created attachment 137333 [details] [review] Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical. Test Plan: 1) Delete all your rules 2) Create an all/all/all rules with an empty string for renewal base period 3) Note that renewing an item does nothing 4) Apply this patch 5) Restart all the things! 6) Renew again 7) Note the renewal uses the issuelength rule as intended
Created attachment 137526 [details] [review] Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical. Test Plan: 1) Delete all your rules 2) Create an all/all/all rules with an empty string for renewal base period 3) Note that renewing an item does nothing 4) Apply this patch 5) Restart all the things! 6) Renew again 7) Note the renewal uses the issuelength rule as intended Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Works as described, thanks Kyle!
Created attachment 137697 [details] [review] Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical. Test Plan: 1) Delete all your rules 2) Create an all/all/all rules with an empty string for renewal base period 3) Note that renewing an item does nothing 4) Apply this patch 5) Restart all the things! 6) Renew again 7) Note the renewal uses the issuelength rule as intended Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Any chance of a unit test for this Kyle?
Created attachment 137708 [details] [review] Bug 31120: Add unit tests
Created attachment 137709 [details] [review] Bug 31120: Add unit tests
Created attachment 137769 [details] [review] Bug 31120: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Cheers Kyle, Passing now
Pushed to master for 22.11. Nice work everyone, thanks!
Created attachment 138177 [details] [review] Bug 31120: Add unit tests Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 138178 [details] [review] Bug 31120: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value If the rule renewalperiod is the blank empty string instead of being null/undefined or non-existant, Koha will interpret the renewal period as being zero days instead of falling back to the issuelength rule. It makes sense to me that a literal 0 here should make it renew for zero days even though that is nonsensical. Test Plan: 1) Delete all your rules 2) Create an all/all/all rules with an empty string for renewal base period 3) Note that renewing an item does nothing 4) Apply this patch 5) Restart all the things! 6) Renew again 7) Note the renewal uses the issuelength rule as intended Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Backported to 22.05.x for 22.05.05
Thanks! Pushed to 21.11 for 21.11.12
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed. Nothing to document it seems, marking resolved.