Bug 33611

Summary: Holds being placed in the future if DefaultHoldExpirationdate is set
Product: Koha Reporter: Nick Clemens <nick>
Component: Hold requestsAssignee: Nick Clemens <nick>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: anneli.osterman, gmcharlt, jonathan.druart, lucas, slavashishkin
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Bug Depends on: 30718    
Bug Blocks:    
Attachments: Bug 33611: WIP - needs tests
Bug 33611: Unit test
Bug 33611: Ensure dt_from_string always returns a new object
Bug 33611: Unit test
Bug 33611: Ensure dt_from_string always returns a new object
Bug 33611: Unit test
Bug 33611: Ensure dt_from_string always returns a new object
Bug 33611: Unit test
Bug 33611: Ensure dt_from_string always returns a new object

Description Nick Clemens 2023-04-25 21:25:19 UTC
Bug 30718 altered C4::Reserves::AddReserve to use either the date passed in, or a dt_from_string object

When storing the hold, we call Koha::Hold::_set_default_expirationdate

This assumed reservedate was always a string, and used it to create a dt object, and add days for the expiration.

Now that reservedate is already a dt object, this means we add the days to the object, so the reservedate is set into the future, as well as the expiration

To recreate
1 - Set DefaultHoldExpiration to Set
2 - Set DefaultHoldExpirationPeriod to 365
3 - Set DefaultHoldExpirationunitOfTimeToDays
4 - Place a hold
5 - Note reserve date is 1 year in the future, same as expiration
Comment 1 Nick Clemens 2023-04-25 21:25:53 UTC
Created attachment 150223 [details] [review]
Bug 33611: WIP - needs tests
Comment 2 Jonathan Druart 2023-04-27 05:05:13 UTC
Hum, wondering if this is not safer to fix it in dt_from_string actually.
Comment 3 Nick Clemens 2023-04-27 13:08:42 UTC
Created attachment 150294 [details] [review]
Bug 33611: Unit test
Comment 4 Nick Clemens 2023-04-27 13:08:44 UTC
Created attachment 150295 [details] [review]
Bug 33611: Ensure dt_from_string always returns a new object

This patch alters dt_from_string to pass a cloned object if called
on an existing date time object

This resolves an issue where a holds reserve date was being altered when
the default expiration date was set

To test:
1 - Set DefaultHoldExpiration to Set
2 - Set DefaultHoldExpirationPeriod to 365
3 - Set DefaultHoldExpirationunitOfTimeToDays
4 - Place a hold
5 - Note reserve date is 1 year in the future, same as expiration
6 - Apply patch
7 - Restart all
8 - Place another hold
9 - Note reserve date is set to today
Comment 5 David Nind 2023-05-01 17:56:58 UTC
Created attachment 150463 [details] [review]
Bug 33611: Unit test

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-05-01 17:57:00 UTC
Created attachment 150464 [details] [review]
Bug 33611: Ensure dt_from_string always returns a new object

This patch alters dt_from_string to pass a cloned object if called
on an existing date time object

This resolves an issue where a holds reserve date was being altered when
the default expiration date was set

To test:
1 - Set DefaultHoldExpiration to Set
2 - Set DefaultHoldExpirationPeriod to 365
3 - Set DefaultHoldExpirationunitOfTimeToDays
4 - Place a hold
5 - Note reserve date is 1 year in the future, same as expiration
6 - Apply patch
7 - Restart all
8 - Place another hold
9 - Note reserve date is set to today

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 Jonathan Druart 2023-05-02 14:07:58 UTC
Created attachment 150519 [details] [review]
Bug 33611: Unit test

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2023-05-02 14:08:01 UTC
Created attachment 150520 [details] [review]
Bug 33611: Ensure dt_from_string always returns a new object

This patch alters dt_from_string to pass a cloned object if called
on an existing date time object

This resolves an issue where a holds reserve date was being altered when
the default expiration date was set

To test:
1 - Set DefaultHoldExpiration to Set
2 - Set DefaultHoldExpirationPeriod to 365
3 - Set DefaultHoldExpirationunitOfTimeToDays
4 - Place a hold
5 - Note reserve date is 1 year in the future, same as expiration
6 - Apply patch
7 - Restart all
8 - Place another hold
9 - Note reserve date is set to today

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Anneli Österman 2023-05-05 06:02:02 UTC
Is this bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32563 about the same problem as this one?
Comment 10 Nick Clemens 2023-05-05 11:25:32 UTC
*** Bug 32563 has been marked as a duplicate of this bug. ***
Comment 11 Hinemoea Viault 2023-05-08 15:16:25 UTC
Created attachment 150797 [details] [review]
Bug 33611: Unit test

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Hinemoea Viault <hinemoea.viault@inlibro.com>
Comment 12 Hinemoea Viault 2023-05-08 15:16:27 UTC
Created attachment 150798 [details] [review]
Bug 33611: Ensure dt_from_string always returns a new object

This patch alters dt_from_string to pass a cloned object if called
on an existing date time object

This resolves an issue where a holds reserve date was being altered when
the default expiration date was set

To test:
1 - Set DefaultHoldExpiration to Set
2 - Set DefaultHoldExpirationPeriod to 365
3 - Set DefaultHoldExpirationunitOfTimeToDays
4 - Place a hold
5 - Note reserve date is 1 year in the future, same as expiration
6 - Apply patch
7 - Restart all
8 - Place another hold
9 - Note reserve date is set to today

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Hinemoea Viault <hinemoea.viault@inlibro.com>
Comment 13 Tomás Cohen Arazi 2023-05-09 14:00:21 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 14 Matt Blenkinsop 2023-05-15 10:36:41 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 15 Lucas Gass 2023-06-15 16:44:50 UTC
Missing dependencies for 22.05.x, no backport.