Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. Here we are going to deal with opac-detail
Created attachment 138821 [details] [review] Bug 31313: Remove GetItemsInfo from opac-detail Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. Here we are going to deal with opac-detail Test plan: List items on the modified view and confirm that all the info is displayed correctly
Created attachment 138822 [details] [review] Bug 31313: Fix availability - OPAC opac-detail Has to move some code to a method
Created attachment 138847 [details] [review] Bug 31313: Remove GetItemsInfo from opac-detail Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. Here we are going to deal with opac-detail Test plan: List items on the modified view and confirm that all the info is displayed correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 138848 [details] [review] Bug 31313: Fix availability - OPAC opac-detail Has to move some code to a method Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 139119 [details] [review] Bug 31313: Remove GetItemsInfo from opac-detail Bug 27272 is going to remove C4::Items::GetItemsInfo in favour of Koha::Items->search_ordered. Here we are going to deal with opac-detail Test plan: List items on the modified view and confirm that all the info is displayed correctly Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 139120 [details] [review] Bug 31313: Fix availability - OPAC opac-detail Has to move some code to a method Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 139121 [details] [review] Bug 31313: (QA follow-up) Search Koha::Items This ensures host items from otehr biblios are returned as well Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 139122 [details] [review] Bug 31313: (QA follow-up) Prefetch checkouts and branches This prefetches objects we are using in templates to avoid extar DB calls The branch fetches won't actually be used without the extra patch on 31314 that adapts home_branch and holding_branch routines to use result objects Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks for the follow-ups, Nick!
Created attachment 139193 [details] [review] Bug 31313: Unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Pushed to master for 22.11. Nice work everyone, thanks!
I am rebasing 29144 on top of this one. And have the idea that this is wrong code: $item->{holds_count} = $item_reserves{ $item->itemnumber }; $item->{priority} = $priority{ $item->itemnumber }; Should that be in $item_info instead ?
Created attachment 139239 [details] [review] Bug 31313: Fix holds_count and priority
(In reply to Marcel de Rooy from comment #12) > I am rebasing 29144 on top of this one. > And have the idea that this is wrong code: > > $item->{holds_count} = $item_reserves{ $item->itemnumber }; > $item->{priority} = $priority{ $item->itemnumber }; > > Should that be in $item_info instead ? Yes!
Follow-up pushed. Thanks for promptly addressing it, Jonathan!
regression found: the item transfer status is not shown anymore.
(In reply to Joonas Kylmälä from comment #16) > regression found: the item transfer status is not shown anymore. Seems like few other fields could be missing too, in addition I found item.waiting to be missing.
(In reply to Joonas Kylmälä from comment #17) > (In reply to Joonas Kylmälä from comment #16) > > regression found: the item transfer status is not shown anymore. > > Seems like few other fields could be missing too, in addition I found > item.waiting to be missing. Working on the fixes.
(In reply to Joonas Kylmälä from comment #18) > (In reply to Joonas Kylmälä from comment #17) > > (In reply to Joonas Kylmälä from comment #16) > > > regression found: the item transfer status is not shown anymore. > > > > Seems like few other fields could be missing too, in addition I found > > item.waiting to be missing. > > Working on the fixes. Thank you! I've been trapped by other devs.
Created attachment 139693 [details] [review] Bug 31313: (QA follow-up) Show status "On hold" correctly if there are waiting holds The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) cause the OPAC detail page only show "Available" status even when there were waiting holds. The onhold item field is removed at the same go as it was confusing and unused since "Bug 2655: Fixes availabilty counts if items are on hold shelf. Patch (2/2)" (a37545eb1) Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Created attachment 139694 [details] [review] Bug 31313: (QA follow-up) Show transfer status correctly if item in transit The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) caused the OPAC detail page only show "Available" status even when the item was in transit. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Created attachment 139695 [details] [review] Bug 31313: (QA follow-up) Remove unused bundle_host field from item_info The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) lead to koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc use the bundle_host method directly through the passed Koha::Item object, thus the bundle_host field added by this script is unused. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi>
Tomás, I attached a few important fixes here that fix the availability status shown in the opac-detail.pl page, could you push them directly from here to master after reviewing and making sure they look alright? However, there is one more bug that needs to be solved: item.on_order. The order info is not shown yet, and it has a quite complex implementation in opac-detail.pl and I'm afraid if it was to be implemented on the template side it will cause unneeded SQL queries to be made and slow down the opac-detail.pl page load time. I don't have time to work on it today, could someone else look into this? I went through item-status.inc file and the missing ordering info should be the only thing left to fix (after applying my patches attached to this bug report).
Created attachment 139761 [details] [review] Bug 31313: (QA follow-up) Show status "On hold" correctly if there are waiting holds The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) cause the OPAC detail page only show "Available" status even when there were waiting holds. The onhold item field is removed at the same go as it was confusing and unused since "Bug 2655: Fixes availabilty counts if items are on hold shelf. Patch (2/2)" (a37545eb1) Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 139762 [details] [review] Bug 31313: (QA follow-up) Show transfer status correctly if item in transit The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) caused the OPAC detail page only show "Available" status even when the item was in transit. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 139763 [details] [review] Bug 31313: (QA follow-up) Remove unused bundle_host field from item_info The patch "Bug 31313: Fix availability - OPAC opac-detail" (70cb4e22c07) lead to koha-tmpl/opac-tmpl/bootstrap/en/includes/item-status.inc use the bundle_host method directly through the passed Koha::Item object, thus the bundle_host field added by this script is unused. Signed-off-by: Joonas Kylmälä <joonas.kylmala@iki.fi> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Follow-ups pushed to master. Thanks! Any other follow-up please make it its own bug (set) ot avoid confusion.
Missing dependencies for 22.05.x, no backport