igorbenav / fastcrud

FastCRUD is a Python package for FastAPI, offering robust async CRUD operations and flexible endpoint creation utilities.
MIT License
530 stars 32 forks source link

Add unpaginated versions of multi-row get methods (w/tests) #62

Closed slaarti closed 2 months ago

slaarti commented 2 months ago

Description

Per #59, this PR is to add new methods to FastCRUD to do multi-row gets (without and with joins) that return all rows in a single list without pagination.

Changes

Added new methods get_multi_all and get_multi_joined_all that are basically copies minus the offset, limit, and return_total_count args as well as without wrapping the list of result rows in a dict (since that happens in the existing methods to help with pagination).

Tests

test_get_multi.py and test_get_multi_joined.py for both sqlalchemy and sqlmodel were copied into new files and adjusted to test the new methods (including removing tests that only made sense in the context of pagination).

Checklist

igorbenav commented 2 months ago

@slaarti could you please refactor? I think doing it this way would be bettet indeed

slaarti commented 2 months ago

Okay, sure. Not tonight, might take a couple days, but I'm fine with the alternate approach. 👍

igorbenav commented 2 months ago

Nice, thanks!

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (5e45aee) to head (661357a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #62 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 64 64 Lines 4282 4336 +54 ========================================= + Hits 4282 4336 +54 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

slaarti commented 2 months ago

Okay, I rolled back the original branch (well, renamed it and did a fresh checkout) and implemented the alternate method, including checks on the endpoint creator and crud router (with a flag to bypass the checks). The force push is for the new branch.

igorbenav commented 2 months ago

Sorry, could you refactor it to use None instead of 0? I think someone using 0 to retrieve no record might make sense (think you have a limit for someone to retrieve x objects, he might get 10 objects, then 5 more, you subtract it from x until it becomes zero, then the person can't retrieve).

It's a dumb example, but I think it makes more sense with limit=None.

slaarti commented 2 months ago

I'd opted for 0 as much to avoid having to deal with changes in typing and looking for None vs. 0 as feeling like saying you want zero rows made no sense... but yeah, okay, you make a good point. Will do. 👍

slaarti commented 2 months ago

...Actually, come to think of it, an interesting aspect of changing it to None is that I'm pretty sure there's no good way to pass None as a path/query parameter, which eliminates the need for the changes made for sanitizing the automatically generated endpoints.

Anyone who wants to provide an unlimited get in their (presumably private, as in my cases) API will have to create an endpoint specifically for it, but I'm okay with that.

igorbenav commented 2 months ago

I guess everything worked out even better, great! Thanks @slaarti and welcome to fastcrud contributors🎉