Convert pledge date fields to use datepicker rather than jcalendar #15170

seamuslee001 · 2019-08-30T22:51:51Z

Overview

This PR converts the pledge form dates to use date picker rather than jcalendar and in the process renames the field pledge_payment_date on search forms to be pledge_payment_scheduled_date

Before

Jcalendar date field used

After

date picker used

Sideeffects are

A bug whereby pledge based smart groups were not 'moving with the times' - saving relative date ranges - was fixed
search url params now work for these fields on the pledge search screen eg

civicrm/pledge/search?reset=1&pledge_end_date_relative=this.month&force=1
civicrm/pledge/search?reset=1&pledge_start_date_relative=this.month&force=1

ping @eileenmcnaughton

civibot · 2019-08-30T22:51:53Z

(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-08-31T22:23:30Z

CRM/Pledge/Form/Search.php

+      $this->addSearchFieldMetadata(['Pledge' => CRM_Pledge_BAO_Query::getSearchFieldMetadata()]);
+      $this->addSearchFieldMetadata(['PledgePayment' => CRM_Pledge_BAO_Query::getPledgePaymentSearchFieldMetadata()]);
+      $this->addFormFieldsFromMetadata();
+      $this->setFormValues();


So this is a change from the approach elsewhere - if I look at CRM_Contribute_Form_Search we are relying on this being done in parent::setDefaults (which would run here too) or in preProcess - which is not very 'mature' in that the forms are not sharing a function.

I'm gonna poke around a bit more on this one

I couldn't see how parent::setDefaults was setting the formValues where as setFormValues was calling setDefaults() and this is in the situation where we have &force=1 in the URL param

ok i think re-looking at the Contribute search this function is actually called in the postProcess function so i'll move it there

eileenmcnaughton · 2019-08-31T22:24:14Z

CRM/Pledge/BAO/Query.php

@@ -82,6 +82,12 @@ public static function select(&$query) {
      $query->_tables['civicrm_pledge'] = $query->_whereTables['civicrm_pledge'] = 1;
    }

+    if (!empty($query->_returnProperties['pledge_end_date'])) {


hmm this shouldn't be needed - I will look & try to see why it was

This was how i fixed an issue with the api_v3_SyntaxConformanceTest

OK - it's in SELECT not WHERE - which I guess is where we've been focussing cleanup

eileenmcnaughton · 2019-08-31T22:27:28Z

xml/schema/Pledge/Pledge.xml

    </html>
+    <uniqueName>pledge_end_date</uniqueName>
+    <uniqueTitle>Payments Ended Date</uniqueTitle>


OK this confused me - I think it should be pledge end date

@eileenmcnaughton why? i thought unique title was for the search form but title would be the more standard pledge end date

OK - I looks like it's unchanged in the UI so perhaps it's just me that thought pledge end made sense

eileenmcnaughton · 2019-08-31T22:51:58Z

@seamuslee001 so when all the metadata is being called all those _low & _high bits are obsolete (which is good because they do bad stuff). I've been trying this - but I haven't tried the payment one yet. Perhaps we should do all the xml changes first & merge?

diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 7a6b045758..37989284d0 100644
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -1584,7 +1584,13 @@ class CRM_Contact_BAO_Query {
 
     self::filterCountryFromValuesIfStateExists($formValues);
     // We shouldn't have to whitelist fields to not hack but here we are, for now.
-    $nonLegacyDateFields = ['participant_register_date_relative', 'receive_date_relative'];
+    $nonLegacyDateFields = [
+      'participant_register_date_relative',
+      'receive_date_relative',
+      'pledge_end_date_relative',
:...skipping...
diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 7a6b045758..37989284d0 100644
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -1584,7 +1584,13 @@ class CRM_Contact_BAO_Query {
 
     self::filterCountryFromValuesIfStateExists($formValues);
     // We shouldn't have to whitelist fields to not hack but here we are, for now.
-    $nonLegacyDateFields = ['participant_register_date_relative', 'receive_date_relative'];
+    $nonLegacyDateFields = [
+      'participant_register_date_relative',
+      'receive_date_relative',
+      'pledge_end_date_relative',
+      'pledge_start_date_relative',
+      'pledge_create_date_relative',
+    ];
     // Handle relative dates first
     foreach (array_keys($formValues) as $id) {
:...skipping...
diff --git a/CRM/Contact/BAO/Query.php b/CRM/Contact/BAO/Query.php
index 7a6b045758..37989284d0 100644
--- a/CRM/Contact/BAO/Query.php
+++ b/CRM/Contact/BAO/Query.php
@@ -1584,7 +1584,13 @@ class CRM_Contact_BAO_Query {
 
     self::filterCountryFromValuesIfStateExists($formValues);
     // We shouldn't have to whitelist fields to not hack but here we are, for now.
-    $nonLegacyDateFields = ['participant_register_date_relative', 'receive_date_relative'];
+    $nonLegacyDateFields = [
+      'participant_register_date_relative',
+      'receive_date_relative',
+      'pledge_end_date_relative',
+      'pledge_start_date_relative',
+      'pledge_create_date_relative',
+    ];
     // Handle relative dates first
     foreach (array_keys($formValues) as $id) {
       if (
diff --git a/CRM/Pledge/BAO/Query.php b/CRM/Pledge/BAO/Query.php
index 733e26abda..971993d66e 100644
--- a/CRM/Pledge/BAO/Query.php
+++ b/CRM/Pledge/BAO/Query.php
@@ -252,28 +252,6 @@ class CRM_Pledge_BAO_Query extends CRM_Core_BAO_Query {
     list($name, $op, $value, $grouping, $wildcard) = $values;
 
     switch ($name) {
-      case 'pledge_create_date_low':
-      case 'pledge_create_date_high':
-        // process to / from date
-        $query->dateQueryBuilder($values,
-          'civicrm_pledge', 'pledge_create_date', 'create_date', 'Pledge Made'
-        );
-      case 'pledge_start_date_low':
-      case 'pledge_start_date_high':
-        // process to / from date
-        $query->dateQueryBuilder($values,
-          'civicrm_pledge', 'pledge_start_date', 'start_date', 'Pledge Start Date'
-        );
-        return;
-
-      case 'pledge_end_date_low':
-      case 'pledge_end_date_high':
-        // process to / from date
-        $query->dateQueryBuilder($values,
-          'civicrm_pledge', 'pledge_end_date', 'end_date', 'Pledge End Date'
-        );
-        return;
-
       case 'pledge_payment_date_low':
       case 'pledge_payment_date_high':
         // process to / from date
diff --git a/CRM/Pledge/DAO/Pledge.php b/CRM/Pledge/DAO/Pledge.php
index baddf50a86..76d0e11dd2 100644
--- a/CRM/Pledge/DAO/Pledge.php
+++ b/CRM/Pledge/DAO/Pledge.php
@@ -6,7 +6,7 @@
  *
  * Generated from xml/schema/CRM/Pledge/Pledge.xml
  * DO NOT EDIT.  Generated by CRM_Core_CodeGen
- * (GenCodeChecksum:238d9af98168511a7e6694e3d30e533d)
+ * (GenCodeChecksum:a25cc68d8392b1d60d7179ca484b604a)
  */
 
 /**
@@ -427,6 +427,7 @@ class CRM_Pledge_DAO_Pledge extends CRM_Core_DAO {
           'description' => ts('The date the first scheduled pledge occurs.'),
           'required' => TRUE,
           'where' => 'civicrm_pledge.start_date',
+          'export' => TRUE,
           'table_name' => 'civicrm_pledge',
           'entity' => 'Pledge',
           'bao' => 'CRM_Pledge_BAO_Pledge',
@@ -498,6 +499,7 @@ class CRM_Pledge_DAO_Pledge extends CRM_Core_DAO {
           'title' => ts('Pledge End Date'),
           'description' => ts('Date this pledge finished successfully (total pledge payments equal to or greater than pledged amount).'),
           'where' => 'civicrm_pledge.end_date',
+          'export' => TRUE,
           'table_name' => 'civicrm_pledge',
           'entity' => 'Pledge',
           'bao' => 'CRM_Pledge_BAO_Pledge',
diff --git a/xml/schema/Pledge/Pledge.xml b/xml/schema/Pledge/Pledge.xml
index 28916aed10..73dfec5b20 100644
--- a/xml/schema/Pledge/Pledge.xml
+++ b/xml/schema/Pledge/Pledge.xml
@@ -196,6 +196,7 @@
     <type>datetime</type>
     <title>Pledge Start Date</title>
     <required>true</required>
+    <export>true</export>
     <comment>The date the first scheduled pledge occurs.</comment>
     <add>2.1</add>
     <html>
@@ -211,6 +212,7 @@
     <title>Pledge Made</title>
     <required>true</required>
     <import>true</import>
+    <export>true</export>
     <comment>When this pledge record was created.</comment>
     <add>2.1</add>
     <html>
@@ -248,6 +250,7 @@
     <name>end_date</name>
     <type>datetime</type>
     <title>Pledge End Date</title>
+    <export>true</export>
     <comment>Date this pledge finished successfully (total pledge payments equal to or greater than pledged amount).</comment>
     <add>2.1</add>
     <html>

seamuslee001 · 2019-08-31T23:21:19Z

possibly but the XML change will require the change to the return properties stuff tho. I guess i can see where your going with that

eileenmcnaughton · 2019-08-31T23:52:09Z

@seamuslee001 we need to stop them being converted because otherwise it saves a fixed date not a relative one & relies on magic that won't work to make relative

seamuslee001 · 2019-09-01T01:58:26Z

@eileenmcnaughton i think i got the relative dates sorted but i found that i still needed the code in the CRM/Pledge/BAO/Pledge.php for when you manually set the high and low fields

seamuslee001 · 2019-09-01T21:09:15Z

@eileenmcnaughton i have rebased this now that the xml changes have been merged

Add support for url variables Standardise search form as per contribute form Add in pledge payment xml change

eileenmcnaughton · 2019-09-02T01:14:46Z

test this please

eileenmcnaughton · 2019-09-02T01:39:33Z

In my tests I found

existing 'relative date range saved searches' using pledge fields were converting to hard-coded date searches on save - therefore none were 'working' & it was a moot point as to whether the conversion routine is working. Afterwards the relativeness is correctly maintained
This url works http://dmaster.local/civicrm/pledge/search?reset=1&pledge_end_date_relative=this.month&force=1 to load defaults but a similar one on advanced search did not. This can be out of scope as it is non-regressive

seamuslee001 · 2019-09-02T01:41:43Z

@eileenmcnaughton do we need to update civicrm_mapping_field and civicrm_uf_field at all?

eileenmcnaughton · 2019-09-02T01:44:29Z

UF Field is no - see

eileenmcnaughton · 2019-09-02T01:45:55Z

@seamuslee001 for mapping I believe you had to update ExportTest in order to change the field names - if so I guess we could update those fields in mapping_field.

For import I wouldn't touch it - the way that code works it's using labels instead of names!

eileenmcnaughton · 2019-09-02T02:06:24Z

@seamuslee001 I also figured out what has to happen for urlsearch params to work in advanced search - this makes it work for contribute

civicrm-core/CRM/Contact/Form/Search.php

Line 917 in f90f8d3

protected function loadMetadata() {

we should add activity, participant, pledge there & maybe others are 'ready' - grant I think

seamuslee001 · 2019-09-02T02:12:11Z

@eileenmcnaughton in terms of the mapping_field_id i just wonder if it needs to be name spaced, maybe should leave it and see if anyone jumps?

eileenmcnaughton · 2019-09-02T02:21:27Z

@seamuslee001 yes perhaps - I think we have in practice handled updates to export mappings sometimes & not been called out when we have missed a beat - probably because mappings are generally used infrequently and there is no major ramification of it not being updated, unlike smart groups which can have complex unexpected outcomes

eileenmcnaughton · 2019-09-02T03:45:00Z

yay

civibot bot added the master label Aug 30, 2019

seamuslee001 force-pushed the pledge_date_picker_conversion branch 3 times, most recently from 2e9af46 to e162553 Compare August 31, 2019 06:54

eileenmcnaughton reviewed Aug 31, 2019

View reviewed changes

seamuslee001 force-pushed the pledge_date_picker_conversion branch from e162553 to 18b01ec Compare August 31, 2019 22:36

seamuslee001 mentioned this pull request Sep 1, 2019

Update Pledge XML to support date picker conversion #15180

Merged

seamuslee001 force-pushed the pledge_date_picker_conversion branch from 5b99a12 to 15a4e98 Compare September 1, 2019 21:08

seamuslee001 added 2 commits September 2, 2019 07:10

Convert pledge date fields to use datepicker rather than jcalendar

1519c82

Add support for url variables Standardise search form as per contribute form Add in pledge payment xml change

Ensure that relative dates are handled in the newer way

c485300

seamuslee001 force-pushed the pledge_date_picker_conversion branch from 15a4e98 to c485300 Compare September 1, 2019 21:11

eileenmcnaughton added the merge on pass label Sep 2, 2019

seamuslee001 merged commit 5a67177 into civicrm:master Sep 2, 2019

seamuslee001 deleted the pledge_date_picker_conversion branch September 2, 2019 03:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert pledge date fields to use datepicker rather than jcalendar #15170

Convert pledge date fields to use datepicker rather than jcalendar #15170

seamuslee001 commented Aug 30, 2019 •

edited by eileenmcnaughton

Loading

civibot bot commented Aug 30, 2019

eileenmcnaughton Aug 31, 2019

seamuslee001 Aug 31, 2019

seamuslee001 Aug 31, 2019

eileenmcnaughton Aug 31, 2019

seamuslee001 Aug 31, 2019

eileenmcnaughton Aug 31, 2019

eileenmcnaughton Aug 31, 2019

seamuslee001 Aug 31, 2019

eileenmcnaughton Aug 31, 2019

eileenmcnaughton commented Aug 31, 2019 •

edited

Loading

seamuslee001 commented Aug 31, 2019

eileenmcnaughton commented Aug 31, 2019

seamuslee001 commented Sep 1, 2019

seamuslee001 commented Sep 1, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

seamuslee001 commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

seamuslee001 commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

Convert pledge date fields to use datepicker rather than jcalendar #15170

Convert pledge date fields to use datepicker rather than jcalendar #15170

Conversation

seamuslee001 commented Aug 30, 2019 • edited by eileenmcnaughton Loading

Overview

Before

After

civibot bot commented Aug 30, 2019

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

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

eileenmcnaughton commented Aug 31, 2019 • edited Loading

seamuslee001 commented Aug 31, 2019

eileenmcnaughton commented Aug 31, 2019

seamuslee001 commented Sep 1, 2019

seamuslee001 commented Sep 1, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

seamuslee001 commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

seamuslee001 commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

eileenmcnaughton commented Sep 2, 2019

seamuslee001 commented Aug 30, 2019 •

edited by eileenmcnaughton

Loading

eileenmcnaughton commented Aug 31, 2019 •

edited

Loading