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

Prevent contacts from getting prematurely removed from a segment #2716

Merged
merged 1 commit into from
Oct 11, 2016

Conversation

alanhartless
Copy link
Contributor

Q A
Bug fix? y
New feature? n
Related user documentation PR URL na
Related developer documentation PR URL na
Issues addressed (#s or URLs) #2461 possibly #2419
BC breaks? n
Deprecations? n

Description:

If a contact with an ID between existing IDs of a segment is added to the segment, IDs greater than that of the contact just added will be prematurely removed from the segment - then added right back on the next run.

Steps to test this PR:

Take the steps below and note that the higher contact IDs should not be removed.

Steps to reproduce the bug:

Taken from #2461

To duplicate the error:

  1. create a segment which adds people based on some criteria, like visiting a page (my specific issue) or having a particular field value set should also work to simulate the effect.
  2. get a handful of leads to do whatever they need to qualify for this segment & run cron to get them in.
  3. pick a lead who has a leadid somewhere in between the group that was previously added and then do the thing to this lead that will make it qualify for the segment.
  4. When you run cron next time you will see that this latest lead (it ain't new) gets added into the segment, and then ANYONE whose leadid > than the one you just entered (this is where maxId comes in) gets removed from the segment.
  5. When you run cron again, then the removed people get re-qualified and re-added to the segment, but now their date-added is newer, which will cause problems with campaigns that starts with qualifying into a segment and relies on waiting time before another step, the people keep re-qualifying and restarting the campaign, never getting to trigger the next step...

@escopecz
Copy link
Sponsor Member

Works as described 👍

@escopecz escopecz added the pending-test-confirmation PR's that require one test before they can be merged label Oct 11, 2016
Copy link
Member

@dongilbert dongilbert left a comment

Choose a reason for hiding this comment

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

+1 thanks @alanhartless! I thought I was doing something wrong while testing but discovered that the contact wasn't being removed because it was manually added to the segment. We're all good now and testing was successful.

@dongilbert dongilbert removed the pending-test-confirmation PR's that require one test before they can be merged label Oct 11, 2016
@dongilbert dongilbert merged commit a108b93 into mautic:staging Oct 11, 2016
@alanhartless alanhartless deleted the bug-2461 branch October 24, 2016 01:53
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 ready-to-test PR's that are ready to test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants