Bug 25663 - Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules
Summary: Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement with 1 vote (vote)
Assignee: Martin Renvoize
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 20815
Blocks: 18501
  Show dependency treegraph
 
Reported: 2020-06-03 10:47 UTC by Martin Renvoize
Modified: 2021-12-13 21:12 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00


Attachments
Bug 25663: Add unit tests (5.53 KB, patch)
2020-06-03 13:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add get_lostreturn_policy method to CirculationRules (1.71 KB, patch)
2020-06-03 13:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (19.83 KB, patch)
2020-06-03 13:55 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (24.15 KB, patch)
2020-06-03 14:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add unit tests (5.60 KB, patch)
2020-06-19 16:13 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25663: Add unit tests (5.60 KB, patch)
2020-06-19 16:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25663: Add get_lostreturn_policy method to CirculationRules (1.78 KB, patch)
2020-06-19 16:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (24.22 KB, patch)
2020-06-19 16:15 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25663: Add unit tests (5.55 KB, patch)
2020-07-13 08:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add get_lostreturn_policy method to CirculationRules (1.77 KB, patch)
2020-07-13 08:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (24.21 KB, patch)
2020-07-13 08:26 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add unit tests (5.55 KB, patch)
2020-07-24 12:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add get_lostreturn_policy method to CirculationRules (1.77 KB, patch)
2020-07-24 12:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (24.05 KB, patch)
2020-07-24 12:14 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25663: Add unit tests (5.61 KB, patch)
2020-08-11 13:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25663: Add get_lostreturn_policy method to CirculationRules (1.82 KB, patch)
2020-08-11 13:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25663: Remove Koha::RefundLostItemFeeRule and uses (24.10 KB, patch)
2020-08-11 13:51 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25663: [20.05.x] Koha::RefundLostItemFeeRules should be merged into Koha::CirculationRules (31.39 KB, patch)
2020-10-06 13:49 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 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 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 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 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 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 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 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 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 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 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 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 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 2020-07-13 08:27:40 UTC
Rebased
Comment 14 Martin Renvoize 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 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 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 2020-07-24 12:15:59 UTC
Rebased for bug 20815
Comment 18 Tomás Cohen Arazi 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 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 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 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 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