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

Ensure that Relative key is not added for non Select Date is_search_range custom fields #15977

Conversation

seamuslee001
Copy link
Contributor

…ate range search

Overview

the relative key is only useful for Select Dates not any other is_search_range. Dates also used to be _to and _from but that has been changed

Before

Incorrectly adds in the relative key

After

Only adds relative key if still in old format for some reason and its a date custom field

ping @eileenmcnaughton let me know if you think this should be against the RC

@civibot
Copy link

civibot bot commented Nov 28, 2019

(Standard links)

@civibot civibot bot added the master label Nov 28, 2019
@seamuslee001 seamuslee001 force-pushed the fix_issue_number_range_smart_group_empty_values branch from 49b889e to f842c42 Compare November 28, 2019 03:26
// fields with 'from' or 'to' values are displayed when the are set in the smart group
// being loaded. (CRM-17116)
// We should only set the relative key for custom date fields if it is not already set in the array.
if (substr($element, 0, 7) == 'custom_' && CRM_Contact_BAO_Query::isCustomDateField($element)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this right - ie we don't need to strip _from & _to from $element before passing to isCustomDateField?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep you were right have fixed up

@eileenmcnaughton
Copy link
Contributor

I think the rc makes sense

Ensure that we strip the _to _from _high _low from the element name first
@seamuslee001 seamuslee001 force-pushed the fix_issue_number_range_smart_group_empty_values branch from f842c42 to c0b26fc Compare November 28, 2019 20:14
@seamuslee001 seamuslee001 changed the base branch from master to 5.20 November 28, 2019 20:14
@civibot civibot bot added 5.20 and removed master labels Nov 28, 2019
@seamuslee001
Copy link
Contributor Author

ok @eileenmcnaughton have put it against the RC and also expanded the unit test to verify for the select date that the function adds in the appropriate relative key as it isn't in the form values (odd ball case) so i think we are good now

@seamuslee001 seamuslee001 merged commit 64c4d39 into civicrm:5.20 Nov 28, 2019
@eileenmcnaughton eileenmcnaughton deleted the fix_issue_number_range_smart_group_empty_values branch November 28, 2019 21:20
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 the way @jtwyman described the impact of this it might be a candidate for a point release?

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