Bug 29192 - DefaultHoldExpirationDate sends a DateTime object to logs
Summary: DefaultHoldExpirationDate sends a DateTime object to logs
Status: RESOLVED DUPLICATE of bug 28700
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens (kidclamp)
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 26498
Blocks:
  Show dependency treegraph
 
Reported: 2021-10-07 14:33 UTC by Nick Clemens (kidclamp)
Modified: 2021-11-04 10:26 UTC (History)
4 users (show)

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


Attachments
Bug 29192: Set expirationdate as string when storing Hold (1.36 KB, patch)
2021-10-07 14:37 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29192: Set expirationdate as string when storing Hold (1.42 KB, patch)
2021-10-07 14:53 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29192: Discard changes before logging a newly created hold (1.46 KB, patch)
2021-10-07 15:57 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 29192: Discard changes before logging a newly created hold (1.52 KB, patch)
2021-10-08 14:23 UTC, Martin Renvoize
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) 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 ***