Remove people from campaigns when they no longer match campaign segments

[joshuap]

If they don't match campaign segments, they won't match any steps. Removing them early means that if they get re-added to a campaign once the do match again, they'll continue to receive emails where they left off. Currently, the system will just skip every step until they exit the campaign anyway.

[feliperaul]

@joshuap In the readme, it appears to state that this feature is already in place:

You can also narrow entire campaigns to certain users using the segment method. If they upgrade half way through the campaign, Heya will stop sending messages and remove them from the campaign.

Also:

Likewise, you can require that users meet conditions to continue receiving a campaign. Here's a campaign which sends messages only to trial users--non-trial users will be removed from the campaign:

So, I have a PR in the works to implement this and close this issue, and I added some tests.

However, I haven't issued it so far because it is failing the test it waits for segments to match on test/lib/heya/campaigns/scheduler_test.rb.

The test, which I pasted below for a quick reference, goes through step :two, for which the user will NOT have matched it's segment. The test then proceeds to assert if it will receive step :three.

Based on the readme and on this issue, I think this test should be removed; however, it's existence makes me think this behavior is actually intended for now. Which direction should we go? :)

test "it waits for segments to match" do
        action = Minitest::Mock.new
        campaign = create_test_campaign {
          default action: action
          user_type "Contact"
          step :one, wait: 0
          step :two, wait: 2.days, segment: ->(u) { u.traits["foo"] == "bar" }
          step :three, wait: 1.day, segment: ->(u) { u.traits["bar"] == "baz" }
        }
        contact = contacts(:one)
        contact.update_attribute(:traits, {bar: "baz"})
        campaign.add(contact, send_now: false)

        action.expect(:new, NullMail, [{
          user: contact,
          step: campaign.steps.first
        }])
        run_twice
        assert_mock action

        Timecop.travel(1.days.from_now)
        run_twice
        assert_mock action

        Timecop.travel(1.days.from_now)
        action.expect(:new, NullMail, [{
          user: contact,
          step: campaign.steps.third
        }])
        run_twice
        assert_mock action

[joshuap]

@feliperaul I'm not sure I follow. Can you explain what specific behavior you're trying to add? (E.g. what does your PR do?)

I think you're right that this issue is already implemented and can be closed. What this issue is referencing is specifically the global campaign segments feature. I think when I created this issue, heya's implementation was different. The way it works now is that it waits the maximum wait time in the campaign, and if no segments match, the user is removed (I think it can take a few scheduler runs for that to happen).

The "it waits for segments to match" test looks correct to me, although maybe it could be named better. Basically what that test is asserting is that the campaign waits the full wait time for each step, even if the user doesn't match a segment in the meantime. That's because the user could match by the time the email is sent. In the test, it sends step :one immediately (segment match), then waits the full 2.days for step :two before skipping to step :three. Because step :three has a wait of only 1.day, it sends it immediately (because it's been 2 days since the user received an email.