diff --git a/.capy-cache/dependencies.sha256 b/.capy-cache/dependencies.sha256 new file mode 100644 index 000000000000..47f290b5a253 --- /dev/null +++ b/.capy-cache/dependencies.sha256 @@ -0,0 +1 @@ +8620910f03b78c62ebc5462fc6ef8351843f0a2525ee8213a01b0cfc28ba250b diff --git a/.capy-cache/docker-compose.capy.yml b/.capy-cache/docker-compose.capy.yml new file mode 100644 index 000000000000..268109a66294 --- /dev/null +++ b/.capy-cache/docker-compose.capy.yml @@ -0,0 +1,19 @@ +services: + clickhouse: + mem_limit: !reset null + cpus: !reset null + proxy: + extra_hosts: !override + - 'plugins:host-gateway' + - 'capture:host-gateway' + - 'capture-ai:host-gateway' + - 'capture-logs:host-gateway' + - 'replay-capture:host-gateway' + - 'feature-flags:host-gateway' + - 'hypercache-server:host-gateway' + web: + image: caddy:latest + entrypoint: socat + command: ['TCP-LISTEN:8000,fork,reuseaddr', 'TCP:host.docker.internal:8011'] + extra_hosts: + - 'host.docker.internal:host-gateway' diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 18e8abe82883..b25964c1626d 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -18,6 +18,68 @@ ], "type": "string" }, + "AccountHealthFactor": { + "additionalProperties": false, + "description": "One usage metric's contribution to an account's health score.", + "properties": { + "change_pct": { + "description": "Percentage change current vs previous, or null when previous is zero.", + "type": ["number", "null"] + }, + "current": { + "description": "Total in the current period.", + "type": "number" + }, + "factor_score": { + "description": "0–100 retained-usage score, or null when there is no signal (both periods zero).", + "type": ["number", "null"] + }, + "interval": { + "$ref": "#/definitions/integer", + "description": "Lookback window in days for both the current and the immediately preceding period." + }, + "metric_id": { + "description": "The GroupUsageMetric id this factor was derived from.", + "type": "string" + }, + "metric_name": { + "type": "string" + }, + "previous": { + "description": "Total in the immediately preceding equal-length period.", + "type": "number" + } + }, + "required": ["metric_id", "metric_name", "interval", "current", "previous", "factor_score", "change_pct"], + "type": "object" + }, + "AccountHealthScore": { + "additionalProperties": false, + "description": "Explainable account health score derived from the team's usage-metric definitions.", + "properties": { + "factors": { + "description": "Per-metric breakdown that explains the overall score.", + "items": { + "$ref": "#/definitions/AccountHealthFactor" + }, + "type": "array" + }, + "score": { + "description": "0–100 overall score (rounded mean of non-null factor scores), or null when no_data.", + "type": ["number", "null"] + }, + "status": { + "$ref": "#/definitions/AccountHealthStatus" + } + }, + "required": ["score", "status", "factors"], + "type": "object" + }, + "AccountHealthStatus": { + "description": "Health bucket an account falls into. `no_data` means we could not score it (no external id, no usage-metric config, no defined metrics, or no usable signal).", + "enum": ["healthy", "needs_attention", "at_risk", "no_data"], + "type": "string" + }, "AccountsQuery": { "additionalProperties": false, "properties": { @@ -69,6 +131,7 @@ "type": "string" }, "select": { + "description": "Columns to return. Most are HogQL expressions resolved against `system.accounts`, but the synthetic `health_score` column is computed by the runner from the team's usage-metric definitions and returned as an `AccountHealthScore` object per row. Callers that omit it do zero health work.", "items": { "$ref": "#/definitions/HogQLExpression" }, diff --git a/frontend/src/queries/schema/schema-general.ts b/frontend/src/queries/schema/schema-general.ts index d05dca5b1e4b..2301aded1b4b 100644 --- a/frontend/src/queries/schema/schema-general.ts +++ b/frontend/src/queries/schema/schema-general.ts @@ -2306,6 +2306,36 @@ export interface GroupsQuery extends DataNode { offset?: integer } +/** Health bucket an account falls into. `no_data` means we could not score it (no + * external id, no usage-metric config, no defined metrics, or no usable signal). */ +export type AccountHealthStatus = 'healthy' | 'needs_attention' | 'at_risk' | 'no_data' + +/** One usage metric's contribution to an account's health score. */ +export interface AccountHealthFactor { + /** The GroupUsageMetric id this factor was derived from. */ + metric_id: string + metric_name: string + /** Lookback window in days for both the current and the immediately preceding period. */ + interval: integer + /** Total in the current period. */ + current: number + /** Total in the immediately preceding equal-length period. */ + previous: number + /** 0–100 retained-usage score, or null when there is no signal (both periods zero). */ + factor_score: number | null + /** Percentage change current vs previous, or null when previous is zero. */ + change_pct: number | null +} + +/** Explainable account health score derived from the team's usage-metric definitions. */ +export interface AccountHealthScore { + /** 0–100 overall score (rounded mean of non-null factor scores), or null when no_data. */ + score: number | null + status: AccountHealthStatus + /** Per-metric breakdown that explains the overall score. */ + factors: AccountHealthFactor[] +} + export type CachedAccountsQueryResponse = CachedQueryResponse export interface AccountsQueryResponse extends AnalyticsQueryResponseBase { @@ -2323,6 +2353,10 @@ export interface AccountsQueryResponse extends AnalyticsQueryResponseBase { export interface AccountsQuery extends DataNode { kind: NodeKind.AccountsQuery + /** Columns to return. Most are HogQL expressions resolved against `system.accounts`, but the + * synthetic `health_score` column is computed by the runner from the team's usage-metric + * definitions and returned as an `AccountHealthScore` object per row. Callers that omit it do + * zero health work. */ select?: HogQLExpression[] /** Aggregation expressions evaluated against the filtered account set; one value per metric is returned in `metricsResults`. When `metrics` is set without a `select`, the runner skips the regular row fetch and returns only the aggregated values. */ metrics?: HogQLExpression[] diff --git a/posthog/schema.py b/posthog/schema.py index 53ba1d2bf98e..2cbb614d632c 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -10,6 +10,7 @@ from pydantic import AwareDatetime, BaseModel, ConfigDict, Field, RootModel, confloat, conint from posthog.schema_enums import ( + AccountHealthStatus as AccountHealthStatus, Action as Action, Action1 as Action1, AgentMode as AgentMode, @@ -3030,6 +3031,40 @@ class Integer(RootModel[int]): root: int +class AccountHealthFactor(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + change_pct: float | None = Field( + ..., + description=("Percentage change current vs previous, or null when previous is zero."), + ) + current: float = Field(..., description="Total in the current period.") + factor_score: float | None = Field( + ..., + description=("0–100 retained-usage score, or null when there is no signal (both periods zero)."), + ) + interval: int = Field( + ..., + description=("Lookback window in days for both the current and the immediately preceding period."), + ) + metric_id: str = Field(..., description="The GroupUsageMetric id this factor was derived from.") + metric_name: str + previous: float = Field(..., description="Total in the immediately preceding equal-length period.") + + +class AccountHealthScore(BaseModel): + model_config = ConfigDict( + extra="forbid", + ) + factors: list[AccountHealthFactor] = Field(..., description="Per-metric breakdown that explains the overall score.") + score: float | None = Field( + ..., + description=("0–100 overall score (rounded mean of non-null factor scores), or null when no_data."), + ) + status: AccountHealthStatus + + class ActionConversionGoal(BaseModel): model_config = ConfigDict( extra="forbid", @@ -20615,7 +20650,16 @@ class AccountsQuery(BaseModel): orderBy: list[str] | None = None response: AccountsQueryResponse | None = None search: str | None = None - select: list[str] | None = None + select: list[str] | None = Field( + default=None, + description=( + "Columns to return. Most are HogQL expressions resolved against" + " `system.accounts`, but the synthetic `health_score` column is computed by" + " the runner from the team's usage-metric definitions and returned as an" + " `AccountHealthScore` object per row. Callers that omit it do zero health" + " work." + ), + ) tagNames: list[str] | None = None tags: QueryLogTags | None = None version: float | None = Field(default=None, description="version of the node, used for schema migrations") diff --git a/posthog/schema_enums.py b/posthog/schema_enums.py index 4b06c10d5f6b..01730e5853cc 100644 --- a/posthog/schema_enums.py +++ b/posthog/schema_enums.py @@ -21,6 +21,13 @@ class AIEventType(StrEnum): FIELD_AI_GENERATION_CLUSTERS = "$ai_generation_clusters" +class AccountHealthStatus(StrEnum): + HEALTHY = "healthy" + NEEDS_ATTENTION = "needs_attention" + AT_RISK = "at_risk" + NO_DATA = "no_data" + + class MathGroupTypeIndex(float, Enum): NUMBER_0 = 0 NUMBER_1 = 1 diff --git a/products/customer_analytics/backend/hogql_queries/accounts_query_runner.py b/products/customer_analytics/backend/hogql_queries/accounts_query_runner.py index 22d67ac5a83b..d1147aa3fc82 100644 --- a/products/customer_analytics/backend/hogql_queries/accounts_query_runner.py +++ b/products/customer_analytics/backend/hogql_queries/accounts_query_runner.py @@ -1,3 +1,7 @@ +from collections.abc import Sequence +from functools import cached_property +from typing import cast + from posthog.schema import AccountsQuery, AccountsQueryResponse, CachedAccountsQueryResponse from posthog.hogql import ast @@ -6,13 +10,21 @@ from posthog.hogql.query import execute_hogql_query from posthog.errors import ExposedCHQueryError, InternalCHQueryError +from posthog.exceptions_capture import capture_exception from posthog.hogql_queries.insights.paginators import HogQLHasMorePaginator from posthog.hogql_queries.query_runner import AnalyticsQueryRunner from posthog.models import User from posthog.rbac.user_access_control import UserAccessControl +from products.customer_analytics.backend.services.account_health import AccountHealthScorer, no_data_score + NAME_COLUMN = "name" +# Synthetic column: not a real `system.accounts` field. When selected, the runner computes an +# explainable AccountHealthScore per row from the team's GroupUsageMetric definitions and injects +# it into the results. Callers that omit it do zero health work. +HEALTH_COLUMN = "health_score" + DEFAULT_COLUMNS = (NAME_COLUMN, "created_at") DEFAULT_ORDER_BY = "created_at DESC" @@ -47,20 +59,34 @@ def __init__(self, *args, **kwargs): # resolution. A combined query carries both `select` and `metrics`. self._metrics_only = bool(self.query.metrics) and not self.query.select + # `columns` is the full ordered set the frontend renders (it may include the synthetic + # `health_score`). `_real_columns` / `_select_exprs` are the subset backed by an actual + # HogQL expression, aligned 1:1 with the query result row positions. self.columns: list[str] = [] + self._real_columns: list[str] = [] self._select_exprs: list[ast.Expr] = [] + self._health_requested = False if not self._metrics_only: raw_selects = list(self.query.select) if self.query.select else list(DEFAULT_COLUMNS) seen: set[str] = set() for raw in raw_selects: + if raw == HEALTH_COLUMN: + if HEALTH_COLUMN in seen: + continue + seen.add(HEALTH_COLUMN) + self._health_requested = True + self.columns.append(HEALTH_COLUMN) + continue column_name, expr = self._resolve_column(raw) if column_name in seen: continue seen.add(column_name) self.columns.append(column_name) + self._real_columns.append(column_name) self._select_exprs.append(expr) if NAME_COLUMN not in seen: self.columns.insert(0, NAME_COLUMN) + self._real_columns.insert(0, NAME_COLUMN) self._select_exprs.insert(0, self._name_tuple_expr()) self.paginator = HogQLHasMorePaginator.from_limit_context( @@ -74,6 +100,22 @@ def validate_query_runner_access(self, user: User) -> bool: "customer_analytics", "viewer" ) + @cached_property + def _health_scorer(self) -> AccountHealthScorer: + return AccountHealthScorer(team=self.team, timings=self.timings, modifiers=self.modifiers, user=self.user) + + def get_cache_payload(self) -> dict: + payload = super().get_cache_payload() + # Only health requests depend on usage-metric definitions and the configured group type — + # fold their fingerprints into the cache key so edits invalidate exactly those results, and + # leave non-health callers' cache keys untouched (and free of any GroupUsageMetric query). + if self._health_requested: + payload["account_health"] = { + "usage_metric_fingerprints": self._health_scorer.usage_metric_fingerprints(), + "account_group_type_index": self._health_scorer.config_fingerprint(), + } + return payload + def _resolve_column(self, raw: str) -> tuple[str, ast.Expr]: if raw == NAME_COLUMN: return NAME_COLUMN, self._name_tuple_expr() @@ -127,7 +169,12 @@ def _build_where_exprs(self) -> list[ast.Expr]: def to_query(self) -> ast.SelectQuery: where_exprs = self._build_where_exprs() - order_clauses = self.query.orderBy or [DEFAULT_ORDER_BY] + requested_order_clauses = self.query.orderBy or [] + order_clauses = [ + clause + for clause in requested_order_clauses + if _normalize_order_clause(clause).split(maxsplit=1)[0] != HEALTH_COLUMN + ] or [DEFAULT_ORDER_BY] return ast.SelectQuery( select=self._select_exprs, @@ -230,20 +277,13 @@ def _calculate(self) -> AccountsQueryResponse: modifiers=self.modifiers, ) - name_index = self.columns.index(NAME_COLUMN) - results = [ - [ - {"name": cell[0], "external_id": cell[1], "id": cell[2]} if index == name_index else cell - for index, cell in enumerate(row) - ] - for row in self.paginator.results - ] + results = self._build_results(self.paginator.results) return AccountsQueryResponse( kind="AccountsQuery", columns=list(self.columns), results=results, - types=[t for _, t in response.types] if response.types else [], + types=self._align_types(response.types), metricsResults=metrics_results, hogql=response.hogql or "", timings=response.timings, @@ -251,6 +291,48 @@ def _calculate(self) -> AccountsQueryResponse: **self.paginator.response_params(), ) + def _build_results(self, rows: Sequence[Sequence[object]]) -> list[list[object]]: + # Rows come back aligned to `_real_columns`; rebuild them in `self.columns` order, + # expanding the name tuple into its dict shape and injecting the synthetic health cell. + real_name_index = self._real_columns.index(NAME_COLUMN) + real_index_by_col = {col: index for index, col in enumerate(self._real_columns)} + + health_by_external_id: dict[str, object] = {} + if self._health_requested: + # Batch only the current page's external ids — no per-row (N+1) health queries. + external_ids = [cast(str | None, cast(Sequence[object], row[real_name_index])[1]) for row in rows] + try: + health_by_external_id = self._health_scorer.score_external_ids(external_ids) + except Exception as error: + # Health is a derived enhancement, so a bad usage metric or transient query failure + # must not take down the core accounts list. + capture_exception(error, {"scope": "accounts_query_runner.health", "team_id": self.team.id}) + + results: list[list[object]] = [] + for row in rows: + name_cell = cast(Sequence[object], row[real_name_index]) + external_id = cast(str | None, name_cell[1]) + out_row: list[object] = [] + for col in self.columns: + if col == HEALTH_COLUMN: + score = health_by_external_id.get(external_id) if external_id else None + out_row.append((score or no_data_score()).model_dump(mode="json")) + elif col == NAME_COLUMN: + out_row.append({"name": name_cell[0], "external_id": name_cell[1], "id": name_cell[2]}) + else: + out_row.append(row[real_index_by_col[col]]) + results.append(out_row) + return results + + def _align_types(self, response_types: Sequence[tuple[str, str]] | None) -> list[str]: + # The synthetic health column has no HogQL type; insert a placeholder so `types` stays + # aligned with `columns` for any consumer that zips them. + type_by_col = {} + if response_types: + for real_col, (_, col_type) in zip(self._real_columns, response_types): + type_by_col[real_col] = col_type + return [("AccountHealthScore" if col == HEALTH_COLUMN else type_by_col.get(col, "")) for col in self.columns] + def _compute_metrics_results(self, metrics: list[str]) -> list[float | int | None]: try: response = self._execute_metrics_query(metrics) diff --git a/products/customer_analytics/backend/hogql_queries/test/test_accounts_query_runner.py b/products/customer_analytics/backend/hogql_queries/test/test_accounts_query_runner.py index e4fa85598b6f..bd2bba4df6a6 100644 --- a/products/customer_analytics/backend/hogql_queries/test/test_accounts_query_runner.py +++ b/products/customer_analytics/backend/hogql_queries/test/test_accounts_query_runner.py @@ -1,4 +1,9 @@ -from posthog.test.base import ClickhouseTestMixin, NonAtomicBaseTest +from datetime import datetime, timedelta +from zoneinfo import ZoneInfo + +from freezegun import freeze_time +from posthog.test.base import ClickhouseTestMixin, NonAtomicBaseTest, _create_event, flush_persons_and_events +from unittest.mock import patch from django.test import override_settings from django.utils import timezone @@ -12,10 +17,13 @@ from posthog.api.tagged_item import set_tags_on_object from posthog.constants import AvailableFeature from posthog.models import Tag, User +from posthog.models.group_usage_metric import GroupUsageMetric from posthog.models.team import Team from posthog.rbac.user_access_control import UserAccessControlError +from posthog.test.test_utils import create_group_type_mapping_without_created_at from products.customer_analytics.backend.hogql_queries.accounts_query_runner import AccountsQueryRunner +from products.customer_analytics.backend.services.account_health import AccountHealthScorer from products.customer_analytics.backend.test.factories import create_account from products.notebooks.backend.models import Notebook, ResourceNotebook @@ -510,3 +518,286 @@ def test_validate_query_runner_access_denied(self): runner = AccountsQueryRunner(query=AccountsQuery(), team=self.team) self.assertRaises(UserAccessControlError, runner.validate_query_runner_access, self.user) + + +@override_settings(IN_UNIT_TESTING=True) +class TestAccountsQueryRunnerHealth(ClickhouseTestMixin, NonAtomicBaseTest): + GROUP_TYPE_INDEX = 0 + NOW = datetime(2025, 10, 9, 12, 0, tzinfo=ZoneInfo("UTC")) + CURRENT_TS = NOW - timedelta(days=3) + PREVIOUS_TS = NOW - timedelta(days=10) + + def setUp(self): + super().setUp() + # customer_analytics_config is a cached_property on the class-shared team object; drop any + # value a prior test cached so each test reads the (per-test truncated) DB state fresh. + self.team.__dict__.pop("customer_analytics_config", None) + create_group_type_mapping_without_created_at( + team=self.team, + project_id=self.team.project_id, + group_type="organization", + group_type_index=self.GROUP_TYPE_INDEX, + ) + + def _configure(self, index: int | None = GROUP_TYPE_INDEX) -> None: + config = self.team.customer_analytics_config + config.account_group_type_index = index + config.save() + + def _metric( + self, + name: str, + event_name: str, + *, + math: str = GroupUsageMetric.Math.COUNT, + math_property: str | None = None, + interval: int = 7, + ) -> GroupUsageMetric: + return GroupUsageMetric.objects.create( + team=self.team, + group_type_index=self.GROUP_TYPE_INDEX, + name=name, + interval=interval, + math=math, + math_property=math_property, + format=GroupUsageMetric.Format.NUMERIC, + display=GroupUsageMetric.Display.NUMBER, + filters={"events": [{"id": event_name, "type": "events", "order": 0}]}, + ) + + def _emit(self, group_key: str, event_name: str, count: int, when, properties: dict | None = None) -> None: + for _ in range(count): + _create_event( + event=event_name, + team=self.team, + distinct_id=f"d-{group_key}", + properties={"$group_0": group_key, **(properties or {})}, + timestamp=when, + ) + + def _health_by_external_id(self, **query_kwargs) -> dict: + runner = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"], **query_kwargs), team=self.team, user=self.user + ) + response = runner.calculate() + name_idx = runner.columns.index("name") + health_idx = runner.columns.index("health_score") + return {row[name_idx]["external_id"]: row[health_idx] for row in response.results} + + def test_healthy_at_risk_and_no_data(self): + self._configure() + self._metric("Events ingested", "ev_a") + self._metric("Active users", "ev_b") + create_account(team_id=self.team.id, name="Acme", external_id="acme") + create_account(team_id=self.team.id, name="Globex", external_id="globex") + create_account(team_id=self.team.id, name="Hooli", external_id="hooli") + + with freeze_time(self.NOW): + for event_name in ("ev_a", "ev_b"): + self._emit("acme", event_name, 9, self.CURRENT_TS) + self._emit("acme", event_name, 10, self.PREVIOUS_TS) + self._emit("globex", event_name, 2, self.CURRENT_TS) + self._emit("globex", event_name, 10, self.PREVIOUS_TS) + flush_persons_and_events() + health = self._health_by_external_id() + + assert health["acme"]["score"] == 90 + assert health["acme"]["status"] == "healthy" + assert len(health["acme"]["factors"]) == 2 + # Factors are explainable: current/previous totals and the per-factor score ride along. + events_factor = next(f for f in health["acme"]["factors"] if f["metric_name"] == "Events ingested") + assert events_factor["current"] == 9.0 + assert events_factor["previous"] == 10.0 + assert events_factor["factor_score"] == 90 + + assert health["globex"]["score"] == 20 + assert health["globex"]["status"] == "at_risk" + + # Account has an external id and metrics exist, but no signal at all → no_data. + assert health["hooli"]["score"] is None + assert health["hooli"]["status"] == "no_data" + + def test_sum_metric_scores_on_retained_value(self): + self._configure() + self._metric("Revenue", "purchase", math=GroupUsageMetric.Math.SUM, math_property="amount") + create_account(team_id=self.team.id, name="Acme", external_id="acme") + + with freeze_time(self.NOW): + self._emit("acme", "purchase", 1, self.CURRENT_TS, properties={"amount": 80}) + self._emit("acme", "purchase", 1, self.PREVIOUS_TS, properties={"amount": 100}) + flush_persons_and_events() + health = self._health_by_external_id() + + factor = health["acme"]["factors"][0] + assert factor["current"] == 80.0 + assert factor["previous"] == 100.0 + assert factor["factor_score"] == 80 + assert health["acme"]["status"] == "healthy" + + def test_missing_external_id_is_no_data(self): + self._configure() + self._metric("Events ingested", "ev_a") + create_account(team_id=self.team.id, name="No external id") + + health = self._health_by_external_id() + assert health[None]["status"] == "no_data" + assert health[None]["factors"] == [] + + def test_no_config_is_no_data(self): + # A team whose account_group_type_index is unset has nothing to join accounts to, so every + # account is no_data. Use a fresh team so the (class-shared) configured team can't leak in. + team = Team.objects.create(organization=self.organization) + GroupUsageMetric.objects.create( + team=team, + group_type_index=self.GROUP_TYPE_INDEX, + name="Events ingested", + interval=7, + math=GroupUsageMetric.Math.COUNT, + format=GroupUsageMetric.Format.NUMERIC, + display=GroupUsageMetric.Display.NUMBER, + filters={"events": [{"id": "ev_a", "type": "events", "order": 0}]}, + ) + create_account(team_id=team.id, name="Acme", external_id="acme") + + runner = AccountsQueryRunner(query=AccountsQuery(select=["name", "health_score"]), team=team, user=self.user) + response = runner.calculate() + name_idx = runner.columns.index("name") + health_idx = runner.columns.index("health_score") + health = {row[name_idx]["external_id"]: row[health_idx] for row in response.results} + + assert health["acme"]["status"] == "no_data" + + def test_no_metrics_is_no_data(self): + self._configure() + create_account(team_id=self.team.id, name="Acme", external_id="acme") + + health = self._health_by_external_id() + assert health["acme"]["status"] == "no_data" + assert health["acme"]["factors"] == [] + + def test_health_batches_only_current_page_external_ids(self): + self._configure() + self._metric("Events ingested", "ev_a") + with timezone.override("UTC"): + create_account(team_id=self.team.id, name="Old", external_id="old") + create_account(team_id=self.team.id, name="Mid", external_id="mid") + create_account(team_id=self.team.id, name="New", external_id="new") + + with patch.object(AccountHealthScorer, "score_external_ids", autospec=True, return_value={}) as scorer: + runner = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"], limit=2), team=self.team, user=self.user + ) + runner.calculate() + + # Default order is created_at DESC → only the two newest accounts are on page one. + scored_ids = set(scorer.call_args.args[1]) + assert scored_ids == {"new", "mid"} + assert "old" not in scored_ids + + def test_health_isolates_team_data(self): + self._configure() + self._metric("Events ingested", "ev_a") + other_team = Team.objects.create(organization=self.organization) + create_account(team_id=self.team.id, name="Mine", external_id="shared") + create_account(team_id=other_team.id, name="Theirs", external_id="shared") + + with freeze_time(self.NOW): + # Events belong to the *other* team's group with the same key — they must not leak in. + for _ in range(9): + _create_event( + event="ev_a", + team=other_team, + distinct_id="d-other", + properties={"$group_0": "shared"}, + timestamp=self.CURRENT_TS, + ) + flush_persons_and_events() + health = self._health_by_external_id() + + assert set(health) == {"shared"} + assert health["shared"]["status"] == "no_data" + + def test_health_omitted_does_no_usage_metric_work(self): + self._configure() + self._metric("Events ingested", "ev_a") + create_account(team_id=self.team.id, name="Acme", external_id="acme") + + with patch.object(AccountHealthScorer, "score_external_ids", autospec=True) as scorer: + runner = AccountsQueryRunner(query=AccountsQuery(select=["name"]), team=self.team, user=self.user) + runner.calculate() + scorer.assert_not_called() + + # No health request → no usage-metric fingerprints in the cache key either. + runner = AccountsQueryRunner(query=AccountsQuery(select=["name"]), team=self.team, user=self.user) + assert "account_health" not in runner.get_cache_payload() + + @patch("products.customer_analytics.backend.hogql_queries.accounts_query_runner.capture_exception") + def test_health_failure_does_not_take_down_accounts_list(self, capture_exception_mock): + account = create_account(team_id=self.team.id, name="Acme", external_id="acme") + with patch.object(AccountHealthScorer, "score_external_ids", side_effect=ValueError("bad usage metric")): + runner = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ) + response = runner.calculate() + + name_idx = runner.columns.index("name") + health_idx = runner.columns.index("health_score") + assert response.results[0][name_idx]["id"] == str(account.id) + assert response.results[0][health_idx]["status"] == "no_data" + capture_exception_mock.assert_called_once() + + def test_cache_payload_includes_health_fingerprints_only_when_selected(self): + self._configure() + self._metric("Events ingested", "ev_a") + + with_health = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ) + payload = with_health.get_cache_payload() + assert "account_health" in payload + assert payload["account_health"]["account_group_type_index"] == self.GROUP_TYPE_INDEX + + def test_cache_key_changes_when_usage_metric_changes(self): + self._configure() + metric = self._metric("Events ingested", "ev_a") + key_before = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ).get_cache_key() + + metric.interval = 30 + metric.save(update_fields=["interval"]) + + key_after = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ).get_cache_key() + assert key_before != key_after + + def test_cache_key_changes_when_usage_metric_is_renamed(self): + self._configure() + metric = self._metric("Events ingested", "ev_a") + key_before = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ).get_cache_key() + + metric.name = "Renamed events" + metric.save(update_fields=["name"]) + + key_after = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"]), team=self.team, user=self.user + ).get_cache_key() + assert key_before != key_after + + def test_ordering_by_synthetic_health_column_falls_back_to_default(self): + older = create_account(team_id=self.team.id, name="Older") + newer = create_account(team_id=self.team.id, name="Newer") + + runner = AccountsQueryRunner( + query=AccountsQuery(select=["name", "health_score"], orderBy=["health_score DESC"]), + team=self.team, + user=self.user, + ) + response = runner.calculate() + name_idx = runner.columns.index("name") + ids = [row[name_idx]["id"] for row in response.results] + + assert ids == [str(newer.id), str(older.id)] diff --git a/products/customer_analytics/backend/management/commands/seed_customer_analytics_accounts.py b/products/customer_analytics/backend/management/commands/seed_customer_analytics_accounts.py index f2b3cd1b42de..a2baa6ff1118 100644 --- a/products/customer_analytics/backend/management/commands/seed_customer_analytics_accounts.py +++ b/products/customer_analytics/backend/management/commands/seed_customer_analytics_accounts.py @@ -12,19 +12,32 @@ Re-running is safe: existing accounts, pool users, and notes are left alone. +Optionally, `--with-health-demo` also seeds two demo usage metrics plus deterministic events so the +accounts list's Health column has something healthy and something at risk to show. It is off by +default and is the only part of this command that writes events — the default behavior stays +event-free and Kafka-free. + Usage: python manage.py seed_customer_analytics_accounts --team-id 1 python manage.py seed_customer_analytics_accounts --team-id 1 --users 5 --accounts-with-notes 5 + python manage.py seed_customer_analytics_accounts --team-id 1 --with-health-demo python manage.py seed_customer_analytics_accounts --team-id 1 --dry-run """ +import uuid +from datetime import timedelta from typing import Any from uuid import uuid4 from django.core.management.base import BaseCommand, CommandError from django.db import transaction +from django.utils import timezone +from posthog.clickhouse.client import sync_execute from posthog.models import Group, OrganizationMembership, Team, User +from posthog.models.event.sql import EVENTS_DATA_TABLE +from posthog.models.event.util import create_event +from posthog.models.group_usage_metric import GroupUsageMetric from posthog.models.scoping import team_scope from products.customer_analytics.backend.models.account import Account, AccountAssignment, AccountProperties @@ -40,6 +53,21 @@ ("Support escalation", "Looked into slow shared-link loads. Mitigated for now; permanent fix ETA to follow."), ] +# Stable namespace so health-demo event UUIDs are reproducible (uuid5) — re-running the command +# regenerates the exact same rows instead of piling up duplicates. +HEALTH_DEMO_UUID_NAMESPACE = uuid.UUID("5b6c3d2e-1f4a-4c8b-9e7d-0a1b2c3d4e5f") +HEALTH_DEMO_INTERVAL_DAYS = 7 + +# Two count metrics, one event each. The current/previous counts below produce the demo narrative: +# the first external-id account retains 9/10 (healthy ~90) and the second retains 2/10 (at risk ~20) +# on both factors. +HEALTH_DEMO_METRICS: list[tuple[str, str]] = [ + ("Customer analytics demo: Events ingested", "demo_health_event_ingested"), + ("Customer analytics demo: Active users", "demo_health_active_user"), +] +# (current_period_count, previous_period_count) per account, indexed by account position. +HEALTH_DEMO_COUNTS: list[tuple[int, int]] = [(9, 10), (2, 10)] + class Command(BaseCommand): help = "Seed Customer analytics accounts (with users and notes) from existing group-analytics groups." @@ -61,6 +89,12 @@ def add_arguments(self, parser: Any) -> None: parser.add_argument( "--limit", type=int, default=None, help="Cap how many groups become accounts (default: all)." ) + parser.add_argument( + "--with-health-demo", + action="store_true", + help="Also seed two demo usage metrics and deterministic events so the Health column has " + "healthy and at-risk accounts to show (writes events; off by default).", + ) parser.add_argument("--dry-run", action="store_true", help="Report what would be created without writing.") def handle(self, *args: Any, **options: Any) -> None: @@ -82,12 +116,21 @@ def handle(self, *args: Any, **options: Any) -> None: f"and add up to {note_account_count * options['notes_per_account']} note(s) " f"across {note_account_count} account(s)." ) + if options["with_health_demo"]: + demo_account_count = min(len(HEALTH_DEMO_COUNTS), len(groups)) + event_count = self._health_demo_event_count(demo_account_count) + self.stdout.write( + f"Would also define/update {len(HEALTH_DEMO_METRICS)} demo usage metric(s) and queue " + f"{event_count} event(s) across the first {demo_account_count} account(s) for the Health column." + ) return self._set_config(team) user_pool = self._ensure_user_pool(team, options["users"]) accounts = self._create_accounts(team, groups, user_pool) self._create_notes(team, accounts, user_pool, options["accounts_with_notes"], options["notes_per_account"]) + if options["with_health_demo"]: + self._create_health_demo(team, accounts) self.stdout.write(self.style.SUCCESS("Done.")) def _get_team(self, team_id: int) -> Team: @@ -218,6 +261,86 @@ def _create_notes( created += 1 self.stdout.write(f"Created {created} note(s) across up to {len(selected)} account(s).") + @staticmethod + def _health_demo_event_count(account_count: int) -> int: + per_account = sum(current + previous for current, previous in HEALTH_DEMO_COUNTS[:account_count]) + return per_account * len(HEALTH_DEMO_METRICS) + + def _ensure_health_demo_metrics(self, team: Team) -> list[GroupUsageMetric]: + metrics: list[GroupUsageMetric] = [] + for name, event_name in HEALTH_DEMO_METRICS: + metric, _ = GroupUsageMetric.objects.update_or_create( + team=team, + group_type_index=ACCOUNT_GROUP_TYPE_INDEX, + name=name, + defaults={ + "interval": HEALTH_DEMO_INTERVAL_DAYS, + "math": GroupUsageMetric.Math.COUNT, + "format": GroupUsageMetric.Format.NUMERIC, + "display": GroupUsageMetric.Display.NUMBER, + "filters": {"events": [{"id": event_name, "type": "events", "order": 0}]}, + }, + ) + metrics.append(metric) + return metrics + + def _emit_demo_events( + self, team: Team, external_id: str, distinct_id: str, event_name: str, period: str, count: int, timestamp: Any + ) -> int: + group_property = f"$group_{ACCOUNT_GROUP_TYPE_INDEX}" + for i in range(count): + # Keep identifiers deterministic so this fixture is reproducible after its prior rows are cleared. + event_uuid = uuid.uuid5(HEALTH_DEMO_UUID_NAMESPACE, f"{team.id}:{external_id}:{event_name}:{period}:{i}") + create_event( + event_uuid=event_uuid, + event=event_name, + team=team, + distinct_id=distinct_id, + timestamp=timestamp, + properties={group_property: external_id}, + ) + return count + + def _create_health_demo(self, team: Team, accounts: list[Account]) -> None: + self._clear_health_demo_events(team) + metrics = self._ensure_health_demo_metrics(team) + # Only accounts with an external id can carry group-attributed events. + demo_accounts = [account for account in accounts if account.external_id][: len(HEALTH_DEMO_COUNTS)] + now = timezone.now() + # Current period is the trailing `interval` days; the previous period is the one before it. + # Place events safely inside each window so the relative timestamps stay valid when scored. + current_ts = now - timedelta(days=3) + previous_ts = now - timedelta(days=HEALTH_DEMO_INTERVAL_DAYS + 3) + created = 0 + for account_index, account in enumerate(demo_accounts): + external_id = account.external_id + assert external_id is not None # filtered above; reassures the type checker + current_count, previous_count = HEALTH_DEMO_COUNTS[account_index] + distinct_id = f"ca-health-demo-{external_id}" + for _name, event_name in HEALTH_DEMO_METRICS: + created += self._emit_demo_events( + team, external_id, distinct_id, event_name, "current", current_count, current_ts + ) + created += self._emit_demo_events( + team, external_id, distinct_id, event_name, "previous", previous_count, previous_ts + ) + self.stdout.write( + f"Defined/updated {len(metrics)} demo usage metric(s) and queued {created} event(s) " + f"across {len(demo_accounts)} account(s) for the Health column." + ) + + def _clear_health_demo_events(self, team: Team) -> None: + # Stable UUIDs alone are insufficient because the events table key also includes the event + # date. Remove this seeder's namespaced events before recreating rolling-window fixtures. + sync_execute( + f"ALTER TABLE {EVENTS_DATA_TABLE()} DELETE WHERE team_id = %(team_id)s " + "AND event IN %(event_names)s SETTINGS mutations_sync=1", + { + "team_id": team.id, + "event_names": tuple(event_name for _, event_name in HEALTH_DEMO_METRICS), + }, + ) + def _paragraph_doc(text: str) -> dict[str, Any]: return {"type": "doc", "content": [{"type": "paragraph", "content": [{"type": "text", "text": text}]}]} diff --git a/products/customer_analytics/backend/management/commands/test/test_seed_customer_analytics_accounts.py b/products/customer_analytics/backend/management/commands/test/test_seed_customer_analytics_accounts.py index 3e3a50530e20..7a05bb5a3bb8 100644 --- a/products/customer_analytics/backend/management/commands/test/test_seed_customer_analytics_accounts.py +++ b/products/customer_analytics/backend/management/commands/test/test_seed_customer_analytics_accounts.py @@ -1,16 +1,20 @@ from io import StringIO from posthog.test.base import BaseTest +from unittest.mock import patch from django.core.management import call_command from django.core.management.base import CommandError from posthog.models import Group, OrganizationMembership, User +from posthog.models.group_usage_metric import GroupUsageMetric from products.customer_analytics.backend.models.account import Account from products.customer_analytics.backend.models.team_customer_analytics_config import TeamCustomerAnalyticsConfig from products.notebooks.backend.models import Notebook, ResourceNotebook +SEED_MODULE = "products.customer_analytics.backend.management.commands.seed_customer_analytics_accounts" + class TestSeedCustomerAnalyticsAccounts(BaseTest): def _make_group(self, group_key: str, name: str) -> None: @@ -98,3 +102,82 @@ def test_dry_run_writes_nothing(self): def test_errors_when_no_groups(self): with self.assertRaises(CommandError): self._run() + + def test_default_run_seeds_no_usage_metrics_or_events(self): + self._make_group("acme-id", "Acme") + self._make_group("globex-id", "Globex") + + with patch(f"{SEED_MODULE}.create_event") as create_event: + self._run(users=2, accounts_with_notes=0) + + # The default (health demo off) stays event-free, Kafka-free, and defines no usage metrics. + create_event.assert_not_called() + assert GroupUsageMetric.objects.filter(team=self.team).count() == 0 + + def test_health_demo_creates_two_metrics_and_queues_62_events(self): + self._make_group("acme-id", "Acme") + self._make_group("globex-id", "Globex") + self._make_group("initech-id", "Initech") + + with ( + patch(f"{SEED_MODULE}.create_event") as create_event, + patch(f"{SEED_MODULE}.sync_execute") as sync_execute, + ): + self._run(users=2, accounts_with_notes=0, with_health_demo=True) + + metrics = GroupUsageMetric.objects.filter(team=self.team).order_by("name") + assert [m.name for m in metrics] == [ + "Customer analytics demo: Active users", + "Customer analytics demo: Events ingested", + ] + assert all(m.interval == 7 and m.math == GroupUsageMetric.Math.COUNT for m in metrics) + sync_execute.assert_called_once() + + # Two metrics × first two external-id accounts: (9+10)+(2+10) totals = 31 per metric → 62. + assert create_event.call_count == 62 + + def test_health_demo_is_idempotent_on_metrics(self): + self._make_group("acme-id", "Acme") + self._make_group("globex-id", "Globex") + + with patch(f"{SEED_MODULE}.create_event"), patch(f"{SEED_MODULE}.sync_execute") as sync_execute: + self._run(users=2, accounts_with_notes=0, with_health_demo=True) + self._run(users=2, accounts_with_notes=0, with_health_demo=True) + + assert GroupUsageMetric.objects.filter(team=self.team).count() == 2 + assert sync_execute.call_count == 2 + + def test_health_demo_does_not_overwrite_existing_metrics(self): + self._make_group("acme-id", "Acme") + self._make_group("globex-id", "Globex") + existing = GroupUsageMetric.objects.create( + team=self.team, + group_type_index=0, + name="Events ingested", + interval=30, + math=GroupUsageMetric.Math.SUM, + math_property="bytes", + filters={"events": [{"id": "real_event", "type": "events", "order": 0}]}, + ) + + with patch(f"{SEED_MODULE}.create_event"), patch(f"{SEED_MODULE}.sync_execute"): + self._run(users=2, accounts_with_notes=0, with_health_demo=True) + + existing.refresh_from_db() + assert existing.interval == 30 + assert existing.math == GroupUsageMetric.Math.SUM + assert GroupUsageMetric.objects.filter(team=self.team).count() == 3 + + def test_health_demo_dry_run_describes_optional_work_and_writes_nothing(self): + self._make_group("acme-id", "Acme") + self._make_group("globex-id", "Globex") + + with patch(f"{SEED_MODULE}.create_event") as create_event: + output = self._run(dry_run=True, with_health_demo=True) + + assert "Dry run" in output + assert "62 event(s)" in output + assert "demo usage metric(s)" in output + create_event.assert_not_called() + assert GroupUsageMetric.objects.filter(team=self.team).count() == 0 + assert Account.objects.for_team(self.team.pk).count() == 0 diff --git a/products/customer_analytics/backend/services/account_health.py b/products/customer_analytics/backend/services/account_health.py new file mode 100644 index 000000000000..d5c43acdce36 --- /dev/null +++ b/products/customer_analytics/backend/services/account_health.py @@ -0,0 +1,381 @@ +"""Explainable account health scoring built on a team's GroupUsageMetric definitions. + +Each metric becomes a *factor*: how much of the immediately preceding period's usage was +retained in the current period (current / previous), capped at 100. The overall score is the +rounded mean of the non-null factor scores, which maps to a coarse health bucket. The scoring +functions are pure (no DB / ClickHouse) so they can be unit tested directly; the batched HogQL +evaluation lives in :class:`AccountHealthScorer`. + +The evaluation deliberately mirrors ``UsageMetricsQueryRunner`` (same source-descriptor grouping, +same period windows, same event/data-warehouse handling) but batches every account on the current +page into a single query per source+interval group — one ``GROUP BY`` over the group key — instead +of one query per account, so the accounts list stays free of N+1 queries. +""" + +from collections.abc import Iterable +from copy import deepcopy +from datetime import datetime, timedelta +from functools import cached_property +from zoneinfo import ZoneInfo + +from posthog.schema import AccountHealthFactor, AccountHealthScore, AccountHealthStatus, HogQLQueryModifiers + +from posthog.hogql import ast +from posthog.hogql.parser import parse_expr +from posthog.hogql.query import execute_hogql_query +from posthog.hogql.timings import HogQLTimings + +from posthog.clickhouse.query_tagging import tag_contains_user_hogql +from posthog.models import Team, User +from posthog.models.group_usage_metric import GroupUsageMetric + +# A source descriptor groups metrics that read the same underlying table. Events metrics all share +# ``(EVENTS,)``; each data warehouse table/timestamp/key combination is its own descriptor. +SourceDescriptor = tuple[str, ...] + +# Score thresholds for the coarse health buckets. ``>= HEALTHY`` is healthy, ``>= NEEDS_ATTENTION`` +# needs attention, below that is at risk. +HEALTHY_THRESHOLD = 80 +NEEDS_ATTENTION_THRESHOLD = 50 + + +def _clamp(value: float, low: float, high: float) -> float: + return max(low, min(high, value)) + + +def compute_factor_score(current: float, previous: float) -> int | None: + """Retained-usage score for one factor, 0–100. + + - ``previous > 0``: the share of last period's usage retained this period, capped at 100. + - ``previous == 0`` and ``current > 0``: brand-new usage, treated as fully healthy (100). + - both zero: no signal, returns ``None`` (excluded from the overall mean). + """ + if previous > 0: + return round(_clamp(current / previous * 100, 0, 100)) + if current > 0: + return 100 + return None + + +def compute_change_pct(current: float, previous: float) -> float | None: + """Percentage change current vs previous, or ``None`` when there is no previous baseline.""" + if previous > 0: + return (current - previous) / previous * 100 + return None + + +def compute_overall_score(factor_scores: Iterable[int | None]) -> int | None: + """Rounded mean of the non-null factor scores, or ``None`` when nothing scored.""" + scored = [score for score in factor_scores if score is not None] + if not scored: + return None + return round(sum(scored) / len(scored)) + + +def status_for_score(score: int | None) -> AccountHealthStatus: + if score is None: + return AccountHealthStatus.NO_DATA + if score >= HEALTHY_THRESHOLD: + return AccountHealthStatus.HEALTHY + if score >= NEEDS_ATTENTION_THRESHOLD: + return AccountHealthStatus.NEEDS_ATTENTION + return AccountHealthStatus.AT_RISK + + +def no_data_score() -> AccountHealthScore: + """The score for accounts we cannot evaluate (no external id / config / metrics / signal).""" + return AccountHealthScore(score=None, status=AccountHealthStatus.NO_DATA, factors=[]) + + +class AccountHealthScorer: + def __init__( + self, + team: Team, + *, + timings: HogQLTimings | None = None, + modifiers: HogQLQueryModifiers | None = None, + user: User | None = None, + ) -> None: + self.team = team + self.timings = timings or HogQLTimings() + self.modifiers = modifiers + self.user = user + + @cached_property + def account_group_type_index(self) -> int | None: + return self.team.customer_analytics_config.account_group_type_index + + @cached_property + def usage_metrics(self) -> list[GroupUsageMetric]: + # Mirrors UsageMetricsQueryRunner: every team-owned metric applies, regardless of the + # metric's own (legacy) group_type_index. + with self.timings.measure("account_health_get_usage_metrics"): + return list( + GroupUsageMetric.objects.filter(team=self.team).only( + "id", "name", "interval", "filters", "math", "math_property" + ) + ) + + def usage_metric_fingerprints(self) -> list[tuple[str, ...]]: + """Stable per-metric fingerprints for the query cache key, sorted for determinism.""" + return sorted( + (str(m.id), m.name, m.math, m.math_property or "", str(m.filters), str(m.interval)) + for m in self.usage_metrics + ) + + def config_fingerprint(self) -> int | None: + """The piece of team config that changes scores — the group type accounts join on.""" + return self.account_group_type_index + + def score_external_ids(self, external_ids: Iterable[str | None]) -> dict[str, AccountHealthScore]: + """Score the given (current-page) external ids. Returns a map keyed by external id. + + Accounts with a missing external id are not included in the map — the caller resolves + them to :func:`no_data_score`. When the team has no config or no usable metrics every + present external id maps to no_data. + """ + present_ids = sorted({external_id for external_id in external_ids if external_id}) + if not present_ids: + return {} + + if self.account_group_type_index is None: + return {external_id: no_data_score() for external_id in present_ids} + + source_groups = self._group_metrics_by_source_and_interval() + if not source_groups: + return {external_id: no_data_score() for external_id in present_ids} + + date_to = datetime.now(tz=ZoneInfo("UTC")) + # metric_values[external_id][metric_id] = (current_total, previous_total) + metric_values: dict[str, dict[str, tuple[float, float]]] = {} + for (source_descriptor, interval), group in source_groups.items(): + self._collect_group_values(source_descriptor, interval, group, present_ids, date_to, metric_values) + + ordered_metrics = self._ordered_metrics(source_groups) + return { + external_id: self._score_for(external_id, ordered_metrics, metric_values.get(external_id, {})) + for external_id in present_ids + } + + def _score_for( + self, + external_id: str, + ordered_metrics: list[GroupUsageMetric], + values_by_metric: dict[str, tuple[float, float]], + ) -> AccountHealthScore: + factors: list[AccountHealthFactor] = [] + factor_scores: list[int | None] = [] + for metric in ordered_metrics: + current, previous = values_by_metric.get(str(metric.id), (0.0, 0.0)) + factor_score = compute_factor_score(current, previous) + factor_scores.append(factor_score) + factors.append( + AccountHealthFactor( + metric_id=str(metric.id), + metric_name=metric.name, + interval=metric.interval, + current=current, + previous=previous, + factor_score=factor_score, + change_pct=compute_change_pct(current, previous), + ) + ) + + overall = compute_overall_score(factor_scores) + if overall is None: + # Metrics exist but this account has no usable signal — surface the (all no-signal) + # factors so the detail view can still explain what was evaluated. + return AccountHealthScore(score=None, status=AccountHealthStatus.NO_DATA, factors=factors) + return AccountHealthScore(score=overall, status=status_for_score(overall), factors=factors) + + @staticmethod + def _source_descriptor(metric: GroupUsageMetric) -> SourceDescriptor: + if metric.is_data_warehouse: + filters = metric.filters or {} + return ( + GroupUsageMetric.Source.DATA_WAREHOUSE, + filters.get("table_name"), + filters.get("timestamp_field"), + filters.get("key_field"), + ) + return (GroupUsageMetric.Source.EVENTS,) + + def _group_metrics_by_source_and_interval( + self, + ) -> dict[tuple[SourceDescriptor, int], list[tuple[GroupUsageMetric, ast.Expr]]]: + groups: dict[tuple[SourceDescriptor, int], list[tuple[GroupUsageMetric, ast.Expr]]] = {} + for metric in self.usage_metrics: + if metric.math == GroupUsageMetric.Math.SUM and not metric.math_property: + continue + source_descriptor = self._source_descriptor(metric) + if source_descriptor[0] == GroupUsageMetric.Source.DATA_WAREHOUSE and ( + len(source_descriptor) != 4 or not all(source_descriptor[1:]) + ): + continue + with self.timings.measure("account_health_metric_filter_expr"): + filter_expr = metric.get_expr() + if not metric.is_data_warehouse and filter_expr == ast.Constant(value=True): + # An events metric with no real filter would scan all events; skip it. + continue + key = (source_descriptor, metric.interval) + groups.setdefault(key, []).append((metric, filter_expr)) + return groups + + @staticmethod + def _ordered_metrics( + source_groups: dict[tuple[SourceDescriptor, int], list[tuple[GroupUsageMetric, ast.Expr]]], + ) -> list[GroupUsageMetric]: + metrics = [metric for group in source_groups.values() for metric, _ in group] + # Deterministic factor order regardless of grouping/query order. + return sorted(metrics, key=lambda m: (m.name, str(m.id))) + + def _collect_group_values( + self, + source_descriptor: SourceDescriptor, + interval: int, + group: list[tuple[GroupUsageMetric, ast.Expr]], + external_ids: list[str], + date_to: datetime, + metric_values: dict[str, dict[str, tuple[float, float]]], + ) -> None: + query = self._build_group_query(source_descriptor, interval, group, external_ids, date_to) + with self.timings.measure(f"account_health_{source_descriptor[0]}_{interval}_execute"): + response = execute_hogql_query( + query_type="account_health_query", + query=query, + team=self.team, + user=self.user, + timings=self.timings, + modifiers=self.modifiers, + ) + for row in response.results or []: + external_id = str(row[0]) + for i, (metric, _filter_expr) in enumerate(group): + current = float(row[1 + i * 2] or 0) + previous = float(row[2 + i * 2] or 0) + metric_values.setdefault(external_id, {})[str(metric.id)] = (current, previous) + + def _build_group_query( + self, + source_descriptor: SourceDescriptor, + interval: int, + group: list[tuple[GroupUsageMetric, ast.Expr]], + external_ids: list[str], + date_to: datetime, + ) -> ast.SelectQuery: + # Each helper returns a fresh AST node per call — the HogQL resolver mutates ``node.type`` + # in place, so a single node cannot be shared across multiple positions in the tree. + date_from = date_to - timedelta(days=interval) + prev_date_from = date_to - 2 * timedelta(days=interval) + + current_condition = self._period_condition(source_descriptor, date_from, date_to) + previous_condition = self._period_condition(source_descriptor, prev_date_from, date_from, upper_exclusive=True) + + select_exprs: list[ast.Expr] = [ast.Alias(alias="group_key", expr=self._group_key_expr(source_descriptor))] + for i, (metric, filter_expr) in enumerate(group): + value_expr, prev_expr = self._conditional_aggregation( + metric, filter_expr, current_condition, previous_condition + ) + select_exprs.append(ast.Alias(alias=f"m{i}_value", expr=value_expr)) + select_exprs.append(ast.Alias(alias=f"m{i}_previous", expr=prev_expr)) + + where_exprs: list[ast.Expr] = [ + ast.CompareOperation( + op=ast.CompareOperationOp.In, + left=self._group_key_expr(source_descriptor), + right=ast.Tuple(exprs=[ast.Constant(value=external_id) for external_id in external_ids]), + ), + ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=self._timestamp_expr(source_descriptor), + right=ast.Constant(value=prev_date_from), + ), + ast.CompareOperation( + op=ast.CompareOperationOp.LtEq, + left=self._timestamp_expr(source_descriptor), + right=ast.Constant(value=date_to), + ), + ] + + return ast.SelectQuery( + select=select_exprs, + select_from=ast.JoinExpr(table=self._table_expr(source_descriptor)), + where=ast.And(exprs=where_exprs), + group_by=[ast.Field(chain=["group_key"])], + ) + + def _group_key_expr(self, source_descriptor: SourceDescriptor) -> ast.Expr: + if source_descriptor[0] == GroupUsageMetric.Source.DATA_WAREHOUSE: + key_field = source_descriptor[3] + if not key_field: + raise ValueError("data_warehouse usage metric is missing 'key_field' in filters") + tag_contains_user_hogql() + return ast.Call(name="toString", args=[parse_expr(key_field)]) + return ast.Field(chain=[f"$group_{self.account_group_type_index}"]) + + def _table_expr(self, source_descriptor: SourceDescriptor) -> ast.Field: + if source_descriptor[0] == GroupUsageMetric.Source.DATA_WAREHOUSE: + return ast.Field(chain=[source_descriptor[1]]) + return ast.Field(chain=["events"]) + + def _timestamp_expr(self, source_descriptor: SourceDescriptor) -> ast.Expr: + if source_descriptor[0] == GroupUsageMetric.Source.DATA_WAREHOUSE: + timestamp_field = source_descriptor[2] + if not timestamp_field: + raise ValueError("data_warehouse usage metric is missing 'timestamp_field' in filters") + tag_contains_user_hogql() + return parse_expr(timestamp_field) + return ast.Field(chain=["timestamp"]) + + def _period_condition( + self, + source_descriptor: SourceDescriptor, + period_from: datetime, + period_to: datetime, + upper_exclusive: bool = False, + ) -> ast.Expr: + upper_op = ast.CompareOperationOp.Lt if upper_exclusive else ast.CompareOperationOp.LtEq + return ast.And( + exprs=[ + ast.CompareOperation( + op=ast.CompareOperationOp.GtEq, + left=self._timestamp_expr(source_descriptor), + right=ast.Constant(value=period_from), + ), + ast.CompareOperation( + op=upper_op, + left=self._timestamp_expr(source_descriptor), + right=ast.Constant(value=period_to), + ), + ] + ) + + def _conditional_aggregation( + self, + metric: GroupUsageMetric, + filter_expr: ast.Expr, + current_condition: ast.Expr, + previous_condition: ast.Expr, + ) -> tuple[ast.Expr, ast.Expr]: + current_cond = ast.And(exprs=[deepcopy(filter_expr), deepcopy(current_condition)]) + previous_cond = ast.And(exprs=[deepcopy(filter_expr), deepcopy(previous_condition)]) + + if metric.math == GroupUsageMetric.Math.SUM: + if metric.is_data_warehouse: + tag_contains_user_hogql() + value_arg: ast.Expr = ast.Call(name="toFloat", args=[parse_expr(metric.math_property)]) + else: + value_arg = ast.Call(name="toFloat", args=[ast.Field(chain=["properties", metric.math_property])]) + return ( + ast.Call( + name="ifNull", args=[ast.Call(name="sumIf", args=[value_arg, current_cond]), ast.Constant(value=0)] + ), + ast.Call( + name="ifNull", args=[ast.Call(name="sumIf", args=[value_arg, previous_cond]), ast.Constant(value=0)] + ), + ) + + return ( + ast.Call(name="toFloat", args=[ast.Call(name="countIf", args=[current_cond])]), + ast.Call(name="toFloat", args=[ast.Call(name="countIf", args=[previous_cond])]), + ) diff --git a/products/customer_analytics/backend/services/test/test_account_health.py b/products/customer_analytics/backend/services/test/test_account_health.py new file mode 100644 index 000000000000..d27dd577b49b --- /dev/null +++ b/products/customer_analytics/backend/services/test/test_account_health.py @@ -0,0 +1,86 @@ +from django.test import SimpleTestCase + +from parameterized import parameterized + +from posthog.schema import AccountHealthStatus + +from posthog.hogql import ast + +from posthog.models.group_usage_metric import GroupUsageMetric + +from products.customer_analytics.backend.services.account_health import ( + AccountHealthScorer, + compute_change_pct, + compute_factor_score, + compute_overall_score, + no_data_score, + status_for_score, +) + + +class TestAccountHealthScoring(SimpleTestCase): + @parameterized.expand( + [ + ("retained", 9.0, 10.0, 90), + ("fully_retained", 10.0, 10.0, 100), + ("capped_when_grew", 50.0, 10.0, 100), + ("partial", 2.0, 10.0, 20), + ("fully_churned", 0.0, 10.0, 0), + ("new_usage", 5.0, 0.0, 100), + ("no_signal", 0.0, 0.0, None), + ] + ) + def test_compute_factor_score(self, _name, current, previous, expected): + self.assertEqual(compute_factor_score(current, previous), expected) + + @parameterized.expand( + [ + ("decline", 9.0, 10.0, -10.0), + ("growth", 12.0, 10.0, 20.0), + ("no_baseline", 5.0, 0.0, None), + ("both_zero", 0.0, 0.0, None), + ] + ) + def test_compute_change_pct(self, _name, current, previous, expected): + self.assertEqual(compute_change_pct(current, previous), expected) + + @parameterized.expand( + [ + ("simple_average", [90, 90], 90), + ("rounds_mean", [92, 90], 91), + ("ignores_nulls", [None, 90], 90), + ("all_null", [None, None], None), + ("empty", [], None), + ] + ) + def test_compute_overall_score(self, _name, factor_scores, expected): + self.assertEqual(compute_overall_score(factor_scores), expected) + + @parameterized.expand( + [ + ("top", 100, AccountHealthStatus.HEALTHY), + ("healthy_boundary", 80, AccountHealthStatus.HEALTHY), + ("needs_attention_high", 79, AccountHealthStatus.NEEDS_ATTENTION), + ("needs_attention_boundary", 50, AccountHealthStatus.NEEDS_ATTENTION), + ("at_risk_boundary", 49, AccountHealthStatus.AT_RISK), + ("at_risk_floor", 0, AccountHealthStatus.AT_RISK), + ("none_is_no_data", None, AccountHealthStatus.NO_DATA), + ] + ) + def test_status_for_score(self, _name, score, expected): + self.assertEqual(status_for_score(score), expected) + + def test_no_data_score(self): + score = no_data_score() + self.assertIsNone(score.score) + self.assertEqual(score.status, AccountHealthStatus.NO_DATA) + self.assertEqual(score.factors, []) + + def test_data_warehouse_group_keys_are_normalized_to_strings(self): + scorer = object.__new__(AccountHealthScorer) + expression = scorer._group_key_expr( + (GroupUsageMetric.Source.DATA_WAREHOUSE, "warehouse_table", "created_at", "numeric_account_id") + ) + + assert isinstance(expression, ast.Call) + assert expression.name == "toString" diff --git a/products/customer_analytics/frontend/components/Accounts/AGENTS.md b/products/customer_analytics/frontend/components/Accounts/AGENTS.md index 9dc1223875b0..8b624bf71227 100644 --- a/products/customer_analytics/frontend/components/Accounts/AGENTS.md +++ b/products/customer_analytics/frontend/components/Accounts/AGENTS.md @@ -47,15 +47,20 @@ AccountsTabContent ── binds dataNodeLogic(ACCOUNTS_HOGQL_DATA_NODE_KEY, acc `accountsLogic.hogqlQuery` builds a `DataTableNode` wrapping an `AccountsQuery` (`select`, plus optional `search`, `tagNames`, `allRolesUnassigned`, `assignedToUserIds`, `filterExpression`, `metrics`, `orderBy`). The backend runner (`accounts_query_runner`) returns **rows as arrays** aligned to `visibleColumnNames`. `assignedToUserIds` is the "assigned to" filter — a list of user ids the runner expands into `csm IN ids OR account_executive IN ids` (the single user-facing role filter; there are no separate per-role CSM/AE/owner filters). The `allRolesUnassigned` flag (the "Unassigned only" option, surfaced inside the "Assigned to" picker — mutually exclusive with picking people via the cascade in `accountsLogic` listeners) restricts to accounts with no csm/AE/owner. The "My accounts" checkbox is a client-side shortcut: `accountsLogic` resolves it to `[currentUserId]` (from `userLogic`) before the query is sent, so the backend only ever receives explicit ids and a shared URL resolves to the same accounts for every viewer. Two cell shapes matter: - **`name` column** (mandatory, `ACCOUNTS_NAME_COLUMN`) — emitted as `tuple(name, external_id, id)`, read as `{ name, external_id, id }`. This is the row's identity: `id` (the account PK) drives expansion/scroll/role updates; `external_id` is the copy-able group key. `getNameCell()` in `AccountsHogQLTable.tsx` is the canonical accessor; never assume a column index. +- **`health_score` column** (mandatory + synthetic, `ACCOUNTS_HEALTH_COLUMN`) — not a real `system.accounts` field. The backend (`services/account_health.py`) computes an explainable `AccountHealthScore` (`{ score, status, factors }`) per row from the team's `GroupUsageMetric` definitions and injects it into the result row. Parse it with `parseAccountHealth()` (`accountHealth.ts`); render with the `HealthCell` (a `NN/100` + compact status `LemonTag`). It has **no sort affordance** (synthetic) and clicking it opens the row's Health tab via `accountsExpansionLogic.openAccountTab(id, 'health')`. Callers that omit `health_score` from `select` do **zero** health work (no `GroupUsageMetric` query, no fingerprints in the cache key). - **role columns** (`csm`, `account_executive`, `account_owner`) — emitted as `tuple(id, email)`, rendered with `MemberSelect`. Sorting these uses `tupleElement(col, 2)` (email) so visual order matches. -Default columns (`ACCOUNTS_HOGQL_DEFAULT_SELECT`): `name`, `tag_names`, `notebook_count`, `csm`, `account_executive`, `account_owner`. The name column is force-kept (`ensureNameColumn`) — removing it breaks identity, scroll, and role edits. Extra columns come from account properties, lazy/virtual-table joins under `system.accounts`, data-warehouse joins, or freeform SQL — all assembled by `buildAccountColumnGroups`. +Default columns (`ACCOUNTS_HOGQL_DEFAULT_SELECT`): `name`, `health_score`, `tag_names`, `notebook_count`, `csm`, `account_executive`, `account_owner`. Both `name` and `health_score` are force-kept (`ensureMandatoryColumns` — `name` first, `health_score` pinned immediately after it) and can't be removed in the configurator; this also upgrades older saved column configs and shared view hashes that predate the health column. Extra columns come from account properties, lazy/virtual-table joins under `system.accounts`, data-warehouse joins, or freeform SQL — all assembled by `buildAccountColumnGroups`. Sort safety: removing the sorted column drops the sort (`clearSortIfColumnRemoved`), else the backend gets an `orderBy` referencing a missing alias. ## The expanded row -`AccountsHogQLTable.useExpandable()` makes expansion **controlled** by `accountsExpansionLogic`: `isRowExpanded` reads `expandedAccountIds`, `onRowExpand`/`onRowCollapse` dispatch `toggleAccountExpanded`. The body is `AccountNotebooksExpansion`, a `LemonTabs` over `notes` / `users` / `usage` (`AccountExpansionTab`) plus the Useful links sidebar. Active tab comes from `activeTabFor(accountId)` (defaults to `notes`). +`AccountsHogQLTable.useExpandable()` makes expansion **controlled** by `accountsExpansionLogic`: `isRowExpanded` reads `expandedAccountIds`, `onRowExpand`/`onRowCollapse` dispatch `toggleAccountExpanded`. The body is `AccountNotebooksExpansion`, a `LemonTabs` over `health` / `notes` / `users` / `usage` / `spend` (`AccountExpansionTab`) plus the Useful links sidebar. Active tab comes from `activeTabFor(accountId)` (defaults to `health`). + +The **Health tab** (`AccountHealthExpansion`) is the default. It reads the row's already-fetched `health_score` cell (no extra API call) and renders a compact overall-score visual, an "Account health" heading, the concise scoring formula, and one factor card per usage metric (current vs previous, per-factor score + colored progress, and delta / "New" / "No signal" text). When the status is `no_data` it links to `${urls.customerAnalyticsConfiguration()}?tab=customer-analytics-usage-metrics`. + +**Notes loading is deferred.** The Notes table lives in `AccountNotesTab`, which mounts `accountNotebooksLogic` only when the Notes tab is the active tab (LemonTabs renders just the active content). Expanding straight to Health therefore never fetches notes. Keep notebook loading inside `AccountNotesTab` — don't hoist `accountNotebooksLogic` back up into `AccountNotebooksExpansion`, or every expansion will refetch notes. The Usage tab renders an existing saved billing-usage insight — **point users to it, don't rebuild usage as a new insight.** @@ -115,7 +120,7 @@ We track user actions on the Accounts list with `posthog.capture()`. Conventions | `customer analytics account role assigned` | `accountsLogic` `updateAccountRole` | `role` (`csm` \| `account_executive` \| `account_owner`), `is_assigned`, `assigned_user_id`, `source` (always `list_row` today) | | `customer analytics account link clicked` | `AccountNotebooksExpansion.tsx` useful-link `onClick` | `link_key`, `has_destination` | | `customer analytics account note clicked` | `AccountNotebooksExpansion.tsx` note `` `onClick` | `notebook_short_id` | -| `customer analytics account tab viewed` | `accountsExpansionLogic` `setActiveTab` listener (genuine tab clicks only; programmatic `openAccountTab` navigation does not fire it) | `tab` (`notes` \| `users` \| `usage`) | +| `customer analytics account tab viewed` | `accountsExpansionLogic` `setActiveTab` listener (genuine tab clicks only; programmatic `openAccountTab` navigation does not fire it) | `tab` (`health` \| `notes` \| `users` \| `usage` \| `spend`) | | `customer analytics account related user clicked` | `AccountRelatedUsersExpansion.tsx` user `` `onClick` | _(none — customer end-user PII kept out)_ | > **Keep this table up to date.** Whenever you add, rename, or remove a `posthog.capture()` event in the Accounts area — or change its properties — update this table in the same change. An agent reading this file should be able to trust it as the source of truth for what the Accounts list reports. diff --git a/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.test.tsx b/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.test.tsx new file mode 100644 index 000000000000..379b999bb926 --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.test.tsx @@ -0,0 +1,31 @@ +import { render, screen } from '@testing-library/react' + +import { AccountHealthExpansion } from './AccountHealthExpansion' + +describe('AccountHealthExpansion', () => { + it('renders configured factors when an account has no signal', () => { + render( + + ) + + expect(screen.getByText('Events ingested')).toBeTruthy() + expect(screen.getByText('No signal in either period')).toBeTruthy() + expect(screen.queryByText('Set up usage metrics')).toBeNull() + }) +}) diff --git a/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.tsx b/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.tsx new file mode 100644 index 000000000000..511c3f8a736f --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/AccountHealthExpansion.tsx @@ -0,0 +1,120 @@ +import { LemonTag, Link } from '@posthog/lemon-ui' + +import { LemonProgress } from 'lib/lemon-ui/LemonProgress' +import { humanFriendlyNumber } from 'lib/utils' +import { urls } from 'scenes/urls' + +import type { AccountHealthFactor, AccountHealthScore } from '~/queries/schema/schema-general' + +import { HEALTH_STATUS_LABEL, healthScoreColor, healthStatusTagType } from './accountHealth' + +// Where to send users to define usage metrics when there's nothing to score against. +const USAGE_METRICS_CONFIG_URL = `${urls.customerAnalyticsConfiguration()}?tab=customer-analytics-usage-metrics` + +function formatValue(value: number): string { + return humanFriendlyNumber(value, 2) +} + +function FactorTrend({ factor }: { factor: AccountHealthFactor }): JSX.Element { + if (factor.factor_score === null) { + return No signal in either period + } + if (factor.change_pct === null) { + // Usage this period with no prior baseline — brand new. + return New this period + } + const up = factor.change_pct >= 0 + return ( + + {`${up ? '+' : ''}${Math.round(factor.change_pct)}%`} + + ) +} + +function FactorCard({ factor }: { factor: AccountHealthFactor }): JSX.Element { + const score = factor.factor_score + const color = healthScoreColor(score) + return ( +
+
+
+ {factor.metric_name} + {`${factor.interval}-day window`} +
+ + {score === null ? '—' : score} + +
+ +
+ + {`${formatValue(factor.current)} now · ${formatValue(factor.previous)} before`} + + +
+
+ ) +} + +function NoHealthData(): JSX.Element { + return ( +
+

Account health

+

+ We don't have enough usage data to score this account yet. Health scores are derived from the activity + measured by your team's usage metrics. +

+ Set up usage metrics +
+ ) +} + +export function AccountHealthExpansion({ health }: { health: AccountHealthScore | null }): JSX.Element { + if (!health || (health.status === 'no_data' && health.factors.length === 0)) { + return + } + + const score = health.score + const color = healthScoreColor(score) + + return ( +
+
+
+ + {score ?? '—'} + + out of 100 +
+
+
+

Account health

+ + {HEALTH_STATUS_LABEL[health.status]} + +
+

+ Each factor is how much of the previous period's usage was retained this period (current ÷ + previous, capped at 100). The overall score is the average of the factor scores. +

+
+
+
+ {health.factors.map((factor) => ( + + ))} +
+
+ ) +} diff --git a/products/customer_analytics/frontend/components/Accounts/AccountNotebooksExpansion.tsx b/products/customer_analytics/frontend/components/Accounts/AccountNotebooksExpansion.tsx index d2c21985556e..6b9c960a0022 100644 --- a/products/customer_analytics/frontend/components/Accounts/AccountNotebooksExpansion.tsx +++ b/products/customer_analytics/frontend/components/Accounts/AccountNotebooksExpansion.tsx @@ -1,7 +1,7 @@ import { useActions, useValues } from 'kea' import posthog from 'posthog-js' -import { IconGraph, IconPeople, IconPiggyBank, IconReceipt } from '@posthog/icons' +import { IconGraph, IconHeartFilled, IconPeople, IconPiggyBank, IconReceipt } from '@posthog/icons' import { LemonButton, LemonSkeleton, @@ -17,9 +17,12 @@ import { IconSlack } from 'lib/lemon-ui/icons' import { fullName } from 'lib/utils' import { urls } from 'scenes/urls' +import type { AccountHealthScore } from '~/queries/schema/schema-general' + import type { AccountNotebookApi } from 'products/customer_analytics/frontend/generated/api.schemas' import { AccountBillingExpansion } from './AccountBillingExpansion' +import { AccountHealthExpansion } from './AccountHealthExpansion' import { accountLinksLogic } from './accountLinksLogic' import { accountNotebooksLogic } from './accountNotebooksLogic' import { AccountRelatedUsersExpansion } from './AccountRelatedUsersExpansion' @@ -86,18 +89,10 @@ function UsefulLinks({ accountId }: { accountId: string }): JSX.Element { ) } -export function AccountNotebooksExpansion({ - accountId, - externalId, -}: { - accountId: string - externalId: string -}): JSX.Element { - const logic = accountNotebooksLogic({ accountId }) - const { notebooks, notebooksLoading } = useValues(logic) - const { activeTabFor } = useValues(accountsExpansionLogic) - const { setActiveTab } = useActions(accountsExpansionLogic) - const activeTab = activeTabFor(accountId) +// Mounted only when the Notes tab is active (LemonTabs renders just the active tab's content), so +// expanding straight to the Health tab never triggers the notebooks API call. +function AccountNotesTab({ accountId }: { accountId: string }): JSX.Element { + const { notebooks, notebooksLoading } = useValues(accountNotebooksLogic({ accountId })) const columns: LemonTableColumns = [ { @@ -156,6 +151,32 @@ export function AccountNotebooksExpansion({ }, ] + return ( + + size="small" + embedded + dataSource={notebooks ?? []} + rowKey="short_id" + loading={notebooksLoading} + columns={columns} + emptyState={notebooks === null ? 'Failed to load account notes.' : 'No notes linked to this account yet.'} + /> + ) +} + +export function AccountNotebooksExpansion({ + accountId, + externalId, + health, +}: { + accountId: string + externalId: string + health: AccountHealthScore | null +}): JSX.Element { + const { activeTabFor } = useValues(accountsExpansionLogic) + const { setActiveTab } = useActions(accountsExpansionLogic) + const activeTab = activeTabFor(accountId) + return (
@@ -168,24 +189,20 @@ export function AccountNotebooksExpansion({ onChange={(tab) => setActiveTab(accountId, tab)} size="small" tabs={[ + { + key: 'health', + label: ( + + + Health + + ), + content: , + }, { key: 'notes', label: 'Notes', - content: ( - - size="small" - embedded - dataSource={notebooks ?? []} - rowKey="short_id" - loading={notebooksLoading} - columns={columns} - emptyState={ - notebooks === null - ? 'Failed to load account notes.' - : 'No notes linked to this account yet.' - } - /> - ), + content: , }, { key: 'users', diff --git a/products/customer_analytics/frontend/components/Accounts/AccountsColumnConfigurator.tsx b/products/customer_analytics/frontend/components/Accounts/AccountsColumnConfigurator.tsx index 4abeef592dab..18320c6f5301 100644 --- a/products/customer_analytics/frontend/components/Accounts/AccountsColumnConfigurator.tsx +++ b/products/customer_analytics/frontend/components/Accounts/AccountsColumnConfigurator.tsx @@ -16,6 +16,7 @@ import { Tooltip } from 'lib/lemon-ui/Tooltip' import { extractDisplayLabel } from '~/queries/nodes/DataTable/utils' import { + ACCOUNTS_HEALTH_COLUMN, ACCOUNTS_NAME_COLUMN, AccountColumnGroup, AccountColumnGroupKey, @@ -140,9 +141,14 @@ function SelectedAccountColumn({ }): JSX.Element { const { setNodeRef, attributes, transform, transition, listeners } = useSortable({ id: column }) const label = extractDisplayLabel(column) - // `name` carries the row identity (account id) and external_id for the - // Account cell — removing it would break row expansion and role updates. - const isMandatory = column === ACCOUNTS_NAME_COLUMN + // `name` carries the row identity (account id) and external_id for the Account cell — removing + // it would break row expansion and role updates. `health_score` is synthetic and always shown: + // it's the at-a-glance health summary the list is built around, so it can't be removed either. + const isMandatory = column === ACCOUNTS_NAME_COLUMN || column === ACCOUNTS_HEALTH_COLUMN + const mandatoryReason = + column === ACCOUNTS_HEALTH_COLUMN + ? 'Health is always shown — it summarizes each account from your usage metrics' + : 'This column is required' return (
)} - + onRemove(column)} status="danger" size="small" - disabledReason={isMandatory ? 'This column is required' : undefined} + disabledReason={isMandatory ? mandatoryReason : undefined} > diff --git a/products/customer_analytics/frontend/components/Accounts/AccountsHogQLTable.tsx b/products/customer_analytics/frontend/components/Accounts/AccountsHogQLTable.tsx index 80586e1f1804..8c1fd945994b 100644 --- a/products/customer_analytics/frontend/components/Accounts/AccountsHogQLTable.tsx +++ b/products/customer_analytics/frontend/components/Accounts/AccountsHogQLTable.tsx @@ -1,7 +1,7 @@ import { useActions, useValues } from 'kea' import { useMemo } from 'react' -import { LemonButton, LemonSkeleton, LemonTable, ProfilePicture } from '@posthog/lemon-ui' +import { LemonButton, LemonSkeleton, LemonTable, LemonTag, ProfilePicture } from '@posthog/lemon-ui' import { CopyToClipboardInline } from 'lib/components/CopyToClipboard' import { MemberSelect } from 'lib/components/MemberSelect' @@ -15,8 +15,9 @@ import { DataTableNode } from '~/queries/schema/schema-general' import { QueryContext, QueryContextColumn, QueryContextColumnComponent } from '~/queries/types' import { ACCOUNTS_HOGQL_DATA_NODE_KEY } from '../../constants' +import { HEALTH_STATUS_LABEL, healthScoreColor, healthStatusTagType, parseAccountHealth } from './accountHealth' import { AccountNotebooksExpansion } from './AccountNotebooksExpansion' -import { ACCOUNTS_NAME_COLUMN, accountsColumnConfigLogic } from './accountsColumnConfigLogic' +import { ACCOUNTS_HEALTH_COLUMN, ACCOUNTS_NAME_COLUMN, accountsColumnConfigLogic } from './accountsColumnConfigLogic' import { accountsExpansionLogic } from './accountsExpansionLogic' import { AccountRoleKey, accountsLogic } from './accountsLogic' @@ -33,6 +34,7 @@ const ROLE_LABELS: Record = { const COLUMN_WIDTHS = { name: '240px', + health_score: '170px', tag_names: '280px', notebook_count: '80px', csm: '220px', @@ -98,6 +100,45 @@ function NameCell({ record }: { record: unknown }): JSX.Element { ) } +function HealthCell({ record }: { record: unknown }): JSX.Element { + const { visibleColumnNames } = useValues(accountsColumnConfigLogic) + const { openAccountTab } = useActions(accountsExpansionLogic) + const accountId = getNameCell(record, visibleColumnNames)?.id + const health = parseAccountHealth(getCellAt(record, visibleColumnNames, ACCOUNTS_HEALTH_COLUMN)) + const hasScore = !!health && health.status !== 'no_data' && health.score !== null + return ( + + ) +} + function TagsCell({ record }: { record: unknown }): JSX.Element { const getCell = useGetCell() const raw = getCell(record, 'tag_names') @@ -189,6 +230,8 @@ type KnownColumnTemplate = { label?: string width?: string render?: QueryContextColumnComponent + /** Synthetic columns (health) have no backing expression to sort by. */ + sortable?: boolean } const KNOWN_COLUMN_TEMPLATES: Record = { @@ -197,6 +240,12 @@ const KNOWN_COLUMN_TEMPLATES: Record = { width: COLUMN_WIDTHS.name, render: ({ record }) => , }, + health_score: { + label: 'Health', + width: COLUMN_WIDTHS.health_score, + render: ({ record }) => , + sortable: false, + }, tag_names: { label: 'Tags', width: COLUMN_WIDTHS.tag_names, @@ -232,7 +281,10 @@ function useContextColumns(): Record { const template = KNOWN_COLUMN_TEMPLATES[key] const label = template?.label ?? key columns[key] = { - renderTitle: () => , + renderTitle: + template?.sortable === false + ? () => {label} + : () => , width: template?.width, render: template?.render, } @@ -267,8 +319,13 @@ function useExpandable(): QueryContext['expandable'] { }, expandedRowRender: ({ result }) => { const cell = getNameCell(result, visibleColumnNames) + const health = parseAccountHealth(getCellAt(result, visibleColumnNames, ACCOUNTS_HEALTH_COLUMN)) return cell ? ( - + ) : null }, }), diff --git a/products/customer_analytics/frontend/components/Accounts/AccountsTab.stories.tsx b/products/customer_analytics/frontend/components/Accounts/AccountsTab.stories.tsx index dd59607bd0a7..0985d39c9e29 100644 --- a/products/customer_analytics/frontend/components/Accounts/AccountsTab.stories.tsx +++ b/products/customer_analytics/frontend/components/Accounts/AccountsTab.stories.tsx @@ -16,12 +16,34 @@ const INSIGHTS_ENDPOINT = 'api/environments/:team_id/insights/' type AccountNameCell = { name: string; external_id: string | null; id: string } type AccountRoleCell = [number, string] | null -type AccountRow = [AccountNameCell, string[], number, AccountRoleCell, AccountRoleCell, AccountRoleCell] +type AccountHealthFactorCell = { + metric_id: string + metric_name: string + interval: number + current: number + previous: number + factor_score: number | null + change_pct: number | null +} +type AccountHealthCell = { + score: number | null + status: 'healthy' | 'needs_attention' | 'at_risk' | 'no_data' + factors: AccountHealthFactorCell[] +} +type AccountRow = [ + AccountNameCell, + AccountHealthCell, + string[], + number, + AccountRoleCell, + AccountRoleCell, + AccountRoleCell, +] function buildAccountsQueryResponse(rows: AccountRow[]): Record { return { kind: 'AccountsQuery', - columns: ['name', 'tag_names', 'notebook_count', 'csm', 'account_executive', 'account_owner'], + columns: ['name', 'health_score', 'tag_names', 'notebook_count', 'csm', 'account_executive', 'account_owner'], results: rows, types: [], hogql: '', @@ -33,18 +55,71 @@ function buildAccountsQueryResponse(rows: AccountRow[]): Record } } +// Deterministic health cells matching the AccountHealthScore shape the backend injects per row. +const ACME_HEALTH: AccountHealthCell = { + score: 91, + status: 'healthy', + factors: [ + { + metric_id: 'm-events', + metric_name: 'Events ingested', + interval: 7, + current: 920, + previous: 1000, + factor_score: 92, + change_pct: -8, + }, + { + metric_id: 'm-users', + metric_name: 'Active users', + interval: 7, + current: 90, + previous: 100, + factor_score: 90, + change_pct: -10, + }, + ], +} +const GLOBEX_HEALTH: AccountHealthCell = { + score: 32, + status: 'at_risk', + factors: [ + { + metric_id: 'm-events', + metric_name: 'Events ingested', + interval: 7, + current: 30, + previous: 100, + factor_score: 30, + change_pct: -70, + }, + { + metric_id: 'm-users', + metric_name: 'Active users', + interval: 7, + current: 34, + previous: 100, + factor_score: 34, + change_pct: -66, + }, + ], +} +const NO_HEALTH_DATA: AccountHealthCell = { score: null, status: 'no_data', factors: [] } + const SAMPLE_ROWS: AccountRow[] = [ [ { name: 'Acme Inc', external_id: 'cust_acme_001', id: 'acc-1' }, + ACME_HEALTH, ['enterprise', 'priority'], 0, [1, 'alice@posthog.com'], [2, 'bob@posthog.com'], null, ], - [{ name: 'Globex', external_id: 'cust_globex_002', id: 'acc-2' }, [], 0, null, null, null], + [{ name: 'Globex', external_id: 'cust_globex_002', id: 'acc-2' }, GLOBEX_HEALTH, [], 0, null, null, null], [ { name: 'Hooli', external_id: null, id: 'acc-3' }, + NO_HEALTH_DATA, ['scaleup'], 0, [1, 'alice@posthog.com'], @@ -56,6 +131,7 @@ const SAMPLE_ROWS: AccountRow[] = [ const SINGLE_ROW: AccountRow[] = [ [ { name: 'Acme Inc', external_id: 'cust_acme_001', id: 'acc-1' }, + ACME_HEALTH, ['enterprise', 'priority'], 1, [1, 'alice@posthog.com'], @@ -177,16 +253,15 @@ function billingTabDecorators( ] } -// Expanding a row mounts UsefulLinks (loads the account async) and the notes table -// (loads notebooks async). Both start as skeletons and resolve later, which changes -// the expansion's width and height. Awaiting the settled content here keeps the -// snapshot deterministic — otherwise it races the loads and the Useful links sidebar -// is sometimes absent, sometimes present (the flaky ~7% height/width diff). +// Rows now expand to the Health tab by default, so the notes-focused stories click the Notes tab +// after expanding. Awaiting the settled sidebar + notes content keeps the snapshot deterministic — +// otherwise it races the async loads (the flaky ~7% height/width diff). async function expandFirstRow(canvasElement: HTMLElement, notesLoadedText: string): Promise { const canvas = within(canvasElement) await userEvent.click(await canvas.findByTitle('Show more')) await canvas.findByText('Useful links') await canvas.findByText('Organization') + await userEvent.click(await canvas.findByRole('tab', { name: 'Notes' })) await canvas.findByText(notesLoadedText) } @@ -374,3 +449,31 @@ export const RowExpandedUsagePopulated: Story = { await expandAndOpenTab(canvasElement, 'Usage') }, } + +// Clicking a Health cell expands the row straight to the Health tab (no manual expand/tab click) +// and surfaces the factor breakdown — without fetching notes. +export const HealthCellOpensDetail: Story = { + render: () => , + decorators: [ + mswDecorator({ + get: { + [ACCOUNT_RETRIEVE_ENDPOINT]: ACCOUNT_WITH_LINKS, + [ACCOUNT_NOTEBOOKS_ENDPOINT]: { count: 0, next: null, previous: null, results: [] }, + }, + post: { + [QUERY_ENDPOINT]: mockAccountsQuery(SAMPLE_ROWS), + }, + }), + ], + play: async ({ canvasElement }) => { + const canvas = within(canvasElement) + // The list shows the score for the healthy account; clicking it opens the Health tab. + await userEvent.click(await canvas.findByText('91/100')) + await canvas.findByText('Useful links') + await canvas.findByText('Account health') + // Factor names and the overall score render in the Health detail. + await canvas.findByText('Events ingested') + await canvas.findByText('Active users') + await canvas.findByText('91') + }, +} diff --git a/products/customer_analytics/frontend/components/Accounts/accountHealth.test.ts b/products/customer_analytics/frontend/components/Accounts/accountHealth.test.ts new file mode 100644 index 000000000000..ee37e2a7f83a --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/accountHealth.test.ts @@ -0,0 +1,119 @@ +import type { AccountHealthStatus } from '~/queries/schema/schema-general' + +import { healthScoreColor, healthStatusTagType, parseAccountHealth } from './accountHealth' + +describe('accountHealth', () => { + describe('parseAccountHealth', () => { + it('parses a full health score with factors', () => { + const parsed = parseAccountHealth({ + score: 91, + status: 'healthy', + factors: [ + { + metric_id: 'm-events', + metric_name: 'Events ingested', + interval: 7, + current: 920, + previous: 1000, + factor_score: 92, + change_pct: -8, + }, + ], + }) + expect(parsed).toEqual({ + score: 91, + status: 'healthy', + factors: [ + { + metric_id: 'm-events', + metric_name: 'Events ingested', + interval: 7, + current: 920, + previous: 1000, + factor_score: 92, + change_pct: -8, + }, + ], + }) + }) + + it('parses a no_data score with no factors', () => { + expect(parseAccountHealth({ score: null, status: 'no_data', factors: [] })).toEqual({ + score: null, + status: 'no_data', + factors: [], + }) + }) + + it('coerces a non-numeric score and nullable fields to null', () => { + const parsed = parseAccountHealth({ + score: 'oops', + status: 'at_risk', + factors: [ + { + metric_id: 'm', + metric_name: 'Metric', + interval: 7, + current: 0, + previous: 0, + factor_score: null, + change_pct: null, + }, + ], + }) + expect(parsed?.score).toBeNull() + expect(parsed?.factors[0].factor_score).toBeNull() + expect(parsed?.factors[0].change_pct).toBeNull() + }) + + it('drops malformed factors but keeps valid ones', () => { + const parsed = parseAccountHealth({ + score: 50, + status: 'needs_attention', + factors: [ + { metric_name: 'missing id' }, + { + metric_id: 'ok', + metric_name: 'Good', + interval: 7, + current: 1, + previous: 2, + factor_score: 50, + change_pct: -50, + }, + ], + }) + expect(parsed?.factors).toHaveLength(1) + expect(parsed?.factors[0].metric_id).toBe('ok') + }) + + it.each([[null], [undefined], ['health'], [42], [{ status: 'bogus', factors: [] }], [{ factors: [] }]])( + 'returns null for unrecognized cell %p', + (value) => { + expect(parseAccountHealth(value)).toBeNull() + } + ) + }) + + describe('healthStatusTagType', () => { + it.each([ + ['healthy', 'success'], + ['needs_attention', 'warning'], + ['at_risk', 'danger'], + ['no_data', 'muted'], + ] as [AccountHealthStatus, string][])('maps %s to the %s tag', (status, expected) => { + expect(healthStatusTagType(status)).toBe(expected) + }) + }) + + describe('healthScoreColor', () => { + it('buckets scores by threshold and handles null', () => { + expect(healthScoreColor(90)).toBe('var(--success)') + expect(healthScoreColor(80)).toBe('var(--success)') + expect(healthScoreColor(60)).toBe('var(--warning)') + expect(healthScoreColor(50)).toBe('var(--warning)') + expect(healthScoreColor(20)).toBe('var(--danger)') + expect(healthScoreColor(null)).toBe('var(--color-text-tertiary)') + }) + }) +}) diff --git a/products/customer_analytics/frontend/components/Accounts/accountHealth.ts b/products/customer_analytics/frontend/components/Accounts/accountHealth.ts new file mode 100644 index 000000000000..294edacb3495 --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/accountHealth.ts @@ -0,0 +1,87 @@ +import type { LemonTagType } from '@posthog/lemon-ui' + +import type { AccountHealthFactor, AccountHealthScore, AccountHealthStatus } from '~/queries/schema/schema-general' + +// Score thresholds, mirrored from the backend (products/customer_analytics/backend/services/account_health.py). +export const HEALTHY_THRESHOLD = 80 +export const NEEDS_ATTENTION_THRESHOLD = 50 + +const VALID_STATUSES: AccountHealthStatus[] = ['healthy', 'needs_attention', 'at_risk', 'no_data'] + +export const HEALTH_STATUS_LABEL: Record = { + healthy: 'Healthy', + needs_attention: 'Needs attention', + at_risk: 'At risk', + no_data: 'No data', +} + +export function healthStatusTagType(status: AccountHealthStatus): LemonTagType { + switch (status) { + case 'healthy': + return 'success' + case 'needs_attention': + return 'warning' + case 'at_risk': + return 'danger' + default: + return 'muted' + } +} + +// CSS color for a 0–100 score (overall or per-factor progress), bucketed by the same thresholds. +export function healthScoreColor(score: number | null): string { + if (score === null) { + return 'var(--color-text-tertiary)' + } + if (score >= HEALTHY_THRESHOLD) { + return 'var(--success)' + } + if (score >= NEEDS_ATTENTION_THRESHOLD) { + return 'var(--warning)' + } + return 'var(--danger)' +} + +function asNumberOrNull(value: unknown): number | null { + return typeof value === 'number' && Number.isFinite(value) ? value : null +} + +function parseFactor(value: unknown): AccountHealthFactor | null { + if (!value || typeof value !== 'object') { + return null + } + const factor = value as Record + if (typeof factor.metric_id !== 'string' || typeof factor.metric_name !== 'string') { + return null + } + return { + metric_id: factor.metric_id, + metric_name: factor.metric_name, + interval: typeof factor.interval === 'number' ? factor.interval : 0, + current: typeof factor.current === 'number' ? factor.current : 0, + previous: typeof factor.previous === 'number' ? factor.previous : 0, + factor_score: asNumberOrNull(factor.factor_score), + change_pct: asNumberOrNull(factor.change_pct), + } +} + +// Parse the synthetic `health_score` cell the backend injects into account rows. Returns null for +// anything that isn't a recognizable AccountHealthScore so callers can fall back gracefully. +export function parseAccountHealth(value: unknown): AccountHealthScore | null { + if (!value || typeof value !== 'object') { + return null + } + const cell = value as Record + const status = cell.status + if (typeof status !== 'string' || !VALID_STATUSES.includes(status as AccountHealthStatus)) { + return null + } + const factors = Array.isArray(cell.factors) + ? cell.factors.map(parseFactor).filter((factor): factor is AccountHealthFactor => factor !== null) + : [] + return { + score: asNumberOrNull(cell.score), + status: status as AccountHealthStatus, + factors, + } +} diff --git a/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.test.ts b/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.test.ts new file mode 100644 index 000000000000..8482991910d9 --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.test.ts @@ -0,0 +1,58 @@ +import { + ACCOUNTS_HEALTH_COLUMN, + ACCOUNTS_HOGQL_DEFAULT_SELECT, + ACCOUNTS_NAME_COLUMN, + ensureMandatoryColumns, +} from './accountsColumnConfigLogic' + +describe('accountsColumnConfigLogic columns', () => { + describe('ACCOUNTS_HOGQL_DEFAULT_SELECT', () => { + it('puts the mandatory name and health columns first, in that order', () => { + expect(ACCOUNTS_HOGQL_DEFAULT_SELECT[0]).toBe(ACCOUNTS_NAME_COLUMN) + expect(ACCOUNTS_HOGQL_DEFAULT_SELECT[1]).toBe(ACCOUNTS_HEALTH_COLUMN) + }) + }) + + describe('ensureMandatoryColumns', () => { + it('adds both mandatory columns to an empty list', () => { + expect(ensureMandatoryColumns([])).toEqual([ACCOUNTS_NAME_COLUMN, ACCOUNTS_HEALTH_COLUMN]) + }) + + it('upgrades a legacy config that predates the health column', () => { + expect(ensureMandatoryColumns([ACCOUNTS_NAME_COLUMN, 'csm', 'account_owner'])).toEqual([ + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'csm', + 'account_owner', + ]) + }) + + it('pins health right after name wherever name sits', () => { + expect(ensureMandatoryColumns(['csm', ACCOUNTS_NAME_COLUMN, 'account_owner'])).toEqual([ + 'csm', + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'account_owner', + ]) + }) + + it('is idempotent on an already-upgraded list', () => { + const upgraded = [ACCOUNTS_NAME_COLUMN, ACCOUNTS_HEALTH_COLUMN, 'csm'] + expect(ensureMandatoryColumns(upgraded)).toEqual(upgraded) + }) + + it('moves a misplaced health column back to right after name', () => { + expect(ensureMandatoryColumns([ACCOUNTS_NAME_COLUMN, 'csm', ACCOUNTS_HEALTH_COLUMN])).toEqual([ + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'csm', + ]) + }) + + it('replaces an expression that aliases the reserved health column', () => { + expect( + ensureMandatoryColumns([ACCOUNTS_NAME_COLUMN, 'properties.plan AS health_score', 'account_owner']) + ).toEqual([ACCOUNTS_NAME_COLUMN, ACCOUNTS_HEALTH_COLUMN, 'account_owner']) + }) + }) +}) diff --git a/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.ts b/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.ts index bd489c1281a6..5154e6eba55f 100644 --- a/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.ts +++ b/products/customer_analytics/frontend/components/Accounts/accountsColumnConfigLogic.ts @@ -22,8 +22,15 @@ import { AccountsEvents } from './constants' // row identity (id) and copy-able external_id ride along with the display name. export const ACCOUNTS_NAME_COLUMN = 'name' +// Synthetic, mandatory column. The backend computes an explainable AccountHealthScore per row from +// the team's usage-metric definitions; it isn't a real `system.accounts` field. We force-keep it +// directly after `name` so it's the first signal a user sees and survives older saved column +// configs / shared view hashes that predate it. +export const ACCOUNTS_HEALTH_COLUMN = 'health_score' + export const ACCOUNTS_HOGQL_DEFAULT_SELECT: string[] = [ ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, 'accounts.tags.names AS tag_names', 'accounts.notebooks.count AS notebook_count', 'csm', @@ -35,6 +42,21 @@ function ensureNameColumn(columns: string[]): string[] { return columns.includes(ACCOUNTS_NAME_COLUMN) ? columns : [ACCOUNTS_NAME_COLUMN, ...columns] } +// Pin the synthetic health column immediately after the (mandatory) name column. Idempotent, so it +// upgrades legacy column lists in place without duplicating or reordering anything else. +function ensureHealthColumn(columns: string[]): string[] { + // Drop any user expression that aliases itself to the reserved synthetic column. Keeping both + // would create duplicate display keys and make the detail view read the wrong cell. + const withoutHealth = columns.filter((column) => extractDisplayLabel(column) !== ACCOUNTS_HEALTH_COLUMN) + const nameIndex = withoutHealth.indexOf(ACCOUNTS_NAME_COLUMN) + const insertAt = nameIndex >= 0 ? nameIndex + 1 : 0 + return [...withoutHealth.slice(0, insertAt), ACCOUNTS_HEALTH_COLUMN, ...withoutHealth.slice(insertAt)] +} + +export function ensureMandatoryColumns(columns: string[]): string[] { + return ensureHealthColumn(ensureNameColumn(columns)) +} + export function diffColumnConfiguration( previous: string[], next: string[] @@ -209,10 +231,13 @@ export const accountsColumnConfigLogic = kea([ selectColumns: [ [...ACCOUNTS_HOGQL_DEFAULT_SELECT], { - setSelectColumns: (_, { columns }) => ensureNameColumn(columns), - selectColumn: (state, { column }) => (state.includes(column) ? state : [...state, column]), + setSelectColumns: (_, { columns }) => ensureMandatoryColumns(columns), + selectColumn: (state, { column }) => + ensureMandatoryColumns(state.includes(column) ? state : [...state, column]), unselectColumn: (state, { column }) => - column === ACCOUNTS_NAME_COLUMN ? state : state.filter((c) => c !== column), + column === ACCOUNTS_NAME_COLUMN || column === ACCOUNTS_HEALTH_COLUMN + ? state + : ensureMandatoryColumns(state.filter((c) => c !== column)), moveColumn: (state, { oldIndex, newIndex }) => { if (oldIndex === newIndex || oldIndex < 0 || oldIndex >= state.length) { return state @@ -220,7 +245,7 @@ export const accountsColumnConfigLogic = kea([ const next = [...state] const [removed] = next.splice(oldIndex, 1) next.splice(newIndex, 0, removed) - return next + return ensureMandatoryColumns(next) }, resetColumns: () => [...ACCOUNTS_HOGQL_DEFAULT_SELECT], }, diff --git a/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.test.ts b/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.test.ts new file mode 100644 index 000000000000..9da97dcee76d --- /dev/null +++ b/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.test.ts @@ -0,0 +1,47 @@ +import { expectLogic } from 'kea-test-utils' + +import { initKeaTests } from '~/test/init' + +import { accountsExpansionLogic, DEFAULT_ACCOUNT_TAB } from './accountsExpansionLogic' + +describe('accountsExpansionLogic', () => { + let logic: ReturnType + + beforeEach(() => { + initKeaTests() + logic = accountsExpansionLogic() + logic.mount() + }) + + afterEach(() => { + logic.unmount() + }) + + it('defaults a freshly expanded account to the Health tab', () => { + expect(DEFAULT_ACCOUNT_TAB).toBe('health') + logic.actions.toggleAccountExpanded('acc-1') + expect(logic.values.isAccountExpanded('acc-1')).toBe(true) + expect(logic.values.activeTabFor('acc-1')).toBe('health') + }) + + it('openAccountTab expands the row and selects the given tab', () => { + logic.actions.openAccountTab('acc-2', 'health') + expect(logic.values.expandedAccountIds).toContain('acc-2') + expect(logic.values.activeTabFor('acc-2')).toBe('health') + }) + + it('openAccountTab is idempotent for an already-expanded row', () => { + logic.actions.toggleAccountExpanded('acc-3') + logic.actions.setActiveTab('acc-3', 'notes') + logic.actions.openAccountTab('acc-3', 'health') + expect(logic.values.expandedAccountIds.filter((id) => id === 'acc-3')).toHaveLength(1) + expect(logic.values.activeTabFor('acc-3')).toBe('health') + }) + + it('does not fire the tab-viewed event for programmatic openAccountTab', async () => { + await expectLogic(logic, () => { + logic.actions.openAccountTab('acc-4', 'health') + }).toFinishAllListeners() + expect(logic.values.activeTabFor('acc-4')).toBe('health') + }) +}) diff --git a/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.ts b/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.ts index 35c8c7ca7215..c7eef5d1b958 100644 --- a/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.ts +++ b/products/customer_analytics/frontend/components/Accounts/accountsExpansionLogic.ts @@ -4,9 +4,9 @@ import posthog from 'posthog-js' import type { accountsExpansionLogicType } from './accountsExpansionLogicType' import { AccountsEvents } from './constants' -export type AccountExpansionTab = 'notes' | 'users' | 'usage' | 'spend' +export type AccountExpansionTab = 'health' | 'notes' | 'users' | 'usage' | 'spend' -export const DEFAULT_ACCOUNT_TAB: AccountExpansionTab = 'notes' +export const DEFAULT_ACCOUNT_TAB: AccountExpansionTab = 'health' export const accountsExpansionLogic = kea([ path(['scenes', 'customerAnalytics', 'accounts', 'accountsExpansionLogic']), diff --git a/products/customer_analytics/frontend/components/Accounts/accountsLogic.test.ts b/products/customer_analytics/frontend/components/Accounts/accountsLogic.test.ts index 8876c266375e..e3deff463906 100644 --- a/products/customer_analytics/frontend/components/Accounts/accountsLogic.test.ts +++ b/products/customer_analytics/frontend/components/Accounts/accountsLogic.test.ts @@ -14,6 +14,7 @@ import { accountsPartialUpdate, accountsRetrieve } from 'products/customer_analy import type { AccountApi } from 'products/customer_analytics/frontend/generated/api.schemas' import { + ACCOUNTS_HEALTH_COLUMN, ACCOUNTS_HOGQL_DEFAULT_SELECT, ACCOUNTS_NAME_COLUMN, accountsColumnConfigLogic, @@ -252,6 +253,12 @@ describe('accountsLogic', () => { logic.actions.toggleSort('name') expect(orderByOf(logic.values.hogqlQuery.source)).toEqual(['name DESC']) }) + + it('ignores attempts to sort by the synthetic health column', () => { + logic.actions.toggleSort(ACCOUNTS_HEALTH_COLUMN) + expect(logic.values.sortOrder).toBeNull() + expect(orderByOf(logic.values.hogqlQuery.source)).toBeUndefined() + }) }) describe('selectColumns', () => { @@ -273,16 +280,32 @@ describe('accountsLogic', () => { expect(config?.values.selectColumns).toContain(ACCOUNTS_NAME_COLUMN) }) - it('re-inserts the name column when setSelectColumns omits it', () => { + it('re-inserts the name and health columns when setSelectColumns omits them', () => { const config = accountsColumnConfigLogic.findMounted() config?.actions.setSelectColumns(['csm', 'account_executive']) - expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, 'csm', 'account_executive']) + expect(config?.values.selectColumns).toEqual([ + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'csm', + 'account_executive', + ]) }) - it('keeps user ordering when setSelectColumns already contains name', () => { + it('keeps user ordering but pins health right after name', () => { const config = accountsColumnConfigLogic.findMounted() config?.actions.setSelectColumns(['csm', ACCOUNTS_NAME_COLUMN, 'account_executive']) - expect(config?.values.selectColumns).toEqual(['csm', ACCOUNTS_NAME_COLUMN, 'account_executive']) + expect(config?.values.selectColumns).toEqual([ + 'csm', + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'account_executive', + ]) + }) + + it('refuses to remove the synthetic health column via unselectColumn', () => { + const config = accountsColumnConfigLogic.findMounted() + config?.actions.unselectColumn(ACCOUNTS_HEALTH_COLUMN) + expect(config?.values.selectColumns).toContain(ACCOUNTS_HEALTH_COLUMN) }) }) @@ -332,6 +355,18 @@ describe('accountsLogic', () => { expect(logic.values.tileFilter).toEqual(tileFilter) }) + it('drops a legacy sort that collides with the synthetic health column', async () => { + router.actions.push( + urls.customerAnalyticsAccounts(), + {}, + { view: { sort: { column: ACCOUNTS_HEALTH_COLUMN, direction: 'desc' } } } + ) + await expectLogic(logic).toFinishAllListeners() + + expect(logic.values.sortOrder).toBeNull() + expect(orderByOf(logic.values.hogqlQuery.source)).toBeUndefined() + }) + it('coerces a malformed scalar assignedTo from the view hash into an array', async () => { // normalizeRoleFilter defends the array-shaped filter against a stray // scalar in the hash (hand-edited or stale link) so .length/.map stay safe. @@ -341,7 +376,7 @@ describe('accountsLogic', () => { expect(logic.values.assignedToFilter).toEqual([7]) }) - it('restores columns and shields them from a late saved column config', async () => { + it('restores columns (upgrading legacy hashes to include health) and shields them from a late saved config', async () => { router.actions.push( urls.customerAnalyticsAccounts(), {}, @@ -352,7 +387,8 @@ describe('accountsLogic', () => { await expectLogic(logic).toFinishAllListeners() const config = accountsColumnConfigLogic.findMounted() - expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, 'csm']) + // A share hash that predates the health column is upgraded in place. + expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, ACCOUNTS_HEALTH_COLUMN, 'csm']) // A saved config arriving after the URL was applied must not clobber the shared view. config?.actions.loadSavedColumnConfigurationSuccess({ @@ -360,10 +396,10 @@ describe('accountsLogic', () => { columns: [ACCOUNTS_NAME_COLUMN, 'account_owner'], }) await expectLogic(config!).toFinishAllListeners() - expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, 'csm']) + expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, ACCOUNTS_HEALTH_COLUMN, 'csm']) }) - it('applies the saved column config when the URL has no columns', async () => { + it('applies the saved column config when the URL has no columns, upgrading it to include health', async () => { router.actions.push(urls.customerAnalyticsAccounts(), {}, {}) await expectLogic(logic).toFinishAllListeners() @@ -373,7 +409,11 @@ describe('accountsLogic', () => { columns: [ACCOUNTS_NAME_COLUMN, 'account_owner'], }) await expectLogic(config!).toFinishAllListeners() - expect(config?.values.selectColumns).toEqual([ACCOUNTS_NAME_COLUMN, 'account_owner']) + expect(config?.values.selectColumns).toEqual([ + ACCOUNTS_NAME_COLUMN, + ACCOUNTS_HEALTH_COLUMN, + 'account_owner', + ]) }) }) diff --git a/products/customer_analytics/frontend/components/Accounts/accountsLogic.ts b/products/customer_analytics/frontend/components/Accounts/accountsLogic.ts index da6f21de1882..0fb3a7c08201 100644 --- a/products/customer_analytics/frontend/components/Accounts/accountsLogic.ts +++ b/products/customer_analytics/frontend/components/Accounts/accountsLogic.ts @@ -20,6 +20,7 @@ import type { import { ACCOUNTS_HOGQL_DATA_NODE_KEY, CUSTOMER_ANALYTICS_DEFAULT_QUERY_TAGS } from '../../constants' import { + ACCOUNTS_HEALTH_COLUMN, ACCOUNTS_HOGQL_DEFAULT_SELECT, ACCOUNTS_NAME_COLUMN, accountsColumnConfigLogic, @@ -53,7 +54,7 @@ function clearSortIfColumnRemoved(values: SortLikeValues, actions: SortLikeActio if (!sort) { return } - if (!values.visibleColumnNames.includes(sort.column)) { + if (sort.column === ACCOUNTS_HEALTH_COLUMN || !values.visibleColumnNames.includes(sort.column)) { actions.setSortOrder(null) } } @@ -298,7 +299,7 @@ export const accountsLogic = kea([ if (assignedToFilter.length > 0) { state.assignedTo = assignedToFilter } - if (sortOrder) { + if (sortOrder && sortOrder.column !== ACCOUNTS_HEALTH_COLUMN) { state.sort = sortOrder } if (!objectsEqual(selectColumns, ACCOUNTS_HOGQL_DEFAULT_SELECT)) { @@ -429,6 +430,9 @@ export const accountsLogic = kea([ } }, toggleSort: ({ column }) => { + if (column === ACCOUNTS_HEALTH_COLUMN) { + return + } const current = values.sortOrder let next: AccountSortOrder if (!current || current.column !== column) { @@ -598,7 +602,7 @@ export const accountsLogic = kea([ actions.setAssignedToFilter(nextAssignedTo) } - const sort = view.sort ?? null + const sort = view.sort?.column === ACCOUNTS_HEALTH_COLUMN ? null : (view.sort ?? null) if (!objectsEqual(sort, values.sortOrder)) { actions.setSortOrder(sort) }