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

Fix email dynamic content owner lookup #5741

Merged
merged 1 commit into from Mar 21, 2018

Conversation

kuzmany
Copy link
Member

@kuzmany kuzmany commented Feb 22, 2018

Q A
Bug fix? Yes
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) #5691
BC breaks?
Deprecations?

Description:

Create another version of dynamic content stopped init lookup results for Owner.
This fix add update object mQuery to callback function.

Steps to reproduce the bug:

  1. Create an email
  2. Go on the builder and add a dynamic content bloc
  3. Add a variation
  4. Add a filter and choose contact owner
  5. The list of users does not appear

Steps to test this PR:

  1. Apply PR and call regenerate assets php app/console m:a:g
  2. Repeat steps to reproduce
  3. See if owners are suggested

image

@mautibot mautibot added code-review-needed PR's that require a code review before merging bug Issues or PR's relating to bugs labels Feb 22, 2018
@kuzmany
Copy link
Member Author

kuzmany commented Feb 22, 2018

Label: Ready to test

@mautibot mautibot added the ready-to-test PR's that are ready to test label Feb 22, 2018
@dbhurley dbhurley added this to the 2.13.0 milestone Mar 4, 2018
Copy link

@jimspillane jimspillane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR tested as described.

@mautibot mautibot removed the code-review-needed PR's that require a code review before merging label Mar 12, 2018
Copy link
Member

@npracht npracht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested it works

@npracht
Copy link
Member

npracht commented Mar 12, 2018

Label: Pending test confirmation

@mautibot mautibot added the pending-test-confirmation PR's that require one test before they can be merged label Mar 12, 2018
@Maxell92 Maxell92 self-assigned this Mar 21, 2018
Copy link
Contributor

@Maxell92 Maxell92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, thanks

@mautibot mautibot added the code-review-needed PR's that require a code review before merging label Mar 21, 2018
@escopecz escopecz self-assigned this Mar 21, 2018
Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works 👍 Thanks Zdeno!

@mautibot mautibot removed the code-review-needed PR's that require a code review before merging label Mar 21, 2018
@escopecz escopecz removed pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test labels Mar 21, 2018
@escopecz escopecz merged commit a3ae9de into mautic:staging Mar 21, 2018
@kuzmany kuzmany deleted the fix-email-dynamic-content-owner branch April 7, 2018 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issues or PR's relating to bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants