dev/mail#83 - Import current revision of "Message Admin"

[totten]

Overview

Imports the current revision of msgtplui message_admin, an extension that replaces the screen "Administer => Communications => Message Templates". Most notably, this includes support for:

1. Translating system workflow templates

   

2. Drafting system workflow templates (editing new content without publishing)

   

3. Generating previews of system workflow templates

   

See: https://lab.civicrm.org/dev/mail/-/issues/83

Comments

Git import via script: https://gist.github.com/totten/5572cd70cd4455709cf60d9ef02cce09

I'm submitting based on @eileenmcnaughton's request so that there is a clear/singular place for it to live. However, it likely needs further work - hence <tag>mgmt:hidden</tag>.

[eileenmcnaughton]

@totten - I think the issue is more the fact it is abreviated - 'messagetemplateeditor' is pronouncable - 'msgtpl' isn't

I think we'll be struggling to say & explain 'msgtpl' for years to come - but re-naming is a PITA

[mattwire]

@totten This looks really nice :-)

Is there a reason to import it into core at this point though? I know things like searchkit became very dependent on core and it made no sense to keep them separate but I'm not sure that applies here. And if it's going to remain hidden it seems even less worthwhile importing to core at this point. Wouldn't it be better as a "standard" extension repo at https://lab.civicrm.org/extensions ?

[eileenmcnaughton]

@mattwire the goal is that it will in time replace the existing form in core (and remove a support burden) - our experience of bringing things into core when they start off outside core is that it causes a lot more work, takes a lot longer & causes more breakage when we do so. (how many years for flexmailer? )

The line is really whether it is intended to be part of the core UI in future

[eileenmcnaughton]

Note that to enable

CRM.api3('Extension', 'enable', {
  "keys": "message_admin"
}).then(function(result) {
  // do something with result
}, function(error) {
  // oops
});
CRM.api3('Extension', 'enable', {
  "keys": "message_admin"
}).then(function(result) {
  // do something with result
}, function(error) {
  // oops
});

Enabled currently at http://core-21590-8zz9r.test-3.civicrm.org:8001/civicrm/admin/messageTemplates?reset=1#/edit?id=15

Something wrong at the moment - but I think it might be temporary - this is the template set up for preview so far http://core-21590-8zz9r.test-3.civicrm.org:8001/civicrm/admin/messageTemplates?reset=1#/edit?id=17

[totten]

@eileenmcnaughton That's an odd error. I've been trying to reproduce it a couple ways:

• Used an alternate local site (where I hadn't run the extension before). Activated via JS console.
• Used the same autobuild site that you tested. Go in through CLI to reset it back to original DB/delete caches. Then activate via JS console.

One theory that could produce that symptom on a temporary basis - if the test job was still wrapping up when you tried out the webpage, there may have been flushes or resets going on concurrently? (IIRC, I had pushed up a few times close to each other there - so it may've taken a spell longer than usual to run.)

It's also possible that I just haven't figured the right mix of clear/reset steps to reproduce. Let's ask Jenkins to build again and let it have one more go.

[totten]

jenkins, test this please

[eileenmcnaughton]

@totten yeah it is odd - it might be temporary - I'm just testing against our wmf code as we speak - first observation is the theming on shoreditch

@colemanw made a css change to search kit to better accomodate those buttons which might be worth doing (I don't think it's blocking - since the hidden stage is the opportunity for things like that to bubble up before we unhide)

[eileenmcnaughton]

Hmm these buttons are a bit more shore-ditch problematic

UPDATE - I thought I'd posted the wrong screenshot (what buttons?) - but they are the white bits to the left of 'token'

[eileenmcnaughton]

Another minor usablity thing - navigating back to the list is tricky - @colemanw has been using the breadcrumbs for that in search kit - although I think it was recently suggested that is not great for Wordpress users?

[eileenmcnaughton]

@totten I've hit something a bit more blocking - we have an additional workflow template in our civicrm_msg_template table. It has a workflow_name (but not a workflow_id because we agreed to deprecate workflow_id as the compromise when I wanted to make those option groups behave 'normally') - and it isn't showing up in the list of workflow templates

UPDATE - I 'stole' the workflow_id of another template & it still doesn't show up - however I can plug in civicrm_message_template.id into the edit url to see it

[eileenmcnaughton]

On the theming one other thing I note is the save buttons doesn't indicate that it has been clicked so I keep clicking it

[eileenmcnaughton]

Another thing - definitely non-blocking - the filtered would be better with the new features @colemanw implemented for search kit displays recently - I think that would be a bigger lift though

[eileenmcnaughton]

Ok another thing - I can't figure out how to save a draft - maybe it's a UI feature that is hiding or shoreditch? Or I could be a bit blind...

[eileenmcnaughton]

And another thing I just spotted....

In the civicrm_msg_template table all the html is saved unencoded. In civicrm_translation it is encoded.

This may well be by design & a very good thing but noting it

[eileenmcnaughton]

@totten I'm generally having some issues with the sample data for my custom workflow -but to be honest I think working through this - #21611 will solve most of my questions - since I think te 'just re-use recurring edit' with a smidgeon less logic is common to both (ie the sample data doesn't need to change because none of the templates in question expose any fields differ by recurring contribution status)

[eileenmcnaughton]

I've gotten through some of my issues locally with an update to civicrm_message_template.is_reserved to get rid of NULL values - just working through the last bits but I'm close to being ready to merge this on the understanding that

• it's hidden because it's still in an alpha state - we will unhide it when it's in a beta state
• while in this hidden alpha state the review bar is much lower - which means :

Required	Not required
unit tests on most changes	r-run testing on every change
changes are in line with the overall agreed approach - which basically means honing what is here but no big change of direction	ui 'mobility'
close scrunity on any changes that interact with core code other than by public documented hooks/ api	close code scrutiny of changes internal to the extension that don't interact with core code

[eileenmcnaughton]

Merging per https://chat.civicrm.org/civicrm/pl/78ne6ukiatnsxr5a4sumg43dhy

There are definitely issues still to work through here but I believe it's easier to track them if we merge this (we can log issues in gitlab against merged PRs - it's kinda odd when they aren't).

Note that I covered the fact this is new core extension which will in time replace existing screens in the dev digest

[colemanw]

#21712 addressed the style issues