Skip to content

fix(admin): register moved hogflow and cdp admins via package init#60652

Merged
meikelmosby merged 3 commits into
masterfrom
claude/eloquent-cori-r9Rlt
Jun 1, 2026
Merged

fix(admin): register moved hogflow and cdp admins via package init#60652
meikelmosby merged 3 commits into
masterfrom
claude/eloquent-cori-r9Rlt

Conversation

@dmarchuk
Copy link
Copy Markdown
Contributor

@dmarchuk dmarchuk commented May 29, 2026

Problem

The recent move of the workflows and CDP models into their product apps (#60248) also relocated their Django admin classes into admin/ packages (products/cdp/backend/admin/ and products/workflows/backend/admin/), split across submodules — but the package __init__.py files were left empty.

PostHog registers admins lazily through autodiscover_modules("admin"), which imports only each app's top-level admin module — i.e. the package's __init__.py. With an empty __init__, the submodules holding the @admin.register decorators were never imported, so HogFlow, HogFunction, Plugin, and PluginConfig silently disappeared from Django admin. Every other product uses a single admin.py file, which autodiscover imports directly, so only these two packaged products were affected.

Changes

Screenshot 2026-05-29 at 13 50 55

Fix the regression — import the decorator-bearing submodules from each package __init__.py so their @admin.register decorators fire during autodiscover. This restores exactly the four admins that existed before the move (HogFlow, HogFunction, Plugin, PluginConfig).

New admins (net-new, not part of the regression) — while verifying the workflows admin section, two workflows models had never had a Django admin at all. This PR adds them:

  • HogFlowTemplate — workflow templates, grouped under Workflows.
  • HogFlowBatchJob — batch/broadcast job runs, grouped under Workflows.

Both are read-oriented: foreign-key fields (team, created_by, and hog_flow for the batch job) are declared readonly so the admin change pages don't render full-table <select> widgets, and JSON payload fields are readonly. team/hog_flow render as links to their respective admin change pages.

Regression testposthog/admin/test_admin.py::TestProductAdminRegistration asserts all six models are registered after register_all_admin().

How did you test this code?

I manually tested with locally checked out branch that all the models load as they should.

I'm an agent — I did not do manual UI testing. Automated verification only:

  • Booted Django and checked admin.site.is_registered(...): before the fix the four moved models returned False; after, all True. Confirmed the two new models register and their admin change/changelist URLs reverse cleanly.
  • The regression test fails without the __init__.py fix and passes with it.
  • ruff check / ruff format clean; pre-commit hooks including ty check passed.

🤖 Agent context

Authored by Claude Code at the request of the PR author.

Root cause traced to #60248: that move turned two products' admin modules into packages whose __init__.py was empty, breaking the autodiscover_modules("admin") registration path. Chose explicit submodule imports over the central registry's dynamic pkgutil.iter_modules approach since there are only a few files and it's clearer/grep-friendly. After confirming nothing was missing relative to the pre-move baseline, the author asked to also add admins for the two previously-unregistered workflows models (HogFlowTemplate, HogFlowBatchJob) — these are additive. plugin_attachment_inline is intentionally not imported explicitly: it has no @admin.register decorator and is already pulled in transitively by plugin_config_admin.

Note for reviewers: HogFlowBatchJob has a post_save signal that enqueues a real job invocation on create. The admin allows creates by default (consistent with other admins); flag if you'd prefer add disabled there.

https://claude.ai/code/session_01JaMLKbh61S5uYBQvBgCM77

claude added 2 commits May 29, 2026 11:09
The workflows and cdp model moves left their admin classes split across
submodules of an admin/ package with an empty __init__. autodiscover_modules
only imports the package __init__, so HogFlow, HogFunction, Plugin, and
PluginConfig stopped appearing in Django admin. Import the submodules from
each package __init__ so their @admin.register decorators fire, and add a
regression test.
These workflows models had no Django admin before. Register read-oriented
admins (FK fields readonly so they don't trigger full-table select widgets)
and extend the registration regression test to cover them.
@dmarchuk dmarchuk marked this pull request as ready for review May 29, 2026 11:51
@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team May 29, 2026 11:52
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 29, 2026

Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
posthog/admin/test_admin.py:178-182
The team prefers parameterised tests. A `for` loop over models in a single test method means a failure on the first unregistered model silences all subsequent ones. `@pytest.mark.parametrize` gives independent pass/fail signals per model and clearer CI output.

```suggestion
    @pytest.mark.parametrize(
        "model",
        [HogFlow, HogFlowTemplate, HogFlowBatchJob, HogFunction, Plugin, PluginConfig],
        ids=lambda m: m.__name__,
    )
    def test_moved_product_models_are_registered(self, model):
        # Tests skip the lazy admin registry, so trigger registration explicitly.
        register_all_admin()
        assert admin.site.is_registered(model), f"{model.__name__} is not registered in Django admin"
```

Reviews (1): Last reviewed commit: "feat(admin): add HogFlowTemplate and Hog..." | Re-trigger Greptile

Comment thread posthog/admin/test_admin.py Outdated
Independent pass/fail signal per model instead of a single loop that stops
at the first failure.
@dmarchuk dmarchuk requested a review from webjunkie May 29, 2026 12:17
@meikelmosby meikelmosby merged commit cb5b59f into master Jun 1, 2026
206 checks passed
@meikelmosby meikelmosby deleted the claude/eloquent-cori-r9Rlt branch June 1, 2026 06:15
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented Jun 1, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-06-01 06:36 UTC Run
prod-us ✅ Deployed 2026-06-01 06:49 UTC Run
prod-eu ✅ Deployed 2026-06-01 06:49 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.

3 participants