Undefined subroutine &C4::Search::GetHiddenItemnumbers called at /srv/koha_fer/opac/../C4/Search.pm line 1627. at /srv/koha_fer/opac/opac-search.pl line 780
Created attachment 8700 [details] [review] Bug 7847 Undefined subroutine &C4::Search::GetHiddenItemnumbers
Created attachment 8732 [details] [review] Bug 7847 Undefined subroutine &C4::Search::GetHiddenItemnumbers This bug was uncovered by @mglavica during HackFest in Marseille It also showed it's ugly head during my plack presentation there. It's introduced by commit b65efdaacaf77fb53fee479da7f2e990ba0adeed which is Bug 6027 Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Did you mean in discussion or signed off?
I meant signed off, since Bug 6027 fix also doesn't seem right so this one is lesser evil.
It looks like there are several ways to clear up this issue: 1) this patch 2) remove the C4/Items.pm dependency (which is circular) on C4/Search that is introduced by bug 6027 (patch to fix this attached there) 3) add qw(GetHiddenItemnumbers) to the use C4::Items call on line 33 of C4/Search
Ian, which of those do you prefer?
I'm inclined to remove any circular dependencies, so option #2 would be my preferred solution. Its unclear from bug 6027 why that dependency was introduced, as Paul noted, and I'm not sure why I didn't catch it in QA... sorry, everyone.
QA comment: one-line patch, passed QA
well, in fact, I had a look at bug 6027, and found http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=8733 that says "This change fixes Bug 7847 without other changes, but I'm at loss why." does it mean I should apply both patches ? is attachment 8733 [details] [review] obsolete ? marking "failed QA until I've an answer
Only one of patches should be applied. Ian prefers one from bug 6027 and now that two people took a look at it, it seems more logical to me also. I will put this patch to "In discussion" but I can't change status of bug 6027 to "Needs sign-off" for some reason.
Created attachment 8891 [details] [review] Bug 7847 - OPAC search dies with plack This bug was uncovered by @mglavica during HackFest in Marseille It also showed it's ugly head during my plack presentation there. Following chang removes circural dependecy introduced by commit b65efdaacaf77fb53fee479da7f2e990ba0adeed which is part of Bug 6027
No comment. It works on both koha-standard and koha-plack Signing off
Created attachment 8905 [details] [review] [Signed off] Bug 7847 - OPAC search dies with plack This bug was uncovered by @mglavica during HackFest in Marseille It also showed it's ugly head during my plack presentation there. Following chang removes circural dependecy introduced by commit b65efdaacaf77fb53fee479da7f2e990ba0adeed which is part of Bug 6027 Signed-off-by: Alex Arnaud <alex.arnaud@biblibre.com>
QA comment: tiny patch, passed QA
I reopen this bug since there is a problem removing 'use C4::Search' from C4::Items. C4::Items::GetAnalyticsCount needs C4::Search for SimpleSearch and NZanalyze. In bug 7162, the patch modify DelOrder to use DelItemCheck instead of DelItem, but it causes perl to die. This can be bypassed adding 'use C4::Search' in the .pl file that call DelOrder, but I don't think it is the way to do it. Looking at C4::Search, it seems that C4::Items is only used for C4::Items::get_authorised_value_images, which is unrelated to items, so I think this sub should be moved (probably in C4::Koha, with others authorised_value related subs) so that C4::Search won't need C4::Items anymore.
I'm a little bit lost with this bug and bug 6027. Is there something that still needs to be pushed ?
C4::Search is used by C4::Items in current master, so that much is settled. The original problem, however, is probably still relevant, as GetHiddenItemnumbers is not explicitly scoped to C4::Items::
Created attachment 9806 [details] [review] Bug 7847: OPAC search dies with Plack Due to a dependency cycle between C4::Search and C4::Items, searches in the OPAC die spectacularly under Plack. This counter-patch extends dpavlin's solution and replaces use with require for C4::Search in C4::Items and for C4::Items in C4::Search. Obsoleting previous patch as this one eliminates the side effects under CGI.
Created attachment 9887 [details] [review] Bug 7847: OPAC search dies with Plack Due to a dependency cycle between C4::Search and C4::Items, searches in the OPAC die spectacularly under Plack. This counter-patch extends dpavlin's solution and replaces use with require for C4::Search in C4::Items and for C4::Items in C4::Search. Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Testing : * BEFORE the patch - opac & staff without Plack = work fine - opac with Plack = Undefined subroutine &C4::Search::GetHiddenItemnumbers called at /home/paul/koha.dev/koha-community/C4/Search.pm line 1634. at /home/paul/koha.dev/koha-community/opac/opac-search.pl line 806 - staff with Plack = work fine * AFTER the patch - both opac & staff work fine with or without Plack signed off
Bends (but does not completely break) circular dependency between C4::Items and C4::Search by moving the inclusion into the specific subroutines that require the module. That, along with some scoping, are the primary changes here. As far as I can tell, these two subroutines are all that require the other modules, so this is safe. marking as Passed QA.
This patch, related to Plack and OPAC should be backported to 3.8 I think, as Plack is announced as working for OPAC in 3.8
Paul is correct, pushing to 3.8.x, will be in 3.8.2