Bug 27574 - Don't use NOW() in Koha::Accounts
Summary: Don't use NOW() in Koha::Accounts
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Martin Renvoize
QA Contact: Testopia
URL:
Keywords: Academy
Depends on:
Blocks: 27573
  Show dependency treegraph
 
Reported: 2021-01-29 08:28 UTC by Martin Renvoize
Modified: 2021-12-13 21:10 UTC (History)
3 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 27574: Update \NOW() to dt_from_string for Accounts (3.35 KB, patch)
2021-01-29 08:49 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27574: Update \NOW() to dt_from_string for Accounts (10.81 KB, patch)
2021-01-29 13:35 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 Martin Renvoize 2021-01-29 08:28:39 UTC
Using NOW() to set a date field should be avoided as the SQL server timezone may not match reflect the koha instance timezone.
Comment 1 Martin Renvoize 2021-01-29 08:49:55 UTC
Created attachment 116011 [details] [review]
Bug 27574: Update \NOW() to dt_from_string for Accounts

This patch replaces all instances of setting the date/timestamp fields
for accounts related tables from the SQL literal \'NOW()' to
Koha::DateUtils dt_from_string so we get properly localised dates.
Comment 2 Martin Renvoize 2021-01-29 12:53:14 UTC
Working on unit tests for this
Comment 3 Martin Renvoize 2021-01-29 13:34:38 UTC
Hmm... after discussing this with Jonathan we agree this is perhaps a solved problem already with bug 20123
Comment 4 Martin Renvoize 2021-01-29 13:35:45 UTC
Created attachment 116029 [details] [review]
Bug 27574: Update \NOW() to dt_from_string for Accounts

This patch replaces all instances of setting the date/timestamp fields
for accounts related tables from the SQL literal \'NOW()' to
Koha::DateUtils dt_from_string so we get properly localised dates.

We also update the unit tests
Comment 5 Tomás Cohen Arazi 2021-01-29 19:58:10 UTC
Are we sure? Aren't we setting the timezone on connection time? In that case, wouldn't NOW() be already using the timezone? I'm worried we are double escaping here (hehe).