Bug 21184 - C4::Items - Remove GetBarcodeFromItemnumber
Summary: C4::Items - Remove GetBarcodeFromItemnumber
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 21183
Blocks: 18252 21206 21201 21471
  Show dependency treegraph
 
Reported: 2018-08-08 18:32 UTC by Jonathan Druart
Modified: 2018-10-03 15:27 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls (4.16 KB, patch)
2018-08-08 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21184: Remove GetBarcodeFromItemnumber (1.25 KB, patch)
2018-08-08 18:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls (4.17 KB, patch)
2018-08-16 19:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21184: Remove GetBarcodeFromItemnumber (1.25 KB, patch)
2018-08-16 19:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls (4.21 KB, patch)
2018-08-17 05:36 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21184: Remove GetBarcodeFromItemnumber (1.30 KB, patch)
2018-08-17 05:36 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21184: (follow-up) Fix adding barcode to waiting hold item in SIP (1.56 KB, patch)
2018-08-17 05:36 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 21184: Replace C4::Items::GetBarcodeFromItemnumber calls (4.30 KB, patch)
2018-08-24 08:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21184: Remove GetBarcodeFromItemnumber (1.40 KB, patch)
2018-08-24 08:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21184: (follow-up) Fix adding barcode to waiting hold item in SIP (1.66 KB, patch)
2018-08-24 08:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2018-08-08 18:32:04 UTC
my $barcode = GetBarcodeFromItemnumber($itemnumber);

will become:

my $barcode = Koha::Items->find($itemnumber)->barcode;
Comment 1 Jonathan Druart 2018-08-08 18:56:29 UTC
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?
Comment 2 Jonathan Druart 2018-08-08 18:56:33 UTC
Created attachment 77590 [details] [review]
Bug 21184: Remove GetBarcodeFromItemnumber

Test plan:
  git grep GetBarcodeFromItemnumber
should not return anything
Comment 3 Josef Moravec 2018-08-16 16:05:11 UTC
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 ;)
Comment 4 Jonathan Druart 2018-08-16 19:45:48 UTC
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?
Comment 5 Jonathan Druart 2018-08-16 19:45:52 UTC
Created attachment 77956 [details] [review]
Bug 21184: Remove GetBarcodeFromItemnumber

Test plan:
  git grep GetBarcodeFromItemnumber
should not return anything
Comment 6 Jonathan Druart 2018-08-16 19:46:48 UTC
(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 :)
Comment 7 Josef Moravec 2018-08-17 05:36:01 UTC
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
Comment 8 Josef Moravec 2018-08-17 05:36:38 UTC
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>
Comment 9 Josef Moravec 2018-08-17 05:36:43 UTC
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>
Comment 10 Josef Moravec 2018-08-17 05:36:49 UTC
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>
Comment 11 Marcel de Rooy 2018-08-24 08:10:11 UTC
QA: Looking here
Comment 12 Marcel de Rooy 2018-08-24 08:16:55 UTC
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>
Comment 13 Marcel de Rooy 2018-08-24 08:16:59 UTC
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>
Comment 14 Marcel de Rooy 2018-08-24 08:17:04 UTC
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>
Comment 15 Nick Clemens 2018-08-31 12:59:37 UTC
Awesome work all!

Pushed to master for 18.11
Comment 16 Martin Renvoize 2018-09-05 14:33:12 UTC
Enhancement, will not be backported to 18.05.x series.