Bug 19300

Summary: Move C4::Reserves::OPACItemHoldsAllowed to the Koha namespace
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: m.de.rooy, nick
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17728, 19297, 19301, 19302    
Attachments: Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - tests
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed
Bug 19300: Replace C4::Reserves::OPACItemHoldsAllowed - Remove the subroutine

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!