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: Create change requests for segments #4265

Open
wants to merge 120 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
120 commits
Select commit Hold shift + click to select a range
1ac4b83
Add change request to Segment
zachaysan Jun 28, 2024
365e8a0
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Jun 28, 2024
d78bc71
Add migration for adding change requests to segments
zachaysan Jun 28, 2024
98c56f4
chore(build): API test image
khvn26 Jun 28, 2024
4e7421f
Also publish flagsmith-api-test for main
khvn26 Jun 28, 2024
d2fda11
fix target stage name
khvn26 Jun 28, 2024
c7c3ed0
don't scan test images
khvn26 Jun 28, 2024
e15c0cc
Add shallow clone
zachaysan Jul 1, 2024
12dcae8
Merge branch 'chore/api-test-image' into feat/create_change_requests_…
zachaysan Jul 1, 2024
08b693f
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Jul 1, 2024
d9e3a45
Switch to optional due to string issue
zachaysan Jul 1, 2024
edd2eeb
Set version to None
zachaysan Jul 17, 2024
8e8bccb
Add missing attributes to shallow clone
zachaysan Jul 17, 2024
0255717
Fix conflicts and merge branch 'main' into feat/create_change_request…
zachaysan Jul 17, 2024
ab52d25
Update Django migration number
zachaysan Jul 17, 2024
b0d5119
Revert Dockerfile change
zachaysan Jul 17, 2024
29b0fd5
Remove editor noise
zachaysan Jul 17, 2024
9f93b0a
Attempt using a property instead of the many to many relation
zachaysan Jul 18, 2024
0583c15
Create test for segment retrieval
zachaysan Jul 18, 2024
4f52f28
Add migration for relation name change
zachaysan Jul 18, 2024
5d37755
Update migrations
zachaysan Jul 22, 2024
197f7eb
Add django polymorphic
zachaysan Jul 22, 2024
b4af2f3
Add polymorphic managers
zachaysan Jul 22, 2024
5d1fc03
Remove segments property
zachaysan Jul 22, 2024
8ce03ab
Add polymorphic to segments
zachaysan Jul 22, 2024
82d67c6
Update test
zachaysan Jul 22, 2024
299ecd9
Create polymorphic managers and models
zachaysan Jul 23, 2024
d1381a7
Use polymorphic manager
zachaysan Jul 23, 2024
fc10d49
Add docstring
zachaysan Jul 23, 2024
89a744a
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Jul 23, 2024
1ad0c6c
Update migration to new plan
zachaysan Aug 8, 2024
2a87984
Remove polymorphic concerns from core models
zachaysan Aug 8, 2024
cf2bf1f
Remove polymorphic concerns from managers and switch to live manager
zachaysan Aug 8, 2024
11eb706
Remove AllSegment model and remove polymorphic concerns
zachaysan Aug 8, 2024
6bcc37e
Add segment manager
zachaysan Aug 8, 2024
c3ed2ca
Change name to segments
zachaysan Aug 8, 2024
6e95ee6
Update manager naming
zachaysan Aug 8, 2024
eaed7cd
Fix conflicts and merge branch 'main' into feat/create_change_request…
zachaysan Aug 8, 2024
aa7c771
Create test with model manager returning segments
zachaysan Aug 8, 2024
8569c85
Switch to live objects manager
zachaysan Aug 8, 2024
398e1ab
Remove stale comment
zachaysan Aug 8, 2024
8722dae
Remove manager (which is weird, because it's still there...
zachaysan Aug 9, 2024
06f7146
Use live_objects manager
zachaysan Aug 12, 2024
32e63cb
Switch manager
zachaysan Aug 15, 2024
4e037ae
Remove stale all segment type
zachaysan Aug 15, 2024
d08c3ab
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Aug 15, 2024
cbfa0cc
Set Prefect to live_objects Segment manager
zachaysan Aug 16, 2024
3aaa93e
Add segment tests for getting environment document
zachaysan Aug 16, 2024
7be93dc
Revet to calling project.segments for prefetch functionality and add …
zachaysan Aug 16, 2024
e50499f
Filter project segments to canonical version
zachaysan Aug 19, 2024
366ade9
Switch to objects manager
zachaysan Aug 19, 2024
cc7c753
Fix test to use default manager
zachaysan Aug 19, 2024
b52b2f7
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Aug 19, 2024
bba8d9b
Remove stale change request checking code
zachaysan Aug 20, 2024
082b023
Trigger build
zachaysan Aug 20, 2024
84287df
Fix typing
zachaysan Aug 20, 2024
3221446
Fix typing
zachaysan Aug 20, 2024
ef295d0
Trigger build
zachaysan Aug 27, 2024
f656603
Add publishing segments to change requests
zachaysan Aug 28, 2024
ed4f12d
Add test for publishing segments
zachaysan Aug 28, 2024
494ed76
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Aug 29, 2024
a8e0bee
Fix spelling of guard
zachaysan Sep 6, 2024
a126336
Create get_project_segments_from_cache function
zachaysan Sep 6, 2024
aaa5d7a
Set blank to True
zachaysan Sep 6, 2024
a18ad86
Move code to a function
zachaysan Sep 6, 2024
4fb4176
Update mock
zachaysan Sep 6, 2024
6c0429d
Test change request commit instead of private method
zachaysan Sep 6, 2024
b266156
Skip early return
zachaysan Sep 6, 2024
d277a1d
Fix conflicts and merge branch 'main' into feat/create_change_request…
zachaysan Sep 6, 2024
daee2c6
Add project to change request via migration
zachaysan Sep 18, 2024
5d02c15
Add project permissions for project level change requests via migration
zachaysan Sep 18, 2024
de18843
Add test for deleting a change request via task
zachaysan Sep 18, 2024
e123371
Create delete change request task
zachaysan Sep 18, 2024
703776f
Add delete change request to project delete task
zachaysan Sep 18, 2024
64d241b
Reorder task
zachaysan Sep 18, 2024
06c138f
Add project level permissions for change requests
zachaysan Sep 18, 2024
b36b730
Add project to change request and set environment to nullable
zachaysan Sep 18, 2024
57f8e0e
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Sep 18, 2024
ec71d35
Update comment
zachaysan Sep 18, 2024
c65dc1b
Add project change request viewset to urls.py
zachaysan Sep 18, 2024
6b9626d
Attempt nested router
zachaysan Sep 19, 2024
ae44b79
Fix projects
zachaysan Sep 19, 2024
8d85fe1
Try projects
zachaysan Sep 19, 2024
b65065f
Try including the router
zachaysan Sep 19, 2024
88e1104
Fix project lookup
zachaysan Sep 19, 2024
ab25f93
Check for environment
zachaysan Sep 19, 2024
508f0aa
Update url generation code for missing environments
zachaysan Sep 20, 2024
a7a7377
Add minimum_change_request_approvals to project
zachaysan Sep 20, 2024
086a1e2
Add project related methods
zachaysan Sep 20, 2024
b34b822
Update permission list count
zachaysan Sep 25, 2024
0316fb8
Add change request to successful delete
zachaysan Sep 25, 2024
3e30460
Add unit tests for change requests
zachaysan Sep 25, 2024
b5e86cd
Add pragma: no cover since this is a migration
zachaysan Sep 25, 2024
2127cd3
Add pragma: no cover since this is a migration
zachaysan Sep 25, 2024
fc7b5f8
Add pragma: no cover since these exceptions are not possible due to t…
zachaysan Sep 25, 2024
5f983f2
Create a project change request fixture
zachaysan Sep 25, 2024
d35adc5
Add pragma: no cover since these are tested from the workflows repo's…
zachaysan Sep 25, 2024
1568aa5
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Sep 25, 2024
5a840f9
Trigger build
zachaysan Sep 25, 2024
18796e3
Switch urls to projects module
zachaysan Oct 3, 2024
3cadb1b
Fix conflicts and merge branch 'main' into feat/create_change_request…
zachaysan Oct 3, 2024
6b6e0d4
Revise change request to account for test coverage and naming
zachaysan Oct 3, 2024
3c30584
Add typing
zachaysan Oct 3, 2024
46647f9
Add migration test for 0011_add_project_to_change_requests.py
zachaysan Oct 3, 2024
11305b1
Set project to be non-null
zachaysan Oct 3, 2024
95f4960
Set project as non-null and use the environment to default to the pro…
zachaysan Oct 3, 2024
63d10d0
Delete task and related testing logic
zachaysan Oct 3, 2024
0d8ede4
Remove now-unnecessary task
zachaysan Oct 3, 2024
4a806f0
Update project related logic now that it's non-nullable
zachaysan Oct 3, 2024
c0ec3cb
Update migration to remove dynamically loaded descriptions
zachaysan Oct 3, 2024
539086c
Add in minimum change request for approvals to serializer
zachaysan Oct 3, 2024
27d6338
Switch to project_id
zachaysan Oct 3, 2024
ab25380
Remove change request from project test
zachaysan Oct 3, 2024
778dbc3
Merge branch 'main' into feat/create_change_requests_for_segments
zachaysan Oct 4, 2024
04a1c55
Update pyproject for build step
zachaysan Oct 4, 2024
4be5757
Update lock
zachaysan Oct 7, 2024
dbe95fe
Update typing since library upgrade
zachaysan Oct 7, 2024
cae0b9e
Update lock
zachaysan Oct 7, 2024
fc652e4
Update lock
zachaysan Oct 7, 2024
b2c0668
Update lock
zachaysan Oct 7, 2024
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
2 changes: 1 addition & 1 deletion api/app/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
if settings.SAML_INSTALLED:
urlpatterns.append(path("api/v1/auth/saml/", include("saml.urls")))

if settings.WORKFLOWS_LOGIC_INSTALLED:
if settings.WORKFLOWS_LOGIC_INSTALLED: # pragma: no cover
workflow_views = importlib.import_module("workflows_logic.views")
urlpatterns.extend(
[
Expand Down
12 changes: 10 additions & 2 deletions api/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from rest_framework.authtoken.models import Token
from rest_framework.test import APIClient
from task_processor.task_run_method import TaskRunMethod
from urllib3 import HTTPResponse
from urllib3.connectionpool import HTTPConnectionPool
from xdist import get_xdist_worker_id

Expand Down Expand Up @@ -163,7 +164,7 @@ def urlopen_mock(
url: str,
*args,
**kwargs,
) -> HTTPConnectionPool.ResponseCls:
) -> HTTPResponse:
if self.host in allowed_hosts:
return original_urlopen(self, method, url, *args, **kwargs)

Expand Down Expand Up @@ -513,12 +514,19 @@ def feature(project: Project, environment: Environment) -> Feature:


@pytest.fixture()
def change_request(environment, admin_user):
def change_request(environment: Environment, admin_user: FFAdminUser) -> ChangeRequest:
return ChangeRequest.objects.create(
environment=environment, title="Test CR", user_id=admin_user.id
)


@pytest.fixture()
def project_change_request(project: Project, admin_user: FFAdminUser) -> ChangeRequest:
return ChangeRequest.objects.create(
project=project, title="Test Project CR", user_id=admin_user.id
)


@pytest.fixture()
def feature_state(feature: Feature, environment: Environment) -> FeatureState:
return FeatureState.objects.get(environment=environment, feature=feature)
Expand Down
6 changes: 5 additions & 1 deletion api/environments/managers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from features.models import FeatureSegment, FeatureState
from features.multivariate.models import MultivariateFeatureStateValue
from segments.models import Segment


class EnvironmentManager(SoftDeleteManager):
Expand All @@ -21,7 +22,10 @@ def filter_for_document_builder(
*extra_select_related or (),
)
.prefetch_related(
"project__segments",
Prefetch(
"project__segments",
queryset=Segment.live_objects.all(),
),
"project__segments__rules",
"project__segments__rules__rules",
"project__segments__rules__conditions",
Expand Down
2 changes: 1 addition & 1 deletion api/environments/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ def get_segments_from_cache(self) -> typing.List[Segment]:
segments = environment_segments_cache.get(self.id)
if not segments:
segments = list(
Segment.objects.filter(
Segment.live_objects.filter(
feature_segments__feature_states__environment=self
).prefetch_related(
"rules",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# Generated by Django 4.2.15 on 2024-09-17 15:34

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


def set_project_for_existing_change_requests(apps, schema_model):
ChangeRequest = apps.get_model("workflows_core", "ChangeRequest")

for change_request in ChangeRequest.objects.filter(
environment_id__isnull=False
).select_related("environment"):
change_request.project = change_request.environment.project
change_request.save()


class Migration(migrations.Migration):

dependencies = [
("environments", "0035_add_use_identity_overrides_in_local_eval"),
("projects", "0025_add_change_request_project_permissions"),
("workflows_core", "0010_add_ignore_conflicts_option"),
]

operations = [
migrations.AddField(
model_name="changerequest",
name="project",
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.SET_NULL,
related_name="change_requests",
to="projects.project",
),
),
migrations.AddField(
model_name="historicalchangerequest",
name="project",
field=models.ForeignKey(
blank=True,
db_constraint=False,
null=True,
on_delete=django.db.models.deletion.DO_NOTHING,
related_name="+",
to="projects.project",
),
),
migrations.AlterField(
model_name="changerequest",
name="environment",
field=models.ForeignKey(
null=True,
on_delete=django.db.models.deletion.CASCADE,
related_name="change_requests",
to="environments.environment",
),
),
migrations.RunPython(
set_project_for_existing_change_requests,
reverse_code=migrations.RunPython.noop,
),
migrations.AlterField(
model_name="changerequest",
name="project",
field=models.ForeignKey(
null=False,
on_delete=django.db.models.deletion.CASCADE,
related_name="change_requests",
to="projects.project",
),
),
]
64 changes: 60 additions & 4 deletions api/features/workflows/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
AFTER_CREATE,
AFTER_SAVE,
AFTER_UPDATE,
BEFORE_CREATE,
BEFORE_DELETE,
LifecycleModel,
LifecycleModelMixin,
Expand Down Expand Up @@ -77,10 +78,19 @@ class ChangeRequest(
null=True,
)

# Change requests get deleted in a delegated task when a project is deleted.
project = models.ForeignKey(
"projects.Project",
on_delete=models.CASCADE,
related_name="change_requests",
null=False,
)

environment = models.ForeignKey(
"environments.Environment",
on_delete=models.CASCADE,
related_name="change_requests",
null=True,
)

committed_at = models.DateTimeField(null=True)
Expand Down Expand Up @@ -114,6 +124,7 @@ def commit(self, committed_by: "FFAdminUser"):
self._publish_feature_states()
self._publish_environment_feature_versions(committed_by)
self._publish_change_sets(committed_by)
self._publish_segments()

self.committed_at = timezone.now()
self.committed_by = committed_by
Expand Down Expand Up @@ -181,6 +192,30 @@ def _publish_change_sets(self, published_by: "FFAdminUser") -> None:
for change_set in self.change_sets.all():
change_set.publish(user=published_by)

def _publish_segments(self) -> None:
for segment in self.segments.all():
target_segment = segment.version_of
assert target_segment != segment

# Deep clone the segment to establish historical version this is required
# because the target segment will be altered when the segment is published.
# Think of it like a regular update to a segment where we create the clone
# to create the version, then modifying the new 'draft' version with the
# data from the change request.
target_segment.deep_clone()
zachaysan marked this conversation as resolved.
Show resolved Hide resolved

# Set the properties of the change request's segment to the properties
# of the target (i.e., canonical) segment.
target_segment.name = segment.name
target_segment.description = segment.description
target_segment.feature = segment.feature
target_segment.save()

# Delete the rules in order to replace them with copies of the segment.
target_segment.rules.all().delete()
for rule in segment.rules.all():
rule.deep_clone(target_segment)

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

Expand Down Expand Up @@ -208,10 +243,21 @@ def get_audit_log_author(self, history_instance) -> typing.Optional["FFAdminUser
def _get_environment(self) -> typing.Optional["Environment"]:
return self.environment

def _get_project(self) -> typing.Optional["Project"]:
return self.environment.project
def _get_project(self) -> "Project":
return self.project

def is_approved(self):
if self.environment:
return self.is_approved_via_environment()
return self.is_approved_via_project()

def is_approved_via_project(self):
return self.project.minimum_change_request_approvals is None or (
self.approvals.filter(approved_at__isnull=False).count()
>= self.project.minimum_change_request_approvals
)

def is_approved_via_environment(self):
return self.environment.minimum_change_request_approvals is None or (
self.approvals.filter(approved_at__isnull=False).count()
>= self.environment.minimum_change_request_approvals
Expand All @@ -228,15 +274,22 @@ def url(self):
"Change request must be saved before it has a url attribute."
)
url = get_current_site_url()
url += f"/project/{self.environment.project_id}"
url += f"/environment/{self.environment.api_key}"
if self.environment:
url += f"/project/{self.environment.project_id}"
url += f"/environment/{self.environment.api_key}"
else:
url += f"/projects/{self.project_id}"
url += f"/change-requests/{self.id}"
return url

@property
def email_subject(self):
return f"Flagsmith Change Request: {self.title} (#{self.id})"

@hook(BEFORE_CREATE, when="project", is_now=None)
def set_project_from_environment(self):
self.project_id = self.environment.project_id

@hook(AFTER_CREATE, when="committed_at", is_not=None)
@hook(AFTER_SAVE, when="committed_at", was=None, is_not=None)
def create_audit_log_for_related_feature_state(self):
Expand Down Expand Up @@ -368,6 +421,9 @@ def get_audit_log_author(self, history_instance) -> "FFAdminUser":
def _get_environment(self):
return self.change_request.environment

def _get_project(self):
return self.change_request._get_project()


class ChangeRequestGroupAssignment(AbstractBaseExportableModel, LifecycleModel):
change_request = models.ForeignKey(
Expand Down
Loading
Loading