Created attachment 145808 [details] [review] Bug 32712: Fix OPAC breakage when OPACShowCheckoutName enabled Bug 31313 removed GetItemsInfo and proper Koha::Object-based objects are passed now. But some automatic search & replace kept patron-related attributes incorrectly linked to Koha::Item. This patch makes the item-status.inc file reuse the checkout object and get the linked patron to gather the required information. Some simplification is done on hte same patch... should be trivial. To test: 1. Have OpacShowCheckoutName enabled 2. Pick a biblio with items (all of them not checkecd-out) 3. Open the OPAC detail view for the biblio => SUCCESS: You see the biblio and the items 4. Check one of the items out 5. Refresh the OPAC page => FAIL: The page explodes! 6. Apply this patch 7. Refresh the OPAC page => SUCCESS: You see the biblio and the items, the patron info is correctly displayed. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 145831 [details] [review] Bug 32712: Fix OPAC breakage when OPACShowCheckoutName enabled Bug 31313 removed GetItemsInfo and proper Koha::Object-based objects are passed now. But some automatic search & replace kept patron-related attributes incorrectly linked to Koha::Item. This patch makes the item-status.inc file reuse the checkout object and get the linked patron to gather the required information. Some simplification is done on hte same patch... should be trivial. To test: 1. Have OpacShowCheckoutName enabled 2. Pick a biblio with items (all of them not checkecd-out) 3. Open the OPAC detail view for the biblio => SUCCESS: You see the biblio and the items 4. Check one of the items out 5. Refresh the OPAC page => FAIL: The page explodes! 6. Apply this patch 7. Refresh the OPAC page => SUCCESS: You see the biblio and the items, the patron info is correctly displayed. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 145832 [details] [review] Bug 32712: Fix OPAC breakage when OPACShowCheckoutName enabled Bug 31313 removed GetItemsInfo and proper Koha::Object-based objects are passed now. But some automatic search & replace kept patron-related attributes incorrectly linked to Koha::Item. This patch makes the item-status.inc file reuse the checkout object and get the linked patron to gather the required information. Some simplification is done on hte same patch... should be trivial. To test: 1. Have OpacShowCheckoutName enabled 2. Pick a biblio with items (all of them not checkecd-out) 3. Open the OPAC detail view for the biblio => SUCCESS: You see the biblio and the items 4. Check one of the items out 5. Refresh the OPAC page => FAIL: The page explodes! 6. Apply this patch 7. Refresh the OPAC page => SUCCESS: You see the biblio and the items, the patron info is correctly displayed. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Created attachment 145840 [details] [review] Bug 32712: Fix OPAC breakage when OPACShowCheckoutName enabled Bug 31313 removed GetItemsInfo and proper Koha::Object-based objects are passed now. But some automatic search & replace kept patron-related attributes incorrectly linked to Koha::Item. This patch makes the item-status.inc file reuse the checkout object and get the linked patron to gather the required information. Some simplification is done on hte same patch... should be trivial. To test: 1. Have OpacShowCheckoutName enabled 2. Pick a biblio with items (all of them not checkecd-out) 3. Open the OPAC detail view for the biblio => SUCCESS: You see the biblio and the items 4. Check one of the items out 5. Refresh the OPAC page => FAIL: The page explodes! 6. Apply this patch 7. Refresh the OPAC page => SUCCESS: You see the biblio and the items, the patron info is correctly displayed. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 145841 [details] [review] Bug 32712: Fix OPAC breakage when OPACShowCheckoutName enabled Bug 31313 removed GetItemsInfo and proper Koha::Object-based objects are passed now. But some automatic search & replace kept patron-related attributes incorrectly linked to Koha::Item. This patch makes the item-status.inc file reuse the checkout object and get the linked patron to gather the required information. Some simplification is done on hte same patch... should be trivial. To test: 1. Have OpacShowCheckoutName enabled 2. Pick a biblio with items (all of them not checkecd-out) 3. Open the OPAC detail view for the biblio => SUCCESS: You see the biblio and the items 4. Check one of the items out 5. Refresh the OPAC page => FAIL: The page explodes! 6. Apply this patch 7. Refresh the OPAC page => SUCCESS: You see the biblio and the items, the patron info is correctly displayed. 8. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Thanks, Tomas, better than mine. I would have been tempted to use Koha.Preference for OPACShowCheckoutName, but that's for another day ;) Worth noting: the only difference would be (theorically) if we have a checkout without due date, which does not make sense. By the way, shouldn't we have it not null in DB structure? (In reply to Jonathan Druart from comment #7) > I would have been tempted to use Koha.Preference for OPACShowCheckoutName, > but that's for another day ;) Yeah, it just needs tweaking several .pl and felt out of the scope. (In reply to Jonathan Druart from comment #8) > Worth noting: the only difference would be (theorically) if we have a > checkout without due date, which does not make sense. We should (at some point) allow 'indefinite checkouts'. I felt like we should be prepared for due_date=NULL for the future. Anyway, as you also noticed, we are not setting due_date=NULL so the behavior is kept. Created attachment 145847 [details] [review] Bug 32712: (QA follow-up) Use Koha.Preference Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Pushed to master for 23.05. Nice work everyone, thanks! Nice work, thanks everyone! Pushed to 22.11.x for the next release. (In reply to Tomás Cohen Arazi from comment #10) > Created attachment 145847 [details] [review] [review] > Bug 32712: (QA follow-up) Use Koha.Preference > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc is missing [% USE Koha %] (In reply to Jonathan Druart from comment #13) > (In reply to Tomás Cohen Arazi from comment #10) > > Created attachment 145847 [details] [review] [review] [review] > > Bug 32712: (QA follow-up) Use Koha.Preference > > > > Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> > > koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc is missing [% USE > Koha %] That was on purpose. Do you think it needs to be added even if the callers already include it? It should be included where used. We faced issues because of that in the past (w hen backported for instance). Created attachment 146014 [details] [review] Bug 32712: (QA follow-up) Add missing include I assumed it wasn't needed because teh .tt files using this already includ teh Koha plugin. But we shuoldn't skip, just in case someone doesn't notice when reusing this, and debugging will be a pain. Sorry for that. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Not needed in 22.05.x, no backport All in 22.11.x already. |
When OpacShoeCheckoutName is turned on, the OPAC shows this error if an item is checked out: Template process failed: undef error - The method Koha::Item->firstname is not covered by tests! I had talked to Joubu about it briefly, the fix could be: diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc index 0573b90e068..be5dbf004f9 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc +++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc @@ -47,15 +47,16 @@ [% IF datedue %] [% SET itemavailable = 0 %] + [% SET checkout_patron = item.checkout.patron %] [% IF onsite_checkout %] [% IF ( OPACShowCheckoutName ) %] - <span class="item-status checkedout">Currently in local use by [% item.firstname | html %] [% item.surname | html %] [% IF ( item.cardnumber ) %]([% item.cardnumber | html %])[% END %]</span> + <span class="item-status checkedout">Currently in local use by [% checkout_patron.firstname | html %] [% checkout_patron.surname | html %] [% IF ( checkout_patron.cardnumber ) %]([% checkout_patron.cardnumber | html %])[% END %]</span> [% ELSE %] <span class="item-status checkedout">Currently in local use</span> [% END %] [% ELSE %] [% IF ( OPACShowCheckoutName ) %] - <span class="item-status checkedout">Checked out to [% item.firstname | html %] [% item.surname | html %] [% IF ( item.cardnumber ) %]([% item.cardnumber | html %])[% END %]</span> + <span class="item-status checkedout">Checked out to [% checkout_patron.firstname | html %] [% checkout_patron.surname | html %] [% IF ( checkout_patron.cardnumber ) %]([% checkout_patron.cardnumber | html %])[% END %]</span> [% ELSE %] <span class="item-status checkedout">Checked out</span> [% END %]