Prepared emails should display the value of variables like author name

[NateWr]

Prepared emails show placeholders like NAME for variables that will be injected to the email when it's sent. Users reported confusion about this and would prefer to see the actual name that will be used displayed.

From UI/UX testing outcomes: https://docs.google.com/spreadsheets/d/1RkZ0avL3UIUU-hUScbM77lLCsGtk-T2_dL-FEx_vKcM/

[defstat]

I have found that to be true at the reviewer forms.
The difference with other cases is that the email template and the parameters that the email template is displaying are on the same form. So the email template doesn't now the values of its parameters before hand. (for example the "review due date" at the "New Reviewer" form, or the name of the reviewer in the same form)

That is not the case with the NAME placeholder at the "select a reviewer" form (when the user selects a reviewer for a submission). Even though the email template displayed form (after the Select Reviewer button is pushed) should know the reviewer name, the email editor displays the NAME placeholder.
That happens because the "select reviewer" button does not call a new form, but rather it hides/shows an already created form element.
To solve this we could either separate the two forms or to add some code to the AdvancedReviewerSearchHandler.js so that the editor displays the correct name (I have already done that) - but the other, not yet defined parameters still have their placeholders shown.

If we need to have some javascript functionality for that (for example if the user selects a "review due date" the editor should change too), I suggest to add an Editor callback that takes as parameter the editor id or name and an array of parameters/values and apply it on form events ('click' or 'change' or other)

I am looking for other instances of the above, but couldn't yet find any.

[NateWr]

@defstat I think you've diagnosed the problem correctly. For this to work properly, we're going to want a general way to process the variable data in the browser (JS), not the server, so that we can update it on-the-fly.

But we will run into issues after we have processed the variables. If we replace the placeholder with the name, and then the user edits the form, and then changes the recipient, we won't have the placeholder to safely insert the name again.

One possibility would be to write a TinyMCE integration which rendered the value of the placeholder in the WYSIWYG view, but kept the placeholder in the actual submitted code. Another approach could be to wrap the values in a <span data-placeholder="name"></span> tag, which would allow us to identify and replace the name even after it's been changed. But we may need to strip that from the submitted data before it's processed.

This is a challenging problem, but if a good system was in place, it could be used widely in our system to supply relevant data inserts for emails and messages of all kinds.

[asmecher]

@NateWr, we already have a TinyMCE integration: it displays {$someVariable} as Some Pretty Name, prevents it from being edited, and allows dropdown-based picking of additional variables that might be useful but might not be included in the default template. It's missing dynamic refreshing of Some Pretty Name should the surrounding conditions change, OTOH, but otherwise it's there. (We haven't made TinyMCE aware of variables in much of the software yet, but look e.g. at the reviewer assignment email template.)

[NateWr]

Cool, so it sounds like all we need is a method we can call on the TinyMCE and pass a hash of variables to, and have the existing content updated, eg:

editor.setPlaceholders({
  variableName: value,
});

[defstat]

@NateWr @asmecher I think that there could be a mechanism that would do something like the following:

• at the FormHandler.js constructor, we could add a TinyMCE tag (at its tag map) for each form input element with appropriate data-symbolic attribute
• at the FormHandler::changeEvent, we could look into both the tagMap data and the content of the TinyMCE for a data-symbolic for the element that triggered the changeEvent, and change the tag's value with this element's value on-the-fly.

That way we can add more tags to the already existing email templates, and the user can see the value of the tags (not their symbolic name), as he changes the form's input values.

I have not found a general way/mechanism to do the same with already existing email template placeholders, mainly because the emailTemplates seem to have a very tight connection with the already existing placeholder-replacing mechanism which is depending on server-side code.

For example, let's consider the "Email to be sent to reviewer" email template, upon the creation of a new reviewer. There, the TinyMCE editor has a <span class="pkpTag mceNonEditable" data-symbolic="reviewerName" contenteditable="false" data-mce-selected="1">Name</span>, which is populated after the submission of the html form with 'reviewerName' => $reviewer->getFullName().
I can't imagine a general way to overcome that placeholder assignment with client side code.

Another approach could involve a different display of the emailTemplate's placeholders. We could change the display of a tag in the TinyMCE editor (let's say to display it in red) if there is no server-side assignment to it during the loading of the form, or if there is no form element that can assign value to it on-the-fly, upon a form element change event. That way the user can be notified regarding the placeholders that will be assigned later by the server-side code. (not sure if that is more confusing for the user, it could help me as a user I think).

Any thoughts on these?

[NateWr]

FormHandler::changeEvent, we could look into both the tagMap data and the content of the TinyMCE for a data-symbolic for the element that triggered the changeEvent, and change the tag's value with this element's value on-the-fly.

That sounds good. One hiccup you'll probably run into is when form data is changed but the tag data needs to be fetched from the server. For instance, I may select a user ID, but the TinyMCE needs to update the user's first name.

To get around this, I'd encourage you to build it with callbacks in mind. So in FormHandler::changeEvent you might be able to run an ajax request as necessary, and only update the tags when that request is back.

I'd encourage you to abandon the server-side replacement in the template altogether. Instead, I'd handle the replacement entirely in the client. That way you don't need to worry about syncing the original template with the data. Just pass the initial data into TinyMCE when it's loaded, and repeat the procedure any time the data is updated. Then accept whatever is sent to the server as the final rendered version.

[defstat]

@NateWr I think that the tag data could be manipulated at client side. The problem I wanted to point out is actually what happens if the server couldn't have a "valid" response for a requested placeholder name. For example what if the reviewer email is displayed before the server knows about the reviewer object (because we are at the form for creating a reviewer). In that case the Ajax approach will not work.

[NateWr]

@defstat I'd set it up so that I can pass an empty data object into the prepared email (in the client) and it shouldn't be destructive. Instead, it should show the placeholders when no data is available.

[asmecher]

@defstat and @NateWr, what's the status of this issue? Are there requirements issues to be unknotted?

[defstat]

@asmecher It's my fault that I haven't look into that for so long. I will go through the requirements and start developing next week, if that's ok.

[asmecher]

Not a problem, thanks! Just raising it because we've had a few recent issues filed about variables in emails, and there's likely further work that'll follow this one getting closed off.

[asmecher]

Just a note that I think the biggest source of confusion is on the reviewer assignment message compose window. That's the only case where we're presenting variable names in the default template rather than just replacing them before showing them.

[asmecher]

@defstat, if you're not sure where to start with this, the most frequent point of confusion is why we see the label "Name" rather than an actual name in the reviewer assign email compose box. But before you make any changes there, check quickly with @NateWr whether that aspect will be affected by his current reviewer assignment work.

[NateWr]

Nope, it shouldn't be effected.

[defstat]

PRs

pkp-lib: #3402
OJS: pkp/ojs#1847

[forgive38]

Just our opinion for this: we think that just display name and not other variables like URL or DATE is ambiguous.
Perhaps one solution is to have a preview tab or a step with the mail in final form but read only and the user can confirm or go back.

Ps: I have tried the PR, see my comments.

[NateWr]

This looks really nice @defstat! I think we do need to extend it to the other forms. So right now it's working when you create a new reviewer. There's also email fields associated with selecting an existing reviewer and selecting an existing user (who isn't a reviewer). It should work for those too. Other comments:

• When switching email templates and switching back, it would be nice if the variables were replaced automatically.
• It'd be nice to have a live update of the Response and Review Due Date variables as well.
• It looks like there's an unnecessary Editor Decision email template in the list there. Maybe check with Alec but I don't think that's needed and we should remove it if not.
• There are a bunch of other variables in the email templates for reviewer selection. We should try to replace those too.

[asmecher]

@defstat, this is not a priority for 3.3, but please don't forget this issue!

[NateWr]

This has been addressed as part of the new email composer UI introduced in #5717. It is not available for every prepared email but it will become available once we convert each email to the new composer UI.