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 report filters for bounce and unsubscribe ratio #7070

Merged
merged 6 commits into from Jan 15, 2019

Conversation

Projects
3 participants
@kuzmany
Copy link
Contributor

kuzmany commented Dec 29, 2018

Please be sure you are submitting this against the staging branch.

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

Description:

Unsubscribed and bounce ratio in report filters doesn't work because aggregated columns can't work in WHERE statement. This PR fixed it.

Steps to reproduce the bug:

  1. Create a report and choose email as data source
    image
  2. add some columns.
  3. Save. See that there is no problem with the report
  4. Now go back editing the report and add unsubscribe ratio (or bounce ratio) as a filter. Choose greater than and choose a number
    image
  5. Save and close. See error 500
    image

Steps to test this PR:

  1. Load up this PR
  2. Repeat all steps and see If unsubscribe and bounce ratio works properly.

List deprecations along with the new alternative:

List backwards compatibility breaks:

@kuzmany kuzmany added this to the 2.15.1 milestone Dec 29, 2018

kuzmany added some commits Dec 29, 2018

@kuzmany kuzmany added this to Ready to Test (first time) in Mautic 2 Dec 29, 2018

@johbuch

This comment has been minimized.

Copy link

johbuch commented Dec 31, 2018

does not work for me, I still have SQL error. Even without adding the unsubscribe ratio as a filter.

@kuzmany kuzmany added WIP and removed Ready To Test labels Jan 2, 2019

@kuzmany kuzmany added Ready To Test and removed WIP labels Jan 2, 2019

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Jan 2, 2019

@johbuch Fixed, please re-test

@johbuch

johbuch approved these changes Jan 3, 2019

Copy link

johbuch left a comment

It works ! thanks zdeno

@npracht npracht moved this from Ready to Test (first time) to Ready to Test (confirmation) in Mautic 2 Jan 3, 2019

@alanhartless alanhartless added this to Needs Second Test in 2.15.1 Jan 14, 2019

Mautic 2 automation moved this from Ready to Test (confirmation) to Ready to Test (first time) Jan 15, 2019

@alanhartless alanhartless merged commit eb25c49 into mautic:staging Jan 15, 2019

2 checks passed

Scrutinizer Analysis: 6 new issues – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Mautic 2 automation moved this from Ready to Test (first time) to Merged Jan 15, 2019

@alanhartless alanhartless moved this from Needs Second Test to Merged in 2.15.1 Jan 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.