From f62b5edf369ae2abc7288d3fb97cee615e2d347a Mon Sep 17 00:00:00 2001 From: Surbhi Date: Wed, 18 Dec 2024 17:02:24 -0500 Subject: [PATCH 1/5] access control capture and test --- posthog/api/team.py | 19 ++++++++++++++- posthog/api/test/test_team.py | 44 +++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/posthog/api/team.py b/posthog/api/team.py index d2b9ca018dbdf..ad0d4d82edd64 100644 --- a/posthog/api/team.py +++ b/posthog/api/team.py @@ -14,7 +14,7 @@ from posthog.api.utils import action from posthog.auth import PersonalAPIKeyAuthentication from posthog.constants import AvailableFeature -from posthog.event_usage import report_user_action +from posthog.event_usage import report_user_action, groups from posthog.geoip import get_geoip_properties from posthog.jwt import PosthogJwtAudience, encode_jwt from posthog.models import ProductIntent, Team, User @@ -52,6 +52,7 @@ get_ip_address, get_week_start_for_country_code, ) +import posthoganalytics class PremiumMultiProjectPermissions(BasePermission): # TODO: Rename to include "Env" in name @@ -373,6 +374,22 @@ def create(self, validated_data: dict[str, Any], **kwargs) -> Team: def update(self, instance: Team, validated_data: dict[str, Any]) -> Team: before_update = instance.__dict__.copy() + if "access_control" in validated_data and validated_data["access_control"] != instance.access_control: + user = cast(User, self.context["request"].user) + posthoganalytics.capture( + str(user.distinct_id), + "project access control toggled", + properties={ + "enabled": validated_data["access_control"], + "project_id": str(instance.id), + "project_name": instance.name, + "organization_id": str(instance.organization_id), + "organization_name": instance.organization.name, + "user_role": user.organization_memberships.get(organization=instance.organization).level, + }, + groups=groups(instance.organization), + ) + if "survey_config" in validated_data: if instance.survey_config is not None and validated_data.get("survey_config") is not None: validated_data["survey_config"] = { diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 1da488504f57d..30256b3aee79f 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -28,6 +28,7 @@ from posthog.temporal.common.schedule import describe_schedule from posthog.test.base import APIBaseTest from posthog.utils import get_instance_realm +from posthog.event_usage import groups def team_api_test_factory(): @@ -1223,6 +1224,49 @@ def _patch_linked_flag_config( assert response.status_code == expected_status, response.json() return response + @patch("posthoganalytics.capture") + def test_access_control_toggle_capture(self, mock_capture): + self.organization_membership.level = OrganizationMembership.Level.ADMIN + self.organization_membership.save() + + mock_capture.reset_mock() + + response = self.client.patch(f"/api/environments/@current/", {"access_control": True}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + mock_capture.assert_called_with( + str(self.user.distinct_id), + "project access control toggled", + properties={ + "enabled": True, + "project_id": str(self.team.id), + "project_name": self.team.name, + "organization_id": str(self.organization.id), + "organization_name": self.organization.name, + "user_role": OrganizationMembership.Level.ADMIN, + }, + groups=groups(self.organization), + ) + + # Test toggling back to false + mock_capture.reset_mock() + response = self.client.patch(f"/api/environments/@current/", {"access_control": False}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + mock_capture.assert_called_with( + str(self.user.distinct_id), + "project access control toggled", + properties={ + "enabled": False, + "project_id": str(self.team.id), + "project_name": self.team.name, + "organization_id": str(self.organization.id), + "organization_name": self.organization.name, + "user_role": OrganizationMembership.Level.ADMIN, + }, + groups=groups(self.organization), + ) + return TestTeamAPI From b6c64f7e02f1226190ef3784de3f5ca53345c4a4 Mon Sep 17 00:00:00 2001 From: Surbhi Date: Mon, 23 Dec 2024 10:33:58 -0500 Subject: [PATCH 2/5] debugging tests --- posthog/api/team.py | 3 ++- posthog/api/test/test_team.py | 27 ++++++--------------------- 2 files changed, 8 insertions(+), 22 deletions(-) diff --git a/posthog/api/team.py b/posthog/api/team.py index ad0d4d82edd64..8fc028cc65933 100644 --- a/posthog/api/team.py +++ b/posthog/api/team.py @@ -1,4 +1,6 @@ import json +import posthoganalytics + from datetime import UTC, datetime, timedelta from functools import cached_property from typing import Any, Optional, cast @@ -52,7 +54,6 @@ get_ip_address, get_week_start_for_country_code, ) -import posthoganalytics class PremiumMultiProjectPermissions(BasePermission): # TODO: Rename to include "Env" in name diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 30256b3aee79f..32c35f8f14b78 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -1228,36 +1228,21 @@ def _patch_linked_flag_config( def test_access_control_toggle_capture(self, mock_capture): self.organization_membership.level = OrganizationMembership.Level.ADMIN self.organization_membership.save() - mock_capture.reset_mock() - response = self.client.patch(f"/api/environments/@current/", {"access_control": True}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - mock_capture.assert_called_with( - str(self.user.distinct_id), - "project access control toggled", - properties={ - "enabled": True, - "project_id": str(self.team.id), - "project_name": self.team.name, - "organization_id": str(self.organization.id), - "organization_name": self.organization.name, - "user_role": OrganizationMembership.Level.ADMIN, - }, - groups=groups(self.organization), - ) + response = self.client.get("/api/environments/@current/") + assert response.status_code == status.HTTP_200_OK - # Test toggling back to false - mock_capture.reset_mock() - response = self.client.patch(f"/api/environments/@current/", {"access_control": False}) + current_access_control = response.json()["access_control"] + new_setting = not current_access_control + response = self.client.patch(f"/api/environments/@current/", {"access_control": new_setting}) self.assertEqual(response.status_code, status.HTTP_200_OK) mock_capture.assert_called_with( str(self.user.distinct_id), "project access control toggled", properties={ - "enabled": False, + "enabled": new_setting, "project_id": str(self.team.id), "project_name": self.team.name, "organization_id": str(self.organization.id), From 61e7352ed6e2796b2a3b5aaea15ac45578e22c18 Mon Sep 17 00:00:00 2001 From: Zach Waterfield Date: Mon, 23 Dec 2024 14:34:06 -0500 Subject: [PATCH 3/5] don't reset mock --- posthog/api/test/test_team.py | 1 - 1 file changed, 1 deletion(-) diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 32c35f8f14b78..733cfb606f418 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -1228,7 +1228,6 @@ def _patch_linked_flag_config( def test_access_control_toggle_capture(self, mock_capture): self.organization_membership.level = OrganizationMembership.Level.ADMIN self.organization_membership.save() - mock_capture.reset_mock() response = self.client.get("/api/environments/@current/") assert response.status_code == status.HTTP_200_OK From 41db7e88b18488a44d629813230db69cde039fe2 Mon Sep 17 00:00:00 2001 From: Surbhi Date: Mon, 23 Dec 2024 15:13:55 -0500 Subject: [PATCH 4/5] removing tests --- posthog/api/test/test_team.py | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 32c35f8f14b78..1da488504f57d 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -28,7 +28,6 @@ from posthog.temporal.common.schedule import describe_schedule from posthog.test.base import APIBaseTest from posthog.utils import get_instance_realm -from posthog.event_usage import groups def team_api_test_factory(): @@ -1224,34 +1223,6 @@ def _patch_linked_flag_config( assert response.status_code == expected_status, response.json() return response - @patch("posthoganalytics.capture") - def test_access_control_toggle_capture(self, mock_capture): - self.organization_membership.level = OrganizationMembership.Level.ADMIN - self.organization_membership.save() - mock_capture.reset_mock() - - response = self.client.get("/api/environments/@current/") - assert response.status_code == status.HTTP_200_OK - - current_access_control = response.json()["access_control"] - new_setting = not current_access_control - response = self.client.patch(f"/api/environments/@current/", {"access_control": new_setting}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - mock_capture.assert_called_with( - str(self.user.distinct_id), - "project access control toggled", - properties={ - "enabled": new_setting, - "project_id": str(self.team.id), - "project_name": self.team.name, - "organization_id": str(self.organization.id), - "organization_name": self.organization.name, - "user_role": OrganizationMembership.Level.ADMIN, - }, - groups=groups(self.organization), - ) - return TestTeamAPI From 4496d2b190939fbcd2f85bd065621e234506b216 Mon Sep 17 00:00:00 2001 From: Surbhi Date: Mon, 23 Dec 2024 15:22:18 -0500 Subject: [PATCH 5/5] removing tests --- posthog/api/test/test_team.py | 28 ---------------------------- 1 file changed, 28 deletions(-) diff --git a/posthog/api/test/test_team.py b/posthog/api/test/test_team.py index 733cfb606f418..1da488504f57d 100644 --- a/posthog/api/test/test_team.py +++ b/posthog/api/test/test_team.py @@ -28,7 +28,6 @@ from posthog.temporal.common.schedule import describe_schedule from posthog.test.base import APIBaseTest from posthog.utils import get_instance_realm -from posthog.event_usage import groups def team_api_test_factory(): @@ -1224,33 +1223,6 @@ def _patch_linked_flag_config( assert response.status_code == expected_status, response.json() return response - @patch("posthoganalytics.capture") - def test_access_control_toggle_capture(self, mock_capture): - self.organization_membership.level = OrganizationMembership.Level.ADMIN - self.organization_membership.save() - - response = self.client.get("/api/environments/@current/") - assert response.status_code == status.HTTP_200_OK - - current_access_control = response.json()["access_control"] - new_setting = not current_access_control - response = self.client.patch(f"/api/environments/@current/", {"access_control": new_setting}) - self.assertEqual(response.status_code, status.HTTP_200_OK) - - mock_capture.assert_called_with( - str(self.user.distinct_id), - "project access control toggled", - properties={ - "enabled": new_setting, - "project_id": str(self.team.id), - "project_name": self.team.name, - "organization_id": str(self.organization.id), - "organization_name": self.organization.name, - "user_role": OrganizationMembership.Level.ADMIN, - }, - groups=groups(self.organization), - ) - return TestTeamAPI