ContributionRecur modified_date should update to current_timestamp on update

[mattwire]

Overview

A modified_date field should update on update... All others do!

Before

contribution_recur.modified_date does not update automatically.

After

contribution_recur.modified_date does update automatically.

Technical Details

Missing ON UPDATE CURRENT_TIMESTAMP() in definition.

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I think this is fine if the test is fixed & r-run testing passes

api_v3_SyntaxConformanceTest::testCreateSingleValueAlter with data set #9 ('ContributionRecur')
api result array comparison failed checking if contact_id was correctly updated
Array
(
[update-params] => Array
(
[id] => 40
[contact_id] => 1730

[mattwire]

@eileenmcnaughton The issue with the test is the API3 ContributionRecur.get - returnParams need to ignore contribution_recur_modified_date when the default is to ignore modified_date only.

[seamuslee001]

@mattwire just wondering should this be timestamp not datetime? I note it currently is a datetime I think but for modified date should we change to timestamp here?

[eileenmcnaughton]

It might be one of those ones where we made it datetime because of earlier mysql having limits around timestamp. I agreed with Timestamp being the right format

[seamuslee001]

I just have one question around the column type but apart from that this is mergeable adding merge ready

[mattwire]

@seamuslee001 @eileenmcnaughton I've updated to convert to timestamp. There are various other date fields which are datetime on civicrm_contribution_recur - I don't know if we should be converting those too?

[eileenmcnaughton]

doesn't this need to change too ^^

[mattwire]

Yes.. fixed

[eileenmcnaughton]

@seamuslee001 @eileenmcnaughton I've updated to convert to timestamp. There are various other date fields which are datetime on civicrm_contribution_recur - I don't know if we should be converting those too?

In the case of 'created_date' I think it's a straight forward yes.

For the other fields I feel like they ideally would be timestamps but I don't know if converting them has any implications - especially with regards to 'next scheduled date'

[demeritcowboy]

This is conflicted now.

[mattwire]

@demeritcowboy resolved conflicts