CRM-20892 Add in modified_date column to civicrm_mailing to assist in… #10953
Conversation
4341fb9 to
8f8cd49
Compare
|
If Jenkins is happy with it I am |
|
Hmm he isn't ERROR 1293 (HY000) at line 3729: Incorrect table definition; there can be only one TIMESTAMP column with CURRENT_TIMESTAMP in DEFAULT or ON UPDATE clause |
|
So the issue is that both created and this new modifed_date have CURRENT_TIMESTAMP as default see https://stackoverflow.com/questions/4489548/why-there-can-be-only-one-timestamp-column-with-current-timestamp-in-default-cla |
|
Hmm contact table does this
|
|
@totten any thoughts on the above? |
|
@seamuslee001 PR need rebase ! |
|
We discussed this a bit more on https://chat.civicrm.org/civicrm/pl/68cbe4pie3fufq7tgy9h3ebb3o ; TLDR: one column can use |
431bd5f to
80d7658
Compare
|
@monishdeb @totten @eileenmcnaughton i think i have it sorted now, |
CRM/Mailing/DAO/Mailing.php
Outdated
| 'name' => 'modified_date', | ||
| 'type' => CRM_Utils_Type::T_TIMESTAMP, | ||
| 'title' => ts('Modified Date') , | ||
| 'description' => 'When was the mailing (or closely related entity) was created or modified or deleted.', |
80d7658 to
83934fd
Compare
xml/schema/Mailing/Mailing.xml
Outdated
| @@ -487,4 +487,14 @@ | |||
| <type>Select</type> | |||
| </html> | |||
| </field> | |||
| <field> | |||
There was a problem hiding this comment.
I think if you move this right after created_date it will be in a more logical place in the table
|
I just tested this on a new install and it seems to be working at the mysql level. Oddly when I edit the mailing through the form rather than via mysql modified date does not update. I am guessing something in the form is setting modified_date. I have not yet tested upgrade |
… preventing cross edit of mailings
…set it Remove duplicate was Move field around in table definition as per Eileen's comment
23b1edd to
e7a6eae
Compare
|
@eileenmcnaughton i believe that i have fixed your issue now, please test Upgrade and let me know how you go. I think this part is probably right to be merged |
|
I did an upgrade test, checking out 4.7.25 & recreating & then checking out master, pulling this patch & upgrading and then added a new mailing & then edited it. Modified & created_date were correctly created/ updated. Looks like an enotice issue remains but I'm giving this merge on pass |
4a5b8b5 to
56944d3
Compare
|
@eileenmcnaughton tests has passed now |
On digging I determined this trigger hokiness was all about mysql 5.5 and we can ditch it now https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-5.html#mysqld-5-6-5-data-types and civicrm#10953 (comment) Note that we still support mysql 5.6 and the current version is 5.6.51 I think it's fine to require that the 5.6 minimum is 5.6.5 as released in 2012 I'll do the upgrade script as a follow up
On digging I determined this trigger hokiness was all about mysql 5.5 and we can ditch it now https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-5.html#mysqld-5-6-5-data-types and civicrm#10953 (comment) Note that we still support mysql 5.6 and the current version is 5.6.51 I think it's fine to require that the 5.6 minimum is 5.6.5 as released in 2012 I'll do the upgrade script as a follow up
On digging I determined this trigger hokiness was all about mysql 5.5 and we can ditch it now https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-5.html#mysqld-5-6-5-data-types and civicrm#10953 (comment) Note that we still support mysql 5.6 and the current version is 5.6.51 I think it's fine to require that the 5.6 minimum is 5.6.5 as released in 2012 I'll do the upgrade script as a follow up
On digging I determined this trigger hokiness was all about mysql 5.5 and we can ditch it now https://dev.mysql.com/doc/relnotes/mysql/5.6/en/news-5-6-5.html#mysqld-5-6-5-data-types and civicrm#10953 (comment) Note that we still support mysql 5.6 and the current version is 5.6.51 I think it's fine to require that the 5.6 minimum is 5.6.5 as released in 2012 I'll do the upgrade script as a follow up
… preventing cross edit of mailings
Overview
This adds in the column modified_date column to civicrm_mailing in part to help fix the ability to prevent cross tab edits of mailings. Pulling this bit out to try to help keep the other PR from going stale as often
Comments
@eileenmcnaughton @totten @JKingsnorth i think this is what was suggested on #10864