Bug 33736 - Add bookings to collect circulation report
Summary: Add bookings to collect circulation report
Status: Needs documenting
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature
Assignee: Martin Renvoize (ashimema)
QA Contact: Paul Derscheid
URL:
Keywords: roadmap_24_11
Depends on: 29002
Blocks: 36119 37652 38119 38165
  Show dependency treegraph
 
Reported: 2023-05-15 14:46 UTC by Martin Renvoize (ashimema)
Modified: 2024-12-05 21:19 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
The enhancement adds a new 'Bookings to collect' report into the Circulation module. It parallels the 'Holds to pull' report allowing staff to easily report against upcoming bookings and collect them from the shelves ready for collection by the patron who has the item booked.
Version(s) released in:
24.11.00
Circulation function: bookings


Attachments
Bug 33736: Add pendingbookings report (13.09 KB, patch)
2023-11-03 17:02 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.63 KB, patch)
2024-01-15 16:00 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.75 KB, patch)
2024-04-17 14:03 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.79 KB, patch)
2024-04-18 19:47 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.77 KB, patch)
2024-04-22 12:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (882 bytes, patch)
2024-04-22 12:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.78 KB, patch)
2024-04-22 12:53 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (882 bytes, patch)
2024-04-22 12:54 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.78 KB, patch)
2024-08-15 16:18 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (882 bytes, patch)
2024-08-15 16:18 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add filter for holding library (5.69 KB, patch)
2024-08-15 16:18 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.78 KB, patch)
2024-10-14 14:17 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.78 KB, patch)
2024-10-14 14:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (882 bytes, patch)
2024-10-14 14:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add filter for holding library (5.69 KB, patch)
2024-10-14 14:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add item location and filter by library to bookings to collect report (3.16 KB, patch)
2024-10-14 14:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Bootstrap 5 update (4.07 KB, patch)
2024-10-14 14:23 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (follow-up) Remove top-level embed in bookings.yaml (775 bytes, patch)
2024-10-15 08:15 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Ensure datatable filter row takes visible into account (1.61 KB, patch)
2024-10-16 09:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Don't search on start_date if no date filter is defined (1.26 KB, patch)
2024-10-16 09:52 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add column settings definition for bookings report (1.38 KB, patch)
2024-10-16 10:22 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Don't allow sorting on Location (1.73 KB, patch)
2024-10-16 15:01 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.83 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (940 bytes, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Add filter for holding library (5.75 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Add item location and filter by library to bookings to collect report (3.21 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Bootstrap 5 update (4.12 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: (follow-up) Remove top-level embed in bookings.yaml (835 bytes, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Ensure datatable filter row takes visible into account (1.67 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Don't search on start_date if no date filter is defined (1.32 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Add column settings definition for bookings report (1.43 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Don't allow sorting on Location (1.79 KB, patch)
2024-10-17 08:39 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 33736: Add pendingbookings report (12.86 KB, patch)
2024-10-24 16:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Set default window to 1 week (1.01 KB, patch)
2024-10-24 16:40 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add filter for holding library (5.84 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add item location and filter by library to bookings to collect report (3.31 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Bootstrap 5 update (4.21 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (follow-up) Remove top-level embed in bookings.yaml (928 bytes, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Ensure datatable filter row takes visible into account (1.76 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Don't search on start_date if no date filter is defined (1.41 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Add column settings definition for bookings report (1.53 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: Don't allow sorting on Location (1.88 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (QA follow-up) Holds & Bookings > Holds and bookings (997 bytes, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (QA follow-up) Wrap title strings for translation (3.29 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (follow-up) Rebase fix for changed api date handling (1.72 KB, patch)
2024-10-24 16:41 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 33736: (follow-up) Cleanup and consistency (5.46 KB, patch)
2024-11-15 13:37 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize (ashimema) 2023-05-15 14:46:23 UTC
As part of the bookings workflow, we should add a "bookings to collect" report along similar lines to the "holds to pull" system.
Comment 1 Martin Renvoize (ashimema) 2023-11-03 17:02:16 UTC
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
Comment 2 Martin Renvoize (ashimema) 2024-01-15 16:00:31 UTC
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
Comment 3 Martin Renvoize (ashimema) 2024-01-15 16:01:24 UTC
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.
Comment 4 Laurence Rault 2024-04-11 14:07:27 UTC
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
Comment 5 Martin Renvoize (ashimema) 2024-04-17 14:03:10 UTC
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
Comment 6 Martin Renvoize (ashimema) 2024-04-17 14:04:05 UTC
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.
Comment 7 ByWater Sandboxes 2024-04-18 19:47:39 UTC
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>
Comment 8 Esther Melander 2024-04-18 19:51:03 UTC
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.
Comment 9 Martin Renvoize (ashimema) 2024-04-22 12:52:35 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize (ashimema) 2024-04-22 12:52:38 UTC Comment hidden (obsolete)
Comment 11 Martin Renvoize (ashimema) 2024-04-22 12:53:57 UTC
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>
Comment 12 Martin Renvoize (ashimema) 2024-04-22 12:54:00 UTC
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.
Comment 13 Martin Renvoize (ashimema) 2024-04-22 12:54:57 UTC
Thanks for the signoff and feedback Esther :)
Comment 14 Aude Charillon 2024-08-12 08:19:11 UTC
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.
Comment 15 Martin Renvoize (ashimema) 2024-08-15 16:18:47 UTC
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>
Comment 16 Martin Renvoize (ashimema) 2024-08-15 16:18:49 UTC
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.
Comment 17 Martin Renvoize (ashimema) 2024-08-15 16:18:52 UTC
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.
Comment 18 Martin Renvoize (ashimema) 2024-08-15 16:21:40 UTC
Library location now taken into account.. status is next
Comment 19 Martin Renvoize (ashimema) 2024-10-14 14:17:48 UTC
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>
Comment 20 Martin Renvoize (ashimema) 2024-10-14 14:23:13 UTC
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>
Comment 21 Martin Renvoize (ashimema) 2024-10-14 14:23:15 UTC
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.
Comment 22 Martin Renvoize (ashimema) 2024-10-14 14:23:18 UTC
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.
Comment 23 Martin Renvoize (ashimema) 2024-10-14 14:23:22 UTC
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).
Comment 24 Martin Renvoize (ashimema) 2024-10-14 14:23:24 UTC
Created attachment 172721 [details] [review]
Bug 33736: Bootstrap 5 update
Comment 25 Paul Derscheid 2024-10-15 08:15:38 UTC
Created attachment 172765 [details] [review]
Bug 33736: (follow-up) Remove top-level embed in bookings.yaml

For reference, see: bug 30536.
Comment 26 Paul Derscheid 2024-10-15 09:07:11 UTC
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.
Comment 27 Martin Renvoize (ashimema) 2024-10-16 09:40:33 UTC
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.
Comment 28 Martin Renvoize (ashimema) 2024-10-16 09:52:45 UTC
Created attachment 172808 [details] [review]
Bug 33736: Don't search on start_date if no date filter is defined
Comment 29 Martin Renvoize (ashimema) 2024-10-16 10:22:51 UTC
Created attachment 172810 [details] [review]
Bug 33736: Add column settings definition for bookings report
Comment 30 Martin Renvoize (ashimema) 2024-10-16 15:01:44 UTC
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.
Comment 31 Paul Derscheid 2024-10-17 08:39:05 UTC
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>
Comment 32 Paul Derscheid 2024-10-17 08:39:08 UTC
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>
Comment 33 Paul Derscheid 2024-10-17 08:39:11 UTC
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>
Comment 34 Paul Derscheid 2024-10-17 08:39:14 UTC
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>
Comment 35 Paul Derscheid 2024-10-17 08:39:16 UTC
Created attachment 172840 [details] [review]
Bug 33736: Bootstrap 5 update

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 36 Paul Derscheid 2024-10-17 08:39:19 UTC
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>
Comment 37 Paul Derscheid 2024-10-17 08:39:22 UTC
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>
Comment 38 Paul Derscheid 2024-10-17 08:39:25 UTC
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>
Comment 39 Paul Derscheid 2024-10-17 08:39:28 UTC
Created attachment 172844 [details] [review]
Bug 33736: Add column settings definition for bookings report

Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 40 Paul Derscheid 2024-10-17 08:39:30 UTC
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>
Comment 41 Katrin Fischer 2024-10-24 15:38:22 UTC
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    |
+------------+-----------+-----------+---------+-------------------+---------------------+---------------------+---------------------+---------------------+--------+
Comment 42 Martin Renvoize (ashimema) 2024-10-24 16:40:56 UTC Comment hidden (obsolete)
Comment 43 Martin Renvoize (ashimema) 2024-10-24 16:40:59 UTC Comment hidden (obsolete)
Comment 44 Martin Renvoize (ashimema) 2024-10-24 16:41:03 UTC Comment hidden (obsolete)
Comment 45 Martin Renvoize (ashimema) 2024-10-24 16:41:06 UTC Comment hidden (obsolete)
Comment 46 Martin Renvoize (ashimema) 2024-10-24 16:41:09 UTC Comment hidden (obsolete)
Comment 47 Martin Renvoize (ashimema) 2024-10-24 16:41:12 UTC Comment hidden (obsolete)
Comment 48 Martin Renvoize (ashimema) 2024-10-24 16:41:15 UTC
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>
Comment 49 Martin Renvoize (ashimema) 2024-10-24 16:41:19 UTC Comment hidden (obsolete)
Comment 50 Martin Renvoize (ashimema) 2024-10-24 16:41:22 UTC Comment hidden (obsolete)
Comment 51 Martin Renvoize (ashimema) 2024-10-24 16:41:25 UTC Comment hidden (obsolete)
Comment 52 Martin Renvoize (ashimema) 2024-10-24 16:41:28 UTC
Created attachment 173301 [details] [review]
Bug 33736: (QA follow-up) Holds & Bookings > Holds and bookings
Comment 53 Martin Renvoize (ashimema) 2024-10-24 16:41:32 UTC
Created attachment 173302 [details] [review]
Bug 33736: (QA follow-up) Wrap title strings for translation
Comment 54 Martin Renvoize (ashimema) 2024-10-24 16:41:35 UTC
Created attachment 173303 [details] [review]
Bug 33736: (follow-up) Rebase fix for changed api date handling
Comment 55 Katrin Fischer 2024-10-25 09:09:36 UTC
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?
Comment 56 Paul Derscheid 2024-10-25 09:10:30 UTC
Yes to all of that.
Comment 57 Katrin Fischer 2024-10-25 09:12:32 UTC
Pushed for 24.11!

Well done everyone, thank you!
Comment 58 Jonathan Druart 2024-10-28 09:10:54 UTC
(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.
Comment 59 Jonathan Druart 2024-10-28 09:16:11 UTC
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").
Comment 60 Jonathan Druart 2024-10-28 09:17:44 UTC
(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"}
Comment 61 Jonathan Druart 2024-10-28 09:21:01 UTC
137                     status: { '!=': 'waiting' },

waiting?

DB does not list it.
| status              | enum('new','cancelled','completed') | NO   |     | new                 |                               |
Comment 62 Jonathan Druart 2024-10-28 09:27:43 UTC
  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.
Comment 63 Jonathan Druart 2024-10-28 09:34:32 UTC
Do we have an "Action" column that is always empty?
Comment 64 Jonathan Druart 2024-10-28 09:36:09 UTC
The "Pickup library" column filter should be a dropdown list with the list of libraries (see patron search, item list on bib detail, etc.)
Comment 65 Jonathan Druart 2024-10-28 09:42:44 UTC
(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.
Comment 66 Katrin Fischer 2024-10-28 09:44:59 UTC
I feel like this is too much of a new feature for backport. There might also be some hidden dependencies on other new code.
Comment 67 Martin Renvoize (ashimema) 2024-10-28 09:56:46 UTC
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.
Comment 68 Martin Renvoize (ashimema) 2024-11-07 16:53:35 UTC
Is there actually any 'addition_work_needed' here still.. I can't spot any so can we remove the keyword?
Comment 69 Paul Derscheid 2024-11-07 16:55:17 UTC
I'd say so.
Comment 70 Paul Derscheid 2024-11-07 16:55:46 UTC
Sorry, what I mean is, I think there's no additional work needed anymore.
Comment 71 Jonathan Druart 2024-11-07 20:33:30 UTC
Comments 61, 63, 64?
Comment 72 Katrin Fischer 2024-11-14 15:44:58 UTC
(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?
Comment 73 Martin Renvoize (ashimema) 2024-11-15 13:37:38 UTC
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>
Comment 74 Katrin Fischer 2024-11-18 16:14:32 UTC
Picked last patch "(follow-up) Cleanup and consistency" for main.