Bug 19300 - Move C4::Reserves::OPACItemHoldsAllowed to the Koha namespace
Summary: Move C4::Reserves::OPACItemHoldsAllowed to the Koha namespace
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17728 19297 19302 19301
  Show dependency treegraph
 
Reported: 2017-09-12 21:35 UTC by Jonathan Druart
Modified: 2018-01-16 12:21 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests (4.77 KB, patch)
2017-09-13 19:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed (7.34 KB, patch)
2017-09-13 19:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine (2.66 KB, patch)
2017-09-13 19:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests (4.87 KB, patch)
2017-09-27 04:05 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed (7.42 KB, patch)
2017-09-27 04:07 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine (2.71 KB, patch)
2017-09-27 04:08 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests (4.96 KB, patch)
2017-12-22 07:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed (7.51 KB, patch)
2017-12-22 07:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine (2.81 KB, patch)
2017-12-22 07:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2017-09-12 21:35:55 UTC

    
Comment 1 Jonathan Druart 2017-09-13 19:22:23 UTC
Created attachment 67119 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests

This patch proves that we will not introduce trivial regression.
With the same tests, we will execute the existing code and the new code.

Test plan:
With only this patch applied, prove t/db_dependent/Koha/IssuingRules.t
should return green
Comment 2 Jonathan Druart 2017-09-13 19:22:26 UTC
Created attachment 67120 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed

This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy

On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.

Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.
Comment 3 Jonathan Druart 2017-09-13 19:22:30 UTC
Created attachment 67121 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine

At this point the subroutine is not longer in use, we can remove it
Comment 4 Alex Buckley 2017-09-27 04:05:51 UTC
Created attachment 67388 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests

This patch proves that we will not introduce trivial regression.
With the same tests, we will execute the existing code and the new code.

Test plan:
With only this patch applied, prove t/db_dependent/Koha/IssuingRules.t
should return green

Followed test plan, patches worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 5 Alex Buckley 2017-09-27 04:07:14 UTC
Created attachment 67389 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed

This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy

On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.

Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.

Followed test plan which worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 Alex Buckley 2017-09-27 04:08:19 UTC
Created attachment 67390 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine

At this point the subroutine is not longer in use, we can remove it

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 7 Marcel de Rooy 2017-12-22 07:39:06 UTC
QA: Looking here now too
Comment 8 Marcel de Rooy 2017-12-22 07:42:04 UTC
require C4::Reserves;
Did I really see that ?
Comment 9 Marcel de Rooy 2017-12-22 07:56:44 UTC
Created attachment 70101 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests

This patch proves that we will not introduce trivial regression.
With the same tests, we will execute the existing code and the new code.

Test plan:
With only this patch applied, prove t/db_dependent/Koha/IssuingRules.t
should return green

Followed test plan, patches worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-12-22 07:56:48 UTC
Created attachment 70102 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed

This patchset move The OPACItemHoldsAllowed logic
(issuingrules.opacitemholds) to a new class method of
Koha::IssuingRules: get_opacitemholds_policy

On the way, this patch will certainly fix the same problem as bug
19298 with onshelfholds.

Test plan:
Make sure the opacitemholds policy is correct when placing a hold at the
OPAC or the staff interface.

Followed test plan which worked as described
Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Marcel de Rooy 2017-12-22 07:56:55 UTC
Created attachment 70103 [details] [review]
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine

At this point the subroutine is not longer in use, we can remove it

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Jonathan Druart 2017-12-22 13:15:19 UTC
(In reply to Marcel de Rooy from comment #8)
> require C4::Reserves;
> Did I really see that ?

It is removed in the next patch. The goal was to prove that the same tests were passing with the current code and the new code :)
Comment 13 Jonathan Druart 2018-01-02 15:12:51 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 14 Nick Clemens 2018-01-16 12:21:34 UTC
Enhancement, skipping for 17.11.x.
Awesome work everybody!