Skip to content

Fix admin import without django-stubs runtime monkeypatch#113

Merged
pfouque merged 2 commits intodjango-commons:mainfrom
samiashi:samielachi/fix-admin-runtime-generics
Mar 9, 2026
Merged

Fix admin import without django-stubs runtime monkeypatch#113
pfouque merged 2 commits intodjango-commons:mainfrom
samiashi:samielachi/fix-admin-runtime-generics

Conversation

@samiashi
Copy link
Copy Markdown
Contributor

@samiashi samiashi commented Mar 9, 2026

Fix import-time admin crash when Django generic types are not runtime-subscriptable

Description

This PR fixes an import-time crash in django_fsm.admin introduced by the 4.2.0 typing/admin integration changes.

At module import time, django_fsm.admin evaluated typed Django generics such as ModelForm[...] and ModelAdmin[...]. In downstream projects that do not call django_stubs_ext.monkeypatch() before Django admin autodiscovery, those classes are not runtime-subscriptable, which raises:

TypeError: type 'ModelForm' is not subscriptable

This change keeps the precise generic typing under typing.TYPE_CHECKING and uses unsubscripted runtime fallbacks instead. That preserves static typing for type checkers while allowing django_fsm.admin to import cleanly without requiring a runtime monkeypatch in consumer projects.

This PR also adds a regression test that imports django_fsm.admin in a fresh subprocess with only minimal Django settings configured. The test verifies the module can be imported without inheriting the test suite's django_stubs_ext.monkeypatch() setup.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Other (no functional changes)

Checklist

  • I have read the CONTRIBUTING.md guide
  • Tests added/updated for the changes
  • All tests pass locally (uv run pytest or pytest)
  • Linting passes (uv run ruff check .)
  • Documentation updated (if applicable)
  • CHANGELOG.rst updated (for user-facing changes)

Testing

Ran the full test suite and lint locally.

uv run pytest
uvx ruff check .
uvx ruff format --check .

Additionally, the new regression coverage is in:

  • tests/testapp/tests/test_admin.py::test_admin_module_imports_without_django_stubs_monkeypatch

This test spawns a fresh Python process, configures minimal Django settings, and verifies import django_fsm.admin succeeds without relying on the test suite's django_stubs_ext monkeypatch.

Related Issues

Related to the import-time runtime typing regression in django_fsm.admin introduced in 4.2.0 admin integration.

Keep the Django generic typing in django_fsm.admin under TYPE_CHECKING so the module no longer evaluates ModelForm[...] or ModelAdmin[...] at runtime. This avoids import-time failures in downstream projects that do not call django_stubs_ext.monkeypatch() before Django admin autodiscovery.

Add a regression test that imports django_fsm.admin in a fresh subprocess with only minimal Django settings configured, proving the admin module loads without relying on the test suite's django-stubs monkeypatch. Also document the fix under Unreleased in the changelog.
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
django_fsm/admin.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pfouque
Copy link
Copy Markdown
Member

pfouque commented Mar 9, 2026

Thanks for the great fix and the regression test — nice catch. 👍

I only moved the test into a dedicated file for consistency with the rest of the suite.

I'll ship this in today's release. ⛵

@pfouque pfouque merged commit 2fd3f00 into django-commons:main Mar 9, 2026
8 checks passed
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