Bug 30432 - get_items_that_can_fill needs to specify table for biblionumbers
Summary: get_items_that_can_fill needs to specify table for biblionumbers
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-01 12:48 UTC by Nick Clemens
Modified: 2023-12-28 20:42 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.05.00,21.11.05


Attachments
Bug 30432: Unit test (1.72 KB, patch)
2022-04-01 12:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30432: Specify 'me' for biblionumber when limiting items (1.10 KB, patch)
2022-04-01 12:51 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 30432: Unit test (1.77 KB, patch)
2022-04-04 11:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30432: Specify 'me' for biblionumber when limiting items (1.15 KB, patch)
2022-04-04 11:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 30432: Unit test (1.84 KB, patch)
2022-04-06 14:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30432: Specify 'me' for biblionumber when limiting items (1.23 KB, patch)
2022-04-06 14:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30432: Fix random failure (1006 bytes, patch)
2022-04-06 14:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 30432: [21.05.x] Specify 'me' for biblionumber when limiting items (2.83 KB, patch)
2022-04-26 12:10 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-04-01 12:48:12 UTC
When not using item_level-itypes i.e. it is set to 'bibliographic record', we join to biblioitems to get the itemtype

This causes ambiguity in the 'filter_by_for_hold' call in 'get_items_that_can_fill' as we only specify 'biblionumber' in our bibs_or_items variable

The error is:
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'biblionumber' in where clause is ambiguous at /kohadevbox/koha/Koha/Objects.pm line 394
Comment 1 Nick Clemens 2022-04-01 12:51:05 UTC
Created attachment 132862 [details] [review]
Bug 30432: Unit test
Comment 2 Nick Clemens 2022-04-01 12:51:09 UTC
Created attachment 132863 [details] [review]
Bug 30432: Specify 'me' for biblionumber when limiting items

To test:
1 - Apply only unit tests
2 - prove t/db_dependent/Koha/Holds.t
3 - It fails
4 - Apply this patch
5 - prove t/db_dependent/Koha/Holds.t
6 - It passes!
Comment 3 Owen Leonard 2022-04-04 11:48:51 UTC
Created attachment 132930 [details] [review]
Bug 30432: Unit test

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2022-04-04 11:48:55 UTC
Created attachment 132931 [details] [review]
Bug 30432: Specify 'me' for biblionumber when limiting items

To test:
1 - Apply only unit tests
2 - prove t/db_dependent/Koha/Holds.t
3 - It fails
4 - Apply this patch
5 - prove t/db_dependent/Koha/Holds.t
6 - It passes!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Jonathan Druart 2022-04-06 14:19:18 UTC
Tests are failing for me

t/db_dependent/Koha/Holds.t .. 5/7 
    #   Failed test 'Items 1, 2, and 5 are available for filling the holds'
    #   at t/db_dependent/Koha/Holds.t line 516.
    #     Structures begin differing at:
    #          $got->[0] = '1006'
    #     $expected->[0] = '1005'
    # Looks like you failed 1 test of 6.
Comment 6 Jonathan Druart 2022-04-06 14:23:50 UTC
This may be a fix, as we don't specify an order by clause in get_items_that_can_fill:

-    is_deeply( [ map { $_->itemnumber } $items->as_list ],
+    is_deeply( [ sort map { $_->itemnumber } $items->as_list ],
Comment 7 Jonathan Druart 2022-04-06 14:35:26 UTC
Created attachment 133039 [details] [review]
Bug 30432: Unit test

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2022-04-06 14:35:30 UTC
Created attachment 133040 [details] [review]
Bug 30432: Specify 'me' for biblionumber when limiting items

To test:
1 - Apply only unit tests
2 - prove t/db_dependent/Koha/Holds.t
3 - It fails
4 - Apply this patch
5 - prove t/db_dependent/Koha/Holds.t
6 - It passes!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2022-04-06 14:35:34 UTC
Created attachment 133041 [details] [review]
Bug 30432: Fix random failure

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Fridolin Somers 2022-04-12 09:42:32 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 11 Kyle M Hall 2022-04-15 11:21:49 UTC
Pushed to 21.11.x for 21.11.05
Comment 12 Andrew Fuerste-Henry 2022-04-25 20:35:23 UTC
I get a bunch of errors from the QA tool after applying these to 21.05. Please rebase for backport:

 FAIL	t/db_dependent/Koha/Holds.t
   FAIL	  valid
		Use of bare << to mean <<"" is deprecated 
		Use of bare << to mean <<"" is deprecated 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_1" requires explicit package name (did you forget to declare "my $itype_1"?) 
		Global symbol "$itype_2" requires explicit package name (did you forget to declare "my $itype_2"?) 
		Global symbol "$item_1" requires explicit package name (did you forget to declare "my $item_1"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_1" requires explicit package name (did you forget to declare "my $itype_1"?) 
		Global symbol "$item_2" requires explicit package name (did you forget to declare "my $item_2"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_1" requires explicit package name (did you forget to declare "my $itype_1"?) 
		Global symbol "$item_3" requires explicit package name (did you forget to declare "my $item_3"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_1" requires explicit package name (did you forget to declare "my $itype_1"?) 
		Global symbol "$item_4" requires explicit package name (did you forget to declare "my $item_4"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_1" requires explicit package name (did you forget to declare "my $itype_1"?) 
		Global symbol "$item_5" requires explicit package name (did you forget to declare "my $item_5"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$itype_2" requires explicit package name (did you forget to declare "my $itype_2"?) 
		Global symbol "$lost" requires explicit package name (did you forget to declare "my $lost"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$withdrawn" requires explicit package name (did you forget to declare "my $withdrawn"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		Global symbol "$notforloan" requires explicit package name (did you forget to declare "my $notforloan"?) 
		Global symbol "$biblio" requires explicit package name (did you forget to declare "my $biblio"?) 
		syntax error 
		    my "
		  (Might be a runaway multi-line << string starting on line 419)
		t/db_dependent/Koha/Holds.t has too many errors.
Comment 13 Nick Clemens 2022-04-26 12:10:25 UTC
Created attachment 133877 [details] [review]
Bug 30432: [21.05.x] Specify 'me' for biblionumber when limiting items

To test:
1 - Apply only unit tests
2 - prove t/db_dependent/Koha/Holds.t
3 - It fails
4 - Apply this patch
5 - prove t/db_dependent/Koha/Holds.t
6 - It passes!

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 30432: Unit test

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Bug 30432: Fix random failure

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>