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
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
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.
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
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
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>
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>
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>
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>
Patch no longer applies. Please rebase? Needed to test bug 23091
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>
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>
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>
Rebased
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>
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>
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>
Rebased for bug 20815
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>
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>
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>
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.
Pushed to master for 20.11, thanks to everybody involved!
enhancement will not be backported to 20.05.x
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