diff --git a/backend/src/interfaces/admin/views/tiers.py b/backend/src/interfaces/admin/views/tiers.py index 2f249e91..67e80724 100644 --- a/backend/src/interfaces/admin/views/tiers.py +++ b/backend/src/interfaces/admin/views/tiers.py @@ -20,7 +20,7 @@ class TierAdmin(DataclassModelMixin, ModelView, model=Tier): category = "Users & Access" column_list = [Tier.id, Tier.name, Tier.description] - column_details_list = "__all__" + column_details_exclude_list = [Tier.users] column_searchable_list = [Tier.name] column_sortable_list = [Tier.id, Tier.name] @@ -32,6 +32,7 @@ class TierAdmin(DataclassModelMixin, ModelView, model=Tier): form_create_rules = list(TierCreate.model_fields.keys()) form_edit_rules = list(TierUpdate.model_fields.keys()) + form_excluded_columns = [Tier.users] async def delete_model(self, request: Request, pk: str) -> None: """Override delete to permanently remove tier from database. diff --git a/backend/src/modules/tier/models.py b/backend/src/modules/tier/models.py index cfc3e15a..ec952711 100644 --- a/backend/src/modules/tier/models.py +++ b/backend/src/modules/tier/models.py @@ -25,7 +25,7 @@ class Tier(Base, TimestampMixin, SoftDeleteMixin): name: Mapped[str] = mapped_column(String, nullable=False, unique=True) description: Mapped[str | None] = mapped_column(Text, default=None) - users: Mapped[list["User"]] = relationship("User", back_populates="tier", lazy="selectin", default_factory=list, init=False) + users: Mapped[list["User"]] = relationship("User", back_populates="tier", lazy="select", default_factory=list, init=False) def __repr__(self) -> str: return self.name diff --git a/backend/tests/unit/interfaces/admin/test_tier_view.py b/backend/tests/unit/interfaces/admin/test_tier_view.py new file mode 100644 index 00000000..45320c7a --- /dev/null +++ b/backend/tests/unit/interfaces/admin/test_tier_view.py @@ -0,0 +1,13 @@ +"""Tests for the Tier admin view configuration.""" + +from src.interfaces.admin.views.tiers import TierAdmin + + +def test_tier_admin_form_does_not_include_users(): + """The tier form must not preload a tier's users or list every user as an option.""" + assert "users" not in TierAdmin().get_form_columns() + + +def test_tier_admin_details_do_not_include_users(): + """The tier details page must not preload every user assigned to the tier.""" + assert "users" not in TierAdmin().get_details_columns() diff --git a/backend/tests/unit/modules/tier/test_models.py b/backend/tests/unit/modules/tier/test_models.py new file mode 100644 index 00000000..5e12b246 --- /dev/null +++ b/backend/tests/unit/modules/tier/test_models.py @@ -0,0 +1,11 @@ +"""Unit tests for the Tier ORM model configuration.""" + +from sqlalchemy import inspect + +from src.modules.tier.models import Tier + + +def test_tier_users_is_lazy_select(): + """Loading a tier must not load every user assigned to it.""" + rel = inspect(Tier).relationships["users"] + assert rel.lazy == "select", f"Tier.users should be lazy='select', got {rel.lazy!r}" diff --git a/docs/user-guide/admin-panel/adding-models.md b/docs/user-guide/admin-panel/adding-models.md index ee90b412..78643efb 100644 --- a/docs/user-guide/admin-panel/adding-models.md +++ b/docs/user-guide/admin-panel/adding-models.md @@ -186,7 +186,16 @@ form_create_rules = [*WidgetCreate.model_fields.keys(), "owner_id"] ### `lazy="selectin"` Is Required -SQLAdmin runs in async context, so relationships must use `lazy="selectin"` to avoid lazy-loading errors. Symptom of forgetting: `MissingGreenlet` or `greenlet_spawn has not been called`. Both User and Tier models in the boilerplate already use this pattern. +SQLAdmin runs in async context, so relationships must use `lazy="selectin"` to avoid lazy-loading errors. Symptom of forgetting: `MissingGreenlet` or `greenlet_spawn has not been called`. `User.tier` in the boilerplate already uses this pattern. + +The exception is large one-to-many collections such as `Tier.users`, which uses `lazy="select"` so that loading a tier doesn't load every user in it. SQLAdmin still loads a relationship whenever a page uses it: it `selectinload`s the relationships in `column_list` and in the form columns (the details page reuses the edit-form query), and a relationship form field lists every row of the related table. So keep large collections out of the list, the form and the details page, as `TierAdmin` does: + +```python +form_excluded_columns = [Tier.users] +column_details_exclude_list = [Tier.users] +``` + +`column_details_exclude_list` replaces `column_details_list = "__all__"`; SQLAdmin doesn't accept both on the same view, and the details page shows every other column by default. ### Don't Set `default=None` on Relationships diff --git a/docs/user-guide/database/models.md b/docs/user-guide/database/models.md index 77798978..32870ebe 100644 --- a/docs/user-guide/database/models.md +++ b/docs/user-guide/database/models.md @@ -73,7 +73,7 @@ When you add a new module, **add its models here** so Alembic's `--autogenerate` ## Relationships -The boilerplate uses SQLAlchemy `relationship()` where it makes sense, with `lazy="selectin"` to avoid N+1 problems by fetching related rows in a single follow-up query. +The boilerplate uses SQLAlchemy `relationship()` where it makes sense. Relationships that are routinely read (like `User.tier`) use `lazy="selectin"` to avoid N+1 problems by fetching related rows in a single follow-up query. Large collections that are rarely read (like `Tier.users`) use `lazy="select"`, so loading a tier doesn't pull in every user assigned to it. For example, `User.tier` and `Tier.users` are both wired up: @@ -94,7 +94,7 @@ class Tier(Base, TimestampMixin, SoftDeleteMixin): __tablename__ = "tiers" ... users: Mapped[list["User"]] = relationship( - "User", back_populates="tier", lazy="selectin", + "User", back_populates="tier", lazy="select", default_factory=list, init=False, ) ``` diff --git a/docs/user-guide/development.md b/docs/user-guide/development.md index 554987ce..cde75b00 100644 --- a/docs/user-guide/development.md +++ b/docs/user-guide/development.md @@ -371,7 +371,7 @@ The boilerplate uses `import_models("src.modules")` in Alembic to discover model ### Forgetting `lazy="selectin"` on a relationship -SQLAdmin runs in async context. A relationship without `lazy="selectin"` raises `MissingGreenlet` when the admin tries to render it. Both `User.tier` and other relationships in the boilerplate already use this pattern — copy from those. +SQLAdmin runs in async context. A relationship without `lazy="selectin"` raises `MissingGreenlet` when the admin tries to render it. `User.tier` and other relationships in the boilerplate already use this pattern — copy from those. (`Tier.users` is a deliberate exception: it uses `lazy="select"` so loading a tier doesn't load every user in it.) ### Dataclass models without `init=False` on relationships