When biblionumber is in the opachiddenitems preference the query to find hidden items dies: DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'biblionumber' in where clause is ambiguous [for Statement "SELECT `me`.`itemnumber`, `me`.`biblionumber`, `me`.`biblioitemnumber`, `me`.`barcode`, `me`.`dateaccessioned`, `me`.`booksellerid`, `me`.`homebranch`, `me`.`price`, `me`.`replacementprice`, `me`.`replacementpricedate`, `me`.`datelastborrowed`, `me`.`datelastseen`, `me`.`stack`, `me`.`notforloan`, `me`.`damaged`, `me`.`damaged_on`, `me`.`itemlost`, `me`.`itemlost_on`, `me`.`withdrawn`, `me`.`withdrawn_on`, `me`.`itemcallnumber`, `me`.`coded_location_qualifier`, `me`.`issues`, `me`.`renewals`, `me`.`reserves`, `me`.`restricted`, `me`.`itemnotes`, `me`.`itemnotes_nonpublic`, `me`.`holdingbranch`, `me`.`timestamp`, `me`.`location`, `me`.`permanent_location`, `me`.`onloan`, `me`.`cn_source`, `me`.`cn_sort`, `me`.`ccode`, `me`.`materials`, `me`.`uri`, `me`.`itype`, `me`.`more_subfields_xml`, `me`.`enumchron`, `me`.`copynumber`, `me`.`stocknumber`, `me`.`new_status`, `me`.`exclude_from_local_holds_priority`, `branchtransfers`.`branchtransfer_id`, `branchtransfers`.`itemnumber`, `branchtransfers`.`daterequested`, `branchtransfers`.`datesent`, `branchtransfers`.`frombranch`, `branchtransfers`.`datearrived`, `branchtransfers`.`datecancelled`, `branchtransfers`.`tobranch`, `branchtransfers`.`comments`, `branchtransfers`.`reason`, `branchtransfers`.`cancellation_reason`, `reserves`.`reserve_id`, `reserves`.`borrowernumber`, `reserves`.`reservedate`, `reserves`.`biblionumber`, `reserves`.`branchcode`, `reserves`.`desk_id`, `reserves`.`notificationdate`, `reserves`.`reminderdate`, `reserves`.`cancellationdate`, `reserves`.`cancellation_reason`, `reserves`.`reservenotes`, `reserves`.`priority`, `reserves`.`found`, `reserves`.`timestamp`, `reserves`.`itemnumber`, `reserves`.`waitingdate`, `reserves`.`expirationdate`, `reserves`.`lowestPriority`, `reserves`.`suspend`, `reserves`.`suspend_until`, `reserves`.`itemtype`, `reserves`.`item_level_hold`, `reserves`.`non_priority` FROM `items` `me` LEFT JOIN `branchtransfers` `branchtransfers` ON `branchtransfers`.`itemnumber` = `me`.`itemnumber` LEFT JOIN `reserves` `reserves` ON `reserves`.`itemnumber` = `me`.`itemnumber` WHERE ( ( ( `biblionumber` NOT IN ( ?, ?, ?, ?, ?, ? ) OR `biblionumber` IS NULL ) AND `itemlost` = ? AND ( `itype` NOT IN ( ?, ? ) OR `itype` IS NULL ) AND ( `location` NOT IN ( ?, ? ) OR `location` IS NULL ) AND `me`.`biblionumber` = ? AND 1=1 ) ) ORDER BY `me`.`itemnumber`"
Created attachment 129101 [details] [review] Bug 29802: add 'me' to filter_by_visible_in_opac query This patch prefixes all of the fields in OpacHiddenItems with "me." before searching. Unit tests added to cover this case1 To test: 1 - Create a public list 2 - Set OpacHiddenItems to: biblionumber: [1] 3 - Attempt to view list in OPAC 4 - Booom 5 - Aply patch 6 - Reload list 7 - Success
Indeed, this comes from C4::XSLT::buildKohaItemsNamespace : $items_rs->search( $query, { prefetch => [ 'branchtransfers', 'reserves' ] } );
Created attachment 131099 [details] [review] Bug 29802: add 'me' to filter_by_visible_in_opac query This patch prefixes all of the fields in OpacHiddenItems with "me." before searching. Unit tests added to cover this case1 To test: 1 - Create a public list 2 - Set OpacHiddenItems to: biblionumber: [1] 3 - Attempt to view list in OPAC 4 - Booom 5 - Aply patch 6 - Reload list 7 - Success Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Please show that it breaks in your test. And that your change resolves that. And tell me what this does: + my $mocked_context = Test::MockModule->new('C4::Context'); + $mocked_context->mock( 'yaml_preference', sub { + return { biblionumber => [ $biblio2->biblionumber ] }; + }); I think it is useless code.
Created attachment 131911 [details] [review] Bug 29802: Unit tests prove -v t/db_dependent/Koha/Items.t prove -v t/db_dependent/XSLT.t
Created attachment 131912 [details] [review] Bug 29802: add 'me' to filter_by_visible_in_opac query This patch prefixes all of the fields in OpacHiddenItems with "me." before searching. Unit tests added to cover this case1 To test: 1 - Create a public list 2 - Set OpacHiddenItems to: biblionumber: [1] 3 - Attempt to view list in OPAC 4 - Booom 5 - Aply patch 6 - Reload list 7 - Success Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(In reply to Marcel de Rooy from comment #4) > Please show that it breaks in your test. And that your change resolves that. Tests updated and moved to own patch > > And tell me what this does: > + my $mocked_context = Test::MockModule->new('C4::Context'); > + $mocked_context->mock( 'yaml_preference', sub { > + return { biblionumber => [ $biblio2->biblionumber ] }; > + }); > > I think it is useless code. Looks like you are correct - copied from Items.t - seems to have been done for convenience there, mocking pref directly works
Created attachment 131970 [details] [review] Bug 29802: Unit tests prove -v t/db_dependent/Koha/Items.t prove -v t/db_dependent/XSLT.t Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 131971 [details] [review] Bug 29802: add 'me' to filter_by_visible_in_opac query This patch prefixes all of the fields in OpacHiddenItems with "me." before searching. Unit tests added to cover this case1 To test: 1 - Create a public list 2 - Set OpacHiddenItems to: biblionumber: [1] 3 - Attempt to view list in OPAC 4 - Booom 5 - Aply patch 6 - Reload list 7 - Success Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Sorry Marcel, I didn't you were assigned as QA contact here. Feel free to switch it back to NQA if you want to have another look.
(In reply to Jonathan Druart from comment #10) > Sorry Marcel, I didn't you were assigned as QA contact here. Feel free to > switch it back to NQA if you want to have another look. No reason to :)
Whaoo what a complex programing paradox ;) great job and great unit tests
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Pushed to 21.11.x for 21.11.05