I chose to split out the inventory tool additions from the bundles bug.. that bug was already big enough ;)
Created attachment 125207 [details] [review] Bug 29099: Add bundle support to inventory tool
Created attachment 140898 [details] [review] Bug 29099: Add bundle support to inventory tool Test plan: 1. Create an item bundle (see bug 28854 comment 458) containing 2 items with barcodes B1 and B2 2. Go to the inventory tool 3. Enter B1 inside the barcode list 4. Select the item bundle in "Item location filters" fieldset 5. Submit the form In the result table you should see B2 with the problem "Missing (not scanned)"
Patch rebased and reworked to work with bug 28854 Also added unit tests for GetItemsForInventory
Thanks for your work here Julian, I'm so bad at remembering unit tests at the moment :(. Is your customer intending to test and sign this one off.. We don't really use the inventory at all here, so although I'm reasonably confident this achieves what's required, it would be great to hear from someone using it in earnest. It would be great to see this go in within this cycle to have the feature complete at release time.
Created attachment 146009 [details] [review] Bug 29099: Add bundle support to inventory tool Test plan: 1. Create an item bundle (see bug 28854 comment 458) containing 2 items with barcodes B1 and B2 2. Go to the inventory tool 3. Enter B1 inside the barcode list 4. Select the item bundle in "Item location filters" fieldset 5. Submit the form In the result table you should see B2 with the problem "Missing (not scanned)" Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
I'd have loved to have seen a customer signoff here.. but I'm confident enough in the code and have tested it's working.. So I'm signing off myself.
(In reply to Martin Renvoize from comment #6) > I'd have loved to have seen a customer signoff here.. but I'm confident > enough in the code and have tested it's working.. So I'm signing off myself. It is not really a trivial one. But lets see. Looking here now
I think that this needs a bit more attention still but did not yet finish qa on it. Came across this exception with inventory form: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: 'koha_myclone.me.biblionumber' isn't in GROUP BY at /usr/share/koha/Koha/Objects.pm line 394 at /usr/share/perl5/DBIx/Class/Exception.pm line 77 Resuming next week. Fine for me to keep it BLOCKED now.
[1] The following constructs look quite complicated and does not work: -my $schema = Koha::Database->new()->schema(); -my $item_bundles_rs = Koha::Items->search( - { - 'item_bundles_hosts.item' => { '!=' => undef }, - }, - { - join => 'item_bundles_hosts', - group_by => 'itemnumber', - } -); - -my @item_bundles = $item_bundles_rs->as_list; I am adding a follow-up as proposal to change this code.
General observation: + $item_bundle->_result->add_to_item_bundles_hosts({ item => $item1->itemnumber }); + $item_bundle->_result->add_to_item_bundles_hosts({ item => $item2->itemnumber }); If those were the first lines introducing ->result->add_to_$rel, I might say something about Koha objects. But apparently the door has been opened already for more of those ;)
Created attachment 148550 [details] [review] Bug 29099: Add Koha objects for item bundle(s)
The choice for the column 'item' on the parent report present difficulties here when you want to go from item to item if you understand what I mean..
Created attachment 148555 [details] [review] Bug 29099: Add bundle support to inventory tool Test plan: 1. Create an item bundle (see bug 28854 comment 458) containing 2 items with barcodes B1 and B2 2. Go to the inventory tool 3. Enter B1 inside the barcode list 4. Select the item bundle in "Item location filters" fieldset 5. Submit the form In the result table you should see B2 with the problem "Missing (not scanned)" Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 148556 [details] [review] Bug 29099: Fix item bundle fetch in inventory script Using the newly created Koha objects.
(In reply to Martin Renvoize from comment #6) > I'd have loved to have seen a customer signoff here.. but I'm confident > enough in the code and have tested it's working.. So I'm signing off myself. The inventory form crashed. Please ask for another signoff when submitting adjustments now.
The last follow-up seems to fix the crash. Another point: The biblio title in the inventory combo may appear double when you have multiple bundles on the same parent biblio. This is confusing. The bundle should probably have its own name?
If you agree with the two new patches, please switch to NSO. Moving it to FQA now first.
Ping Martin
Ping Martin :)
Applying: Bug 29099: Add bundle support to inventory tool Using index info to reconstruct a base tree... M C4/Items.pm M koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt M tools/inventory.pl Falling back to patching base and 3-way merge... Auto-merging tools/inventory.pl Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/tools/inventory.tt Auto-merging C4/Items.pm error: Failed to merge in the changes. Patch failed at 0001 Bug 29099: Add bundle support to inventory tool
Created attachment 176237 [details] [review] Bug 29099: Add bundle support to inventory tool
Created attachment 176238 [details] [review] Bug 29099: Add Koha objects for item bundle(s)
Created attachment 176239 [details] [review] Bug 29099: Fix item bundle fetch in inventory script Using the newly created Koha objects.