fix(sqla): Improve primary key handling in model conversion

[noamsto]

This PR improves how primary key fields are handled in the SQLAlchemy model converter, addressing both field ordering and inheritance support.

Changes:

1. Primary Key Field Ordering

   • Move primary key fields to the start of the converted fields list
   • Maintain consistent form layout with PKs at the top
   • Preserve ordering of non-PK fields

2. Polymorphic Inheritance Support

   • Add support for inherited primary key columns
   • Bypass multiple-column property restriction for inherited PKs
   • Maintain existing validation for other column cases

These changes ensure a more robust and consistent handling of primary keys across both simple and inherited model structures.

[noamsto]

@jowilf

With this fix it's the following simple case is working:

from uuid import UUID, uuid4

from sqlalchemy import ForeignKey, Integer, String
from sqlalchemy.orm import DeclarativeBase, Mapped, mapped_column, relationship


class Base(DeclarativeBase):
    pass


class Product(Base):
    __tablename__ = "products"
    __mapper_args__ = {
        "polymorphic_identity": "product",
        "polymorphic_on": "_type",
    }

    _type: Mapped[str] = mapped_column(String(50))
    id: Mapped[UUID] = mapped_column(primary_key=True, default=uuid4)
    name: Mapped[str] = mapped_column(String(100))
    price: Mapped[int] = mapped_column(Integer())
    store_id: Mapped[UUID] = mapped_column(ForeignKey("stores.id"))
    store: Mapped[Store] = relationship("Store")


class Book(Product):
    __tablename__ = "books"
    __mapper_args__ = {
        "polymorphic_identity": "book",
        "polymorphic_on": "_type",
    }
    id: Mapped[UUID] = mapped_column(ForeignKey("products.id"), primary_key=True)


class Movie(Product):
    __tablename__ = "movies"
    __mapper_args__ = {
        "polymorphic_identity": "movie",
        "polymorphic_on": "_type",
    }
    id: Mapped[UUID] = mapped_column(ForeignKey("products.id"), primary_key=True)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2540c51) to head (89e4651).
Report is 33 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #629    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           75        76     +1     
  Lines         5817      5926   +109     
==========================================
+ Hits          5817      5926   +109     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jowilf]

Thanks for the PR, can you add a test case for this? Ideally something like

starlette-admin/tests/sqla/test_sqla_utils.py

Line 90 in 33abffb

async def test_model_fields_conversion():

[jowilf]

let's maintain the order in the model class definition

[noamsto]

I see this doesn't have so much effect. Since you have to declare the PK in the inheriting model.
So 💯

[noamsto]

I see this doesn't have so much effect. Since you have to declare the PK in the inheriting model. So 💯

This is actually wrong... The test showed differently,
Consider these models:

class BaseItem(Base):
    __tablename__ = "base_items"
    __mapper_args__ = {
        "polymorphic_identity": "base_item",
        "polymorphic_on": "_type",
    }

    id = Column(Integer, primary_key=True)
    name = Column(String(50))
    _type = Column(String(50))


class SpecialItem(BaseItem):
    __mapper_args__ = {"polymorphic_identity": "special_item"}
    special_power = Column(String(50))

Without the insert, the id primary key will be after the fields of SpecialItem, i.e.:
["special_power", "id", "name", "_type"]

And if there would be more fields it will be pushed back...
This is bad UX in my opinion, you can end up with the ID out of the screen

[noamsto]

@jowilf
I add a test, hope it is good enough.
Tell me if you want me to squash the commits into one