|
Description
Lari Taskula
2017-02-07 14:37:03 UTC
Created attachment 59979 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Created attachment 59980 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Created attachment 59981 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Created attachment 59982 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Created attachment 60009 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 60010 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 60011 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> I'd suggest to swap $to, $from in the parameters.
->can_be_transferred( $from, $to )
sounds more logical.
When could even pass $hashref { from => $from, to => $to } to avoid ambiguity.
I am not sure the tests on the existence of the libraries are necessary.
(In reply to Josef Moravec from comment #6) > Created attachment 60010 [details] [review] [review] > Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit > functions Do you plan to replace this subroutine? (In reply to Jonathan Druart from comment #8) > I'd suggest to swap $to, $from in the parameters. > ->can_be_transferred( $from, $to ) > sounds more logical. > When could even pass $hashref { from => $from, to => $to } to avoid > ambiguity. I used this particular order because one may leave $from undefined, in which case the item's holdingbranch will be used as $from, and the typical use case would be simply $item->can_be_transferred($to). I agree with your point for the advantage of using hashref. (In reply to Jonathan Druart from comment #9) > (In reply to Josef Moravec from comment #6) > > Created attachment 60010 [details] [review] [review] [review] > > Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit > > functions > > Do you plan to replace this subroutine? No. I would like to replace all of them and write proper tests for that, but unfortunately I don't have time to work with that now. Created attachment 60116 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Created attachment 60117 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Created attachment 60782 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 60783 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Created attachment 60784 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Rebased on top of current master. Created attachment 63683 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63684 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63685 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63686 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 63687 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Created attachment 66724 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Very easy conflict with bug 18292 fixed. Created attachment 67049 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67050 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67051 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67052 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67053 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67054 [details] [review] Bug 18072: (QA followup) Remove warning from tests This patch removes a warning from Items.t due to bad parameters. It also makes the tests use Test::Exception. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 67209 [details] [review] Bug 18072: Only accept Koha::Library in parameters I do not think we should allowed branchode and Koha::Library objects, it adds confusion in callers (we never know if we have a branchcode of a library object). (In reply to Jonathan Druart from comment #30) > Created attachment 67209 [details] [review] [review] > Bug 18072: Only accept Koha::Library in parameters > > I do not think we should allowed branchode and Koha::Library objects, it > adds confusion in callers (we never know if we have a branchcode of a > library object). Does this patch make sense? Created attachment 76282 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76283 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76284 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76285 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76286 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76287 [details] [review] Bug 18072: (QA followup) Remove warning from tests This patch removes a warning from Items.t due to bad parameters. It also makes the tests use Test::Exception. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 76288 [details] [review] Bug 18072: Only accept Koha::Library in parameters I do not think we should allowed branchode and Koha::Library objects, it adds confusion in callers (we never know if we have a branchcode of a library object). (In reply to Jonathan Druart from comment #31) > (In reply to Jonathan Druart from comment #30) > > Created attachment 67209 [details] [review] [review] [review] > > Bug 18072: Only accept Koha::Library in parameters > > > > I do not think we should allowed branchode and Koha::Library objects, it > > adds confusion in callers (we never know if we have a branchcode of a > > library object). > > Does this patch make sense? Yes, yes it does ;) Created attachment 76292 [details] [review] Bug 18072: Add Koha objects for Branch Transfer Limit This patch also fixes documentation in Koha/Item/Transfer.pm and Koha/Item/Transfers.pm. Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76293 [details] [review] Bug 18072: Add deprecation warning to C4::Circulation Branch Transfer Limit functions Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76294 [details] [review] Bug 18072: Add Koha::Item->can_be_transferred This patch adds a new method Koha::Item->can_be_transferred. Includes unit test. To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76295 [details] [review] Bug 18072: (QA-follow-up) let Koha::Item->can_be_transferred take HASHref as param This patch changes Koha::Item->can_be_transferred to accept a HASHref as follows: $item->can_be_transferred({ to => $library, from => $library2 }) To test: 1. prove t/db_dependent/Koha/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76296 [details] [review] Bug 18072: Add Koha::Biblio->can_be_transferred This patch adds a new method Koha::Biblio->can_be_transferred. The method checks if at least one of the item of that biblio can be transferred to desired location. This method will be useful for building a smarter pickup location list for holds, because we will be able to hide those libraries to which none of the items of this biblio can be transferred to due to branch transfer limits (see Bug 7614). To test: 1. prove t/db_dependent/Koha/Biblios.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76297 [details] [review] Bug 18072: (QA followup) Remove warning from tests This patch removes a warning from Items.t due to bad parameters. It also makes the tests use Test::Exception. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 76298 [details] [review] Bug 18072: Only accept Koha::Library in parameters I do not think we should allowed branchode and Koha::Library objects, it adds confusion in callers (we never know if we have a branchcode of a library object). Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Awesome work all! Pushed to master for 18.11 Enhancement, will not be backported to 18.05.x series. |