CRM-19690 - Declare Mailing.template_type, Mailing.template_options, Hook::mailingTemplateTypes. #9562
Conversation
…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).
d66d24b to
6bc3944
Compare
|
Fixed/rebased to address test failures. |
| @@ -3197,4 +3197,51 @@ public static function getPublicViewUrl($id, $absolute = TRUE) { | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
| * @return array | |||
| * A list of template-types, keyed by name. Each defines: | |||
There was a problem hiding this comment.
I think this is incorrect. It is keyed by an arbitrary weight-based number. The alternate function is keyed by name.
Also, I think our comment standard (the full one not the jenkins enforced one) is to have a sentence before the params (cap letter, full stop) that makes ides all warm & fuzzy
There was a problem hiding this comment.
Good catch, that's stale. I'll update the docs.
| @@ -43,6 +43,9 @@ | |||
| * @throws \Civi\API\Exception\UnauthorizedException | |||
| */ | |||
| function civicrm_api3_mailing_create($params) { | |||
| if (isset($params['template_options']) && is_array($params['template_options'])) { | |||
| $params['template_options'] = $params['template_options'] === array() ? '{}' : json_encode($params['template_options']); | |||
There was a problem hiding this comment.
This line would be more readable with parentheses around
($params['template_options'] === array())
I found it harder to read than a normal ternery because of all the ( and { noise
|
I've added merge on pass because my suggestions don't constitute a blocker - although I think they would improve it |
|
Note my risk assessment is that this is very low risk as it is adding to the schema / api but not changing existing flows as yet. There are tests for the json handling (it does occur to me that could be a generic thing & we could add json as a schema type in the xml). I think test coverage on the upgrade & correct schema creation on install is adequate |
|
The reported test failure ( I've added another commit to address the docblock/style comments. Will wait for that test run to pass. |
|
Latest test run passes. |
| */ | ||
| public function upgrade_4_7_16($rev) { | ||
| $this->addTask(ts('Upgrade DB to %1: SQL', array(1 => $rev)), 'runSql', $rev); | ||
| $this->addTask('Add new CiviMail fields', 'addMailingTemplateType'); |
There was a problem hiding this comment.
@totten Just want to point out that you could save yourself the trouble of adding that addMailingTemplateType callback function by using the generic addColumn callback. See some examples above.
There was a problem hiding this comment.
@colemanw Nice! Thanks. Will keep that in mind in the future.
This adds two new fields to the schema,
template_typeandtemplate_options:template_typecan be declared usinghook_civicrm_mailingTemplateTypes.template_optionsis an open-ended array/JSON-object (generally in the same style asCaseType.definition).For example, when composing a mailing with Mosaico, one would need to store
additional metadata about the template to support editing and reuse.
For more details, see the commit notes.