Bug 25114 - Remove duplicated logic from GetLoanLength()
Summary: Remove duplicated logic from GetLoanLength()
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lari Taskula
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 25089 26238
  Show dependency treegraph
 
Reported: 2020-04-11 19:59 UTC by Lari Taskula
Modified: 2020-09-04 15:31 UTC (History)
2 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 25089: Remove duplicated logic from GetLoanLength() (3.09 KB, patch)
2020-04-11 20:01 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25114: Add checkout_type to set_rules (24.83 KB, patch)
2020-04-11 20:02 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25114: Remove duplicated logic from GetLoanLength() (3.03 KB, patch)
2020-04-11 20:03 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 25114: Remove duplicated logic from GetLoanLength() (3.19 KB, patch)
2020-06-12 10:04 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 25114: Remove duplicated logic from GetLoanLength() (3.25 KB, patch)
2020-08-16 10:14 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lari Taskula 2020-04-11 19:59:50 UTC
Remove duplicated logic for searching circulation rules.
Comment 1 Lari Taskula 2020-04-11 20:01:49 UTC Comment hidden (obsolete)
Comment 2 Lari Taskula 2020-04-11 20:02:43 UTC Comment hidden (obsolete)
Comment 3 Lari Taskula 2020-04-11 20:03:58 UTC
Created attachment 102768 [details] [review]
Bug 25114: Remove duplicated logic from GetLoanLength()

Remove duplicated logic for searching circulation rules.

This can be replaced with get_effective_rules().

To test:
1. prove t/db_dependent/Circulation/GetHardDueDate.t
Comment 4 Joonas Kylmälä 2020-06-12 10:04:18 UTC
Created attachment 105785 [details] [review]
Bug 25114: Remove duplicated logic from GetLoanLength()

Remove duplicated logic for searching circulation rules.

This can be replaced with get_effective_rules().

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

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 5 Katrin Fischer 2020-08-16 10:14:27 UTC
Created attachment 108349 [details] [review]
Bug 25114: Remove duplicated logic from GetLoanLength()

Remove duplicated logic for searching circulation rules.

This can be replaced with get_effective_rules().

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

Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2020-08-16 10:17:07 UTC
The change made me wonder if we should keep GetLoanLength at all. It's currently only used by CalcDueDate.

Should we replace the call there and get rid of this method?
Comment 7 Jonathan Druart 2020-08-18 15:46:26 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 8 Lari Taskula 2020-08-18 15:58:08 UTC
Forgot to mention this the commit text but this was sponsored by The National Library of Finland.

(In reply to Katrin Fischer from comment #6)
> The change made me wonder if we should keep GetLoanLength at all. It's
> currently only used by CalcDueDate.
> 
> Should we replace the call there and get rid of this method?

I agree and confirm it is only being used in C4::CalcDueDate(). These days one can easily Get Loan Length via Koha::CirculationRules->get_effective_rules(). Let's open a new Bug for it.
Comment 9 Lucas Gass 2020-09-04 15:31:01 UTC
enhancement will not be backported to 20.05.x