Keep the taxa-list filters when opening a taxon's occurrences, and add Device & Site filters#1347
Keep the taxa-list filters when opening a taxon's occurrences, and add Device & Site filters#1347mihow wants to merge 7 commits into
Conversation
On the taxa list, clicking a row's occurrence or verified count opened the occurrence list showing every occurrence of that taxon, ignoring the station and verification filters that were active on the taxa list. Drilling in now carries those filters over so the occurrence list stays scoped to the same selection (station, session, device, site, verification, taxa list, default filters). The verified-count link still forces verified=true, since that is what the column represents. Both the taxa and occurrence lists also gain Device and Site filters, matching the existing Station filter. A deployment records a device (camera/hardware configuration) and a research site; users can now scope either list to one device or one site, e.g. to build a per-site presence matrix. Combined with the existing verification filter this supports the presence-verification workflow in issue #1320 (a quick preliminary step ahead of the larger Example-column work). Backend: deployment__device and deployment__research_site are added to the occurrence filterset, and the taxa view's get_occurrence_filters reads the same two params (restricting taxa membership, 404 on an unknown id). No model change, so no migration. Refs #1320 Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for antenna-ssec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Warning Review limit reached
More reviews will be available in 29 minutes and 50 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds backend support for deployment device/site occurrence filters, new UI filter options, carry-over filter contracts, and species/species-details navigation that preserves active filter state when linking to taxa and occurrences views. ChangesDeployment device/site filtering
Sequence DiagramsequenceDiagram
participant User
participant SpeciesPage
participant useCarryOverFilters
participant getAppRoute
participant TaxonViewSet
participant Device
participant Site
User->>SpeciesPage: open species view
SpeciesPage->>useCarryOverFilters: derive carry-over filters
SpeciesPage->>getAppRoute: build taxa and occurrence links
User->>TaxonViewSet: follow filtered link
TaxonViewSet->>Device: resolve deployment__device id
TaxonViewSet->>Site: resolve deployment__research_site id
TaxonViewSet-->>User: filtered taxa or occurrences
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #1349 |
get_occurrence_filters validated related-object ids with Model.objects.get(id=...) inside a try that only caught ObjectDoesNotExist. A non-integer value such as ?deployment__device=abc raised ValueError, which fell through as an unhandled 500. The occurrence list returns a 400 for the same input (via django-filter), so the two endpoints disagreed. Catch the non-integer case and raise a 400 so both endpoints agree. This covers the new deployment__device / deployment__research_site params and the pre-existing deployment / event / collection / occurrence ids, which shared the same lookup. Co-Authored-By: Claude <noreply@anthropic.com>
| | 'capture' | ||
| | 'collection' | ||
| | 'collections' | ||
| | 'date_end' |
There was a problem hiding this comment.
I see date params and not_taxa_list_id are being added in addition to the device & site params. are those additional ones missing/broken on live currently?
There was a problem hiding this comment.
Claude says: These are type-only entries in the FilterType union — adding a key here changes no runtime behavior, it just lets a link reference that param in a URL. None of them are missing or broken on live: date_start/date_end are the existing date-range filter in the occurrence "More filters" section, and not_taxa_list_id is "Exclude taxa from list" — all already shipped and working.
They showed up here because I extended the union while wiring the filter carry-over, but they turned out unnecessary: carry-over spreads a string-keyed record into getAppRoute({ filters }), which is already assignable without per-field union entries — only fields used as explicit literal keys (e.g. taxon, verified) need to be in the union. So I've trimmed date_start / date_end / not_taxa_list_id in 25d66a7. deployment__device / deployment__research_site stay, since those are the new filter params this PR introduces.
… on the detail panel The taxa list and the taxon detail panel both link into the occurrence list from an occurrence count and a verified count. The list rows carried the active taxa-list filters (station, device, site, verification, ...) into that link, but the detail panel did not — it reset to every occurrence of the taxon. The two now behave the same. The carry-over field list and the logic that builds the filter object move into useFilters.ts as TAXA_OCCURRENCE_CARRY_OVER_FIELDS, buildCarryOverFilters, and the useCarryOverFilters hook, so there is a single source of truth instead of a constant living in one page component. The taxa table (via species.tsx) and the detail panel both consume the hook. No behavior change for the taxa table; the detail panel's two links now carry the same filters. Co-Authored-By: Claude <noreply@anthropic.com>
…ilters into the child-taxa link Reorder the taxa-list filter panel to mirror the occurrence list: the primary section now leads with Taxon, then Taxa in list / not in list, Verification status, Show unobserved taxa, and Default filters; Station, Device, Site, and the tag filters move into a "More filters" section that opens automatically when one of them is set. The two list views now read the same way. The taxon detail panel's "Child taxa" link also carries the active filters now, so stepping from a taxon to its children keeps the same station / device / site / verification scope, matching the occurrence and verified links next to it. Rename TAXA_OCCURRENCE_CARRY_OVER_FIELDS to CARRY_OVER_FILTER_FIELDS since the same set now feeds both the occurrence links and the child-taxa link. Co-Authored-By: Claude <noreply@anthropic.com>
…S / FILTERS_TO_TAXA) Replace the single carry-over field list with two destination-keyed sets in useFilters.ts: FILTERS_TO_OCCURRENCES and FILTERS_TO_TAXA, each listing the filter fields its destination list understands. Carry-over is now the intersection of the current view's active filters with the destination's set, so the source is implicit and any view that links to a destination reuses the same set — the behavior is consistent no matter where the link is. This also corrects two cases for the taxon detail panel: "show unobserved taxa" and the tag filters now carry along the child-taxa link (they are taxa-list filters) but still do not leak into the occurrence list (which does not support them). buildCarryOverFilters and the useCarryOverFilters hook take the destination field set as an argument. Only the taxa views consume them so far; the sets are ready for other views' links to reuse. Co-Authored-By: Claude <noreply@anthropic.com>
Each list's carry-over contract now lives next to that list's page: FILTERS_TO_OCCURRENCES in pages/occurrences/occurrence-filters.ts and FILTERS_TO_TAXA in pages/species/species-filters.ts, each documenting the two bounds it must satisfy (the backend honors the field, and the panel can display it so a carried filter is visible and clearable). The carry-over helper stays generic and destination-agnostic. Also: - Extract the pure buildCarryOverFilters into its own dependency-free module so it can be unit-tested without loading the filter registry; the useCarryOverFilters hook keeps it in useFilters.ts. - Add carryOverFilters.test.ts: the intersection behavior, no pagination/sort leak, and the "show unobserved taxa" taxa-only invariant. - Drop date_start / date_end / not_taxa_list_id from getAppRoute's FilterType union — they were type-only additions that are not used as explicit link params (carry-over spreads a string-keyed record, which does not need them in the union). Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR improves taxa/occurrence list navigation by preserving the user’s active filter context when drilling down from taxa → occurrences (and taxa child links), and adds Device + Site filtering across both lists with backend support and regression tests.
Changes:
- Add Device (
deployment__device) and Site (deployment__research_site) filters to taxa and occurrence list UIs, plus carry-over support between list routes. - Introduce explicit “carry-over contracts” (
FILTERS_TO_OCCURRENCES,FILTERS_TO_TAXA) and a sharedbuildCarryOverFiltershelper/hook to avoid leaking source-only state (page/sort) into destination URLs. - Extend the occurrence/taxa backend filtering to accept the new parameters and add API regression coverage (including 400-on-non-integer behavior).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ui/src/utils/useFilters.ts | Adds Device/Site to the filter registry; introduces useCarryOverFilters hook. |
| ui/src/utils/getAppRoute.ts | Extends route filter typing to include new query params. |
| ui/src/utils/carryOverFilters.test.ts | Unit tests for buildCarryOverFilters and carry-over contracts. |
| ui/src/utils/buildCarryOverFilters.ts | New pure helper for carrying active filters into destination URLs. |
| ui/src/pages/species/species.tsx | Reorders taxa filters into sections; wires carry-over into taxa table links. |
| ui/src/pages/species/species-filters.ts | Defines taxa-list carry-over contract. |
| ui/src/pages/species/species-columns.tsx | Applies carried filters when linking from taxa rows to occurrences. |
| ui/src/pages/species-details/species-details.tsx | Applies carry-over filters for “view all” / child taxa / verified links. |
| ui/src/pages/occurrences/occurrences.tsx | Adds Device/Site controls to “More filters” and auto-open logic. |
| ui/src/pages/occurrences/occurrence-filters.ts | Defines occurrence-list carry-over contract. |
| ui/src/components/filtering/filters/device-filter.tsx | New Device entity-picker filter control. |
| ui/src/components/filtering/filters/site-filter.tsx | New Site entity-picker filter control. |
| ui/src/components/filtering/filter-control.tsx | Registers device/site filter components. |
| ami/main/tests.py | Adds API tests validating device/site filtering and error semantics. |
| ami/main/api/views.py | Adds backend support for new filter params + 400-on-non-integer handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if occurrence_id: | ||
| Occurrence.objects.get(id=occurrence_id) | ||
| # This query does not need the same filtering as the others | ||
| filters &= models.Q(**{field("id"): occurrence_id}) | ||
| if deployment_id: | ||
| Deployment.objects.get(id=deployment_id) | ||
| filters &= models.Q(**{field("deployment"): deployment_id}) | ||
| if device_id: | ||
| Device.objects.get(id=device_id) | ||
| filters &= models.Q(**{field("deployment__device"): device_id}) | ||
| if site_id: | ||
| Site.objects.get(id=site_id) | ||
| filters &= models.Q(**{field("deployment__research_site"): site_id}) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ui/src/utils/useFilters.ts (2)
81-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute the
Device/Sitelabels through the translation layer.Per the project guideline, user-facing strings should use
translate(STRING.KEY)with keys added to theSTRINGenum andENGLISH_STRINGS. The neighboring'Station'entry predates this, but the new entries should ideally follow the rule.As per coding guidelines: "All user-facing strings must go through the translation layer using
translate(STRING.KEY)."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/utils/useFilters.ts` around lines 81 - 88, The Device and Site filter labels are hardcoded user-facing strings and should be routed through the translation layer like the other translated labels. Update the filter definitions in useFilters so they use translate(STRING.KEY) instead of literal text, and add the corresponding keys to STRING and ENGLISH_STRINGS to match the existing localization pattern.Source: Coding guidelines
292-300: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
useMemohere is effectively a no-op.
useFilters()returns a freshly-builtfiltersarray on every render, so the[filters, fields]dependency changes each render and the memo recomputes every time (also returning a new object identity). The computation is cheap so this is harmless today, but if a consumer feeds the returned record into effect deps or query keys it could trigger extra work. Memoizing on a stable serialization (or accepting the recompute and droppinguseMemo) would be clearer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/src/utils/useFilters.ts` around lines 292 - 300, The memoization in useCarryOverFilters is ineffective because useFilters returns a new filters array every render, so useMemo always recomputes and still produces a new object identity. Either remove useMemo and return buildCarryOverFilters(filters, fields) directly, or switch the memoization to a stable dependency strategy if you truly need caching; keep the fix centered on useCarryOverFilters, useFilters, and buildCarryOverFilters so the behavior is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ami/main/api/views.py`:
- Around line 1787-1791: The exception handling in the API view’s
ValueError/TypeError branch should explicitly preserve the original traceback
when raising api_exceptions.ValidationError. Update the re-raise in this
filter-id validation path to use exception chaining from the caught error so the
original failure context is not lost and the code stays compliant with the
project’s flake8 B904 rule. Locate the try/except block in the view logic that
validates integer filter ids and adjust the ValidationError raise accordingly.
In `@ui/src/pages/species/species.tsx`:
- Around line 94-95: The FilterSection title in species.tsx is hardcoded
user-facing copy and must be moved through the translation layer. Add a shared
STRING key and ENGLISH_STRINGS entry for this label, then update the
FilterSection usage in species.tsx to call translate(STRING.KEY) instead of the
literal text; also update occurrences.tsx to reuse the same shared key if it has
the same “More filters” label.
---
Nitpick comments:
In `@ui/src/utils/useFilters.ts`:
- Around line 81-88: The Device and Site filter labels are hardcoded user-facing
strings and should be routed through the translation layer like the other
translated labels. Update the filter definitions in useFilters so they use
translate(STRING.KEY) instead of literal text, and add the corresponding keys to
STRING and ENGLISH_STRINGS to match the existing localization pattern.
- Around line 292-300: The memoization in useCarryOverFilters is ineffective
because useFilters returns a new filters array every render, so useMemo always
recomputes and still produces a new object identity. Either remove useMemo and
return buildCarryOverFilters(filters, fields) directly, or switch the
memoization to a stable dependency strategy if you truly need caching; keep the
fix centered on useCarryOverFilters, useFilters, and buildCarryOverFilters so
the behavior is explicit.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2349cd0-91fa-46f1-8001-73cc1a5937a4
📒 Files selected for processing (15)
ami/main/api/views.pyami/main/tests.pyui/src/components/filtering/filter-control.tsxui/src/components/filtering/filters/device-filter.tsxui/src/components/filtering/filters/site-filter.tsxui/src/pages/occurrences/occurrence-filters.tsui/src/pages/occurrences/occurrences.tsxui/src/pages/species-details/species-details.tsxui/src/pages/species/species-columns.tsxui/src/pages/species/species-filters.tsui/src/pages/species/species.tsxui/src/utils/buildCarryOverFilters.tsui/src/utils/carryOverFilters.test.tsui/src/utils/getAppRoute.tsui/src/utils/useFilters.ts
… label through i18n Two review fixes: - get_occurrence_filters now chains its re-raised exceptions (raise ... from e) for both the NotFound and the ValidationError paths, so the original ObjectDoesNotExist / ValueError traceback is preserved for debugging. Matches the existing pattern in ami/base/fields.py. - The "More filters" section title now goes through translate(STRING.MORE_FILTERS) instead of a hardcoded literal, shared by the taxa and occurrence filter panels. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
On the taxa list you can narrow the view to a station and a verification status, but until now clicking a row's occurrence count or verified count threw that context away: it opened the occurrence list showing every occurrence of the taxon, across all stations and both verified and unverified. This PR keeps the filters you had set, so drilling into a taxon lands you on exactly the occurrences you were looking at — the same station, device, site, session, verification status, taxa list, and default-filter settings carry over.
It also adds two filters that were missing from both the taxa and occurrence lists: Device and Site. A deployment records the camera/hardware device it runs and the research site it sits at, so you can now scope either list to one device or one site. Combined with the existing verification filter, this is the small, immediately useful piece of the presence-verification workflow in #1320 — for example, sweeping unverified taxa one station (or one site) at a time — ahead of the larger Example-column work in that issue.
List of Changes
FILTERS_TO_OCCURRENCESinpages/occurrences/occurrence-filters.ts,FILTERS_TO_TAXAinpages/species/species-filters.ts), listing the fields that destination honors and can display. The genericuseCarryOverFiltershook (useFilters.ts) plus a dependency-freebuildCarryOverFiltershelper carry whichever of those fields is currently active. Explicit allow-lists (not the whole query string) keep source-only state like sort order, page number, and "show unobserved taxa" out of the occurrence URL, while the child-taxa link still carries "show unobserved taxa" and the tag filters because those are taxa-list filters. The taxa table (species-columns.tsx) and detail panel (species-details.tsx) share the same helper.species.tsxnow renders twoFilterSections mirroringoccurrences.tsx.verified=trueafter the carried filters.EntityPickerfilters (device-filter.tsx,site-filter.tsx) pointing at the existingdeployments/devicesanddeployments/sitesendpoints, wired into the filter registry and the taxa filter panel.deployment__deviceanddeployment__research_siteOCCURRENCE_FILTERSET_FIELDS.TaxonViewSet.get_occurrence_filtersreads and validates both params.?deployment__device=abc) now returns 400 on the taxa endpoint instead of a 500get_occurrence_filterscatches the non-integer case and raises a 400, matching what the occurrence endpoint already returns. This also fixes the same pre-existing 500 fordeployment,event, andcollection, which shared the lookup.TestDeviceAndSiteFilters(6 tests) pins occurrence filtering, taxa membership restriction, the 404-on-unknown-id behaviour, and the 400-on-non-integer behaviour for both device and site. Frontend:carryOverFilters.test.ts(5 tests) pins the carry-over intersection, that no pagination/sort state leaks, and that "show unobserved taxa" stays a taxa-only carry.Related Issues
Relates to #1320 (a preliminary step; does not close it).
Detailed Description
The filter labelled "Device" maps to the
Devicemodel (the camera/hardware configuration,verbose_name = "Device Configuration"); there is no separate device-type concept in the schema, so the filter is by device. "Site" maps to the deployment'sresearch_site.Both endpoints use the same query-param keys —
deployment__deviceanddeployment__research_site— so the frontend filter wiring is identical for the two lists. On the occurrence endpoint these are plain django-filter exact lookups. On the taxa endpoint they flow throughget_occurrence_filters, which already special-casesdeployment/event/collection; the new params are validated the same way (a missingDevice/SiteraisesNotFound) and restrict taxa membership through the existing observation-count subquery and aggregation paths, so all dispatch paths (default, verified, event, deployment, collection) inherit them.No model fields changed, so there is no migration.
Interaction with "show unobserved taxa". On the taxa list, the Device and Site filters restrict which taxa appear only when "show unobserved taxa" is off (the default). With it on, the list shows the full project checklist and the Device/Site/Station/Session filters scope each row's occurrence counts but not membership — the device/site filters ride the same
direct_filterspath as the existing Station and Session filters, which already behave this way (the membership restriction is intentionally bypassed wheninclude_unobservedis set). This is existing behavior, not introduced here; making those filters also restrict membership in that mode would be a separate change touching all of them.How to Test the Changes
Automated, backend:
python manage.py test ami.main.tests.TestDeviceAndSiteFilters(6 tests) plus the existingTestTaxaVerification/TestTaxonomyViewssuites. Frontend:cd ui && yarn test(includessrc/utils/carryOverFilters.test.ts).Manual (verified on a project with more than one device/site): on the taxa list, set the Device filter and the verification status, then click a row's occurrence count — the occurrence list opens with the same Device and verification filters applied and the result count matches the bubble. Repeat with the Site filter.
Deployment Notes
No migration and no configuration change. Deployed to a dev box for review.
Checklist
Summary by CodeRabbit
New Features
Bug Fixes