Bug 29192

Summary: DefaultHoldExpirationDate sends a DateTime object to logs
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Hold requestsAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED DUPLICATE QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: normal    
Priority: P5 - low CC: gmcharlt, jonathan.druart, martin.renvoize, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28700
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 26498    
Bug Blocks:    
Attachments: Bug 29192: Set expirationdate as string when storing Hold
Bug 29192: Set expirationdate as string when storing Hold
Bug 29192: Discard changes before logging a newly created hold
Bug 29192: Discard changes before logging a newly created hold

Description Nick Clemens (kidclamp) 2021-10-07 14:33:24 UTC
Bug 26498 added the syspref 'DefaultHoldExpirationDate'

It is implemented in Koha::Hold->store by saving a DateTime object into the expirationdate

This works fine, but the logs are flooded by this object
Comment 1 Nick Clemens (kidclamp) 2021-10-07 14:37:24 UTC
Created attachment 125889 [details] [review]
Bug 29192: Set expirationdate as string when storing Hold

To test:
 1 - Enable HoldsLog in system preferences
 2 - Set system preferences:
    DefaultHoldExpirationDate: Set
    DefaultHoldExpirationdatePeriod: 5
    DefaultHoldExpirationdateUnitOfTime: days
 3 - Create a hold for a patron
 4 - Note the expiration date is set
 5 - Check the action logs - SELECT * FROM action_logs
 6 - Note that there is a lot of data, this is from DateTime object in expirationdate
 7 - Apply patch
 8 - Restart all
 9 - Place another hold
10 - Confirm expiration date set correctly
11 - View the logs
12 - Entry for new hold contains only the date, no extra data
Comment 2 Andrew Fuerste-Henry 2021-10-07 14:53:35 UTC
Created attachment 125891 [details] [review]
Bug 29192: Set expirationdate as string when storing Hold

To test:
 1 - Enable HoldsLog in system preferences
 2 - Set system preferences:
    DefaultHoldExpirationDate: Set
    DefaultHoldExpirationdatePeriod: 5
    DefaultHoldExpirationdateUnitOfTime: days
 3 - Create a hold for a patron
 4 - Note the expiration date is set
 5 - Check the action logs - SELECT * FROM action_logs
 6 - Note that there is a lot of data, this is from DateTime object in expirationdate
 7 - Apply patch
 8 - Restart all
 9 - Place another hold
10 - Confirm expiration date set correctly
11 - View the logs
12 - Entry for new hold contains only the date, no extra data

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 3 Nick Clemens (kidclamp) 2021-10-07 15:57:12 UTC
Created attachment 125898 [details] [review]
Bug 29192: Discard changes before logging a newly created hold

When logging a newly create hold it can have a dt object in the expiration date field
Per bug 28700 this does not get unblessed
We add an explicit call to discard changes to avoid logging a blessed datetime

To test:
 1 - Enable HoldsLog in system preferences
 2 - Set system preferences:
    DefaultHoldExpirationDate: Set
    DefaultHoldExpirationdatePeriod: 5
    DefaultHoldExpirationdateUnitOfTime: days
 3 - Create a hold for a patron
 4 - Note the expiration date is set
 5 - Check the action logs - SELECT * FROM action_logs
 6 - Note that there is a lot of data, this is from DateTime object in expirationdate
 7 - Apply patch
 8 - Restart all
 9 - Place another hold
10 - Confirm expiration date set correctly
11 - View the logs
12 - Entry for new hold contains only the date, no extra data
Comment 4 Martin Renvoize 2021-10-08 14:23:07 UTC
Created attachment 125966 [details] [review]
Bug 29192: Discard changes before logging a newly created hold

When logging a newly create hold it can have a dt object in the expiration date field
Per bug 28700 this does not get unblessed
We add an explicit call to discard changes to avoid logging a blessed datetime

To test:
 1 - Enable HoldsLog in system preferences
 2 - Set system preferences:
    DefaultHoldExpirationDate: Set
    DefaultHoldExpirationdatePeriod: 5
    DefaultHoldExpirationdateUnitOfTime: days
 3 - Create a hold for a patron
 4 - Note the expiration date is set
 5 - Check the action logs - SELECT * FROM action_logs
 6 - Note that there is a lot of data, this is from DateTime object in expirationdate
 7 - Apply patch
 8 - Restart all
 9 - Place another hold
10 - Confirm expiration date set correctly
11 - View the logs
12 - Entry for new hold contains only the date, no extra data

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2021-10-08 14:23:46 UTC
Simple fix, no side effects, passes qa scripts.

Going straight for PQA
Comment 6 Jonathan Druart 2021-10-08 14:53:14 UTC
Missing test coverage.
Comment 7 Jonathan Druart 2021-10-12 09:37:46 UTC
I was going to write the tests when I remembered bug 28700. Please help there!
Comment 8 Jonathan Druart 2021-11-04 10:26:51 UTC

*** This bug has been marked as a duplicate of bug 28700 ***