Bug 35306 - Expired holds are not displayed correctly in staff interface
Summary: Expired holds are not displayed correctly in staff interface
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low critical (vote)
Assignee: Lucas Gass
QA Contact: Testopia
URL:
Keywords:
Depends on: 33575
Blocks:
  Show dependency treegraph
 
Reported: 2023-11-09 21:17 UTC by Nick Clemens (kidclamp)
Modified: 2023-11-14 18:53 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 35306: Fix Expired holds when modding request (2.50 KB, patch)
2023-11-10 15:02 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35306: Fix Expired holds when modding request (2.55 KB, patch)
2023-11-10 18:20 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 35306: Fix Expired holds when modding request (2.60 KB, patch)
2023-11-10 19:47 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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