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

CRM-19690 - CiviMail/FlexMailer - Allow alternative email templating systems #9497

Closed
wants to merge 19 commits into from

Conversation

totten
Copy link
Member

@totten totten commented Dec 3, 2016

(Note: This PR is still under active development. It may be rebased/squashed without notice.)

CiviMail has traditionally included a templating language and a delivery system. This patch generally aims to open those systems up so that extensions can define better templating processes (e.g. Mosaico) while minimizing changes or risks for existing CiviMail users. It breaks down into three areas:

  1. Data Model: civicrm_mailing supports two new fields, template_type and template_options (open-ended JSON data).
  2. UI (Frontend): The screen civicrm/a/#/mailing/123 can be overloaded by an extension. This allows them to set template_type and template_options -- and to display a new user interface.
  3. Delivery (Backend): For mailings which use template_type, an extension can tap into the batching/composition/delivery system.

Data Model: template_type and template_options

The template_type stores a string, such as "traditional" or "mosaico" or "none" or "twig".

The template_options stores an open-ended JSON object. The content of this field is not specified by core. Each template_type may store any fields it deems useful/necessary.

Extending the data-model in this way allows different mailings and systems to coexist. For example, you have may have 3 or 4 CiviMail templates (for newsletters vs event-announcements vs emergency-alerts). You install Mosaico because it seems cool and you went try it for one or two mailings. However, you still have several templates/mailings/people/business-processes based on the old system. That's fine -- because your old-style content and new-style content are stored with different template_types.

By default, template_type is traditional, which means that CiviMail should retain current behaviors -- i.e. use the old/current/existing user-interface and use the old/current/existing delivery system.

Frontend: CiviMail and Angular layouts

The current CiviMail UI includes a rich-text editor (ckeditor) and a token selector. However, if you have a different content-model, then these elements need to be replaced -- and (depending on how your content model works) you may rethink the general layout/pageflow of the composition screen.

When defining a new template_type, you need to provide an Angular *.html file describing the look-and-feel of the editor. Register this using hook_civicrm_mailingTemplateTypes, e.g.

function mymod_civicrm_mailingTemplateTypes(&$types) {
  $types[] = array(
    // Specify the unique identifier for the template type
    'name' => 'mytpltype',
    // Specify the HTML file.
    'editorUrl' => '~/myAngularModule/myeditor.html',
    // Specify the relative weight. The lowest available weight will
    // determine the default for new mailings.
    'weight' => -10,
   );
}

Backend: FlexMailer

CiviMail includes a cron task (Job.process_mailing API). This has several responsibilities, such as:

  • Identify mailings are that scheduled to go out.
  • Break the recipient list into smaller chunks for throttling.
  • Perform batch-loads of recipient data (for use in the mailing).
  • Composing an actual email message based on the given template/data/metadata.
  • Send emails to a third party service (e.g. SMTP).

Most of this logic is handled by a series of coupled functions: MailingJob::deliver(), MailingJob::deliverGroup(), Mailing:compose(), Mailing::getPreparedTemplates(), etal.

If our goal is to replace the templating system, then we probably want to reuse most of the functionality -- but change small aspects (specifically in composing the message). Unfortunately, this code has (over time) taken on toxic characteristics -- for example, the batching function (MailingJob::deliver()) does some of the email composition, and the email composition function (Mailing::compose()) represents the same data in multiple formats (and converts back/forth multiple times). We could refactor/cleanup this code in place, but there'd be a high-risk of regressions -- and we don't want normal CiviMail users to bear the risk that. Rather, that risk belongs with the early-adopters who choose to use a newer templating system.

The resolution: for now, there are two delivery pipelines:

  • MailingJob::deliver() (etal) represent the old system. Only a couple lines of this code changes. It is used for any mailing with template_type==traditional (and specifically any SMS-based mailings).
  • FlexMailer is a refactored, event-driven version. It is only used on mailings which have a new template_type. The events are discussed more in the docblocks, and the template-evaluation logic is now shared with the Scheduled Reminders (e.g. via TokenProcessor).

FlexMailer aims to provide comparable behaviors and features -- much of the code is pilferred from BAOs, and it passes the same test-casess (api_v3_JobProcessMailingTest and CRM_Mailing_MailingSystemTest). However, the test scenarios may not be exhaustive, so it's not enabled by default. (If you'd like to enable it for all mailings, enable the hidden setting experimentalFlexMailerEngine.)

Note: The main motivation of implementing FlexMailer now is to support new composition/templating logic; however, the architecture allows other changes (such batch-oriented message delivery, alternative tracking logic, or alternative batching/distribution patterns).


@totten
Copy link
Member Author

totten commented Dec 7, 2016

We had some discussion on Hangouts/Mattermost about the prospects of backporting this to 4.6. To further reduce change, I've removed some of the smaller cleanups (like fixing weirdly-named variables) and also moved some of the new helpers from Mailing BAO to new classes under Civi/FlexMailer.

Still an open question: whether to move the entire folder Civi/FlexMailer to an extension. Some thoughts:

  • Several of the classes (esp Civi/FlexMailer/Listeners/Default*) are coupled to the Mailing BAO's and TokenProcessor -- which are part of core. From that perspective, it's more cohesive to keep it in core.
  • I don't think it's a particularly good idea for other extensions to implement alternatives to FlexMailer -- however, it is useful to listen for the FlexMailer and replace individual components/phases.
  • The folder Civi/FlexMailer is pretty discrete. From that perspective, it's a good opportunity to improve modularity by moving to an extension.

@totten totten force-pushed the master-19690 branch 4 times, most recently from 294d4d8 to fa4706e Compare December 17, 2016 04:30
totten added a commit to totten/civicrm-core that referenced this pull request Dec 17, 2016
This adds the TokenProcessor and associated classes to the container in 4.6.
These are not *used* at runtime -- but they're published so that extensions
may use them.

This folds in a few more recent patches to TokenProcessor subsystem provided
by civicrm#9497 and removes the parts defined for scheduled reminders.
@totten
Copy link
Member Author

totten commented Dec 17, 2016

I've rebased and done a round of revisions to improve the prospects of backporting to v4.6. Generally, most of these commits should be individually small changes to CiviMail, and the idea is to apply those equally to master and 4.6. However, there will be two key differences between the master and 4.6 patches:

  • The master PR includes the new classes in Civi/FlexMailer ("Implement FlexMailer. Add Tests."). If someone wants to use FlexMailer on 4.6, they'll need to manually download that folder. (We could theoretically put them in an extension... but 4.6 is missing some necessary extension points, and... thinking long-term about how we want the dependency-graph to look... this code probably belongs as part of the innards of CiviMail.)
  • The 4.6 PR includes an extra backport of Civi\Token folder (but simplified by omitting the scheduled reminder support).

@totten totten changed the title WIP - CRM-19690 - CiviMail/FlexMailer - Allow alternative email templating systems CRM-19690 - CiviMail/FlexMailer - Allow alternative email templating systems Dec 17, 2016
@@ -156,7 +156,7 @@
mailings: {include: [], exclude: []}
},
template_type: "traditional",
template_options: "",
template_options: {nonce: 1}, // Workaround CRM-19756
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the full code standards spec disallows comments at the end of the line

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I could put it on the preceding line. I like having the comment adjacent to the code.

@totten totten force-pushed the master-19690 branch 2 times, most recently from fefacb4 to 77b95f7 Compare December 20, 2016 01:00
@totten
Copy link
Member Author

totten commented Dec 20, 2016

Based on discussion with @eileenmcnaughton, I've extracted some smaller PR's from here:

I'm keeping this branch/PR around because it's useful (a) for integration testing, (b) Mosaico development, and (c) remembering what all needs backporting.

…Hook::mailingTemplateTypes.

This adds two new fields to the schema.

Template types can be declared (along with preferred editor) using
hook_civicrm_mailingTemplateTypes, e.g.

```
function mymod_civicrm_mailingTemplateTypes(&$types) {
  $types[] = array(
   'name' => 'moasico',
   'editorUrl' => '~/crmMosaico/EditMailingCtrl/mosaico.html',
  );
}
```

Note: This only stores data.  Other commits will make use of that data in
more meaningful ways.
Present API consumers with a consistent, array-based interface for reading
and writing properties in a Mailing.

Suppose you're submitting a REST request to create a mailing.  The REST
request as a whole is encoded with JSON.  With the default API behavior, you
would need to double-encode the `template_options`, e.g. roughly

```
POST rest.php
  json_encode([
    entity => Mailing
    action => create
    params => [
      template_options => json_encode([...])
    ]
  ])
```

With this patch, you only need to encode the request once.

This parallels the approach used in CaseType API (for the XML `definition`
field).
General notes:

 1. When creating a mailing, use the path `civicrm/a/#/mailing/new` to create a mailing with the default `template_type` (aka first-preferred, by weight).
 2. When creating a mailing, use the path `civicrm/a/#/mailing/new/{template_type}` to create a mailing with a specific `template_type`.
 3. When editing a mailing, check the `template_type` and load the appropriate editor.
 4. Some of the boilerplate from `2step.html`, `unified.html`, etal has been moved to `base.html`.

Note that this breaks a hidden feature -- before, you could switch among a
few different layouts (`2step`, `unified`, `unified2`, `wizard`) by manually
editing the URL (e.g.  `civicrm/a/#/mailing/2/unified`). Now, to change
the layout of the traditional-style mailings, you can implement a hook, e.g.

```
function mymod_civicrm_mailingTemplateTypes(&$types) {
  foreach ($types as &$typeDef) {
if ($typeDef['name'] === 'traditional') {
  $typeDef['editorUrl'] = '~/crmMailing/EditMailingCtrl/unified.html';
}
  }
}
```
… `static`

This function is only called once, and it previously lacked test coverage,
so the `static` warning previously snuck through. Fix it!
…public

These two functions will be useful in FlexMailer's DefaultEngine.
This is more significant with mailing tokens because some of them are fairly
sizeable/expensive.
This includes support for both `{mailing.*}` and `{action.*}` tokens used in
CiviMail.
If you install an extension under `$cmsRoot/vendor/org.example.foo`, the
resulting URL contains an extraneous `/` (eg
`http://example.org//vendor/org.example.foo`).  (In Windows, I suspect it's
even worse because it uses DIRECTORY_SEPARATOR in the URl -- eg
`http://example.org/\vendor/org.example.foo`.)

This patch checks for and removes the extraneous slash -- and always
constructs the URL with the appropriate delimiter (`/`).

Problem observed in `dmaster`.
During development of FlexMailer, I flirted with using a special token (e.g.
`{actions.trackOpen}`) but ultimately found that was positioned well in
the order of operations.
totten added a commit to totten/civicrm-core that referenced this pull request Jan 5, 2017
This adds the TokenProcessor and associated classes to the container in 4.6.
These are not *used* at runtime -- but they're published so that extensions
may use them.

This folds in a few more recent patches to TokenProcessor subsystem provided
by civicrm#9497 and removes the parts defined for scheduled reminders.
@totten
Copy link
Member Author

totten commented Jan 5, 2017

Recap: The changes developed were separately evaluated/merged as separate PRs (#9563, #9564, #9565, #9566, #9617, #9618, #9619), so I'll close this one.

@totten totten closed this Jan 5, 2017
totten added a commit to totten/civicrm-core that referenced this pull request Jan 18, 2017
This adds the TokenProcessor and associated classes to the container in 4.6.
These are not *used* at runtime -- but they're published so that extensions
may use them.

This folds in a few more recent patches to TokenProcessor subsystem provided
by civicrm#9497 and removes the parts defined for scheduled reminders.
agileware-fj pushed a commit to agileware/civicrm-core that referenced this pull request Mar 1, 2018
This adds the TokenProcessor and associated classes to the container in 4.6.
These are not *used* at runtime -- but they're published so that extensions
may use them.

This folds in a few more recent patches to TokenProcessor subsystem provided
by civicrm#9497 and removes the parts defined for scheduled reminders.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants