diff --git a/deployments/api/src/stitch/api/db/og_field_resource_actions.py b/deployments/api/src/stitch/api/db/og_field_resource_actions.py index 7b4dda3a..4ba25aff 100644 --- a/deployments/api/src/stitch/api/db/og_field_resource_actions.py +++ b/deployments/api/src/stitch/api/db/og_field_resource_actions.py @@ -16,6 +16,7 @@ from stitch.api.observability.context import named_query from stitch.api.entities import ( FILTER_OPTION_FIELDS, + OGFieldFilterParams, OGFieldQueryParams, ) from stitch.api.db.og_field_source_actions import ( @@ -93,16 +94,23 @@ async def query( async def filter_options( session: AsyncSession, + params: OGFieldFilterParams, licensed_sources: Collection[OGSISrcKey] | None = None, ) -> dict[str, list[str]]: - """Distinct coalesced values for every filterable field, in one query.""" + """Each filterable field's values still reachable under ``params``. + + Still one query: the per-field branches are unioned in SQL. Each field is + narrowed by every *other* filter but not by its own, so ticking one value + leaves that dropdown's remaining values on offer. Sorting happens here + rather than in SQL because SQLite and Postgres order strings differently. + """ options: dict[str, list[str]] = {field: [] for field in FILTER_OPTION_FIELDS} with named_query("resources.filter_options"): for colname, value in await session.execute( - filter_option_rows(licensed_sources) + filter_option_rows(params, licensed_sources) ): options[colname].append(value) - return options + return {field: sorted(values) for field, values in options.items()} async def get( diff --git a/deployments/api/src/stitch/api/db/queries.py b/deployments/api/src/stitch/api/db/queries.py index 02ba5d5c..e7e1c206 100644 --- a/deployments/api/src/stitch/api/db/queries.py +++ b/deployments/api/src/stitch/api/db/queries.py @@ -18,14 +18,17 @@ from sqlalchemy import ( CTE, ColumnElement, + CompoundSelect, Select, and_, asc, case, desc, func, + literal, or_, select, + union_all, ) from stitch.api.db.model import ( @@ -38,7 +41,11 @@ ResourceModel, ) from stitch.api.db.model.oil_gas_field_source_value import value_attr_for -from stitch.api.entities import FILTER_OPTION_FIELDS, OGFieldQueryParams +from stitch.api.entities import ( + FILTER_OPTION_FIELDS, + OGFieldFilterParams, + OGFieldQueryParams, +) from stitch.ogsi.model.types import OGSISrcKey # Single source of truth for the source-list field metadata. This is a shared @@ -66,7 +73,7 @@ _HEADER_SORT_FIELDS: Final[frozenset[str]] = frozenset({"id", "source", "resource_id"}) -def _filter_values(params: OGFieldQueryParams, field_name: str) -> list[Any]: +def _filter_values(params: OGFieldFilterParams, field_name: str) -> list[Any]: """The values an exact-match filter is set to, as a list. Multi-select fields arrive as lists; the rest are scalars, returned as a @@ -264,14 +271,39 @@ def coalesced_winner_rows( def filter_option_rows( + params: OGFieldFilterParams, licensed_sources: Collection[OGSISrcKey] | None = None, -) -> Select[tuple[str, str]]: - """Distinct winning ``(colname, value)`` pairs for every filterable field.""" +) -> CompoundSelect: + """Distinct ``(colname, value)`` pairs still reachable under ``params``. + + One branch per filterable field, unioned into a single statement so the + endpoint stays one round trip. Each branch applies every filter *except* the + branch field's own, so a field is never narrowed by its own selection: after + ticking one country the country list still offers the rest. Clearing the + field on a copy of ``params`` is what expresses that, which keeps ``q``, + multi-select, and the non-faceted exact filters working here exactly as they + do on the list query. + + Unordered on purpose: SQLite and Postgres disagree on string collation, so + the caller sorts in Python. + """ m = MembershipModel v = OilGasFieldSourceValueModel + r = ResourceModel p = OGFieldSourcePriority o = OGFieldResourceSourcePriority + # The faceted fields are always needed, since each one is a branch target. + # The rest are needed only when a filter or the search term actually reads + # them, so ranking and pivoting stay as narrow as the current params allow. + involved = list(FILTER_OPTION_FIELDS) + for field_name in EXACT_MATCH_FIELDS: + if _filter_values(params, field_name): + involved.append(field_name) + if params.q: + involved += Q_FIELDS + involved = list(dict.fromkeys(involved)) + base = ( select( m.resource_id.label("resource_id"), @@ -283,25 +315,45 @@ def filter_option_rows( v.value_text, ) .select_from(m) + .join(r, r.id == m.resource_id) .join(v, v.source_pk == m.source_pk) .join(p, p.source == m.source) .outerjoin(o, _override_join(m.resource_id)) .where( + # A merged-away resource matches no list row, so its values must not + # be offered as options either. + r.repointed_id.is_(None), m.status == MembershipStatus.ACTIVE, - v.colname.in_(FILTER_OPTION_FIELDS), + v.colname.in_(involved), ) ) if licensed_sources is not None: base = base.where(m.source.in_(list(dict.fromkeys(licensed_sources)))) ranked = add_ranking(base.cte("filter_option_base")).cte("filter_option_ranked") - c = ranked.c - return ( - select(c.colname, c.value_text) - .where(c.value_text.is_not(None)) # rn == 1 already applied by add_ranking - .distinct() - .order_by(c.colname, c.value_text) + + # Pivot the winning values to one row per resource: a branch listing one + # field's values has to filter on the *other* fields of the same resource. + pivot = _add_pivot_columns( + select(ranked.c.resource_id.label("resource_id")), + involved, + ranked.c.colname, + lambda field_name: getattr(ranked.c, value_attr_for(field_name)), ) + pivot_cte = pivot.group_by(ranked.c.resource_id).cte("filter_option_pivot") + + branches: list[Select[tuple[str, str]]] = [] + for field_name in FILTER_OPTION_FIELDS: + column = _require_column(pivot_cte, field_name) + branch = select( + literal(field_name).label("colname"), column.label("value") + ).where(column.is_not(None)) + for cond in _build_field_conditions( + pivot_cte, params.model_copy(update={field_name: None}) + ): + branch = branch.where(cond) + branches.append(branch.distinct()) + return union_all(*branches) def field_source_candidates( @@ -489,7 +541,7 @@ def _require_column(cte: CTE | Select, field_name: str) -> ColumnElement[Any]: def _build_field_conditions( cte: CTE | Select, - params: OGFieldQueryParams, + params: OGFieldFilterParams, ) -> list[ColumnElement[bool]]: """Path-agnostic q-ILIKE + exact-match filters over the pivoted columns. diff --git a/deployments/api/src/stitch/api/routers/oil_gas_fields.py b/deployments/api/src/stitch/api/routers/oil_gas_fields.py index 9e0c5c04..0f8d9c25 100644 --- a/deployments/api/src/stitch/api/routers/oil_gas_fields.py +++ b/deployments/api/src/stitch/api/routers/oil_gas_fields.py @@ -18,6 +18,7 @@ MergeCandidateDetailView, MergeCandidateReviewRequest, MergeCandidateView, + OGFieldFilterParams, OGFieldQueryParams, PaginatedResponse, SetFieldPriorityRequest, @@ -91,9 +92,11 @@ async def get_resource_filter_options( uow: UnitOfWorkDep, _user: CurrentUser, claims: Claims, + params: Annotated[OGFieldFilterParams, Query()], ) -> OGFieldFilterOptionsResponse: opts = await resource_actions.filter_options( session=uow.session, + params=params, licensed_sources=licensed_sources(claims), ) return OGFieldFilterOptionsResponse(**opts) diff --git a/deployments/api/tests/db/test_resource_actions.py b/deployments/api/tests/db/test_resource_actions.py index 01ea3dad..94149673 100644 --- a/deployments/api/tests/db/test_resource_actions.py +++ b/deployments/api/tests/db/test_resource_actions.py @@ -24,6 +24,7 @@ from stitch.api.db.queries import filter_option_rows from stitch.api.entities import ( FILTER_OPTION_FIELDS, + OGFieldFilterParams, OGFieldQueryParams, User, ) @@ -1016,7 +1017,9 @@ async def test_returns_distinct_sorted_coalesced_values( {"source": "rmi", "country": None}, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["CAN", "USA"] @@ -1040,6 +1043,7 @@ async def test_honors_licensed_sources_after_coalescing( options = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm", "llm"}), ) @@ -1063,12 +1067,14 @@ async def test_licensing_promotes_next_priority_value( with_wm = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm"}), ) assert with_wm["country"] == ["USA"] without_wm = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem"}), ) assert without_wm["country"] == ["CAN"] @@ -1097,7 +1103,9 @@ async def test_excludes_inactive_memberships( inactive_membership.status = MembershipStatus.INACTIVE await seeded_integration_session.flush() - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["USA"] @@ -1113,7 +1121,9 @@ async def test_returns_every_field_including_empty_ones( {"source": "rmi", "country": "USA"}, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert set(options.keys()) == set(FILTER_OPTION_FIELDS) assert options["country"] == ["USA"] @@ -1148,16 +1158,24 @@ async def test_returns_each_field_from_one_pass( }, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["CAN", "USA"] assert options["state_province"] == ["Alberta", "Texas"] assert options["basin"] == ["Permian", "Texas"] assert options["region"] == ["North America"] - def test_postgres_distinct_query_orders_by_selected_columns(self): - """``filter_option_rows`` compiles on Postgres.""" + def test_postgres_query_unions_one_distinct_branch_per_field(self): + """``filter_option_rows`` compiles on Postgres as one unioned statement. + + One ``DISTINCT`` branch per filterable field, unioned, so the endpoint + stays a single round trip. Sorting is deliberately absent: the two + dialects collate strings differently, so the action sorts in Python. + """ stmt = filter_option_rows( + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm", "rmi", "llm"}), ) @@ -1168,10 +1186,27 @@ def test_postgres_distinct_query_orders_by_selected_columns(self): ) ) - assert "SELECT DISTINCT" in sql - assert "ORDER BY" in sql - ordered = sql[sql.index("ORDER BY") :] - assert "colname" in ordered and "value_text" in ordered + assert sql.count("SELECT DISTINCT") >= len(FILTER_OPTION_FIELDS) + assert sql.count("UNION ALL") == len(FILTER_OPTION_FIELDS) - 1 + for field in FILTER_OPTION_FIELDS: + assert f"'{field}' AS colname" in sql + + def test_self_exclusion_omits_only_the_branch_fields_own_filter(self): + """A field's branch drops its own filter and keeps every other one.""" + stmt = filter_option_rows( + OGFieldFilterParams(country=["CAN"], state_province=["Alberta"]), + ) + + sql = str( + stmt.compile( + dialect=postgresql.dialect(), + compile_kwargs={"literal_binds": True}, + ) + ) + + # Applied in every branch but the field's own -> one fewer than six. + assert sql.count("IN ('CAN')") == len(FILTER_OPTION_FIELDS) - 1 + assert sql.count("IN ('Alberta')") == len(FILTER_OPTION_FIELDS) - 1 @pytest.mark.anyio async def test_only_unlicensed_selected_sources_still_return_resource( @@ -1340,6 +1375,234 @@ async def test_repointed_resources_are_excluded( assert [item.id for item in items] == [root_id] +class TestResourceFilterOptionsCascade: + """Options offered for one field reflect the *other* fields' selections. + + The bug this closes: with global option lists a user could pick Country = + Canada, then pick Alaska from a State/Province list that still offered every + state, and land on "No resources match". + """ + + async def _seed_two_countries( + self, + session: AsyncSession, + user: User, + ) -> None: + await _create_resource_with_sources( + session, + user, + { + "source": "gem", + "name": "Canadian field", + "country": "CAN", + "state_province": "Alberta", + "basin": "Western Canadian", + }, + ) + await _create_resource_with_sources( + session, + user, + { + "source": "gem", + "name": "American field", + "country": "USA", + "state_province": "Alaska", + "basin": "North Slope", + }, + ) + + @pytest.mark.anyio + async def test_country_narrows_state_province( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(country=["CAN"]) + ) + + assert options["state_province"] == ["Alberta"] + assert options["basin"] == ["Western Canadian"] + + @pytest.mark.anyio + async def test_state_province_narrows_country( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """The reverse direction: picking a province narrows Country.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(state_province=["Alberta"]), + ) + + assert options["country"] == ["CAN"] + + @pytest.mark.anyio + async def test_basin_spanning_two_countries_keeps_both( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Narrowing is by reachability, not by a one-to-one hierarchy.""" + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "CAN", "basin": "Williston"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "USA", "basin": "Williston"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "NOR", "basin": "North Sea"}, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(basin=["Williston"]) + ) + + assert options["country"] == ["CAN", "USA"] + + @pytest.mark.anyio + async def test_field_is_not_narrowed_by_its_own_selection( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Multi-select keeps working: Country still offers the un-ticked ones.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(country=["CAN"]) + ) + + assert options["country"] == ["CAN", "USA"] + + @pytest.mark.anyio + async def test_multi_select_unions_the_other_fields( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Two countries ticked -> the other fields offer both countries' values.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(country=["CAN", "USA"]), + ) + + assert options["state_province"] == ["Alaska", "Alberta"] + + @pytest.mark.anyio + async def test_search_term_cascades_into_every_field( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(q="Canadian") + ) + + assert options["country"] == ["CAN"] + assert options["state_province"] == ["Alberta"] + + @pytest.mark.anyio + async def test_search_term_is_not_self_excluded( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """``q`` is never a branch target, so it narrows even the fields it + matches on. ``basin`` is searchable, so a term matching only the Canadian + basin must still leave Basin showing that one value alone.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(q="Western Canadian") + ) + + assert options["basin"] == ["Western Canadian"] + + @pytest.mark.anyio + async def test_unfiltered_params_offer_every_value( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """No filters set -> the pre-cascade behaviour, every reachable value.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) + + assert options["country"] == ["CAN", "USA"] + assert options["state_province"] == ["Alaska", "Alberta"] + + @pytest.mark.anyio + async def test_excludes_repointed_resources( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """A merged-away resource's values are not offered. + + Behaviour change: the previous query never joined ``ResourceModel``, so a + repointed resource's values stayed in the dropdowns while matching no + list row. Sharing the list query's base removes them. + """ + root_id = await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "USA"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "CAN"}, + repointed_to=root_id, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) + + assert options["country"] == ["USA"] + + @pytest.mark.anyio + async def test_cascade_respects_licensed_sources( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Filtering on an unlicensed source's value offers nothing elsewhere.""" + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "wm", "country": "USA", "basin": "Permian"}, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(country=["USA"]), + licensed_sources=frozenset({"gem"}), + ) + + assert options["basin"] == [] + assert options["country"] == [] + + class TestResourcePriorityOverride: """A per-field override re-ranks sources, flipping the coalesced winner.""" diff --git a/deployments/api/tests/observability/test_query_name_actions.py b/deployments/api/tests/observability/test_query_name_actions.py index 619d2af3..b2c542cc 100644 --- a/deployments/api/tests/observability/test_query_name_actions.py +++ b/deployments/api/tests/observability/test_query_name_actions.py @@ -28,6 +28,7 @@ from stitch.api.entities import ( MergeCandidateCreateRequest, MergeCandidateReviewRequest, + OGFieldFilterParams, OGFieldQueryParams, User, ) @@ -172,7 +173,7 @@ async def test_resource_read_labels( ) captured_query_events.clear() - await resource_actions.filter_options(session) + await resource_actions.filter_options(session, OGFieldFilterParams()) _assert_labels(captured_query_events, {"resources.filter_options"}) captured_query_events.clear() diff --git a/deployments/api/tests/routers/test_resources_unit.py b/deployments/api/tests/routers/test_resources_unit.py index 5585acdc..5efc4941 100644 --- a/deployments/api/tests/routers/test_resources_unit.py +++ b/deployments/api/tests/routers/test_resources_unit.py @@ -324,3 +324,63 @@ async def override_get_uow(): assert call_kwargs["licensed_sources"] == frozenset( {"rmi", "gem", "wm", "ccr", "alb", "bc", "nor", "llm"} ) + + @pytest.mark.anyio + async def test_passes_filter_params(self, async_client, mock_uow): + """The current selections reach the action, so options can cascade.""" + + async def override_get_uow(): + yield mock_uow + + app.dependency_overrides[get_uow] = override_get_uow + + with patch("stitch.api.routers.oil_gas_fields.resource_actions") as mock_repo: + mock_repo.filter_options = AsyncMock( + return_value=dict( + basin=[], + country=[], + field_status=[], + primary_hydrocarbon_group=[], + region=[], + state_province=[], + ) + ) + + response = await async_client.get( + "/oil-gas-fields/filter-options" + "?country=NOR&country=SAU&state_province=Alberta&q=ghawar" + ) + + assert response.status_code == 200 + params = mock_repo.filter_options.call_args.kwargs["params"] + assert params.country == ["NOR", "SAU"] + assert params.state_province == ["Alberta"] + assert params.q == "ghawar" + + @pytest.mark.anyio + async def test_no_query_string_still_works(self, async_client, mock_uow): + """Every param is optional: callers that send none are unaffected.""" + + async def override_get_uow(): + yield mock_uow + + app.dependency_overrides[get_uow] = override_get_uow + + with patch("stitch.api.routers.oil_gas_fields.resource_actions") as mock_repo: + mock_repo.filter_options = AsyncMock( + return_value=dict( + basin=[], + country=[], + field_status=[], + primary_hydrocarbon_group=[], + region=[], + state_province=[], + ) + ) + + response = await async_client.get("/oil-gas-fields/filter-options") + + assert response.status_code == 200 + params = mock_repo.filter_options.call_args.kwargs["params"] + assert params.country is None + assert params.q is None diff --git a/deployments/stitch-frontend/src/components/FilterBar.jsx b/deployments/stitch-frontend/src/components/FilterBar.jsx index bad32b52..7a82d7d8 100644 --- a/deployments/stitch-frontend/src/components/FilterBar.jsx +++ b/deployments/stitch-frontend/src/components/FilterBar.jsx @@ -2,8 +2,20 @@ import FilterDropdown from "./FilterDropdown"; import { FILTER_FIELDS, EMPTY_FILTERS } from "../config/filters"; import { useResourceFilterOptions } from "../hooks/useResources"; -export default function FilterBar({ endpoint, filters, onFiltersChange }) { - const { data: filterOptions } = useResourceFilterOptions(endpoint); +export default function FilterBar({ + endpoint, + filters, + onFiltersChange, + optionParams, +}) { + // `filters` drives the ticked boxes and updates on every click. The options + // request runs on `optionParams`, which the caller debounces alongside the + // list request, so a settled change costs one options request, not one per + // click. + const { data: filterOptions } = useResourceFilterOptions( + endpoint, + optionParams, + ); // Flatten active filters into chips: [{ field, label, value, displayValue }, ...] // `value` is the stored/API value; `displayValue` is what the user sees. diff --git a/deployments/stitch-frontend/src/components/ResourcesView.jsx b/deployments/stitch-frontend/src/components/ResourcesView.jsx index 023e3bad..0b2db1cf 100644 --- a/deployments/stitch-frontend/src/components/ResourcesView.jsx +++ b/deployments/stitch-frontend/src/components/ResourcesView.jsx @@ -191,6 +191,10 @@ export default function ResourcesView({ className = "", endpoint }) { endpoint={endpoint} filters={filters} onFiltersChange={handleFiltersChange} + optionParams={{ + filters: settledListParams.filters, + q: settledListParams.q, + }} /> diff --git a/deployments/stitch-frontend/src/components/ResourcesView.test.jsx b/deployments/stitch-frontend/src/components/ResourcesView.test.jsx index 142c2524..489c20a4 100644 --- a/deployments/stitch-frontend/src/components/ResourcesView.test.jsx +++ b/deployments/stitch-frontend/src/components/ResourcesView.test.jsx @@ -555,7 +555,27 @@ describe("ResourcesView", () => { renderWithQueryClient(); expect(useResourceFilterOptions).toHaveBeenCalledTimes(1); - expect(useResourceFilterOptions).toHaveBeenCalledWith(ENDPOINT); + expect(useResourceFilterOptions).toHaveBeenCalledWith( + ENDPOINT, + expect.objectContaining({ filters: expect.any(Object) }), + ); + }); + + it("sends the active filters and search to the options query", () => { + vi.mocked(useResources).mockReturnValue({ + ...defaultHookReturn, + data: mockResourceData, + }); + + renderWithQueryClient(, { + initialEntries: ["/?country=NOR&q=ghawar"], + }); + + expect(useResourceFilterOptions).toHaveBeenCalledTimes(1); + expect(useResourceFilterOptions).toHaveBeenLastCalledWith(ENDPOINT, { + filters: expect.objectContaining({ country: ["NOR"] }), + q: "ghawar", + }); }); it("shows country options as conventional names but filters by the code", () => { diff --git a/deployments/stitch-frontend/src/hooks/useResources.js b/deployments/stitch-frontend/src/hooks/useResources.js index 14009924..dc5c572d 100644 --- a/deployments/stitch-frontend/src/hooks/useResources.js +++ b/deployments/stitch-frontend/src/hooks/useResources.js @@ -55,10 +55,13 @@ function useResourcesReal( }); } -function useResourceFilterOptionsReal(endpoint = "resources", enabled = true) { +function useResourceFilterOptionsReal( + endpoint = "resources", + { filters = {}, q, enabled = true } = {}, +) { const config = useConfig(); return useAuthenticatedQuery({ - ...resourceQueries.filterOptions(config, endpoint), + ...resourceQueries.filterOptions(config, endpoint, filters, q), enabled, }); } @@ -214,15 +217,19 @@ function getMockResourcePage({ }; } -function getMockFilterOptions() { +function getMockFilterOptions({ filters = {}, q } = {}) { + const searched = applyMockSearch(MOCK_RESOURCE_ITEMS, q); + return Object.fromEntries( FILTER_FIELDS.map(({ key: field }) => [ field, Array.from( new Set( - MOCK_RESOURCE_ITEMS.map((resource) => - getResourceField(resource, field), - ) + // Leave-one-out, matching the backend: a field's options are the + // values reachable under every *other* filter, so ticking one value + // never hides that field's remaining values. + applyMockFilters(searched, { ...filters, [field]: [] }) + .map((resource) => getResourceField(resource, field)) .filter((value) => value != null && value !== "") .map(String), ), @@ -269,10 +276,13 @@ function useResourcesMock( }); } -function useResourceFilterOptionsMock(endpoint = "resources", enabled = true) { +function useResourceFilterOptionsMock( + endpoint = "resources", + { filters = {}, q, enabled = true } = {}, +) { return useQuery({ - ...resourceQueries.filterOptions(UNUSED_MOCK_CONFIG, endpoint), - queryFn: () => Promise.resolve(getMockFilterOptions()), + ...resourceQueries.filterOptions(UNUSED_MOCK_CONFIG, endpoint, filters, q), + queryFn: () => Promise.resolve(getMockFilterOptions({ filters, q })), enabled, }); } diff --git a/deployments/stitch-frontend/src/queries/api.js b/deployments/stitch-frontend/src/queries/api.js index fa1cfdb9..f45d4521 100644 --- a/deployments/stitch-frontend/src/queries/api.js +++ b/deployments/stitch-frontend/src/queries/api.js @@ -1,16 +1,27 @@ -export async function getResources( - config, - fetcher, - endpoint = "resources", - { page = 1, page_size = 50, filters = {}, q, sort_by, sort_order } = {}, -) { - const params = new URLSearchParams({ page, page_size }); +// The list query and the filter-options query must serialize the current +// selections identically: the options a dropdown offers are the values still +// reachable under exactly the filters the list is about to apply. +function appendFilterParams(params, filters = {}, q) { for (const [key, values] of Object.entries(filters)) { for (const v of values) { params.append(key, v); } } if (q) params.set("q", q); + return params; +} + +export async function getResources( + config, + fetcher, + endpoint = "resources", + { page = 1, page_size = 50, filters = {}, q, sort_by, sort_order } = {}, +) { + const params = appendFilterParams( + new URLSearchParams({ page, page_size }), + filters, + q, + ); if (sort_by) params.set("sort_by", sort_by); if (sort_order) params.set("sort_order", sort_order); const url = `${config.apiBaseUrl}/${endpoint}/?${params}`; @@ -27,8 +38,16 @@ export async function getResourceFilterOptions( config, fetcher, endpoint = "resources", + { filters = {}, q } = {}, ) { - const url = `${config.apiBaseUrl}/${endpoint}/filter-options`; + const query = appendFilterParams( + new URLSearchParams(), + filters, + q, + ).toString(); + const url = `${config.apiBaseUrl}/${endpoint}/filter-options${ + query ? `?${query}` : "" + }`; const response = await fetcher(url); if (!response.ok) { const error = new Error(`HTTP error! status: ${response.status}`); diff --git a/deployments/stitch-frontend/src/queries/api.test.js b/deployments/stitch-frontend/src/queries/api.test.js index 5722bb08..b8a127e4 100644 --- a/deployments/stitch-frontend/src/queries/api.test.js +++ b/deployments/stitch-frontend/src/queries/api.test.js @@ -156,6 +156,24 @@ describe("API Functions", () => { expect(result).toEqual(mockOptions); }); + it("sends the current filters and search so the options cascade", async () => { + mockFetcher.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({}), + }); + + await getResourceFilterOptions(config, mockFetcher, "oil-gas-fields", { + filters: { country: ["NOR", "SAU"], basin: [] }, + q: "ghawar", + }); + + expect(mockFetcher).toHaveBeenCalledWith( + "http://localhost:8000/api/v1/oil-gas-fields/filter-options" + + "?country=NOR&country=SAU&q=ghawar", + ); + }); + it("throws error when filter options response is not ok", async () => { mockFetcher.mockResolvedValueOnce({ ok: false, diff --git a/deployments/stitch-frontend/src/queries/resources.js b/deployments/stitch-frontend/src/queries/resources.js index 4e8332eb..3559c9d3 100644 --- a/deployments/stitch-frontend/src/queries/resources.js +++ b/deployments/stitch-frontend/src/queries/resources.js @@ -28,9 +28,10 @@ const keys = { all: (endpoint = "resources") => [endpoint], lists: (endpoint = "resources") => [...keys.all(endpoint), "list"], list: (endpoint = "resources", filters) => [...keys.lists(endpoint), filters], - filterOptions: (endpoint = "resources") => [ + filterOptions: (endpoint = "resources", params) => [ ...keys.all(endpoint), "filter-options", + params, ], details: (endpoint = "resources") => [...keys.all(endpoint), "detail"], detail: (endpoint = "resources", id) => [...keys.details(endpoint), id], @@ -92,10 +93,13 @@ export const resourceQueries = { placeholderData: keepPreviousData, }), - filterOptions: (config, endpoint = "resources") => + // The options are per-selection, so the selections belong in the key, in + // the same shape the list key uses. + filterOptions: (config, endpoint = "resources", filters = {}, q) => queryOptions({ - queryKey: keys.filterOptions(endpoint), - queryFn: (fetcher) => getResourceFilterOptions(config, fetcher, endpoint), + queryKey: keys.filterOptions(endpoint, { ...filters, q }), + queryFn: (fetcher) => + getResourceFilterOptions(config, fetcher, endpoint, { filters, q }), staleTime: DEFAULT_STALE_TIME, }),