diff --git a/ami/main/api/views.py b/ami/main/api/views.py index e44ee1b60..9be02fca3 100644 --- a/ami/main/api/views.py +++ b/ami/main/api/views.py @@ -1350,6 +1350,8 @@ def filter_queryset(self, request, queryset, view): OCCURRENCE_FILTERSET_FIELDS = ( "event", "deployment", + "deployment__device", + "deployment__research_site", "determination__rank", "detections__source_image", ) @@ -1744,6 +1746,8 @@ def get_occurrence_filters(self, project: Project, accessor: str = "") -> models deployment_id = self.request.query_params.get("deployment") or self.request.query_params.get( "occurrences__deployment" ) + device_id = self.request.query_params.get("deployment__device") + site_id = self.request.query_params.get("deployment__research_site") event_id = self.request.query_params.get("event") or self.request.query_params.get("occurrences__event") collection_id = self.request.query_params.get("collection") @@ -1765,6 +1769,12 @@ def field(path: str) -> str: 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}) if event_id: Event.objects.get(id=event_id) filters &= models.Q(**{field("event"): event_id}) @@ -1773,7 +1783,12 @@ def field(path: str) -> str: filters &= models.Q(**{field("detections__source_image__collections"): collection_id}) except exceptions.ObjectDoesNotExist as e: # Raise a 404 if any of the related objects don't exist - raise NotFound(detail=str(e)) + raise NotFound(detail=str(e)) from e + except (ValueError, TypeError) as e: + # A non-integer id (e.g. ?deployment__device=abc) is a client error. Return a + # 400 instead of letting the .get(id=...) lookup surface an unhandled 500, so + # this endpoint matches the 400 the occurrence list returns for the same input. + raise api_exceptions.ValidationError(detail="Filter ids must be integers.") from e return filters diff --git a/ami/main/tests.py b/ami/main/tests.py index 4c8b4b219..98f6f6dd3 100644 --- a/ami/main/tests.py +++ b/ami/main/tests.py @@ -6011,6 +6011,92 @@ def test_verified_count_not_inflated_by_collection_join(self): self.assertEqual(rows["Vanessa cardui"]["verified_count"], 2) +class TestDeviceAndSiteFilters(APITestCase): + """Filtering occurrences and taxa by the deployment's device and research site. + + A deployment records both a ``device`` (the camera/hardware configuration) and a + ``research_site``. Users can scope the occurrence and taxa lists to one device or + one site so they can, for example, build a presence matrix per site. These tests + pin that the occurrence list filters exactly to the chosen device/site, that the + taxa list restricts membership the same way (not just its annotated counts), and + that an unknown device/site id is rejected with a 404 on the taxa endpoint. + """ + + def setUp(self): + self.project, self.deployment_a = setup_test_project(reuse=False) + create_taxa(self.project) + self.cardui = Taxon.objects.get(name="Vanessa cardui") + self.atalanta = Taxon.objects.get(name="Vanessa atalanta") + + # Two devices and two sites, each pinned to its own deployment. + self.device_a = Device.objects.create(name="Device A", project=self.project) + self.device_b = Device.objects.create(name="Device B", project=self.project) + self.site_a = Site.objects.create(name="Site A", project=self.project) + self.site_b = Site.objects.create(name="Site B", project=self.project) + + self.deployment_a.device = self.device_a + self.deployment_a.research_site = self.site_a + self.deployment_a.save() + self.deployment_b = Deployment.objects.create( + name="Deployment B", + project=self.project, + device=self.device_b, + research_site=self.site_b, + ) + + # Deployment A only sees cardui; deployment B only sees atalanta. This lets the + # device/site filter be checked by both the occurrence count and the taxa membership. + create_captures(deployment=self.deployment_a, num_nights=1, images_per_night=2) + create_captures(deployment=self.deployment_b, num_nights=1, images_per_night=2) + create_occurrences(deployment=self.deployment_a, num=3, taxon=self.cardui, determination_score=0.9) + create_occurrences(deployment=self.deployment_b, num=2, taxon=self.atalanta, determination_score=0.9) + + def _occurrence_count(self, **params): + query = "&".join(f"{k}={v}" for k, v in params.items()) + res = self.client.get(f"/api/v2/occurrences/?project_id={self.project.pk}&{query}") + self.assertEqual(res.status_code, status.HTTP_200_OK) + return res.json()["count"] + + def _taxa_names(self, **params): + query = "&".join(f"{k}={v}" for k, v in params.items()) + res = self.client.get(f"/api/v2/taxa/?project_id={self.project.pk}&limit=1000&{query}") + self.assertEqual(res.status_code, status.HTTP_200_OK) + return {row["name"] for row in res.json()["results"]} + + def test_occurrences_filtered_by_device(self): + self.assertEqual(self._occurrence_count(), 5) + self.assertEqual(self._occurrence_count(deployment__device=self.device_a.pk), 3) + self.assertEqual(self._occurrence_count(deployment__device=self.device_b.pk), 2) + + def test_occurrences_filtered_by_site(self): + self.assertEqual(self._occurrence_count(deployment__research_site=self.site_a.pk), 3) + self.assertEqual(self._occurrence_count(deployment__research_site=self.site_b.pk), 2) + + def test_taxa_membership_restricted_by_device(self): + # The taxa list must drop to only the taxa observed on the chosen device, not + # merely re-scope the counts of the full taxa set. + self.assertEqual(self._taxa_names(deployment__device=self.device_a.pk), {"Vanessa cardui"}) + self.assertEqual(self._taxa_names(deployment__device=self.device_b.pk), {"Vanessa atalanta"}) + + def test_taxa_membership_restricted_by_site(self): + self.assertEqual(self._taxa_names(deployment__research_site=self.site_a.pk), {"Vanessa cardui"}) + self.assertEqual(self._taxa_names(deployment__research_site=self.site_b.pk), {"Vanessa atalanta"}) + + def test_unknown_device_or_site_returns_404_on_taxa(self): + for param in ("deployment__device", "deployment__research_site"): + res = self.client.get(f"/api/v2/taxa/?project_id={self.project.pk}&{param}=999999") + self.assertEqual(res.status_code, status.HTTP_404_NOT_FOUND, param) + + def test_non_integer_id_returns_400_not_500(self): + # A malformed id must be a client error on both endpoints, not an unhandled 500. + # The taxa view validates the id before the existence lookup; the occurrence view + # gets the same 400 from django-filter. Both must agree. + for endpoint in ("taxa", "occurrences"): + for param in ("deployment__device", "deployment__research_site"): + res = self.client.get(f"/api/v2/{endpoint}/?project_id={self.project.pk}&{param}=abc") + self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST, f"{endpoint}?{param}=abc") + + class TestDetectionNullMarker(TestCase): """ Covers the null-marker abstraction added for Issue #1310 follow-up: diff --git a/ui/src/components/filtering/filter-control.tsx b/ui/src/components/filtering/filter-control.tsx index 0e0944919..64dafd9cc 100644 --- a/ui/src/components/filtering/filter-control.tsx +++ b/ui/src/components/filtering/filter-control.tsx @@ -6,9 +6,11 @@ import { AlgorithmFilter, NotAlgorithmFilter } from './filters/algorithm-filter' import { BooleanFilter } from './filters/boolean-filter' import { CaptureSetFilter } from './filters/capture-set-filter' import { DateFilter } from './filters/date-filter' +import { DeviceFilter } from './filters/device-filter' import { ImageFilter } from './filters/image-filter' import { PipelineFilter } from './filters/pipeline-filter' import { SessionFilter } from './filters/session-filter' +import { SiteFilter } from './filters/site-filter' import { StationFilter } from './filters/station-filter' import { StatusFilter } from './filters/status-filter' import { TagFilter } from './filters/tag-filter' @@ -29,6 +31,8 @@ const ComponentMap: { date_end: DateFilter, date_start: DateFilter, deployment: StationFilter, + deployment__device: DeviceFilter, + deployment__research_site: SiteFilter, detections__source_image: ImageFilter, event: SessionFilter, processed: ProcessingStatusFilter, diff --git a/ui/src/components/filtering/filters/device-filter.tsx b/ui/src/components/filtering/filters/device-filter.tsx new file mode 100644 index 000000000..eb4b025a5 --- /dev/null +++ b/ui/src/components/filtering/filters/device-filter.tsx @@ -0,0 +1,17 @@ +import { API_ROUTES } from 'data-services/constants' +import { EntityPicker } from 'nova-ui-kit' +import { FilterProps } from './types' + +export const DeviceFilter = ({ onAdd, onClear, value }: FilterProps) => ( + { + if (value) { + onAdd(value) + } else { + onClear() + } + }} + value={value} + /> +) diff --git a/ui/src/components/filtering/filters/site-filter.tsx b/ui/src/components/filtering/filters/site-filter.tsx new file mode 100644 index 000000000..27cc7f05e --- /dev/null +++ b/ui/src/components/filtering/filters/site-filter.tsx @@ -0,0 +1,17 @@ +import { API_ROUTES } from 'data-services/constants' +import { EntityPicker } from 'nova-ui-kit' +import { FilterProps } from './types' + +export const SiteFilter = ({ onAdd, onClear, value }: FilterProps) => ( + { + if (value) { + onAdd(value) + } else { + onClear() + } + }} + value={value} + /> +) diff --git a/ui/src/pages/occurrences/occurrence-filters.ts b/ui/src/pages/occurrences/occurrence-filters.ts new file mode 100644 index 000000000..a8fff9c0b --- /dev/null +++ b/ui/src/pages/occurrences/occurrence-filters.ts @@ -0,0 +1,33 @@ +// The occurrence list's filter carry-over contract: filter fields that another view may +// carry into the occurrence list (see useCarryOverFilters). A field belongs here only if +// both bounds hold: +// +// 1. The occurrence list backend honors it — keep this in sync by hand with the server +// filterset (OCCURRENCE_FILTERSET_FIELDS, the custom occurrence filter backends, and +// TaxonViewSet.get_occurrence_filters in ami/main/api/views.py). +// 2. The occurrence filter panel can display it — every field here has a control on the +// panel (a pickable control, or a readonly chip such as event/capture that appears +// once set), so a carried filter is always visible and clearable on arrival rather +// than silently shrinking the list. +// +// This is a curated subset, not "every field the panel renders" — a field can be on the +// panel yet deliberately left out of carry-over. The carryOverFilters test pins that every +// field here is a registered filter. +export const FILTERS_TO_OCCURRENCES = [ + 'detections__source_image', + 'event', + 'taxon', + 'taxa_list_id', + 'not_taxa_list_id', + 'verified', + 'verified_by_me', + 'collection', + 'date_start', + 'date_end', + 'deployment', + 'deployment__device', + 'deployment__research_site', + 'algorithm', + 'not_algorithm', + 'apply_defaults', +] diff --git a/ui/src/pages/occurrences/occurrences.tsx b/ui/src/pages/occurrences/occurrences.tsx index a527c04ed..79bacc563 100644 --- a/ui/src/pages/occurrences/occurrences.tsx +++ b/ui/src/pages/occurrences/occurrences.tsx @@ -111,9 +111,16 @@ export const Occurrences = () => { @@ -121,6 +128,8 @@ export const Occurrences = () => { + + diff --git a/ui/src/pages/species-details/species-details.tsx b/ui/src/pages/species-details/species-details.tsx index 6d4adc5f1..81e8c5f53 100644 --- a/ui/src/pages/species-details/species-details.tsx +++ b/ui/src/pages/species-details/species-details.tsx @@ -20,6 +20,9 @@ import { APP_ROUTES } from 'utils/constants' import { getFormatedDateTimeString } from 'utils/date/getFormatedDateTimeString/getFormatedDateTimeString' import { getAppRoute } from 'utils/getAppRoute' import { STRING, translate } from 'utils/language' +import { useCarryOverFilters } from 'utils/useFilters' +import { FILTERS_TO_OCCURRENCES } from 'pages/occurrences/occurrence-filters' +import { FILTERS_TO_TAXA } from 'pages/species/species-filters' import { UserPermission } from 'utils/user/types' import styles from './species-details.module.scss' @@ -40,6 +43,8 @@ export const SpeciesDetails = ({ const { projectId } = useParams() const navigate = useNavigate() const { project } = useProjectDetails(projectId as string, true) + const occurrenceFilters = useCarryOverFilters(FILTERS_TO_OCCURRENCES) + const taxaFilters = useCarryOverFilters(FILTERS_TO_TAXA) const canUpdate = species.userPermissions.includes(UserPermission.Update) const hasChildren = species.rank !== 'SPECIES' @@ -139,7 +144,7 @@ export const SpeciesDetails = ({ to: APP_ROUTES.TAXA({ projectId: projectId as string, }), - filters: { taxon: species.id }, + filters: { ...taxaFilters, taxon: species.id }, })} /> @@ -154,7 +159,7 @@ export const SpeciesDetails = ({ to: APP_ROUTES.OCCURRENCES({ projectId: projectId as string, }), - filters: { taxon: species.id }, + filters: { ...occurrenceFilters, taxon: species.id }, })} /> @@ -165,7 +170,11 @@ export const SpeciesDetails = ({ to: APP_ROUTES.OCCURRENCES({ projectId: projectId as string, }), - filters: { taxon: species.id, verified: 'true' }, + filters: { + ...occurrenceFilters, + taxon: species.id, + verified: 'true', + }, })} /> diff --git a/ui/src/pages/species/species-columns.tsx b/ui/src/pages/species/species-columns.tsx index b7f7a33ec..98d5218b2 100644 --- a/ui/src/pages/species/species-columns.tsx +++ b/ui/src/pages/species/species-columns.tsx @@ -19,7 +19,15 @@ import { STRING, translate } from 'utils/language' export const columns: (project: { projectId: string featureFlags?: { [key: string]: boolean } -}) => TableColumn[] = ({ projectId, featureFlags }) => [ + // Active taxa-list filters (station, verified, device, site, …) carried over + // when drilling into a taxon's occurrences so the occurrence list stays scoped + // to the same selection instead of showing every occurrence of the taxon. + carryFilters?: Record +}) => TableColumn[] = ({ + projectId, + featureFlags, + carryFilters = {}, +}) => [ { id: 'cover-image', name: translate(STRING.FIELD_LABEL_IMAGE), @@ -88,7 +96,7 @@ export const columns: (project: { @@ -106,7 +114,7 @@ export const columns: (project: { diff --git a/ui/src/pages/species/species-filters.ts b/ui/src/pages/species/species-filters.ts new file mode 100644 index 000000000..157f76352 --- /dev/null +++ b/ui/src/pages/species/species-filters.ts @@ -0,0 +1,28 @@ +// The taxa list's filter carry-over contract: filter fields that another view may carry +// into the taxa list (see useCarryOverFilters). A field belongs here only if both bounds +// hold: +// +// 1. The taxa list backend honors it — keep this in sync by hand with the server +// filterset (TaxonViewSet.filterset_fields and get_occurrence_filters in +// ami/main/api/views.py). +// 2. The taxa filter panel can display it — every field here has a control on the panel +// (pickable, or a readonly chip such as event that appears once set), so a carried +// filter is always visible and clearable on arrival. +// +// Differs from FILTERS_TO_OCCURRENCES where the lists differ: the taxa list carries +// "show unobserved taxa" and the tag filters (its own filters) but not the occurrence-only +// ones. The carryOverFilters test pins that every field here is a registered filter. +export const FILTERS_TO_TAXA = [ + 'event', + 'taxon', + 'taxa_list_id', + 'not_taxa_list_id', + 'verified', + 'include_unobserved', + 'deployment', + 'deployment__device', + 'deployment__research_site', + 'tag_id', + 'not_tag_id', + 'apply_defaults', +] diff --git a/ui/src/pages/species/species.tsx b/ui/src/pages/species/species.tsx index 60b8e460f..f2a3b0274 100644 --- a/ui/src/pages/species/species.tsx +++ b/ui/src/pages/species/species.tsx @@ -1,6 +1,7 @@ import { DefaultFiltersControl } from 'components/filtering/default-filter-control' import { FilterControl } from 'components/filtering/filter-control' import { FilterSection } from 'components/filtering/filter-section' +import { someActive } from 'components/filtering/utils' import { useProjectDetails } from 'data-services/hooks/projects/useProjectDetails' import { useSpecies } from 'data-services/hooks/species/useSpecies' import { useSpeciesDetails } from 'data-services/hooks/species/useSpeciesDetails' @@ -25,7 +26,8 @@ import { APP_ROUTES } from 'utils/constants' import { getAppRoute } from 'utils/getAppRoute' import { STRING, translate } from 'utils/language' import { useColumnSettings } from 'utils/useColumnSettings' -import { useFilters } from 'utils/useFilters' +import { useCarryOverFilters, useFilters } from 'utils/useFilters' +import { FILTERS_TO_OCCURRENCES } from 'pages/occurrences/occurrence-filters' import { usePagination } from 'utils/usePagination' import { useSelectedView } from 'utils/useSelectedView' import { useSort } from 'utils/useSort' @@ -48,7 +50,7 @@ export const Species = () => { }) const { sort, setSort } = useSort({ field: 'name', order: 'asc' }) const { pagination, setPage } = usePagination() - const { filters } = useFilters() + const { activeFilters, filters } = useFilters() const { species, total, isLoading, isFetching, error } = useSpecies({ projectId, sort, @@ -58,6 +60,7 @@ export const Species = () => { const { selectedView, setSelectedView } = useSelectedView('table') const { taxaLists = [] } = useTaxaLists({ projectId: projectId as string }) const { tags = [] } = useTags({ projectId: projectId as string }) + const carryFilters = useCarryOverFilters(FILTERS_TO_OCCURRENCES) const pageTitle = useMemo(() => { const taxaListFilter = filters.find( (filter) => filter.field === 'taxa_list_id' @@ -74,26 +77,44 @@ export const Species = () => { return ( <>
- - - - - {taxaLists.length > 0 && ( - <> - - - - )} - - - {project?.featureFlags.tags ? ( - <> - - - - ) : null} - - +
+ + + + {taxaLists.length > 0 && ( + <> + + + + )} + + + + + + + + + {project?.featureFlags.tags ? ( + <> + + + + ) : null} + +
{ columns={columns({ projectId: projectId as string, featureFlags: project?.featureFlags, + carryFilters, }).filter((column) => !!columnSettings[column.id])} error={error} isLoading={!id && isLoading} diff --git a/ui/src/utils/buildCarryOverFilters.ts b/ui/src/utils/buildCarryOverFilters.ts new file mode 100644 index 000000000..a40a8c1e7 --- /dev/null +++ b/ui/src/utils/buildCarryOverFilters.ts @@ -0,0 +1,23 @@ +// Carry filters from one list view into another. +// +// `fields` is the DESTINATION list's carry contract — the filter fields that destination +// honors and is willing to receive — defined as a constant next to that destination's page +// (e.g. FILTERS_TO_OCCURRENCES in pages/occurrences). The source is implicit: whichever of +// those fields is currently active in the source view is carried into the destination URL, +// so the destination keeps the same scope (station, device, site, verification, ...) +// instead of resetting. Passing the destination's own field list — rather than copying the +// whole query string — keeps source-only state (sort order, page number, or a filter the +// destination does not support) out of the URL. +// +// Pure and dependency-free so it can be unit-tested without loading the filter registry. +// The hook form, useCarryOverFilters, lives in useFilters.ts. +export const buildCarryOverFilters = ( + filters: { field: string; value?: string }[], + fields: string[] +): Record => + filters.reduce>((acc, filter) => { + if (filter.value && fields.includes(filter.field)) { + acc[filter.field] = filter.value + } + return acc + }, {}) diff --git a/ui/src/utils/carryOverFilters.test.ts b/ui/src/utils/carryOverFilters.test.ts new file mode 100644 index 000000000..4844816af --- /dev/null +++ b/ui/src/utils/carryOverFilters.test.ts @@ -0,0 +1,48 @@ +import { FILTERS_TO_OCCURRENCES } from 'pages/occurrences/occurrence-filters' +import { FILTERS_TO_TAXA } from 'pages/species/species-filters' +import { buildCarryOverFilters } from 'utils/buildCarryOverFilters' + +describe('buildCarryOverFilters', () => { + it('carries only active filters whose field is in the destination set', () => { + const filters = [ + { field: 'deployment', value: '5' }, + { field: 'verified', value: 'false' }, + // active, but not part of the occurrence carry contract -> dropped + { field: 'include_unobserved', value: 'true' }, + // part of the set, but inactive -> dropped + { field: 'taxon', value: undefined }, + ] + + expect(buildCarryOverFilters(filters, FILTERS_TO_OCCURRENCES)).toEqual({ + deployment: '5', + verified: 'false', + }) + }) + + it('returns an empty object when no active filter is in the set', () => { + const filters = [{ field: 'page', value: '3' }] + + expect(buildCarryOverFilters(filters, FILTERS_TO_OCCURRENCES)).toEqual({}) + }) +}) + +describe('carry-over contracts', () => { + // Source-only state must never carry into a destination URL, regardless of destination. + const SOURCE_ONLY = ['page', 'ordering'] + + it.each([ + ['FILTERS_TO_OCCURRENCES', FILTERS_TO_OCCURRENCES], + ['FILTERS_TO_TAXA', FILTERS_TO_TAXA], + ])( + '%s carries no pagination or sort state and has no duplicates', + (_n, fields) => { + expect(fields.filter((f) => SOURCE_ONLY.includes(f))).toEqual([]) + expect(new Set(fields).size).toBe(fields.length) + } + ) + + it('keeps "show unobserved taxa" a taxa-only filter, never carried to occurrences', () => { + expect(FILTERS_TO_TAXA).toContain('include_unobserved') + expect(FILTERS_TO_OCCURRENCES).not.toContain('include_unobserved') + }) +}) diff --git a/ui/src/utils/getAppRoute.ts b/ui/src/utils/getAppRoute.ts index ca4012f94..03048396a 100644 --- a/ui/src/utils/getAppRoute.ts +++ b/ui/src/utils/getAppRoute.ts @@ -4,6 +4,8 @@ type FilterType = | 'collection' | 'collections' | 'deployment' + | 'deployment__device' + | 'deployment__research_site' | 'detections__source_image' | 'event' | 'include_unobserved' diff --git a/ui/src/utils/language.ts b/ui/src/utils/language.ts index c07f6038c..2bea4f53a 100644 --- a/ui/src/utils/language.ts +++ b/ui/src/utils/language.ts @@ -24,6 +24,7 @@ export enum STRING { LOGOUT, MANAGE_ACCESS, MORE, + MORE_FILTERS, NEXT, POPULATE, PREVIOUS, @@ -381,6 +382,7 @@ const ENGLISH_STRINGS: { [key in STRING]: string } = { [STRING.LOGOUT]: 'Logout', [STRING.MANAGE_ACCESS]: 'Manage access', [STRING.MORE]: 'More', + [STRING.MORE_FILTERS]: 'More filters', [STRING.NEXT]: 'Next', [STRING.POPULATE]: 'Populate', [STRING.PREVIOUS]: 'Previous', diff --git a/ui/src/utils/useFilters.ts b/ui/src/utils/useFilters.ts index b028a2592..8e9dd3336 100644 --- a/ui/src/utils/useFilters.ts +++ b/ui/src/utils/useFilters.ts @@ -1,5 +1,7 @@ import { isBefore, isValid } from 'date-fns' +import { useMemo } from 'react' import { useParams, useSearchParams } from 'react-router-dom' +import { buildCarryOverFilters } from './buildCarryOverFilters' import { APP_ROUTES } from './constants' import { STRING, translate } from './language' import { SEARCH_PARAM_KEY_PAGE } from './usePagination' @@ -76,6 +78,14 @@ export const AVAILABLE_FILTERS = (projectId: string): FilterConfig[] => [ }, }, }, + { + label: 'Device', + field: 'deployment__device', + }, + { + label: 'Site', + field: 'deployment__research_site', + }, { label: 'End date', field: 'date_end', @@ -275,3 +285,16 @@ export const useFilters = (defaultFilters?: { [field: string]: string }) => { filters, } } + +// Hook form of buildCarryOverFilters: pass the destination's carry contract (e.g. +// FILTERS_TO_OCCURRENCES). Reads the active filters of the current view, so any link into +// that destination — from any source view — carries a consistent set. +export const useCarryOverFilters = ( + fields: string[] +): Record => { + const { filters } = useFilters() + return useMemo( + () => buildCarryOverFilters(filters, fields), + [filters, fields] + ) +}