At cgi-bin/koha/catalogue/detail.pl 'Place Hold' button is not hidden in : ISBD an Items view
Created attachment 107949 [details] [review] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module
Depending on bug 23166 for Context.Scalar
A with 0 item, button is there ! BAD B with 1 item not for loan , no button GOOD C with 1 item for loan, button is there GOOD D with 2 items, only one for loan, button is there GOOD
Created attachment 108159 [details] [review] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module QA note: This patch is only centralizing the existing code, but it is still too naive. To manage the visibility of this button we certainly want to copy what is done in C4::Search::searchResults: # can place a hold on a item if # not lost nor withdrawn # not damaged unless AllowHoldsOnDamagedItems is true # item is either for loan or on order (notforloan < 0) $can_place_holds = 1 if ( !$item->{itemlost} && !$item->{withdrawn} && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') ) && ( !$item->{notforloan} || $item->{notforloan} < 0 ) );
Created attachment 108379 [details] [review] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module QA note: This patch is only centralizing the existing code, but it is still too naive. To manage the visibility of this button we certainly want to copy what is done in C4::Search::searchResults: # can place a hold on a item if # not lost nor withdrawn # not damaged unless AllowHoldsOnDamagedItems is true # item is either for loan or on order (notforloan < 0) $can_place_holds = 1 if ( !$item->{itemlost} && !$item->{withdrawn} && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') ) && ( !$item->{notforloan} || $item->{notforloan} < 0 ) ); Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
This depends on bug 23166 that hasn't been signed off yet - moving to BLOCKED. Please bring it back into the QA queue once the dependency is resolved. This patch also does no longer apply. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 26139: Centralize code for "Place hold" button (detail) Using index info to reconstruct a base tree... M t/db_dependent/Koha/Items.t Falling back to patching base and 3-way merge... Auto-merging t/db_dependent/Koha/Items.t CONFLICT (content): Merge conflict in t/db_dependent/Koha/Items.t error: Failed to merge in the changes. Patch failed at 0001 Bug 26139: Centralize code for "Place hold" button (detail) The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-26139-Centralize-code-for-Place-hold-button-de-zcySqH.patch
Created attachment 108965 [details] [review] Bug 26139: Koha::Template::Plugin::Context There is something wrong with our ->search method and TT behaviours, it is hard to retrieve object list in a scalar context. If [% objects.method.count %] is called, objects.method will get the first object of the list and count will explode (Koha::Object->count does not exist) We need to force the call in a scalar context to retrieve an iterator and prevent to fetch all the objects (we could have called all then size, but it's not efficient)
This patch removes the dependency on bug 23166.
Created attachment 108966 [details] [review] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module QA note: This patch is only centralizing the existing code, but it is still too naive. To manage the visibility of this button we certainly want to copy what is done in C4::Search::searchResults: # can place a hold on a item if # not lost nor withdrawn # not damaged unless AllowHoldsOnDamagedItems is true # item is either for loan or on order (notforloan < 0) $can_place_holds = 1 if ( !$item->{itemlost} && !$item->{withdrawn} && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') ) && ( !$item->{notforloan} || $item->{notforloan} < 0 ) ); Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com>
Moving back to "Needs Signoff" since the patch "Bug 26139: Koha::Template::Plugin::Context" is not reviewed yet by anybody.
(In reply to Joonas Kylmälä from comment #10) > Moving back to "Needs Signoff" since the patch "Bug 26139: > Koha::Template::Plugin::Context" is not reviewed yet by anybody. It has been tested by Didier when he first tested this patch. Koha::Template::Plugin::Context is coming from another bug report, I isolated it to note depend on another bug report.
I get the following error with biblio without items when I click the ISBD view in biblio page in staff interface: > Template process failed: undef error - Can't call method "filter_by_for_loan" on unblessed reference at /kohadevbox/koha/Koha/Template/Plugin/Context.pm line 43. > at /kohadevbox/koha/C4/Templates.pm line 122 > C4::Templates::output('C4::Templates=HASH(0x56350c1d2188)') called at /kohadevbox/koha/catalogue/moredetail.pl line 301 > eval {...} at /kohadevbox/koha/catalogue/moredetail.pl line 2 > CGI::Compile::ROOT::kohadevbox_koha_catalogue_moredetail_2epl::__ANON__('CGI::Compile=HASH(0x56350a3aafa0)', '', '/kohadevbox/koha/catalogue/moredetail.pl', '/kohadevbox/koha/catalogue', 'ARRAY(0x56350b321fb0)') called at /usr/share/perl5/CGI/Compile.pm line 139
Re-adding the dependency, we need it for some minor refactoring, not only the TT plugin.
Blocked by bug 23166.
Created attachment 109057 [details] [review] Bug 26139: Centralize code for "Place hold" button (detail) There is a "norequest" boolean passed to the include cat-toolbar.inc, to display or not the "Pace hold" button. This flag was not calculated in some place (ISBDdetail and moredetail for instance) Here we centralize the code to make it more robust and less regression prone. Note that the same problem appears at the OPAC (opac-MARCdetail is always display the button). That will have to be fixed separately Test plan: Create biblio A with 0 item, B and C with 1 item and D with 2 items item for B is not for loan, C is for loan and D has 1 of each Go to the bibliographic detail page of each record and confirm that the "Place hold" button appears appropriately. Test the different tabs of the catalogue module QA note: This patch is only centralizing the existing code, but it is still too naive. To manage the visibility of this button we certainly want to copy what is done in C4::Search::searchResults: # can place a hold on a item if # not lost nor withdrawn # not damaged unless AllowHoldsOnDamagedItems is true # item is either for loan or on order (notforloan < 0) $can_place_holds = 1 if ( !$item->{itemlost} && !$item->{withdrawn} && ( !$item->{damaged} || C4::Context->preference('AllowHoldsOnDamagedItems') ) && ( !$item->{notforloan} || $item->{notforloan} < 0 ) ); Signed-off-by: Didier Gautheron <didier.gautheron@biblibre.com> Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Pushed to master for 20.11, thanks to everybody involved!
missing dependencies for 20.05.x, no backport