Skip to content

Commit 27883eb

Browse files
committed
fix(cohorts): queue membership count refreshes for cohort segments
1 parent 76da16c commit 27883eb

3 files changed

Lines changed: 67 additions & 999 deletions

File tree

api/cohorts/services.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import csv
22
import io
33
import typing
4+
from datetime import timedelta
45
from uuid import UUID
56

67
import structlog
8+
from django.conf import settings
79
from django.db import transaction
810
from django.db.models import F, QuerySet
911
from django.utils import timezone
@@ -101,6 +103,21 @@ def apply_pending_memberships(cohort: Cohort) -> bool:
101103
removed_count
102104
)
103105
if added_count or removed_count:
106+
if settings.CLICKHOUSE_ENABLED:
107+
from segment_membership.services import enqueue_membership_refresh
108+
109+
# Cohort membership is the one way a segment's members change
110+
# without a segment or identity edit, so nothing else queues a
111+
# count refresh. The delay lets the identity changes reach
112+
# ClickHouse before the recount; repeated batches collapse into
113+
# the one pending refresh.
114+
enqueue_membership_refresh(
115+
environment.project,
116+
delay_until=timezone.now()
117+
+ timedelta(
118+
seconds=settings.SEGMENT_MEMBERSHIP_DELETE_REFRESH_DELAY_SECONDS
119+
),
120+
)
104121
logger.info(
105122
"membership.applied",
106123
cohort__id=cohort.id,
@@ -158,6 +175,13 @@ def create_cohort(
158175
project__id=environment.project_id,
159176
organisation__id=environment.project.organisation_id,
160177
)
178+
if settings.CLICKHOUSE_ENABLED:
179+
from segment_membership.services import enqueue_membership_refresh
180+
181+
# The segment is created directly rather than through the segment
182+
# serializer, which is where membership count refreshes are normally
183+
# queued from.
184+
enqueue_membership_refresh(environment.project)
161185
return cohort
162186

163187

api/tests/unit/cohorts/test_services.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import io
22

33
import pytest
4+
from django.utils import timezone
45
from flag_engine.segments.constants import IS_SET
6+
from pytest_django.fixtures import SettingsWrapper
57
from pytest_mock import MockerFixture
68
from pytest_structlog import StructuredLogCapture
79
from rest_framework.exceptions import ValidationError
@@ -546,3 +548,44 @@ def test_sync_cohort_memberships_from_csv__edge_cohort__applies_traits(
546548
assert document["system_traits"] == {edge_cohort.system_trait_key: True}
547549
membership = CohortMembership.objects.get(cohort=edge_cohort)
548550
assert membership.state == CohortMembershipState.APPLIED
551+
552+
553+
def test_create_cohort__clickhouse_enabled__queues_membership_refresh(
554+
environment: Environment,
555+
settings: SettingsWrapper,
556+
mocker: MockerFixture,
557+
) -> None:
558+
# Given
559+
settings.CLICKHOUSE_ENABLED = True
560+
enqueue_mock = mocker.patch(
561+
"segment_membership.services.enqueue_membership_refresh"
562+
)
563+
564+
# When
565+
create_cohort(environment=environment, name="Beta users")
566+
567+
# Then
568+
enqueue_mock.assert_called_once_with(environment.project)
569+
570+
571+
def test_apply_pending_memberships__clickhouse_enabled__queues_delayed_refresh(
572+
cohort: Cohort,
573+
settings: SettingsWrapper,
574+
mocker: MockerFixture,
575+
) -> None:
576+
# Given
577+
settings.CLICKHOUSE_ENABLED = True
578+
settings.SEGMENT_MEMBERSHIP_DELETE_REFRESH_DELAY_SECONDS = 60
579+
enqueue_mock = mocker.patch(
580+
"segment_membership.services.enqueue_membership_refresh"
581+
)
582+
CohortMembership.objects.create(cohort=cohort, identifier="user-1")
583+
584+
# When
585+
apply_pending_memberships(cohort)
586+
587+
# Then
588+
enqueue_mock.assert_called_once()
589+
args, kwargs = enqueue_mock.call_args
590+
assert args == (cohort.environment.project,)
591+
assert kwargs["delay_until"] > timezone.now()

0 commit comments

Comments
 (0)