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

MessageTemplate - Deprecate valueName. Emphasize workflow. Consolidate converters. #21657

Merged
merged 5 commits into from
Sep 29, 2021

Conversation

totten
Copy link
Member

@totten totten commented Sep 28, 2021

Overview

CRM_Core_BAO_MessageTemplate::sendTemplate() and ::renderTemplate() accept an argument called valueName. The the option has grown misleading -- if message templates will no longer be identified by OptionValue per se. Of course, the content is still important as it identifies the workflow step (a la civicrm_msg_template.workflow_name WorkflowMessage::WORKFLOW).

The primary change of this branch is to deprecate valueName in favor of workflow. Additionally, it does some cleanup for other fields that went through a similar change.

Before

  • The fields contactId and disableSmarty are translated to tokenContext.contactId and tokenContext.smarty. However, these translations are sprinkled around.
  • sendTemplate() and renderTemplate() use valueName
  • WorkflowMessage and hook_alterMailParams use the same params as ^^ -- so they use valueName

After

  • The translations for valueName, contactId, and disableSmarty all work the same way - and they've each been boiled down to 1-line call (CRM_Utils_Array::pathSync())
  • sendTemplate() and renderTemplate() use workflow (with alias support for valueName)
  • WorkflowMessage and hook_alterMailParams use the same params as ^^ -- so they use workflow (with alias support for valueName)

Comments

It's tempting to add an extra warning at the start of renderTemplateRaw(), although this would make the patch a longer (ie we'll get test-failures for old callers - and need to update each).

if ($badGuys = array_intersect(array_keys($params), ['valueName', 'disableSmarty', 'contactId'])) {
  \CRM_Core_Error::deprecatedWarning("renderTemplate() received deprecated option(s): " . implode(', ', $badGuys));
}

This changes some of the plumbing in how it interprets 'paramSpecs' - but
doesn't change anything substantive.

Before: You must specify a 'type' constraint for every param.
After: You may omit the 'type' constraint for some params.

Before: If you specify a nullable string with a regex, then null-values will fail.
After: If you specify a nullable string with a regex, then null-values will pass.
…e alias bits.

The params given to `MessageTemplate::sendTemplate()` are evolving. But they are also
exposed via hook. Blerg.

This technically does two things:

1. It takes a couple of bits which are responsible for aliasing/transitioning specific
   params, and it moves them to a common `$sync()` function.

2. It deprecates `valueName` in favor of `workflow`, and it updates the hook test
   accordingly.
@civibot
Copy link

civibot bot commented Sep 28, 2021

(Standard links)

@civibot civibot bot added the master label Sep 28, 2021
@eileenmcnaughton eileenmcnaughton merged commit 9e15f0d into civicrm:master Sep 29, 2021
@totten totten deleted the master-wfvn branch September 29, 2021 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants