https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #1 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I can't confirm on master. Which version did you test with Emmi? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #3 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #1)

I can't confirm on master. Which version did you test with Emmi?

I tested on both master and 21.11 which we use. It seems this happens because in batchMod.pl line 269 we don't pass biblionumber as parameter to Koha::UI::Form::Builder::Item->new and thus Koha fails to fetch biblios marc record. However when I tested this on "clean" master database where default framework has no values in it error wasn't raised. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |emmi.takkinen@koha-suomi.fi |ity.org | Status|NEW |Needs Signoff --- Comment #5 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Created attachment 138124 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=138124&action=edit Bug 31154: Pass biblionumber to Koha::UI::Form::Builder::Item->new from batchMod.pl to prevent error 500 When one tries to batch mod items and has "Use default values" checkbox checked on and their default framework contains default values, Koha dies on error 500. In logs it reads: Can't call method "field" on an undefined value at .../Koha/Koha/UI/Form/Builder/Item.pm line 164. This happens because in batchMod.pl line 269 we don't pass biblionumber as parameter to Koha::UI::Form::Builder::Item->new and thus Koha fails to fetch biblios marc record. To test: 1. Update default framework 952 and add a default value * Example: Add REF as default for 952$8 with standard sample data 2. Find some item(s) to modify 3. Provide barcodes of those item(s) and check checkbox "Use default values" 4. Hit "Continue" => error 500 is raised 5. Apply patch. 6. Repeat steps 3. and 4. Sponsored-by: Koha-Suomi Oy -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Emmanuel Bétemps <e.betemps@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |e.betemps@gmail.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #8 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Caroline Cyr La Rose from comment #7)

I cannot reproduce the problematic behaviour either in a sandbox nor on my local dev install with the steps from comment 4. In all cases, I am brought to the batch item modification page without problem and am able to launch the batch modification.

I do get a 500 error when I click on the "View detail of the enqueued job" link to see the background job.

I tried with and without the patch and get the same behaviour...

Hmm, it could also be that error is actually raised when subfield has authorised value linked to it? However at least for me saving authorised value to subfield is currently broken on master (see bug 31238) so it might be little tricky to reproduce. I'm unable to reproduce 500 error by clicking "View detail of the enqueued job". Do you know what logs say when this happens? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #10 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- (In reply to Katrin Fischer from comment #9)

I can no longer replicate the problem with the test plan that worked before :(

Hmm, how about if you save authorised value for 952$8? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Amanda Campbell <acampbell@hmcpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |acampbell@hmcpl.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #13 from Emmi Takkinen <emmi.takkinen@koha-suomi.fi> --- Well this is confusing. I was able to repeat this bug again by adding a value in syspref "itemcallnumber" e.g. 084a. This triggers if-statement in Koha/Koha/UI/Form/Builder/Item.pm starting from line 151 which eventually leads Koha to crash on line 165. It seems that having a default value in default framework had nothing to with this at all since I too am unable to trigger this bug again with just default values. Here's an updated test plan: To test: 1. Confirm you have/set value to syspref "itemcallnumber" e.g. 084a 2. Find some item(s) to modify 3. Provide barcodes of those item(s) and check checkbox "Use default values" 4. Hit "Continue" => error 500 is raised -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Emmi Takkinen <emmi.takkinen@koha-suomi.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #140518|0 |1 is obsolete| | --- Comment #15 from David Nind <david@davidnind.com> --- Created attachment 140602 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140602&action=edit Bug 31154: Pass biblionumber to Koha::UI::Form::Builder::Item->new from batchMod.pl to prevent error 500 When one tries to batch mod items and has "Use default values" checkbox checked on and they have value in syspref "itemcallnumber", Koha dies on error 500. In logs it reads: Can't call method "field" on an undefined value at .../Koha/Koha/UI/Form/Builder/Item.pm line 164. This happens because in batchMod.pl line 269 we don't pass biblionumber as parameter to Koha::UI::Form::Builder::Item->new and thus Koha fails to fetch biblios marc record. To test: 1. Confirm you have/set value to syspref "itemcallnumber" e.g. 084a 2. Find some item(s) to modify 3. Provide barcodes of those item(s) and check checkbox "Use default values" 4. Hit "Continue" => error 500 is raised 5. Apply patch. 6. Repeat steps 3. and 4. => no error is raised Sponsored-by: Koha-Suomi Oy Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #17 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- =head3 new my $form = Koha::UI::Form::Builder::Item->new( { biblionumber => $biblionumber, item => $current_item, } ); Constructor. biblionumber should be passed if we are creating a new item. For edition, an hashref representing the item to edit item must be passed. Hmm. Why did the author not just pick the biblionumber of the item? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- We are really wasting resources here. First we do: my %itemdata = map { lc($_->{barcode}) => $_->{itemnumber} } @{ Koha::Items->search({ barcode => { -in => \@barcodelist } }, { columns => [ 'itemnumber', 'barcode' ] } )->unblessed }; Lets discard that info. And later do: sub build_table { my ( $self, $params ) = @_; my %itemnumbers_to_idx = map { $self->{itemnumbers}->[$_] => $_ } 0..$#{$self->{itemnumbers}}; my $items = Koha::Items->search( { itemnumber => $self->{itemnumbers} } ); We are looking up all items again ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|BLOCKED |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #21 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 140695 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=140695&action=edit Bug 31154: (QA follow-up) Fix UI form builder The change in batchMod.pl is not needed when we fix the builder. But we could at least add a comment there! Note that passing the biblionumber of the first item does not make sense since we are modifying items from several biblio records normally. We most probably do not want this MARC record's itemcallnumber (via the syspref) in all our items. Test plan: Test batchMod with and without the Populate fields checkbox. Fill syspref 'itemcallnumber' with e.g. 084a. Check Populate, and try again. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #24 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Marcel de Rooy from comment #22)

Jonathan, left comment17 and comment19 for your consideration :)

Why didn't you add that on the original bug report? :) (In reply to Marcel de Rooy from comment #17)

=head3 new

my $form = Koha::UI::Form::Builder::Item->new( { biblionumber => $biblionumber, item => $current_item, } );

Constructor. biblionumber should be passed if we are creating a new item. For edition, an hashref representing the item to edit item must be passed.

Hmm. Why did the author not just pick the biblionumber of the item?

The item is not created yet, and so does not belongs to the biblio yet. But I agree we could add it to the item hashref, remove the param and use item->{biblionumber} instead. (In reply to Marcel de Rooy from comment #19)

We are really wasting resources here.

First we do: my %itemdata = map { lc($_->{barcode}) => $_->{itemnumber} } @{ Koha::Items->search({ barcode => { -in => \@barcodelist } }, { columns => [ 'itemnumber', 'barcode' ] } )->unblessed };

Lets discard that info. And later do:

sub build_table { my ( $self, $params ) = @_; my %itemnumbers_to_idx = map { $self->{itemnumbers}->[$_] => $_ } 0..$#{$self->{itemnumbers}}; my $items = Koha::Items->search( { itemnumber => $self->{itemnumbers} } );

We are looking up all items again ;)

From controller we are only fetching itemnumber and barcode. That's extra processing for the "max items to display" feature. From the module we are fetching everything. I am expecting the query from the controller to be very fast, using DBMS indexes. What did we do before 28445 and friends?

-- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #25)

This changes forgot to add unit tests btw.

We have t/db_dependent/Koha/UI/Form/Builder/Item.t, but generate_subfield_form is not tested directly. Cannot request imo to add tests for trivial changes where the original author did not add tests himself. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #28 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 141974 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=141974&action=edit Bug 31154: Avoid unecessary loop We don't need to loop over the subfield and enter this block if there is no record. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 --- Comment #30 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Thanks. Looking here now -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #141974|0 |1 is obsolete| | --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 142052 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=142052&action=edit Bug 31154: Avoid unecessary loop We don't need to loop over the subfield and enter this block if there is no record. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #33 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Tomás: Last two patches please. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|additional_work_needed | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Arthur Suzuki <arthur.suzuki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable CC| |arthur.suzuki@biblibre.com Version(s)|22.11.00, 22.05.07 |22.11.00, 22.05.07, released in| |21.11.14 --- Comment #37 from Arthur Suzuki <arthur.suzuki@biblibre.com> --- applied to 21.11 for 21.11.14 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=31154 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #39 from Aleisha Amohia <aleisha@catalyst.net.nz> --- *** Bug 32446 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.