Bug 7614 - Use branch transfer limits for determining available opac holds pickup locations
Summary: Use branch transfer limits for determining available opac holds pickup locations
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Lari Taskula
QA Contact: Kyle M Hall
URL:
Keywords:
: 20257 (view as bug list)
Depends on: 7534 18072
Blocks: 22330 22688 23774 23821
  Show dependency treegraph
 
Reported: 2012-02-28 16:38 UTC by Paul Poulain
Modified: 2020-11-30 21:45 UTC (History)
14 users (show)

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


Attachments
Bug 7614: Add a new method Koha::Libraries->pickup_locations (19.81 KB, patch)
2017-02-13 11:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved (5.62 KB, patch)
2017-02-13 11:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Check branch transfer limits in opac-reserve.pl (2.33 KB, patch)
2017-02-13 11:20 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches (1.48 KB, patch)
2017-02-13 11:21 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits (3.93 KB, patch)
2017-02-13 11:22 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Check branch transfer limits in opac-reserve.pl (2.29 KB, patch)
2017-03-01 14:55 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches (1.48 KB, patch)
2017-03-01 14:55 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits (4.23 KB, patch)
2017-03-01 14:55 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614 - Fix my variable earlier declaration bug. (833 bytes, patch)
2017-03-09 11:07 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7614 - Fixes "Looks like you planned 61 tests but ran 62" at t/db_dependent/Holds.t (686 bytes, patch)
2017-03-09 12:47 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one (3.16 KB, patch)
2017-12-21 15:07 UTC, Lari Taskula
Details | Diff | Splinter Review
Bug 7614: Add a new method Koha::Libraries->pickup_locations (19.70 KB, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved (3.46 KB, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches (1.62 KB, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits (3.34 KB, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614 - Fix my variable earlier declaration bug. (840 bytes, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one (2.71 KB, patch)
2018-08-16 11:31 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown (4.50 KB, patch)
2018-08-20 14:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Add a new method Koha::Libraries->pickup_locations (19.73 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved (5.16 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches (1.70 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits (3.43 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614 - Fix my variable earlier declaration bug. (753 bytes, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one (2.34 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown (4.37 KB, patch)
2019-01-22 18:23 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Add a new method Koha::Libraries->pickup_locations (19.78 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved (5.21 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches (1.75 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits (3.48 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Fix my variable earlier declaration bug. (805 bytes, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one (2.39 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown (4.42 KB, patch)
2019-01-22 19:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614 [QA Follow-up]: Fix POD merge missed in rebase (2.07 KB, patch)
2019-01-22 19:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614 [QA Follow-up]: Tidy code to remove tab characters (1.70 KB, patch)
2019-01-22 19:08 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 7614 [QA Follow-up]: Remove unused variable (756 bytes, patch)
2019-01-22 19:08 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2012-02-28 16:38:50 UTC
When you have excluded transfers from libA to libB, it means that a book from libB can't be checked in at libA.

But it also means that a hold shouldn't be placed on a book from libA for pickup at libB
For now, there is no limit control on opac-reserve.pl (not checked on staff reserve.pl, but I bet it's the same thing)
a patron can place a hold on an item from libA, asking for pickup at libB
Comment 1 Paul Poulain 2012-02-28 16:41:51 UTC
(BibLibre mantis MT9018)
Comment 2 Katrin Fischer 2015-06-06 22:23:11 UTC
Paul, you put a mantis number on this bug, could you check its status in your ticket system?
Comment 3 Lari Taskula 2017-02-10 14:39:12 UTC
I would like to work with this issue. I have prepared some stuff in other Bugs that should make this one easier to solve, see Bug 7534 (let each library define if they don't want to act as a pickup library) and Bug 18072 (easily check branch transfer limitations for a specific item and biblio).

My proposal is that in this Bug we do the following:
1. Add branch transfer limits checks to CanBook/ItemBeReserved
2. Attempt to construct a "smarter" pickup location list that considers both 1. library's configuration on being a pickup location, and 2. branch transfer limits for that particular book

Dependencies:
Bug 7534
Bug 18072
Comment 4 Lari Taskula 2017-02-13 11:20:06 UTC
Created attachment 60141 [details] [review]
Bug 7614: Add a new method Koha::Libraries->pickup_locations

This patch adds a new method, Koha::Libraries->pickup_locations. This method
takes either an item or a biblio as a parameter, and returns the list of
acceptable pickup locations by considering libraries' configuration to act
as a pickup location as well as any branch transfer limits that may apply.

To test:
1. prove t/db_dependent/Koha/Libraries.t
Comment 5 Lari Taskula 2017-02-13 11:20:32 UTC
Created attachment 60142 [details] [review]
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved

This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold. This patch
checks if the library is configured to be a pickup location (see Bug 7534), and
also if the item can be transferred into the given library (see Bug 18072).

To test:
1. prove t/db_dependent/Holds.t
Comment 6 Lari Taskula 2017-02-13 11:20:57 UTC
Created attachment 60143 [details] [review]
Bug 7614: Check branch transfer limits in opac-reserve.pl
Comment 7 Lari Taskula 2017-02-13 11:21:49 UTC
Created attachment 60144 [details] [review]
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches

Consider transfer limits in Koha::Template::Plugin::Branches->pickup_locations.

This patch modified the mentioned method to consider branch transfer limits and
does not include any library in the returned list of libraries in case a branch
transfer limit applies for a given book or item.

To be tested in following patches.
Comment 8 Lari Taskula 2017-02-13 11:22:13 UTC
Created attachment 60145 [details] [review]
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

This patch enables the checks for transfer limits for given biblionumber in
OPAC when patron proceeds to place a hold. In case none of the items of that
biblio can be transferred to a library, that library will not be included in
the list of pickup locations.

To test:
1. Make sure you meet the following requirements:
   1.1: You are able to make holds in OPAC
   1.2: System preference UseBranchTransferLimits set to "Enforce"
   1.3: System preference BranchTransferLimitsType set to "item type"
        (for the sake of simplicity of this test plan, but you may also
         set it to "collection code" and deal with ccodes instead)
   1.4: System preference item-level_itypes set to "specific item"
   1.5: You have the default libraries and item types, otherwise replace
        the branchcodes and itemtypes in this test plan with your own
2. Make sure 'Fairfield' in cgi-bin/koha/admin/branches.pl has not disabled
   its ability to act as a "Pickup location"
3. Have a biblio with exactly one item of item-level itemtype 'BK'
4. Set item holdingbranch as 'CPL'
5. Go to cgi-bin/koha/admin/branch_transfer_limits.pl (keep this page open in
   a second tab for faster testing)
6. Select library 'Centerville'
7. Uncheck the checkbox 'FFL - Fairfield' and click Save
8. Go to OPAC and find your biblio, and start placing a hold on it
9. Click at the list of "Pick up location"s.
10. Observe that it is missing 'Fairfield'
11. Go back to cgi-bin/koha/admin/branch_transfer_limits.pl
12. Repeat step 6 and this time, check the checkbox 'FFL - Fairfield'
13. Refresh the page you arrived at step 8
14. Observe that it now includes 'Fairfield'
Comment 9 Lari Taskula 2017-02-13 11:24:43 UTC
I have posted some patches for my proposed solution to be tested in OPAC. Please apply Bug 7534 and Bug 18072 before applying these patches.
Comment 10 Lari Taskula 2017-03-01 14:55:10 UTC
Created attachment 60785 [details] [review]
Bug 7614: Check branch transfer limits in opac-reserve.pl
Comment 11 Lari Taskula 2017-03-01 14:55:25 UTC
Created attachment 60786 [details] [review]
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches

Consider transfer limits in Koha::Template::Plugin::Branches->pickup_locations.

This patch modified the mentioned method to consider branch transfer limits and
does not include any library in the returned list of libraries in case a branch
transfer limit applies for a given book or item.

To be tested in following patches.
Comment 12 Lari Taskula 2017-03-01 14:55:45 UTC
Created attachment 60787 [details] [review]
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

This patch enables the checks for transfer limits for given biblionumber in
OPAC when patron proceeds to place a hold. In case none of the items of that
biblio can be transferred to a library, that library will not be included in
the list of pickup locations.

To test:
1. Make sure you meet the following requirements:
   1.1: You are able to make holds in OPAC
   1.2: System preference UseBranchTransferLimits set to "Enforce"
   1.3: System preference BranchTransferLimitsType set to "item type"
        (for the sake of simplicity of this test plan, but you may also
         set it to "collection code" and deal with ccodes instead)
   1.4: System preference item-level_itypes set to "specific item"
   1.5: You have the default libraries and item types, otherwise replace
        the branchcodes and itemtypes in this test plan with your own
2. Make sure 'Fairfield' in cgi-bin/koha/admin/branches.pl has not disabled
   its ability to act as a "Pickup location"
3. Have a biblio with exactly one item of item-level itemtype 'BK'
4. Set item holdingbranch as 'CPL'
5. Go to cgi-bin/koha/admin/branch_transfer_limits.pl (keep this page open in
   a second tab for faster testing)
6. Select library 'Centerville'
7. Uncheck the checkbox 'FFL - Fairfield' and click Save
8. Go to OPAC and find your biblio, and start placing a hold on it
9. Click at the list of "Pick up location"s.
10. Observe that it is missing 'Fairfield'
11. Go back to cgi-bin/koha/admin/branch_transfer_limits.pl
12. Repeat step 6 and this time, check the checkbox 'FFL - Fairfield'
13. Refresh the page you arrived at step 8
14. Observe that it now includes 'Fairfield'
Comment 13 Koha Team AMU 2017-03-09 10:07:44 UTC
I tried to test in sandbox but failed : 
The sandbox you've requested is not ready.
Some problems occurred applying patches from bug 7614:
<h1>Something went wrong !</h1>Applying: Bug 7614: Add a new method Koha::Libraries->pickup_locations
Applying: Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved
Applying: Bug 7614: Check branch transfer limits in opac-reserve.pl
Applying: Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches
fatal: sha1 information is lacking or useless (Koha/Template/Plugin/Branches.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.

Patch failed at 0001 Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches
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.
Bug 7614 - Branch transfer limit and holds

60141 - Bug 7614: Add a new method Koha::Libraries->pickup_locations
60142 - Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved
60785 - Bug 7614: Check branch transfer limits in opac-reserve.pl
60786 - Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches
60787 - Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

Apply? [(y)es, (n)o, (i)nteractive] Patch left in /tmp/Bug-7614-Consider-transfer-limits-in-KohaTemplateP-J1NHLc.patch .
Comment 14 Olli-Antti Kivilahti 2017-03-09 11:07:15 UTC
Created attachment 60938 [details] [review]
Bug 7614 - Fix my variable earlier declaration bug.
Comment 15 Olli-Antti Kivilahti 2017-03-09 12:47:52 UTC
Created attachment 60946 [details] [review]
Bug 7614 - Fixes "Looks like you planned 61 tests but ran 62" at t/db_dependent/Holds.t
Comment 16 Lari Taskula 2017-03-13 16:05:00 UTC
(In reply to Koha Team AMU from comment #13)
> I tried to test in sandbox but failed : 

Hi and thanks for testing! This patch set is not rebased yet on top of Bug 7534 as it is currently going through some changes in QA process. I will rebase these patches once Bug 7534 passes QA.

(In reply to Olli-Antti Kivilahti from comment #14)
> Created attachment 60938 [details] [review] [review]
> Bug 7614 - Fix my variable earlier declaration bug.
(In reply to Olli-Antti Kivilahti from comment #15)
> Created attachment 60946 [details] [review] [review]
> Bug 7614 - Fixes "Looks like you planned 61 tests but ran 62" at
> t/db_dependent/Holds.t

Thanks Olli-Antti!
Comment 17 Lari Taskula 2017-12-21 15:07:14 UTC
Created attachment 70047 [details] [review]
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one

This patch fixes an issue where patron is not allowed to place a hold in OPAC
while their home library is not a pickup library.

Instead, they should be presented with a list of other available pickup locations.
Comment 18 Kyle M Hall 2018-08-16 11:31:20 UTC
Created attachment 77895 [details] [review]
Bug 7614: Add a new method Koha::Libraries->pickup_locations

This patch adds a new method, Koha::Libraries->pickup_locations. This method
takes either an item or a biblio as a parameter, and returns the list of
acceptable pickup locations by considering libraries' configuration to act
as a pickup location as well as any branch transfer limits that may apply.

To test:
1. prove t/db_dependent/Koha/Libraries.t
Comment 19 Kyle M Hall 2018-08-16 11:31:30 UTC
Created attachment 77896 [details] [review]
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved

This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold. This patch
checks if the library is configured to be a pickup location (see Bug 7534), and
also if the item can be transferred into the given library (see Bug 18072).

To test:
1. prove t/db_dependent/Holds.t
Comment 20 Kyle M Hall 2018-08-16 11:31:34 UTC
Created attachment 77897 [details] [review]
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches

Consider transfer limits in Koha::Template::Plugin::Branches->pickup_locations.

This patch modified the mentioned method to consider branch transfer limits and
does not include any library in the returned list of libraries in case a branch
transfer limit applies for a given book or item.

To be tested in following patches.
Comment 21 Kyle M Hall 2018-08-16 11:31:38 UTC
Created attachment 77898 [details] [review]
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

This patch enables the checks for transfer limits for given biblionumber in
OPAC when patron proceeds to place a hold. In case none of the items of that
biblio can be transferred to a library, that library will not be included in
the list of pickup locations.

To test:
1. Make sure you meet the following requirements:
   1.1: You are able to make holds in OPAC
   1.2: System preference UseBranchTransferLimits set to "Enforce"
   1.3: System preference BranchTransferLimitsType set to "item type"
        (for the sake of simplicity of this test plan, but you may also
         set it to "collection code" and deal with ccodes instead)
   1.4: System preference item-level_itypes set to "specific item"
   1.5: You have the default libraries and item types, otherwise replace
        the branchcodes and itemtypes in this test plan with your own
2. Make sure 'Fairfield' in cgi-bin/koha/admin/branches.pl has not disabled
   its ability to act as a "Pickup location"
3. Have a biblio with exactly one item of item-level itemtype 'BK'
4. Set item holdingbranch as 'CPL'
5. Go to cgi-bin/koha/admin/branch_transfer_limits.pl (keep this page open in
   a second tab for faster testing)
6. Select library 'Centerville'
7. Uncheck the checkbox 'FFL - Fairfield' and click Save
8. Go to OPAC and find your biblio, and start placing a hold on it
9. Click at the list of "Pick up location"s.
10. Observe that it is missing 'Fairfield'
11. Go back to cgi-bin/koha/admin/branch_transfer_limits.pl
12. Repeat step 6 and this time, check the checkbox 'FFL - Fairfield'
13. Refresh the page you arrived at step 8
14. Observe that it now includes 'Fairfield'
Comment 22 Kyle M Hall 2018-08-16 11:31:42 UTC
Created attachment 77899 [details] [review]
Bug 7614 - Fix my variable earlier declaration bug.
Comment 23 Kyle M Hall 2018-08-16 11:31:45 UTC
Created attachment 77900 [details] [review]
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one

This patch fixes an issue where patron is not allowed to place a hold in OPAC
while their home library is not a pickup library.

Instead, they should be presented with a list of other available pickup locations.
Comment 24 Kyle M Hall 2018-08-20 14:49:34 UTC
Created attachment 78020 [details] [review]
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown

Also fully qualifies some subroutine calls that fail for reasons unkown.
Comment 25 Kyle M Hall 2018-09-19 14:13:32 UTC
*** Bug 20257 has been marked as a duplicate of this bug. ***
Comment 26 Alex Buckley 2018-10-24 20:05:34 UTC
Sorry the patches do not apply, can you please rebase:

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 7614: Add a new method Koha::Libraries->pickup_locations
Using index info to reconstruct a base tree...
M	t/db_dependent/Koha/Libraries.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Koha/Libraries.t
CONFLICT (content): Merge conflict in t/db_dependent/Koha/Libraries.t
Failed to merge in the changes.
Patch failed at 0001 Bug 7614: Add a new method Koha::Libraries->pickup_locations
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-7614-Add-a-new-method-KohaLibraries-pickuploca-2UIRVc.patch
You have new mail in /var/mail/vagrant
Comment 27 Kyle M Hall 2019-01-22 18:23:02 UTC
Created attachment 84295 [details] [review]
Bug 7614: Add a new method Koha::Libraries->pickup_locations

This patch adds a new method, Koha::Libraries->pickup_locations. This method
takes either an item or a biblio as a parameter, and returns the list of
acceptable pickup locations by considering libraries' configuration to act
as a pickup location as well as any branch transfer limits that may apply.

To test:
1. prove t/db_dependent/Koha/Libraries.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 28 Kyle M Hall 2019-01-22 18:23:16 UTC
Created attachment 84296 [details] [review]
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved

This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold. This patch
checks if the library is configured to be a pickup location (see Bug 7534), and
also if the item can be transferred into the given library (see Bug 18072).

To test:
1. prove t/db_dependent/Holds.t

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 29 Kyle M Hall 2019-01-22 18:23:20 UTC
Created attachment 84297 [details] [review]
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches

Consider transfer limits in Koha::Template::Plugin::Branches->pickup_locations.

This patch modified the mentioned method to consider branch transfer limits and
does not include any library in the returned list of libraries in case a branch
transfer limit applies for a given book or item.

To be tested in following patches.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 30 Kyle M Hall 2019-01-22 18:23:23 UTC
Created attachment 84298 [details] [review]
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

This patch enables the checks for transfer limits for given biblionumber in
OPAC when patron proceeds to place a hold. In case none of the items of that
biblio can be transferred to a library, that library will not be included in
the list of pickup locations.

To test:
1. Make sure you meet the following requirements:
   1.1: You are able to make holds in OPAC
   1.2: System preference UseBranchTransferLimits set to "Enforce"
   1.3: System preference BranchTransferLimitsType set to "item type"
        (for the sake of simplicity of this test plan, but you may also
         set it to "collection code" and deal with ccodes instead)
   1.4: System preference item-level_itypes set to "specific item"
   1.5: You have the default libraries and item types, otherwise replace
        the branchcodes and itemtypes in this test plan with your own
2. Make sure 'Fairfield' in cgi-bin/koha/admin/branches.pl has not disabled
   its ability to act as a "Pickup location"
3. Have a biblio with exactly one item of item-level itemtype 'BK'
4. Set item holdingbranch as 'CPL'
5. Go to cgi-bin/koha/admin/branch_transfer_limits.pl (keep this page open in
   a second tab for faster testing)
6. Select library 'Centerville'
7. Uncheck the checkbox 'FFL - Fairfield' and click Save
8. Go to OPAC and find your biblio, and start placing a hold on it
9. Click at the list of "Pick up location"s.
10. Observe that it is missing 'Fairfield'
11. Go back to cgi-bin/koha/admin/branch_transfer_limits.pl
12. Repeat step 6 and this time, check the checkbox 'FFL - Fairfield'
13. Refresh the page you arrived at step 8
14. Observe that it now includes 'Fairfield'

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 31 Kyle M Hall 2019-01-22 18:23:27 UTC
Created attachment 84299 [details] [review]
Bug 7614 - Fix my variable earlier declaration bug.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 32 Kyle M Hall 2019-01-22 18:23:31 UTC
Created attachment 84300 [details] [review]
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one

This patch fixes an issue where patron is not allowed to place a hold in OPAC
while their home library is not a pickup library.

Instead, they should be presented with a list of other available pickup locations.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2019-01-22 18:23:34 UTC
Created attachment 84301 [details] [review]
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown

Also fully qualifies some subroutine calls that fail for reasons unkown.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2019-01-22 19:07:31 UTC
Created attachment 84302 [details] [review]
Bug 7614: Add a new method Koha::Libraries->pickup_locations

This patch adds a new method, Koha::Libraries->pickup_locations. This method
takes either an item or a biblio as a parameter, and returns the list of
acceptable pickup locations by considering libraries' configuration to act
as a pickup location as well as any branch transfer limits that may apply.

To test:
1. prove t/db_dependent/Koha/Libraries.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 35 Kyle M Hall 2019-01-22 19:07:41 UTC
Created attachment 84303 [details] [review]
Bug 7614: Check transfer limit in CanBookBeReserved and CanItemBeReserved

This patch adds $branchcode_to parameter to CanBookBeReserved and
CanItemBeReserved. It represents the pickup location for the hold. This patch
checks if the library is configured to be a pickup location (see Bug 7534), and
also if the item can be transferred into the given library (see Bug 18072).

To test:
1. prove t/db_dependent/Holds.t

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 36 Kyle M Hall 2019-01-22 19:07:45 UTC
Created attachment 84304 [details] [review]
Bug 7614: Consider transfer limits in Koha::Template::Plugin::Branches

Consider transfer limits in Koha::Template::Plugin::Branches->pickup_locations.

This patch modified the mentioned method to consider branch transfer limits and
does not include any library in the returned list of libraries in case a branch
transfer limit applies for a given book or item.

To be tested in following patches.

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 37 Kyle M Hall 2019-01-22 19:07:48 UTC
Created attachment 84305 [details] [review]
Bug 7614: Build a list of pickup locations in OPAC considering transfer limits

This patch enables the checks for transfer limits for given biblionumber in
OPAC when patron proceeds to place a hold. In case none of the items of that
biblio can be transferred to a library, that library will not be included in
the list of pickup locations.

To test:
1. Make sure you meet the following requirements:
   1.1: You are able to make holds in OPAC
   1.2: System preference UseBranchTransferLimits set to "Enforce"
   1.3: System preference BranchTransferLimitsType set to "item type"
        (for the sake of simplicity of this test plan, but you may also
         set it to "collection code" and deal with ccodes instead)
   1.4: System preference item-level_itypes set to "specific item"
   1.5: You have the default libraries and item types, otherwise replace
        the branchcodes and itemtypes in this test plan with your own
2. Make sure 'Fairfield' in cgi-bin/koha/admin/branches.pl has not disabled
   its ability to act as a "Pickup location"
3. Have a biblio with exactly one item of item-level itemtype 'BK'
4. Set item holdingbranch as 'CPL'
5. Go to cgi-bin/koha/admin/branch_transfer_limits.pl (keep this page open in
   a second tab for faster testing)
6. Select library 'Centerville'
7. Uncheck the checkbox 'FFL - Fairfield' and click Save
8. Go to OPAC and find your biblio, and start placing a hold on it
9. Click at the list of "Pick up location"s.
10. Observe that it is missing 'Fairfield'
11. Go back to cgi-bin/koha/admin/branch_transfer_limits.pl
12. Repeat step 6 and this time, check the checkbox 'FFL - Fairfield'
13. Refresh the page you arrived at step 8
14. Observe that it now includes 'Fairfield'

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 38 Kyle M Hall 2019-01-22 19:07:52 UTC
Created attachment 84306 [details] [review]
Bug 7614: Fix my variable earlier declaration bug.

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 39 Kyle M Hall 2019-01-22 19:07:55 UTC
Created attachment 84307 [details] [review]
Bug 7614: (follow-up) If patron's home library is not a pickup library, let them choose another one

This patch fixes an issue where patron is not allowed to place a hold in OPAC
while their home library is not a pickup library.

Instead, they should be presented with a list of other available pickup locations.

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 40 Kyle M Hall 2019-01-22 19:07:59 UTC
Created attachment 84308 [details] [review]
Bug 7614: Don't allow un-usable pickup locations in the opac pickup locations pulldown

Also fully qualifies some subroutine calls that fail for reasons unkown.

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 41 Kyle M Hall 2019-01-22 19:08:03 UTC
Created attachment 84309 [details] [review]
Bug 7614 [QA Follow-up]: Fix POD merge missed in rebase
Comment 42 Kyle M Hall 2019-01-22 19:08:07 UTC
Created attachment 84310 [details] [review]
Bug 7614 [QA Follow-up]: Tidy code to remove tab characters
Comment 43 Kyle M Hall 2019-01-22 19:08:11 UTC
Created attachment 84311 [details] [review]
Bug 7614 [QA Follow-up]: Remove unused variable
Comment 44 Nick Clemens 2019-01-25 17:57:06 UTC
Awesome work all!

Pushed to master for 19.05
Comment 45 Martin Renvoize 2019-01-31 16:35:02 UTC
Pushed to 18.11.x for 18.11.03. 

This should probably have got pushed alongside it's dependants before now but unfortunately didn't quite make the final cut.  Bugfix.
Comment 46 Lucas Gass 2019-02-01 19:25:18 UTC
missing dependency for 18.05.x wont backport
Comment 47 Jonathan Druart 2019-10-16 10:05:14 UTC
The constraint should have been enforce at higher level than template, see bug 23774.