Currently on the bookings tab of the patron pages, we always filter to active bookings. We should add the option to display completed bookings too; This would match the functionality introduced in the biblio details view for bookings in bug 37065
Created attachment 167934 [details] [review] Bug 37141: Add 'Include completed' option to bookings tab This patch adds the 'Include completed' filter toggle option to the bookings display tab on both the patron details and circulation pages.
Created attachment 168420 [details] [review] Bug 37141: Add 'Show completed' option to bookings tab This patch adds the 'Show completed' filter toggle option to the bookings display tab on both the patron details and circulation pages.
Is there a test plan for this one?
I tested like this: 1) Create a booking for any item. 2) Open a db shell with `koha-mysql <INSTANCE>` 3) Update the created booking with: ` update bookings set start_date = '<date in the past>', end_date = '<date also in the past'> where booking_id = '<booking id of the booking you just created>'. ` 4) Go the the bookings tab of said item. 5) Notice that a 'Show expired' link/button appears on the top left of the table. Also note that the booking you created isn't displayed because it's in the past. 6) Click the 'Show expired' button. 7) Notice that the expired booking is now shown. 8) Sign off.
PS: after you applied the patch :)
Created attachment 170057 [details] [review] Bug 37141: Add 'Show completed' option to bookings tab This patch adds the 'Show completed' filter toggle option to the bookings display tab on both the patron details and circulation pages. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
This doesn't work for me, I had to move the line: let filter_expired = true; up in scope , then the link worked. There is another issue, if the patron has no current bookings the table is not loaded and you can't see their expired bookings
Strange that the first issue didn't come up for me. I'll take a look at it again in a bit.
I've looked at the wrong page because it didn't read the title of this bug. My sign-off is therefore invalid.
Created attachment 170063 [details] [review] Bug 37141: (follow-up) Move filter_expired to outer scope This allows access by the bookings-tab and the expired_filter click handlers which are both required for the state change to work.
Created attachment 170064 [details] [review] Bug 37141: (follow-up) Check for expired bookings when rendering bookings table We preserve the existing logic of this bug and add another case for when we just encounter expired bookings. In that case we render the fieldset without the 'filtered' class and determine the value of filter_expired in bookings.js by the class being set.
Created attachment 170081 [details] [review] Bug 37141: Add 'Show completed' option to bookings tab This patch adds the 'Show completed' filter toggle option to the bookings display tab on both the patron details and circulation pages. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: David Nind <david@davidnind.com>
Created attachment 170082 [details] [review] Bug 37141: (follow-up) Move filter_expired to outer scope This allows access by the bookings-tab and the expired_filter click handlers which are both required for the state change to work. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 170083 [details] [review] Bug 37141: (follow-up) Check for expired bookings when rendering bookings table We preserve the existing logic of this bug and add another case for when we just encounter expired bookings. In that case we render the fieldset without the 'filtered' class and determine the value of filter_expired in bookings.js by the class being set. Signed-off-by: David Nind <david@davidnind.com>
Created attachment 170088 [details] [review] Bug 37141: Add 'Show completed' option to bookings tab This patch adds the 'Show completed' filter toggle option to the bookings display tab on both the patron details and circulation pages. Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 170089 [details] [review] Bug 37141: (follow-up) Move filter_expired to outer scope This allows access by the bookings-tab and the expired_filter click handlers which are both required for the state change to work. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 170090 [details] [review] Bug 37141: (follow-up) Check for expired bookings when rendering bookings table We preserve the existing logic of this bug and add another case for when we just encounter expired bookings. In that case we render the fieldset without the 'filtered' class and determine the value of filter_expired in bookings.js by the class being set. Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 170091 [details] [review] Bug 37141: Tidy JS Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Thanks so much for taking this one over Paul.. you've done a great job with the follow-ups I hadn't managed to get on to yet! Thanks for the testing and quick QA Nick! Go teamwork!
Please keep in mind, we need to use double underscores for translatable strings in .js files. Fixed in a follow-up. Fixed in a follow-up.
Pushed for 24.11! Well done everyone, thank you!