litestar-org / litestar

Production-ready, Light, Flexible and Extensible ASGI API framework | Effortlessly Build Performant APIs
https://litestar.dev/
MIT License
5.45k stars 373 forks source link

Enhancement: allow setting individual optional DTO fields #2005

Closed LonelyVikingMichael closed 1 year ago

LonelyVikingMichael commented 1 year ago

Summary

I'd like to be able to mark individual DTO fields as optional while other remain required. This is helpful where more nuance is required than DTOConfig.partial can provide.

An optional keyword argument on DTOConfig seems like the right approach.

Basic Example

class User(DeclarativeBase):
    email: Mapped[str] = mapped_column(String(320), nullable=False, unique=True)
    password_hash: Mapped[str] = mapped_column(String(1024))
    is_active: Mapped[bool] = mapped_column(Boolean(), nullable=False, default=False)
    is_verified: Mapped[bool] = mapped_column(Boolean(), nullable=False, default=False)

class UserReadDTO(SQLAlchemyDTO[User]):
    """DTO with optional `is_active` and `is_verified` fields."""
    config = DTOConfig(optional={"is_active", "is_verified"})

Drawbacks and Impact

I'm not sure how much benefit there would be to the community at large - this would be particularly helpful to litestar-users

Unresolved questions

No response

Funding

Fund with Polar

provinzkraut commented 1 year ago

Does this do what you want?

https://docs.litestar.dev/2/usage/dto/1-dto-factory.html#marking-fields

LonelyVikingMichael commented 1 year ago

Not quite - I'll provide context.

During registration is_verified is silently ignored, else the user could just skip verification by setting a bool. However, internally we have a user creation method with a process_unsafe_fields flag that takes is_verified into account. The use case being "I am a privileged superuser who can create trusted accounts" - in which case skipping verification is sensible. Edit: This of course requires the field to be present during model instantiation.

I am aware that this can be solved by yet another DTO, making the distinction between UserRegister and UserCreate, it just seems arbitrary to users.

peterschutt commented 1 year ago

A self contained, minimal example that demonstrates your issue would be helpful.

I'd like to be able to mark individual DTO fields as optional

"optional" as in field type is to be made a union with None, or not required to be submitted in the client payload?

What if a user isn't privileged, but in the api docs the is_verified parameter is documented as an optional parameter, isn't this leaking abstraction to them?

As for the idea of granular setting of partial, I think it makes sense. What about making DTOConfig.partial accept either a bool, or set[str] where the latter would be a set of field names to be made partial fields?