Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CiviMail - New autocompletes for selecting recipients #26585

Merged
merged 4 commits into from Jun 24, 2023

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Jun 21, 2023

Overview

When composing a mailing, this replaces the single include/exclude/groups/mailings widget with two widgets, one for include and another for exclude. Groups and mailing are still combined together in autocomplete results, but are loaded more efficiently by APIv4.

Original proposal: https://lab.civicrm.org/dev/core/-/issues/4045

Replaces #26522

Before

image

After

  • 2 widgets on separate lines (Include/Exclude)
  • Little wrench button given some text to make it more visible
  • "Hidden Smart Group 123456" relabeled "Search Results"

image

Technical Details

Builds on #26536 and #26583 to create a SearchDisplay that UNIONS groups and mailings.

Generally improves handling of Autocomplete searchDisplays based on EntitySets
@civibot
Copy link

civibot bot commented Jun 21, 2023

(Standard links)

This uses an autocomplete callback with UNIONs to select from both groups and mailings,
but instead of a single widget for incluce/exclude it provides two widgets for better usability.
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@agileware-justin
Copy link
Contributor

Would be good to get rid of the almost completely useless "~ 94 recipients" text and instead replace this with a URL "Show Recipients" which links to a contact listing of the actual recipients for the mailing based on the criteria. Open in new page - similar to the Mailing report links.

Because showing some of the recipients is far less useful than showing the actual recipients.

Otherwise, definitely better than the current feature. Thanks!

@colemanw
Copy link
Member Author

@agileware-justin just to keep things focused, please open a different issue for that. This PR doesn't touch that piece.

@agileware-justin
Copy link
Contributor

No worries

@demeritcowboy demeritcowboy added the merge ready PR will be merged after a few days if there are no objections label Jun 23, 2023
@demeritcowboy
Copy link
Contributor

This works for me.
I can no longer select null, so I can't no longer send nothing to nobody. But my set of thoughts on that is empty.

@demeritcowboy demeritcowboy merged commit d6dd768 into civicrm:master Jun 24, 2023
3 checks passed
@colemanw colemanw deleted the crmMailingRecipientsSet branch June 24, 2023 03:17
@colemanw
Copy link
Member Author

Now how will @demeritcowboy get in touch with his old friend Null?

@demeritcowboy
Copy link
Contributor

Ah, clever reference!

// CRM-15522 ends

Yeah, if only.

@colemanw
Copy link
Member Author

// CRM-15522 lives!

@colemanw
Copy link
Member Author

Even in APIv4, it lives on!

/*
* Because of the wacky way that database values are saved we need to format
* some of the values here. In this strange world the string 'null' is used to
* unset values. If we encounter true null at this layer we change it to an empty string
* and it will be converted to 'null' by CRM_Core_DAO::copyValues.
*
* If we encounter the string 'null' then we assume the user actually wants to
* set the value to string null. However since the string null is reserved for
* unsetting values we must change it. Another quirk of the DB_DataObject is
* that it allows 'Null' to be set, but any other variation of string 'null'
* will be converted to true null, e.g. 'nuLL', 'NUlL' etc. so we change it to
* 'Null'.
*/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
4 participants