diff --git a/frontend/src/lib/components/ActivityLog/activityLogLogic.tsx b/frontend/src/lib/components/ActivityLog/activityLogLogic.tsx
index 4ba47decd2902..250f42da557cd 100644
--- a/frontend/src/lib/components/ActivityLog/activityLogLogic.tsx
+++ b/frontend/src/lib/components/ActivityLog/activityLogLogic.tsx
@@ -11,6 +11,7 @@ import {
} from 'lib/components/ActivityLog/humanizeActivity'
import { ACTIVITY_PAGE_SIZE } from 'lib/constants'
import { PaginationManual } from 'lib/lemon-ui/PaginationControl'
+import { cohortActivityDescriber } from 'scenes/cohorts/activityDescriptions'
import { dataManagementActivityDescriber } from 'scenes/data-management/dataManagementDescribers'
import { flagActivityDescriber } from 'scenes/feature-flags/activityDescriptions'
import { notebookActivityDescriber } from 'scenes/notebooks/Notebook/notebookActivityDescriber'
@@ -36,6 +37,8 @@ export const describerFor = (logItem?: ActivityLogItem): Describer | undefined =
case ActivityScope.PLUGIN:
case ActivityScope.PLUGIN_CONFIG:
return pluginActivityDescriber
+ case ActivityScope.COHORT:
+ return cohortActivityDescriber
case ActivityScope.INSIGHT:
return insightActivityDescriber
case ActivityScope.PERSON:
diff --git a/frontend/src/scenes/cohorts/activityDescriptions.tsx b/frontend/src/scenes/cohorts/activityDescriptions.tsx
new file mode 100644
index 0000000000000..ceb4726ce7b77
--- /dev/null
+++ b/frontend/src/scenes/cohorts/activityDescriptions.tsx
@@ -0,0 +1,55 @@
+import '../../lib/components/Cards/InsightCard/InsightCard.scss'
+
+import {
+ ActivityLogItem,
+ defaultDescriber,
+ HumanizedChange,
+ userNameForLogItem,
+} from 'lib/components/ActivityLog/humanizeActivity'
+import { Link } from 'lib/lemon-ui/Link'
+import { urls } from 'scenes/urls'
+
+const nameOrLinkToCohort = (id?: string | null, name?: string | null): string | JSX.Element => {
+ const displayName = name || '(empty string)'
+ return id ? {displayName} : displayName
+}
+
+export function cohortActivityDescriber(logItem: ActivityLogItem, asNotification?: boolean): HumanizedChange {
+ if (logItem.scope != 'Cohort') {
+ console.error('cohort describer received a non-cohort activity')
+ return { description: null }
+ }
+
+ if (logItem.activity == 'created') {
+ return {
+ description: (
+ <>
+ {userNameForLogItem(logItem)} created the cohort:{' '}
+ {nameOrLinkToCohort(logItem?.item_id, logItem?.detail.name)}
+
+ ),
+ }
+ }
+
+ if (logItem.activity == 'deleted') {
+ return {
+ description: (
+ <>
+ {userNameForLogItem(logItem)} deleted the cohort: {logItem.detail.name}
+
+ ),
+ }
+ }
+
+ if (logItem.activity == 'updated') {
+ return {
+ description: (
+ <>
+ {userNameForLogItem(logItem)} updated the cohort:{' '}
+ {nameOrLinkToCohort(logItem?.item_id, logItem?.detail.name)}
+
+ ),
+ }
+ }
+ return defaultDescriber(logItem, asNotification, nameOrLinkToCohort(logItem?.detail.short_id))
+}
diff --git a/frontend/src/types.ts b/frontend/src/types.ts
index b3cca7bd90f98..b91060b2e20be 100644
--- a/frontend/src/types.ts
+++ b/frontend/src/types.ts
@@ -3686,6 +3686,7 @@ export enum ActivityScope {
SURVEY = 'Survey',
EARLY_ACCESS_FEATURE = 'EarlyAccessFeature',
COMMENT = 'Comment',
+ COHORT = 'Cohort',
TEAM = 'Team',
}
diff --git a/posthog/api/activity_log.py b/posthog/api/activity_log.py
index dd369f94c98fb..5645d038bd6a7 100644
--- a/posthog/api/activity_log.py
+++ b/posthog/api/activity_log.py
@@ -11,7 +11,7 @@
from posthog.api.routing import TeamAndOrgViewSetMixin
from posthog.api.shared import UserBasicSerializer
-from posthog.models import ActivityLog, FeatureFlag, Insight, NotificationViewed, User
+from posthog.models import ActivityLog, FeatureFlag, Insight, NotificationViewed, User, Cohort
from posthog.models.comment import Comment
from posthog.models.notebook.notebook import Notebook
@@ -114,6 +114,7 @@ def important_changes(self, request: Request, *args: Any, **kwargs: Any) -> Resp
my_comments = list(
Comment.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True)
)
+ my_cohorts = list(Cohort.objects.filter(created_by=user, team_id=self.team.pk).values_list("id", flat=True))
# then things they edited
interesting_changes = [
@@ -168,6 +169,17 @@ def important_changes(self, request: Request, *args: Any, **kwargs: Any) -> Resp
.values_list("item_id", flat=True)
)
+ my_changed_cohorts = list(
+ ActivityLog.objects.filter(
+ team_id=self.team.id,
+ activity__in=interesting_changes,
+ user_id=user.pk,
+ scope="Cohort",
+ )
+ .exclude(item_id__in=my_cohorts)
+ .values_list("item_id", flat=True)
+ )
+
last_read_date = NotificationViewed.objects.filter(user=user).first()
last_read_filter = ""
@@ -218,6 +230,7 @@ def important_changes(self, request: Request, *args: Any, **kwargs: Any) -> Resp
& Q(id__in=deduplicated_notebook_activity_ids)
)
| Q(Q(scope="Comment") & Q(item_id__in=my_comments))
+ | Q(Q(scope="Cohort") & Q(item_id__in=my_cohorts))
)
| Q(
# don't want to see creation of these things since that was before the user edited these things
@@ -231,6 +244,7 @@ def important_changes(self, request: Request, *args: Any, **kwargs: Any) -> Resp
& Q(id__in=deduplicated_notebook_activity_ids)
)
| Q(Q(scope="Comment") & Q(item_id__in=my_changed_comments))
+ | Q(Q(scope="Cohort") & Q(item_id__in=my_changed_cohorts))
)
)
)
diff --git a/posthog/api/cohort.py b/posthog/api/cohort.py
index 139f8f3747d24..3e776167b8de4 100644
--- a/posthog/api/cohort.py
+++ b/posthog/api/cohort.py
@@ -2,9 +2,12 @@
from posthog.clickhouse.client.connection import Workload
from django.db import DatabaseError
+from loginas.utils import is_impersonated_session
from sentry_sdk import start_span
import structlog
+from posthog.models.activity_logging.activity_log import log_activity, Detail, changes_between, load_activity
+from posthog.models.activity_logging.activity_page import activity_page_response
from posthog.models.feature_flag.flag_matching import (
FeatureFlagMatcher,
FlagsMatcherCache,
@@ -22,7 +25,7 @@
from django.db.models import QuerySet, Prefetch, prefetch_related_objects, OuterRef, Subquery
from django.db.models.expressions import F
from django.utils import timezone
-from rest_framework import serializers, viewsets
+from rest_framework import serializers, viewsets, request, status
from rest_framework.decorators import action
from rest_framework.exceptions import ValidationError
from rest_framework.request import Request
@@ -403,6 +406,69 @@ def persons(self, request: Request, **kwargs) -> Response:
return Response({"results": serialized_actors, "next": next_url, "previous": previous_url})
+ @action(methods=["GET"], url_path="activity", detail=False, required_scopes=["activity_log:read"])
+ def all_activity(self, request: request.Request, **kwargs):
+ limit = int(request.query_params.get("limit", "10"))
+ page = int(request.query_params.get("page", "1"))
+
+ activity_page = load_activity(scope="Cohort", team_id=self.team_id, limit=limit, page=page)
+
+ return activity_page_response(activity_page, limit, page, request)
+
+ @action(methods=["GET"], detail=True, required_scopes=["activity_log:read"])
+ def activity(self, request: request.Request, **kwargs):
+ limit = int(request.query_params.get("limit", "10"))
+ page = int(request.query_params.get("page", "1"))
+
+ item_id = kwargs["pk"]
+ if not Cohort.objects.filter(id=item_id, team_id=self.team_id).exists():
+ return Response("", status=status.HTTP_404_NOT_FOUND)
+
+ activity_page = load_activity(
+ scope="Cohort",
+ team_id=self.team_id,
+ item_ids=[str(item_id)],
+ limit=limit,
+ page=page,
+ )
+ return activity_page_response(activity_page, limit, page, request)
+
+ def perform_create(self, serializer):
+ serializer.save()
+ log_activity(
+ organization_id=self.organization.id,
+ team_id=self.team_id,
+ user=serializer.context["request"].user,
+ was_impersonated=is_impersonated_session(serializer.context["request"]),
+ item_id=serializer.instance.id,
+ scope="Cohort",
+ activity="created",
+ detail=Detail(name=serializer.instance.name),
+ )
+
+ def perform_update(self, serializer):
+ instance_id = serializer.instance.id
+
+ try:
+ before_update = Cohort.objects.get(pk=instance_id)
+ except Cohort.DoesNotExist:
+ before_update = None
+
+ serializer.save()
+
+ changes = changes_between("Cohort", previous=before_update, current=serializer.instance)
+
+ log_activity(
+ organization_id=self.organization.id,
+ team_id=self.team_id,
+ user=serializer.context["request"].user,
+ was_impersonated=is_impersonated_session(serializer.context["request"]),
+ item_id=instance_id,
+ scope="Cohort",
+ activity="updated",
+ detail=Detail(changes=changes, name=serializer.instance.name),
+ )
+
class LegacyCohortViewSet(CohortViewSet):
derive_current_team_from_user_only = True
diff --git a/posthog/api/test/__snapshots__/test_api_docs.ambr b/posthog/api/test/__snapshots__/test_api_docs.ambr
index 70fccf17e398f..8793984c350a5 100644
--- a/posthog/api/test/__snapshots__/test_api_docs.ambr
+++ b/posthog/api/test/__snapshots__/test_api_docs.ambr
@@ -110,6 +110,7 @@
'Warning: operationId "batch_exports_pause_create" has collisions [(\'/api/organizations/{organization_id}/batch_exports/{id}/pause/\', \'post\'), (\'/api/projects/{project_id}/batch_exports/{id}/pause/\', \'post\')]. resolving with numeral suffixes.',
'Warning: operationId "batch_exports_unpause_create" has collisions [(\'/api/organizations/{organization_id}/batch_exports/{id}/unpause/\', \'post\'), (\'/api/projects/{project_id}/batch_exports/{id}/unpause/\', \'post\')]. resolving with numeral suffixes.',
'Warning: operationId "app_metrics_historical_exports_retrieve" has collisions [(\'/api/projects/{project_id}/app_metrics/{plugin_config_id}/historical_exports/\', \'get\'), (\'/api/projects/{project_id}/app_metrics/{plugin_config_id}/historical_exports/{id}/\', \'get\')]. resolving with numeral suffixes.',
+ 'Warning: operationId "cohorts_activity_retrieve" has collisions [(\'/api/projects/{project_id}/cohorts/{id}/activity/\', \'get\'), (\'/api/projects/{project_id}/cohorts/activity/\', \'get\')]. resolving with numeral suffixes.',
'Warning: operationId "event_definitions_retrieve" has collisions [(\'/api/projects/{project_id}/event_definitions/\', \'get\'), (\'/api/projects/{project_id}/event_definitions/{id}/\', \'get\')]. resolving with numeral suffixes.',
'Warning: operationId "feature_flags_activity_retrieve" has collisions [(\'/api/projects/{project_id}/feature_flags/{id}/activity/\', \'get\'), (\'/api/projects/{project_id}/feature_flags/activity/\', \'get\')]. resolving with numeral suffixes.',
'Warning: operationId "insights_activity_retrieve" has collisions [(\'/api/projects/{project_id}/insights/{id}/activity/\', \'get\'), (\'/api/projects/{project_id}/insights/activity/\', \'get\')]. resolving with numeral suffixes.',
diff --git a/posthog/api/test/__snapshots__/test_insight.ambr b/posthog/api/test/__snapshots__/test_insight.ambr
index 1376e752451f9..972bf8c24e289 100644
--- a/posthog/api/test/__snapshots__/test_insight.ambr
+++ b/posthog/api/test/__snapshots__/test_insight.ambr
@@ -1628,6 +1628,24 @@
LIMIT 21
'''
# ---
+# name: TestInsight.test_listing_insights_does_not_nplus1.30
+ '''
+ SELECT "posthog_taggeditem"."id",
+ "posthog_taggeditem"."tag_id",
+ "posthog_taggeditem"."dashboard_id",
+ "posthog_taggeditem"."insight_id",
+ "posthog_taggeditem"."event_definition_id",
+ "posthog_taggeditem"."property_definition_id",
+ "posthog_taggeditem"."action_id",
+ "posthog_taggeditem"."feature_flag_id"
+ FROM "posthog_taggeditem"
+ WHERE "posthog_taggeditem"."insight_id" IN (1,
+ 2,
+ 3,
+ 4,
+ 5 /* ... */)
+ '''
+# ---
# name: TestInsight.test_listing_insights_does_not_nplus1.4
'''
SELECT "posthog_team"."id",
diff --git a/posthog/api/test/test_cohort.py b/posthog/api/test/test_cohort.py
index 82740d73ed515..14b6b60b51484 100644
--- a/posthog/api/test/test_cohort.py
+++ b/posthog/api/test/test_cohort.py
@@ -1,6 +1,7 @@
import json
from datetime import datetime, timedelta
-from typing import Any
+from typing import Optional, Any
+from unittest import mock
from unittest.mock import patch
from django.core.files.uploadedfile import SimpleUploadedFile
@@ -34,6 +35,31 @@ class TestCohort(TestExportMixin, ClickhouseTestMixin, APIBaseTest, QueryMatchin
def capture_select_queries(self):
return self.capture_queries(("INSERT INTO cohortpeople", "SELECT", "ALTER", "select", "DELETE"))
+ def _get_cohort_activity(
+ self,
+ flag_id: Optional[int] = None,
+ team_id: Optional[int] = None,
+ expected_status: int = status.HTTP_200_OK,
+ ):
+ if team_id is None:
+ team_id = self.team.id
+
+ if flag_id:
+ url = f"/api/projects/{team_id}/cohorts/{flag_id}/activity"
+ else:
+ url = f"/api/projects/{team_id}/cohorts/activity"
+
+ activity = self.client.get(url)
+ self.assertEqual(activity.status_code, expected_status)
+ return activity.json()
+
+ def assert_cohort_activity(self, cohort_id: Optional[int], expected: list[dict]):
+ activity_response = self._get_cohort_activity(cohort_id)
+
+ activity: list[dict] = activity_response["results"]
+ self.maxDiff = None
+ assert activity == expected
+
@patch("posthog.api.cohort.report_user_action")
@patch("posthog.tasks.calculate_cohort.calculate_cohort_ch.delay", side_effect=calculate_cohort_ch)
@patch("posthog.models.cohort.util.sync_execute", side_effect=sync_execute)
@@ -281,6 +307,96 @@ def test_cohort_list(self):
self.assertEqual(response["results"][0]["name"], "whatever")
self.assertEqual(response["results"][0]["created_by"]["id"], self.user.id)
+ def test_cohort_activity_log(self):
+ self.team.app_urls = ["http://somewebsite.com"]
+ self.team.save()
+ Person.objects.create(team=self.team, properties={"prop": 5})
+ Person.objects.create(team=self.team, properties={"prop": 6})
+
+ self.client.post(
+ f"/api/projects/{self.team.id}/cohorts",
+ data={"name": "whatever", "groups": [{"properties": {"prop": 5}}]},
+ )
+
+ cohort = Cohort.objects.filter(team=self.team).last()
+ assert cohort is not None
+
+ self.assert_cohort_activity(
+ cohort_id=cohort.pk,
+ expected=[
+ {
+ "user": {"first_name": "", "email": "user1@posthog.com"},
+ "activity": "created",
+ "scope": "Cohort",
+ "item_id": str(cohort.pk),
+ "detail": {"changes": None, "trigger": None, "name": "whatever", "short_id": None, "type": None},
+ "created_at": mock.ANY,
+ }
+ ],
+ )
+
+ self.client.patch(
+ f"/api/projects/{self.team.id}/cohorts/{cohort.pk}",
+ data={"name": "woohoo", "groups": [{"properties": {"prop": 6}}]},
+ )
+ cohort.refresh_from_db()
+ assert cohort.name == "woohoo"
+
+ self.assert_cohort_activity(
+ cohort_id=cohort.pk,
+ expected=[
+ {
+ "user": {"first_name": "", "email": "user1@posthog.com"},
+ "activity": "updated",
+ "scope": "Cohort",
+ "item_id": str(cohort.pk),
+ "detail": {
+ "changes": [
+ {
+ "type": "Cohort",
+ "action": "changed",
+ "field": "name",
+ "before": "whatever",
+ "after": "woohoo",
+ },
+ {
+ "type": "Cohort",
+ "action": "changed",
+ "field": "groups",
+ "before": [
+ {
+ "days": None,
+ "count": None,
+ "label": None,
+ "end_date": None,
+ "event_id": None,
+ "action_id": None,
+ "properties": [{"key": "prop", "type": "person", "value": 5}],
+ "start_date": None,
+ "count_operator": None,
+ }
+ ],
+ "after": [{"properties": [{"key": "prop", "type": "person", "value": 6}]}],
+ },
+ ],
+ "trigger": None,
+ "name": "woohoo",
+ "short_id": None,
+ "type": None,
+ },
+ "created_at": mock.ANY,
+ },
+ {
+ "user": {"first_name": "", "email": "user1@posthog.com"},
+ "activity": "created",
+ "scope": "Cohort",
+ "item_id": str(cohort.pk),
+ "detail": {"changes": None, "trigger": None, "name": "whatever", "short_id": None, "type": None},
+ "created_at": mock.ANY,
+ },
+ ],
+ )
+
def test_csv_export_new(self):
# Test 100s of distinct_ids, we only want ~10
Person.objects.create(
diff --git a/posthog/models/activity_logging/activity_log.py b/posthog/models/activity_logging/activity_log.py
index 141130ea4f80e..c05b0cac48bbd 100644
--- a/posthog/models/activity_logging/activity_log.py
+++ b/posthog/models/activity_logging/activity_log.py
@@ -18,6 +18,7 @@
logger = structlog.get_logger(__name__)
ActivityScope = Literal[
+ "Cohort",
"FeatureFlag",
"Person",
"Insight",
@@ -133,6 +134,14 @@ class Meta:
field_exclusions: dict[ActivityScope, list[str]] = {
+ "Cohort": [
+ "version",
+ "pending_version",
+ "count",
+ "is_calculating",
+ "last_calculation",
+ "errors_calculating",
+ ],
"Notebook": [
"text_content",
],