Bug 22273 - Column article_requests.created_on should not be updated
Summary: Column article_requests.created_on should not be updated
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-02-05 13:31 UTC by Marcel de Rooy
Modified: 2020-04-01 20:54 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05


Attachments
Bug 22273: Replace typo opan by opac in POD (780 bytes, patch)
2019-12-18 15:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22273: Database revision for table article_requests (2.48 KB, patch)
2019-12-18 15:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on (2.58 KB, patch)
2019-12-18 15:12 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22273: Replace typo opan by opac in POD (746 bytes, patch)
2020-02-28 17:21 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22273: Database revision for table article_requests (2.47 KB, patch)
2020-02-28 17:21 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on (2.70 KB, patch)
2020-02-28 17:21 UTC, Bernardo Gonzalez Kriegel
Details | Diff | Splinter Review
Bug 22273: Replace typo opan by opac in POD (804 bytes, patch)
2020-02-29 21:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22273: Database revision for table article_requests (2.53 KB, patch)
2020-02-29 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on (2.75 KB, patch)
2020-02-29 21:39 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2019-02-05 13:31:59 UTC
`created_on` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
  `updated_on` timestamp NULL DEFAULT NULL,
=> The update clause should not be with created_on.

In addition to this, the Koha::ArticleRequest store method contains:
    $self->updated_on($now);
This actually should not have been coded, but left to the database.
Comment 1 Marcel de Rooy 2019-12-18 15:12:42 UTC
Created attachment 96424 [details] [review]
Bug 22273: Replace typo opan by opac in POD

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2019-12-18 15:12:47 UTC
Created attachment 96425 [details] [review]
Bug 22273: Database revision for table article_requests

Column created_on should not be updated. It is filled once by the
store method of Koha::ArticleRequest.
Column updated_on should be a timestamp, updated by the database.

Note: Although higher versions of MySQL and MariaDB support two timestamps
(NOT NULL) in one table, I kept on the safe side by allowing NULL on the
created_on column.

@RM: Don't forget to run DBIx update.

Test plan:
Run new install or upgrade.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Marcel de Rooy 2019-12-18 15:12:51 UTC
Created attachment 96426 [details] [review]
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on

For a new record, store should fill created_on.
The database will always update the timestamp updated_on.
Since open also calls SUPER::store, we do not need to call it twice.

Test plan:
[1] Run t/db_dependent/ArticleRequests.t
[2] In the interface, add two article requests. Change the status of one
    to PROCESSING. Check created_on and updated_on in the article_requests
    table. The changed request should have updated_on > created_on.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Bernardo Gonzalez Kriegel 2020-02-28 17:21:06 UTC
Created attachment 99787 [details] [review]
Bug 22273: Replace typo opan by opac in POD

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Comment 5 Bernardo Gonzalez Kriegel 2020-02-28 17:21:10 UTC
Created attachment 99788 [details] [review]
Bug 22273: Database revision for table article_requests

Column created_on should not be updated. It is filled once by the
store method of Koha::ArticleRequest.
Column updated_on should be a timestamp, updated by the database.

Note: Although higher versions of MySQL and MariaDB support two timestamps
(NOT NULL) in one table, I kept on the safe side by allowing NULL on the
created_on column.

@RM: Don't forget to run DBIx update.

Test plan:
Run new install or upgrade.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested running update.
Comment 6 Bernardo Gonzalez Kriegel 2020-02-28 17:21:13 UTC
Created attachment 99789 [details] [review]
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on

For a new record, store should fill created_on.
The database will always update the timestamp updated_on.
Since open also calls SUPER::store, we do not need to call it twice.

Test plan:
[1] Run t/db_dependent/ArticleRequests.t
[2] In the interface, add two article requests. Change the status of one
    to PROCESSING. Check created_on and updated_on in the article_requests
    table. The changed request should have updated_on > created_on.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
[1] Tests pass
ok 12 - New article request has created_on date set
ok 13 - New article request has updated_on date set

[2] Work as described.
No errors
Comment 7 Katrin Fischer 2020-02-29 21:37:44 UTC
This works well. Should we try and fix the updated_on with the created_on on update?
Comment 8 Katrin Fischer 2020-02-29 21:38:58 UTC
Created attachment 99836 [details] [review]
Bug 22273: Replace typo opan by opac in POD

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2020-02-29 21:39:01 UTC
Created attachment 99837 [details] [review]
Bug 22273: Database revision for table article_requests

Column created_on should not be updated. It is filled once by the
store method of Koha::ArticleRequest.
Column updated_on should be a timestamp, updated by the database.

Note: Although higher versions of MySQL and MariaDB support two timestamps
(NOT NULL) in one table, I kept on the safe side by allowing NULL on the
created_on column.

@RM: Don't forget to run DBIx update.

Test plan:
Run new install or upgrade.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Tested running update.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2020-02-29 21:39:04 UTC
Created attachment 99838 [details] [review]
Bug 22273: Modify Koha::ArticleRequest->store to fill created_on

For a new record, store should fill created_on.
The database will always update the timestamp updated_on.
Since open also calls SUPER::store, we do not need to call it twice.

Test plan:
[1] Run t/db_dependent/ArticleRequests.t
[2] In the interface, add two article requests. Change the status of one
    to PROCESSING. Check created_on and updated_on in the article_requests
    table. The changed request should have updated_on > created_on.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
[1] Tests pass
ok 12 - New article request has created_on date set
ok 13 - New article request has updated_on date set

[2] Work as described.
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Marcel de Rooy 2020-03-02 08:26:29 UTC
Thanks Bernardo and Katrin
Comment 12 Marcel de Rooy 2020-03-02 08:29:28 UTC
(In reply to Katrin Fischer from comment #7)
> This works well. Should we try and fix the updated_on with the created_on on
> update?

Did not think that it was so important for article requests to do so. Normally I expect these dates to be quite close to each other.
Comment 13 Martin Renvoize 2020-03-02 09:47:15 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 14 Katrin Fischer 2020-03-02 09:55:03 UTC
We got quite a few reports for libraries that use article requests - was thinking more along these lines :)
Comment 15 Joy Nelson 2020-04-01 20:54:41 UTC
Pushed to 19.11.x branch for 19.11.05