Get rid of jcalendar range in custom data

[eileenmcnaughton]

Overview

This is the last jcalendar field in search so it has to go. My testing (& recollection) suggests a conversion routine is not required for saved searches using these.

Before

Jcalendar

After

Datepicker rules KO

Technical Details

@seamuslee001 first attempt - the layout is a bit wonky in this first cut.

Comments

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[seamuslee001]

@eileenmcnaughton I tested this and agree the formatting looks a bit odd and i'm sure we can improve on that.

I also created a bunch of smart groups, If you use a relative search it will be fine as is thankfully, the stored form values are

;a:5:{i:0;s:17:"custom_7_relative";i:1;s:1:"=";i:2;s:10:"this.month";i:3;i:0;i:4;i:0;}

However if you select the choose date range option then things become a bit odd ball so to speak

The stored form values are

a:5:{i:0;s:17:"custom_7_relative";i:1;s:1:"=";i:2;s:1:"0";i:3;i:0;i:4;i:0;}i:5;a:5:{i:0;s:8:"custom_7";i:1;s:1:"=";i:2;a:1:{s:7:"BETWEEN";a:2:{i:0;s:14:"20191101000000";i:1;s:14:"20191130235959";}}

so the relative value is correct however we probably need to split out that custom_7 array within array into _lower and _higher. At least it looks like the dates are stored correctly

[eileenmcnaughton]

@seamuslee001 so the above is before the change - or before AND after?

[seamuslee001]

@eileenmcnaughton before the change only, when i switched to your branch the first one worked fine the 2nd correctly had the choose date range set but no values were populated

[eileenmcnaughton]

@seamuslee001 so we need a conversion routing for the second case.

However it seems we also need to resolve 2 issues maybe now on it working post convert

[seamuslee001]

@eileenmcnaughton do we want to get rid of this function also? https://github.com/civicrm/civicrm-core/blob/master/CRM/Contact/BAO/Query.php#L4733

[eileenmcnaughton]

I think so!

[eileenmcnaughton]

@seamuslee001 more cruft removal added - I think it might fix my double fields above

[seamuslee001]

@eileenmcnaughton i just tried testing this and when specifying a low value i got a fatal error

    [to_string] => [db_error: message="DB Error: no such field" code=-19 mode=callback callback=CRM_Core_Error::handle prefix="" info="SELECT DISTINCT LEFT(contact_a.sort_name, 1) as sort_name
       FROM civicrm_contact contact_a
      WHERE  ( civicrm_value_constituent_information_1.custom_7 >= '20191101000000' )  AND (contact_a.is_deleted = 0)

      GROUP BY sort_name
      ORDER BY sort_name asc [nativecode=1054 ** Unknown column 'civicrm_value_constituent_information_1.custom_7' in 'where clause']"]

Relevant parts of the backtrace

#10 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/packages/DB/DataObject.php(1607): DB_DataObject->_query("SELECT DISTINCT LEFT(contact_a.sort_name, 1) as sort_name\n       FROM civicr...")
#11 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Core/DAO.php(435): DB_DataObject->query("SELECT DISTINCT LEFT(contact_a.sort_name, 1) as sort_name\n       FROM civicr...")
#12 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Core/DAO.php(1428): CRM_Core_DAO->query("SELECT DISTINCT LEFT(contact_a.sort_name, 1) as sort_name\n       FROM civicr...", TRUE)
#13 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/BAO/Query.php(5022): CRM_Core_DAO::executeQuery("SELECT DISTINCT LEFT(contact_a.sort_name, 1) as sort_name\n       FROM civicr...")
#14 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/Selector.php(1194): CRM_Contact_BAO_Query->alphabetQuery()
#15 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Utils/PagerAToZ.php(108): CRM_Contact_Selector->alphabetQuery()
#16 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Utils/PagerAToZ.php(136): CRM_Utils_PagerAToZ::getDynamicCharacters(Object(CRM_Contact_Selector), FALSE)
#17 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Utils/PagerAToZ.php(52): CRM_Utils_PagerAToZ::createLinks(Object(CRM_Contact_Selector), NULL, FALSE)
#18 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/Form/Search.php(869): CRM_Utils_PagerAToZ::getAToZBar(Object(CRM_Contact_Selector), NULL)
#19 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/Form/Search/Advanced.php(320): CRM_Contact_Form_Search->postProcess()
#20 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Core/Form.php(495): CRM_Contact_Form_Search_Advanced->postProcess()

[seamuslee001]

Note that was on the previous iteration

[seamuslee001]

@eileenmcnaughton re-tested on your latest code and still get the same fatal error

[eileenmcnaughton]

@seamuslee001 this seems to be working for the date field now but it's ignoring the filter on marital status when in combo for me now - digging

[eileenmcnaughton]

ok @seamuslee001 I think the last version is all good except the formatting

[eileenmcnaughton]

@seamuslee001 we'd better test creating both a custom date relative field smart group & a integer (or other ) range one - I think the tests caught the probs but just followed a rabbit into a hole

[seamuslee001]

@eileenmcnaughton i tested both together and they seemed to be stored correctly and loaded right post change that is, Which makes me think the test rig might be problematic

[eileenmcnaughton]

@seamuslee001 that turned out to be a very rabbity hole! I think once the prequels to this are merged we can merge this and you convert script & be done

[seamuslee001]

@eileenmcnaughton needs a rebase now

[eileenmcnaughton]

Yep - rebased!

[seamuslee001]

Re tested this and confirmed that it works well and also confirmed that number range searches still work as expected. Merge on Pass

[eileenmcnaughton]

Wow - that's a lot of date wrangling code now gone from saved search & smart groups!