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

Support for sub queries in segment filters #7424

Open
wants to merge 5 commits into
base: staging
from

Conversation

Projects
None yet
1 participant
@kuzmany
Copy link
Contributor

commented Apr 16, 2019

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) #5732
BC breaks?
Deprecations?

Description:

This PR trying solve #5732
Current segment filters works separately. That means IF we use page hit date condition and page count condition, both are executed independently. That cause logic when we expect communicate between table data.
We create sub query for each table (page_hit, email_stats) and cover these queries until OR statement is executed. Or cover it on the end of filter query builder. On each cycle, we reset current sub queries.

My tests filter

image

Query:

SELECT l.id FROM leads l WHERE (EXISTS(SELECT NULL FROM page_hits qkpoIBuO WHERE (qkpoIBuO.lead_id = l.id) AND (qkpoIBuO.url LIKE '%mautic%') AND (qkpoIBuO.date_hit > '2019-04-11') GROUP BY qkpoIBuO.lead_id HAVING count(DISTINCT qkpoIBuO.id) < 2)) AND (EXISTS(SELECT NULL FROM email_stats beuFTQCU WHERE (beuFTQCU.lead_id = l.id) AND (beuFTQCU.is_read = 1) AND (beuFTQCU.email_id IN (1, 5, 6, 10)) GROUP BY beuFTQCU.lead_id HAVING COALESCE(SUM(beuFTQCU.open_count), 0) > 1)))

Steps to reproduce the bug:

  1. Create new segment
  2. Add filter "Visited URL" and choose an URL
  3. Add filter "Count Visited URL" and choose a condition
  4. Inspect history of each contact added to the segment, you'll see the bug

Steps to test this PR:

  1. Load up this PR
  2. Test it with page hits or email hits

List backwards compatibility breaks:

  1. We add new extra decorators, then there is no BC breaks

@kuzmany kuzmany added this to the 2.16.0 milestone Apr 16, 2019

kuzmany added some commits Apr 16, 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.