Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve implementation of "multiple models" #376

Open
jpmckinney opened this issue Aug 20, 2024 · 0 comments
Open

Improve implementation of "multiple models" #376

jpmckinney opened this issue Aug 20, 2024 · 0 comments
Assignees
Labels
chore topic: models Relating to the ORM or database integration

Comments

@jpmckinney
Copy link
Member

jpmckinney commented Aug 20, 2024

https://sqlmodel.tiangolo.com/tutorial/fastapi/multiple-models/

Moved from open-contracting/credere-frontend#124

In principle, the API should not be returning data that the frontend does not need or use. FastAPI recommends separate models for parsers, serializers and database tables.

This is done in some cases but not all. In cases where a base table is reused across contexts (models.User, models.Lender and models.CreditProduct), the field (like borrower_types) ends up being included where it's not intended.

That said, we might conclude that the situation is "good enough" - in which case this becomes a documentation issue (see below horizontal rule).

This mainly affects the user, lender and credit product models.

In some places, the backend uses the same model for different frontend interfaces. For example:

  • Request formats
    • BasicUser
      • UpdatePasswordPayload (PUT /users/change-password)
      • LoginInput (POST /users/login)
      • ResetPasswordInput (POST /users/forgot-password)
    • models.User
      • CreateUserInput (POST /users)
      • UpdateUserInput (PUT /users/{user_id})
      • also used as IUser response format for POST /users and GET /users/{user_id}
    • models.CreditProduct
      • ICreditProductBase (POST /lenders/{lender_id}/credit-products)
      • ICreditProductUpdate (PUT /credit-products/{credit_product_id})
      • also used as ICreditProduct response format, for these two endpoints
  • Response formats in the backend all have a consistent schema in the frontend

In other places, the backend uses different models for the same frontend interface. For example:

  • Request formats
    • IUpdateBorrower
      • UpdateDataField (PUT /applications/{id}/verify-data-field)
      • BorrowerUpdate (PUT /applications/{id}/borrower)
  • Response formats
    • ICreditProduct
      • models.CreditProduct (POST /lenders/{lender_id}/credit-products, PUT /credit-products/{credit_product_id})
      • models.CreditProductWithLender (GET /credit-products/{credit_product_id}, POST /applications/credit-product-options via IApplicationCreditOptions)
    • ILender
      • models.Lender (POST /lenders, PUT /lenders/{lender_id})
      • models.LenderWithRelations (GET /lenders/{lender_id})
    • IUser
      • models.User (POST /users, GET /users/{user_id})
      • models.UserWithLender (PUT /users/{user_id})

I listed all the obvious cases above, but I didn't go through cases where a schema or model is wrapped inside another, like with IApplicationCreditOptions above.

See https://credere.readthedocs.io/en/latest/frontend.html#schemas-and-models Sorting by different columns makes it easier to see where maybe there's an opportunity to improve the implementation.


Even without doing the above, we should document how we want to implement the approach in https://sqlmodel.tiangolo.com/tutorial/fastapi/multiple-models/, to be consistent going forward.

@jpmckinney jpmckinney changed the title Fix implementation of "multiple models" Improve implementation of "multiple models" Aug 21, 2024
@jpmckinney jpmckinney added the topic: models Relating to the ORM or database integration label Aug 21, 2024
@jpmckinney jpmckinney self-assigned this Aug 21, 2024
This was referenced Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore topic: models Relating to the ORM or database integration
Projects
None yet
Development

No branches or pull requests

1 participant