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

Add workflow template for 'recurring edit' workflow #21356

Merged
merged 8 commits into from
Sep 16, 2021

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Sep 3, 2021

Overview

Add workflow template for 'recurring edit' workflow

Before

now you don't see it

After

now you do

Technical Details

This adds a declared workflow template for the recurring edit workflow

Comments

@civibot
Copy link

civibot bot commented Sep 3, 2021

(Standard links)

@totten
Copy link
Member

totten commented Sep 15, 2021

@eileenmcnaughton I've pushed a few more commits up here. Here's what the "Preview" dialog looks like with the new RecurringEdit example:

Screen Shot 2021-09-15 at 3 51 55 AM

eileenmcnaughton and others added 7 commits September 15, 2021 16:02
This changes the signature on a new helper method. This method is not widely used, and each reference is updated here.

Before: `Civi\Test::example($name)` returns the *metadata* for the example.

After: `Civi\Test::example($name)` returns the *data* for the example.

Comment: It's more convenient to stitch together examples from the data.  Of
course, metadata may also be useful -- it's still available through
`Civi\Test::examples()->getFoo(...)` (with a few different `getFoo()`
methods).
1. Use revised class format
2. Use `entity/{$ENTITY}/{$EXAMPLE}` instead of `workflow/{$WORKFLOW}/{$EXAMPLE}`
@totten
Copy link
Member

totten commented Sep 15, 2021

@eileenmcnaughton I've done revisions and rebasing/squashing to use the Civi/Test/ExampleData/* (with data-sets named like entity/Contact/Alex and entity/ContributionRecur/Euro5990/cancelled.

@eileenmcnaughton
Copy link
Contributor Author

@totten that looks good to me - I'd be happy to merge-on-pass this (with a little PR template editing).

I would like to talk about the 'from_email_address' one - because I feel like that could be generic - once this is merged

@eileenmcnaughton eileenmcnaughton changed the title [WIP] Msg testing Add workflow template for 'recurring edit' workflow Sep 16, 2021
…missing properties.

This creates a new example of the `RecurringEdit` workflow message.

Note that the example is tagged `phpunit` and defines a list of `asserts`. These
assertions are evaluated using the default message-template.

The test was not passing because some important properties were missing from `RecurringEdit`.
@totten
Copy link
Member

totten commented Sep 16, 2021

(@eileenmcnaughton) I would like to talk about the 'from_email_address' one - because I feel like that could be generic - once this is merged

Agree on both points. The Smarty variable receipt_from_email feels a bit quirky, and it'd make sense to expose the To/From/etc in a more consistent+common way.

@totten totten added the merge ready PR will be merged after a few days if there are no objections label Sep 16, 2021
@eileenmcnaughton
Copy link
Contributor Author

@totten also note that I don't think it's really intentional that the template winds up with the [name + email] format rather than just email - it doesn't read right

@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 @colemanw can one of you escalate this to merge-on-pass. I think @totten & I are agreed it should be merged now so just want someone not already a committer on this PR to approve

@colemanw
Copy link
Member

Lots of tests here :)

@colemanw colemanw merged commit 09d6e80 into civicrm:master Sep 16, 2021
@colemanw colemanw deleted the msg_testing branch September 16, 2021 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master merge ready PR will be merged after a few days if there are no objections
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants