Bug 25663

Summary: Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules
Product: Koha Reporter: Martin Renvoize (ashimema) <martin.renvoize>
Component: Architecture, internals, and plumbingAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi (tcohen) <tomascohen>
Severity: enhancement    
Priority: P5 - low CC: andrew, jonathan.druart, josef.moravec, kyle, lucas, nick, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00
Circulation function:
Bug Depends on: 20815    
Bug Blocks: 18501    
Attachments: Bug 25663: Add unit tests
Bug 25663: Add get_lostreturn_policy method to CirculationRules
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: Add unit tests
Bug 25663: Add unit tests
Bug 25663: Add get_lostreturn_policy method to CirculationRules
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: Add unit tests
Bug 25663: Add get_lostreturn_policy method to CirculationRules
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: Add unit tests
Bug 25663: Add get_lostreturn_policy method to CirculationRules
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: Add unit tests
Bug 25663: Add get_lostreturn_policy method to CirculationRules
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses
Bug 25663: [20.05.x] Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules

Description Martin Renvoize (ashimema) 2020-06-03 10:47:04 UTC
Now that we have a clearer set of methods within the Koha::CirculationRules module, we should clean up the stepping stone RefundLostItemFeeRules modules.

This also paves the way for some enhancements to this area of functionality I am looking to do in bug 23091
Comment 1 Martin Renvoize (ashimema) 2020-06-03 13:55:09 UTC
Created attachment 105524 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff
Comment 2 Martin Renvoize (ashimema) 2020-06-03 13:55:12 UTC
Created attachment 105525 [details] [review]
Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.
Comment 3 Martin Renvoize (ashimema) 2020-06-03 13:55:15 UTC
Created attachment 105526 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff
Comment 4 Martin Renvoize (ashimema) 2020-06-03 14:09:40 UTC
Created attachment 105527 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff
Comment 5 Kyle M Hall (khall) 2020-06-19 16:13:42 UTC
Created attachment 106105 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall (khall) 2020-06-19 16:15:13 UTC
Created attachment 106106 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall (khall) 2020-06-19 16:15:22 UTC
Created attachment 106107 [details] [review]
Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Kyle M Hall (khall) 2020-06-19 16:15:25 UTC
Created attachment 106108 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Andrew Fuerste-Henry 2020-07-12 15:40:14 UTC
Patch no longer applies. Please rebase? Needed to test bug 23091
Comment 10 Martin Renvoize (ashimema) 2020-07-13 08:26:36 UTC
Created attachment 106823 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Martin Renvoize (ashimema) 2020-07-13 08:26:39 UTC
Created attachment 106824 [details] [review]
Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 12 Martin Renvoize (ashimema) 2020-07-13 08:26:43 UTC
Created attachment 106825 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 13 Martin Renvoize (ashimema) 2020-07-13 08:27:40 UTC
Rebased
Comment 14 Martin Renvoize (ashimema) 2020-07-24 12:14:41 UTC
Created attachment 107332 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 15 Martin Renvoize (ashimema) 2020-07-24 12:14:45 UTC
Created attachment 107333 [details] [review]
Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 16 Martin Renvoize (ashimema) 2020-07-24 12:14:48 UTC
Created attachment 107334 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Martin Renvoize (ashimema) 2020-07-24 12:15:59 UTC
Rebased for bug 20815
Comment 18 Tomás Cohen Arazi (tcohen) 2020-08-11 13:51:45 UTC
Created attachment 108069 [details] [review]
Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 19 Tomás Cohen Arazi (tcohen) 2020-08-11 13:51:51 UTC
Created attachment 108070 [details] [review]
Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Tomás Cohen Arazi (tcohen) 2020-08-11 13:51:58 UTC
Created attachment 108071 [details] [review]
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi (tcohen) 2020-08-11 13:56:54 UTC
I'm not convinced by the current approach, but consistency is a good stepping stone for evolving. The patches are clean, covered by tests and work as expected.
QA scripts say this are fine as well.
Comment 22 Jonathan Druart 2020-08-13 05:57:20 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 23 Lucas Gass (lukeg) 2020-08-17 15:26:09 UTC
enhancement will not be backported to 20.05.x
Comment 24 Nick Clemens (kidclamp) 2020-10-06 13:49:03 UTC
Created attachment 111309 [details] [review]
Bug 25663: [20.05.x] Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules

Bug 25663: Add unit tests

This patch adds unit tests for a new get_lostretun_policy method which
is getting added to Koha::CirculationRules.

Test plan:
1/ Read the additions to t/db_dependent/CirculationRules.t code
   and compare to the t/db_dependent/RefundLostItemFeeRule.t.
2/ Run the test after applying the patch that adds the new method
3/ If the tests all pass, signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 25663: Add get_lostreturn_policy method to CirculationRules

This patch adds a new get_lostreturn_policy method to
Koha::CirculationRules which returns a boolean to the caller which
denotes whether a refund should be applied or not on the return of
a lost item.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Bug 25663: Remove Koha::RefundLostItemFeeRule and uses

This patch replaces all calls to RefundLostItemFeeRules with
Koha::CirculationRules->get_lostreturn_policy and removes the module it
makes redundant.

Test plan
1/ Confirm that there are no longer any uses of RefundLostItemFeeRules
in the codebase
2/ Confirm circulation tests still all pass
3/ Confirm you can still set and unset the lost return rules
4/ Signoff

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

https://bugs.koha-community.org/show_bug.cgi?id=25633