-
Notifications
You must be signed in to change notification settings - Fork 594
refactor(case): main table to use fastapi-filters #6082
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors both frontend and backend to leverage FastAPI-Filter for case list filtering and sorting, replacing custom parameter builders with standardized filters.
- Added client-side utilities (
createFastAPIFilterParameters
,createFastAPIFilterParams
) to generate FastAPI-filter query parameters. - Updated Vuex store action to use the new FastAPI-filter parameter helper.
- Refactored the backend
get_cases
endpoint to useCaseFilter
,FilterDepends
, and new pagination JSON methods. - Introduced
CaseFilter
and auxiliary filter classes; updated dependency pins and VSCode formatter settings.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/dispatch/static/dispatch/src/search/utils.js | Added FastAPI-filter parameter builders for client-side filtering and sorting. |
src/dispatch/static/dispatch/src/case/store.js | Switched store action to use createFastAPIFilterParameters . |
src/dispatch/case/views.py | Refactored get_cases to use CaseFilter , FilterDepends , and model_dump_json . |
src/dispatch/case/filters.py | Added CaseFilter and related filter classes for backend filtering logic. |
requirements-base.txt & requirements-base.in | Added fastapi-filter , advanced-alchemy ; bumped sqlalchemy version. |
.vscode/settings.json | Updated Python formatter and Ruff code-action settings. |
from fastapi_filter.contrib.sqlalchemy import Filter | ||
from pydantic import Field | ||
from sqlalchemy.orm import Query | ||
from sqlalchemy.orm.query import Query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate import of Query
; remove the redundant import on this line to clean up dependencies.
from sqlalchemy.orm.query import Query |
Copilot uses AI. Check for mistakes.
|
||
|
||
# Alternative approach using separate filters for different concerns | ||
class CaseBasicFilter(Filter): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] These alternative filter classes (e.g., CaseBasicFilter
, CaseStatusFilter
, etc.) are not referenced in the PR; consider moving them to a separate module or removing unused code to improve maintainability.
Copilot uses AI. Check for mistakes.
|
||
// Add sorting parameters | ||
if (sortBy && sortBy.length > 0) { | ||
params.order_by = descending[0] ? `-${sortBy[0]}` : sortBy[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Currently only the first sortBy
field is applied (order_by = sortBy[0]
); if multi-column sorting is required, consider iterating over all fields or explicitly documenting that only the primary sort is supported.
params.order_by = descending[0] ? `-${sortBy[0]}` : sortBy[0] | |
params.order_by = sortBy.map((field, index) => | |
descending[index] ? `-${field}` : field | |
).join(","); |
Copilot uses AI. Check for mistakes.
"""Retrieves all cases.""" | ||
pagination = search_filter_sort_paginate(model="Case", **common) | ||
"""Retrieves all cases using FastAPI-Filter.""" | ||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move the import json
statement to the top of the file with other imports to keep import statements organized.
import json | |
# json module is used for serialization/deserialization |
Copilot uses AI. Check for mistakes.
No description provided.