Bug 18295 - C4::Items - Remove get_itemnumbers_of
Summary: C4::Items - Remove get_itemnumbers_of
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 18093
Blocks: 18252 18279 18296
  Show dependency treegraph
 
Reported: 2017-03-17 16:19 UTC by Jonathan Druart
Modified: 2018-06-04 20:10 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 18295: C4::Items - get_itemnumbers_of (9.45 KB, patch)
2017-03-17 17:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18295: C4::Items - get_itemnumbers_of (9.44 KB, patch)
2017-05-04 14:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18295: C4::Items - get_itemnumbers_of (9.56 KB, patch)
2017-05-31 18:51 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18295: C4::Items - get_itemnumbers_of (9.59 KB, patch)
2017-06-02 07:14 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 18295: C4::Items - get_itemnumbers_of (9.58 KB, patch)
2017-06-02 07:15 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 2017-03-17 16:19:20 UTC

    
Comment 1 Jonathan Druart 2017-03-17 17:27:03 UTC
Created attachment 61225 [details] [review]
Bug 18295: C4::Items - get_itemnumbers_of

The code from scripts and subroutines using this subroutine was not very
elegant. Most of the time the code was unnecessarily complex.
This patch removes this subroutine and adapt the code to use
Koha::Items instead.

1. C4::Items::get_hostitemnumbers_of
I did not understand why the code was so complicated, it seems that we
only want to know if a given item has a given biblionumber
2. cataloguing/merge.pl
We want to retrieve the itemnumber for a given biblio.
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items;
3. labels/label-item-search.pl
We want to loop over the items for a given biblio, no need to use
get_itemnumbers_of and GetItemInfosOf.
We just need to use:
  Koha::Items->search({ biblionumber => $biblionumber })
4. reserve/request.pl
We want to retrieve the itemnumbers of the biblio's items
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber');

Test plan:
1.You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
2. Merge two bibliographic records (with items), the resulting record
should contain items from both original records
3. Create a new label batch, edit it.
Add items to this batch ('Add items' button).
Fill the input with a barcode.
You should see all the items of a biblio.
Comment 2 Marcel de Rooy 2017-03-31 15:25:20 UTC
Dependency with 18093 is the other way around
Comment 3 Josef Moravec 2017-05-04 11:18:20 UTC
fatal: sha1 information is lacking or useless (C4/Items.pm)
Comment 4 Jonathan Druart 2017-05-04 14:17:10 UTC
Created attachment 63111 [details] [review]
Bug 18295: C4::Items - get_itemnumbers_of

The code from scripts and subroutines using this subroutine was not very
elegant. Most of the time the code was unnecessarily complex.
This patch removes this subroutine and adapt the code to use
Koha::Items instead.

1. C4::Items::get_hostitemnumbers_of
I did not understand why the code was so complicated, it seems that we
only want to know if a given item has a given biblionumber
2. cataloguing/merge.pl
We want to retrieve the itemnumber for a given biblio.
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items;
3. labels/label-item-search.pl
We want to loop over the items for a given biblio, no need to use
get_itemnumbers_of and GetItemInfosOf.
We just need to use:
  Koha::Items->search({ biblionumber => $biblionumber })
4. reserve/request.pl
We want to retrieve the itemnumbers of the biblio's items
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber');

Test plan:
1.You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
2. Merge two bibliographic records (with items), the resulting record
should contain items from both original records
3. Create a new label batch, edit it.
Add items to this batch ('Add items' button).
Fill the input with a barcode.
You should see all the items of a biblio.
Comment 5 Josef Moravec 2017-05-31 18:45:33 UTC
you can probably remove import of C4::Items from labels/label-item-search.pl, the only one imported routine GetItemInfosOf was removed in refactored code...

Otherwise everything looks OK
Comment 6 Josef Moravec 2017-05-31 18:47:23 UTC
(In reply to Josef Moravec from comment #5)
> you can probably remove import of C4::Items from
> labels/label-item-search.pl, the only one imported routine GetItemInfosOf
> was removed in refactored code...
> 

Now looking to bug 18296, you are removing it here, so ok, I am signing of ;)
Comment 7 Josef Moravec 2017-05-31 18:51:46 UTC
Created attachment 63887 [details] [review]
[SIGNED-OFF] Bug 18295: C4::Items - get_itemnumbers_of

The code from scripts and subroutines using this subroutine was not very
elegant. Most of the time the code was unnecessarily complex.
This patch removes this subroutine and adapt the code to use
Koha::Items instead.

1. C4::Items::get_hostitemnumbers_of
I did not understand why the code was so complicated, it seems that we
only want to know if a given item has a given biblionumber
2. cataloguing/merge.pl
We want to retrieve the itemnumber for a given biblio.
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items;
3. labels/label-item-search.pl
We want to loop over the items for a given biblio, no need to use
get_itemnumbers_of and GetItemInfosOf.
We just need to use:
  Koha::Items->search({ biblionumber => $biblionumber })
4. reserve/request.pl
We want to retrieve the itemnumbers of the biblio's items
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber');

Test plan:
1.You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
2. Merge two bibliographic records (with items), the resulting record
should contain items from both original records
3. Create a new label batch, edit it.
Add items to this batch ('Add items' button).
Fill the input with a barcode.
You should see all the items of a biblio.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

https://bugs.koha-community.org/show_bug.cgi?id=18285
Comment 8 Marcel de Rooy 2017-06-02 07:14:42 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2017-06-02 07:15:38 UTC
Created attachment 63918 [details] [review]
Bug 18295: C4::Items - get_itemnumbers_of

The code from scripts and subroutines using this subroutine was not very
elegant. Most of the time the code was unnecessarily complex.
This patch removes this subroutine and adapt the code to use
Koha::Items instead.

1. C4::Items::get_hostitemnumbers_of
I did not understand why the code was so complicated, it seems that we
only want to know if a given item has a given biblionumber
2. cataloguing/merge.pl
We want to retrieve the itemnumber for a given biblio.
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items;
3. labels/label-item-search.pl
We want to loop over the items for a given biblio, no need to use
get_itemnumbers_of and GetItemInfosOf.
We just need to use:
  Koha::Items->search({ biblionumber => $biblionumber })
4. reserve/request.pl
We want to retrieve the itemnumbers of the biblio's items
We could also have done that with:
  Koha::Biblios->find( $biblionumber )->items->get_column('itemnumber');

Test plan:
1.You need to create analytical record relationships (
EasyAnalyticalRecords needs to be set). Link an item to a biblio using
the 'Edit > Link to host item' menu from the biblio detail page.
From the staff interface place a hold on the biblio. You should see the
items from the biblio and the one you just linked
2. Merge two bibliographic records (with items), the resulting record
should contain items from both original records
3. Create a new label batch, edit it.
Add items to this batch ('Add items' button).
Fill the input with a barcode.
You should see all the items of a biblio.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2017-06-02 07:18:08 UTC
QA Comment:
Looks good to me.
Being liberal on unit tests here, but I know some QAers would certainly fail qa on this patch because get_hostitemnumbers_of is changed but not tested :)
Comment 11 Jonathan Druart 2017-06-05 14:56:44 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 12 Katrin Fischer 2017-06-11 19:45:57 UTC
It looks like this was pushed to 17.05?
Comment 13 Fridolin Somers 2017-06-12 07:18:52 UTC
(In reply to Katrin Fischer from comment #12)
> It looks like this was pushed to 17.05?

Oups, I've reverted.
This is big refactoring, not good for stable branch.
Comment 14 Fridolin Somers 2017-06-13 07:24:32 UTC
Enhancement not pushed to 17.05.x