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

Fixed two segment membership based errors with companies #5472

Merged
merged 3 commits into from
Dec 21, 2017
Merged

Fixed two segment membership based errors with companies #5472

merged 3 commits into from
Dec 21, 2017

Conversation

alanhartless
Copy link
Contributor

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

Description:

This fixes two bugs triggered by using segment membership filters that are based on segments using company fields.

Steps to reproduce the bug:

  1. Create a segment that has a single company field filter (i.e. company city = something)
  2. Create a second segment that has a company custom field (ie. company state = something) and a second filter that is segment membership excludes the segment created above.
  3. Run the 'mautic:segment:update' command for the segment in step 2. Notice the SQL error.

Steps to test this PR:

  1. Repeat and there will be no SQL errors
  2. Run the new segment related functional tests

@alanhartless alanhartless added bug Issues or PR's relating to bugs ready-to-test PR's that are ready to test labels Dec 21, 2017
@alanhartless alanhartless added this to the 2.12.1 milestone Dec 21, 2017
@escopecz escopecz self-assigned this Dec 21, 2017
escopecz
escopecz approved these changes Dec 21, 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.

The error is gone 👍

@escopecz escopecz added the pending-test-confirmation PR's that require one test before they can be merged label Dec 21, 2017
@escopecz escopecz force-pushed the bug-segment-membership-errors branch from 27e3554 to 72afb5d Compare December 21, 2017 09:46
@Hadamcik
Copy link
Contributor

Works for me, error is no more

@Hadamcik Hadamcik merged commit ad1e448 into mautic:staging Dec 21, 2017
@dbhurley dbhurley removed the ready-to-test PR's that are ready to test label Dec 21, 2017
@Hadamcik Hadamcik removed the pending-test-confirmation PR's that require one test before they can be merged label Dec 21, 2017
@alanhartless alanhartless deleted the bug-segment-membership-errors branch January 5, 2018 21:18
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

Labels
bug Issues or PR's relating to bugs
Projects
None yet
Milestone
2.12.1
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@alanhartless @Hadamcik @escopecz @dbhurley