diff --git a/api/app/urls.py b/api/app/urls.py index 65caa58df8ba..e7338cd5988d 100644 --- a/api/app/urls.py +++ b/api/app/urls.py @@ -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( [ diff --git a/api/audit/permissions.py b/api/audit/permissions.py index 6695ac696fec..b8b0ae3b48e7 100644 --- a/api/audit/permissions.py +++ b/api/audit/permissions.py @@ -1,10 +1,10 @@ +from common.projects.permissions import VIEW_AUDIT_LOG from django.views import View from rest_framework.permissions import BasePermission from rest_framework.request import Request from organisations.models import Organisation from projects.models import Project -from projects.permissions import VIEW_AUDIT_LOG class OrganisationAuditLogPermissions(BasePermission): diff --git a/api/conftest.py b/api/conftest.py index d5249fad1554..6f3600650e4c 100644 --- a/api/conftest.py +++ b/api/conftest.py @@ -5,6 +5,12 @@ import boto3 import pytest +from common.environments.permissions import ( + MANAGE_IDENTITIES, + VIEW_ENVIRONMENT, + VIEW_IDENTITIES, +) +from common.projects.permissions import VIEW_PROJECT from django.contrib.contenttypes.models import ContentType from django.core.cache import caches from django.db.backends.base.creation import TEST_DATABASE_PREFIX @@ -27,11 +33,6 @@ from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment, EnvironmentAPIKey -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_ENVIRONMENT, - VIEW_IDENTITIES, -) from environments.permissions.models import ( UserEnvironmentPermission, UserPermissionGroupEnvironmentPermission, @@ -72,7 +73,6 @@ UserPermissionGroupProjectPermission, UserProjectPermission, ) -from projects.permissions import VIEW_PROJECT from projects.tags.models import Tag from segments.models import Condition, Segment, SegmentRule from tests.test_helpers import fix_issue_3869 @@ -547,12 +547,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) diff --git a/api/e2etests/e2e_seed_data.py b/api/e2etests/e2e_seed_data.py index eba97bd65cb8..37b7605e2a17 100644 --- a/api/e2etests/e2e_seed_data.py +++ b/api/e2etests/e2e_seed_data.py @@ -1,205 +1,205 @@ -from django.conf import settings -from flag_engine.identities.models import IdentityModel as EngineIdentity - -from edge_api.identities.models import EdgeIdentity -from environments.identities.models import Identity -from environments.models import Environment -from environments.permissions.constants import ( - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, - VIEW_IDENTITIES, -) -from environments.permissions.models import UserEnvironmentPermission -from organisations.models import Organisation, OrganisationRole, Subscription -from organisations.permissions.models import UserOrganisationPermission -from organisations.permissions.permissions import ( - CREATE_PROJECT, - MANAGE_USER_GROUPS, -) -from organisations.subscriptions.constants import SCALE_UP -from projects.models import Project, UserProjectPermission -from projects.permissions import ( - CREATE_ENVIRONMENT, - CREATE_FEATURE, - VIEW_AUDIT_LOG, - VIEW_PROJECT, -) -from users.models import FFAdminUser, UserPermissionGroup - -# Password used by all the test users -PASSWORD = "Str0ngp4ssw0rd!" - -PROJECT_PERMISSION_PROJECT = "My Test Project 5 Project Permission" -ENV_PERMISSION_PROJECT = "My Test Project 6 Env Permission" - - -def delete_user_and_its_organisations(user_email: str) -> None: - user: FFAdminUser | None = FFAdminUser.objects.filter(email=user_email).first() - - if user: - user.organisations.all().delete() - user.delete() - - -def teardown() -> None: - # delete users and their orgs created for e2e test by front end - delete_user_and_its_organisations(user_email=settings.E2E_SIGNUP_USER) - delete_user_and_its_organisations(user_email=settings.E2E_USER) - delete_user_and_its_organisations(user_email=settings.E2E_CHANGE_EMAIL_USER) - delete_user_and_its_organisations( - user_email=settings.E2E_NON_ADMIN_USER_WITH_ORG_PERMISSIONS - ) - delete_user_and_its_organisations( - user_email=settings.E2E_NON_ADMIN_USER_WITH_PROJECT_PERMISSIONS - ) - delete_user_and_its_organisations( - user_email=settings.E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS - ) - delete_user_and_its_organisations( - user_email=settings.E2E_NON_ADMIN_USER_WITH_A_ROLE - ) - - -def seed_data() -> None: - # create user and organisation for e2e test by front end - organisation: Organisation = Organisation.objects.create(name="Bullet Train Ltd") - org_admin: FFAdminUser = FFAdminUser.objects.create_user( - email=settings.E2E_USER, - password=PASSWORD, - username=settings.E2E_USER, - ) - org_admin.add_organisation(organisation, OrganisationRole.ADMIN) - non_admin_user_with_org_permissions: FFAdminUser = FFAdminUser.objects.create_user( - email=settings.E2E_NON_ADMIN_USER_WITH_ORG_PERMISSIONS, - password=PASSWORD, - ) - non_admin_user_with_project_permissions: FFAdminUser = ( - FFAdminUser.objects.create_user( - email=settings.E2E_NON_ADMIN_USER_WITH_PROJECT_PERMISSIONS, - password=PASSWORD, - ) - ) - non_admin_user_with_env_permissions: FFAdminUser = FFAdminUser.objects.create_user( - email=settings.E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS, - password=PASSWORD, - ) - non_admin_user_with_a_role: FFAdminUser = FFAdminUser.objects.create_user( - email=settings.E2E_NON_ADMIN_USER_WITH_A_ROLE, - password=PASSWORD, - ) - non_admin_user_with_org_permissions.add_organisation( - organisation, - ) - non_admin_user_with_project_permissions.add_organisation( - organisation, - ) - non_admin_user_with_env_permissions.add_organisation( - organisation, - ) - non_admin_user_with_a_role.add_organisation( - organisation, - ) - - # Add permissions to the non-admin user with org permissions - user_org_permission = UserOrganisationPermission.objects.create( - user=non_admin_user_with_org_permissions, organisation=organisation - ) - user_org_permission.add_permission(CREATE_PROJECT) - user_org_permission.add_permission(MANAGE_USER_GROUPS) - UserPermissionGroup.objects.create(name="TestGroup", organisation=organisation) - - # We add different projects and environments to give each e2e test its own isolated context. - project_test_data = [ - { - "name": "My Test Project", - "environments": [ - "Development", - "Production", - ], - }, - {"name": "My Test Project 2", "environments": ["Development"]}, - {"name": "My Test Project 3", "environments": ["Development"]}, - {"name": "My Test Project 4", "environments": ["Development"]}, - { - "name": PROJECT_PERMISSION_PROJECT, - "environments": ["Development"], - }, - {"name": ENV_PERMISSION_PROJECT, "environments": ["Development"]}, - {"name": "My Test Project 7 Role", "environments": ["Development"]}, - ] - # Upgrade organisation seats - Subscription.objects.filter(organisation__in=org_admin.organisations.all()).update( - max_seats=8, plan=SCALE_UP, subscription_id="test_subscription_id" - ) - - # Create projects and environments - projects = [] - environments = [] - for project_info in project_test_data: - project = Project.objects.create( - name=project_info["name"], organisation=organisation - ) - if project_info["name"] == PROJECT_PERMISSION_PROJECT: - # Add permissions to the non-admin user with project permissions - user_proj_permission: UserProjectPermission = ( - UserProjectPermission.objects.create( - user=non_admin_user_with_project_permissions, project=project - ) - ) - [ - user_proj_permission.add_permission(permission_key) - for permission_key in [ - VIEW_PROJECT, - CREATE_ENVIRONMENT, - CREATE_FEATURE, - VIEW_AUDIT_LOG, - ] - ] - projects.append(project) - - for env_name in project_info["environments"]: - environment = Environment.objects.create(name=env_name, project=project) - - if project_info["name"] == ENV_PERMISSION_PROJECT: - # Add permissions to the non-admin user with env permissions - user_env_permission = UserEnvironmentPermission.objects.create( - user=non_admin_user_with_env_permissions, environment=environment - ) - user_env_proj_permission: UserProjectPermission = ( - UserProjectPermission.objects.create( - user=non_admin_user_with_env_permissions, project=project - ) - ) - user_env_proj_permission.add_permission(VIEW_PROJECT) - user_env_proj_permission.add_permission(CREATE_FEATURE) - [ - user_env_permission.add_permission(permission_key) - for permission_key in [ - VIEW_ENVIRONMENT, - UPDATE_FEATURE_STATE, - VIEW_IDENTITIES, - ] - ] - environments.append(environment) - - # We're only creating identities for 6 of the 7 environments because - # they are necessary for the environments created above and to keep - # the e2e tests isolated." - identities_test_data = [ - {"identifier": settings.E2E_IDENTITY, "environment": environments[2]}, - {"identifier": settings.E2E_IDENTITY, "environment": environments[3]}, - {"identifier": settings.E2E_IDENTITY, "environment": environments[4]}, - {"identifier": settings.E2E_IDENTITY, "environment": environments[5]}, - {"identifier": settings.E2E_IDENTITY, "environment": environments[6]}, - {"identifier": settings.E2E_IDENTITY, "environment": environments[7]}, - ] - - for identity_info in identities_test_data: - if settings.IDENTITIES_TABLE_NAME_DYNAMO: - engine_identity = EngineIdentity( - identifier=identity_info["identifier"], - environment_api_key=identity_info["environment"].api_key, - ) - EdgeIdentity(engine_identity).save() - else: - Identity.objects.create(**identity_info) +from common.environments.permissions import ( + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, + VIEW_IDENTITIES, +) +from common.projects.permissions import ( + CREATE_ENVIRONMENT, + CREATE_FEATURE, + VIEW_AUDIT_LOG, + VIEW_PROJECT, +) +from django.conf import settings +from flag_engine.identities.models import IdentityModel as EngineIdentity + +from edge_api.identities.models import EdgeIdentity +from environments.identities.models import Identity +from environments.models import Environment +from environments.permissions.models import UserEnvironmentPermission +from organisations.models import Organisation, OrganisationRole, Subscription +from organisations.permissions.models import UserOrganisationPermission +from organisations.permissions.permissions import ( + CREATE_PROJECT, + MANAGE_USER_GROUPS, +) +from organisations.subscriptions.constants import SCALE_UP +from projects.models import Project, UserProjectPermission +from users.models import FFAdminUser, UserPermissionGroup + +# Password used by all the test users +PASSWORD = "Str0ngp4ssw0rd!" + +PROJECT_PERMISSION_PROJECT = "My Test Project 5 Project Permission" +ENV_PERMISSION_PROJECT = "My Test Project 6 Env Permission" + + +def delete_user_and_its_organisations(user_email: str) -> None: + user: FFAdminUser | None = FFAdminUser.objects.filter(email=user_email).first() + + if user: + user.organisations.all().delete() + user.delete() + + +def teardown() -> None: + # delete users and their orgs created for e2e test by front end + delete_user_and_its_organisations(user_email=settings.E2E_SIGNUP_USER) + delete_user_and_its_organisations(user_email=settings.E2E_USER) + delete_user_and_its_organisations(user_email=settings.E2E_CHANGE_EMAIL_USER) + delete_user_and_its_organisations( + user_email=settings.E2E_NON_ADMIN_USER_WITH_ORG_PERMISSIONS + ) + delete_user_and_its_organisations( + user_email=settings.E2E_NON_ADMIN_USER_WITH_PROJECT_PERMISSIONS + ) + delete_user_and_its_organisations( + user_email=settings.E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS + ) + delete_user_and_its_organisations( + user_email=settings.E2E_NON_ADMIN_USER_WITH_A_ROLE + ) + + +def seed_data() -> None: + # create user and organisation for e2e test by front end + organisation: Organisation = Organisation.objects.create(name="Bullet Train Ltd") + org_admin: FFAdminUser = FFAdminUser.objects.create_user( + email=settings.E2E_USER, + password=PASSWORD, + username=settings.E2E_USER, + ) + org_admin.add_organisation(organisation, OrganisationRole.ADMIN) + non_admin_user_with_org_permissions: FFAdminUser = FFAdminUser.objects.create_user( + email=settings.E2E_NON_ADMIN_USER_WITH_ORG_PERMISSIONS, + password=PASSWORD, + ) + non_admin_user_with_project_permissions: FFAdminUser = ( + FFAdminUser.objects.create_user( + email=settings.E2E_NON_ADMIN_USER_WITH_PROJECT_PERMISSIONS, + password=PASSWORD, + ) + ) + non_admin_user_with_env_permissions: FFAdminUser = FFAdminUser.objects.create_user( + email=settings.E2E_NON_ADMIN_USER_WITH_ENV_PERMISSIONS, + password=PASSWORD, + ) + non_admin_user_with_a_role: FFAdminUser = FFAdminUser.objects.create_user( + email=settings.E2E_NON_ADMIN_USER_WITH_A_ROLE, + password=PASSWORD, + ) + non_admin_user_with_org_permissions.add_organisation( + organisation, + ) + non_admin_user_with_project_permissions.add_organisation( + organisation, + ) + non_admin_user_with_env_permissions.add_organisation( + organisation, + ) + non_admin_user_with_a_role.add_organisation( + organisation, + ) + + # Add permissions to the non-admin user with org permissions + user_org_permission = UserOrganisationPermission.objects.create( + user=non_admin_user_with_org_permissions, organisation=organisation + ) + user_org_permission.add_permission(CREATE_PROJECT) + user_org_permission.add_permission(MANAGE_USER_GROUPS) + UserPermissionGroup.objects.create(name="TestGroup", organisation=organisation) + + # We add different projects and environments to give each e2e test its own isolated context. + project_test_data = [ + { + "name": "My Test Project", + "environments": [ + "Development", + "Production", + ], + }, + {"name": "My Test Project 2", "environments": ["Development"]}, + {"name": "My Test Project 3", "environments": ["Development"]}, + {"name": "My Test Project 4", "environments": ["Development"]}, + { + "name": PROJECT_PERMISSION_PROJECT, + "environments": ["Development"], + }, + {"name": ENV_PERMISSION_PROJECT, "environments": ["Development"]}, + {"name": "My Test Project 7 Role", "environments": ["Development"]}, + ] + # Upgrade organisation seats + Subscription.objects.filter(organisation__in=org_admin.organisations.all()).update( + max_seats=8, plan=SCALE_UP, subscription_id="test_subscription_id" + ) + + # Create projects and environments + projects = [] + environments = [] + for project_info in project_test_data: + project = Project.objects.create( + name=project_info["name"], organisation=organisation + ) + if project_info["name"] == PROJECT_PERMISSION_PROJECT: + # Add permissions to the non-admin user with project permissions + user_proj_permission: UserProjectPermission = ( + UserProjectPermission.objects.create( + user=non_admin_user_with_project_permissions, project=project + ) + ) + [ + user_proj_permission.add_permission(permission_key) + for permission_key in [ + VIEW_PROJECT, + CREATE_ENVIRONMENT, + CREATE_FEATURE, + VIEW_AUDIT_LOG, + ] + ] + projects.append(project) + + for env_name in project_info["environments"]: + environment = Environment.objects.create(name=env_name, project=project) + + if project_info["name"] == ENV_PERMISSION_PROJECT: + # Add permissions to the non-admin user with env permissions + user_env_permission = UserEnvironmentPermission.objects.create( + user=non_admin_user_with_env_permissions, environment=environment + ) + user_env_proj_permission: UserProjectPermission = ( + UserProjectPermission.objects.create( + user=non_admin_user_with_env_permissions, project=project + ) + ) + user_env_proj_permission.add_permission(VIEW_PROJECT) + user_env_proj_permission.add_permission(CREATE_FEATURE) + [ + user_env_permission.add_permission(permission_key) + for permission_key in [ + VIEW_ENVIRONMENT, + UPDATE_FEATURE_STATE, + VIEW_IDENTITIES, + ] + ] + environments.append(environment) + + # We're only creating identities for 6 of the 7 environments because + # they are necessary for the environments created above and to keep + # the e2e tests isolated." + identities_test_data = [ + {"identifier": settings.E2E_IDENTITY, "environment": environments[2]}, + {"identifier": settings.E2E_IDENTITY, "environment": environments[3]}, + {"identifier": settings.E2E_IDENTITY, "environment": environments[4]}, + {"identifier": settings.E2E_IDENTITY, "environment": environments[5]}, + {"identifier": settings.E2E_IDENTITY, "environment": environments[6]}, + {"identifier": settings.E2E_IDENTITY, "environment": environments[7]}, + ] + + for identity_info in identities_test_data: + if settings.IDENTITIES_TABLE_NAME_DYNAMO: + engine_identity = EngineIdentity( # pragma: no cover + identifier=identity_info["identifier"], + environment_api_key=identity_info["environment"].api_key, + ) + EdgeIdentity(engine_identity).save() # pragma: no cover + else: + Identity.objects.create(**identity_info) diff --git a/api/edge_api/identities/permissions.py b/api/edge_api/identities/permissions.py index fd67d235589c..580bdd81e14c 100644 --- a/api/edge_api/identities/permissions.py +++ b/api/edge_api/identities/permissions.py @@ -1,14 +1,14 @@ from contextlib import suppress +from common.environments.permissions import ( + UPDATE_FEATURE_STATE, + VIEW_IDENTITIES, +) from django.http import HttpRequest from django.views import View from rest_framework.permissions import BasePermission from environments.models import Environment -from environments.permissions.constants import ( - UPDATE_FEATURE_STATE, - VIEW_IDENTITIES, -) class EdgeIdentityWithIdentifierViewPermissions(BasePermission): diff --git a/api/edge_api/identities/views.py b/api/edge_api/identities/views.py index ac2994277e89..d49ae3ec4250 100644 --- a/api/edge_api/identities/views.py +++ b/api/edge_api/identities/views.py @@ -3,6 +3,7 @@ import typing import pydantic +from common.environments.permissions import MANAGE_IDENTITIES, VIEW_IDENTITIES from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema @@ -51,10 +52,6 @@ IdentityAllFeatureStatesSerializer, ) from environments.models import Environment -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_IDENTITIES, -) from environments.permissions.permissions import NestedEnvironmentPermissions from features.models import FeatureState from features.permissions import IdentityFeatureStatePermissions diff --git a/api/environments/identities/traits/views.py b/api/environments/identities/traits/views.py index 201512643e65..4b434ce539fd 100644 --- a/api/environments/identities/traits/views.py +++ b/api/environments/identities/traits/views.py @@ -1,3 +1,4 @@ +from common.environments.permissions import MANAGE_IDENTITIES, VIEW_IDENTITIES from django.conf import settings from django.core.exceptions import BadRequest from django.db.models import Q @@ -21,10 +22,6 @@ TraitSerializerBasic, TraitSerializerFull, ) -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_IDENTITIES, -) from environments.permissions.permissions import ( EnvironmentKeyPermissions, NestedEnvironmentPermissions, diff --git a/api/environments/identities/views.py b/api/environments/identities/views.py index a86d240f73d1..49f0f365ebac 100644 --- a/api/environments/identities/views.py +++ b/api/environments/identities/views.py @@ -1,6 +1,7 @@ import typing from collections import namedtuple +from common.environments.permissions import MANAGE_IDENTITIES, VIEW_IDENTITIES from core.constants import FLAGSMITH_UPDATED_AT_HEADER from core.request_origin import RequestOrigin from django.conf import settings @@ -22,10 +23,6 @@ SDKIdentitiesResponseSerializer, ) from environments.models import Environment -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_IDENTITIES, -) from environments.permissions.permissions import NestedEnvironmentPermissions from environments.sdk.serializers import ( IdentifyWithTraitsSerializer, diff --git a/api/environments/managers.py b/api/environments/managers.py index fe0e0a4b51df..c170bc44ea28 100644 --- a/api/environments/managers.py +++ b/api/environments/managers.py @@ -3,6 +3,7 @@ from features.models import FeatureSegment, FeatureState from features.multivariate.models import MultivariateFeatureStateValue +from segments.models import Segment class EnvironmentManager(SoftDeleteManager): @@ -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", diff --git a/api/environments/migrations/0010_auto_20200219_2343.py b/api/environments/migrations/0010_auto_20200219_2343.py index 119513c81c84..48c221f0e0b7 100644 --- a/api/environments/migrations/0010_auto_20200219_2343.py +++ b/api/environments/migrations/0010_auto_20200219_2343.py @@ -1,7 +1,14 @@ # Generated by Django 2.2.10 on 2020-02-19 23:43 +from common.environments.permissions import ( + APPROVE_CHANGE_REQUEST, + CREATE_CHANGE_REQUEST, + MANAGE_IDENTITIES, + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) from django.db import migrations -from environments.permissions.constants import VIEW_ENVIRONMENT, UPDATE_FEATURE_STATE, MANAGE_IDENTITIES, CREATE_CHANGE_REQUEST, APPROVE_CHANGE_REQUEST + ENVIRONMENT_PERMISSIONS = [ (VIEW_ENVIRONMENT, "View permission for the given environment."), (UPDATE_FEATURE_STATE, "Update the state or value for a given feature state."), @@ -16,12 +23,15 @@ ), ] + def create_default_permissions(apps, schema_editor): - EnvironmentPermission = apps.get_model('environments', 'EnvironmentPermission') + EnvironmentPermission = apps.get_model("environments", "EnvironmentPermission") environment_permissions = [] for permission in ENVIRONMENT_PERMISSIONS: - environment_permissions.append(EnvironmentPermission(key=permission[0], description=permission[1])) + environment_permissions.append( + EnvironmentPermission(key=permission[0], description=permission[1]) + ) EnvironmentPermission.objects.bulk_create(environment_permissions) @@ -29,9 +39,11 @@ def create_default_permissions(apps, schema_editor): class Migration(migrations.Migration): dependencies = [ - ('environments', '0009_auto_20200219_1922'), + ("environments", "0009_auto_20200219_1922"), ] operations = [ - migrations.RunPython(create_default_permissions, reverse_code=lambda *args: None) + migrations.RunPython( + create_default_permissions, reverse_code=lambda *args: None + ) ] diff --git a/api/environments/models.py b/api/environments/models.py index 11bf982fd5a5..f1cf5adc5132 100644 --- a/api/environments/models.py +++ b/api/environments/models.py @@ -335,7 +335,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", diff --git a/api/environments/permissions/constants.py b/api/environments/permissions/constants.py deleted file mode 100644 index 90b4f6a192ea..000000000000 --- a/api/environments/permissions/constants.py +++ /dev/null @@ -1,10 +0,0 @@ -# Maintain a list of permissions here -VIEW_ENVIRONMENT = "VIEW_ENVIRONMENT" -UPDATE_FEATURE_STATE = "UPDATE_FEATURE_STATE" -MANAGE_IDENTITIES = "MANAGE_IDENTITIES" -VIEW_IDENTITIES = "VIEW_IDENTITIES" -CREATE_CHANGE_REQUEST = "CREATE_CHANGE_REQUEST" -APPROVE_CHANGE_REQUEST = "APPROVE_CHANGE_REQUEST" -MANAGE_SEGMENT_OVERRIDES = "MANAGE_SEGMENT_OVERRIDES" - -TAG_SUPPORTED_PERMISSIONS = [UPDATE_FEATURE_STATE] diff --git a/api/environments/permissions/migrations/0002_add_update_feature_state_permission.py b/api/environments/permissions/migrations/0002_add_update_feature_state_permission.py index 2d01699d970e..8833af04f959 100644 --- a/api/environments/permissions/migrations/0002_add_update_feature_state_permission.py +++ b/api/environments/permissions/migrations/0002_add_update_feature_state_permission.py @@ -1,8 +1,8 @@ # Generated by Django 2.2.24 on 2021-12-07 17:48 +from common.environments.permissions import UPDATE_FEATURE_STATE from django.db import migrations -from environments.permissions.constants import UPDATE_FEATURE_STATE from permissions.models import ENVIRONMENT_PERMISSION_TYPE @@ -26,7 +26,7 @@ class Migration(migrations.Migration): dependencies = [ ("environment_permissions", "0001_initial"), - ("features", "0035_auto_20211109_0603") + ("features", "0035_auto_20211109_0603"), ] operations = [ diff --git a/api/environments/permissions/migrations/0003_add_manage_identities_permission.py b/api/environments/permissions/migrations/0003_add_manage_identities_permission.py index 958d51a08e5b..27a4ac2e3626 100644 --- a/api/environments/permissions/migrations/0003_add_manage_identities_permission.py +++ b/api/environments/permissions/migrations/0003_add_manage_identities_permission.py @@ -1,8 +1,8 @@ # Generated by Django 2.2.24 on 2021-12-07 17:48 +from common.environments.permissions import MANAGE_IDENTITIES from django.db import migrations -from environments.permissions.constants import MANAGE_IDENTITIES from permissions.models import ENVIRONMENT_PERMISSION_TYPE diff --git a/api/environments/permissions/migrations/0004_add_change_request_permissions.py b/api/environments/permissions/migrations/0004_add_change_request_permissions.py index f9c637fed995..5cf1fea24f36 100644 --- a/api/environments/permissions/migrations/0004_add_change_request_permissions.py +++ b/api/environments/permissions/migrations/0004_add_change_request_permissions.py @@ -1,16 +1,15 @@ # Generated by Django 3.2.15 on 2022-09-13 19:18 import typing -from django.db import migrations - -from environments.permissions.constants import ( - CREATE_CHANGE_REQUEST, +from common.environments.permissions import ( APPROVE_CHANGE_REQUEST, + CREATE_CHANGE_REQUEST, UPDATE_FEATURE_STATE, ) -from permissions.models import ENVIRONMENT_PERMISSION_TYPE - from core.migration_helpers import create_new_environment_permissions +from django.db import migrations + +from permissions.models import ENVIRONMENT_PERMISSION_TYPE def add_change_request_permissions(apps, schema_editor): diff --git a/api/environments/permissions/migrations/0005_add_view_identity_permissions.py b/api/environments/permissions/migrations/0005_add_view_identity_permissions.py index 4c852f4bea1d..4c7ef9fbb805 100644 --- a/api/environments/permissions/migrations/0005_add_view_identity_permissions.py +++ b/api/environments/permissions/migrations/0005_add_view_identity_permissions.py @@ -1,9 +1,8 @@ # Generated by Django 3.2.16 on 2022-12-19 10:18 -from django.db import migrations - -from environments.permissions.constants import VIEW_IDENTITIES, MANAGE_IDENTITIES +from common.environments.permissions import MANAGE_IDENTITIES, VIEW_IDENTITIES from core.migration_helpers import create_new_environment_permissions +from django.db import migrations from permissions.models import ENVIRONMENT_PERMISSION_TYPE diff --git a/api/environments/permissions/migrations/0008_add_manage_segment_overrides_permission.py b/api/environments/permissions/migrations/0008_add_manage_segment_overrides_permission.py index 4ac562b6ceeb..313946651d58 100644 --- a/api/environments/permissions/migrations/0008_add_manage_segment_overrides_permission.py +++ b/api/environments/permissions/migrations/0008_add_manage_segment_overrides_permission.py @@ -1,11 +1,15 @@ # Generated by Django 3.2.20 on 2023-11-01 19:54 -from django.db import migrations -from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES, UPDATE_FEATURE_STATE +from common.environments.permissions import ( + MANAGE_SEGMENT_OVERRIDES, + UPDATE_FEATURE_STATE, +) from core.migration_helpers import create_new_environment_permissions +from django.db import migrations from permissions.models import ENVIRONMENT_PERMISSION_TYPE + def add_manage_segment_overrides_permission(apps, schema_editor): PermissionModel = apps.get_model("permissions", "PermissionModel") UserEnvironmentPermission = apps.get_model( @@ -41,10 +45,11 @@ def remove_manage_segment_overrides_permission(apps, schema_editor): PermissionModel = apps.get_model("permissions", "PermissionModel") PermissionModel.objects.filter(key=MANAGE_SEGMENT_OVERRIDES).delete() + class Migration(migrations.Migration): dependencies = [ - ('environment_permissions', '0007_add_unique_permission_constraint'), + ("environment_permissions", "0007_add_unique_permission_constraint"), ] operations = [ diff --git a/api/environments/permissions/permissions.py b/api/environments/permissions/permissions.py index 52530853f667..21cfc8a95b94 100644 --- a/api/environments/permissions/permissions.py +++ b/api/environments/permissions/permissions.py @@ -1,13 +1,13 @@ import typing +from common.environments.permissions import VIEW_ENVIRONMENT +from common.projects.permissions import CREATE_ENVIRONMENT from django.db.models import Model, Q from rest_framework import exceptions from rest_framework.permissions import BasePermission, IsAuthenticated from environments.models import Environment -from environments.permissions.constants import VIEW_ENVIRONMENT from projects.models import Project -from projects.permissions import CREATE_ENVIRONMENT class EnvironmentKeyPermissions(BasePermission): diff --git a/api/environments/serializers.py b/api/environments/serializers.py index 508eac9fbc9d..4d2836732859 100644 --- a/api/environments/serializers.py +++ b/api/environments/serializers.py @@ -1,10 +1,13 @@ import typing +from common.metadata.serializers import ( + MetadataSerializer, + SerializerWithMetadata, +) from rest_framework import serializers from environments.models import Environment, EnvironmentAPIKey, Webhook from features.serializers import FeatureStateSerializerFull -from metadata.serializers import MetadataSerializer, SerializerWithMetadata from organisations.models import Subscription from organisations.subscriptions.serializers.mixins import ( ReadOnlyIfNotValidPlanMixin, diff --git a/api/environments/views.py b/api/environments/views.py index e32e4ccab848..b60b8a1ab929 100644 --- a/api/environments/views.py +++ b/api/environments/views.py @@ -1,5 +1,6 @@ import logging +from common.environments.permissions import TAG_SUPPORTED_PERMISSIONS from django.db.models import Count, Q from django.utils.decorators import method_decorator from drf_yasg import openapi @@ -11,7 +12,6 @@ from rest_framework.request import Request from rest_framework.response import Response -from environments.permissions.constants import TAG_SUPPORTED_PERMISSIONS from environments.permissions.permissions import ( EnvironmentAdminPermission, EnvironmentPermissions, diff --git a/api/features/feature_segments/permissions.py b/api/features/feature_segments/permissions.py index 134e0320e89f..f057754e4f89 100644 --- a/api/features/feature_segments/permissions.py +++ b/api/features/feature_segments/permissions.py @@ -1,9 +1,9 @@ from contextlib import suppress +from common.environments.permissions import MANAGE_SEGMENT_OVERRIDES from rest_framework.permissions import IsAuthenticated from environments.models import Environment -from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES class FeatureSegmentPermissions(IsAuthenticated): diff --git a/api/features/feature_segments/serializers.py b/api/features/feature_segments/serializers.py index 1064d807dad1..0919328724c9 100644 --- a/api/features/feature_segments/serializers.py +++ b/api/features/feature_segments/serializers.py @@ -1,3 +1,4 @@ +from common.environments.permissions import MANAGE_SEGMENT_OVERRIDES from common.features.serializers import ( CreateSegmentOverrideFeatureSegmentSerializer, ) @@ -5,7 +6,6 @@ from rest_framework import serializers from rest_framework.exceptions import PermissionDenied -from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES from features.feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, exceeds_segment_override_limit, diff --git a/api/features/feature_segments/views.py b/api/features/feature_segments/views.py index 691c916fff16..320ad2a298ae 100644 --- a/api/features/feature_segments/views.py +++ b/api/features/feature_segments/views.py @@ -1,5 +1,6 @@ import logging +from common.projects.permissions import VIEW_PROJECT from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema from rest_framework import viewsets @@ -18,7 +19,6 @@ from features.versioning.versioning_service import ( get_current_live_environment_feature_version, ) -from projects.permissions import VIEW_PROJECT from .permissions import FeatureSegmentPermissions diff --git a/api/features/import_export/permissions.py b/api/features/import_export/permissions.py index 8325e5d1c15e..ce20e525e72a 100644 --- a/api/features/import_export/permissions.py +++ b/api/features/import_export/permissions.py @@ -1,3 +1,4 @@ +from common.projects.permissions import VIEW_PROJECT from rest_framework.generics import ListAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request @@ -5,7 +6,6 @@ from environments.models import Environment from features.import_export.models import FeatureExport from projects.models import Project -from projects.permissions import VIEW_PROJECT class FeatureImportPermissions(IsAuthenticated): diff --git a/api/features/multivariate/views.py b/api/features/multivariate/views.py index 4f115d7634a6..9bd08fc036cb 100644 --- a/api/features/multivariate/views.py +++ b/api/features/multivariate/views.py @@ -1,3 +1,4 @@ +from common.projects.permissions import CREATE_FEATURE, VIEW_PROJECT from drf_yasg.utils import swagger_auto_schema from rest_framework import viewsets from rest_framework.decorators import api_view @@ -5,11 +6,7 @@ from rest_framework.response import Response from features.models import Feature -from projects.permissions import ( - CREATE_FEATURE, - VIEW_PROJECT, - NestedProjectPermissions, -) +from projects.permissions import NestedProjectPermissions from .models import MultivariateFeatureOption from .serializers import MultivariateFeatureOptionSerializer diff --git a/api/features/permissions.py b/api/features/permissions.py index 57cd146fe153..15f471d06829 100644 --- a/api/features/permissions.py +++ b/api/features/permissions.py @@ -1,26 +1,26 @@ from contextlib import suppress +from common.environments.permissions import MANAGE_SEGMENT_OVERRIDES +from common.environments.permissions import ( + TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS, +) +from common.environments.permissions import ( + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import CREATE_FEATURE, DELETE_FEATURE +from common.projects.permissions import ( + TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_PROJECT_PERMISSIONS, +) +from common.projects.permissions import VIEW_PROJECT from django.shortcuts import get_object_or_404 from rest_framework.permissions import IsAuthenticated from rest_framework.request import Request from rest_framework.viewsets import GenericViewSet from environments.models import Environment -from environments.permissions.constants import MANAGE_SEGMENT_OVERRIDES -from environments.permissions.constants import ( - TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS, -) -from environments.permissions.constants import ( - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, -) from features.models import Feature, FeatureState from projects.models import Project -from projects.permissions import CREATE_FEATURE, DELETE_FEATURE -from projects.permissions import ( - TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_PROJECT_PERMISSIONS, -) -from projects.permissions import VIEW_PROJECT ACTION_PERMISSIONS_MAP = { "retrieve": VIEW_PROJECT, diff --git a/api/features/serializers.py b/api/features/serializers.py index a00f597cf2f9..7c954758f96f 100644 --- a/api/features/serializers.py +++ b/api/features/serializers.py @@ -9,6 +9,10 @@ CreateSegmentOverrideFeatureStateSerializer, FeatureStateValueSerializer, ) +from common.metadata.serializers import ( + MetadataSerializer, + SerializerWithMetadata, +) from drf_writable_nested import WritableNestedModelSerializer from drf_yasg.utils import swagger_serializer_method from rest_framework import serializers @@ -20,7 +24,6 @@ ) from integrations.github.constants import GitHubEventType from integrations.github.github import call_github_task -from metadata.serializers import MetadataSerializer, SerializerWithMetadata from projects.models import Project from users.serializers import ( UserIdsSerializer, diff --git a/api/features/versioning/permissions.py b/api/features/versioning/permissions.py index 0023ff33b6f6..b0a955c31605 100644 --- a/api/features/versioning/permissions.py +++ b/api/features/versioning/permissions.py @@ -1,15 +1,15 @@ -from rest_framework.permissions import BasePermission -from rest_framework.request import Request -from rest_framework.viewsets import GenericViewSet - -from environments.models import Environment -from environments.permissions.constants import ( +from common.environments.permissions import ( TAG_SUPPORTED_PERMISSIONS as TAG_SUPPORTED_ENVIRONMENT_PERMISSIONS, ) -from environments.permissions.constants import ( +from common.environments.permissions import ( UPDATE_FEATURE_STATE, VIEW_ENVIRONMENT, ) +from rest_framework.permissions import BasePermission +from rest_framework.request import Request +from rest_framework.viewsets import GenericViewSet + +from environments.models import Environment from features.models import Feature, FeatureState from features.versioning.models import EnvironmentFeatureVersion diff --git a/api/features/versioning/views.py b/api/features/versioning/views.py index cef5f5fab9dc..268094028af7 100644 --- a/api/features/versioning/views.py +++ b/api/features/versioning/views.py @@ -1,5 +1,7 @@ from datetime import timedelta +from common.environments.permissions import VIEW_ENVIRONMENT +from common.projects.permissions import VIEW_PROJECT from django.db.models import BooleanField, ExpressionWrapper, Q, QuerySet from django.shortcuts import get_object_or_404 from django.utils import timezone @@ -21,7 +23,6 @@ from app.pagination import CustomPagination from environments.models import Environment -from environments.permissions.constants import VIEW_ENVIRONMENT from features.models import Feature, FeatureState from features.serializers import ( CustomCreateSegmentOverrideFeatureStateSerializer, @@ -41,7 +42,6 @@ EnvironmentFeatureVersionRetrieveSerializer, EnvironmentFeatureVersionSerializer, ) -from projects.permissions import VIEW_PROJECT from users.models import FFAdminUser diff --git a/api/features/views.py b/api/features/views.py index b62973aceca1..c12084565f38 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -5,6 +5,7 @@ from app_analytics.analytics_db_service import get_feature_evaluation_data from app_analytics.influxdb_wrapper import get_multiple_event_list_for_feature +from common.projects.permissions import VIEW_PROJECT from core.constants import FLAGSMITH_UPDATED_AT_HEADER from core.request_origin import RequestOrigin from django.conf import settings @@ -38,7 +39,6 @@ ) from features.value_types import BOOLEAN, INTEGER, STRING from projects.models import Project -from projects.permissions import VIEW_PROJECT from users.models import FFAdminUser, UserPermissionGroup from webhooks.webhooks import WebhookEventType diff --git a/api/features/workflows/core/migrations/0011_add_project_to_change_requests.py b/api/features/workflows/core/migrations/0011_add_project_to_change_requests.py new file mode 100644 index 000000000000..d5df27aeca58 --- /dev/null +++ b/api/features/workflows/core/migrations/0011_add_project_to_change_requests.py @@ -0,0 +1,75 @@ +# 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") + + change_requests = [] + for change_request in ChangeRequest.objects.filter( + environment_id__isnull=False + ).select_related("environment", "environment__project"): + change_request.project = change_request.environment.project + change_requests.append(change_request) + + ChangeRequest.objects.bulk_update(change_requests, ["project"]) + + +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", + ), + ), + ] diff --git a/api/features/workflows/core/models.py b/api/features/workflows/core/models.py index 00d18d652f2a..3d1c99bf9d27 100644 --- a/api/features/workflows/core/models.py +++ b/api/features/workflows/core/models.py @@ -18,6 +18,7 @@ AFTER_CREATE, AFTER_SAVE, AFTER_UPDATE, + BEFORE_CREATE, BEFORE_DELETE, LifecycleModel, LifecycleModelMixin, @@ -77,10 +78,18 @@ class ChangeRequest( null=True, ) + 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) @@ -114,6 +123,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 @@ -181,6 +191,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() + + # 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 @@ -208,10 +242,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 @@ -228,8 +273,11 @@ 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 @@ -237,6 +285,10 @@ def url(self): 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): @@ -368,6 +420,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( diff --git a/api/import_export/export.py b/api/import_export/export.py index fd373bbc7255..efcdf630a1e5 100644 --- a/api/import_export/export.py +++ b/api/import_export/export.py @@ -114,7 +114,9 @@ def export_projects(organisation_id: int) -> typing.List[dict]: return _export_entities( _EntityExportConfig(Project, Q(organisation__id=organisation_id)), - _EntityExportConfig(Segment, default_filter), + _EntityExportConfig( + Segment, Q(project__organisation__id=organisation_id, id=F("version_of")) + ), _EntityExportConfig( SegmentRule, Q( diff --git a/api/integrations/common/views.py b/api/integrations/common/views.py index f4ed2d230608..64a37f91f352 100644 --- a/api/integrations/common/views.py +++ b/api/integrations/common/views.py @@ -1,3 +1,5 @@ +from common.environments.permissions import VIEW_ENVIRONMENT +from common.projects.permissions import VIEW_PROJECT from django.db.models import QuerySet from django.shortcuts import get_object_or_404 from rest_framework import viewsets @@ -7,12 +9,11 @@ from rest_framework.serializers import BaseSerializer from environments.models import Environment -from environments.permissions.constants import VIEW_ENVIRONMENT from environments.permissions.permissions import NestedEnvironmentPermissions from organisations.permissions.permissions import ( NestedOrganisationEntityPermission, ) -from projects.permissions import VIEW_PROJECT, NestedProjectPermissions +from projects.permissions import NestedProjectPermissions class EnvironmentIntegrationCommonViewSet(viewsets.ModelViewSet): diff --git a/api/integrations/dynatrace/dynatrace.py b/api/integrations/dynatrace/dynatrace.py index 0a448d24a27b..d888e668c0ef 100644 --- a/api/integrations/dynatrace/dynatrace.py +++ b/api/integrations/dynatrace/dynatrace.py @@ -95,7 +95,7 @@ def _get_deployment_name_for_feature( def _get_deployment_name_for_segment(object_id: int) -> str: - if segment := Segment.objects.all_with_deleted().filter(id=object_id).first(): + if segment := Segment.live_objects.all_with_deleted().filter(id=object_id).first(): return f"Flagsmith Deployment - Segment Changed: {segment.name}" return DEFAULT_DEPLOYMENT_NAME diff --git a/api/integrations/launch_darkly/services.py b/api/integrations/launch_darkly/services.py index c4650a44b6b1..a0a480733b50 100644 --- a/api/integrations/launch_darkly/services.py +++ b/api/integrations/launch_darkly/services.py @@ -241,7 +241,7 @@ def _create_feature_segments_for_segment_match_clauses( targeted_segment_name = segments_by_ld_key[targeted_segment_key].name # We assume segment is already created. - segment = Segment.objects.get(name=targeted_segment_name, project=project) + segment = Segment.live_objects.get(name=targeted_segment_name, project=project) feature_segment, _ = FeatureSegment.objects.update_or_create( feature=feature, @@ -368,7 +368,7 @@ def _create_feature_segment_from_clauses( ) # Create a feature specific segment for the rule. - segment, _ = Segment.objects.update_or_create( + segment, _ = Segment.live_objects.update_or_create( name=rule_name, project=project, feature=feature ) @@ -961,7 +961,7 @@ def _create_segments_from_ld( continue # Make sure consecutive updates do not create the same segment. - segment, _ = Segment.objects.update_or_create( + segment, _ = Segment.live_objects.update_or_create( name=_get_segment_name(ld_segment["name"], env), project_id=project_id, ) diff --git a/api/integrations/launch_darkly/views.py b/api/integrations/launch_darkly/views.py index 93e2ddc0cc81..a781d416a8e5 100644 --- a/api/integrations/launch_darkly/views.py +++ b/api/integrations/launch_darkly/views.py @@ -1,3 +1,4 @@ +from common.projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT from django.db.models import QuerySet from django.db.utils import IntegrityError from django.shortcuts import get_object_or_404 @@ -18,11 +19,7 @@ process_launch_darkly_import_request, ) from projects.models import Project -from projects.permissions import ( - CREATE_ENVIRONMENT, - VIEW_PROJECT, - NestedProjectPermissions, -) +from projects.permissions import NestedProjectPermissions class LaunchDarklyImportRequestViewSet( diff --git a/api/metadata/serializers.py b/api/metadata/serializers.py index a6fdddf89582..be46aea7a8ce 100644 --- a/api/metadata/serializers.py +++ b/api/metadata/serializers.py @@ -1,15 +1,11 @@ from django.contrib.contenttypes.models import ContentType -from django.db.models import Model from rest_framework import serializers -from organisations.models import Organisation -from projects.models import Project from util.drf_writable_nested.serializers import ( DeleteBeforeUpdateWritableNestedModelSerializer, ) from .models import ( - Metadata, MetadataField, MetadataModelField, MetadataModelFieldRequirement, @@ -71,69 +67,3 @@ class ContentTypeSerializer(serializers.ModelSerializer): class Meta: model = ContentType fields = ("id", "app_label", "model") - - -class MetadataSerializer(serializers.ModelSerializer): - class Meta: - model = Metadata - fields = ("id", "model_field", "field_value") - - def validate(self, data): - data = super().validate(data) - if not data["model_field"].field.is_field_value_valid(data["field_value"]): - raise serializers.ValidationError( - f"Invalid value for field {data['model_field'].field.name}" - ) - - return data - - -class SerializerWithMetadata(serializers.BaseSerializer): - def get_organisation(self, validated_data: dict = None) -> Organisation: - return self.get_project(validated_data).organisation - - def get_project(self, validated_data: dict = None) -> Project: - raise NotImplementedError() - - def get_required_for_object( - self, requirement: MetadataModelFieldRequirement, data: dict - ) -> Model: - model_name = requirement.content_type.model - try: - return getattr(self, f"get_{model_name}")(data) - except AttributeError: - raise ValueError( - f"`get_{model_name}_from_validated_data` method does not exist" - ) - - def validate_required_metadata(self, data): - metadata = data.get("metadata", []) - - content_type = ContentType.objects.get_for_model(self.Meta.model) - - organisation = self.get_organisation(data) - - requirements = MetadataModelFieldRequirement.objects.filter( - model_field__content_type=content_type, - model_field__field__organisation=organisation, - ) - - for requirement in requirements: - required_for = self.get_required_for_object(requirement, data) - if required_for.id == requirement.object_id: - if not any( - [ - field["model_field"] == requirement.model_field - for field in metadata - ] - ): - raise serializers.ValidationError( - { - "metadata": f"Missing required metadata field: {requirement.model_field.field.name}" - } - ) - - def validate(self, data): - data = super().validate(data) - self.validate_required_metadata(data) - return data diff --git a/api/permissions/migrations/0001_initial.py b/api/permissions/migrations/0001_initial.py index 33a0f25ce2d8..5c2e4a57233b 100644 --- a/api/permissions/migrations/0001_initial.py +++ b/api/permissions/migrations/0001_initial.py @@ -1,10 +1,20 @@ # Generated by Django 2.2.10 on 2020-02-20 00:24 +from common.environments.permissions import ( + APPROVE_CHANGE_REQUEST, + CREATE_CHANGE_REQUEST, + MANAGE_IDENTITIES, + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import PROJECT_PERMISSIONS from django.db import migrations, models -from permissions.models import PROJECT_PERMISSION_TYPE, ENVIRONMENT_PERMISSION_TYPE -from projects.permissions import PROJECT_PERMISSIONS -from environments.permissions.constants import VIEW_ENVIRONMENT, UPDATE_FEATURE_STATE, MANAGE_IDENTITIES, CREATE_CHANGE_REQUEST, APPROVE_CHANGE_REQUEST +from permissions.models import ( + ENVIRONMENT_PERMISSION_TYPE, + PROJECT_PERMISSION_TYPE, +) + ENVIRONMENT_PERMISSIONS = [ (VIEW_ENVIRONMENT, "View permission for the given environment."), (UPDATE_FEATURE_STATE, "Update the state or value for a given feature state."), @@ -21,23 +31,33 @@ def insert_default_project_permissions(apps, schema_model): - PermissionModel = apps.get_model('permissions', 'PermissionModel') + PermissionModel = apps.get_model("permissions", "PermissionModel") project_permissions = [] for permission in PROJECT_PERMISSIONS: project_permissions.append( - PermissionModel(key=permission[0], description=permission[1], type=PROJECT_PERMISSION_TYPE)) + PermissionModel( + key=permission[0], + description=permission[1], + type=PROJECT_PERMISSION_TYPE, + ) + ) PermissionModel.objects.bulk_create(project_permissions) def insert_default_environment_permissions(apps, schema_model): - PermissionModel = apps.get_model('permissions', 'PermissionModel') + PermissionModel = apps.get_model("permissions", "PermissionModel") environment_permissions = [] for permission in ENVIRONMENT_PERMISSIONS: environment_permissions.append( - PermissionModel(key=permission[0], description=permission[1], type=ENVIRONMENT_PERMISSION_TYPE)) + PermissionModel( + key=permission[0], + description=permission[1], + type=ENVIRONMENT_PERMISSION_TYPE, + ) + ) PermissionModel.objects.bulk_create(environment_permissions) @@ -46,18 +66,34 @@ class Migration(migrations.Migration): initial = True - dependencies = [ - ] + dependencies = [] operations = [ migrations.CreateModel( - name='PermissionModel', + name="PermissionModel", fields=[ - ('key', models.CharField(max_length=100, primary_key=True, serialize=False)), - ('description', models.TextField()), - ('type', models.CharField(choices=[('PROJECT', 'Project'), ('ENVIRONMENT', 'Environment')], max_length=100, null=True)), + ( + "key", + models.CharField(max_length=100, primary_key=True, serialize=False), + ), + ("description", models.TextField()), + ( + "type", + models.CharField( + choices=[ + ("PROJECT", "Project"), + ("ENVIRONMENT", "Environment"), + ], + max_length=100, + null=True, + ), + ), ], ), - migrations.RunPython(insert_default_project_permissions, reverse_code=lambda *args: None), - migrations.RunPython(insert_default_environment_permissions, reverse_code=lambda *args: None), + migrations.RunPython( + insert_default_project_permissions, reverse_code=lambda *args: None + ), + migrations.RunPython( + insert_default_environment_permissions, reverse_code=lambda *args: None + ), ] diff --git a/api/permissions/migrations/0008_add_view_audit_log_permission.py b/api/permissions/migrations/0008_add_view_audit_log_permission.py index 3e1b351602c1..fb4e2a712f11 100644 --- a/api/permissions/migrations/0008_add_view_audit_log_permission.py +++ b/api/permissions/migrations/0008_add_view_audit_log_permission.py @@ -1,10 +1,10 @@ # Generated by Django 3.2.16 on 2022-12-08 11:02 +from common.projects.permissions import VIEW_AUDIT_LOG from django.apps.registry import Apps from django.db import migrations from django.db.backends.base.schema import BaseDatabaseSchemaEditor -from projects.permissions import VIEW_AUDIT_LOG from permissions.models import ORGANISATION_PERMISSION_TYPE diff --git a/api/permissions/migrations/0009_move_view_audit_log_permission.py b/api/permissions/migrations/0009_move_view_audit_log_permission.py index b3c03233867d..e11762440d37 100644 --- a/api/permissions/migrations/0009_move_view_audit_log_permission.py +++ b/api/permissions/migrations/0009_move_view_audit_log_permission.py @@ -1,16 +1,17 @@ # Generated by Django 3.2.16 on 2022-12-08 11:02 +from common.projects.permissions import VIEW_AUDIT_LOG from django.apps.registry import Apps from django.db import migrations from django.db.backends.base.schema import BaseDatabaseSchemaEditor -from projects.permissions import VIEW_AUDIT_LOG -from permissions.models import ORGANISATION_PERMISSION_TYPE, PROJECT_PERMISSION_TYPE +from permissions.models import ( + ORGANISATION_PERMISSION_TYPE, + PROJECT_PERMISSION_TYPE, +) -def move_permission_to_project( - apps: Apps, schema_editor: BaseDatabaseSchemaEditor -): +def move_permission_to_project(apps: Apps, schema_editor: BaseDatabaseSchemaEditor): permission_model_class = apps.get_model("permissions", "PermissionModel") permission_model_class.objects.filter( @@ -43,4 +44,4 @@ class Migration(migrations.Migration): migrations.RunPython( move_permission_to_project, reverse_code=move_permission_to_organisation ) - ] \ No newline at end of file + ] diff --git a/api/permissions/migrations/0010_add_manage_tags_permission.py b/api/permissions/migrations/0010_add_manage_tags_permission.py index c7ddfd24e538..7e10801b6bde 100644 --- a/api/permissions/migrations/0010_add_manage_tags_permission.py +++ b/api/permissions/migrations/0010_add_manage_tags_permission.py @@ -1,13 +1,15 @@ # Generated by Django 4.2.15 on 2024-09-13 16:18 +from common.projects.permissions import MANAGE_TAGS from django.apps.registry import Apps from django.db import migrations from django.db.backends.base.schema import BaseDatabaseSchemaEditor from permissions.models import PROJECT_PERMISSION_TYPE -from projects.permissions import MANAGE_TAGS -def add_manage_tags_permission(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: +def add_manage_tags_permission( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +) -> None: permission_model_class = apps.get_model("permissions", "permissionmodel") permission_model_class.objects.get_or_create( key=MANAGE_TAGS, @@ -16,7 +18,9 @@ def add_manage_tags_permission(apps: Apps, schema_editor: BaseDatabaseSchemaEdit ) -def reverse(apps: Apps, schema_editor: BaseDatabaseSchemaEditor) -> None: # pragma: no cover +def reverse( + apps: Apps, schema_editor: BaseDatabaseSchemaEditor +) -> None: # pragma: no cover permission_model_class = apps.get_model("permissions", "permissionmodel") permission_model_class.objects.filter(key=MANAGE_TAGS).delete() diff --git a/api/poetry.lock b/api/poetry.lock index a9160aeebb48..ba834dbae554 100644 --- a/api/poetry.lock +++ b/api/poetry.lock @@ -1353,15 +1353,17 @@ files = [] develop = false [package.dependencies] -django = "*" +django = "<5.0.0" djangorestframework = "*" +djangorestframework-recursive = "*" drf-writable-nested = "*" +flagsmith-flag-engine = "*" [package.source] type = "git" url = "https://github.com/Flagsmith/flagsmith-common" -reference = "v1.0.0" -resolved_reference = "f3809f6d592b2c6cfdfa88e0b345ce722ac47727" +reference = "v1.1.0" +resolved_reference = "27fbd8b7d889dc1529df08972a8c1bfaba5a7e03" [[package]] name = "flagsmith-flag-engine" @@ -3338,7 +3340,6 @@ files = [ {file = "PyYAML-6.0.1-cp311-cp311-win_amd64.whl", hash = "sha256:bf07ee2fef7014951eeb99f56f39c9bb4af143d8aa3c21b1677805985307da34"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_10_9_x86_64.whl", hash = "sha256:855fb52b0dc35af121542a76b9a84f8d1cd886ea97c84703eaa6d88e37a2ad28"}, {file = "PyYAML-6.0.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:40df9b996c2b73138957fe23a16a4f0ba614f4c0efce1e9406a184b6d07fa3a9"}, - {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a08c6f0fe150303c1c6b71ebcd7213c2858041a7e01975da3a99aed1e7a378ef"}, {file = "PyYAML-6.0.1-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:6c22bec3fbe2524cde73d7ada88f6566758a8f7227bfbf93a408a9d86bcc12a0"}, {file = "PyYAML-6.0.1-cp312-cp312-musllinux_1_1_x86_64.whl", hash = "sha256:8d4e9c88387b0f5c7d5f281e55304de64cf7f9c0021a3525bd3b1c542da3b0e4"}, {file = "PyYAML-6.0.1-cp312-cp312-win32.whl", hash = "sha256:d483d2cdf104e7c9fa60c544d92981f12ad66a457afae824d146093b8c294c54"}, @@ -4057,13 +4058,16 @@ files = [] develop = false [package.dependencies] -flagsmith-common = {git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.0.0"} +djangorestframework = "*" +djangorestframework-recursive = "*" +flagsmith-common = {git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.1.0"} +flagsmith-flag-engine = "*" [package.source] type = "git" url = "https://github.com/flagsmith/flagsmith-workflows" -reference = "v2.5.0" -resolved_reference = "9fd951a470de537389c8d08c186656464500f3ed" +reference = "v2.6.0" +resolved_reference = "06b03d428484f5aed2f400cef431ab5aae0d0df0" [[package]] name = "wrapt" @@ -4182,4 +4186,4 @@ files = [ [metadata] lock-version = "2.0" python-versions = ">=3.11, <3.13" -content-hash = "27333f5bbd3bb607cdb7d728dae6d0a6a11658cba09b45d69a6ee5a744111ad5" +content-hash = "df2e02787204f56111c560faa7f7800a09e87e9bdc0b9802660af5bd08aa03ee" diff --git a/api/projects/migrations/0003_auto_20200216_2050.py b/api/projects/migrations/0003_auto_20200216_2050.py index 21e8b7ea115d..eb61d86910b7 100644 --- a/api/projects/migrations/0003_auto_20200216_2050.py +++ b/api/projects/migrations/0003_auto_20200216_2050.py @@ -1,16 +1,17 @@ # Generated by Django 2.2.10 on 2020-02-16 20:50 +from common.projects.permissions import PROJECT_PERMISSIONS from django.db import migrations -from projects.permissions import PROJECT_PERMISSIONS - def insert_default_permissions(apps, schema_model): - ProjectPermission = apps.get_model('projects', 'ProjectPermission') + ProjectPermission = apps.get_model("projects", "ProjectPermission") project_permissions = [] for permission in PROJECT_PERMISSIONS: - project_permissions.append(ProjectPermission(key=permission[0], description=permission[1])) + project_permissions.append( + ProjectPermission(key=permission[0], description=permission[1]) + ) ProjectPermission.objects.bulk_create(project_permissions) @@ -18,9 +19,14 @@ def insert_default_permissions(apps, schema_model): class Migration(migrations.Migration): dependencies = [ - ('projects', '0002_projectpermission_userpermissiongroupprojectpermission_userprojectpermission'), + ( + "projects", + "0002_projectpermission_userpermissiongroupprojectpermission_userprojectpermission", + ), ] operations = [ - migrations.RunPython(insert_default_permissions, reverse_code=lambda *args: None) + migrations.RunPython( + insert_default_permissions, reverse_code=lambda *args: None + ) ] diff --git a/api/projects/migrations/0025_add_change_request_project_permissions.py b/api/projects/migrations/0025_add_change_request_project_permissions.py new file mode 100644 index 000000000000..336c209eeb82 --- /dev/null +++ b/api/projects/migrations/0025_add_change_request_project_permissions.py @@ -0,0 +1,48 @@ +# Generated by Django 4.2.15 on 2024-09-17 14:07 + +from common.projects.permissions import ( + APPROVE_PROJECT_LEVEL_CHANGE_REQUESTS, + MANAGE_PROJECT_LEVEL_CHANGE_REQUESTS, + PROJECT_PERMISSIONS, +) +from django.db import migrations + +from permissions.models import PROJECT_PERMISSION_TYPE + + +def remove_default_project_permissions(apps, schema_model): # pragma: no cover + PermissionModel = apps.get_model("permissions", "PermissionModel") + PermissionModel.objects.get(key=MANAGE_PROJECT_LEVEL_CHANGE_REQUESTS).delete() + PermissionModel.objects.get(key=APPROVE_PROJECT_LEVEL_CHANGE_REQUESTS).delete() + + +def insert_default_project_permissions(apps, schema_model): + PermissionModel = apps.get_model("permissions", "PermissionModel") + + manage_description = "Ability to manage change requests associated with a project." + approve_description = "Ability to approve project level change requests." + + PermissionModel.objects.get_or_create( + key=MANAGE_PROJECT_LEVEL_CHANGE_REQUESTS, + description=manage_description, + type=PROJECT_PERMISSION_TYPE, + ) + PermissionModel.objects.get_or_create( + key=APPROVE_PROJECT_LEVEL_CHANGE_REQUESTS, + description=approve_description, + type=PROJECT_PERMISSION_TYPE, + ) + + +class Migration(migrations.Migration): + + dependencies = [ + ("projects", "0024_add_project_edge_v2_migration_read_capacity_budget"), + ] + + operations = [ + migrations.RunPython( + insert_default_project_permissions, + reverse_code=remove_default_project_permissions, + ), + ] diff --git a/api/projects/migrations/0026_add_change_request_approval_limit_to_projects.py b/api/projects/migrations/0026_add_change_request_approval_limit_to_projects.py new file mode 100644 index 000000000000..209e6d1f5519 --- /dev/null +++ b/api/projects/migrations/0026_add_change_request_approval_limit_to_projects.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.15 on 2024-09-20 14:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("projects", "0025_add_change_request_project_permissions"), + ] + + operations = [ + migrations.AddField( + model_name="project", + name="minimum_change_request_approvals", + field=models.IntegerField(blank=True, null=True), + ), + ] diff --git a/api/projects/models.py b/api/projects/models.py index d20c26eea833..0eee8a630b66 100644 --- a/api/projects/models.py +++ b/api/projects/models.py @@ -25,13 +25,13 @@ PermissionModel, ) from projects.managers import ProjectManager +from projects.services import get_project_segments_from_cache from projects.tasks import ( handle_cascade_delete, migrate_project_environments_to_v2, write_environments_to_dynamodb, ) -project_segments_cache = caches[settings.PROJECT_SEGMENTS_CACHE_LOCATION] environment_cache = caches[settings.ENVIRONMENT_CACHE_NAME] @@ -106,6 +106,7 @@ class Project(LifecycleModelMixin, SoftDeleteExportableModel): default=30, help_text="Number of days without modification in any environment before a flag is considered stale.", ) + minimum_change_request_approvals = models.IntegerField(blank=True, null=True) objects = ProjectManager() @@ -119,7 +120,7 @@ def __str__(self): def is_too_large(self) -> bool: return ( self.features.count() > self.max_features_allowed - or self.segments.count() > self.max_segments_allowed + or self.live_segment_count() > self.max_segments_allowed or self.environments.annotate( segment_override_count=Count("feature_segments") ) @@ -142,24 +143,7 @@ def edge_v2_identity_overrides_migrated(self) -> bool: return self.edge_v2_migration_status == EdgeV2MigrationStatus.COMPLETE def get_segments_from_cache(self): - segments = project_segments_cache.get(self.id) - - if not segments: - # This is optimised to account for rules nested one levels deep (since we - # don't support anything above that from the UI at the moment). Anything - # past that will require additional queries / thought on how to optimise. - segments = self.segments.all().prefetch_related( - "rules", - "rules__conditions", - "rules__rules", - "rules__rules__conditions", - "rules__rules__rules", - ) - project_segments_cache.set( - self.id, segments, timeout=settings.CACHE_PROJECT_SEGMENTS_SECONDS - ) - - return segments + return get_project_segments_from_cache(self.id) @hook(BEFORE_CREATE) def set_enable_dynamo_db(self): @@ -204,6 +188,11 @@ def is_edge_project_by_default(self) -> bool: and self.created_date >= settings.EDGE_RELEASE_DATETIME ) + def live_segment_count(self) -> int: + from segments.models import Segment + + return Segment.live_objects.filter(project=self).count() + def is_feature_name_valid(self, feature_name: str) -> bool: """ Validate the feature name based on the feature_name_regex attribute. diff --git a/api/projects/permissions.py b/api/projects/permissions.py index 1903c94a6b18..abd5c6f74de2 100644 --- a/api/projects/permissions.py +++ b/api/projects/permissions.py @@ -1,5 +1,6 @@ import typing +from common.projects.permissions import VIEW_PROJECT from django.db.models import Model from rest_framework.exceptions import APIException, PermissionDenied from rest_framework.permissions import BasePermission, IsAuthenticated @@ -8,30 +9,6 @@ from organisations.permissions.permissions import CREATE_PROJECT from projects.models import Project -VIEW_AUDIT_LOG = "VIEW_AUDIT_LOG" - -# Maintain a list of permissions here -VIEW_PROJECT = "VIEW_PROJECT" -CREATE_ENVIRONMENT = "CREATE_ENVIRONMENT" -DELETE_FEATURE = "DELETE_FEATURE" -CREATE_FEATURE = "CREATE_FEATURE" -EDIT_FEATURE = "EDIT_FEATURE" -MANAGE_SEGMENTS = "MANAGE_SEGMENTS" -MANAGE_TAGS = "MANAGE_TAGS" - -TAG_SUPPORTED_PERMISSIONS = [DELETE_FEATURE] - -PROJECT_PERMISSIONS = [ - (VIEW_PROJECT, "View permission for the given project."), - (CREATE_ENVIRONMENT, "Ability to create an environment in the given project."), - (DELETE_FEATURE, "Ability to delete features in the given project."), - (CREATE_FEATURE, "Ability to create features in the given project."), - (EDIT_FEATURE, "Ability to edit features in the given project."), - (MANAGE_SEGMENTS, "Ability to manage segments in the given project."), - (VIEW_AUDIT_LOG, "Allows the user to view the audit logs for this organisation."), - (MANAGE_TAGS, "Allows the user to manage tags in the given project."), -] - class ProjectPermissions(IsAuthenticated): def has_permission(self, request, view): diff --git a/api/projects/serializers.py b/api/projects/serializers.py index 7a934861d7c8..339b1b8fa90a 100644 --- a/api/projects/serializers.py +++ b/api/projects/serializers.py @@ -43,6 +43,7 @@ class Meta: "show_edge_identity_overrides_for_feature", "stale_flags_limit_days", "edge_v2_migration_status", + "minimum_change_request_approvals", ) read_only_fields = ( "enable_dynamo_db", @@ -130,7 +131,7 @@ def get_total_features(self, instance: Project) -> int: def get_total_segments(self, instance: Project) -> int: # added here to prevent need for annotate(Count("segments", distinct=True)) # which causes performance issues. - return instance.segments.count() + return instance.live_segment_count() class CreateUpdateUserProjectPermissionSerializer( diff --git a/api/projects/services.py b/api/projects/services.py new file mode 100644 index 000000000000..f69414101df5 --- /dev/null +++ b/api/projects/services.py @@ -0,0 +1,35 @@ +import typing + +from django.apps import apps +from django.conf import settings +from django.core.cache import caches + +if typing.TYPE_CHECKING: + from django.db.models import QuerySet + + from segments.models import Segment + +project_segments_cache = caches[settings.PROJECT_SEGMENTS_CACHE_LOCATION] + + +def get_project_segments_from_cache(project_id: int) -> "QuerySet[Segment]": + Segment = apps.get_model("segments", "Segment") + + segments = project_segments_cache.get(project_id) + if not segments: + # This is optimised to account for rules nested one levels deep (since we + # don't support anything above that from the UI at the moment). Anything + # past that will require additional queries / thought on how to optimise. + segments = Segment.live_objects.filter(project_id=project_id).prefetch_related( + "rules", + "rules__conditions", + "rules__rules", + "rules__rules__conditions", + "rules__rules__rules", + ) + + project_segments_cache.set( + project_id, segments, timeout=settings.CACHE_PROJECT_SEGMENTS_SECONDS + ) + + return segments diff --git a/api/projects/tags/permissions.py b/api/projects/tags/permissions.py index 88a480624952..41c7f2d9dfec 100644 --- a/api/projects/tags/permissions.py +++ b/api/projects/tags/permissions.py @@ -1,7 +1,7 @@ +from common.projects.permissions import MANAGE_TAGS, VIEW_PROJECT from rest_framework.permissions import BasePermission from projects.models import Project -from projects.permissions import MANAGE_TAGS, VIEW_PROJECT class TagPermissions(BasePermission): diff --git a/api/projects/tags/views.py b/api/projects/tags/views.py index 49b163316240..bd0a07a73217 100644 --- a/api/projects/tags/views.py +++ b/api/projects/tags/views.py @@ -1,3 +1,4 @@ +from common.projects.permissions import VIEW_PROJECT from rest_framework import status, viewsets from rest_framework.decorators import action from rest_framework.generics import get_object_or_404 @@ -5,8 +6,6 @@ from rest_framework.request import Request from rest_framework.response import Response -from projects.permissions import VIEW_PROJECT - from . import serializers from .models import Tag from .permissions import TagPermissions diff --git a/api/projects/urls.py b/api/projects/urls.py index 4a38160dce16..b0150651f82a 100644 --- a/api/projects/urls.py +++ b/api/projects/urls.py @@ -1,3 +1,6 @@ +import importlib + +from django.conf import settings from django.urls import include, path, re_path from rest_framework_nested import routers @@ -67,6 +70,16 @@ ProjectAuditLogViewSet, basename="project-audit", ) + +if settings.WORKFLOWS_LOGIC_INSTALLED: # pragma: no cover + workflow_views = importlib.import_module("workflows_logic.views") + projects_router.register( + r"change-requests", + workflow_views.ProjectChangeRequestViewSet, + basename="project-change-requests", + ) + + nested_features_router = routers.NestedSimpleRouter( projects_router, r"features", lookup="feature" ) diff --git a/api/projects/views.py b/api/projects/views.py index 27c720f6a138..f75269e45036 100644 --- a/api/projects/views.py +++ b/api/projects/views.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +from common.projects.permissions import TAG_SUPPORTED_PERMISSIONS, VIEW_PROJECT from django.conf import settings from django.utils.decorators import method_decorator from drf_yasg import openapi @@ -33,12 +34,7 @@ UserPermissionGroupProjectPermission, UserProjectPermission, ) -from projects.permissions import ( - TAG_SUPPORTED_PERMISSIONS, - VIEW_PROJECT, - IsProjectAdmin, - ProjectPermissions, -) +from projects.permissions import IsProjectAdmin, ProjectPermissions from projects.serializers import ( CreateUpdateUserPermissionGroupProjectPermissionSerializer, CreateUpdateUserProjectPermissionSerializer, diff --git a/api/pyproject.toml b/api/pyproject.toml index aaac71259708..90a75efd26d8 100644 --- a/api/pyproject.toml +++ b/api/pyproject.toml @@ -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.0.2" } -flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.0.0" } +flagsmith-common = { git = "https://github.com/Flagsmith/flagsmith-common", tag = "v1.1.0" } tzdata = "^2024.1" djangorestframework-simplejwt = "^5.3.1" @@ -196,7 +196,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.5.0" } +workflows-logic = { git = "https://github.com/flagsmith/flagsmith-workflows", tag = "v2.6.0" } [tool.poetry.group.dev.dependencies] django-test-migrations = "~1.2.0" diff --git a/api/sales_dashboard/templates/sales_dashboard/organisation.html b/api/sales_dashboard/templates/sales_dashboard/organisation.html index f0e7563a973f..3ae4faa87db0 100644 --- a/api/sales_dashboard/templates/sales_dashboard/organisation.html +++ b/api/sales_dashboard/templates/sales_dashboard/organisation.html @@ -129,7 +129,7 @@

Projects

{{project.name}} {{project.environments.all.count}} {{project.features.all.count}} - {{project.segments.all.count}} + {{project.live_segment_count}} {{project.enable_dynamo_db}} {{identity_count_dict|get_item:project.id|intcomma}} {{identity_migration_status_dict|get_item:project.id}} diff --git a/api/segments/managers.py b/api/segments/managers.py index d1f977ca06ed..48f4b6da0eac 100644 --- a/api/segments/managers.py +++ b/api/segments/managers.py @@ -3,6 +3,10 @@ class SegmentManager(SoftDeleteExportableManager): + pass + + +class LiveSegmentManager(SoftDeleteExportableManager): def get_queryset(self): """ Returns only the canonical segments, which will always be diff --git a/api/segments/migrations/0026_add_change_request_to_segments.py b/api/segments/migrations/0026_add_change_request_to_segments.py new file mode 100644 index 000000000000..04c51be17e20 --- /dev/null +++ b/api/segments/migrations/0026_add_change_request_to_segments.py @@ -0,0 +1,38 @@ +# Generated by Django 4.2.15 on 2024-09-06 15:36 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("workflows_core", "0009_prevent_cascade_delete_from_user_delete"), + ("segments", "0025_set_default_version_on_segment"), + ] + + operations = [ + migrations.AddField( + model_name="historicalsegment", + name="change_request", + field=models.ForeignKey( + blank=True, + db_constraint=False, + null=True, + on_delete=django.db.models.deletion.DO_NOTHING, + related_name="+", + to="workflows_core.changerequest", + ), + ), + migrations.AddField( + model_name="segment", + name="change_request", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="segments", + to="workflows_core.changerequest", + ), + ), + ] diff --git a/api/segments/models.py b/api/segments/models.py index 8b8553484985..22b8c30d19d1 100644 --- a/api/segments/models.py +++ b/api/segments/models.py @@ -4,7 +4,6 @@ from copy import deepcopy from core.models import ( - SoftDeleteExportableManager, SoftDeleteExportableModel, abstract_base_auditable_model_factory, ) @@ -31,7 +30,7 @@ from projects.models import Project from .helpers import segment_audit_log_helper -from .managers import SegmentManager +from .managers import LiveSegmentManager, SegmentManager logger = logging.getLogger(__name__) @@ -60,7 +59,6 @@ class Segment( # This defaults to 1 for newly created segments. version = models.IntegerField(null=True) - # The related_name is not useful without specifying all_objects as a manager. version_of = models.ForeignKey( "self", on_delete=models.CASCADE, @@ -68,16 +66,24 @@ class Segment( null=True, blank=True, ) + + change_request = models.ForeignKey( + "workflows_core.ChangeRequest", + on_delete=models.CASCADE, + null=True, + blank=True, + related_name="segments", + ) + metadata = GenericRelation(Metadata) created_at = models.DateTimeField(null=True, auto_now_add=True) updated_at = models.DateTimeField(null=True, auto_now=True) - # Only serves segments that are the canonical version. objects = SegmentManager() - # Includes versioned segments. - all_objects = SoftDeleteExportableManager() + # Only serves segments that are the canonical version. + live_objects = LiveSegmentManager() class Meta: ordering = ("id",) # explicit ordering to prevent pagination warnings @@ -145,6 +151,26 @@ def set_version_of_to_self_if_none(self): self.save() segment_audit_log_helper.unset_skip_audit_log(self.id) + def shallow_clone( + self, + name: str, + description: str, + change_request: typing.Optional["ChangeRequest"], # noqa: F821 + ) -> "Segment": + cloned_segment = Segment( + version_of=self, + uuid=uuid.uuid4(), + name=name, + description=description, + change_request=change_request, + project=self.project, + feature=self.feature, + version=None, + ) + cloned_segment.history.update() + cloned_segment.save() + return cloned_segment + def deep_clone(self) -> "Segment": cloned_segment = deepcopy(self) cloned_segment.id = None diff --git a/api/segments/permissions.py b/api/segments/permissions.py index efb901bc0177..283acec53596 100644 --- a/api/segments/permissions.py +++ b/api/segments/permissions.py @@ -1,7 +1,7 @@ +from common.projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT from rest_framework.permissions import IsAuthenticated from projects.models import Project -from projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT class SegmentPermissions(IsAuthenticated): diff --git a/api/segments/serializers.py b/api/segments/serializers.py index 053d119d0499..644a8a4d804b 100644 --- a/api/segments/serializers.py +++ b/api/segments/serializers.py @@ -1,248 +1,6 @@ -import logging -import typing - -from django.conf import settings -from django.contrib.contenttypes.models import ContentType -from flag_engine.segments.constants import PERCENTAGE_SPLIT from rest_framework import serializers -from rest_framework.exceptions import ValidationError -from rest_framework.serializers import ListSerializer -from rest_framework_recursive.fields import RecursiveField - -from metadata.models import Metadata -from metadata.serializers import MetadataSerializer, SerializerWithMetadata -from projects.models import Project -from segments.models import Condition, Segment, SegmentRule - -logger = logging.getLogger(__name__) - - -class ConditionSerializer(serializers.ModelSerializer): - delete = serializers.BooleanField(write_only=True, required=False) - - class Meta: - model = Condition - fields = ("id", "operator", "property", "value", "description", "delete") - - def validate(self, attrs): - super(ConditionSerializer, self).validate(attrs) - if attrs.get("operator") != PERCENTAGE_SPLIT and not attrs.get("property"): - raise ValidationError({"property": ["This field may not be blank."]}) - return attrs - - def to_internal_value(self, data): - # convert value to a string - conversion to correct value type is handled elsewhere - data["value"] = str(data["value"]) if "value" in data else None - return super(ConditionSerializer, self).to_internal_value(data) - - -class RuleSerializer(serializers.ModelSerializer): - delete = serializers.BooleanField(write_only=True, required=False) - conditions = ConditionSerializer(many=True, required=False) - rules = ListSerializer(child=RecursiveField(), required=False) - - class Meta: - model = SegmentRule - fields = ("id", "type", "rules", "conditions", "delete") - - -class SegmentSerializer(serializers.ModelSerializer, SerializerWithMetadata): - rules = RuleSerializer(many=True) - metadata = MetadataSerializer(required=False, many=True) - - class Meta: - model = Segment - fields = "__all__" - - def validate(self, attrs): - attrs = super().validate(attrs) - self.validate_required_metadata(attrs) - if not attrs.get("rules"): - raise ValidationError( - {"rules": "Segment cannot be created without any rules."} - ) - return attrs - - def get_project(self, validated_data: dict = None) -> Project: - return validated_data.get("project") or Project.objects.get( - id=self.context["view"].kwargs["project_pk"] - ) - - def create(self, validated_data): - project = validated_data["project"] - self.validate_project_segment_limit(project) - - rules_data = validated_data.pop("rules", []) - metadata_data = validated_data.pop("metadata", []) - self.validate_segment_rules_conditions_limit(rules_data) - - # create segment with nested rules and conditions - segment = Segment.objects.create(**validated_data) - self._update_or_create_segment_rules( - rules_data, segment=segment, is_create=True - ) - self._update_or_create_metadata(metadata_data, segment=segment) - return segment - - def update(self, instance: Segment, validated_data: dict[str, typing.Any]) -> None: - # use the initial data since we need the ids included to determine which to update & which to create - rules_data = self.initial_data.pop("rules", []) - metadata_data = validated_data.pop("metadata", []) - self.validate_segment_rules_conditions_limit(rules_data) - - # Create a version of the segment now that we're updating. - cloned_segment = instance.deep_clone() - logger.info( - f"Updating cloned segment {cloned_segment.id} for original segment {instance.id}" - ) - - try: - self._update_segment_rules(rules_data, segment=instance) - self._update_or_create_metadata(metadata_data, segment=instance) - - # remove rules from validated data to prevent error trying to create segment with nested rules - del validated_data["rules"] - response = super().update(instance, validated_data) - except Exception: - # Since there was a problem during the update we now delete the cloned segment, - # since we no longer need a versioned segment. - instance.refresh_from_db() - instance.version = cloned_segment.version - instance.save() - cloned_segment.hard_delete() - raise - return response - - def validate_project_segment_limit(self, project: Project) -> None: - if project.segments.count() >= project.max_segments_allowed: - raise ValidationError( - { - "project": "The project has reached the maximum allowed segments limit." - } - ) - - def validate_segment_rules_conditions_limit( - self, rules_data: dict[str, object] - ) -> None: - if self.instance and getattr(self.instance, "whitelisted_segment", None): - return - - count = self._calculate_condition_count(rules_data) - - if self.instance: - logger.info(f"Segment {self.instance.id} has count of conditions {count}") - - if count > settings.SEGMENT_RULES_CONDITIONS_LIMIT: - raise ValidationError( - { - "segment": f"The segment has {count} conditions, which exceeds the maximum " - f"condition count of {settings.SEGMENT_RULES_CONDITIONS_LIMIT}." - } - ) - - def _calculate_condition_count( - self, - rules_data: dict[str, object], - ) -> None: - count: int = 0 - - for rule_data in rules_data: - child_rules = rule_data.get("rules", []) - if child_rules: - count += self._calculate_condition_count(child_rules) - conditions = rule_data.get("conditions", []) - for condition in conditions: - if condition.get("delete", False) is True: - continue - count += 1 - return count - - def _update_segment_rules(self, rules_data, segment=None): - """ - Since we don't have a unique identifier for the rules / conditions for the update, we assume that the client - passes up the new configuration for the rules of the segment and simply wipe the old ones and create new ones - """ - # traverse the rules / conditions tree - if no ids are provided, then maintain the previous behaviour (clear - # existing rules and create the ones that were sent) - # note: we do this to preserve backwards compatibility after adding logic to include the id in requests - if not Segment.id_exists_in_rules_data(rules_data): - segment.rules.set([]) - - self._update_or_create_segment_rules(rules_data, segment=segment) - - def _update_or_create_segment_rules( - self, rules_data, segment=None, rule=None, is_create: bool = False - ): - if all(x is None for x in {segment, rule}): - raise RuntimeError("Can't create rule without parent segment or rule") - - for rule_data in rules_data: - child_rules = rule_data.pop("rules", []) - conditions = rule_data.pop("conditions", []) - - child_rule = self._update_or_create_segment_rule( - rule_data, segment=segment, rule=rule - ) - if not child_rule: - # child rule was deleted - continue - - self._update_or_create_conditions( - conditions, child_rule, is_create=is_create - ) - - self._update_or_create_segment_rules( - child_rules, rule=child_rule, is_create=is_create - ) - - def _update_or_create_metadata( - self, metadata_data: typing.Dict, segment: typing.Optional[Segment] = None - ) -> None: - if len(metadata_data) == 0: - Metadata.objects.filter(object_id=segment.id).delete() - return - if metadata_data is not None: - for metadata_item in metadata_data: - metadata_model_field = metadata_item.pop("model_field", None) - if metadata_item.get("delete"): - Metadata.objects.filter(model_field=metadata_model_field).delete() - continue - - Metadata.objects.update_or_create( - model_field=metadata_model_field, - defaults={ - **metadata_item, - "content_type": ContentType.objects.get_for_model(Segment), - "object_id": segment.id, - }, - ) - - @staticmethod - def _update_or_create_segment_rule( - rule_data: dict, segment: Segment = None, rule: SegmentRule = None - ) -> typing.Optional[SegmentRule]: - rule_id = rule_data.pop("id", None) - if rule_data.get("delete"): - SegmentRule.objects.filter(id=rule_id).delete() - return - - segment_rule, _ = SegmentRule.objects.update_or_create( - id=rule_id, defaults={"segment": segment, "rule": rule, **rule_data} - ) - return segment_rule - - @staticmethod - def _update_or_create_conditions(conditions_data, rule, is_create: bool = False): - for condition in conditions_data: - condition_id = condition.pop("id", None) - if condition.get("delete"): - Condition.objects.filter(id=condition_id).delete() - continue - Condition.objects.update_or_create( - id=condition_id, - defaults={**condition, "created_with_segment": is_create, "rule": rule}, - ) +from segments.models import Segment class SegmentSerializerBasic(serializers.ModelSerializer): diff --git a/api/segments/views.py b/api/segments/views.py index c64e949d529a..c740ef90e6c4 100644 --- a/api/segments/views.py +++ b/api/segments/views.py @@ -1,5 +1,7 @@ import logging +from common.projects.permissions import VIEW_PROJECT +from common.segments.serializers import SegmentSerializer from django.utils.decorators import method_decorator from drf_yasg.utils import swagger_auto_schema from rest_framework import viewsets @@ -17,11 +19,10 @@ SegmentAssociatedFeatureStateSerializer, ) from features.versioning.models import EnvironmentFeatureVersion -from projects.permissions import VIEW_PROJECT from .models import Segment from .permissions import SegmentPermissions -from .serializers import SegmentListQuerySerializer, SegmentSerializer +from .serializers import SegmentListQuerySerializer logger = logging.getLogger() @@ -44,7 +45,7 @@ def get_queryset(self): ) project = get_object_or_404(permitted_projects, pk=self.kwargs["project_pk"]) - queryset = project.segments.all() + queryset = Segment.live_objects.filter(project=project) if self.action == "list": # TODO: at the moment, the UI only shows the name and description of the segment in the list view. @@ -121,7 +122,7 @@ def associated_features(self, request, *args, **kwargs): @api_view(["GET"]) def get_segment_by_uuid(request, uuid): accessible_projects = request.user.get_permitted_projects(VIEW_PROJECT) - qs = Segment.objects.filter(project__in=accessible_projects) + qs = Segment.live_objects.filter(project__in=accessible_projects) segment = get_object_or_404(qs, uuid=uuid) serializer = SegmentSerializer(instance=segment) return Response(serializer.data) diff --git a/api/tests/unit/audit/conftest.py b/api/tests/unit/audit/conftest.py index 93b0741394a4..605341072cc4 100644 --- a/api/tests/unit/audit/conftest.py +++ b/api/tests/unit/audit/conftest.py @@ -1,12 +1,12 @@ import typing import pytest as pytest +from common.projects.permissions import VIEW_AUDIT_LOG from django.db.models import Model from organisations.models import OrganisationRole from permissions.models import PermissionModel from projects.models import Project, UserProjectPermission -from projects.permissions import VIEW_AUDIT_LOG @pytest.fixture() diff --git a/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py b/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py index 6aaea120e978..8f0d3c5c1a87 100644 --- a/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py +++ b/api/tests/unit/edge_api/identities/test_edge_api_identities_views.py @@ -1,3 +1,8 @@ +from common.environments.permissions import ( + MANAGE_IDENTITIES, + VIEW_ENVIRONMENT, + VIEW_IDENTITIES, +) from django.urls import reverse from pytest_mock import MockerFixture from rest_framework import status @@ -7,11 +12,6 @@ from edge_api.identities.models import EdgeIdentity from edge_api.identities.views import EdgeIdentityViewSet from environments.models import Environment -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_ENVIRONMENT, - VIEW_IDENTITIES, -) from environments.permissions.permissions import NestedEnvironmentPermissions from features.models import Feature from tests.types import WithEnvironmentPermissionsCallable diff --git a/api/tests/unit/edge_api/identities/test_permissions.py b/api/tests/unit/edge_api/identities/test_permissions.py index 32361dcfa4ee..6bb7c4de592d 100644 --- a/api/tests/unit/edge_api/identities/test_permissions.py +++ b/api/tests/unit/edge_api/identities/test_permissions.py @@ -1,7 +1,8 @@ +from common.environments.permissions import UPDATE_FEATURE_STATE + from edge_api.identities.permissions import ( EdgeIdentityWithIdentifierViewPermissions, ) -from environments.permissions.constants import UPDATE_FEATURE_STATE def test_edge_identity_with_identifier_view_permissions_has_permissions_calls_has_environment_permission( diff --git a/api/tests/unit/environments/helpers.py b/api/tests/unit/environments/helpers.py index bf55c8ffab51..c6e89569c79b 100644 --- a/api/tests/unit/environments/helpers.py +++ b/api/tests/unit/environments/helpers.py @@ -1,5 +1,6 @@ import typing +from common.projects.permissions import VIEW_PROJECT from rest_framework.test import APIClient from environments.models import Environment @@ -8,7 +9,6 @@ UserEnvironmentPermission, ) from projects.models import ProjectPermissionModel, UserProjectPermission -from projects.permissions import VIEW_PROJECT from users.models import FFAdminUser diff --git a/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py b/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py index a2d3c3185c03..eae6add8bcd5 100644 --- a/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py +++ b/api/tests/unit/environments/identities/test_unit_identities_feature_states_views.py @@ -1,6 +1,10 @@ import json import pytest +from common.environments.permissions import ( + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) from core.constants import STRING from django.test import Client from django.urls import reverse @@ -8,10 +12,6 @@ from environments.identities.models import Identity from environments.models import Environment -from environments.permissions.constants import ( - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, -) from features.models import Feature, FeatureState, FeatureStateValue from features.multivariate.models import ( MultivariateFeatureOption, diff --git a/api/tests/unit/environments/identities/test_unit_identities_views.py b/api/tests/unit/environments/identities/test_unit_identities_views.py index b1e31cdb50b2..e62c12dbf88e 100644 --- a/api/tests/unit/environments/identities/test_unit_identities_views.py +++ b/api/tests/unit/environments/identities/test_unit_identities_views.py @@ -4,6 +4,7 @@ from unittest import mock import pytest +from common.environments.permissions import MANAGE_IDENTITIES, VIEW_IDENTITIES from core.constants import FLAGSMITH_UPDATED_AT_HEADER, STRING from django.test import override_settings from django.urls import reverse @@ -21,10 +22,6 @@ from environments.identities.traits.models import Trait from environments.identities.views import IdentityViewSet from environments.models import Environment, EnvironmentAPIKey -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_IDENTITIES, -) from environments.permissions.permissions import NestedEnvironmentPermissions from features.models import Feature, FeatureSegment, FeatureState from integrations.amplitude.models import AmplitudeConfiguration diff --git a/api/tests/unit/environments/identities/traits/test_traits_views.py b/api/tests/unit/environments/identities/traits/test_traits_views.py index 321b76915463..61831fe93871 100644 --- a/api/tests/unit/environments/identities/traits/test_traits_views.py +++ b/api/tests/unit/environments/identities/traits/test_traits_views.py @@ -1,6 +1,12 @@ import json from unittest import mock +from common.environments.permissions import ( + MANAGE_IDENTITIES, + VIEW_ENVIRONMENT, + VIEW_IDENTITIES, +) +from common.projects.permissions import VIEW_PROJECT from core.constants import INTEGER, STRING from django.test import override_settings from django.urls import reverse @@ -15,17 +21,11 @@ from environments.identities.traits.models import Trait from environments.identities.traits.views import TraitViewSet from environments.models import Environment, EnvironmentAPIKey -from environments.permissions.constants import ( - MANAGE_IDENTITIES, - VIEW_ENVIRONMENT, - VIEW_IDENTITIES, -) from environments.permissions.models import UserEnvironmentPermission from environments.permissions.permissions import NestedEnvironmentPermissions from organisations.models import Organisation from permissions.models import PermissionModel from projects.models import Project, UserProjectPermission -from projects.permissions import VIEW_PROJECT def test_can_set_trait_for_an_identity( diff --git a/api/tests/unit/environments/permissions/test_unit_environments_permissions.py b/api/tests/unit/environments/permissions/test_unit_environments_permissions.py index 8c36ea8cabe5..f1fbcddbbf45 100644 --- a/api/tests/unit/environments/permissions/test_unit_environments_permissions.py +++ b/api/tests/unit/environments/permissions/test_unit_environments_permissions.py @@ -1,5 +1,7 @@ from unittest import mock +from common.projects.permissions import CREATE_ENVIRONMENT + from environments.identities.models import Identity from environments.models import Environment from environments.permissions.models import UserEnvironmentPermission @@ -13,7 +15,6 @@ ProjectPermissionModel, UserProjectPermission, ) -from projects.permissions import CREATE_ENVIRONMENT from users.models import FFAdminUser mock_view = mock.MagicMock() diff --git a/api/tests/unit/environments/permissions/test_unit_environments_permissions_migrations.py b/api/tests/unit/environments/permissions/test_unit_environments_permissions_migrations.py index 6a6a33b726e9..3cc3b0b557f6 100644 --- a/api/tests/unit/environments/permissions/test_unit_environments_permissions_migrations.py +++ b/api/tests/unit/environments/permissions/test_unit_environments_permissions_migrations.py @@ -1,13 +1,12 @@ import pytest -from django.conf import settings - -from environments.permissions.constants import ( +from common.environments.permissions import ( APPROVE_CHANGE_REQUEST, CREATE_CHANGE_REQUEST, MANAGE_IDENTITIES, UPDATE_FEATURE_STATE, VIEW_IDENTITIES, ) +from django.conf import settings if settings.SKIP_MIGRATION_TESTS is True: pytest.skip( diff --git a/api/tests/unit/environments/permissions/test_unit_environments_views.py b/api/tests/unit/environments/permissions/test_unit_environments_views.py index 9001397f57d5..78e5847f1fac 100644 --- a/api/tests/unit/environments/permissions/test_unit_environments_views.py +++ b/api/tests/unit/environments/permissions/test_unit_environments_views.py @@ -1,12 +1,12 @@ import json import pytest +from common.environments.permissions import VIEW_ENVIRONMENT from django.urls import reverse from rest_framework import status from rest_framework.test import APIClient from environments.models import Environment -from environments.permissions.constants import VIEW_ENVIRONMENT from environments.permissions.models import ( EnvironmentPermissionModel, UserEnvironmentPermission, diff --git a/api/tests/unit/environments/test_unit_environments_feature_states_views.py b/api/tests/unit/environments/test_unit_environments_feature_states_views.py index ec335dcf8cb7..205c189ab899 100644 --- a/api/tests/unit/environments/test_unit_environments_feature_states_views.py +++ b/api/tests/unit/environments/test_unit_environments_feature_states_views.py @@ -1,13 +1,13 @@ import json import pytest -from django.urls import reverse -from rest_framework import status - -from environments.permissions.constants import ( +from common.environments.permissions import ( UPDATE_FEATURE_STATE, VIEW_ENVIRONMENT, ) +from django.urls import reverse +from rest_framework import status + from tests.unit.environments.helpers import get_environment_user_client diff --git a/api/tests/unit/environments/test_unit_environments_views.py b/api/tests/unit/environments/test_unit_environments_views.py index d3b84a1d2880..bbe4102330f7 100644 --- a/api/tests/unit/environments/test_unit_environments_views.py +++ b/api/tests/unit/environments/test_unit_environments_views.py @@ -2,6 +2,11 @@ from unittest import mock import pytest +from common.environments.permissions import ( + TAG_SUPPORTED_PERMISSIONS, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import CREATE_ENVIRONMENT from core.constants import STRING from django.conf import settings from django.contrib.contenttypes.models import ContentType @@ -18,17 +23,12 @@ from environments.identities.models import Identity from environments.identities.traits.models import Trait from environments.models import Environment, EnvironmentAPIKey, Webhook -from environments.permissions.constants import ( - TAG_SUPPORTED_PERMISSIONS, - VIEW_ENVIRONMENT, -) from environments.permissions.models import UserEnvironmentPermission from features.models import Feature, FeatureState from features.versioning.models import EnvironmentFeatureVersion from metadata.models import Metadata, MetadataModelField from organisations.models import Organisation from projects.models import Project -from projects.permissions import CREATE_ENVIRONMENT from segments.models import Condition, Segment, SegmentRule from tests.types import WithEnvironmentPermissionsCallable from users.models import FFAdminUser diff --git a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py index c14b976d5bcd..6726b67e3840 100644 --- a/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py +++ b/api/tests/unit/environments/test_unit_environments_views_sdk_environment.py @@ -17,24 +17,27 @@ FeatureStateValue, ) from features.multivariate.models import MultivariateFeatureOption +from projects.models import Project from segments.models import Condition, Segment, SegmentRule if TYPE_CHECKING: from pytest_django import DjangoAssertNumQueries from organisations.models import Organisation - from projects.models import Project def test_get_environment_document( organisation_one: "Organisation", + organisation_two: "Organisation", organisation_one_project_one: "Project", django_assert_num_queries: "DjangoAssertNumQueries", ) -> None: # Given project = organisation_one_project_one + project2 = Project.objects.create( + name="standin_project", organisation=organisation_two + ) - # an environment environment = Environment.objects.create(name="Test Environment", project=project) api_key = EnvironmentAPIKey.objects.create(environment=environment) client = APIClient() @@ -44,6 +47,21 @@ def test_get_environment_document( feature = Feature.objects.create(name="test_feature", project=project) for i in range(10): segment = Segment.objects.create(project=project) + + # Create a shallow clone which should not be returned in the document. + segment.shallow_clone( + name=f"disregarded-clone-{i}", + description=f"some-disregarded-clone-{i}", + change_request=None, + ) + + # Create some other segments to ensure that the segments manager was + # properly set. + Segment.objects.create( + project=project2, + name=f"standin_segment{i}", + description=f"Should not be selected {i}", + ) segment_rule = SegmentRule.objects.create( segment=segment, type=SegmentRule.ALL_RULE ) @@ -114,6 +132,7 @@ def test_get_environment_document( # Then assert response.status_code == status.HTTP_200_OK assert response.json() + assert len(response.data["project"]["segments"]) == 10 assert response.headers[FLAGSMITH_UPDATED_AT_HEADER] == str( environment.updated_at.timestamp() ) diff --git a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py index f6a68460015e..0e984f2ae543 100644 --- a/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py +++ b/api/tests/unit/features/feature_segments/test_unit_feature_segments_views.py @@ -1,6 +1,12 @@ import json import pytest +from common.environments.permissions import ( + MANAGE_SEGMENT_OVERRIDES, + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import VIEW_PROJECT from django.conf import settings from django.urls import reverse from pytest_django import DjangoAssertNumQueries @@ -12,15 +18,9 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from environments.models import Environment -from environments.permissions.constants import ( - MANAGE_SEGMENT_OVERRIDES, - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, -) from features.models import Feature, FeatureSegment, FeatureState from features.versioning.models import EnvironmentFeatureVersion from projects.models import Project, UserProjectPermission -from projects.permissions import VIEW_PROJECT from segments.models import Segment from tests.types import ( WithEnvironmentPermissionsCallable, diff --git a/api/tests/unit/features/import_export/test_unit_features_import_export_views.py b/api/tests/unit/features/import_export/test_unit_features_import_export_views.py index 7c45edbd4580..55dd5551674d 100644 --- a/api/tests/unit/features/import_export/test_unit_features_import_export_views.py +++ b/api/tests/unit/features/import_export/test_unit_features_import_export_views.py @@ -1,6 +1,7 @@ import json import pytest +from common.projects.permissions import VIEW_PROJECT from django.core.files.uploadedfile import SimpleUploadedFile from django.urls import reverse from pytest_django.fixtures import SettingsWrapper @@ -16,7 +17,6 @@ FlagsmithOnFlagsmithFeatureExport, ) from projects.models import Project -from projects.permissions import VIEW_PROJECT from projects.tags.models import Tag from tests.types import WithProjectPermissionsCallable from users.models import FFAdminUser diff --git a/api/tests/unit/features/multivariate/test_unit_multivariate_views.py b/api/tests/unit/features/multivariate/test_unit_multivariate_views.py index 89f2b355b6c8..84b7c53294cf 100644 --- a/api/tests/unit/features/multivariate/test_unit_multivariate_views.py +++ b/api/tests/unit/features/multivariate/test_unit_multivariate_views.py @@ -1,16 +1,13 @@ import uuid import pytest +from common.projects.permissions import CREATE_FEATURE, VIEW_PROJECT from django.urls import reverse from pytest_lazyfixture import lazy_fixture from rest_framework import status from features.multivariate.views import MultivariateFeatureOptionViewSet -from projects.permissions import ( - CREATE_FEATURE, - VIEW_PROJECT, - NestedProjectPermissions, -) +from projects.permissions import NestedProjectPermissions def test_multivariate_feature_options_view_set_get_permissions(): diff --git a/api/tests/unit/features/test_unit_feature_external_resources_views.py b/api/tests/unit/features/test_unit_feature_external_resources_views.py index 370c25ec1bd9..8d2af473f346 100644 --- a/api/tests/unit/features/test_unit_feature_external_resources_views.py +++ b/api/tests/unit/features/test_unit_feature_external_resources_views.py @@ -4,6 +4,7 @@ import pytest import responses import simplejson as json +from common.environments.permissions import UPDATE_FEATURE_STATE from django.core.serializers.json import DjangoJSONEncoder from django.urls import reverse from django.utils.formats import get_format @@ -12,7 +13,6 @@ from rest_framework.test import APIClient from environments.models import Environment -from environments.permissions.constants import UPDATE_FEATURE_STATE from features.feature_external_resources.models import FeatureExternalResource from features.models import Feature, FeatureSegment, FeatureState from features.serializers import ( diff --git a/api/tests/unit/features/test_unit_features_permissions.py b/api/tests/unit/features/test_unit_features_permissions.py index e1dc4f864e64..46f437bbaa40 100644 --- a/api/tests/unit/features/test_unit_features_permissions.py +++ b/api/tests/unit/features/test_unit_features_permissions.py @@ -2,6 +2,11 @@ from unittest.mock import MagicMock import pytest +from common.projects.permissions import ( + CREATE_FEATURE, + DELETE_FEATURE, + VIEW_PROJECT, +) from features.models import Feature from features.permissions import FeaturePermissions @@ -12,12 +17,7 @@ UserPermissionGroupProjectPermission, UserProjectPermission, ) -from projects.permissions import ( - CREATE_FEATURE, - DELETE_FEATURE, - VIEW_PROJECT, - NestedProjectPermissions, -) +from projects.permissions import NestedProjectPermissions from tests.types import WithProjectPermissionsCallable from users.models import FFAdminUser, UserPermissionGroup diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 02705c03c8ef..248ecf656d3d 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -6,6 +6,12 @@ import pytest import pytz from app_analytics.dataclasses import FeatureEvaluationData +from common.environments.permissions import ( + MANAGE_SEGMENT_OVERRIDES, + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import CREATE_FEATURE, VIEW_PROJECT from core.constants import FLAGSMITH_UPDATED_AT_HEADER from django.conf import settings from django.forms import model_to_dict @@ -27,11 +33,6 @@ from audit.models import AuditLog, RelatedObjectType from environments.identities.models import Identity from environments.models import Environment, EnvironmentAPIKey -from environments.permissions.constants import ( - MANAGE_SEGMENT_OVERRIDES, - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, -) from environments.permissions.models import UserEnvironmentPermission from features.feature_types import MULTIVARIATE from features.models import Feature, FeatureSegment, FeatureState @@ -41,7 +42,6 @@ from metadata.models import MetadataModelField from organisations.models import Organisation, OrganisationRole from projects.models import Project, UserProjectPermission -from projects.permissions import CREATE_FEATURE, VIEW_PROJECT from projects.tags.models import Tag from segments.models import Segment from tests.types import ( diff --git a/api/tests/unit/features/versioning/test_unit_versioning_views.py b/api/tests/unit/features/versioning/test_unit_versioning_views.py index e0da742be6c0..7eba620a1b06 100644 --- a/api/tests/unit/features/versioning/test_unit_versioning_views.py +++ b/api/tests/unit/features/versioning/test_unit_versioning_views.py @@ -3,6 +3,11 @@ from datetime import datetime, timedelta import pytest +from common.environments.permissions import ( + UPDATE_FEATURE_STATE, + VIEW_ENVIRONMENT, +) +from common.projects.permissions import VIEW_PROJECT from core.constants import STRING from django.urls import reverse from django.utils import timezone @@ -17,10 +22,6 @@ from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from environments.models import Environment -from environments.permissions.constants import ( - UPDATE_FEATURE_STATE, - VIEW_ENVIRONMENT, -) from features.feature_segments.limits import ( SEGMENT_OVERRIDE_LIMIT_EXCEEDED_MESSAGE, ) @@ -34,7 +35,6 @@ ) from organisations.subscriptions.constants import SubscriptionPlanFamily from projects.models import Project -from projects.permissions import VIEW_PROJECT from segments.models import Segment from tests.types import ( WithEnvironmentPermissionsCallable, diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_migrations.py b/api/tests/unit/features/workflows/core/test_unit_workflows_migrations.py new file mode 100644 index 000000000000..38736946e5a8 --- /dev/null +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_migrations.py @@ -0,0 +1,40 @@ +import pytest +from django.conf import settings +from django_test_migrations.migrator import Migrator + + +@pytest.mark.skipif( + settings.SKIP_MIGRATION_TESTS is True, + reason="Skip migration tests to speed up tests where necessary", +) +def test_migrate_add_project_to_change_request(migrator: Migrator) -> None: + old_state = migrator.apply_initial_migration( + ("workflows_core", "0010_add_ignore_conflicts_option"), + ) + OldOrganisation = old_state.apps.get_model("organisations", "Organisation") + OldProject = old_state.apps.get_model("projects", "Project") + OldEnvironment = old_state.apps.get_model("environments", "Environment") + OldFFAdminUser = old_state.apps.get_model("users", "FFAdminUser") + OldChangeRequest = old_state.apps.get_model("workflows_core", "ChangeRequest") + + organisation = OldOrganisation.objects.create(name="Test Org") + project = OldProject.objects.create(name="Test Project", organisation=organisation) + environment = OldEnvironment.objects.create( + name="Test Environment", project=project + ) + user = OldFFAdminUser.objects.create(email="staff@example.co") + change_request = OldChangeRequest.objects.create( + environment=environment, title="Test CR", user_id=user.id + ) + + assert hasattr(change_request, "project") is False + + # When + new_state = migrator.apply_tested_migration( + ("workflows_core", "0011_add_project_to_change_requests") + ) + + # Then + NewChangeRequest = new_state.apps.get_model("workflows_core", "ChangeRequest") + new_change_request = NewChangeRequest.objects.get(id=change_request.id) + assert new_change_request.project_id == project.id diff --git a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py index 447394c2793c..37edd3e7059a 100644 --- a/api/tests/unit/features/workflows/core/test_unit_workflows_models.py +++ b/api/tests/unit/features/workflows/core/test_unit_workflows_models.py @@ -3,10 +3,11 @@ import freezegun import pytest +from core.helpers import get_current_site_url from django.contrib.sites.models import Site from django.db.models import Q from django.utils import timezone -from flag_engine.segments.constants import PERCENTAGE_SPLIT +from flag_engine.segments.constants import EQUAL, PERCENTAGE_SPLIT from freezegun.api import FrozenDateTimeFactory from pytest_mock import MockerFixture @@ -744,6 +745,26 @@ def test_committing_change_request_with_environment_feature_versions_creates_pub ).exists() +def test_retrieving_segments( + change_request: ChangeRequest, +) -> None: + # Given + base_segment = Segment.objects.create( + name="Base Segment", + description="Segment description", + project=change_request.environment.project, + ) + + # When + segment = base_segment.shallow_clone( + name="New Name", description="New description", change_request=change_request + ) + + # Then + assert change_request.segments.count() == 1 + assert change_request.segments.first() == segment + + def test_change_request_live_from_for_change_request_with_change_set( feature: Feature, environment_v2_versioning: Environment, @@ -781,6 +802,65 @@ def test_change_request_live_from_for_change_request_with_change_set( assert change_request.live_from == now +def test_publishing_segments_as_part_of_commit( + segment: Segment, + change_request: ChangeRequest, + admin_user: FFAdminUser, +) -> None: + # Given + assert segment.version == 2 + cr_segment = segment.shallow_clone("Test Name", "Test Description", change_request) + assert cr_segment.rules.count() == 0 + + # Add some rules that the original segment will be cloning from + parent_rule = SegmentRule.objects.create( + segment=cr_segment, type=SegmentRule.ALL_RULE + ) + + child_rule1 = SegmentRule.objects.create( + rule=parent_rule, type=SegmentRule.ANY_RULE + ) + child_rule2 = SegmentRule.objects.create( + rule=parent_rule, type=SegmentRule.NONE_RULE + ) + Condition.objects.create( + rule=child_rule1, + property="child_rule1", + operator=EQUAL, + value="condition1", + created_with_segment=True, + ) + Condition.objects.create( + rule=child_rule2, + property="child_rule2", + operator=PERCENTAGE_SPLIT, + value="0.2", + created_with_segment=False, + ) + + # When + change_request.commit(admin_user) + + # Then + segment.refresh_from_db() + assert segment.version == 3 + assert segment.name == "Test Name" + assert segment.description == "Test Description" + assert segment.rules.count() == 1 + parent_rule2 = segment.rules.first() + assert parent_rule2.type == SegmentRule.ALL_RULE + assert parent_rule2.rules.count() == 2 + child_rule3, child_rule4 = list(parent_rule2.rules.all()) + assert child_rule3.type == SegmentRule.ANY_RULE + assert child_rule4.type == SegmentRule.NONE_RULE + assert child_rule3.conditions.count() == 1 + assert child_rule4.conditions.count() == 1 + condition1 = child_rule3.conditions.first() + condition2 = child_rule4.conditions.first() + assert condition1.value == "condition1" + assert condition2.value == "0.2" + + def test_ignore_conflicts_for_multiple_scheduled_change_requests( feature: Feature, environment_v2_versioning: Environment, @@ -897,3 +977,31 @@ def _create_segment(percentage_value: int) -> Segment: ) assert len(after_cr_1_flags) == 1 assert after_cr_1_flags[0].feature_segment.segment == twenty_percent_segment + + +def test_approval_via_project(project_change_request: ChangeRequest) -> None: + # Given - The project change request fixture + assert project_change_request.environment is None + assert project_change_request.project.minimum_change_request_approvals is None + + # When + is_approved = project_change_request.is_approved() + + # Then + assert is_approved is True + + +def test_url_via_project(project_change_request: ChangeRequest) -> None: + # Given + assert project_change_request.environment is None + + # When + url = project_change_request.url + + # Then + project_id = project_change_request.project_id + expected_url = get_current_site_url() + expected_url += ( + f"/projects/{project_id}/change-requests/{project_change_request.id}" + ) + assert url == expected_url diff --git a/api/tests/unit/metadata/test_serializers.py b/api/tests/unit/metadata/test_serializers.py index 3603288943bb..eb0ee7f4434c 100644 --- a/api/tests/unit/metadata/test_serializers.py +++ b/api/tests/unit/metadata/test_serializers.py @@ -1,11 +1,11 @@ import pytest +from common.metadata.serializers import MetadataSerializer from metadata.models import ( FIELD_VALUE_MAX_LENGTH, MetadataField, MetadataModelField, ) -from metadata.serializers import MetadataSerializer @pytest.mark.parametrize( diff --git a/api/tests/unit/permissions/permission_service/test_get_permitted_environments_for_user.py b/api/tests/unit/permissions/permission_service/test_get_permitted_environments_for_user.py index 0383b98d7e98..6e2a5d442f50 100644 --- a/api/tests/unit/permissions/permission_service/test_get_permitted_environments_for_user.py +++ b/api/tests/unit/permissions/permission_service/test_get_permitted_environments_for_user.py @@ -1,11 +1,11 @@ import pytest -from pytest_lazyfixture import lazy_fixture - -from environments.permissions.constants import ( +from common.environments.permissions import ( MANAGE_IDENTITIES, UPDATE_FEATURE_STATE, VIEW_ENVIRONMENT, ) +from pytest_lazyfixture import lazy_fixture + from environments.permissions.models import EnvironmentPermissionModel from permissions.permission_service import get_permitted_environments_for_user diff --git a/api/tests/unit/permissions/permission_service/test_get_permitted_projects_for_user.py b/api/tests/unit/permissions/permission_service/test_get_permitted_projects_for_user.py index 2af30708882c..5820fde1bed2 100644 --- a/api/tests/unit/permissions/permission_service/test_get_permitted_projects_for_user.py +++ b/api/tests/unit/permissions/permission_service/test_get_permitted_projects_for_user.py @@ -1,13 +1,13 @@ import pytest -from pytest_lazyfixture import lazy_fixture - -from permissions.permission_service import get_permitted_projects_for_user -from projects.models import ProjectPermissionModel -from projects.permissions import ( +from common.projects.permissions import ( CREATE_ENVIRONMENT, DELETE_FEATURE, VIEW_PROJECT, ) +from pytest_lazyfixture import lazy_fixture + +from permissions.permission_service import get_permitted_projects_for_user +from projects.models import ProjectPermissionModel def test_get_permitted_projects_for_user_returns_all_projects_for_org_admin( diff --git a/api/tests/unit/permissions/test_unit_permissions_calculator.py b/api/tests/unit/permissions/test_unit_permissions_calculator.py index 76a240d6dfa7..cd89119b03de 100644 --- a/api/tests/unit/permissions/test_unit_permissions_calculator.py +++ b/api/tests/unit/permissions/test_unit_permissions_calculator.py @@ -1,9 +1,10 @@ import pytest - -from environments.permissions.constants import ( +from common.environments.permissions import ( UPDATE_FEATURE_STATE, VIEW_ENVIRONMENT, ) +from common.projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT + from environments.permissions.models import ( EnvironmentPermissionModel, UserEnvironmentPermission, @@ -29,7 +30,6 @@ UserPermissionGroupProjectPermission, UserProjectPermission, ) -from projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT from users.models import UserPermissionGroup diff --git a/api/tests/unit/projects/tags/test_unit_projects_tags_permissions.py b/api/tests/unit/projects/tags/test_unit_projects_tags_permissions.py index e79c96828e1f..46121e7c4d28 100644 --- a/api/tests/unit/projects/tags/test_unit_projects_tags_permissions.py +++ b/api/tests/unit/projects/tags/test_unit_projects_tags_permissions.py @@ -1,7 +1,8 @@ from unittest import mock +from common.projects.permissions import MANAGE_TAGS, VIEW_PROJECT + from projects.models import Project -from projects.permissions import MANAGE_TAGS, VIEW_PROJECT from projects.tags.models import Tag from projects.tags.permissions import TagPermissions from tests.types import WithProjectPermissionsCallable diff --git a/api/tests/unit/projects/tags/test_unit_projects_tags_views.py b/api/tests/unit/projects/tags/test_unit_projects_tags_views.py index d1942cdbb6f6..f278e6821ca1 100644 --- a/api/tests/unit/projects/tags/test_unit_projects_tags_views.py +++ b/api/tests/unit/projects/tags/test_unit_projects_tags_views.py @@ -1,13 +1,13 @@ import json import pytest +from common.projects.permissions import VIEW_PROJECT from django.urls import reverse from pytest_lazyfixture import lazy_fixture from rest_framework import status from rest_framework.test import APIClient from projects.models import Project -from projects.permissions import VIEW_PROJECT from projects.tags.models import Tag from tests.types import WithProjectPermissionsCallable diff --git a/api/tests/unit/projects/test_migrations.py b/api/tests/unit/projects/test_migrations.py index 23c51b363c6d..20f3ef4b19dd 100644 --- a/api/tests/unit/projects/test_migrations.py +++ b/api/tests/unit/projects/test_migrations.py @@ -1,8 +1,7 @@ import pytest +from common.projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT from django.conf import settings -from projects.permissions import CREATE_ENVIRONMENT, VIEW_PROJECT - @pytest.mark.skipif( settings.SKIP_MIGRATION_TESTS is True, diff --git a/api/tests/unit/projects/test_unit_projects_models.py b/api/tests/unit/projects/test_unit_projects_models.py index a377fa4bae10..6448e190e864 100644 --- a/api/tests/unit/projects/test_unit_projects_models.py +++ b/api/tests/unit/projects/test_unit_projects_models.py @@ -8,6 +8,7 @@ from organisations.models import Organisation from projects.models import EdgeV2MigrationStatus, Project +from segments.models import Segment now = timezone.now() tomorrow = now + timedelta(days=1) @@ -21,7 +22,7 @@ def test_get_segments_from_cache(project, monkeypatch): mock_project_segments_cache.get.return_value = None monkeypatch.setattr( - "projects.models.project_segments_cache", mock_project_segments_cache + "projects.services.project_segments_cache", mock_project_segments_cache ) # When @@ -41,7 +42,7 @@ def test_get_segments_from_cache_set_not_called(project, segments, monkeypatch): mock_project_segments_cache.get.return_value = project.segments.all() monkeypatch.setattr( - "projects.models.project_segments_cache", mock_project_segments_cache + "projects.services.project_segments_cache", mock_project_segments_cache ) # When @@ -55,6 +56,34 @@ def test_get_segments_from_cache_set_not_called(project, segments, monkeypatch): mock_project_segments_cache.set.assert_not_called() +def test_get_segments_from_cache_set_to_empty_list( + project: Project, + segment: Segment, + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Given + mock_project_segments_cache = mock.MagicMock() + mock_project_segments_cache.get.return_value = [] + + monkeypatch.setattr( + "projects.services.project_segments_cache", mock_project_segments_cache + ) + + # When + segments = project.get_segments_from_cache() + + # Then + # Since we're calling the live_objects manager in the method, + # only one copy of the segment should be returned, not the + # other versioned copy of the segment. + assert segments.count() == 1 + assert segments.first() == segment + + # And correct calls to cache are made + mock_project_segments_cache.get.assert_called_once_with(project.id) + mock_project_segments_cache.set.assert_called_once() + + @pytest.mark.parametrize( "edge_enabled, expected_enable_dynamo_db_value", ((True, True), (False, False)), diff --git a/api/tests/unit/projects/test_unit_projects_permissions.py b/api/tests/unit/projects/test_unit_projects_permissions.py index f7b874ab0b81..496aab1c0b68 100644 --- a/api/tests/unit/projects/test_unit_projects_permissions.py +++ b/api/tests/unit/projects/test_unit_projects_permissions.py @@ -2,17 +2,14 @@ from unittest import mock import pytest +from common.projects.permissions import VIEW_PROJECT from django.conf import settings from rest_framework.exceptions import APIException, PermissionDenied from organisations.models import Organisation, OrganisationRole from organisations.permissions.permissions import CREATE_PROJECT from projects.models import Project, UserPermissionGroupProjectPermission -from projects.permissions import ( - VIEW_PROJECT, - IsProjectAdmin, - ProjectPermissions, -) +from projects.permissions import IsProjectAdmin, ProjectPermissions from tests.types import ( WithOrganisationPermissionsCallable, WithProjectPermissionsCallable, diff --git a/api/tests/unit/projects/test_unit_projects_views.py b/api/tests/unit/projects/test_unit_projects_views.py index ed1d349e5a50..71f078356dbc 100644 --- a/api/tests/unit/projects/test_unit_projects_views.py +++ b/api/tests/unit/projects/test_unit_projects_views.py @@ -2,6 +2,12 @@ from datetime import timedelta import pytest +from common.projects.permissions import ( + CREATE_ENVIRONMENT, + CREATE_FEATURE, + TAG_SUPPORTED_PERMISSIONS, + VIEW_PROJECT, +) from django.urls import reverse from django.utils import timezone from pytest_django.fixtures import SettingsWrapper @@ -26,12 +32,6 @@ UserPermissionGroupProjectPermission, UserProjectPermission, ) -from projects.permissions import ( - CREATE_ENVIRONMENT, - CREATE_FEATURE, - TAG_SUPPORTED_PERMISSIONS, - VIEW_PROJECT, -) from segments.models import Segment from tests.types import WithProjectPermissionsCallable from users.models import FFAdminUser, UserPermissionGroup @@ -176,9 +176,8 @@ def test_can_list_project_permission(client: APIClient, project: Project) -> Non # Then assert response.status_code == status.HTTP_200_OK - assert ( - len(response.json()) == 7 - ) # hard code how many permissions we expect there to be + # Hard code how many permissions we expect there to be. + assert len(response.json()) == 9 returned_supported_permissions = [ permission["key"] diff --git a/api/tests/unit/segments/test_unit_segments_models.py b/api/tests/unit/segments/test_unit_segments_models.py index 351a2fc7cf90..60ae2750e110 100644 --- a/api/tests/unit/segments/test_unit_segments_models.py +++ b/api/tests/unit/segments/test_unit_segments_models.py @@ -336,10 +336,10 @@ def test_manager_returns_only_highest_version_of_segments( assert segment.version == 3 # When - queryset1 = Segment.objects.filter(id=cloned_segment.id) - queryset2 = Segment.all_objects.filter(id=cloned_segment.id) - queryset3 = Segment.objects.filter(id=segment.id) - queryset4 = Segment.all_objects.filter(id=segment.id) + queryset1 = Segment.live_objects.filter(id=cloned_segment.id) + queryset2 = Segment.objects.filter(id=cloned_segment.id) + queryset3 = Segment.live_objects.filter(id=segment.id) + queryset4 = Segment.objects.filter(id=segment.id) # Then assert not queryset1.exists() diff --git a/api/tests/unit/segments/test_unit_segments_permissions.py b/api/tests/unit/segments/test_unit_segments_permissions.py index b338c96b1d9f..5b6f92c8a1cc 100644 --- a/api/tests/unit/segments/test_unit_segments_permissions.py +++ b/api/tests/unit/segments/test_unit_segments_permissions.py @@ -1,11 +1,12 @@ import uuid from unittest import mock +from common.projects.permissions import VIEW_PROJECT + from environments.identities.models import Identity from environments.models import Environment from permissions.models import PermissionModel from projects.models import Project, UserProjectPermission -from projects.permissions import VIEW_PROJECT from segments.models import Segment from segments.permissions import SegmentPermissions from tests.types import ( diff --git a/api/tests/unit/segments/test_unit_segments_views.py b/api/tests/unit/segments/test_unit_segments_views.py index 0f8f3f58e2db..4e29c6d3f1e5 100644 --- a/api/tests/unit/segments/test_unit_segments_views.py +++ b/api/tests/unit/segments/test_unit_segments_views.py @@ -2,6 +2,7 @@ import random import pytest +from common.projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT from django.conf import settings from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType @@ -22,7 +23,6 @@ from features.versioning.models import EnvironmentFeatureVersion from metadata.models import Metadata, MetadataModelField from projects.models import Project -from projects.permissions import MANAGE_SEGMENTS, VIEW_PROJECT from segments.models import Condition, Segment, SegmentRule, WhitelistedSegment from tests.types import WithProjectPermissionsCallable from util.mappers import map_identity_to_identity_document @@ -689,7 +689,7 @@ def test_update_segment_versioned_segment( # Before updating the segment confirm pre-existing version count which is # automatically set by the fixture. - assert Segment.all_objects.filter(version_of=segment).count() == 2 + assert Segment.objects.filter(version_of=segment).count() == 2 new_condition_property = "foo2" new_condition_value = "bar" @@ -736,13 +736,11 @@ def test_update_segment_versioned_segment( assert response.status_code == status.HTTP_200_OK # Now verify that a new versioned segment has been set. - assert Segment.all_objects.filter(version_of=segment).count() == 3 + assert Segment.objects.filter(version_of=segment).count() == 3 # Now check the previously versioned segment to match former count of conditions. - versioned_segment = Segment.all_objects.filter( - version_of=segment, version=2 - ).first() + versioned_segment = Segment.objects.filter(version_of=segment, version=2).first() assert versioned_segment != segment assert versioned_segment.rules.count() == 1 versioned_rule = versioned_segment.rules.first() @@ -773,9 +771,7 @@ def test_update_segment_versioned_segment_with_thrown_exception( rule=nested_rule, property="foo", operator=EQUAL, value="bar" ) - assert ( - segment.version == 2 == Segment.all_objects.filter(version_of=segment).count() - ) + assert segment.version == 2 == Segment.objects.filter(version_of=segment).count() new_condition_property = "foo2" new_condition_value = "bar" @@ -826,9 +822,7 @@ def test_update_segment_versioned_segment_with_thrown_exception( segment.refresh_from_db() # Now verify that the version of the segment has not been changed. - assert ( - segment.version == 2 == Segment.all_objects.filter(version_of=segment).count() - ) + assert segment.version == 2 == Segment.objects.filter(version_of=segment).count() @pytest.mark.parametrize( diff --git a/api/tests/unit/users/test_unit_users_models.py b/api/tests/unit/users/test_unit_users_models.py index fcc17d4a8157..8d4dcf32889d 100644 --- a/api/tests/unit/users/test_unit_users_models.py +++ b/api/tests/unit/users/test_unit_users_models.py @@ -1,11 +1,11 @@ import pytest +from common.projects.permissions import VIEW_PROJECT from django.db.utils import IntegrityError from organisations.models import Organisation, OrganisationRole from organisations.permissions.models import UserOrganisationPermission from organisations.permissions.permissions import ORGANISATION_PERMISSIONS from projects.models import Project -from projects.permissions import VIEW_PROJECT from tests.types import WithProjectPermissionsCallable from users.models import FFAdminUser diff --git a/api/util/mappers/engine.py b/api/util/mappers/engine.py index 9e111d5748d0..411d4f0ffa3f 100644 --- a/api/util/mappers/engine.py +++ b/api/util/mappers/engine.py @@ -27,6 +27,7 @@ from environments.constants import IDENTITY_INTEGRATIONS_RELATION_NAMES from features.versioning.models import EnvironmentFeatureVersion +from segments.models import Segment if TYPE_CHECKING: # pragma: no cover from environments.identities.models import Identity, Trait @@ -40,7 +41,7 @@ from integrations.webhook.models import WebhookConfiguration from organisations.models import Organisation from projects.models import Project - from segments.models import Segment, SegmentRule + from segments.models import SegmentRule __all__ = ( @@ -194,7 +195,11 @@ def map_environment_to_engine( organisation: "Organisation" = project.organisation # Read relationships - grab all the data needed from the ORM here. - project_segments: List["Segment"] = project.segments.all() + + project_segments = [ + ps for ps in project.segments.all() if ps.id == ps.version_of_id + ] + project_segment_rules_by_segment_id: Dict[ int, Iterable["SegmentRule"],