-
Notifications
You must be signed in to change notification settings - Fork 5
Advanced collection filters #895
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
Conversation
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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 enables advanced filtering options on source image collections by introducing utilities for parsing and validating comma-separated integer lists, updating the frontend form to include new filter fields, and extending the backend to accept and apply event_ids
and research_site_ids
.
- Added
validateInteger
,parseIntegerList
,formatIntegerList
, andvalidateIntegerList
utilities. - Extended
CollectionDetailsForm
with "Advanced Filters" fields and wired uptoApiValue
/toFormValue
transformations. - Updated Django models and serializers to accept and filter by
event_ids
andresearch_site_ids
. - Added an
ID
column in the entities table for better sorting and display.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
ui/src/utils/fieldProcessors.ts | Introduced integer list parsing/formatting/validation utilities |
ui/src/pages/project/entities/entities-columns.tsx | Added id column to the entities table |
ui/src/pages/project/entities/details-form/collection-details-form.tsx | Integrated new filter fields and refactored integer validation in the collection details form |
ui/src/components/form/types.ts | Extended FieldConfig type with toApiValue and toFormValue |
ami/main/models.py | Updated sampling methods and _filter_sample to handle event_ids and research_site_ids |
ami/main/api/serializers.py | Added event_ids and research_site_ids to the common kwargs serializer |
Comments suppressed due to low confidence (2)
ui/src/utils/fieldProcessors.ts:23
- Add unit tests covering
parseIntegerList
,formatIntegerList
, andvalidateIntegerList
to ensure these utilities handle edge cases (empty strings, invalid tokens) correctly.
export const parseIntegerList = (
ami/main/models.py:3146
- [nitpick] Switching to
*args, **kwargs
hides the expected filter parameters and reduces readability; consider restoring explicit parameters (hour_start
,hour_end
, etc.) in the signature for clarity.
def get_queryset(
ui/src/utils/fieldProcessors.ts
Outdated
if (!value || value.trim() === '') return undefined // Optional field | ||
const pattern = /^\s*\d+\s*(?:\s*,\s*\d+\s*)*$/ | ||
if (!pattern.test(value)) { | ||
return 'Enter comma-separated integers (e.g., 1, 2, 3).' |
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.
Use the translate
function for this error message to ensure consistency with localization (e.g., translate(STRING.MESSAGE_INTEGER_LIST_INVALID)
).
Copilot uses AI. Check for mistakes.
74269f1
to
5d33718
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Thanks @annavik! I agree with you and I look forward to a Show More button here. The Site filter will be really convenient when we add it because you only need to specify one Site instead of looking up all the Station IDs. But I agree that just Station IDs & Session IDs keeps it more flexible and just one row of new fields. Another option (for a follow-up PR) is to add a general config with a JSON editor. Then we don't have to restrict any functionality and encourage users to switch away from Antenna back to their own scripts, etc. That would be my preference across the site (hidden by default). I think this is a common pattern, where the UI provides a visual config builder for basic things, but then lets you switch to the advanced/code mode to edit some syntax directly (QGIS, NewRelic, LabelStudio, Overleaf, Weights & Biases, any WYSIWYG editor, are some examples that come to mind). |
…ed-collection-filters-2
…ckLab/antenna into feat/advanced-collection-filters-2
Summary
Enable several needed filter methods for source image collections to the UI, using raw IDs.
List of Changes
deployment_ids
filter to the UI - very helpful, but currently requires Django admin to usestation_ids
- requested by partnerevent_ids
- required for testing trackingImplementation Details
This is a simple way to allow filtering by related data using existing UI components. However a custom validator was added for comma separated integers, as well as a way to serialize & deserialize JSON lists to & from the API.