Ensure relative dates are preserved for custom fields in smart group #12824
Conversation
|
Is this failing because it is using "custom_13" to run the unit test? We will try this on our local dev. |
|
Ah - thanks for the feedback. Is there a protocol I should use? I could replace it with a very high number to avoid conflicts. Or, do I need to actually create a custom field and then use the number that I created? |
|
@jmcclelland yes you will need to create the custom group + custom field and you should delete them after the last assert you can see https://github.com/civicrm/civicrm-core/blob/master/tests/phpunit/api/v3/CustomFieldTest.php for examples on using API to create custom group etc |
|
We applied the patch on our dev and it doesn't seem to fix the issue for us. The dates do not remain relative after creating the smart group. |
|
@sleewok - Weird. I'm not sure why it is not working for you. I just made a screencast demonstrating it working for me: Can you take a look and let me know if we are doing different things to test? @seamuslee001 thanks for the tip. I will rework the test. |
This change helps fix problems with unit tests. https://lab.civicrm.org/dev/core/issues/389
|
it is going to fail again because of this: |
|
I tested again, and it works! It appears the file is modified for version 5.5 and the patch was not being applied properly on my dev running 5.5 beta1. |
|
Is the unit test necessary? Maybe remove it so you can get this passed. |
|
@sleewok yes a unit test for this kind of change would be needed to ensure we lock in the fix. |
|
@sleewok i should also note that the most recent run of unit test is equivalent to a "pass" (tests are failing for environmental reasons following test boxes rebuild) and i note the unit test @jmcclelland actually passed https://test.civicrm.org/job/CiviCRM-Core-PR/22446/testReport/(root)/CRM_Contact_BAO_SavedSearchTest/testCustomFieldRelativeDates/ |
|
Thanks @seamuslee001 for the explanation - I think this means we should wait until the test boxes are fixed and then request that this be tested again to ensure it's not my test causing the problems with the other tests. |
|
test this please |
1 similar comment
|
test this please |
|
this seems to have had adequate testing by @sleewok & it has test coverage. I'm expecting the tests should be OK now & we can merge |
|
unrelated fails |
|
I believe there's still an issue with this. Although the relative dates are initially correct, once the relative time period has passed (ie. 1 week) the Smart Group Criteria is no longer correct. For example, I created a smart group with a custom field checking for dates matching "This Week". After 1 week had passed, I went back in and it returns 0 results (it should be returning some results). The message after clicking edit smart group search criteria shows "No matches found for: However, when I go in and look at the Welcome Card Date field (under edit criteria) it still says, "This week". If I hit search (without making any changes) it searches using the correct parameters. |
|
This happens on my install as well. Easy to see why it was missed since it doesn't show up until the period has passed. |
See https://lab.civicrm.org/dev/core/issues/389
Overview
Saving smart groups with custom fields in which a relative date is specified does not work as expected. The date range is hard coded and does not updated when re-run on a different day.
Before
When you create a smart group using a custom date field and you choose a relative date option (e.g. This calendar year), the field is saved with the hard-coded dates for the current time, rather than the relative date range specified.
After
With this patch in place, relative dates are properly saved and displayed to the user.