Skip to content

feat(devex): move deletion request editing into the admin console - #130755

Merged
trunk-io[bot] merged 3 commits into
feat/devex-console-record-previewfrom
feat/devex-console-deletion-request-editing
Nov 6, 2026
Merged

trunk-io[bot] merged 3 commits into
feat/devex-console-record-previewfrom
feat/devex-console-deletion-request-editing

Conversation

@webjunkie

@webjunkie webjunkie commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Changes

  • Creating, editing and duplicating a request move to the console: a "New deletion request" tool, plus a Criteria panel with edit and duplicate.
  • Django admin for deletion requests becomes read-only browsing with a link to the console. The old add URL redirects with its query string.
  • The save rules move next to the model unchanged, except a notes-only edit no longer resets a pending request.
  • An edit that races an approval fails with 409 instead of overwriting it.
  • Member-only request types are enforced on save, not only hidden in the form.
  • Freeze findings 148 → 146: no admin template override or custom form is left for this model.

Request page with the Criteria panel
Edit criteria form

How did you test this code?

Test rationale: the save rules are now no-database model tests. Console tests cover create, member-only types, the add URL redirect, pending reset, notes-only edits, no edit after approval, and duplicate.

  • Not checked on a running stack; pages checked as test renders.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 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-programs

More context for AI agents
  • Decisions: the request form inherits ModelForm and Form, because console operations type their form as type[forms.Form].
  • Decisions: the array preview script from the admin change page is dropped; the approval preview lists the targets instead.
  • Decisions: CodeRabbit pass skipped by owner preference.
  • Read closely: DataDeletionRequest.apply_staff_edit, edit_criteria in deletion_request_panels.py (lock and status re-check).

@webjunkie webjunkie self-assigned this Nov 6, 2026
@webjunkie
webjunkie added this pull request to stack #130717 November 6, 2026 09:33
@webjunkie
webjunkie requested review from a team November 6, 2026 09:33
@github-actions

github-actions Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ Duplication (Python) — clean

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Duplication (TypeScript) — clean

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

✅ Playwright — all passed

All tests passed.

View test results →

@webjunkie
webjunkie marked this pull request as ready for review November 6, 2026 10:15
@webjunkie

Copy link
Copy Markdown
Contributor Author

@codex review

@parameterai

parameterai Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Risk: No findings

The only change since the last review is a test-only import fix (PropertyDefinition now imported from products.event_definitions) in a pytest fixture; no production code or security-relevant surface was altered.

Sentinel reviewed 174ca40 · Review settings

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-06T10:17:40.175958Z 0df74e6 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 0df74e6551

ℹ️ 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".

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PostHog Review · Standard · reviewed 0df74e6: 2 Should fix.

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.

Comment thread posthog/models/data_deletion_request.py Resolved
Creating, editing and duplicating data deletion requests still ran through a Django admin change form with a template override, a custom ModelForm and save_model rules. That kept two findings of the no-new-custom-admin-views freeze and split one request's lifecycle between two UIs. The Django admin page is now read-only browsing; every write goes through PostHog Admin with the same audit LogEntry.

- New "New deletion request" tool (Data group). Support links prefill it from the query string as before, naive datetimes read as UTC, unparseable ones dropped. The old admin add URL redirects there with its query string.
- New Criteria panel on the deletion request page with "Edit criteria" (draft and pending only) and "Duplicate as a new draft".
- The criteria-change rules moved from save_model to DataDeletionRequest.apply_staff_edit, and the copy rules to duplicate_as_draft, so they sit next to the model instead of in an admin class.
- Property removal stays limited to the ClickHouse Team. The create form hides it from others, and both handlers refuse it, because record forms are built without the request.
- An edit locks the row and re-checks the status, so a concurrent approval cannot land on criteria that just moved back to draft.
- List fields compare parsed values, so a notes-only edit no longer counts as a criteria change and resets a pending request.
- The inline list preview script is gone with the template; the console has no per-form scripts.

Freeze findings drop from 148 to 146.
A list of (label, value) tuples is read positionally, which the devex tuple-return rule blocks for new code.
…ir checks

Review found two gaps in the console editing layer:
- an edit locked only the status, then saved the whole request it had
  read earlier, so a concurrent edit or stats fetch could be undone
  silently; the form now carries the request's updated_at, and the save
  refuses with 409 when it changed
- duplicating a request copied its type with no check, so staff outside
  the ClickHouse Team could mint property, person or HogQL removals the
  create form does not offer them; duplicate now needs the group for
  those types
@trunk-io
trunk-io Bot force-pushed the feat/devex-console-deletion-request-editing branch from 210a936 to 174ca40 Compare November 6, 2026 14:58
@trunk-io

trunk-io Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

@trunk-io
trunk-io Bot merged commit 89ff6df into master Nov 6, 2026
269 checks passed
@trunk-io
trunk-io Bot deleted the feat/devex-console-deletion-request-editing branch November 6, 2026 15:46
@deployment-status-posthog

deployment-status-posthog Bot commented Nov 6, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-06 16:11 UTC Run
prod-us ✅ Deployed 2026-11-06 16:23 UTC Run
prod-eu ✅ Deployed 2026-11-06 16:27 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants