V1.0.0.2#666
Conversation
…n join Document Thing virtual filter fields in query_helper. Add contacts filter matching any linked Contact.name with contains, ncontains, eq, ne, startswith, endswith.
- Accept repeated filter query params via order_sort_filter merge of filters list - Contact virtual filter field things: EXISTS join ThingContactAssociation and Thing.name for contains/null operators (inverse of Thing contacts filter) - Thing virtual sorts for properties and proxies that cannot use ORDER BY on ORM attributes: monitoring_status, well_status, datalogger_suitability_status, site_name (NMBGMR thing_id_link), contacts and aquifers (min lower name), open_status (CASE on Open Status history), measuring_point_height (latest MP row) - Contact virtual sort for things: min lower Thing.name across associations - Document dispatch order in _apply_json_filter_clause and module header - Add sqlalchemy case and nulls_last imports
- Switch get_db_contacts to keyword-only filters argument matching order_sort_filter(filters=...) - Document virtual things filter and get_db_contacts link to list endpoint - Split add_contact signature across lines for line length
- Replace single filter string with Annotated list from Query(alias=filter) so Refine can send multiple JSON filter objects (AND semantics) - Point get_contacts docstring at virtual things filter and get_db_contacts - Add typing Annotated import for filter_params
- things contains, no match, and nnull filter cases - Multiple filter parameters AND together (things plus name) - Sort by things (min site name) returns 200 and includes linked contact - Module-scoped auth override matching other API tests
- monitoring_status and well_status descending or ascending - site_name, contacts, open_status, measuring_point_height, aquifers sort smoke tests (expect 200, SQL-backed ORDER BY) - Covers Thing virtual sort keys used by the wells DataGrid
- Features bullet notes repeated filter params and virtual association fields - Points readers to docs/refine-json-filters-and-virtual-fields.md for rationale
- New subsection under architecture for DataGrid filter and sort wiring - Cross-link to refine-json-filters-and-virtual-fields.md for maintainers
- Explains repeated filter query params, virtual filter fields, EXISTS pattern, and inverse Thing contacts versus Contact things associations - Documents sorting for non-column Thing and Contact fields including scalar summaries (min names, StatusHistory subqueries, site_name via thing_id_link) - Tables map API behavior to query_helper helpers; notes tests to read Note: docs/ is gitignored by default; this file is tracked via force-add so the guide stays in version control alongside query_helper.
…st-filtering BDMS 774 Filtering and Sorting Wells & Contacts
There was a problem hiding this comment.
Pull request overview
Adds Refine-compatible list filtering/sorting behavior (repeated JSON filter= params and “virtual” association/derived fields) and updates the Contacts endpoint plus tests/docs to match, as part of the final pre-launch release.
Changes:
- Extend
order_sort_filter/ query filtering to support repeated JSON filters and virtual fields (Thing.contacts,Contact.things) plus multiple virtual sort keys. - Update
GET /contactto acceptfilteraslist[str]and plumb it throughget_db_contacts. - Add/expand tests and documentation describing Refine filter semantics and virtual field handling.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_thing.py | Adds tests for water-well list filtering on contacts and sorting on several virtual fields. |
| tests/test_contact_filters.py | New tests for contacts list filtering/sorting using the things virtual field and multiple repeated filters. |
| services/query_helper.py | Implements virtual filter/sort logic and supports repeated filter params in order_sort_filter. |
| services/contact_helper.py | Updates contacts query helper to accept filters: list[str] and use order_sort_filter accordingly. |
| docs/refine-json-filters-and-virtual-fields.md | New documentation describing repeated filter params, virtual fields, and virtual sorting semantics. |
| api/contact.py | Changes filter query param to list[str] and passes through to get_db_contacts. |
| README.md | Notes Refine repeated JSON filters and links to the new docs. |
| CLAUDE.md | Adds repo guidance about Refine list filters and virtual sorting/filtering behavior. |
|
|
||
| Each call applies a **single** predicate. ``order_sort_filter`` chains | ||
| multiple JSON filters with AND semantics. | ||
| """ |
There was a problem hiding this comment.
_apply_json_filter_clause assumes f is a dict and calls f.keys(). If the filter query param contains valid JSON that is not an object (e.g. a list/string/number), this will raise an AttributeError and bubble up as a 500. Consider validating isinstance(f, dict) (and returning a 422/400 with a clear message) before accessing keys.
| """ | |
| """ | |
| if not isinstance(f, dict): | |
| raise HTTPException( | |
| status_code=422, | |
| detail="Filter must be a JSON object", | |
| ) |
| def str_order(expr): | ||
| if ord_ == "asc": | ||
| return sql.order_by( | ||
| nulls_last(expr.asc()), | ||
| thing_table.id.asc(), | ||
| ) | ||
| return sql.order_by( | ||
| nulls_last(expr.desc()), | ||
| thing_table.id.desc(), | ||
| ) | ||
|
|
||
| def num_order(expr): | ||
| if ord_ == "asc": | ||
| return sql.order_by( | ||
| nulls_last(expr.asc()), | ||
| thing_table.id.asc(), | ||
| ) | ||
| return sql.order_by( | ||
| nulls_last(expr.desc()), | ||
| thing_table.id.desc(), | ||
| ) | ||
|
|
There was a problem hiding this comment.
str_order and num_order are currently identical (same nulls_last(...), thing_table.id tie-break). This duplication makes future changes error-prone (easy to update one and forget the other). Consider collapsing to a single helper (or making the numeric vs string distinction explicit if different behavior is intended).
| "/thing/water-well", | ||
| params={ | ||
| "sort": "measuring_point_height", | ||
| "order": "desc", |
There was a problem hiding this comment.
The test name says _asc, but the request uses order: "desc". This mismatch makes the test intent unclear; either rename the test to _desc or change the request to order: "asc".
| "order": "desc", | |
| "order": "asc", |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce1e533aba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sh.start_date == max_start, | ||
| ) | ||
| .correlate(thing_table) | ||
| .scalar_subquery() |
There was a problem hiding this comment.
The new latest-status sort subquery can return more than one row when a thing has multiple open StatusHistory entries with the same start_date (there is no uniqueness constraint in db/status_history.py to prevent this). In PostgreSQL, using that multi-row result as a scalar expression causes a cardinality error at runtime, so sorting by monitoring_status, well_status, datalogger_suitability_status, or open_status can fail for affected data. Add a deterministic tie-break (for example ORDER BY ... LIMIT 1) in this scalar subquery.
Useful? React with 👍 / 👎.
Final release before launch