Bug 19301 - Move C4::Reserves::OnShelfHoldsAllowed to the Koha namespace
Summary: Move C4::Reserves::OnShelfHoldsAllowed 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: 4319 19298 19300
Blocks: 17728 19297 19302
  Show dependency treegraph
 
Reported: 2017-09-12 21:36 UTC by Jonathan Druart
Modified: 2018-06-04 20:15 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 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy (6.34 KB, patch)
2017-09-13 19:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed (3.23 KB, patch)
2017-09-13 19:23 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy (6.36 KB, patch)
2017-09-27 23:45 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed (3.27 KB, patch)
2017-09-27 23:47 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy (6.38 KB, patch)
2017-12-18 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed (3.21 KB, patch)
2017-12-18 14:16 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy (6.44 KB, patch)
2017-12-22 09:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed (3.29 KB, patch)
2017-12-22 09:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19301: (QA follow-up) Add POD, use statements and correct typo (2.39 KB, patch)
2017-12-22 09:34 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 19301: (QA follow-up) Add a few simple tests (2.01 KB, patch)
2017-12-22 09:34 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:36:37 UTC

    
Comment 1 Jonathan Druart 2017-09-13 19:23:10 UTC
Created attachment 67122 [details] [review]
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy

Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy

Test plan:
Make sure the onshelfholds policy is correct when placing a hold
Comment 2 Jonathan Druart 2017-09-13 19:23:13 UTC
Created attachment 67123 [details] [review]
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed

At this point the subroutine is not used anymore
Comment 3 Alex Buckley 2017-09-27 04:11:39 UTC
I had issues with applying this patch:

Apply? [(y)es, (n)o, (i)nteractive] i
Applying: Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy
fatal: sha1 information is lacking or useless (C4/Reserves.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-19301-Move-C4ReservesOnShelfHoldsAllowed-to-ge-7LzZnO.patch
Comment 4 Jonathan Druart 2017-09-27 13:11:15 UTC
(In reply to Alex Buckley from comment #3)
> I had issues with applying this patch:

One dependency was missing, please try again on top of 19298 and 19300.
Comment 5 Alex Buckley 2017-09-27 23:45:47 UTC
Created attachment 67420 [details] [review]
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy

Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy

Test plan:
Make sure the onshelfholds policy is correct when placing a hold

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 6 Alex Buckley 2017-09-27 23:47:18 UTC
Created attachment 67421 [details] [review]
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed

At this point the subroutine is not used anymore

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 7 Marcel de Rooy 2017-12-15 09:20:28 UTC
May I suggest to rebase this one on top of 4319 (which has taken quite some time already) ?
Comment 8 Jonathan Druart 2017-12-18 14:16:46 UTC
Created attachment 69846 [details] [review]
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy

Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy

Test plan:
Make sure the onshelfholds policy is correct when placing a hold

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 9 Jonathan Druart 2017-12-18 14:16:51 UTC
Created attachment 69847 [details] [review]
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed

At this point the subroutine is not used anymore

Signed-off-by: Alex Buckley <alexbuckley@catalyst.net.nz>
Comment 10 Jonathan Druart 2017-12-18 14:17:39 UTC
Rebased on top of bug 4319.
Comment 11 Marcel de Rooy 2017-12-22 07:36:41 UTC
QA: Looking here now
Comment 12 Marcel de Rooy 2017-12-22 09:34:01 UTC
Created attachment 70106 [details] [review]
Bug 19301: Move C4::Reserves::OnShelfHoldsAllowed to get_onshelfholds_policy

Following the pattern introduced by bug 19300, we are going to move the
OnShelfHoldsAllowed logic to Koha::IssuingRules->get_onshelfholds_policy

Test plan:
Make sure the onshelfholds policy is correct when placing a hold

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2017-12-22 09:34:05 UTC
Created attachment 70107 [details] [review]
Bug 19301: Remove C4::Reserves::OnShelfHoldsAllowed

At this point the subroutine is not used anymore

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

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2017-12-22 09:34:09 UTC
Created attachment 70108 [details] [review]
Bug 19301: (QA follow-up) Add POD, use statements and correct typo

Add POD for new sub in Koha/IssuingRules.pm.
Adding use Koha::IssuingRules to opac-detail and opac-MARCdetail.
Adding use Koha::Items to opac-detail and opac-MARCdetail.
Correct typo $items => $item in opac-MARCdetail.pl.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2017-12-22 09:34:13 UTC
Created attachment 70109 [details] [review]
Bug 19301: (QA follow-up) Add a few simple tests

We removed tests from Reserves.t, but would be nice to test the new
sub in IssuingRules.t too.
Adding a subtest there.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 16 Jonathan Druart 2018-02-13 17:17:43 UTC
Pushed to master for 18.05, thanks to everybody involved!