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

feat: Add version_of and matches to rules and conditions #4988

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ hubspot-api-client = "^8.2.1"
djangorestframework-dataclasses = "^1.3.1"
pyotp = "^2.9.0"
flagsmith-task-processor = { git = "https://github.com/Flagsmith/flagsmith-task-processor", tag = "v1.1.1" }
flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.4.2" }
flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", branch = "feat/add_version_of_to_rules_and_conditions" }
tzdata = "^2024.1"
djangorestframework-simplejwt = "^5.3.1"
structlog = "^24.4.0"
Expand All @@ -197,7 +197,7 @@ flagsmith-ldap = { git = "https://github.com/flagsmith/flagsmith-ldap", tag = "v
optional = true

[tool.poetry.group.workflows.dependencies]
workflows-logic = { git = "https://github.com/flagsmith/flagsmith-workflows", tag = "v2.7.5" }
workflows-logic = { git = "https://github.com/flagsmith/flagsmith-workflows", branch = "feat/add_matches_to_workflows" }

[tool.poetry.group.licensing]
optional = true
Expand Down
48 changes: 48 additions & 0 deletions api/segments/migrations/0027_version_rules_and_conditions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
# Generated by Django 4.2.17 on 2025-01-08 15:21

from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
("segments", "0026_add_change_request_to_segments"),
]

operations = [
migrations.AddField(
model_name="condition",
name="version_of",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="versioned_conditions",
to="segments.condition",
),
),
migrations.AddField(
model_name="historicalcondition",
name="version_of",
field=models.ForeignKey(
blank=True,
db_constraint=False,
null=True,
on_delete=django.db.models.deletion.DO_NOTHING,
related_name="+",
to="segments.condition",
),
),
migrations.AddField(
model_name="segmentrule",
name="version_of",
field=models.ForeignKey(
blank=True,
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="versioned_rules",
to="segments.segmentrule",
),
),
]
79 changes: 79 additions & 0 deletions api/segments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,38 @@ def deep_clone(self) -> "Segment":

return cloned_segment

def match_rules_to_segment(self, segment: "Segment") -> None:
_rules = self.rules.all()
matched_rules = set()
matched_sub_rules = set()

for rule in segment.rules.all():
sub_rules = rule.rules.all()
for sub_rule in sub_rules:
sub_rule_matched = False
for _rule in _rules:
if sub_rule_matched:
break

if _rule in matched_rules and _rule.version_of != rule:
continue

if rule.type != _rule.type:
continue
for _sub_rule in _rule.rules.all():
if _sub_rule in matched_sub_rules: # pragma: no cover
continue
if _sub_rule.matches_rule(sub_rule):
_sub_rule.version_of = sub_rule
sub_rule_matched = True
matched_sub_rules.add(_sub_rule)
_rule.version_of = rule
matched_rules.add(_rule)
break
SegmentRule.objects.bulk_update(
matched_rules | matched_sub_rules, fields=["version_of"]
)

def get_create_log_message(self, history_instance) -> typing.Optional[str]:
return SEGMENT_CREATED_MESSAGE % self.name

Expand All @@ -224,6 +256,13 @@ class SegmentRule(SoftDeleteExportableModel):
rule = models.ForeignKey(
"self", on_delete=models.CASCADE, related_name="rules", null=True, blank=True
)
version_of = models.ForeignKey(
"self",
on_delete=models.SET_NULL,
related_name="versioned_rules",
null=True,
blank=True,
)

type = models.CharField(max_length=50, choices=RULE_TYPES)

Expand Down Expand Up @@ -261,13 +300,44 @@ def get_segment(self):
rule = rule.rule
return rule.segment

def matches_rule(self, rule: "SegmentRule") -> bool:
if rule.type != self.type:
return False

conditions = rule.conditions.all()
_conditions = self.conditions.all()

if not conditions and not _conditions:
# Empty rule with the same type matches.
return True

matched_conditions = set()

for condition in conditions:
for _condition in _conditions:
if _condition in matched_conditions:
continue
if (
condition.operator == _condition.operator
and condition.property == _condition.property
):
matched_conditions.add(_condition)
_condition.version_of = condition
break
if not matched_conditions:
return False

Condition.objects.bulk_update(matched_conditions, fields=["version_of"])
return True

def deep_clone(self, cloned_segment: Segment) -> "SegmentRule":
if self.rule:
# Since we're expecting a rule that is only belonging to a
# segment, since a rule either belongs to a segment xor belongs
# to a rule, we don't expect there also to be a rule associated.
assert False, "Unexpected rule, expecting segment set not rule"
cloned_rule = deepcopy(self)
cloned_rule.version_of = self
cloned_rule.segment = cloned_segment
cloned_rule.uuid = uuid.uuid4()
cloned_rule.id = None
Expand All @@ -284,6 +354,7 @@ def deep_clone(self, cloned_segment: Segment) -> "SegmentRule":
assert False, "Expected two layers of rules, not more"

cloned_sub_rule = deepcopy(sub_rule)
cloned_sub_rule.version_of = sub_rule
cloned_sub_rule.rule = cloned_rule
cloned_sub_rule.uuid = uuid.uuid4()
cloned_sub_rule.id = None
Expand All @@ -296,6 +367,7 @@ def deep_clone(self, cloned_segment: Segment) -> "SegmentRule":
cloned_conditions = []
for condition in sub_rule.conditions.all():
cloned_condition = deepcopy(condition)
cloned_condition.version_of = condition
cloned_condition.rule = cloned_sub_rule
cloned_condition.uuid = uuid.uuid4()
cloned_condition.id = None
Expand Down Expand Up @@ -348,6 +420,13 @@ class Condition(
rule = models.ForeignKey(
SegmentRule, on_delete=models.CASCADE, related_name="conditions"
)
version_of = models.ForeignKey(
"self",
on_delete=models.SET_NULL,
related_name="versioned_conditions",
null=True,
blank=True,
)

created_at = models.DateTimeField(null=True, auto_now_add=True)
updated_at = models.DateTimeField(null=True, auto_now=True)
Expand Down
Loading
Loading