As part of the bookings workflow, we should add a "bookings to collect" report along similar lines to the "holds to pull" system.
Created attachment 158356 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. NOTE: Refinements still required
Created attachment 161044 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. NOTE: Refinements still required
This is the basic report, but I need feedback on what actions people would like to take from it.. it's a work in progress.
Apply? [(y)es, (n)o, (i)nteractive] Applying: Bug 33736: Add pendingbookings report Using index info to reconstruct a base tree... M api/v1/swagger/paths/bookings.yaml M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt Falling back to patching base and 3-way merge... Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt Auto-merging api/v1/swagger/paths/bookings.yaml error: Failed to merge in the changes. Patch failed at 0001 Bug 33736: Add pendingbookings report [33mhint: Use 'git am --show-current-patch=diff' to see the failed patch[m 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-33736-Add-pendingbookings-report-66f05im8.patch
Created attachment 165014 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. NOTE: Refinements still required
Rebased ready for testing. The page is very basic as it stands, with the idea that we'll iterate and add actions into it once they become apparent.
Created attachment 165144 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. NOTE: Refinements still required Signed-off-by: Esther <esther@bywatersolutions.com>
My test plan: 1. Search the catalog and make several items bookable. 2. Schedule bookings for those items. Try to do a variety of bookings, such as multiple patrons for the same item, overlap dates, specific item, any item, different titles. 3. Navigate to circulation, click on bookings. 4. The date range displayed may need adjustment to display the table. Adjust if needed and see the table. The table defaults to only showing bookings to be collected for "today". In order to see the pending bookings coming up, the dates selected needed to be adjusted in order to display all the ones I had created for the next month. I would prefer to have the pending bookings display for the next week by default. I suppose larger consortia may need it to be daily.
Created attachment 165277 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther <esther@bywatersolutions.com>
Created attachment 165278 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days.
Created attachment 165279 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther Melander <esther@bywatersolutions.com>
Created attachment 165280 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days.
Thanks for the signoff and feedback Esther :)
A library team we work with has been experimenting with this. They made the following comments: 1) If a booking has been issued, it stays on the list. This is not useful to the library team: they would prefer to see only those bookings still to be collected. 2) This report needs to filter by library. It is not useful to see bookings for all libraries; the team needs to see bookings for their own library.
Created attachment 170398 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther Melander <esther@bywatersolutions.com>
Created attachment 170399 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days.
Created attachment 170400 [details] [review] Bug 33736: Add filter for holding library This patch adds the option to filter the pending bookings to collect report by items current holding library and defaults to the users logged in library. We also add the 'Pickup library' column to the report output so staff know whether the item needs collecting for sending to another library or for their own.
Library location now taken into account.. status is next
Created attachment 172716 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther Melander <esther@bywatersolutions.com>
Created attachment 172717 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther Melander <esther@bywatersolutions.com>
Created attachment 172718 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days.
Created attachment 172719 [details] [review] Bug 33736: Add filter for holding library This patch adds the option to filter the pending bookings to collect report by items current holding library and defaults to the users logged in library. We also add the 'Pickup library' column to the report output so staff know whether the item needs collecting for sending to another library or for their own.
Created attachment 172720 [details] [review] Bug 33736: Add item location and filter by library to bookings to collect report This patch adds a new 'Location' field to the bookings to collect report. We display with the current item location or an onloan status with the due date. We also add filtering for status != 'waiting' to the base query to rule out items marked as ready for collection already (however, we don't yet have a way of marking such a status so this won't have any effect yet).
Created attachment 172721 [details] [review] Bug 33736: Bootstrap 5 update
Created attachment 172765 [details] [review] Bug 33736: (follow-up) Remove top-level embed in bookings.yaml For reference, see: bug 30536.
Unfortunately I found some issues: - There's a link to the configure tables view even though there's no associated table config. - The patron column has no search input even though it IS marked as searchable in the column object. - This one is mainly a question on UX, if I leave an input empty in the filter component it appears to filter w/ the last value it had. No deal-breaker in any way, just something that I noticed.
Created attachment 172807 [details] [review] Bug 33736: Ensure datatable filter row takes visible into account We clone the header row, and then iterate it assuming the iterator index lines up with the column definition index of the datatables settings. However, if a column is hidden, this is not the case. Said column will not appear in the header row that's cloned and as such your iterator will no longer line up with the column settings index number of the settings array.
Created attachment 172808 [details] [review] Bug 33736: Don't search on start_date if no date filter is defined
Created attachment 172810 [details] [review] Bug 33736: Add column settings definition for bookings report
Created attachment 172823 [details] [review] Bug 33736: Don't allow sorting on Location We need some special handling to sort on Location as it's got both a custom renderer and relies on _strings so the ordering and search won't return expected results without some custom function building.
Created attachment 172836 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172837 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172838 [details] [review] Bug 33736: Add filter for holding library This patch adds the option to filter the pending bookings to collect report by items current holding library and defaults to the users logged in library. We also add the 'Pickup library' column to the report output so staff know whether the item needs collecting for sending to another library or for their own. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172839 [details] [review] Bug 33736: Add item location and filter by library to bookings to collect report This patch adds a new 'Location' field to the bookings to collect report. We display with the current item location or an onloan status with the due date. We also add filtering for status != 'waiting' to the base query to rule out items marked as ready for collection already (however, we don't yet have a way of marking such a status so this won't have any effect yet). Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172840 [details] [review] Bug 33736: Bootstrap 5 update Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172841 [details] [review] Bug 33736: (follow-up) Remove top-level embed in bookings.yaml For reference, see: bug 30536. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172842 [details] [review] Bug 33736: Ensure datatable filter row takes visible into account We clone the header row, and then iterate it assuming the iterator index lines up with the column definition index of the datatables settings. However, if a column is hidden, this is not the case. Said column will not appear in the header row that's cloned and as such your iterator will no longer line up with the column settings index number of the settings array. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172843 [details] [review] Bug 33736: Don't search on start_date if no date filter is defined Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172844 [details] [review] Bug 33736: Add column settings definition for bookings report Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 172845 [details] [review] Bug 33736: Don't allow sorting on Location We need some special handling to sort on Location as it's got both a custom renderer and relies on _strings so the ordering and search won't return expected results without some custom function building. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Joubu has asked not to push changes to table configuration settings while the datatables bugs are progressing. Will check with him before pushing. I'll use the time to ask for a few small follow-ups: 1) Capitalization Holds & Bookings > Holds and bookings I am pretty sure we have removed all the & in the past, I think to ease translations. 2) Translatability a) These won't be translatable: koha-tmpl/intranet-tmpl/prog/en/modules/circ/pendingbookings.tt + "title": "Pickup library", Please check all "title": b) This won't be tranlatable as well: tt file = single underscore + return __("On loan, due: ") . $date(row.item.checked_out_date); 3) Internal server error [2024/10/24 15:35:27] [ERROR] GET /api/v1/bookings: unhandled exception (Koha::Exceptions::BadParameter)<<Exception 'Koha::Exceptions::BadParameter' thrown 'A bad parameter was given' with parameter => start_date>> MariaDB [koha_kohadev]> select * from bookings; +------------+-----------+-----------+---------+-------------------+---------------------+---------------------+---------------------+---------------------+--------+ | booking_id | patron_id | biblio_id | item_id | pickup_library_id | start_date | end_date | creation_date | modification_date | status | +------------+-----------+-----------+---------+-------------------+---------------------+---------------------+---------------------+---------------------+--------+ | 11 | 19 | 184 | 393 | CPL | 2024-10-24 22:00:00 | 2024-10-30 22:59:59 | 2024-10-24 15:32:11 | 2024-10-24 15:32:11 | new | +------------+-----------+-----------+---------+-------------------+---------------------+---------------------+---------------------+---------------------+--------+
Created attachment 173291 [details] [review] Bug 33736: Add pendingbookings report This patch adds a 'Bookings to collect' page to circulation that allows for a workflow similar to 'Holds to pull', but for bookings. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173292 [details] [review] Bug 33736: Set default window to 1 week This update the default display from today + 1 to today + 7 days. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173293 [details] [review] Bug 33736: Add filter for holding library This patch adds the option to filter the pending bookings to collect report by items current holding library and defaults to the users logged in library. We also add the 'Pickup library' column to the report output so staff know whether the item needs collecting for sending to another library or for their own. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173294 [details] [review] Bug 33736: Add item location and filter by library to bookings to collect report This patch adds a new 'Location' field to the bookings to collect report. We display with the current item location or an onloan status with the due date. We also add filtering for status != 'waiting' to the base query to rule out items marked as ready for collection already (however, we don't yet have a way of marking such a status so this won't have any effect yet). Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173295 [details] [review] Bug 33736: Bootstrap 5 update Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173296 [details] [review] Bug 33736: (follow-up) Remove top-level embed in bookings.yaml For reference, see: bug 30536. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173297 [details] [review] Bug 33736: Ensure datatable filter row takes visible into account We clone the header row, and then iterate it assuming the iterator index lines up with the column definition index of the datatables settings. However, if a column is hidden, this is not the case. Said column will not appear in the header row that's cloned and as such your iterator will no longer line up with the column settings index number of the settings array. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173298 [details] [review] Bug 33736: Don't search on start_date if no date filter is defined Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173299 [details] [review] Bug 33736: Add column settings definition for bookings report Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173300 [details] [review] Bug 33736: Don't allow sorting on Location We need some special handling to sort on Location as it's got both a custom renderer and relies on _strings so the ordering and search won't return expected results without some custom function building. Sponsored-by: PTFS Europe Ltd Signed-off-by: Esther Melander <esther@bywatersolutions.com> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Created attachment 173301 [details] [review] Bug 33736: (QA follow-up) Holds & Bookings > Holds and bookings
Created attachment 173302 [details] [review] Bug 33736: (QA follow-up) Wrap title strings for translation
Created attachment 173303 [details] [review] Bug 33736: (follow-up) Rebase fix for changed api date handling
I know that Joubu will not be too happy, but I feel that this is a very important workflow improvement for the bookings that should be in this release. I hope we can work together to get the datatables work in as well and this won't cause too big a hassle. Thanks Martin for the quick follow-ups. I updated some more strings to make them also translatable in another follow-up. Wondering if the "Booking dates" column sorting doesn't need some more thought, but I am OK to handle that separately. What do you think?
Yes to all of that.
Pushed for 24.11! Well done everyone, thank you!
(In reply to Katrin Fischer from comment #55) > I know that Joubu will not be too happy, but I feel that this is a very > important workflow improvement for the bookings that should be in this > release. I hope we can work together to get the datatables work in as well > and this won't cause too big a hassle. Yes, touching column visibility with the other work in the queue is not a good idea. Wondering why we didn't push without the columns settings.
Just looking at the code, with the other work related to the REST API and timezone, I don't think what you are doing with the date filters is correct. You should do what is done on the background job page ("filter by last hour").
(In reply to Jonathan Druart from comment #59) > Just looking at the code, with the other work related to the REST API and > timezone, I don't think what you are doing with the date filters is correct. > > You should do what is done on the background job page ("filter by last > hour"). Confirmed: q={"start_date":{">=":"2024-10-27T23:00:00.000Z","<=":"2024-11-04T22:59:59.999Z"}
137 status: { '!=': 'waiting' }, waiting? DB does not list it. | status | enum('new','cancelled','completed') | NO | | new | |
commit 025ed2d09aadc229490c94c5c82234e6942dbcf9 Bug 33736: Ensure datatable filter row takes visible into account I am really concerned about this commit. On bug 33484 AND bug 36640 I am adjusting this code to fix problems with the column visibility. I made sure that we didn't have conflict with the state, the column visibility, the hidden column (forced in the config), and the different tables. This patch should really be on its own bug and the different tables tested, not only this one. I am going to drop the changes made here for the other 2 bugs, but if we have 4 different codes (pre 33736, post 33736, 33736+33484, post 33736+33484+36640) to maintain in main and stable, it will be a nightmare to fix.
Do we have an "Action" column that is always empty?
The "Pickup library" column filter should be a dropdown list with the list of libraries (see patron search, item list on bib detail, etc.)
(In reply to Jonathan Druart from comment #62) > commit 025ed2d09aadc229490c94c5c82234e6942dbcf9 > Bug 33736: Ensure datatable filter row takes visible into account > > I am really concerned about this commit. > On bug 33484 AND bug 36640 I am adjusting this code to fix problems with the > column visibility. I made sure that we didn't have conflict with the state, > the column visibility, the hidden column (forced in the config), and the > different tables. > > This patch should really be on its own bug and the different tables tested, > not only this one. > > I am going to drop the changes made here for the other 2 bugs, but if we > have 4 different codes (pre 33736, post 33736, 33736+33484, post > 33736+33484+36640) to maintain in main and stable, it will be a nightmare to > fix. I have rebased bug 33484 on top of main and dropped the changes from this commit. Everything is ok on this table (as expected). If we plan to have 33484 in 24.11 and not bug 33736 backported then we can just ignore this situation. Otherwise we should maybe consider moving this specific patch on its own bug.
I feel like this is too much of a new feature for backport. There might also be some hidden dependencies on other new code.
Agreed, I don't think this should be backported, it's certainly an enhancement to the new feature of bookings. The datatables work is next on my list for today.
Is there actually any 'addition_work_needed' here still.. I can't spot any so can we remove the keyword?
I'd say so.
Sorry, what I mean is, I think there's no additional work needed anymore.
Comments 61, 63, 64?
(In reply to Jonathan Druart from comment #71) > Comments 61, 63, 64? Martin and Paul, can you please review and split into new bugs were needed?
Created attachment 174582 [details] [review] Bug 33736: (follow-up) Cleanup and consistency Given Jonathans comments, this patch performs the requested cleanups. * We use the new select option for kohaTables for the pickup library search. * We add pickup library to the aside filters. * We remove the empty placeholder 'actions' column. * We drop the filtering on 'waiting' which will be reintroduced by the next but in the tree. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Picked last patch "(follow-up) Cleanup and consistency" for main.