Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .agents/skills/extending-the-admin-console/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ Invoke `/writing-user-facing-copy`. In the console that means:
1. Point the operation at the old view with `AdminViewBridge(Model, SomeAdmin.view_method)` while the logic still lives in the `ModelAdmin`.
2. Replace the old button in Django admin with `MovedToConsole.action("<record>", obj, "<panel-key>", "<what moved>")`, so staff who look in the usual place find the new one.
3. When a whole page moves, keep its old URL name and route it to `MovedToConsole.page_redirect`, which tells staff the page moved.
4. Every old URL keeps working, query parameters included. Support staff keep bookmarks to old admin pages and actions. When you delete a record action's view, add its old path to `MOVED_TEAM_ACTIONS` or `MOVED_ORGANIZATION_ACTIONS` in `posthog/admin/console/urls.py`, so it redirects to the console form or panel.
4. Every old URL keeps working, query parameters included. Support staff keep bookmarks to old admin pages and actions. When you delete a record action's view, add its old path to the `MOVED_*_ACTIONS` map of its record type in `posthog/admin/console/urls.py`, so it redirects to the console form or panel.

## 6. Before you call it done

Expand Down
432 changes: 1 addition & 431 deletions posthog/admin/admins/data_deletion_request_admin.py

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions posthog/admin/console/AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Before you add or change a panel, operation or tool, invoke `/extending-the-admi

## Add a panel

1. In `products/<product>/backend/console.py`, subclass `Panel` and decorate it with `@console.register_panel`. Set `record` to a key from `RECORD_TYPES` in `records.py` (`team` or `organization`). Import only your product's `facade/`, `posthog.admin.console`, and, for a bridge, the `ModelAdmin` that still holds the view. `products/workflows/backend/console.py` is the reference.
1. In `products/<product>/backend/console.py`, subclass `Panel` and decorate it with `@console.register_panel`. Set `record` to a key from `RECORD_TYPES` in `records.py` (`team`, `organization` or `deletion-request`). Import only your product's `facade/`, `posthog.admin.console`, and, for a bridge, the `ModelAdmin` that still holds the view. `products/workflows/backend/console.py` is the reference.
2. Set `owner = Owner(name=..., crest=...)`. The crest is a mini crest slug from `@posthog/brand`, for example `workflows`.
3. For each action, return an `Operation`. Put its inputs in a Django `forms.Form` (`form_class`); the console renders and validates it. Its `handler(request, obj, form)` returns `None` when done. The handler is a service function, or `AdminViewBridge(Model, SomeAdmin.view_method)` while the logic still lives in a `ModelAdmin`. A bridged view must take a POST and answer with a redirect. Later, replace the bridge with a service function; the panel stays as it is.
4. Run `pnpm --filter=@posthog/frontend build:admin-console` when you add an owner crest or change a template. `posthog/admin/test_admin_console.py` fails when a template class, a linked asset or an owner crest is missing from the build output.
Expand All @@ -37,7 +37,7 @@ A tool is a page that is not about one record, for example a report across all t
## Move a Django admin action

Replace the old button with `MovedToConsole.action(record, obj, panel_key, label)` from `moved.py`. It links to the panel on the console record page, so staff who look in the usual place find the new one. When a whole page moves, route its old URL to `MovedToConsole.page_redirect(title, url)`, which tells staff the page moved.
Old URLs never break, because staff keep bookmarks to them, query parameters included. When a record action's view is deleted, its old path goes into `MOVED_TEAM_ACTIONS` or `MOVED_ORGANIZATION_ACTIONS` in `urls.py`, which redirects to the console form or panel.
Old URLs never break, because staff keep bookmarks to them, query parameters included. When a record action's view is deleted, its old path goes into the `MOVED_*_ACTIONS` map of its record type in `urls.py`, which redirects to the console form or panel.

## Read when needed

Expand Down
577 changes: 577 additions & 0 deletions posthog/admin/console/deletion_request_panels.py

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions posthog/admin/console/panels.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
from django.db.models import Model
from django.http import HttpRequest, HttpResponse

from posthog.admin.console.tools import Page


class Tone(StrEnum):
DEFAULT = "default"
Expand Down Expand Up @@ -86,6 +88,15 @@ class OperationHandler(Protocol):
def __call__(self, request: "HttpRequest", obj: "Model", form: forms.Form | None) -> "HttpResponse | None": ...


class RecordPreviewHandler(Protocol):
"""Shows what an operation would change on the record before it runs, as a `Page`. It must not change anything.

To refuse the input, add an error to the form; the console then shows the form again.
"""

def __call__(self, request: "HttpRequest", obj: "Model", form: forms.Form) -> "Page": ...


@frozen
class Operation:
"""An action on a record. The handler returns None when it is done, and the console sends staff back to the record.
Expand All @@ -103,9 +114,13 @@ class Operation:
description: str = ""
undo_hint: str = ""
gate: Gate | None = None
# Staff confirm on a page that shows what will change, for an action that is hard to take back.
preview: RecordPreviewHandler | None = None

def __post_init__(self) -> None:
check_danger(self.key, self.danger, self.form_class, self.undo_hint)
if self.preview is not None and self.form_class is None:
raise ValueError(f"Operation {self.key!r} has a preview, so it needs a form for the preview to read")


class ReasonForm(forms.Form):
Expand Down
46 changes: 45 additions & 1 deletion posthog/admin/console/records.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.urls import reverse

from posthog.dataclasses import frozen
from posthog.models.data_deletion_request import DataDeletionRequest
from posthog.models.organization import Organization
from posthog.models.team.team import Team

Expand Down Expand Up @@ -113,4 +114,47 @@ def links(self, obj: Model) -> list[RecordLink]:
]


RECORD_TYPES: dict[str, RecordType] = {record.key: record for record in (TeamRecord(), OrganizationRecord())}
def request_type_label(obj: DataDeletionRequest) -> str:
"""The request type in sentence case. The model's choice labels are title case, such as "Event Removal"."""
label = obj.get_request_type_display()
return label if label.startswith("HogQL") else label.capitalize()


class DeletionRequestRecord(RecordType):
key = "deletion-request"
plural = "deletion-requests"
label = "Deletion request"
pk_converter = "uuid"

def queryset(self) -> QuerySet:
return DataDeletionRequest.objects.select_related("approved_by")

def title(self, obj: Model) -> str:
assert isinstance(obj, DataDeletionRequest)
return f"{request_type_label(obj)} request"

def subtitle(self, obj: Model) -> str:
assert isinstance(obj, DataDeletionRequest)
return f"Team {obj.team_id}"

def parents(self, obj: Model) -> list[RecordLink]:
assert isinstance(obj, DataDeletionRequest)
# The request keeps a bare team ID, and the team may be gone by now.
team = Team.objects.select_related("organization").filter(pk=obj.team_id).first()
if team is None:
return []
return [*TeamRecord().parents(team), RecordLink(label=team.name, href=TeamRecord().url(team))]

def links(self, obj: Model) -> list[RecordLink]:
return [
RecordLink(
label="Edit in Django admin",
href=reverse("admin:posthog_datadeletionrequest_change", args=[obj.pk]),
title="The raw request, where its criteria are edited",
),
]


RECORD_TYPES: dict[str, RecordType] = {
record.key: record for record in (TeamRecord(), OrganizationRecord(), DeletionRequestRecord())
}
1 change: 1 addition & 0 deletions posthog/admin/console/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def discover(self) -> None:
import posthog.admin.console.usage_reports_tool # noqa: F401, PLC0415 — registers a core tool
import posthog.admin.console.organization_panels # noqa: F401, PLC0415 — registers the core Organization panels
import posthog.admin.console.events_retention_tool # noqa: F401, PLC0415 — registers a core tool
import posthog.admin.console.deletion_request_panels # noqa: F401, PLC0415 — registers the core deletion request panels

autodiscover_modules("console")
# Set only after the imports succeed, so a failed import raises again on the next request instead of
Expand Down
17 changes: 17 additions & 0 deletions posthog/admin/console/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,16 @@
"sync-to-billing": "#panel-billing",
}

MOVED_DELETION_REQUEST_ACTIONS = {
"submit": "workflow/submit/",
"approve": "workflow/approve/",
"retry": "workflow/retry/",
"revert-to-draft": "#panel-workflow",
"verify": "#panel-workflow",
"fetch-stats": "#panel-stats",
"preview-stats": "#panel-stats",
}

# Every route sits under /admin/, so AdminOAuth2Middleware adds its Google step-up, and admin_view
# adds the staff check, the login redirect and CSRF protection. The console needs no auth of its own.
urlpatterns = [
Expand Down Expand Up @@ -74,6 +84,13 @@
)
for old, target in MOVED_ORGANIZATION_ACTIONS.items()
],
*[
path(
f"admin/posthog/datadeletionrequest/<uuid:pk>/{old}/",
admin.site.admin_view(MovedToConsole.record_redirect("deletion-requests", target)),
)
for old, target in MOVED_DELETION_REQUEST_ACTIONS.items()
],
path(
"admin/posthog/organization/bulk-deactivate/",
MovedToConsole.page_redirect("Bulk deactivation", "/admin/console/tools/bulk-deactivate-organizations/"),
Expand Down
71 changes: 50 additions & 21 deletions posthog/admin/console/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,41 @@
}


class ConfirmStep:
"""The form data an operation runs with: the submitted form, or the input a confirmed preview showed."""

def __init__(self, request: HttpRequest, confirmation: ConfirmationToken) -> None:
self.confirmation = confirmation
self.token = request.POST.get("confirm_token") if request.method == "POST" else None
submitted = request.POST if request.method == "POST" else None
self.data: QueryDict | None = confirmation.load(self.token) if self.token is not None else submitted

@property
def confirmed(self) -> bool:
return self.token is not None

@property
def expired(self) -> bool:
return self.confirmed and self.data is None

def render_expired(self, request: HttpRequest) -> HttpResponse:
messages.error(request, "This confirmation expired or was made for something else. Review the change again.")
return redirect(request.get_full_path())

def render_preview(
self, request: HttpRequest, context: dict[str, object], form: forms.Form, preview: Callable[[], Page]
) -> HttpResponse:
page = preview()
if form.errors:
return render(request, "admin_console/operation.html", context, status=400)
return render(
request,
"admin_console/confirm.html",
{**context, "page": page, "confirm_token": self.confirmation.sign(form)},
)


def run_operation(request: HttpRequest, record: str, pk: object, panel_key: str, operation_key: str) -> HttpResponse:

Check warning on line 320 in posthog/admin/console/views.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

lint:complexity

`run_operation` has cyclomatic complexity 13 (warn >10)

Check warning on line 320 in posthog/admin/console/views.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

`run_operation` has cyclomatic complexity 13 (warn >10)
record_type = _record_type(record)
obj = get_object_or_404(record_type.queryset(), pk=pk)
panel = console.get_panel(record_type.key, panel_key)
Expand All @@ -305,9 +339,13 @@
if request.method == "GET" and operation.form_class is None:
return redirect(record_url)

# The panel is part of the target, so a confirmation for one record or panel cannot confirm another.
step = ConfirmStep(request, ConfirmationToken(request, f"{record_type.key}:{obj.pk}:{panel.key}", operation.key))
if step.expired:
return step.render_expired(request)
form = None
if operation.form_class is not None:
form = operation.form_class(request.POST if request.method == "POST" else None, initial=operation.form_initial)
form = operation.form_class(step.data, initial=operation.form_initial)
title = record_type.title(obj)
target = OperationTarget(
kicker="This acts on",
Expand All @@ -328,6 +366,10 @@
if form is not None and not form.is_valid():
return render(request, "admin_console/operation.html", context, status=400)

preview = operation.preview
if preview is not None and form is not None and not step.confirmed:
return step.render_preview(request, context, form, lambda: preview(request, obj, form))

audit = audit.bind(reason=form.cleaned_data.get("reason") if form is not None else None)
response = _audited(audit, lambda: operation.handler(request, obj, form))
if response is None:
Expand Down Expand Up @@ -409,7 +451,7 @@
return response


def run_tool_operation(request: HttpRequest, tool_key: str, operation_key: str) -> HttpResponse:

Check warning on line 454 in posthog/admin/console/views.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

lint:complexity

`run_tool_operation` has cyclomatic complexity 14 (warn >10)

Check warning on line 454 in posthog/admin/console/views.py

View workflow job for this annotation

GitHub Actions / Python code quality (depot-ubuntu-24.04)

`run_tool_operation` has cyclomatic complexity 14 (warn >10)
tool = _get_tool(tool_key)
operation = next((op for op in tool.operations(request) if op.key == operation_key), None)
if operation is None:
Expand All @@ -432,35 +474,22 @@
breadcrumbs=(RecordLink(label=tool.title, href=tool_url),),
is_record=False,
)
confirmation = ConfirmationToken(request, f"tool:{tool.key}", operation.key)
data: QueryDict | None = request.POST if request.method == "POST" else None
confirm_token = request.POST.get("confirm_token") if request.method == "POST" else None
if confirm_token is not None:
data = confirmation.load(confirm_token)
if data is None:
messages.error(
request, "This confirmation expired or was made for something else. Review the change again."
)
return redirect(request.get_full_path())
step = ConfirmStep(request, ConfirmationToken(request, f"tool:{tool.key}", operation.key))
if step.expired:
return step.render_expired(request)

form = None
if operation.form_class is not None:
form = operation.form_class(data, initial=operation.form_initial)
form = operation.form_class(step.data, initial=operation.form_initial)
context = _operation_context(target, operation, form, request)
if request.method == "GET":
return render(request, "admin_console/operation.html", context)
if form is not None and not form.is_valid():
return render(request, "admin_console/operation.html", context, status=400)

if operation.preview is not None and form is not None and confirm_token is None:
preview = operation.preview(request, form)
if form.errors:
return render(request, "admin_console/operation.html", context, status=400)
return render(
request,
"admin_console/confirm.html",
{**context, "page": preview, "confirm_token": confirmation.sign(form)},
)
preview = operation.preview
if preview is not None and form is not None and not step.confirmed:
return step.render_preview(request, context, form, lambda: preview(request, form))

audit = audit.bind(reason=form.cleaned_data.get("reason") if form is not None else None)
result = _audited(audit, lambda: operation.handler(request, form))
Expand Down
19 changes: 17 additions & 2 deletions posthog/admin/test_admin_console.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from django.conf import settings
from django.contrib.auth.models import Group
from django.db.models import Model
from django.http import HttpRequest, HttpResponse
from django.template.loader import render_to_string
from django.test import Client, RequestFactory, SimpleTestCase, override_settings
Expand All @@ -21,6 +22,7 @@
from posthog.admin.console.registry import ConsoleRegistry, console
from posthog.admin.console.tools import Cell, ConsoleTool, Live, Notice, Page, Paging, Row, Table
from posthog.csp_middleware import CSPMiddleware
from posthog.models.data_deletion_request import DataDeletionRequest, RequestType
from posthog.models.events_retention_config import OrganizationEventsRetentionConfig
from posthog.models.organization import Organization
from posthog.models.team.team import Team
Expand Down Expand Up @@ -318,16 +320,29 @@ def test_a_name_with_one_match_opens_the_team(self) -> None:
("form_page_keeps_its_parameters", "team", "set-api-token/?from=ticket", "api-token/set/?from=ticket"),
("button_action_lands_on_its_panel", "team", "rebuild-cache/", "#panel-remote-config"),
("organization_action", "organization", "limit-product/?resource=events", "?resource=events#panel-billing"),
(
"deletion_request_form_page",
"datadeletionrequest",
"approve/?from=ticket",
"workflow/approve/?from=ticket",
),
]
)
def test_an_old_django_admin_action_url_lands_on_the_console(
self, _name: str, record: str, old: str, new: str
) -> None:
obj = self.team if record == "team" else self.organization
obj: Model
if record == "team":
obj, plural = self.team, "teams"
elif record == "organization":
obj, plural = self.organization, "organizations"
else:
obj = DataDeletionRequest.objects.create(team_id=self.team.pk, request_type=RequestType.EVENT_REMOVAL)
plural = "deletion-requests"
response = self.client.get(f"/admin/posthog/{record}/{obj.pk}/{old}")

assert response.status_code == 302
assert response["Location"] == f"/admin/console/{record}s/{obj.pk}/{new}"
assert response["Location"] == f"/admin/console/{plural}/{obj.pk}/{new}"

def test_django_admin_team_page_points_moved_actions_to_the_console(self) -> None:
response = self.client.get(f"/admin/posthog/team/{self.team.pk}/change/")
Expand Down
Loading
Loading