Let's use Koha::Items!
Created attachment 77699 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq
Created attachment 77700 [details] [review] Bug 21206: Remove GetItem
Created attachment 77701 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later
The whole dependency tree has been pushed to https://gitlab.com/joubu/Koha/commits/bug_21206
This is ready for testing, no test plan provided.
Comment on attachment 77699 [details] [review] Bug 21206: Replace C4::Items::GetItem Review of attachment 77699 [details] [review]: ----------------------------------------------------------------- ::: C4/ILSDI/Services.pm @@ +615,5 @@ > # Add renewal if possible > my @renewal = CanBookBeRenewed( $borrowernumber, $itemnumber ); > if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber ); } > > + my $issue = $item ? $item->checkout : return; # FIXME should be handled This does not return.... ::: labels/label-edit-batch.pl @@ +87,4 @@ > my @numbers_list = split /\n/, $number_list; # Entries are effectively passed in as a <cr> separated list > foreach my $number (@numbers_list) { > $number =~ s/\r$//; # strip any naughty return chars > + if( $number_type eq "itemnumber" && Koha::ITems->find($number) ) { ITems -> Items
Also QA tool fails: FAIL acqui/orderreceive.pl FAIL valid Global symbol "@items" requires explicit package name acqui/orderreceive.pl had compilation errors. FAIL catalogue/getitem-ajax.pl FAIL valid catalogue/getitem-ajax.pl had compilation errors. Global symbol "$item_unblessed" requires explicit package name FAIL misc/recreateIssueStatistics.pl FAIL forbidden patterns forbidden pattern: tab char (line 124) FAIL t/db_dependent/Holds/DisallowHoldIfItemsAvailable.t FAIL valid syntax error
Created attachment 78080 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq
Created attachment 78081 [details] [review] Bug 21206: Remove GetItem
Created attachment 78082 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later
Comment on attachment 78080 [details] [review] Bug 21206: Replace C4::Items::GetItem Review of attachment 78080 [details] [review]: ----------------------------------------------------------------- ::: C4/ILSDI/Services.pm @@ +614,5 @@ > if ( $renewal[0] ) { AddRenewal( $borrowernumber, $itemnumber ); } > > + return unless $item; # FIXME should be handled > + > + my $issue = $item->checkout; I see still little problem here: If item exists and is not checked out -> boom The check and return on item is done on line 610 here, so here should be "return unless $issue;"
Created attachment 78153 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq
Created attachment 78154 [details] [review] Bug 21206: Remove GetItem
Created attachment 78155 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later
Created attachment 82831 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82832 [details] [review] Bug 21206: Remove GetItem Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82833 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 82834 [details] [review] Bug 21206: (follow-up) Fix Circulation.t Test plan: prove t/db_dependent/Circulation.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84006 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84007 [details] [review] Bug 21206: Remove GetItem Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84008 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84009 [details] [review] Bug 21206: (follow-up) Fix Circulation.t Test plan: prove t/db_dependent/Circulation.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 84010 [details] [review] Bug 21206: (QA follow-up) Rebase problem and leftover mocked GetItem Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I reviewed the patches, everything looks good. I fixed minor issues too. Will mark PQA once I cover all the changes on the UI too.
Created attachment 84184 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84185 [details] [review] Bug 21206: Remove GetItem Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84186 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84187 [details] [review] Bug 21206: (follow-up) Fix Circulation.t Test plan: prove t/db_dependent/Circulation.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 84188 [details] [review] Bug 21206: (QA follow-up) Rebase problem and leftover mocked GetItem Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Needs a rebase after removal of dropbox mode
Created attachment 85163 [details] [review] Bug 21206: Replace C4::Items::GetItem Note: This is here for information purpose, feel free to test it if you wan to play with it. TODO: C4::Reserves::_get_itype is not longer in use No more GetItem must be returned by: git grep GetItem|grep -v GetItemsAvailableToFillHoldRequestsForBib|grep -v GetItemsForInventory|grep -v GetItemsInfo|grep -v GetItemsLocationInfo|grep -v GetItemsInCollection|grep -v GetItemCourseReservesInfo|grep -v GetItemnumbersFromOrder|grep -v GetItemSearchField|grep -v GetItemTypesCategorized|grep -v GetItemNumbersFromImportBatch|cut -d':' -f1|sort|uniq Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85164 [details] [review] Bug 21206: Remove GetItem Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85165 [details] [review] Bug 21206: Fix Returns.t we are calling several times effective_itemtype Must be fixed later Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85166 [details] [review] Bug 21206: (follow-up) Fix Circulation.t Test plan: prove t/db_dependent/Circulation.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85167 [details] [review] Bug 21206: (QA follow-up) Rebase problem and leftover mocked GetItem Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 85168 [details] [review] Bug 21206: Fix SwitchOnSiteCheckouts.t Fix conflict with commit f8544ba5799c457057d42aefc63c88e15a08b096 Bug 21999: Move attributes to a variable to not dup them Thanks tests!
Awesome work all! Pushed to master for 19.05
Enhancement will not be backported to 18.11.x series.