Bug 33611 - Holds being placed in the future if DefaultHoldExpirationdate is set
Summary: Holds being placed in the future if DefaultHoldExpirationdate is set
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low major
Assignee: Nick Clemens (kidclamp)
QA Contact: Testopia
URL:
Keywords:
: 32563 (view as bug list)
Depends on: 30718
Blocks:
  Show dependency treegraph
 
Reported: 2023-04-25 21:25 UTC by Nick Clemens (kidclamp)
Modified: 2024-07-04 20:37 UTC (History)
5 users (show)

See Also:
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
Circulation function:


Attachments
Bug 33611: WIP - needs tests (955 bytes, patch)
2023-04-25 21:25 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33611: Unit test (1.64 KB, patch)
2023-04-27 13:08 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33611: Ensure dt_from_string always returns a new object (1.32 KB, patch)
2023-04-27 13:08 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33611: Unit test (1.69 KB, patch)
2023-05-01 17:56 UTC, David Nind
Details | Diff | Splinter Review
Bug 33611: Ensure dt_from_string always returns a new object (1.37 KB, patch)
2023-05-01 17:57 UTC, David Nind
Details | Diff | Splinter Review
Bug 33611: Unit test (1.76 KB, patch)
2023-05-02 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33611: Ensure dt_from_string always returns a new object (1.44 KB, patch)
2023-05-02 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33611: Unit test (1.82 KB, patch)
2023-05-08 15:16 UTC, Hinemoea Viault
Details | Diff | Splinter Review
Bug 33611: Ensure dt_from_string always returns a new object (1.50 KB, patch)
2023-05-08 15:16 UTC, Hinemoea Viault
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-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 (kidclamp) 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 (kidclamp) 2023-04-27 13:08:42 UTC
Created attachment 150294 [details] [review]
Bug 33611: Unit test
Comment 4 Nick Clemens (kidclamp) 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 (kidclamp) 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 (tcohen) 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 (lukeg) 2023-06-15 16:44:50 UTC
Missing dependencies for 22.05.x, no backport.