Bug 10872

Summary: C4::Items GetHiddenItems fix and optimization
Product: Koha Reporter: Mark Tompsett <mtompset>
Component: Architecture, internals, and plumbingAssignee: Mark Tompsett <mtompset>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: gmcharlt, tomascohen
Version: master   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10876
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10584
Change sponsored?: --- Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 10584    
Attachments: Bug 10872 - C4::Items.pm has YAML::Load in GetHiddenItems but lacks use or require
Bug 10872 - C4::Items GetHiddenItems fix and optimization
Bug 10872 - C4::Items GetHiddenItems fix and optimization
Bug 10872 - C4::Items GetHiddenItems fix and optimization
Bug 10872: UT for GetHiddenItemnumbers and POD fix

Description Mark Tompsett 2013-09-10 20:13:21 UTC
I believe the description covers it all. There is no testing, except does anything using C4::Items still work?

There should be a:
"require YAML;"
or
"use YAML qw/Load/;"

as the GetHiddenItems routine has a reference to YAML::Load.

This was discovered while adding GetHiddenItems() calls into opac/opac-MARCdetail.pl. I believe this problem dates back to bug 6488 or bug 5984.
Comment 1 Mark Tompsett 2013-09-10 20:22:30 UTC Comment hidden (obsolete)
Comment 2 Mark Tompsett 2013-09-10 20:48:52 UTC
It's not really an enhancement, as it should have been optimized in the first place. Changed to a "minor" bug as a result, since part of this wasn't even visible without another bug fix which is yet to come.
Comment 3 Mark Tompsett 2013-09-10 21:14:58 UTC Comment hidden (obsolete)
Comment 4 Mason James 2013-09-13 03:49:26 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2013-09-13 09:18:27 UTC
Created attachment 21075 [details] [review]
Bug 10872 - C4::Items GetHiddenItems fix and optimization

There should be a:
"require YAML;"
or
"use YAML qw/Load/;"

as the GetHiddenItems routine has a reference to YAML::Load.

This was discovered while adding a GetHiddenItems() call into
opac/opac-MARCdetail.pl. I believe this problem dates back to
bug 6488 or bug 5984.

I also added an optimization to GetHiddenItems to prevent
processing if there is nothing in the system preference. Test
by searching for a biblio which has some or all of its items
hidden.

Signed-off-by: Mason James <mtj@kohaaloha.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Maybe
  return () unless $yaml =~ /\S/;
or
  return () if $yaml =~ /^\s*$/;
would have been easier to read.
Comment 6 Tomás Cohen Arazi 2013-09-16 18:39:08 UTC
Created attachment 21125 [details] [review]
Bug 10872: UT for GetHiddenItemnumbers and POD fix

This patch adds several unit tests for GetHiddenItemnumbers and fixes the POD for it.
It also wraps the tests for rollback, modernizes and adds a license text to it.

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba
Comment 7 Galen Charlton 2013-09-18 15:59:40 UTC
Reviewed and passed QA on this, as it is part of a patch series that I want to push as a unit.
Comment 8 Galen Charlton 2013-09-18 16:08:15 UTC
Pushed to master.  Thanks, Mark!
Comment 9 Tomás Cohen Arazi 2013-11-11 15:13:12 UTC
This patch has been pushed to 3.12.x, will be in 3.12.7.

Thanks Mark!