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

Implement select_schema on EndpointCreator and crud_router #169

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ljmc-github
Copy link

@ljmc-github ljmc-github commented Oct 9, 2024

Pull Request Template for FastCRUD

Description

Adds an optional kwarg for the EndpointCreator and crud_router to specify a Pydantic schema for returning objects rather than dicts.

I've added documentation on the classes docstrings wrt to the new kwarg, but will need some guidance/help to add to the pages and on what to build examples.

Closes #113

Changes

  • add a select_schema optional kwarg to EndpointCreator and crud_router
  • add a SelectSchemaType
  • replaces BaseModel in the schema_to_select arguments and appropriate returns
  • add if/else logic for FastCRUD._read_item and FastCRUD._read_items
  • refactor of the logic around is_paginated and has_offset_limit in FastCRUD._read_items
  • addition of client fixture with select_schema set to read_schema fixture and tests with model validation

Tests

I copy pasted the "simple" client tests for get and get_multi items, replacing the client for the new client with select_schema and added read_schema.model_validate(...) in the assertions.

I've created them on the SQLModel side, happy to duplicate that on the SQLAlchemy side, but it's not part of my current cases so I'll have to learn how.

I'm not certain tests with read_schema.model_validate(...) are quite enough, I would prefer a isinstance somewhere, but couldn't find the right set of tests to build upon.

Checklist

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • I have added necessary documentation (if appropriate). NB. I'm happy to do more on this front
  • I have added tests that cover my changes (if applicable).
  • All new and existing tests passed.

Additional Notes

Fairly new to SQLModel and FastAPI, there might be things I missed.

@ljmc-github
Copy link
Author

sorry for failed pipeline I had forgotten a change

@igorbenav
Copy link
Owner

Hey, @ljmc-github, thanks for the PR and sorry for the delay!

Looking good, it’d be better if you could just add examples with input and output to the docstrings, I don’t think it’s necessary to add to the docs directly (since these docstring examples will be in the API docs anyway). One example for EndpointCreator and the same one for crud_router would be enough

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.

Adding Response Model - Return Type
2 participants