29537 – Simplify auto-renewal code in CanBookBeRenewed

Bug 29537 - Simplify auto-renewal code in CanBookBeRenewed

Summary: Simplify auto-renewal code in CanBookBeRenewed

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Circulation (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low normal
Assignee:	Nick Clemens (kidclamp)
QA Contact:	Marcel de Rooy

URL:
Keywords:

Depends on:	29474 29476
Blocks:	29623
	Show dependency tree / graph

Reported:	2021-11-20 11:52 UTC by Joonas Kylmälä
Modified:	2023-06-08 22:26 UTC (History)
CC List:	9 users (show)

See Also:	30167
Change sponsored?:	---
Patch complexity:	Small patch
Documentation contact:	Martin Renvoize
Documentation submission:
Text to go in the release notes:
Version(s) released in:	22.05.00,21.11.06
Circulation function:

Attachments
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.46 KB, patch) 2021-11-20 11:56 UTC, Joonas Kylmälä	Details \| Diff \| Splinter Review
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.46 KB, patch) 2021-11-20 11:59 UTC, Joonas Kylmälä	Details \| Diff \| Splinter Review
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.51 KB, patch) 2021-11-29 21:38 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.47 KB, patch) 2022-03-24 11:46 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.52 KB, patch) 2022-04-20 22:15 UTC, David Nind	Details \| Diff \| Splinter Review
Bug 29537: Remove unnecessary if-else checks for soonest renew date (1.62 KB, patch) 2022-04-29 08:32 UTC, Marcel de Rooy	Details \| Diff \| Splinter Review
Show Obsolete (5) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Joonas Kylmälä 2021-11-20 11:52:41 UTC

Comment 1 Joonas Kylmälä 2021-11-20 11:56:01 UTC Comment hidden (obsolete)

Created attachment 127901 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29537 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Comment 2 Joonas Kylmälä 2021-11-20 11:59:28 UTC

Created attachment 127902 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Comment 3 David Nind 2021-11-29 21:38:11 UTC

Created attachment 128095 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Signed-off-by: David Nind <david@davidnind.com>

Comment 4 Jonathan Druart 2021-12-15 10:38:36 UTC

No longer apply correctly.

Comment 5 Nick Clemens (kidclamp) 2022-03-24 11:46:49 UTC

Created attachment 132129 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Comment 6 David Nind 2022-04-20 22:15:01 UTC

Created attachment 133505 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Signed-off-by: David Nind <david@davidnind.com>

Comment 7 Marcel de Rooy 2022-04-29 07:45:11 UTC

QAing

Comment 8 Marcel de Rooy 2022-04-29 08:32:41 UTC

Created attachment 134348 [details] [review]
Bug 29537: Remove unnecessary if-else checks for soonest renew date

GetSoonestRenewDate() calculates the renewal due date now also
correctly for auto-renewals after the fixes from bug 29476 so we don't
have to add any if-else checks because of it being buggy anymore.

To test:
 1) prove t/db_dependent/Circulation.t

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Comment 9 Marcel de Rooy 2022-04-29 08:33:49 UTC

Subtest: GetSoonestRenewDate tests passes, altough currently the whole Circulation.t fails (also without this patch)

Comment 10 Fridolin Somers 2022-05-03 20:50:00 UTC

Circulation.t is OK for me with and without this patch

Comment 11 Fridolin Somers 2022-05-03 21:27:35 UTC

Pushed to master for 22.05, thanks to everybody involved ﷐[U+1F984]﷑

Comment 12 Kyle M Hall (khall) 2022-05-13 16:07:42 UTC

Pushed to 21.11.x for 21.11.06

Comment 13 Victor Grousset/tuxayo 2022-06-23 01:58:21 UTC

Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Comment 14 Martin Renvoize (ashimema) 2022-06-23 13:38:52 UTC

Architectural change, no workflow differences. No documentation changes required.