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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Dynamic Email Content filters using Custom Fields #3719

Closed
wants to merge 11 commits into from
Closed

Bugfix: Dynamic Email Content filters using Custom Fields #3719

wants to merge 11 commits into from

Conversation

dongilbert
Copy link
Member

@dongilbert dongilbert commented Mar 21, 2017 •
edited by alanhartless

Q A
Bug fix? Y
New feature? N
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs) #3719
BC breaks? N/A
Deprecations? N/A

Description:

This PR fixes dynamic content filters that use custom fields as their filters. It also fixes an issue where segment filters and DEC filters used the label for multiselects rather than the values.

Steps to reproduce the bug:

  1. Create a Multi-select custom field with multiple values, open a contact, and select a value for the custom field you created. Save the contact.
  2. Create an email with a DEC item, with a variant using the created multi-select field as the filter.
  3. Send that email to the contact you modified. You'll see that the default content is returned, instead of the matching variant content.

Steps to test this PR:

  1. Apply PR
  2. Retest. The received email will have the appropriate variant.

@matishaladiwala matishaladiwala added this to the 2.8.0 milestone Mar 21, 2017
@alanhartless alanhartless added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Mar 21, 2017
@alanhartless alanhartless mentioned this pull request Mar 22, 2017
Closed
@virlatinus virlatinus mentioned this pull request Apr 3, 2017
Closed
@escopecz escopecz self-assigned this Apr 6, 2017
escopecz
escopecz approved these changes Apr 10, 2017
View reviewed changes
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.

Other than the debug line, it works as described. Thanks! 👍

app/bundles/CoreBundle/Assets/js/libraries/4j.codemirror.js Outdated
window.focus();

if (typeof Mautic != 'undefined' && Mautic.codeMode != true) {
console.log(Mautic);
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

This is probably a forgotten debug code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Need to update this as the JS fixes for dwc are not longer required - fixed in another PR by @virlatinus

@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged WIP PR's that are not ready for review and are currently in progress and 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 Apr 10, 2017
@escopecz escopecz added the has-conflicts Pull requests that cannot be merged until conflicts have been resolved label Apr 19, 2017
@virlatinus virlatinus added ready-to-test PR's that are ready to test and removed has-conflicts Pull requests that cannot be merged until conflicts have been resolved labels Apr 19, 2017
@escopecz escopecz added pending-test-confirmation PR's that require one test before they can be merged and removed WIP PR's that are not ready for review and are currently in progress labels Apr 19, 2017
@virlatinus
Copy link
Contributor

I checked it out to my current staging and I'm getting an error
mautic.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\ContextErrorException: "PHP Warning - Invalid argument supplied for foreach()" at /var/www/mautic-fork/app/bundles/CoreBundle/Helper/AbstractFormFieldHelper.php

@virlatinus virlatinus added the pending-feedback PR's and issues that are awaiting feedback from the author label Apr 19, 2017
@virlatinus
Copy link
Contributor

I followed the steps again to test: created the custom field and when editing a contact I get the error
I then deleted the custom field and the error is gone.

@virlatinus
Copy link
Contributor

This code is already on #3743

@virlatinus virlatinus closed this Apr 20, 2017
@dongilbert dongilbert deleted the bugfix-dec-custom-filters branch May 18, 2018 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@escopecz escopecz escopecz approved these changes

Assignees

@escopecz escopecz

@virlatinus virlatinus

Labels
bug Issues or PR's relating to bugs pending-feedback PR's and issues that are awaiting feedback from the author pending-test-confirmation PR's that require one test before they can be merged ready-to-test PR's that are ready to test
Projects
None yet
Milestone
2.8.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dongilbert @virlatinus @escopecz @alanhartless @matishaladiwala