Convert activity_date_time field to datepicker and add support for url input #13746

eileenmcnaughton · 2019-03-02T06:09:43Z

Overview

Convert activity_date_time field on activity search page to use datepicker, add support for parameters to this in the url

Before

Deprecated jcalendar

After

Preferred datepicker & metadata approaches in use. Generic url support added with tight validation

Technical Details

This converts the form to being a metadata defined field & a date-picker field as well
as adding standardised url support for input.

The input format is
activity_date_time_high=20100101

I stripped out the complex and probably broken based on feedback & not used in core
existing hard-coded url support.

Note that there are some outstanding items

I updated the check to ensure the high date is greater than the low date
-but it DOES display more than once if it fails - I think we could hone that later

Comments

@monishdeb @colemanw you both seem to be working on making search form changes that can be solved by adopting a generic approach - here is how I see it working for the activity date time field. We should be able to update similar fields like 'contribution receive date', membership start date , birth date etc with less code change as the Core_Form & Form_Search changes are to support time in datepicker ranges & url support for date fields (respectively)

civibot · 2019-03-02T06:09:45Z

(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).

eileenmcnaughton · 2019-03-02T06:10:18Z

CRM/Activity/BAO/Query.php

@@ -440,7 +442,7 @@ public static function from($name, $mode, $side) {
   * rather than a static function.
   */
  public static function getSearchFieldMetadata() {
-    $fields = ['activity_type_id'];
+    $fields = ['activity_type_id', 'activity_date_time'];


this is the key change - adding the field to this array means it can be removed from other places

eileenmcnaughton · 2019-03-02T06:10:27Z

CRM/Activity/BAO/Query.php

@@ -288,8 +288,10 @@ public static function whereClauseSingle(&$values, &$query) {
      case 'activity_date':
      case 'activity_date_low':
      case 'activity_date_high':
+      case 'activity_date_time_low':


because I renamed the field to be more std

eileenmcnaughton · 2019-03-02T06:11:11Z

CRM/Activity/BAO/Query.php

-   *
-   * @return bool|array
-   */
-  public static function formRule($fields, $files, $form) {


We can re-add something generic for these if we think it's important (I'm inclined to punt a bit on this since I think it's not currently consistently done for date fields in search which makes is seem not that important)

eileenmcnaughton · 2019-03-02T06:11:57Z

CRM/Contact/BAO/Query.php

@@ -1899,6 +1899,8 @@ public function whereClauseSingle(&$values, $apiEntity = NULL) {
      case 'activity_date':
      case 'activity_date_low':
      case 'activity_date_high':
+      case 'activity_date_time_low':


because of field renaming

eileenmcnaughton · 2019-03-02T06:12:15Z

CRM/Core/Form.php

@@ -1298,8 +1298,10 @@ public function addDateRange($name, $from = '_from', $to = '_to', $label = 'From
   * @param bool $required
   * @param string $fromLabel
   * @param string $toLabel
+   * @param bool $isDateTime


Add time support to addDatePickerRange fn

eileenmcnaughton · 2019-03-02T06:12:29Z

CRM/Core/Form/Search.php

-        if ($fieldSpec['type'] === CRM_Utils_Type::T_DATE) {
-          // Assuming time is false for now as we are not checking for date-time fields as yet.
-          $this->addDatePickerRange($fieldName, $fieldSpec['title'], FALSE);
+        if ($fieldSpec['type'] === CRM_Utils_Type::T_DATE || $fieldSpec['type'] === (CRM_Utils_Type::T_DATE + CRM_Utils_Type::T_TIME)) {


Add time support

eileenmcnaughton · 2019-03-02T06:13:10Z

CRM/Core/Form/Search.php

@@ -184,6 +183,10 @@ protected function getValidationTypeForField($entity, $fieldName) {
      case CRM_Utils_Type::T_INT:
        return 'CommaSeparatedIntegers';

+      case CRM_Utils_Type::T_DATE:


Add validation support - we validate these fields as 20190212 or 20190212123000

seamuslee001 · 2019-03-03T00:42:49Z

@eileenmcnaughton do we need to do any smart group conversion here?

eileenmcnaughton · 2019-03-03T03:06:30Z

@seamuslee001 yes - see outstanding items above - we actually need a routine to RENAME the key first & then convert it - or vice versa

monishdeb · 2019-03-03T05:23:23Z

CRM/Core/Form/Search.php

@@ -204,6 +207,15 @@ protected function getEntityDefaults($entity) {
        if ($value !== FALSE) {
          $defaults[$fieldSpec['name']] = $value;
        }
+        if ($fieldSpec['type'] === CRM_Utils_Type::T_DATE || ($fieldSpec['type'] === CRM_Utils_Type::T_DATE + CRM_Utils_Type::T_TIME)) {
+          $low = CRM_Utils_Request::retrieveValue($fieldSpec['name'] . '_low', 'Timestampe', FALSE, NULL, 'GET');


@eileenmcnaughton I think we should also include '_from'/'_to' names?

Since we'll be converting the fields one by one hopefully we can just standardise them in the process.

monishdeb · 2019-03-03T05:29:24Z

CRM/Activity/Form/Search.php

-      $this->_formValues['activity_date_high'] = $dateHigh;
-      $this->_defaults['activity_date_high'] = $dateHigh;
-    }
-


on grep this datelow/high is only used in Activity search form as url arguments, so its safe to remove.

eileenmcnaughton · 2019-03-03T20:15:47Z

@seamuslee001 I added the upgrade routine

eileenmcnaughton · 2019-03-04T01:01:17Z

I've made some updates to improve the conversion routine & also put in a form rule - the form rule runs twice but as an annoyance this is a real edge case (ie. it might annoy you to see the message twice if you misenter the date but not that many people would hit it & even less would care)

@colemanw I found that I needed to do some more conversion work on relative dates for the activity field - I don't know how well relative dates convert on the age as of field - my guess is they don't work & maybe even shouldn;t

It would be good to get someone else try to hammer this a bit & see if they can find ways that it breaks - it would be good to get it in the rc then we could add some more date fields to the next release & we would have the twin goals of getting rid of jcalender & adding url support for datefields in hand. I think we could also fix the open bug on custom fields with relative dates in the process

monishdeb · 2019-03-04T02:55:28Z

CRM/Contact/Form/Search/Basic.php

@@ -200,7 +200,7 @@ public function postProcess() {
   *
   * @return array|bool
   */
-  public static function formRule($fields) {
+  public static function  formRule($fields, $files, $form) {


@eileenmcnaughton extra space here

eileenmcnaughton · 2019-03-05T20:31:41Z

@monishdeb @colemanw @seamuslee001 does anyone have time to check this before the rc is cut - otherwise I'll need to change the upgrade script

…l input This converts the form to being a metadata defined field & a date-picker field as well as adding standardised url support for input. The input format is activity_date_time_high=20100101 I stripped out the complex and probably broken based on feedback & not used in core existing hard-coded url support. Note that there are some outstanding items 1) the upgrade script item needs to be added - this includes dealing with renaming of the field since I made it more consistent 2) I stripped out the check to ensure the high date is greater than the low date - if we still want this we should re-add as a generic thing. My guess is it's a bit optional

Old format doesn't seem to have it

seamuslee001 · 2019-03-05T21:30:36Z

(CiviCRM Review Template WORD-1.2)

General standards
- (r-explain) Pass
- (r-user) Pass
- (r-doc) Pass
- (r-run) Pass i created 2 separate smart groups 1 with a relative date and one an exact date and confirmed that after running the upgrade they were both correctly converted and loaded correctly. I also confirmed the form rule works as expected.
Developer standards
- (r-tech) Pass
- (r-code) Pass
- (r-maint) Pass
- (r-test) Pass

seamuslee001 · 2019-03-05T22:41:04Z

Merging as per my review and the tag

eileenmcnaughton · 2019-03-05T22:42:13Z

thanks @seamuslee001 hopefully this will make future conversions easier

civicrm#13746 adds a formRule function to CRM_Core_Form_Search which is the parent of CRM_Case_Form_Search. It already has a formRule function but the signatures don't match.

civicrm#13746 adds a formRule function to CRM_Core_Form_Search which is the parent of CRM_Case_Form_Search. It already has a formRule function but the signatures don't match. argh

civicrm#13746 adds a formRule function to CRM_Core_Form_Search which is the parent of CRM_Case_Form_Search. It already has a formRule function but the signatures don't match.

civibot bot added the master label Mar 2, 2019

eileenmcnaughton commented Mar 2, 2019

View reviewed changes

eileenmcnaughton force-pushed the activity_search branch 2 times, most recently from e5da5fb to 859ebc3 Compare March 2, 2019 06:16

monishdeb reviewed Mar 3, 2019

View reviewed changes

eileenmcnaughton force-pushed the activity_search branch from 859ebc3 to a1c5b0f Compare March 3, 2019 20:10

eileenmcnaughton force-pushed the activity_search branch 3 times, most recently from efd2dc5 to ecda3b1 Compare March 4, 2019 00:14

monishdeb reviewed Mar 4, 2019

View reviewed changes

eileenmcnaughton force-pushed the activity_search branch from 5d0ec9f to 3466488 Compare March 4, 2019 03:27

eileenmcnaughton added 6 commits March 6, 2019 10:27

Add upgrade routine

7bab335

Fix group conversion to add relative key

929d5a1

Old format doesn't seem to have it

Add form rule for chronological ordeR

2307be0

Fix related search classes to have the same signature

d7ea25c

Add enotice handling for weird s....'

8e80025

eileenmcnaughton force-pushed the activity_search branch from 3466488 to 8e80025 Compare March 5, 2019 21:30

seamuslee001 added the merge on pass label Mar 5, 2019

seamuslee001 merged commit f043815 into civicrm:master Mar 5, 2019

eileenmcnaughton deleted the activity_search branch March 5, 2019 22:41

demeritcowboy mentioned this pull request Apr 8, 2019

PHP warnings on Case Dashboard and Find Cases demeritcowboy/civicrm-core#8

Merged

demeritcowboy mentioned this pull request Apr 8, 2019

PHP warnings on Case Dashboard and Find Cases #13998

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert activity_date_time field to datepicker and add support for url input #13746

Convert activity_date_time field to datepicker and add support for url input #13746

eileenmcnaughton commented Mar 2, 2019 •

edited

Loading

civibot bot commented Mar 2, 2019

eileenmcnaughton Mar 2, 2019

eileenmcnaughton Mar 2, 2019

eileenmcnaughton Mar 2, 2019 •

edited

Loading

eileenmcnaughton Mar 2, 2019

eileenmcnaughton Mar 2, 2019

eileenmcnaughton Mar 2, 2019

eileenmcnaughton Mar 2, 2019

seamuslee001 commented Mar 3, 2019

eileenmcnaughton commented Mar 3, 2019

monishdeb Mar 3, 2019

eileenmcnaughton Mar 3, 2019

monishdeb Mar 3, 2019

eileenmcnaughton commented Mar 3, 2019

eileenmcnaughton commented Mar 4, 2019

monishdeb Mar 4, 2019

eileenmcnaughton Mar 4, 2019

eileenmcnaughton commented Mar 5, 2019

seamuslee001 commented Mar 5, 2019

seamuslee001 commented Mar 5, 2019

eileenmcnaughton commented Mar 5, 2019

Convert activity_date_time field to datepicker and add support for url input #13746

Convert activity_date_time field to datepicker and add support for url input #13746

Conversation

eileenmcnaughton commented Mar 2, 2019 • edited Loading

Overview

Before

After

Technical Details

Comments

civibot bot commented Mar 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton Mar 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seamuslee001 commented Mar 3, 2019

eileenmcnaughton commented Mar 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Mar 3, 2019

eileenmcnaughton commented Mar 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton commented Mar 5, 2019

seamuslee001 commented Mar 5, 2019

seamuslee001 commented Mar 5, 2019

eileenmcnaughton commented Mar 5, 2019

eileenmcnaughton commented Mar 2, 2019 •

edited

Loading

eileenmcnaughton Mar 2, 2019 •

edited

Loading