Bug 25114

Summary: Remove duplicated logic from GetLoanLength()
Product: Koha Reporter: Lari Taskula <lari.taskula>
Component: Architecture, internals, and plumbingAssignee: Lari Taskula <lari.taskula>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: lari.taskula, lucas
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: Sponsored Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Bug Depends on:    
Bug Blocks: 25089, 26238    
Attachments: Bug 25089: Remove duplicated logic from GetLoanLength()
Bug 25114: Add checkout_type to set_rules
Bug 25114: Remove duplicated logic from GetLoanLength()
Bug 25114: Remove duplicated logic from GetLoanLength()
Bug 25114: Remove duplicated logic from GetLoanLength()

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