Repository navigation
feat(devex): move the data deletion request workflow into the admin console - #130730
Conversation
|
@codex review |
🤖 CI report
|
|
Risk: No findings This increment marks the auto-approvable "Submit for approval" operation for event-removal deletion requests as destructive ( Sentinel reviewed |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed c05ffe6: 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
c05ffe6 to
67d8d16
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67d8d1646e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
67d8d16 to
7e51ea6
Compare
There was a problem hiding this comment.
Not approved yet — waiting on the conditions below.
Re-add the stamphog label to request another review once you have addressed this.
The size gate refused this pull request. It has 1723 substantive lines of change, and the ceiling for auto-review is 1200. That count excludes docs, generated files and snapshots, and most of it comes from the new deletion_request_panels.py (577 lines) and test_deletion_request_console.py (365 lines). The other gates passed, but they don't override the size limit.
To move forward, you can ask a human reviewer to look at it. Alternatively, split it into smaller pull requests, for example the shell change that adds record previews, then the deletion request panels, then the removal of the old admin views and templates. Each piece would need to come in under the ceiling to be considered again.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✗ | too large for auto-review (1723L substantive in global — ceiling is 1200L; 1723L, 11F total, 2719L/16F incl. docs/generated/snapshots) |
| tier | ✓ | T1-agent / T1d-complex (2719L, 16F, two-areas, feat) |
| stamphog 2.7.0 | .stamphog/policy.yml @ unknown · reviewed head 7e51ea6 |
Bulk tool actions already show a preview and run exactly the confirmed input. Record actions such as approving a data deletion request need the same step: staff should see what gets deleted before it runs. The token target names the record type, primary key and panel, so a confirmation made for one record cannot confirm the same operation on another. Both runners now share one ConfirmStep instead of two copies of the token handling.
The data deletion request admin carried seven custom views and two templates for submitting, approving, retrying, verifying and fetching stats. They are counted by the no-new-custom-admin-views freeze and render outside the console's shared design and audit trail. Django admin keeps plain CRUD on requests (criteria form, edit lock, duplicate action); the workflow now lives on a deletion request record page in PostHog Admin. - New "deletion-request" record type with Overview, ClickHouse stats and Workflow panels, owned by ClickHouse. - Approve uses the record preview: staff see the criteria, stored stats, the adhoc deletion backlog and the execution mode before confirming. A backlog count failure shows a notice instead of blocking. - Approval is bound to the criteria revision (criteria_updated_at). A request edited and resubmitted between preview and confirm is no longer approved with criteria nobody reviewed. - Verify reads the criteria revision before counting and only completes that revision, so an edit during the ClickHouse count cannot complete different criteria. - Submit, approve and retry now need a reason, which goes to the audit log. Submit can lead to auto-approval, hence the reason there too. - Execution modes are limited per request type by the form choices instead of being coerced after submit. - Preview stats shows the numbers in a message instead of a session stash rendered on the change page. - Every state change still writes the same Django admin LogEntry, so the request history stays complete. - Old admin URLs redirect to the console form or panel, query string kept, for staff bookmarks. - The console operation page renders hidden form fields and their errors. Freeze findings drop from 165 to 148.
The approval preview showed counts for person targets and left out HogQL variable bindings, so two requests that delete different data could look the same. It now lists the person UUIDs and distinct IDs, up to 20 each with a count of the rest, and the variable bindings. The tests also run on a frozen clock now, because they derive the criteria revision from the current time.
Submitting an event removal without the opt-out lets the auto-approval job approve it with no further human step, so the submit authorizes a deletion. It now carries the risky styling and says how to stop it before the job runs.
|
😎 This pull request was merged. |
7e51ea6 to
090d6dc
Compare
|
This pull request was merged into |
Problem
Changes
How did you test this code?
Test rationale: the view tests move to the console. New cases: a token for one request cannot approve another, a stale approval, and verify after a criteria edit.
Release status
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, claude-opus-5-5
Skills:
/extending-the-admin-console,/writing-tests,/writing-pr-descriptions,/shipping-pr-programsMore context for AI agents
posthog/admin/console/deletion_request_panels.py(approve, verify),posthog/admin/console/views.py(ConfirmStep).