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

CRM-21623 - Smart Group Relative Date not saving for some fields in A… #11486

Merged
merged 1 commit into from
Jan 21, 2018

Conversation

jitendrapurohit
Copy link
Contributor

@jitendrapurohit jitendrapurohit commented Jan 4, 2018

…dvance Search

Overview

Fix Smart groups to save relative dates correctly.

Before

Some relative_date fields on the Advance search are not saved correctly in civicrm_saved_search table. Event Pane is broken after returning from the smart group form.

image

After

works fine. Event dates correctly stored as form_values in civicrm_saved_search table.

Comments

Added unit test.


@seamuslee001
Copy link
Contributor

Jenkins test this please

@seamuslee001
Copy link
Contributor

I tested this and confirmed that relative dates were correctly saved after applying this patch. @jitendrapurohit is there any thoughts about how we should handle already saved smart groups or should we just advise people they would need to be re-saved?

@jitendrapurohit
Copy link
Contributor Author

If I recall any smart group issue created in the past for relative dates, existing smart groups can only be fixed by resaving them. So it should be the same here too.

Thanks for reviewing @seamuslee001

@philmb
Copy link
Contributor

philmb commented Jan 10, 2018

This patch seemed to be working for me, but today I had this problem: A smart group of "upcoming participants" should have a relative date (Event Date = "from start of today") plus the following criteria:

  • Participant Status (ID) In Registered, Pending (pay later), Pending (incomplete transaction), Partially paid ...AND...
  • Participant Role (ID) In Participant, Trip Leader ...AND...
  • Participant is not a Test

After saving the relative date, the other criteria are completely lost. If I save the group with a hard date, everything saves fine. This applies both to a smart group created pre-patch and to a new saved search.

The smart groups that I had originally tested this patch on last week (which continue to work properly) only included relative date and other group membership criteria.

Copy link

@drahdiwaberl drahdiwaberl left a comment

Choose a reason for hiding this comment

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

from what i see in the UI (search criteria correctly reflecting the 'last week' i set instead of a fixed 'to' date only) it seems to be working with this fix.

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 can you clarify your comment above. This appears to be reviewed & tested but you have raised the question of whether an upgrade action is required. Is this because without an action this patch will break some groups? Or because groups can now be unbroken & we should advise people (ie. is there no bad consequence merging this with no action?)

@seamuslee001
Copy link
Contributor

Hi @eileenmcnaughton it is certainly the latter. Any smart groups that were saved broken can now be fixed by being resaved. I figure we should let people know but can be done in follow up pr

@eileenmcnaughton eileenmcnaughton merged commit c279c21 into civicrm:master Jan 21, 2018
@eileenmcnaughton
Copy link
Contributor

thanks @seamuslee001 - merged & thanks @jitendrapurohit for another nice fix with a UNIT TEST :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants