Summary: | Remove duplicated logic from GetLoanLength() | ||
---|---|---|---|
Product: | Koha | Reporter: | Lari Taskula <lari.taskula> |
Component: | Architecture, internals, and plumbing | Assignee: | 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
|
|
Circulation function: | |||
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
Created attachment 102766 [details] [review] Bug 25089: 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 Created attachment 102767 [details] [review] Bug 25114: Add checkout_type to set_rules To test: 1. Execute the following command to run a bunch of tests grep -rl --color=never 'get_effective_rule(|set_rule(|set_rules(' \ | grep '^.*\.t$' | xargs prove 2. Test onboarding tool 2.1. Drop&create your database (take backups first if needed) 2.2. Navigate to web installer 2.3. Continue until you reach onboarding tool step where circulation rules are added. 2.4. Observe new drop down selection "Checkout type". 2.5. Select any checkout type and continue the onboarding process. 2.6. Investigate your database table "circulation_rules". You should now have rules with checkout_type of your choice. Sponsored-by: The National Library of Finland 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 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> 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> 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? Pushed to master for 20.11, thanks to everybody involved! 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. enhancement will not be backported to 20.05.x |