dev/core#561 Convert mailing date search field to using datepicker #15633

seamuslee001 · 2019-10-28T02:16:34Z

Overview

This converts the mailing_date field on the Mailing subsection of advanced search to using date picker rather than jcalender

Before

Jcalender is used

After

Date picker is used

ping @monishdeb @eileenmcnaughton

Note that this isn't fully complete the one thing that doesn't seem to be working at this point is that the where clause doesn't seem to be getting run correctly but am not sure why

https://lab.civicrm.org/dev/core/issues/561

civibot · 2019-10-28T02:16:37Z

(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-10-28T02:19:15Z

@seamuslee001 also this should go

index e53b6bf5fd..4215c09b73 100644
--- a/CRM/Contact/BAO/SavedSearch.php
+++ b/CRM/Contact/BAO/SavedSearch.php
@@ -402,7 +402,6 @@ LEFT JOIN civicrm_email ON (contact_a.id = civicrm_email.contact_id AND civicrm_
'log_date_relative',
'birth_date_relative',
'deceased_date_relative',

 'mailing_date_relative',
 'relation_date_relative',
 'relation_start_date_relative',
 'relation_end_date_relative',

I'll do some testing....

Some more cleanup and also open tab if specified in the URL

eileenmcnaughton · 2019-10-28T02:23:15Z

hmm also here

+++ b/CRM/Mailing/Form/Search.php
@@ -121,20 +121,10 @@ class CRM_Mailing_Form_Search extends CRM_Core_Form_Search {
   public function postProcess() {
     $params = $this->controller->exportValues($this->_name);
 
-    if (!empty($params['mailing_relative'])) {
-      list($params['mailing_low'], $params['mailing_high']) = CRM_Utils_Date::getFromTo($params['mailing_relative'], $params['mailing_low'], $params['mailing_high']);
-      unset($params['mailing_relative']);
-    }
-    elseif (!empty($params['mailing_high'])) {
-      $params['mailing_high'] .= ' ' . '23:59:59';
-    }
-
     $parent = $this->controller->getParent();
     if (!empty($params)) {
       $fields = [
         'mailing_name',
-        'mailing_low',
-        'mailing_high',
         'sort_name',
         'campaign_id',
         'mailing_status',

seamuslee001 · 2019-10-28T02:24:58Z

hmm also here

+++ b/CRM/Mailing/Form/Search.php
@@ -121,20 +121,10 @@ class CRM_Mailing_Form_Search extends CRM_Core_Form_Search {
   public function postProcess() {
     $params = $this->controller->exportValues($this->_name);
 
-    if (!empty($params['mailing_relative'])) {
-      list($params['mailing_low'], $params['mailing_high']) = CRM_Utils_Date::getFromTo($params['mailing_relative'], $params['mailing_low'], $params['mailing_high']);
-      unset($params['mailing_relative']);
-    }
-    elseif (!empty($params['mailing_high'])) {
-      $params['mailing_high'] .= ' ' . '23:59:59';
-    }
-
     $parent = $this->controller->getParent();
     if (!empty($params)) {
       $fields = [
         'mailing_name',
-        'mailing_low',
-        'mailing_high',
         'sort_name',
         'campaign_id',
         'mailing_status',

@eileenmcnaughton i believe that is not connected to the advanced search form as per #15181 (comment)

eileenmcnaughton · 2019-10-28T02:37:06Z

Hmm - ok.

My testing shows relative date not working. I think we need to put it in the metadata

--- a/CRM/Mailing/BAO/Query.php
+++ b/CRM/Mailing/BAO/Query.php
@@ -32,21 +32,23 @@
  */
 class CRM_Mailing_BAO_Query {
 
-  public static $_mailingFields = NULL;
-
   /**
-   * @return array|null
+   * Get fields for the mailing & mailing job entity.
+   *
+   * @return array
    */
   public static function &getFields() {
-    if (!self::$_mailingFields) {
-      self::$_mailingFields = [];
-      $_mailingFields['mailing_id'] = [
-        'name' => 'mailing_id',
-        'title' => ts('Mailing ID'),
-        'where' => 'civicrm_mailing.id',
-      ];
-    }
-    return self::$_mailingFields;
+    $mailingFields = CRM_Mailing_BAO_Mailing::fields();
+    $mailingJobFields = CRM_Mailing_BAO_MailingJob::fields();
+
+    // In general it's good to return as many fields as could possibly be searched, but
+    // with the limitation that if the fields do not have unique names they might
+    // clobber other fields :-(
+    $fields = [
+      'mailing_id' => $mailingFields['id'],
+      'mailing_job_start_date' => $mailingJobFields['mailing_job_start_date'],
+    ];
+    return $fields;
   }

Although the join thing still needs fixing

eileenmcnaughton · 2019-10-28T03:02:56Z

@seamuslee001 I think this is the missing piece on the join issue https://github.com/civicrm/civicrm-core/pull/15634/files#diff-eff49b7e41fa6352d12d539c2e101ac7R472

eileenmcnaughton · 2019-10-28T03:04:19Z

@seamuslee001 I'm OK for this to be merged as is but we definitely need to follow up with a further fix per ^^

seamuslee001 · 2019-10-28T03:21:02Z

@eileenmcnaughton i tested your fix and that fixes it on relative dates but i get an unknown column error when doing it in the choose date field it builds an example query like

    [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   LEFT JOIN civicrm_mailing_recipients ON civicrm_mailing_recipients.contact_id = contact_a.id  LEFT JOIN civicrm_mailing ON civicrm_mailing.id = civicrm_mailing_recipients.mailing_id  LEFT JOIN civicrm_mailing_job ON civicrm_mailing_job.mailing_id = civicrm_mailing.id AND civicrm_mailing_job.is_test != 1
      WHERE  ( civicrm_mailing_job.mailing_job_start_date <= '20191002235959' )  AND (contact_a.is_deleted = 0)

      GROUP BY sort_name
      ORDER BY sort_name asc [nativecode=1054 ** Unknown column 'civicrm_mailing_job.mailing_job_start_date' in 'where clause']"]
)

Relative parts of the backtrace

#12 /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Core/DAO.php(1416): 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(5015): 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(868): 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()

eileenmcnaughton · 2019-10-28T03:26:01Z

hmm - this is without your change? I don't see it but perhaps without results I'm not getting pagerAtoZ running - will try some more data

seamuslee001 · 2019-10-28T03:27:06Z

this is with my change and with your changes applied ontop

eileenmcnaughton · 2019-10-28T03:28:07Z

ah - I'm currently testing with just this one - once I get my data right I'll switch to that

eileenmcnaughton · 2019-10-28T03:45:45Z

@seamuslee001 OK - I might have just pushed a fix for that

seamuslee001 · 2019-10-28T03:48:55Z

@eileenmcnaughton yep that works also just pushed a revised version incorporating the changes to CRM/Contact/BAO/SavedSearch.php but also expanding the list of known mailing fields in advanced search in CRM/Contact/Form/Search.php

eileenmcnaughton · 2019-10-28T03:49:44Z

@seamuslee001 so we should close this?

seamuslee001 · 2019-10-28T03:50:06Z

@eileenmcnaughton no just force pushed onto this pr was that i meant

eileenmcnaughton · 2019-10-28T03:51:39Z

Ah OK - this should be good to merge with the other merged - MOP

civibot bot added the master label Oct 28, 2019

dev/core#561 Convert mailing date search field to using datepicker

0058ef3

Some more cleanup and also open tab if specified in the URL

seamuslee001 force-pushed the dev_core_561_mailing branch from fccd6c4 to 0058ef3 Compare October 28, 2019 03:48

seamuslee001 mentioned this pull request Oct 28, 2019

Improve metadata support for table civicrm_mailing_job in search #15634

Merged

eileenmcnaughton added the merge on pass label Oct 28, 2019

seamuslee001 merged commit 7c7b5f6 into civicrm:master Oct 28, 2019

seamuslee001 deleted the dev_core_561_mailing branch October 28, 2019 04:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev/core#561 Convert mailing date search field to using datepicker #15633

dev/core#561 Convert mailing date search field to using datepicker #15633

seamuslee001 commented Oct 28, 2019 •

edited by eileenmcnaughton

Loading

civibot bot commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

dev/core#561 Convert mailing date search field to using datepicker #15633

dev/core#561 Convert mailing date search field to using datepicker #15633

Conversation

seamuslee001 commented Oct 28, 2019 • edited by eileenmcnaughton Loading

Overview

Before

After

civibot bot commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019

eileenmcnaughton commented Oct 28, 2019

seamuslee001 commented Oct 28, 2019 •

edited by eileenmcnaughton

Loading