my $barcode = GetBarcodeFromItemnumber($itemnumber); will become: my $barcode = Koha::Items->find($itemnumber)->barcode;
Created attachment 77589 [details] [review] Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls Those calls to C4::Items::GetBarcodeFromItemnumber can be replaced with my $barcode = Koha::Items->find($itemnumber)->barcode; But if we are not sure that the item exists, we should test the return of ->find before ->barcode Test plan: - Edit an item - Check an item in - Test SIP - I do not really know how to trigger that code, apparently misc/sip_cli_emulator.pl does not deal with holds. Any ideas?
Created attachment 77590 [details] [review] Bug 21184: Remove GetBarcodeFromItemnumber Test plan: git grep GetBarcodeFromItemnumber should not return anything
Comment on attachment 77589 [details] [review] Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls Review of attachment 77589 [details] [review]: ----------------------------------------------------------------- ::: C4/SIP/ILS/Patron.pm @@ +316,5 @@ > my $self = shift; > my $item_arr = $self->x_items('hold_items', @_); > foreach my $item (@{$item_arr}) { > + my $item = Koha::Items->find($item->{itemnumber}); > + $item->{barcode} = $item ? $item->barcode : undef; I think there is too many variables called $item ;)
Created attachment 77955 [details] [review] Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls Those calls to C4::Items::GetBarcodeFromItemnumber can be replaced with my $barcode = Koha::Items->find($itemnumber)->barcode; But if we are not sure that the item exists, we should test the return of ->find before ->barcode Test plan: - Edit an item - Check an item in - Test SIP - I do not really know how to trigger that code, apparently misc/sip_cli_emulator.pl does not deal with holds. Any ideas?
Created attachment 77956 [details] [review] Bug 21184: Remove GetBarcodeFromItemnumber Test plan: git grep GetBarcodeFromItemnumber should not return anything
(In reply to Josef Moravec from comment #3) > Comment on attachment 77589 [details] [review] [review] > I think there is too many variables called $item ;) Haha yes, good catch :)
You could test this using something like this: misc/sip_cli_emulator.pl -a localhost -p 6001 -su <login> -sp <passsword> -l <library_code> --patron <borrowernumber> -s "Y " -m patron_information
Created attachment 77961 [details] [review] Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls Those calls to C4::Items::GetBarcodeFromItemnumber can be replaced with my $barcode = Koha::Items->find($itemnumber)->barcode; But if we are not sure that the item exists, we should test the return of ->find before ->barcode Test plan: - Edit an item - Check an item in - Test SIP - I do not really know how to trigger that code, apparently misc/sip_cli_emulator.pl does not deal with holds. Any ideas? Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 77962 [details] [review] Bug 21184: Remove GetBarcodeFromItemnumber Test plan: git grep GetBarcodeFromItemnumber should not return anything Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Created attachment 77963 [details] [review] Bug 21184: (follow-up) Fix adding barcode to waiting hold item in SIP Test plan: 1) Set up your SIP: https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup 2) Run: misc/sip_cli_emulator.pl -a localhost -p 6001 -su <login> -sp <passsword> -l <library_code> --patron <borrowernumber> -s "Y " -m patron_information the -s parameter says what kind of additional information you want to see 3) Check the response: if the requested patron has waiting hold, there is field "AS" with the barcode of waiting item The right response looks like this: 64 00120180817 052434000100000000000000000000AOCPL|AA1|AEKoha Admin|BLY|CC5|AS39999000004410|PCS|PIY|AFGreetings from Koha. | More reading ;) http://multimedia.3m.com/mws/media/355361O/sip2-protocol.pdf Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
QA: Looking here
Created attachment 78111 [details] [review] Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls Those calls to C4::Items::GetBarcodeFromItemnumber can be replaced with my $barcode = Koha::Items->find($itemnumber)->barcode; But if we are not sure that the item exists, we should test the return of ->find before ->barcode Test plan: - Edit an item - Check an item in - Test SIP - I do not really know how to trigger that code, apparently misc/sip_cli_emulator.pl does not deal with holds. Any ideas? Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78112 [details] [review] Bug 21184: Remove GetBarcodeFromItemnumber Test plan: git grep GetBarcodeFromItemnumber should not return anything Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 78113 [details] [review] Bug 21184: (follow-up) Fix adding barcode to waiting hold item in SIP Test plan: 1) Set up your SIP: https://wiki.koha-community.org/wiki/Koha_SIP2_server_setup 2) Run: misc/sip_cli_emulator.pl -a localhost -p 6001 -su <login> -sp <passsword> -l <library_code> --patron <borrowernumber> -s "Y " -m patron_information the -s parameter says what kind of additional information you want to see 3) Check the response: if the requested patron has waiting hold, there is field "AS" with the barcode of waiting item The right response looks like this: 64 00120180817 052434000100000000000000000000AOCPL|AA1|AEKoha Admin|BLY|CC5|AS39999000004410|PCS|PIY|AFGreetings from Koha. | More reading ;) http://multimedia.3m.com/mws/media/355361O/sip2-protocol.pdf Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Awesome work all! Pushed to master for 18.11
Enhancement, will not be backported to 18.05.x series.