[Bug 30089] New: Placing holds on OPAC broken
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Bug ID: 30089 Summary: Placing holds on OPAC broken Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: blocker Priority: P5 - low Component: OPAC Assignee: oleonard@myacpl.org Reporter: katrin.fischer@bsz-bw.de QA Contact: testopia@bugs.koha-community.org When trying to place a hold in the OPAC, either multi hold or a single hold, the page explodes with: The method Koha::AuthorisedValues->authorised_value is not covered by tests! I am using ktd, the sample database and my search term is 'perl'. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com --- Comment #1 from Lucas Gass <lucas@bywatersolutions.com> --- I recreate this also. Seems like it is caused by Bug 29844. When I revert commit 4ae2f6d9d8addd4e7fb8480ea0c41adfaa0febb0 I can again place a hold on the OPAC. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29844 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29844 [Bug 29844] Remove uses of wantarray in Koha::Objects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|oleonard@myacpl.org |fridolin.somers@biblibre.co | |m CC| |fridolin.somers@biblibre.co | |m --- Comment #2 from Fridolin Somers <fridolin.somers@biblibre.com> --- I'm on it -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #3 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 130581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130581&action=edit Bug 30089: Fix placing holds on OPAC after Bug 29844 When trying to place a hold in the OPAC, either multi hold or a single hold, the page explodes with: The method Koha::AuthorisedValues->authorised_value is not covered by tests! We could fix by adding ->as_list. This patch fixes by using Koha::AuthorisedValues->get_description_by_koha_field on each item instead of Koha::AuthorisedValues->search_by_koha_field list. Performance should be OK because this method as cache. This is used a lot in staff interface. Test plan : 1) In staff interface find a record with several items available For example in KTD biblionumer=126 2) Edit on item and change 'not for loan' to a non-zero value For example in KTD $7 = Staff Collection 3) Go to OPAC on this record 4) Place hold 5) Click on 'A specific item' => Check you see not for loan authorised value description For example in KTD '(Staff Collection)' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #4 from Fridolin Somers <fridolin.somers@biblibre.com> --- I'm not 100% sure of test plan. Feel free to amend. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #5 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Comment on attachment 130581 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130581 Bug 30089: Fix placing holds on OPAC after Bug 29844 Review of attachment 130581: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=30089&attachment=130581) ----------------------------------------------------------------- ::: opac/opac-reserve.pl @@ +530,5 @@
+ kohafield => 'items.notforloan', + authorised_value => $itemLoopIter->{notforloan} + } + ); + $itemLoopIter->{notforloanvalue} = $notforloan_av->{opac_description} // '';
I believe this line is not right - if OPAC description hasn't been set, the normal description should be used. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #6 from Fridolin Somers <fridolin.somers@biblibre.com> ---
I believe this line is not right - if OPAC description hasn't been set, the normal description should be used.
Correct. But this would change the current behavior no ? Also isn't it a way to hide a value from OPAC an see it only on staff interface ? TT plugin does the fallback in GetDescriptionByKohaField : https://git.koha-community.org/Koha-community/Koha/src/commit/65cc721e9f90fe... But not in GetByCode : https://git.koha-community.org/Koha-community/Koha/src/commit/65cc721e9f90fe... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130581|0 |1 is obsolete| | --- Comment #7 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 130583 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130583&action=edit Bug 30089: Fix placing holds on OPAC after Bug 29844 When trying to place a hold in the OPAC, either multi hold or a single hold, the page explodes with: The method Koha::AuthorisedValues->authorised_value is not covered by tests! We could fix by adding ->as_list. This patch fixes by using Koha::AuthorisedValues->get_description_by_koha_field on each item instead of Koha::AuthorisedValues->search_by_koha_field list. Performance should be OK because this method as cache. This is used a lot in staff interface. Test plan : 1) In staff interface find a record with several items available For example in KTD biblionumer=126 2) Edit on item and change 'not for loan' to a non-zero value For example in KTD $7 = Staff Collection 3) Go to OPAC on this record 4) Place hold 5) Click on 'A specific item' => Check you see not for loan authorised value description For example in KTD '(Staff Collection)' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #8 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Fridolin Somers from comment #6)
I believe this line is not right - if OPAC description hasn't been set, the normal description should be used.
Correct.
But this would change the current behavior no ?
Also isn't it a way to hide a value from OPAC an see it only on staff interface ?
TT plugin does the fallback in GetDescriptionByKohaField : https://git.koha-community.org/Koha-community/Koha/src/commit/ 65cc721e9f90fea4d0eb3d2b85831b60f3046ce6/Koha/Template/Plugin/ AuthorisedValues.pm#L76
But not in GetByCode : https://git.koha-community.org/Koha-community/Koha/src/commit/ 65cc721e9f90fea4d0eb3d2b85831b60f3046ce6/Koha/Template/Plugin/ AuthorisedValues.pm#L27
OK forget it https://git.koha-community.org/Koha-community/Koha/src/branch/master/Koha/Au... 'opac_description' is a method it contains the fallback, db field is 'lib_opac' -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 130592 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130592&action=edit [ALTERNATIVE PATCH] Bug 30089: (bug 29844 follow-up) Fix placing holds - OPAC The method Koha::AuthorisedValues->authorised_value is not covered by tests! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com --- Comment #10 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Fridolin, I would prefer to fix it like we do with the other occurrences and make sure we won't introduce another regression. It could impact performance if there is a lot of items. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|tomascohen@gmail.com |testopia@bugs.koha-communit | |y.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130592|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130583|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 130659 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130659&action=edit Bug 30089: (bug 29844 follow-up) Fix placing holds - OPAC The method Koha::AuthorisedValues->authorised_value is not covered by tests! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #10)
Fridolin, I would prefer to fix it like we do with the other occurrences and make sure we won't introduce another regression. It could impact performance if there is a lot of items.
I agree with this. Further refactorings can be done on their own bugs so we can discuss implementation details. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #13 from Fridolin Somers <fridolin.somers@biblibre.com> --- I agree -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|fridolin.somers@biblibre.co |jonathan.druart+koha@gmail. |m |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #130659|0 |1 is obsolete| | --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 130863 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130863&action=edit Bug 30089: (bug 29844 follow-up) Fix placing holds - OPAC The method Koha::AuthorisedValues->authorised_value is not covered by tests! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 --- Comment #15 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, thanks to everybody involved 🦄 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30089 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to master |RESOLVED CC| |kyle@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org