Bug 35306

Summary: Expired holds are not displayed correctly in staff interface
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Lucas Gass <lucas>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: critical    
Priority: P5 - low CC: david, fridolin.somers, gmcharlt, lucas
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34032
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35307
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00
Bug Depends on: 33575    
Bug Blocks:    
Attachments: Bug 35306: Fix Expired holds when modding request
Bug 35306: Fix Expired holds when modding request
Bug 35306: Fix Expired holds when modding request

Description Nick Clemens (kidclamp) 2023-11-09 21:17:29 UTC
Bug 33575 added datatables to the holds table on request.pl - however, it removed the display of 'Expired: mm/dd/yyyy' for expired holds and used a date picker.

Flatpickr won't show dates prior to today, so the expiration date is now showing as blank

This means that when the holds page is updated, expiration dates are removed from the hold page
Comment 1 Lucas Gass 2023-11-10 15:02:29 UTC
Created attachment 158805 [details] [review]
Bug 35306: Fix Expired holds when modding request

To test:
1. Make some holds on a record and make sure some are set to expired.
2. Go to to reserve/request.pl?biblionumber=X for the biblionumber
3. Try updating something besides the expiration date. i.e. Pickup library
4. Click Update hold(s)
5. Notice the expirated holds have lost their expiration date.
6. APPLY PATCH
7. Try steps 2-4 again, this time the expiration date shold be retained.
8. Make sure Table settings still work and you can properly hide columns.
9. The hold expiration date should be hide-able.
Comment 2 Katrin Fischer 2023-11-10 18:20:14 UTC
Created attachment 158812 [details] [review]
Bug 35306: Fix Expired holds when modding request

To test:
1. Make some holds on a record and make sure some are set to expired.
2. Go to to reserve/request.pl?biblionumber=X for the biblionumber
3. Try updating something besides the expiration date. i.e. Pickup library
4. Click Update hold(s)
5. Notice the expirated holds have lost their expiration date.
6. APPLY PATCH
7. Try steps 2-4 again, this time the expiration date shold be retained.
8. Make sure Table settings still work and you can properly hide columns.
9. The hold expiration date should be hide-able.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 3 David Nind 2023-11-10 19:19:59 UTC
NOTE: I had a go at testing, but Katron had already signed off. Adding my notes/questions anyway.

Hi Lucas.

I had a go at testing, but I couldn't figure out from the test plan or description how to set a hold to expired.

Do I have to update this in the database?

When placing the hold, I can't select a date in the date picker before today, and I can't manually enter it - it remains blank.

Also, when I applied the patch, there are now four columns that cannot be toggled:
- checkbox
- priority
- expiration (newly added after the patch was applied)
- pickup library

In addition, (also before the patch is applied as well), I can't sort on several columns, such as Priority, Change priority, Expiration date. The ones that do seem to work are Patron, Notes, Date, Pickup library, Details.  Maybe for some, it doesn't make sense for them to be sortable?

David
Comment 4 Katrin Fischer 2023-11-10 19:30:15 UTC
(In reply to David Nind from comment #3)
> NOTE: I had a go at testing, but Katron had already signed off. Adding my
> notes/questions anyway.
> 
> Hi Lucas.
> 
> I had a go at testing, but I couldn't figure out from the test plan or
> description how to set a hold to expired.
> 
> Do I have to update this in the database?

Yes, you can't create one from the GUI or only by waiting a few days :)

I used something like this:

update reserves set expirationdate = '2022-01-01', patron_expiration_date = '2022-01-01' where reserve_id = 11;
 
> When placing the hold, I can't select a date in the date picker before
> today, and I can't manually enter it - it remains blank.

That's the expected behaviour.

> Also, when I applied the patch, there are now four columns that cannot be
> toggled:
> - checkbox
> - priority
> - expiration (newly added after the patch was applied)
> - pickup library

The change on the expiration date column is intentional.

> In addition, (also before the patch is applied as well), I can't sort on
> several columns, such as Priority, Change priority, Expiration date. The
> ones that do seem to work are Patron, Notes, Date, Pickup library, Details. 
> Maybe for some, it doesn't make sense for them to be sortable?

That might be something for a separate bug, but it does make some sense. As expiration date can be a text or an input field, it's probably removed from sorting for now?
Comment 5 David Nind 2023-11-10 19:47:43 UTC
Created attachment 158819 [details] [review]
Bug 35306: Fix Expired holds when modding request

To test:
1. Make some holds on a record and make sure some are set to expired.
2. Go to to reserve/request.pl?biblionumber=X for the biblionumber
3. Try updating something besides the expiration date. i.e. Pickup library
4. Click Update hold(s)
5. Notice the expirated holds have lost their expiration date.
6. APPLY PATCH
7. Try steps 2-4 again, this time the expiration date shold be retained.
8. Make sure Table settings still work and you can properly hide columns.
9. The hold expiration date should be hide-able.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-11-10 19:52:04 UTC
Thanks Katrin!

Can now replicate and have added my sign-off as well.

David

Testing notes (using KTD):

1. Use the SQL query in comment #4 to change the expiration date.

2. Holds that are expired should show in the 
expiration column as "Expired: XX/XX/XXXX", but just shows the date picker.

3. After applying the patch and refreshing the page, the text with the expired date should now show.
Comment 7 Tomás Cohen Arazi 2023-11-14 13:33:13 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 8 Fridolin Somers 2023-11-14 18:53:43 UTC
Depends on Bug 33575 not in 23.05.x