Skip to content

Commit 15948bd

Browse files
emyllerclaude
andcommitted
Make LiveSegmentManager the base manager for Segment
Co-authored-by: Claude <[email protected]>
1 parent 4600297 commit 15948bd

File tree

3 files changed

+31
-23
lines changed

3 files changed

+31
-23
lines changed

api/segments/models.py

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,32 @@
3232
logger = logging.getLogger(__name__)
3333

3434

35-
class SegmentManager(SoftDeleteExportableManager):
36-
pass
37-
38-
3935
class LiveSegmentManager(SoftDeleteExportableManager):
4036
def get_queryset(self): # type: ignore[no-untyped-def]
4137
"""
42-
Returns only the canonical segments, which will always be
43-
the highest version.
38+
Returns only canonical segments (where version_of is NULL).
39+
Canonical segments represent the current/live version.
4440
"""
4541
return super().get_queryset().filter(version_of__isnull=True)
4642

4743

44+
class RevisionsManager(SoftDeleteExportableManager):
45+
def get_queryset(self): # type: ignore[no-untyped-def]
46+
"""
47+
Returns only segment revisions (where version_of is NOT NULL).
48+
Revisions are historical versions of segments.
49+
"""
50+
return super().get_queryset().filter(version_of__isnull=False)
51+
52+
53+
class AllSegmentsManager(SoftDeleteExportableManager):
54+
"""
55+
Returns all segments (both canonical and revisions).
56+
Only filters out soft-deleted segments.
57+
"""
58+
pass
59+
60+
4861
class ConfiguredOrderManager(SoftDeleteExportableManager, models.Manager[ModelT]):
4962
setting_name: str
5063

@@ -119,10 +132,11 @@ class Segment(
119132
updated_at = models.DateTimeField(null=True, auto_now=True)
120133
is_system_segment = models.BooleanField(default=False)
121134

122-
objects = SegmentManager() # type: ignore[misc]
123-
124-
# Only serves segments that are the canonical version.
125-
live_objects = LiveSegmentManager()
135+
# Manager declarations - order matters! First manager is the base_manager used in relations.
136+
objects = LiveSegmentManager() # type: ignore[misc] # Default: canonical segments only
137+
live_objects = objects # Explicit alias for clarity
138+
revisions = RevisionsManager() # type: ignore[misc] # Only historical versions
139+
all_objects = AllSegmentsManager() # type: ignore[misc] # Both canonical and revisions
126140

127141
class Meta:
128142
ordering = ("id",) # explicit ordering to prevent pagination warnings

api/tests/unit/segments/test_unit_segments_views.py

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -207,9 +207,7 @@ def test_segments_limit_ignores_old_segment_versions(
207207

208208
# and create some older versions for the segment fixture
209209
segment.clone(is_revision=True)
210-
# With the NULL-based canonical representation, only revisions are counted here
211-
# (the canonical segment has version_of=None).
212-
assert Segment.objects.filter(version_of=segment).count() == 1
210+
assert Segment.revisions.filter(version_of=segment).count() == 1
213211
assert Segment.live_objects.count() == 1
214212

215213
url = reverse("api-v1:projects:project-segments-list", args=[project.id])
@@ -1060,13 +1058,11 @@ def test_update_segment_versioned_segment(
10601058
assert response.status_code == status.HTTP_200_OK
10611059

10621060
# Now verify that a new versioned segment has been set.
1063-
# With the NULL-based canonical representation, canonical segments have
1064-
# version_of=None, so only revisions are counted here.
1065-
assert Segment.objects.filter(version_of=segment).count() == 1
1061+
assert Segment.revisions.filter(version_of=segment).count() == 1
10661062

10671063
# Now check the previously versioned segment to match former count of conditions.
10681064

1069-
versioned_segment = Segment.objects.filter(version_of=segment, version=1).first()
1065+
versioned_segment = Segment.revisions.filter(version_of=segment, version=1).first()
10701066
assert versioned_segment != segment
10711067
assert versioned_segment.rules.count() == 1
10721068
versioned_rule = versioned_segment.rules.first()
@@ -1097,10 +1093,8 @@ def test_update_segment_versioned_segment_with_thrown_exception(
10971093
rule=nested_rule, property="foo", operator=EQUAL, value="bar"
10981094
)
10991095

1100-
# With the NULL-based canonical representation, canonical segments have
1101-
# version_of=None, so they are not included in this count.
11021096
assert segment.version == 1
1103-
assert Segment.objects.filter(version_of=segment).count() == 0
1097+
assert Segment.revisions.filter(version_of=segment).count() == 0
11041098

11051099
new_condition_property = "foo2"
11061100
new_condition_value = "bar"
@@ -1153,7 +1147,7 @@ def test_update_segment_versioned_segment_with_thrown_exception(
11531147
# Now verify that the version of the segment has not been changed.
11541148
# The transaction should have rolled back, so no revisions should exist.
11551149
assert segment.version == 1
1156-
assert Segment.objects.filter(version_of=segment).count() == 0
1150+
assert Segment.revisions.filter(version_of=segment).count() == 0
11571151

11581152

11591153
@pytest.mark.parametrize(

api/util/mappers/engine.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,8 +201,8 @@ def map_environment_to_engine(
201201
organisation: "Organisation" = project.organisation
202202

203203
# Read relationships - grab all the data needed from the ORM here.
204-
# Note: project.segments is prefetched with Segment.live_objects which already
205-
# filters by version_of__isnull=True, so no need to filter again here.
204+
# Note: project.segments uses Segment's base manager (LiveSegmentManager),
205+
# which returns only canonical segments.
206206
project_segments = list(project.segments.all())
207207

208208
project_segment_rules_by_segment_id: Dict[

0 commit comments

Comments
 (0)