fix(cohorts): return 400 for malformed legacy groups payload

[posthog]

Problem

A malformed legacy groups payload in the cohort API threw an uncaught ValueError and returned a 500 instead of a clean 400.

The create/update endpoint exposes the legacy groups field on CohortSerializer with no validation. When a request includes a group that has none of properties, action_id, or event_id, CohortManager.create calls Group(**group), and Group.__init__ raises a plain ValueError. DRF only converts ValidationError into a 400, so this escaped as an HTTP 500 out of perform_create. Low volume, but a real user-triggerable server error rather than a proper client-error response.

Changes

Added a validate_groups method to CohortSerializer that constructs each group through Group(...) and re-raises the resulting ValueError/TypeError as a DRF ValidationError. Bad input now returns a 400 with a helpful message on both create and update.

How did you test this code?

Added one endpoint test (test_creating_cohort_with_malformed_group_returns_400) that posts a group missing all of properties/action_id/event_id and asserts a 400 — locking in the 500→400 fix; no existing test exercised the malformed-group path. Ran ruff check and ruff format --check (both clean). The local sandbox lacks a fully provisioned Django/test runtime, so I was not able to execute the pytest suite here; the test should be run in CI.

🤖 Agent context

Autonomy: Fully autonomous

Authored by PostHog Code (Claude Opus 4.8). Invoked the /improving-drf-endpoints and /writing-tests skills while shaping the change. Chose field-level validate_groups reusing the real Group(...) constructor over a hand-rolled key check, so the validation stays in lockstep with what creation actually accepts and also hardens the update path.

---

Created with PostHog Code from an inbox report.

[trunk-io]

     

View Full Report ↗︎ ⋅ Docs

[scheduled-actions-posthog]

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the stale label – otherwise this will be closed in another week. If you want to permanently keep it open, use the waiting label.

[github-actions]

🤖 CI report

✅ Playwright — all passed

All tests passed.

View test results →

ℹ️ ClickHouse migration SQL — 1 migration(s)

ClickHouse migration SQL per cloud environment

• unset
  • all

    CREATE OR REPLACE VIEW custom_metrics_test
        AS SELECT
            'ClickHouseCustomMetric_Test' AS name,
            map('instance', hostname()) AS labels,
            1 AS value,
            'Test to check that the metric endpoint is working' AS help,
            'gauge' AS type

    CREATE OR REPLACE VIEW custom_metrics
        AS SELECT * REPLACE (toFloat64(value) as value)
        FROM custom_metrics_test

• US, EU, DEV
  • events, shufflehog

    CREATE OR REPLACE VIEW custom_metrics_test
        AS SELECT
            'ClickHouseCustomMetric_Test' AS name,
            map('instance', hostname()) AS labels,
            1 AS value,
            'Test to check that the metric endpoint is working' AS help,
            'gauge' AS type

    CREATE OR REPLACE VIEW custom_metrics
        AS SELECT * REPLACE (toFloat64(value) as value)
        FROM custom_metrics_test

[Copilot]

Pull request overview

Adds serializer-level validation for legacy cohort groups payloads so malformed input (previously raising a plain ValueError in Group.__init__) returns a client-facing HTTP 400 instead of an uncaught 500, and locks the behavior in with an endpoint test.

Changes:

• Added CohortSerializer.validate_groups to pre-validate legacy groups entries by constructing Group(**group) and converting ValueError/TypeError into DRF ValidationError.
• Added an API test ensuring a malformed legacy group returns HTTP 400 with an explanatory message.
• Imported Group into posthog/api/cohort.py for validation reuse.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
posthog/api/cohort.py	Adds serializer validation for legacy groups so malformed payloads return 400 instead of 500.
posthog/api/test/test_cohort.py	Adds regression test covering the malformed legacy groups payload case.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[deployment-status-posthog]

Deploy status

Environment	Status	Deployed At	Workflow
dev	✅ Deployed	2026-07-24 18:25 UTC	Run
prod-us	✅ Deployed	2026-07-24 18:46 UTC	Run
prod-eu	✅ Deployed	2026-07-24 18:48 UTC	Run