Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(dev/core#1226) Fix failure to save relative dates on legacy fields #15194

Merged
merged 1 commit into from
Sep 3, 2019

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 2, 2019

Overview

Fixes a 5.16 regression whereby smart groups relative dates are converted to fixed dates for smart groups created on 5.16 with one of the affected fields in them.

This is an expansion of the previous update #15169.

Before

Groups with the fields
'log_date_relative',
'pledge_payment_date_relative',
'member_join_date_relative',
'member_start_date_relative',
'member_end_date_relative',
'birth_date_relative',
'deceased_date_relative',
'mailing_date_relative',
'relation_date_relative',
'relation_start_date_relative',
'relation_end_date_relative',
'relation_action_date_relative',

on 5.16 are created with the dates converted to fixed dates

After

Saves as relative dates

Technical Details

The regex was wrong in this change

7f17570#diff-54576153cb6a72d6ea7e22a5e8ca63f2R442

This switches to a whitelist of remaining legacy dates

Once merged to master a PR on master would remove converted fields from the list

Comments

I noticed that the custom fields are added differently for Contact custom fields and non-contact custom fields & non contact custom fields are using the jcalendar dates & likely didn't work before or after this. We should fix them. The Contact_BAO_Query object and the others don't have a shared parent & hence have 2 different functions. I think they should have a shared parent & Contact_BAO_Query should share with the others - if existing functions make that hard we can use a trait

@civibot
Copy link

civibot bot commented Sep 2, 2019

(Standard links)

@eileenmcnaughton
Copy link
Contributor Author

test this please

@seamuslee001
Copy link
Contributor

This makes sense to me, Adding merge on pass, suggest we add in a 5.17 upgrade message or some kind

@eileenmcnaughton
Copy link
Contributor Author

@totten despite the upcoming 5.17 I'd like to drop a 5.16 with this since it's a bit of a sleeper that could hit people

@eileenmcnaughton
Copy link
Contributor Author

Hmm test fail doesn't make sense as being related..

@eileenmcnaughton
Copy link
Contributor Author

Test fail unrelated as evidenced by #15197 - merging

Note that the test passes in isolation..

@eileenmcnaughton eileenmcnaughton merged commit 679ffbf into civicrm:5.17 Sep 3, 2019
@eileenmcnaughton eileenmcnaughton deleted the 517_dates branch September 3, 2019 05:24
@totten totten changed the title Fix failure to save relative dates on legacy fields (dev/core#1226) Fix failure to save relative dates on legacy fields Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants