Skip to content

Commit

Permalink
feat: add fields necessary for stale flags (#3263)
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewelwell committed Feb 14, 2024
1 parent 932c62d commit aa1d6bb
Show file tree
Hide file tree
Showing 9 changed files with 269 additions and 6 deletions.
25 changes: 25 additions & 0 deletions api/features/serializers.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import typing
from datetime import datetime

import django.core.exceptions
from drf_writable_nested import WritableNestedModelSerializer
Expand Down Expand Up @@ -121,6 +122,18 @@ class ListCreateFeatureSerializer(DeleteBeforeUpdateWritableNestedModelSerialize
"Note: will return null for Edge enabled projects."
)

last_modified_in_any_environment = serializers.SerializerMethodField(
help_text="Datetime representing the last time that the feature was modified "
"in any environment in the given project. Note: requires feature "
"versioning v2 enabled on the environment."
)
last_modified_in_current_environment = serializers.SerializerMethodField(
help_text="Datetime representing the last time that the feature was modified "
"in any environment in the current environment. Note: requires that "
"the environment query parameter is passed and feature versioning v2 "
"enabled on the environment."
)

class Meta:
model = Feature
fields = (
Expand All @@ -141,6 +154,8 @@ class Meta:
"num_segment_overrides",
"num_identity_overrides",
"is_server_key_only",
"last_modified_in_any_environment",
"last_modified_in_current_environment",
)
read_only_fields = ("feature_segments", "created_date", "uuid", "project")

Expand Down Expand Up @@ -244,6 +259,16 @@ def get_num_identity_overrides(self, instance) -> typing.Optional[int]:
except (KeyError, AttributeError):
return None

def get_last_modified_in_any_environment(
self, instance: Feature
) -> datetime | None:
return getattr(instance, "last_modified_in_any_environment", None)

def get_last_modified_in_current_environment(
self, instance: Feature
) -> datetime | None:
return getattr(instance, "last_modified_in_current_environment", None)


class UpdateFeatureSerializer(ListCreateFeatureSerializer):
"""prevent users from changing certain values after creation"""
Expand Down
27 changes: 24 additions & 3 deletions api/features/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from core.request_origin import RequestOrigin
from django.conf import settings
from django.core.cache import caches
from django.db.models import Q, QuerySet
from django.db.models import Max, Q, QuerySet
from django.utils.decorators import method_decorator
from django.views.decorators.cache import cache_page
from drf_yasg import openapi
Expand Down Expand Up @@ -117,8 +117,18 @@ def get_queryset(self):
accessible_projects = self.request.user.get_permitted_projects(VIEW_PROJECT)

project = get_object_or_404(accessible_projects, pk=self.kwargs["project_pk"])
queryset = project.features.all().prefetch_related(
"multivariate_options", "owners", "tags", "group_owners"

queryset = (
project.features.all()
.annotate(
last_modified_in_any_environment=Max(
"feature_states__environment_feature_version__created_at",
filter=Q(
feature_states__environment_feature_version__published_at__isnull=False
),
),
)
.prefetch_related("multivariate_options", "owners", "tags", "group_owners")
)

query_serializer = FeatureQuerySerializer(data=self.request.query_params)
Expand All @@ -127,6 +137,17 @@ def get_queryset(self):

queryset = self._filter_queryset(queryset)

if environment_id := query_data.get("environment"):
queryset = queryset.annotate(
last_modified_in_current_environment=Max(
"feature_states__environment_feature_version__created_at",
filter=Q(
feature_states__environment=environment_id,
feature_states__environment_feature_version__published_at__isnull=False,
),
)
)

sort = "%s%s" % (
"-" if query_data["sort_direction"] == "DESC" else "",
query_data["sort_field"],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Generated by Django 3.2.23 on 2023-12-06 14:35
from django.db import migrations, models


class Migration(migrations.Migration):
dependencies = [
("projects", "0021_add_identity_overrides_migration_status"),
]

operations = [
migrations.AddField(
model_name="project",
name="stale_flags_limit_days",
field=models.IntegerField(
default=30,
help_text="Number of days without modification in any environment before a flag is considered stale.",
),
),
]
4 changes: 4 additions & 0 deletions api/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ class Project(LifecycleModelMixin, SoftDeleteExportableModel):
choices=IdentityOverridesV2MigrationStatus.choices,
default=IdentityOverridesV2MigrationStatus.NOT_STARTED,
)
stale_flags_limit_days = models.IntegerField(
default=30,
help_text="Number of days without modification in any environment before a flag is considered stale.",
)

objects = ProjectManager()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Generated by Django 3.2.23 on 2023-12-06 15:11
import re

from django.apps.registry import Apps
from django.db import migrations, models
from django.db.backends.base.schema import BaseDatabaseSchemaEditor

from projects.tags.models import TagType

PROTECTED_LABELS = {"protected", "donotdelete", "permanent"}
LABEL_REGEX = re.compile(r"[ _]")


def get_sanitised_label(label: str) -> str:
return LABEL_REGEX.sub("", label.lower())


def mark_existing_tags_as_permanent(
apps: Apps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
"""
Update all tags in the database which the FE treats as 'permanent' through regex matching
to have the new is_permanent label. The FE will subsequently be updated to use the
is_permanent attribute instead of regex matching.
"""
tag_class = apps.get_model("tags", "tag")

to_update = []

for tag in filter(
lambda t: get_sanitised_label(t.label) in PROTECTED_LABELS,
tag_class.objects.all(),
):
tag.is_permanent = True
to_update.append(tag)

tag_class.objects.bulk_update(to_update, fields=["is_permanent"])


def populate_default_tag_type(
apps: Apps, schema_editor: BaseDatabaseSchemaEditor
) -> None:
apps.get_model("tags", "tag").objects.filter(type__isnull=True).update(
type=TagType.NONE.value
)


class Migration(migrations.Migration):
dependencies = [
("tags", "0004_add_uuid_field"),
]

operations = [
migrations.AddField(
model_name="tag",
name="is_permanent",
field=models.BooleanField(
default=False,
help_text="When applied to a feature, it means this feature should be excluded from stale flags logic.",
),
),
migrations.AddField(
model_name="tag",
name="is_system_tag",
field=models.BooleanField(
default=False,
help_text="Indicates that a tag was created by the system, not the user.",
),
),
migrations.RunPython(
mark_existing_tags_as_permanent, reverse_code=migrations.RunPython.noop
),
migrations.AddField(
model_name="tag",
name="type",
field=models.CharField(
choices=[("NONE", "None"), ("STALE", "Stale")],
null=True,
help_text="Field used to provide a consistent identifier for the FE and API to use for business logic.",
max_length=100,
),
),
migrations.RunPython(
populate_default_tag_type, reverse_code=migrations.RunPython.noop
),
migrations.AlterField(
model_name="tag",
name="type",
field=models.CharField(
choices=[("NONE", "None"), ("STALE", "Stale")],
default="NONE",
help_text="Field used to provide a consistent identifier for the FE and API to use for business logic.",
max_length=100,
),
),
]
20 changes: 20 additions & 0 deletions api/projects/tags/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
from projects.models import Project


class TagType(models.Choices):
NONE = "NONE"
STALE = "STALE"


class Tag(AbstractBaseExportableModel):
label = models.CharField(max_length=100)
color = models.CharField(
Expand All @@ -12,6 +17,21 @@ class Tag(AbstractBaseExportableModel):
description = models.CharField(max_length=512, blank=True, null=True)
project = models.ForeignKey(Project, on_delete=models.CASCADE, related_name="tags")

is_system_tag = models.BooleanField(
default=False,
help_text="Indicates that a tag was created by the system, not the user.",
)
is_permanent = models.BooleanField(
default=False,
help_text="When applied to a feature, it means this feature should be excluded from stale flags logic.",
)
type = models.CharField(
default=TagType.NONE.value,
choices=TagType.choices,
help_text="Field used to provide a consistent identifier for the FE and API to use for business logic.",
max_length=100,
)

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

Expand Down
14 changes: 12 additions & 2 deletions api/projects/tags/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,15 @@
class TagSerializer(serializers.ModelSerializer):
class Meta:
model = Tag
fields = ("id", "label", "color", "description", "project", "uuid")
read_only_fields = ("project",)
fields = (
"id",
"label",
"color",
"description",
"project",
"uuid",
"is_permanent",
"is_system_tag",
"type",
)
read_only_fields = ("project", "uuid", "is_system_tag", "type")
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,4 @@ def test_update_feature_state_value_updates_feature_state_value(

# Then
assert response.status_code == status.HTTP_200_OK
response.json()["feature_state_value"] == new_value
assert response.json()["feature_state_value"] == new_value
68 changes: 68 additions & 0 deletions api/tests/unit/features/test_unit_features_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from django.forms import model_to_dict
from django.urls import reverse
from django.utils import timezone
from freezegun import freeze_time
from pytest_django import DjangoAssertNumQueries
from pytest_lazyfixture import lazy_fixture
from rest_framework import status
Expand Down Expand Up @@ -55,6 +56,10 @@
# patch this function as it's triggering extra threads and causing errors
mock.patch("features.signals.trigger_feature_state_change_webhooks").start()

now = timezone.now()
two_hours_ago = now - timedelta(hours=2)
one_hour_ago = now - timedelta(hours=1)


@pytest.mark.django_db
class ProjectFeatureTestCase(TestCase):
Expand Down Expand Up @@ -2554,3 +2559,66 @@ def test_simple_feature_state_returns_only_latest_versions(

response_json = response.json()
assert response_json["count"] == 2


@pytest.mark.freeze_time(two_hours_ago)
def test_feature_list_last_modified_values(
staff_client: APIClient,
staff_user: FFAdminUser,
environment_v2_versioning: Environment,
project: Project,
feature: Feature,
with_project_permissions: WithProjectPermissionsCallable,
django_assert_num_queries: DjangoAssertNumQueries,
) -> None:
# Given
# another v2 versioning environment
environment_v2_versioning_2 = Environment.objects.create(
name="environment 2", project=project, use_v2_feature_versioning=True
)

url = "{base_url}?environment={environment_id}".format(
base_url=reverse("api-v1:projects:project-features-list", args=[project.id]),
environment_id=environment_v2_versioning.id,
)

with_project_permissions([VIEW_PROJECT])

with freeze_time(one_hour_ago):
# create a new published version in another environment, simulated to be one hour ago
environment_v2_versioning_2_version_2 = (
EnvironmentFeatureVersion.objects.create(
environment=environment_v2_versioning_2, feature=feature
)
)
environment_v2_versioning_2_version_2.publish(staff_user)

with freeze_time(now):
# and create a new unpublished version in the current environment, simulated to be now
# this shouldn't affect the values returned
EnvironmentFeatureVersion.objects.create(
environment=environment_v2_versioning, feature=feature
)

# let's add a few more features to ensure we aren't adding N+1 issues
for i in range(2):
Feature.objects.create(name=f"feature_{i}", project=project)

# When
with django_assert_num_queries(14): # TODO: reduce this number of queries!
response = staff_client.get(url)

# Then
assert response.status_code == status.HTTP_200_OK

response_json = response.json()
assert response_json["count"] == 3

feature_data = next(
filter(lambda r: r["id"] == feature.id, response_json["results"])
)
assert feature_data["last_modified_in_any_environment"] == one_hour_ago.isoformat()
assert (
feature_data["last_modified_in_current_environment"]
== two_hours_ago.isoformat()
)

0 comments on commit aa1d6bb

Please sign in to comment.