Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value
Summary: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: 21.05
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-07-07 19:14 UTC by Kyle M Hall
Modified: 2023-06-08 22:26 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00,21.11, 22.05.05


Attachments
Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value (1.47 KB, patch)
2022-07-07 19:16 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value (1.52 KB, patch)
2022-07-11 15:51 UTC, Biblibre Sandboxes
Details | Diff | Splinter Review
Bug 31120 - Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value (1.58 KB, patch)
2022-07-14 09:22 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31120: Add unit tests (2.09 KB, patch)
2022-07-14 10:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31120: Add unit tests (2.10 KB, patch)
2022-07-14 10:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 31120: Add unit tests (2.16 KB, patch)
2022-07-15 15:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 31120: Add unit tests (2.22 KB, patch)
2022-07-27 13:08 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 31120: Items will renew for zero ( 0 ) days if renewalperiod is blank/empty value (1.63 KB, patch)
2022-07-27 13:09 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2022-07-07 19:14:43 UTC
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.
Comment 1 Kyle M Hall 2022-07-07 19:16:53 UTC
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
Comment 2 Biblibre Sandboxes 2022-07-11 15:51:49 UTC
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>
Comment 3 Sally 2022-07-11 15:52:15 UTC
Works as described, thanks Kyle!
Comment 4 Martin Renvoize 2022-07-14 09:22:38 UTC
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>
Comment 5 Martin Renvoize 2022-07-14 09:23:00 UTC
Any chance of a unit test for this Kyle?
Comment 6 Kyle M Hall 2022-07-14 10:39:18 UTC
Created attachment 137708 [details] [review]
Bug 31120: Add unit tests
Comment 7 Kyle M Hall 2022-07-14 10:42:35 UTC
Created attachment 137709 [details] [review]
Bug 31120: Add unit tests
Comment 8 Martin Renvoize 2022-07-15 15:02:27 UTC
Created attachment 137769 [details] [review]
Bug 31120: Add unit tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2022-07-15 15:03:02 UTC
Cheers Kyle, Passing now
Comment 10 Tomás Cohen Arazi 2022-07-27 13:08:05 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 11 Tomás Cohen Arazi 2022-07-27 13:08:57 UTC
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>
Comment 12 Tomás Cohen Arazi 2022-07-27 13:09:03 UTC
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>
Comment 13 Lucas Gass 2022-09-02 20:22:16 UTC
Backported to 22.05.x for 22.05.05
Comment 14 Arthur Suzuki 2022-09-14 12:59:03 UTC
Thanks!

Pushed to 21.11 for 21.11.12
Comment 15 Victor Grousset/tuxayo 2022-09-20 20:03:01 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document it seems, marking resolved.