Core/dev#692 : Support search arguments to Mailing Search form #15369

monishdeb · 2019-10-01T11:57:08Z

Overview

This PR extends the fix and make all possible mailing search filters to be used as a url arguments on forced search using 'Find Mailing' or 'Advance Search'

Before

Search arguments didn't work

After

Search arguments work

Technical Details

Brings some additional imporvement:

Add 'YesNo' html type which renders boolean Yes/No field
Adds HTML properties to Mailing and MailingJob columns
Fixes bug in processSpecialFormValue()

civibot · 2019-10-01T11:57:11Z

(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 · 2019-10-27T20:59:33Z

Jenkins re test this please

seamuslee001 · 2019-10-27T21:10:53Z

@monishdeb I gave this a test and its not quite there

In Advanced Search with the Advanced search Pane enabled for mailings, the mailing job status selector has gone from the top right
When i did a search using the Advanced search form i got the following notices

Notice: Trying to get property '_sms' of non-object in CRM_Mailing_BAO_Query::buildSearchForm() (line 430 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Mailing/BAO/Query.php).
Notice: Undefined index: status_id in CRM_Contact_BAO_Query::processSpecialFormValue() (line 6306 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/BAO/Query.php).
Notice: Undefined index: priority_id in CRM_Contact_BAO_Query::processSpecialFormValue() (line 6306 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Contact/BAO/Query.php).
Notice: Trying to get property '_sms' of non-object in CRM_Mailing_BAO_Query::buildSearchForm() (line 430 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/CRM/Mailing/BAO/Query.php).

I note that for advanced searches at least the mailing status field is switching from a single select to multi select which might be of an issue for smart groups

@eileenmcnaughton

seamuslee001 · 2019-10-27T21:11:08Z

Also there is a style issue here

eileenmcnaughton · 2019-10-28T02:09:35Z

@monishdeb following @seamuslee001 initial review I have pulled out some parts of this & gotten them merged which should simplify - but also means it is stale

monishdeb · 2019-10-29T11:55:24Z

@eileenmcnaughton @seamuslee001 thanks for moving out parts of the codes in seperate PRs and getting those merged. I have rebased my PR and seems like its good to merge. Can you please check now?

eileenmcnaughton · 2019-10-29T19:17:55Z

CRM/Core/Form.php

@@ -1677,6 +1677,10 @@ public function addField($name, $props = [], $required = FALSE, $legacyDate = TR
        $props['size'] = isset($props['size']) ? $props['size'] : 60;
        return $this->add('password', $name, $label, $props, $required);

+      case 'YesNo':


@seamuslee001 we should work through this part first - the YesNo is a new thing for addField - it seems to be a specialised Radio - e.g also used for Contribution Is Test

@eileenmcnaughton yeh its kind of a nicer looking radio i guess or checkbox.

seems like its used in a number of places

CRM/Contact/Form/Search/Criteria.php: $form->addYesNo('uf_user', ts('CMS User?'), TRUE); CRM/Contact/Form/Search/Criteria.php: $form->addYesNo('is_deceased', ts('Deceased'), TRUE); CRM/Pledge/BAO/Query.php: $form->addYesNo('pledge_test', ts('Pledge is a Test?'), TRUE); CRM/Pledge/BAO/Query.php: $form->addYesNo('pledge_acknowledge_date_is_not_null', ts('Acknowledgement sent?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_thankyou_date_is_not_null', ts('Thank-you sent?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_receipt_date_is_not_null', ts('Receipt sent?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_pay_later', ts('Contribution is Pay Later?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_recurring', ts('Contribution is Recurring?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_test', ts('Contribution is a Test?'), TRUE); CRM/Contribute/BAO/Query.php: $form->addYesNo('contribution_pcp_display_in_roll', ts('Personal Campaign Page Honor Roll?'), TRUE); CRM/Activity/BAO/Query.php: $form->addYesNo('activity_test', ts('Activity is a Test?')); CRM/Member/BAO/Query.php: $form->addYesNo('membership_is_current_member', ts('Current Member?'), TRUE); CRM/Member/BAO/Query.php: $form->addYesNo('member_is_primary', ts('Primary Member?'), TRUE); CRM/Member/BAO/Query.php: $form->addYesNo('member_pay_later', ts('Pay Later?'), TRUE); CRM/Member/BAO/Query.php: $form->addYesNo('member_test', ts('Membership is a Test?'), TRUE); CRM/Member/BAO/Query.php: $form->addYesNo('member_is_override', ts('Membership Status Is Overriden?'), TRUE); CRM/Event/Form/ManageEvent/Registration.php: $form->addYesNo('is_confirm_enabled', ts('Use a confirmation screen?'), NULL, NULL, ['onclick' => "return showHideByValue('is_confirm_enabled','','confirm_screen_settings','block','radio',false);"]); CRM/Event/Form/ManageEvent/Registration.php: $form->addYesNo('is_email_confirm', ts('Send Confirmation Email?'), NULL, NULL, ['onclick' => "return showHideByValue('is_email_confirm','','confirmEmail','block','radio',false);"]); CRM/Event/Form/ManageEvent/Fee.php: $this->addYesNo('is_monetary', CRM/Event/BAO/Query.php: $form->addYesNo('participant_test', ts('Participant is a Test?'), TRUE); CRM/Event/BAO/Query.php: $form->addYesNo('participant_is_pay_later', ts('Participant is Pay Later?'), TRUE); CRM/Admin/Form/SettingTrait.php: elseif ($add === 'addYesNo' && ($props['type'] === 'Boolean')) { CRM/Admin/Form/Preferences.php: case 'YesNo': CRM/Admin/Form/Preferences.php: case 'YesNo': CRM/Admin/Form/Setting/Smtp.php: $this->addYesNo('smtpAuth', ts('Authentication?'), CRM_Utils_Array::value('smtpAuth', $props)); CRM/Admin/Form/Setting/Url.php: $this->addYesNo('enableSSL', ts('Force Secure URLs (SSL)')); CRM/Admin/Form/Setting/Url.php: $this->addYesNo('verifySSL', ts('Verify SSL Certs')); CRM/Core/Form.php: public function addYesNo($id, $title, $allowClear = FALSE, $required = NULL, $attributes = []) { CRM/Case/Form/Activity/ChangeCaseType.php: $form->addYesNo('is_reset_timeline', ts('Reset Case Timeline?'), NULL, TRUE); CRM/Mailing/Form/Component.php: $this->addYesNo('is_default', ts('Default?')); CRM/Mailing/Form/Component.php: $this->addYesNo('is_active', ts('Enabled?')); CRM/Mailing/Form/Search.php: $this->addYesNo('is_archived', ts('Mailing is Archived'), TRUE);

@eileenmcnaughton @seamuslee001 I think adding this option for addField extend the ability for boolean fields of the respective entity. Should I move this part in a separate PR ?

eileenmcnaughton · 2019-10-29T19:19:18Z

CRM/Mailing/BAO/Query.php

@@ -138,7 +138,7 @@ public static function select(&$query) {
   * rather than a static function.
   */
  public static function getSearchFieldMetadata() {
-    $fields = ['mailing_job_start_date'];
+    $fields = ['name', 'mailing_job_start_date', 'status', 'is_archived'];


I think we need to give 'name' a unique name in the schema if it doesn't have one yet (easiest as a separate PR)

This is in support of civicrm#15369 Where the names in the search do not match the 'real names' we have 2 techniques 1) where adding a unique name to the schema gives a match we do that - here that is the case for mailing_name 2) otherwise we ensure the schema has a unique name if appropriate & rename the field in the upgrade scriptt In this case when we look at CRM_Mailing_BAO_Query we don't see mailing_name as present (only mailing_id which should be an entity reference field). However it is used in the confusingly-not-related Mailing search so lets rename this one at the same time.

eileenmcnaughton · 2019-10-29T19:37:20Z

CRM/Contact/BAO/Query.php

        }
      }
+      if (in_array($element, array_keys($changeNames))) {


@monishdeb we no longer handle renaming fields in the code -in this case we can probably avoid it altogether by adding uniquenames - #15652

@eileenmcnaughton should we still allow this code and throw a deprecated notice if any of such case arise where unique names were not present?

For current purposes we can just leave it unchanged can't we?

eileenmcnaughton · 2019-10-29T19:39:27Z

CRM/Mailing/BAO/Query.php

-        $query->_qill[$grouping][] = "Mailing Namename $op \"$value\"";
+        $name = 'name';
+        $query->_where[$grouping][] = CRM_Contact_BAO_Query::buildClause("civicrm_mailing.$name", $op, $value, 'String');
+        list($op, $value) = CRM_Contact_BAO_Query::buildQillForFieldValue('CRM_Mailing_DAO_Mailing', $name, $value, $op);


We haven't made it generic to the other forms yet - but we started making the qill handling generic - would need some moving around

bf99326

Also note that we have a getFields() function on Contribution_BAO_Query $fields = self::getFields(); & are working towards using that metadata for the various bits of value

eileenmcnaughton · 2019-10-29T19:48:02Z

templates/CRM/Mailing/Form/Search.tpl

@@ -26,8 +26,8 @@
 <div class="crm-block crm-form-block crm-search-form-block">
  <table class="form-layout">
    <tr>
-       <td>{$form.mailing_name.label} {help id="id-mailing_name"}<br />
-        {$form.mailing_name.html|crmAddClass:big}
+       <td>{$form.name.label} {help id="id-mailing_name"}<br />


@seamuslee001 gave this MOP - #15652 - won't need this when that is done

monishdeb · 2020-02-06T04:53:43Z

@eileenmcnaughton I have updated this PR and moved additional changes in this PR #16478 Please have a look.

jaapjansma · 2020-03-16T12:56:04Z

test this please

jaapjansma · 2020-03-16T14:29:52Z

Betty and I are reviewing PRs and we came across this one. We dont know how to reproduce the problem which this PR fixes. We have asked a step by step instruction in the issue: https://lab.civicrm.org/dev/core/issues/692#note_33154

jaapjansma · 2020-05-11T07:56:37Z

Betty and I are reviewing PR's and we came across this one and we don't know how to test this and we haven't had an answer. So we will close this PR.

civibot bot added the master label Oct 1, 2019

monishdeb force-pushed the core-692-1 branch from 4da896e to f27d959 Compare October 1, 2019 13:15

eileenmcnaughton mentioned this pull request Oct 27, 2019

[NFC] Add date information to MailingJob Schema #15628

Merged

monishdeb force-pushed the core-692-1 branch 2 times, most recently from d5f95c8 to e1ea836 Compare October 29, 2019 09:56

eileenmcnaughton reviewed Oct 29, 2019

View reviewed changes

eileenmcnaughton mentioned this pull request Oct 29, 2019

Add uniquenames for mailing_name, mailing_job_status #15652

Merged

eileenmcnaughton reviewed Oct 29, 2019

View reviewed changes

Core/dev#692 : Support search arguments to Mailing Search form

0198675

monishdeb force-pushed the core-692-1 branch from e1ea836 to ed0ebde Compare February 5, 2020 15:31

additional fix

df0dde8

monishdeb force-pushed the core-692-1 branch from ed0ebde to df0dde8 Compare February 5, 2020 19:32

jaapjansma closed this May 11, 2020

monishdeb mentioned this pull request May 11, 2020

Introduce YesNo widget to addField core fn #16478

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core/dev#692 : Support search arguments to Mailing Search form #15369

Core/dev#692 : Support search arguments to Mailing Search form #15369

monishdeb commented Oct 1, 2019

civibot bot commented Oct 1, 2019

seamuslee001 commented Oct 27, 2019

seamuslee001 commented Oct 27, 2019

seamuslee001 commented Oct 27, 2019

eileenmcnaughton commented Oct 28, 2019

monishdeb commented Oct 29, 2019

eileenmcnaughton Oct 29, 2019 •

edited

Loading

seamuslee001 Oct 29, 2019

monishdeb Oct 30, 2019

eileenmcnaughton Oct 29, 2019 •

edited

Loading

eileenmcnaughton Oct 29, 2019

monishdeb Oct 30, 2019

eileenmcnaughton Oct 30, 2019

eileenmcnaughton Oct 29, 2019

eileenmcnaughton Oct 29, 2019

eileenmcnaughton Oct 29, 2019

monishdeb commented Feb 6, 2020

jaapjansma commented Mar 16, 2020

jaapjansma commented Mar 16, 2020

jaapjansma commented May 11, 2020

Core/dev#692 : Support search arguments to Mailing Search form #15369

Core/dev#692 : Support search arguments to Mailing Search form #15369

Conversation

monishdeb commented Oct 1, 2019

Overview

Before

After

Technical Details

civibot bot commented Oct 1, 2019

seamuslee001 commented Oct 27, 2019

seamuslee001 commented Oct 27, 2019

seamuslee001 commented Oct 27, 2019

eileenmcnaughton commented Oct 28, 2019

monishdeb commented Oct 29, 2019

eileenmcnaughton Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eileenmcnaughton Oct 29, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monishdeb commented Feb 6, 2020

jaapjansma commented Mar 16, 2020

jaapjansma commented Mar 16, 2020

jaapjansma commented May 11, 2020

eileenmcnaughton Oct 29, 2019 •

edited

Loading

eileenmcnaughton Oct 29, 2019 •

edited

Loading