Bug 14788

Summary: Move opac-topissues.pl code into C4::Circulation
Product: Koha Reporter: Julian Maurice <julian.maurice>
Component: Architecture, internals, and plumbingAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, katrin.fischer, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 14749, 11139    
Attachments: Bug 14788: Move opac-topissues.pl code into C4::Circulation
Bug 14788: Move opac-topissues.pl code into C4::Circulation
Bug 14788: Add unit tests for GetTopIssues
Bug 14788: Move opac-topissues.pl code into C4::Circulation
Bug 14788: Add unit tests for GetTopIssues
[PASSED QA] Bug 14788: Move opac-topissues.pl code into C4::Circulation
[PASSED QA] Bug 14788: Add unit tests for GetTopIssues

Description Julian Maurice 2015-09-04 14:22:33 UTC
Move opac-topissues.pl code into C4::Circulation
Comment 1 Julian Maurice 2015-09-04 14:23:27 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2015-10-02 06:29:08 UTC
Hi Julian,

With the patch, the checkouts on the result table do no longer sort numerically but alpabetically, something like:
1 -14 - 2- 26- 89
instead of
1 -2 -14 -26- 89
Comment 3 Julian Maurice 2015-10-02 06:49:27 UTC
Hi Marc,

I have the same behaviour on master, I don't think the patch is responsible for that.
Comment 4 Julian Maurice 2015-10-02 07:33:05 UTC
Created a new bug: Bug 14940

Switching back to Needs signoff
Comment 5 Marc Véron 2015-10-02 09:19:08 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2015-10-02 09:32:26 UTC
Julian, please provide tests.
Comment 7 Julian Maurice 2015-10-05 09:03:17 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-10-06 13:03:36 UTC
CONFLICT (content): Merge conflict in koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-topissues.tt
Comment 9 Julian Maurice 2015-10-07 07:39:11 UTC Comment hidden (obsolete)
Comment 10 Julian Maurice 2015-10-07 07:39:13 UTC Comment hidden (obsolete)
Comment 11 Julian Maurice 2015-10-07 07:40:14 UTC
Patches rebased on master.
Minor conflict resolved, switching back to Signed Off
Comment 12 Katrin Fischer 2015-10-22 06:10:53 UTC
Created attachment 43730 [details] [review]
[PASSED QA] Bug 14788: Move opac-topissues.pl code into C4::Circulation

Tested with syspref 'AdvancedSearchTypes' set to itemtypes an ccode (one at a time).
No problems found.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 13 Katrin Fischer 2015-10-22 06:11:04 UTC
Created attachment 43731 [details] [review]
[PASSED QA] Bug 14788: Add unit tests for GetTopIssues

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2015-10-22 06:12:42 UTC
I think this is an improvement over the current code, but needs to be pushed together with bug 11139 that will actually make it work nicely :)

If you add multiple values in AdvancedSearchTypes with only this patch, it doesn't work quite right yet - but tested with 11139 things look alright.
Comment 15 Tomás Cohen Arazi 2015-10-22 15:02:07 UTC
Patches pushed to master.

Good job Julian!
Comment 16 Jonathan Druart 2015-10-23 14:45:06 UTC
]% prove t/db_dependent/Circulation/GetTopIssues.t
t/db_dependent/Circulation/GetTopIssues.t .. DBD::mysql::st execute failed: Column 'biblioitemnumber' cannot be null at C4/Items.pm line 2196.
DBD::mysql::st execute failed: Column 'biblioitemnumber' cannot be null at C4/Items.pm line 2196.
# Looks like your test exited with 255 before it could output anything.
t/db_dependent/Circulation/GetTopIssues.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 14/14 subtests 

Test Summary Report
-------------------
t/db_dependent/Circulation/GetTopIssues.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 14 tests but ran 0.
Files=1, Tests=0,  2 wallclock secs ( 0.02 usr  0.00 sys +  1.40 cusr  0.02 csys =  1.44 CPU)
Result: FAIL
Comment 17 Jonathan Druart 2015-10-23 14:56:23 UTC
Ok...
The AI on my biblio and biblioitems tables differ.
I don't really know why...