uriyyo / fastapi-pagination

FastAPI pagination 📖
https://uriyyo-fastapi-pagination.netlify.app/
MIT License
1.1k stars 126 forks source link

Be able to use `count_query` on `ext.sqlmodels.paginate` and `ext.sqlalchemy.paginate` #1194

Closed moumoutte closed 1 week ago

moumoutte commented 2 weeks ago

Do not use the boolmethod of count_query as it's not evaluating to a boolean here and instead detect if it's None or not. Also add the count_query as a valid parameter of fastapi_pagination.ext.sqlmodel.paginate

codecov[bot] commented 2 weeks ago

Codecov Report

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

Project coverage is 92.69%. Comparing base (245519c) to head (bcb3f35). Report is 270 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1194 +/- ## ========================================== - Coverage 93.26% 92.69% -0.58% ========================================== Files 35 38 +3 Lines 1040 1314 +274 ========================================== + Hits 970 1218 +248 - Misses 70 96 +26 ```

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

moumoutte commented 1 week ago

If I understand properly your comment, you ask me if I could add the count_query param to the ext.sqlalchemy.paginate function ?

I think this param is already present here : https://github.com/uriyyo/fastapi-pagination/blob/main/fastapi_pagination/ext/sqlalchemy.py#L241 and here https://github.com/uriyyo/fastapi-pagination/blob/main/fastapi_pagination/ext/sqlalchemy.py#L226

Only the function marked as deprecated has no count_query param.

I guess this is a good thing to not maintain a feature on deprecated function. But if you really want to it , I could provide it. Let me know !

uriyyo commented 1 week ago

@moumoutte My bad, I forgot that it already has count_query param 😅

uriyyo commented 1 week ago

@moumoutte Thanks for PR!