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

Remove workflow_id reference #21671

Merged
merged 3 commits into from
Oct 1, 2021
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 29, 2021

Overview

Remove workflow_id reference

Before

Legacy field workflow_id referenced

After

correct field workflow_name referenced

Technical Details

This is a null op - we have code to ensure workflow_name is set when workflow_id is set

Comments

This reduces confusion

@civibot
Copy link

civibot bot commented Sep 29, 2021

(Standard links)

@civibot civibot bot added the master label Sep 29, 2021
@eileenmcnaughton eileenmcnaughton force-pushed the wf_name branch 2 times, most recently from bdbabc3 to dc09d35 Compare September 29, 2021 23:15
@totten
Copy link
Member

totten commented Sep 30, 2021

OK, so the code generally looks sensible, and the tests pass.

I think the main bit for r-run (which I'll take a stab at) is to actually register a msgtpl with workflow_name sans workflow_id and observe that the "Edit Message Templates" and "New Mailing" screens show a reasonably filtered list of templates (incl/excl the existing ones and the sample one, as expected).

@eileenmcnaughton
Copy link
Contributor Author

@totten if you do civibuild create wmff you will get that config - filter for 'failed' on the template screen

@totten
Copy link
Member

totten commented Sep 30, 2021

  • Setup: Added a pair of default+reserved templates for workflow_name=cheese_curdling (https://gist.github.com/totten/18b83ca437daecff4149c983b185b1a7).
  • Expectation: Generally, on the updated files/screens, the new template(s) should be handled like any other system-workflow-template. They should should not appear as user-templates.
  • Mailings => New A/B Test: The patch to CRM/Mailing/Info.php fixes the dropdown of "Mailing Templates" (ie it now hides all workflow-templates from this list).
  • ⚠️ Mailings => New Mailing: Despite the patch to CRM/Mailing/Info.php, it incorrectly shows "Cheese Curdling". Maybe the operative workflow_id filter is actually in ang/crmMailing/Templates.js
  • Admin => Comm => Message Templates: The templates are correctly categorized. The patch fixes a bug where the "Cheese Curdling" template had the wrong actions. (It was showing "Disable"/"Delete", akin to a user-template. It should show "Revert to Default" and "View Default".)
  • ⚠️ Admin => Comm => Message Templates: "Revert to Default": The link shows up now, but it's crashy:
    Screen Shot 2021-09-30 at 4 54 35 PM

… templates

These are now identified by `workflow_name` rather than `workflow_id`.
@totten
Copy link
Member

totten commented Oct 1, 2021

Added fixes for the latter two.

@eileenmcnaughton
Copy link
Contributor Author

Merging this based on agreeing with your updates

@eileenmcnaughton eileenmcnaughton merged commit 078cc1f into civicrm:master Oct 1, 2021
@eileenmcnaughton eileenmcnaughton deleted the wf_name branch October 1, 2021 04:03
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.

2 participants