[REF] Fix regression where adding any date based field onto a profile…

[seamuslee001]

… triggers an error date preferences not configured when previewing the profile

Overview

This fixes recent regression where adding any date field onto a profile causes a fatal error when previewing that profile

Reproduction steps

1. Navigate to the Profiles
2. Click Fields next to the Name and Address profile
3. Add in a datepicker field onto the profile e.g. Revenue Recognition date
4. Try and preview the profile

Before

Fatal Error

After

No Fatal error

Technical Details

This is happening because the dateMetadata has already been added here https://github.com/civicrm/civicrm-core/blob/master/CRM/Core/BAO/UFGroup.php#L510 which includes setting the min year and max year appropriately

ping @demeritcowboy @eileenmcnaughton

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

[eileenmcnaughton]

@seamuslee001 I feel like the original sin here is we are trying to use one field to hold 2 different types of values

What about this

diff --git a/CRM/Core/Form.php b/CRM/Core/Form.php
index 7d48b6569e..8bcd41a573 100644
--- a/CRM/Core/Form.php
+++ b/CRM/Core/Form.php
@@ -391,7 +391,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
     // @see https://docs.civicrm.org/dev/en/latest/framework/ui/#date-picker
     if ($type === 'datepicker') {
       $attributes = $attributes ?: [];
-      if (!empty($attributes['format'])) {
+      if (!empty($attributes['date_preference'])) {
         $dateAttributes = CRM_Core_SelectValues::date($attributes['format'], NULL, NULL, NULL, 'Input');
         if (empty($extra['minDate']) && !empty($dateAttributes['minYear'])) {
           $extra['minDate'] = $dateAttributes['minYear'] . '-01-01';
@@ -1394,7 +1394,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
       $required,
       ['class' => 'crm-select2']
     );
-    $attributes = ['format' => 'searchDate'];
+    $attributes = ['date_preference' => 'searchDate'];
     $extra = ['time' => $isDateTime];
     $this->add('datepicker', $fieldName . $from, ts($fromLabel), $attributes, $required, $extra);
     $this->add('datepicker', $fieldName . $to, ts($toLabel), $attributes, $required, $extra);
~

[seamuslee001]

hmm you would also have to update the selectValues line and what about using formatType given that is what is in the xml?

[eileenmcnaughton]

So the previous method was to 'dress' the xml date data like this

          $fieldSpec = CRM_Utils_Date::addDateMetadataToField($fieldSpec, $fieldSpec);
          $attributes = ['format' => $fieldSpec['date_format']];

[eileenmcnaughton]

It wouldn't hit the selectValues in the other case I don't think

[seamuslee001]

it seems the profile output is doing that but not for where ever you had to add the attribute right? So my thinking is whatever array key we need to use then that has to be the array key passed to selectValues

[eileenmcnaughton]

@seamuslee001 so we have 2 scenarios

1. the date format looks like 'searchDate'
2. the date format looks like 'm/y/d' - this is the case for custom date fields

We should handle these by populating different keys before they hit $form->add() so that $form->add isn't munging them (or relying on a separate param (already processed) to disambiguate them

The current round of regressions came about because we were not handling minDate & maxDate in searches

[seamuslee001]

@seamuslee001 so we have 2 scenarios

1. the date format looks like 'searchDate'
2. the date format looks like 'm/y/d' - this is the case for custom date fields

We should handle these by populating different keys before they hit $form->add() so that $form->add isn't munging them (or relying on a separate param (already processed) to disambiguate them

The current round of regressions came about because we were not handling minDate & maxDate in searches

Sure but my point is that should we not for the first one pass in an array key of formatType rather than format given that is what key we have used in the xml to store those sorts of strings. My other point is that in your example code

$dateAttributes = CRM_Core_SelectValues::date($attributes['format'], NULL, NULL, NULL, 'Input');

Needs to change to be what ever attribute key we end up passing in

[eileenmcnaughton]

@seamuslee001 so the patch winds up looking something like

@@ -391,8 +391,8 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
     // @see https://docs.civicrm.org/dev/en/latest/framework/ui/#date-picker
     if ($type === 'datepicker') {
       $attributes = $attributes ?: [];
-      if (!empty($attributes['format'])) {
-        $dateAttributes = CRM_Core_SelectValues::date($attributes['format'], NULL, NULL, NULL, 'Input');
+      if (!empty($attributes['date_preference'])) {
+        $dateAttributes = CRM_Core_SelectValues::date($attributes['date_preference'], NULL, NULL, NULL, 'Input');
         if (empty($extra['minDate']) && !empty($dateAttributes['minYear'])) {
           $extra['minDate'] = $dateAttributes['minYear'] . '-01-01';
         }
@@ -1394,7 +1394,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
       $required,
       ['class' => 'crm-select2']
     );
-    $attributes = ['format' => 'searchDate'];
+    $attributes = ['date_preference' => 'searchDate'];
     $extra = ['time' => $isDateTime];
     $this->add('datepicker', $fieldName . $from, ts($fromLabel), $attributes, $required, $extra);
     $this->add('datepicker', $fieldName . $to, ts($toLabel), $attributes, $required, $extra);
@@ -1649,7 +1649,7 @@ class CRM_Core_Form extends HTML_QuickForm_Page {
         }
         else {
           $fieldSpec = CRM_Utils_Date::addDateMetadataToField($fieldSpec, $fieldSpec);
-          $attributes = ['format' => $fieldSpec['html']['formatType']];
+          $attributes = ['format' => $fieldSpec['date_format']];
           return $this->add('datepicker', $name, $label, $attributes, $required, $fieldSpec['datepicker']['extra']);
   

because you need to back out the change in addForm & narrow the min max part down to kick in where it's needed

[seamuslee001]

any reason why you want date_preference rather than format_type?

[eileenmcnaughton]

DatePreference is an entity

[seamuslee001]

I guess it just feels better to me to use format_type because that is what the metadata will use but 6/1 (1/2)/12

[eileenmcnaughton]

Yeah - formatType is OK too

[seamuslee001]

updated now @eileenmcnaughton

[eileenmcnaughton]

The fatal has gone for me with this change.

[eileenmcnaughton]

Fails all look familiar / unrelated