|
Description
Paul Poulain
2012-02-28 16:38:50 UTC
(BibLibre mantis MT9018) Paul, you put a mantis number on this bug, could you check its status in your ticket system? 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 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 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 Created attachment 60143 [details] [review] Bug 7614: Check branch transfer limits in opac-reserve.pl 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. 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' 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. Created attachment 60785 [details] [review] Bug 7614: Check branch transfer limits in opac-reserve.pl 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. 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' 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 . Created attachment 60938 [details] [review] Bug 7614 - Fix my variable earlier declaration bug. Created attachment 60946 [details] [review] Bug 7614 - Fixes "Looks like you planned 61 tests but ran 62" at t/db_dependent/Holds.t (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! 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. 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 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 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. 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' Created attachment 77899 [details] [review] Bug 7614 - Fix my variable earlier declaration bug. 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. 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. *** Bug 20257 has been marked as a duplicate of this bug. *** 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 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> 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> 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> 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> Created attachment 84299 [details] [review] Bug 7614 - Fix my variable earlier declaration bug. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> 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> 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> 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> 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> 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> 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> 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> 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> 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> Created attachment 84309 [details] [review] Bug 7614 [QA Follow-up]: Fix POD merge missed in rebase Created attachment 84310 [details] [review] Bug 7614 [QA Follow-up]: Tidy code to remove tab characters Created attachment 84311 [details] [review] Bug 7614 [QA Follow-up]: Remove unused variable Awesome work all! Pushed to master for 19.05 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. missing dependency for 18.05.x wont backport The constraint should have been enforce at higher level than template, see bug 23774. |