diff --git a/deployments/api/API_REFERENCE.md b/deployments/api/API_REFERENCE.md index ee975323..fb1b8683 100644 --- a/deployments/api/API_REFERENCE.md +++ b/deployments/api/API_REFERENCE.md @@ -92,7 +92,12 @@ **Response:** `200` -- array[MergeCandidateView] +- `items`: array[MergeCandidateListItemView] +- `total_count`: integer +- `page`: integer +- `page_size`: integer +- `status_counts`: object[string, integer] +- `total_pages`: integer --- @@ -137,6 +142,7 @@ - `last_updated_by_id`: integer - `reviewed_at`: string | null - `reviewed_by_id`: integer | null +- `name`: string | null - `compare`: array[FieldComparisonView] --- diff --git a/deployments/api/src/stitch/api/db/merge_candidate_actions.py b/deployments/api/src/stitch/api/db/merge_candidate_actions.py index b93a712c..7e4337e8 100644 --- a/deployments/api/src/stitch/api/db/merge_candidate_actions.py +++ b/deployments/api/src/stitch/api/db/merge_candidate_actions.py @@ -1,10 +1,10 @@ from __future__ import annotations -from collections.abc import Collection, Sequence +from collections.abc import Collection, Mapping, Sequence from datetime import datetime, timezone from typing import Any -from sqlalchemy import select +from sqlalchemy import func, select from sqlalchemy.ext.asyncio import AsyncSession from sqlalchemy.orm import selectinload @@ -19,12 +19,15 @@ FieldComparisonView, MergeCandidateCreateRequest, MergeCandidateDetailView, + MergeCandidateListItemView, + MergeCandidatePage, + MergeCandidateQueryParams, MergeCandidateReviewRequest, MergeCandidateStatus, MergeCandidateView, ) from stitch.api.observability.context import named_query -from stitch.ogsi.model import OGFieldSource +from stitch.ogsi.model import SOURCE_PRIORITY, OGFieldResource, OGFieldSource from stitch.ogsi.model.og_field import OilGasFieldBase from stitch.ogsi.model.types import OGSISrcKey @@ -35,7 +38,7 @@ ResourceModel, ) from .og_field_resource_actions import apply_resource_merge -from .utils import coalesce_resources_with_sources +from .utils import coalesce_resources, coalesce_resources_with_sources def _normalize_resource_ids(resource_ids: Sequence[int]) -> list[int]: @@ -172,14 +175,72 @@ async def _default_source_priority(session: AsyncSession) -> dict[str, int]: return {source: priority for source, priority in rows.all()} +def _ordered_resource_ids(model: MergeCandidateModel) -> list[int]: + return [item.resource_id for item in sorted(model.items, key=lambda i: i.position)] + + +def _source_rank(source: OGSISrcKey) -> int: + return ( + SOURCE_PRIORITY.index(source) + if source in SOURCE_PRIORITY + else len(SOURCE_PRIORITY) + ) + + +def _candidate_name( + model: MergeCandidateModel, coalesced: Mapping[int, OGFieldResource] +) -> str | None: + """Display name for a candidate; see ``MergeCandidateListItemView.name``. + + ``coalesced`` must hold the candidate's resources and, once merged, its + merged resource. Source resources are ranked by the global + ``SOURCE_PRIORITY`` (the same rule the frontend used to apply), with ties + going to the earlier resource. + """ + if model.merged_resource_id is not None: + merged = coalesced[model.merged_resource_id].view + if merged is not None and merged.name is not None: + return merged.name + + best: tuple[int, str] | None = None + for resource_id in _ordered_resource_ids(model): + provenance = coalesced[resource_id].provenance.get("name") + if provenance is None: + continue + name, source, _source_pk = provenance + rank = _source_rank(source) + if best is None or rank < best[0]: + best = (rank, name) + return None if best is None else best[1] + + +def _name_resource_ids(models: Sequence[MergeCandidateModel]) -> list[int]: + """Every resource ``_candidate_name`` reads for these candidates.""" + ids: list[int] = [] + for model in models: + ids.extend(_ordered_resource_ids(model)) + if model.merged_resource_id is not None: + ids.append(model.merged_resource_id) + return list(dict.fromkeys(ids)) + + +def _candidate_to_list_item_view( + model: MergeCandidateModel, name: str | None +) -> MergeCandidateListItemView: + return MergeCandidateListItemView( + **_candidate_to_view(model).model_dump(), name=name + ) + + def _candidate_to_detail_view( model: MergeCandidateModel, + name: str | None, compare: Sequence[FieldComparisonView], ) -> MergeCandidateDetailView: - # MergeCandidateDetailView is MergeCandidateView + `compare`; reuse the base + # MergeCandidateDetailView is the list item + `compare`; reuse the base # mapping so the shared fields stay defined in one place. return MergeCandidateDetailView( - **_candidate_to_view(model).model_dump(), + **_candidate_to_list_item_view(model, name).model_dump(), compare=list(compare), ) @@ -200,15 +261,64 @@ async def _load_candidate_model( return model -async def list_merge_candidates(session: AsyncSession) -> list[MergeCandidateView]: +async def list_merge_candidates( + session: AsyncSession, + params: MergeCandidateQueryParams, + licensed_sources: Collection[OGSISrcKey] | None = None, +) -> MergeCandidatePage: + """One page of merge candidates, with whole-queue counts by status.""" + filtered = select(MergeCandidateModel.id) + if params.status: + filtered = filtered.where(MergeCandidateModel.status.in_(params.status)) + + with named_query("merge_candidates.list.count"): + total = ( + await session.scalar(select(func.count()).select_from(filtered.subquery())) + ) or 0 + + sort_column = getattr(MergeCandidateModel, params.sort_by) + if params.sort_order == "asc": + order_by = (sort_column.asc().nulls_last(), MergeCandidateModel.id.asc()) + else: + order_by = (sort_column.desc().nulls_last(), MergeCandidateModel.id.desc()) stmt = ( select(MergeCandidateModel) .options(selectinload(MergeCandidateModel.items)) - .order_by(MergeCandidateModel.created.desc()) + .where(MergeCandidateModel.id.in_(filtered)) + .order_by(*order_by) + .limit(params.limit) + .offset(params.offset) ) with named_query("merge_candidates.list"): candidates = (await session.scalars(stmt)).all() - return [_candidate_to_view(candidate) for candidate in candidates] + + with named_query("merge_candidates.list.status_counts"): + rows = await session.execute( + select(MergeCandidateModel.status, func.count()).group_by( + MergeCandidateModel.status + ) + ) + status_counts = dict.fromkeys(MergeCandidateStatus, 0) + for status, count in rows.all(): + status_counts[MergeCandidateStatus(status)] = count + + with named_query("merge_candidates.list.names"): + coalesced = await coalesce_resources( + session, _name_resource_ids(candidates), licensed_sources + ) + + return MergeCandidatePage( + items=[ + _candidate_to_list_item_view( + candidate, _candidate_name(candidate, coalesced) + ) + for candidate in candidates + ], + total_count=total, + page=params.page, + page_size=params.page_size, + status_counts=status_counts, + ) async def get_merge_candidate( @@ -219,9 +329,7 @@ async def get_merge_candidate( with named_query("merge_candidates.detail.load"): candidate = await _load_candidate_model(session, candidate_id) - resource_ids = [ - item.resource_id for item in sorted(candidate.items, key=lambda i: i.position) - ] + resource_ids = _ordered_resource_ids(candidate) # Computed live: repointed (post-merge) resources coalesce to a null-shell # here, so an APPROVED candidate's `compare` reflects the emptied originals. @@ -251,7 +359,18 @@ async def get_merge_candidate( resource_views = [by_id[rid].view for rid in resource_ids] compare = _build_comparison(resource_views, sources_with_priority) - return _candidate_to_detail_view(candidate, compare) + name_sources: dict[int, OGFieldResource] = dict(by_id) + if candidate.merged_resource_id is not None: + with named_query("merge_candidates.detail.merged_name"): + name_sources.update( + await coalesce_resources( + session, [candidate.merged_resource_id], licensed_sources + ) + ) + + return _candidate_to_detail_view( + candidate, _candidate_name(candidate, name_sources), compare + ) async def create_merge_candidate( diff --git a/deployments/api/src/stitch/api/entities.py b/deployments/api/src/stitch/api/entities.py index 24fe7f42..26e4e459 100644 --- a/deployments/api/src/stitch/api/entities.py +++ b/deployments/api/src/stitch/api/entities.py @@ -184,6 +184,49 @@ class MergeCandidateView(BaseModel): reviewed_by_id: int | None = None +class MergeCandidateListItemView(MergeCandidateView): + """A merge candidate as read back by the list and detail endpoints. + + ``name`` is the candidate's display name: the merged resource's name once + the candidate is approved (its source resources are emptied by the merge), + otherwise the name from the best-ranked source across its resources. It is + null when no licensed source carries a name. + """ + + name: str | None = None + + +class MergeCandidateFilterParams(BaseModel): + """Repeat ``status`` once per value (``?status=PENDING&status=DENIED``) to + match any of them. Omitted, every status is returned.""" + + status: list[MergeCandidateStatus] | None = None + + +class MergeCandidateSortParams(BaseModel): + # `id` is always appended in the same direction as a tiebreak, so pages + # stay stable when candidates share a timestamp (bulk creation). + sort_by: Literal["created", "reviewed_at"] = "created" + sort_order: Literal["asc", "desc"] = "desc" + + +class MergeCandidateQueryParams( + PaginationParams, MergeCandidateFilterParams, MergeCandidateSortParams +): + pass + + +class MergeCandidatePage(PaginatedResponse[MergeCandidateListItemView]): + """A page of merge candidates. + + ``status_counts`` covers the whole queue regardless of the ``status`` + filter, so a reviewer always sees the full workload. Every status is + present, zero when none exist. + """ + + status_counts: dict[MergeCandidateStatus, int] + + class ComparisonValueView(OGFieldSourceValueView): """A source's value in a merge comparison, tagged with ``resource_id`` -- the candidate resource the source is currently attached to. @@ -228,5 +271,5 @@ class FieldComparisonView(BaseModel): values: list[ComparisonValueView] -class MergeCandidateDetailView(MergeCandidateView): +class MergeCandidateDetailView(MergeCandidateListItemView): compare: list[FieldComparisonView] 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..08a13008 100644 --- a/deployments/api/src/stitch/api/routers/oil_gas_fields.py +++ b/deployments/api/src/stitch/api/routers/oil_gas_fields.py @@ -16,6 +16,8 @@ OGFieldFilterOptionsResponse, MergeCandidateCreateRequest, MergeCandidateDetailView, + MergeCandidatePage, + MergeCandidateQueryParams, MergeCandidateReviewRequest, MergeCandidateView, OGFieldQueryParams, @@ -101,13 +103,21 @@ async def get_resource_filter_options( @router.get( "/merge-candidates", - response_model=list[MergeCandidateView], + response_model=MergeCandidatePage, dependencies=[Depends(require_permissions(MERGE_CANDIDATE_READ))], ) async def list_merge_candidates( - *, uow: UnitOfWorkDep, _user: CurrentUser -) -> list[MergeCandidateView]: - return await merge_candidate_actions.list_merge_candidates(session=uow.session) + *, + uow: UnitOfWorkDep, + _user: CurrentUser, + claims: Claims, + params: Annotated[MergeCandidateQueryParams, Query()], +) -> MergeCandidatePage: + return await merge_candidate_actions.list_merge_candidates( + session=uow.session, + params=params, + licensed_sources=licensed_sources(claims), + ) @router.get( diff --git a/deployments/api/tests/observability/test_query_name_actions.py b/deployments/api/tests/observability/test_query_name_actions.py index 619d2af3..450ac947 100644 --- a/deployments/api/tests/observability/test_query_name_actions.py +++ b/deployments/api/tests/observability/test_query_name_actions.py @@ -27,6 +27,7 @@ from stitch.api.db.model.oil_gas_field_source_value import ATTRIBUTE_NAMES from stitch.api.entities import ( MergeCandidateCreateRequest, + MergeCandidateQueryParams, MergeCandidateReviewRequest, OGFieldQueryParams, User, @@ -369,8 +370,16 @@ async def test_merge_candidate_action_labels( ) captured_query_events.clear() - await mca.list_merge_candidates(session) - _assert_labels(captured_query_events, {"merge_candidates.list"}) + await mca.list_merge_candidates(session, MergeCandidateQueryParams()) + _assert_labels( + captured_query_events, + { + "merge_candidates.list.count", + "merge_candidates.list", + "merge_candidates.list.status_counts", + "merge_candidates.list.names", + }, + ) captured_query_events.clear() await mca.get_merge_candidate(session, approved.id) diff --git a/deployments/api/tests/routers/test_merge_candidates_integration.py b/deployments/api/tests/routers/test_merge_candidates_integration.py index b1d9ace7..f4fbf991 100644 --- a/deployments/api/tests/routers/test_merge_candidates_integration.py +++ b/deployments/api/tests/routers/test_merge_candidates_integration.py @@ -1,14 +1,19 @@ -"""Integration tests for the merge-candidate detail endpoint (real SQLite).""" +"""Integration tests for the merge-candidate list and detail endpoints (real SQLite).""" from collections.abc import Sequence +from datetime import datetime, timezone from httpx import AsyncClient import pytest -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from tests.factories import ResourceCreateFactory -from stitch.api.db.model import MembershipModel, OGFieldResourceSourcePriority +from stitch.api.db.model import ( + MembershipModel, + MergeCandidateModel, + OGFieldResourceSourcePriority, +) from stitch.ogsi.model import OGFieldResource, OGFieldSource @@ -534,3 +539,162 @@ async def test_composite_resource_matches_when_winners_agree( (id_a, "gem", "Bar"), (id_b, "rmi", "Foo"), } + + +async def _create_disjoint_candidates( + client: AsyncClient, fact: ResourceCreateFactory, count: int +) -> list[int]: + """``count`` two-resource candidates sharing no resources, in creation order.""" + candidate_ids = [] + for i in range(count): + id_a = await _create_resource(client, fact, f"Field {i}") + id_b = await _create_resource(client, fact, f"Field {i} dup") + candidate_ids.append(await _create_candidate(client, [id_a, id_b])) + return candidate_ids + + +class TestMergeCandidateListIntegration: + @pytest.mark.anyio + async def test_pages_are_stable_when_candidates_share_a_timestamp( + self, + integration_client: AsyncClient, + og_create_res_fact: ResourceCreateFactory, + integration_session_factory: async_sessionmaker[AsyncSession], + ): + candidate_ids = await _create_disjoint_candidates( + integration_client, og_create_res_fact, 5 + ) + # Bulk creation can stamp many candidates with the same time; the id + # tiebreak must still give every candidate exactly one page. + async with integration_session_factory() as session: + await session.execute( + update(MergeCandidateModel).values( + created=datetime(2026, 1, 1, tzinfo=timezone.utc) + ) + ) + await session.commit() + + seen = [] + for page in (1, 2, 3): + resp = await integration_client.get( + "/oil-gas-fields/merge-candidates", + params={"page": page, "page_size": 2}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + assert body["total_count"] == 5 + assert body["total_pages"] == 3 + seen.extend(item["id"] for item in body["items"]) + + assert seen == sorted(candidate_ids, reverse=True) + + @pytest.mark.anyio + async def test_status_filter_narrows_items_but_not_counts( + self, + integration_client: AsyncClient, + og_create_res_fact: ResourceCreateFactory, + ): + approved_id, denied_id, pending_id = await _create_disjoint_candidates( + integration_client, og_create_res_fact, 3 + ) + for candidate_id, action in ((approved_id, "approve"), (denied_id, "deny")): + resp = await integration_client.post( + f"/oil-gas-fields/merge-candidates/{candidate_id}/{action}" + ) + assert resp.status_code == 200, resp.text + + pending = await integration_client.get( + "/oil-gas-fields/merge-candidates", params={"status": "PENDING"} + ) + assert pending.status_code == 200, pending.text + assert [item["id"] for item in pending.json()["items"]] == [pending_id] + assert pending.json()["total_count"] == 1 + assert pending.json()["status_counts"] == { + "PENDING": 1, + "APPROVED": 1, + "DENIED": 1, + } + + reviewed = await integration_client.get( + "/oil-gas-fields/merge-candidates", + params=[("status", "APPROVED"), ("status", "DENIED")], + ) + assert {item["id"] for item in reviewed.json()["items"]} == { + approved_id, + denied_id, + } + + unfiltered = await integration_client.get("/oil-gas-fields/merge-candidates") + assert unfiltered.json()["total_count"] == 3 + + @pytest.mark.anyio + @pytest.mark.parametrize("sort_order", ["asc", "desc"]) + async def test_sort_by_reviewed_at_puts_unreviewed_last( + self, + integration_client: AsyncClient, + og_create_res_fact: ResourceCreateFactory, + sort_order: str, + ): + first, second, unreviewed = await _create_disjoint_candidates( + integration_client, og_create_res_fact, 3 + ) + for candidate_id in (first, second): + resp = await integration_client.post( + f"/oil-gas-fields/merge-candidates/{candidate_id}/deny" + ) + assert resp.status_code == 200, resp.text + + resp = await integration_client.get( + "/oil-gas-fields/merge-candidates", + params={"sort_by": "reviewed_at", "sort_order": sort_order}, + ) + assert resp.status_code == 200, resp.text + reviewed = [first, second] if sort_order == "asc" else [second, first] + assert [item["id"] for item in resp.json()["items"]] == [ + *reviewed, + unreviewed, + ] + + @pytest.mark.anyio + async def test_items_carry_display_names( + self, + integration_client: AsyncClient, + og_create_res_fact: ResourceCreateFactory, + ): + id_a = await _create_resource(integration_client, og_create_res_fact, "Ghawar") + id_b = await _create_resource(integration_client, og_create_res_fact, "Burgan") + candidate_id = await _create_candidate(integration_client, [id_a, id_b]) + + resp = await integration_client.get("/oil-gas-fields/merge-candidates") + assert resp.status_code == 200, resp.text + (item,) = resp.json()["items"] + # Same source on both resources: the tie goes to the first resource. + assert item["name"] == "Ghawar" + + # After approval the originals are emptied; the merged resource names it. + approve = await integration_client.post( + f"/oil-gas-fields/merge-candidates/{candidate_id}/approve" + ) + assert approve.status_code == 200, approve.text + resp = await integration_client.get("/oil-gas-fields/merge-candidates") + (item,) = resp.json()["items"] + assert item["name"] == "Ghawar" + + @pytest.mark.anyio + @pytest.mark.parametrize( + "params", + [ + {"status": "MAYBE"}, + {"sort_by": "name"}, + {"sort_order": "sideways"}, + {"page": 0}, + {"page_size": 201}, + ], + ) + async def test_rejects_invalid_params( + self, integration_client: AsyncClient, params: dict + ): + resp = await integration_client.get( + "/oil-gas-fields/merge-candidates", params=params + ) + assert resp.status_code == 422, resp.text diff --git a/deployments/api/tests/routers/test_route_permissions.py b/deployments/api/tests/routers/test_route_permissions.py index ebaa09a4..4c65225d 100644 --- a/deployments/api/tests/routers/test_route_permissions.py +++ b/deployments/api/tests/routers/test_route_permissions.py @@ -17,6 +17,7 @@ from stitch.api.auth import get_token_claims from stitch.api.db.config import get_uow +from stitch.api.entities import MergeCandidatePage, MergeCandidateStatus from stitch.api.main import app @@ -144,8 +145,16 @@ async def test_merge_candidate_read_permission_allows_list_route( app.dependency_overrides[get_uow] = _uow_override(mock_uow) with patch("stitch.api.routers.oil_gas_fields.merge_candidate_actions") as actions: - actions.list_merge_candidates = AsyncMock(return_value=[]) + actions.list_merge_candidates = AsyncMock( + return_value=MergeCandidatePage( + items=[], + total_count=0, + page=1, + page_size=50, + status_counts=dict.fromkeys(MergeCandidateStatus, 0), + ) + ) response = await async_client.get("/oil-gas-fields/merge-candidates") assert response.status_code == 200 - assert response.json() == [] + assert response.json()["items"] == [] diff --git a/deployments/api/tests/test_merge_candidate_actions.py b/deployments/api/tests/test_merge_candidate_actions.py index b94c9a21..333ef8c4 100644 --- a/deployments/api/tests/test_merge_candidate_actions.py +++ b/deployments/api/tests/test_merge_candidate_actions.py @@ -9,6 +9,7 @@ from stitch.api.entities import ( MergeCandidateCreateRequest, MergeCandidateDetailView, + MergeCandidateQueryParams, MergeCandidateReviewRequest, MergeCandidateStatus, ) @@ -52,6 +53,7 @@ class FakeMergedResource: class FakeSession: scalar_result: object | None = None scalars_result: list[object] = field(default_factory=list) + execute_rows: list[tuple] = field(default_factory=list) added: list[object] = field(default_factory=list) added_all: list[object] = field(default_factory=list) deleted: list[object] = field(default_factory=list) @@ -64,6 +66,9 @@ async def scalar(self, _stmt): async def scalars(self, _stmt): return SimpleNamespace(all=lambda: list(self.scalars_result)) + async def execute(self, _stmt): + return SimpleNamespace(all=lambda: list(self.execute_rows)) + def add(self, obj): self.added.append(obj) @@ -116,8 +121,18 @@ def test_candidate_to_view_sorts_items_by_position(): assert view.id == 1 +def _named(name: str | None, source: str = "rmi", source_pk: int = 1): + """A coalesced resource whose only known field is ``name``.""" + return SimpleNamespace( + view=OilGasFieldBase(name=name, country=None), + provenance={"name": None if name is None else (name, source, source_pk)}, + ) + + @pytest.mark.anyio -async def test_list_merge_candidates_returns_views_in_query_order(): +async def test_list_merge_candidates_returns_page_with_names_and_counts( + monkeypatch, +): first = FakeCandidate( id=2, status=MergeCandidateStatus.PENDING, @@ -125,16 +140,69 @@ async def test_list_merge_candidates_returns_views_in_query_order(): ) second = FakeCandidate( id=1, - status=MergeCandidateStatus.DENIED, + status=MergeCandidateStatus.APPROVED, items=[FakeItem(20, 0), FakeItem(21, 1)], + merged_resource_id=30, + ) + session = FakeSession( + scalar_result=7, + scalars_result=[first, second], + execute_rows=[("PENDING", 5), ("APPROVED", 2)], ) - session = FakeSession(scalars_result=[first, second]) - views = await mca.list_merge_candidates(session) + async def fake_coalesce(session_arg, resource_ids, licensed): + assert resource_ids == [18, 19, 20, 21, 30] + assert licensed == ["rmi"] + return { + 18: _named("From WM", source="wm"), + 19: _named("From RMI", source="rmi"), + # Merged-away originals are null shells; the merged resource names it. + 20: _named(None), + 21: _named(None), + 30: _named("Merged"), + } + + monkeypatch.setattr(mca, "coalesce_resources", fake_coalesce) - assert [view.id for view in views] == [2, 1] - assert views[0].resource_ids == [18, 19] - assert views[1].status == MergeCandidateStatus.DENIED + page = await mca.list_merge_candidates( + session, MergeCandidateQueryParams(page=1, page_size=2), ["rmi"] + ) + + assert [view.id for view in page.items] == [2, 1] + assert page.items[0].resource_ids == [18, 19] + # Best-ranked source wins regardless of resource order. + assert page.items[0].name == "From RMI" + assert page.items[1].name == "Merged" + assert page.total_count == 7 + assert page.total_pages == 4 + # Every status is reported, including ones with no candidates. + assert page.status_counts == { + MergeCandidateStatus.PENDING: 5, + MergeCandidateStatus.APPROVED: 2, + MergeCandidateStatus.DENIED: 0, + } + + +def test_candidate_name_breaks_rank_ties_by_resource_position(): + candidate = FakeCandidate( + id=1, + status=MergeCandidateStatus.PENDING, + items=[FakeItem(19, 1), FakeItem(18, 0)], + ) + coalesced = {18: _named("First"), 19: _named("Second")} + + assert mca._candidate_name(candidate, coalesced) == "First" + + +def test_candidate_name_is_none_without_a_licensed_name(): + candidate = FakeCandidate( + id=1, + status=MergeCandidateStatus.PENDING, + items=[FakeItem(18, 0), FakeItem(19, 1)], + ) + coalesced = {18: _named(None), 19: _named(None)} + + assert mca._candidate_name(candidate, coalesced) is None @pytest.mark.anyio @@ -561,6 +629,7 @@ async def fake_coalesce(session_arg, resource_ids, licensed): rid: SimpleNamespace( source_data=[SimpleNamespace(source="rmi", id=rid, name=f"name-{rid}")], view=OilGasFieldBase(name=f"name-{rid}", country="USA"), + provenance={"name": (f"name-{rid}", "rmi", rid)}, ) for rid in resource_ids } @@ -578,6 +647,7 @@ async def fake_default_priority(session_arg): assert isinstance(view, MergeCandidateDetailView) assert view.resource_ids == [18, 19] + assert view.name == "name-18" # resources detail objects are gone; compare carries the per-source values assert not hasattr(view, "resources") assert [c.field for c in view.compare] == list(OilGasFieldBase.model_fields) @@ -607,21 +677,29 @@ async def fake_load_candidate_model(session_arg, candidate_id): async def fake_coalesce(session_arg, resource_ids, licensed): return { rid: SimpleNamespace( - source_data=[], view=OilGasFieldBase(name=None, country=None) + source_data=[], + view=OilGasFieldBase(name=None, country=None), + provenance={"name": None}, ) for rid in resource_ids } + async def fake_coalesce_merged(session_arg, resource_ids, licensed): + assert resource_ids == [31] + return {31: _named("Merged")} + async def fake_default_priority(session_arg): return {"rmi": 1, "gem": 2, "wm": 3, "llm": 4} monkeypatch.setattr(mca, "_load_candidate_model", fake_load_candidate_model) monkeypatch.setattr(mca, "coalesce_resources_with_sources", fake_coalesce) monkeypatch.setattr(mca, "_default_source_priority", fake_default_priority) + monkeypatch.setattr(mca, "coalesce_resources", fake_coalesce_merged) view = await mca.get_merge_candidate(AsyncMock(), candidate_id=2) assert view.merged_resource_id == 31 + assert view.name == "Merged" # every resource is null on every field -> they all agree (match), no values assert all(c.status == "match" for c in view.compare) assert all(c.values == [] for c in view.compare) diff --git a/deployments/entity-linkage/src/stitch/entity_linkage/client.py b/deployments/entity-linkage/src/stitch/entity_linkage/client.py index fb163927..6c11a9ff 100644 --- a/deployments/entity-linkage/src/stitch/entity_linkage/client.py +++ b/deployments/entity-linkage/src/stitch/entity_linkage/client.py @@ -76,7 +76,11 @@ async def iter_oil_gas_fields( yield self._to_candidate(item) async def list_merge_candidates(self) -> list[dict[str, Any]]: - return await self._client.list_merge_candidates() + """Every merge candidate of every status, gathered page by page. + + De-duplication needs reviewed candidates too, so no status filter. + """ + return [item async for item in self._client.iter_merge_candidates()] @staticmethod def _to_candidate(item: dict[str, Any]) -> FieldCandidate: diff --git a/deployments/entity-linkage/tests/test_client.py b/deployments/entity-linkage/tests/test_client.py index 9e234eec..c1198fa7 100644 --- a/deployments/entity-linkage/tests/test_client.py +++ b/deployments/entity-linkage/tests/test_client.py @@ -71,6 +71,42 @@ def handler(request: httpx.Request) -> httpx.Response: await client.aclose() +@pytest.mark.anyio +async def test_list_merge_candidates_gathers_every_page_of_every_status( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(STITCH_CLIENT_BEARER_TOKEN_ENV_VAR, "token-123") + requests: list[list[tuple[str, str]]] = [] + + def handler(request: httpx.Request) -> httpx.Response: + params = list(request.url.params.multi_items()) + requests.append(params) + page = int(request.url.params["page"]) + # A full first page, then one more candidate. + ids = range(200) if page == 1 else [200] + return httpx.Response( + 200, + json={ + "items": [{"id": i} for i in ids], + "total_count": 201, + "page": page, + "page_size": 200, + "total_pages": 2, + }, + ) + + client = make_client(handler) + + candidates = await client.list_merge_candidates() + + assert [c["id"] for c in candidates] == list(range(201)) + # No status filter: de-dupe must see reviewed candidates too. + assert requests == [ + [("page", "1"), ("page_size", "200")], + [("page", "2"), ("page_size", "200")], + ] + + @pytest.mark.anyio async def test_post_merge_sends_current_branch_payload_shape( monkeypatch: pytest.MonkeyPatch, diff --git a/deployments/stitch-frontend/README.md b/deployments/stitch-frontend/README.md index b6f280c4..cb01ceda 100644 --- a/deployments/stitch-frontend/README.md +++ b/deployments/stitch-frontend/README.md @@ -12,6 +12,7 @@ React frontend application styled with Tailwind CSS and built with Vite. - [Testing](#testing) - [Queries](#queries) - [List view URLs](#list-view-urls) +- [Merge review URLs](#merge-review-urls) - [Resource Detail View](#resource-detail-view) ## Tech Stack @@ -217,6 +218,31 @@ Notes: The schema lives in `src/config/listParams.js`. +## Merge review URLs + +The Merge Review queue (`/merge-candidate-review`) encodes its view the same way, so a copied link opens the same page of the queue for everyone. + +| Param | Values | Omitted when | +| ------------------------ | --------------------------------------------------------- | ----------------------------- | +| `page` | integer ≥ 1 | page 1 | +| `page_size` | 10, 25, 50, 100 | 25 | +| `status` | `PENDING`, `APPROVED`, `DENIED` (repeat per value), `all` | only `PENDING` is selected | +| `sort_by` + `sort_order` | `created` or `reviewed_at`, with `asc` or `desc` | newest first (`created desc`) | + +Example — approved and denied candidates, most recently reviewed first, page 2: + +``` +/merge-candidate-review?page=2&status=APPROVED&status=DENIED&sort_by=reviewed_at&sort_order=desc +``` + +Notes: + +- The queue shows pending candidates by default, so a bare `/merge-candidate-review` is the review work still to do. Clearing every status in the Status filter shows all of them, written as `status=all`. +- The page size is not remembered between visits, unlike the resources list. +- Unknown statuses are dropped, and a malformed `page`, `page_size` or sort falls back to its default. A `page` past the end steps back to the last page. + +The schema lives in `src/config/mergeReviewParams.js`. + ## API To temporarily run the API in development mode, outside Docker: diff --git a/deployments/stitch-frontend/src/components/MergeSourceComparison.jsx b/deployments/stitch-frontend/src/components/MergeSourceComparison.jsx index 0fd5461f..b0487ffd 100644 --- a/deployments/stitch-frontend/src/components/MergeSourceComparison.jsx +++ b/deployments/stitch-frontend/src/components/MergeSourceComparison.jsx @@ -167,9 +167,9 @@ function ColumnHeader({ resourceId, sourceDetails, index }) { // The loaded layout with placeholders in the cells. Headers and field labels are // real: resourceIds is a prop and FIELD_META is static, so neither changes when // the data lands. The trailing bar stands in for the collapsed accordion summary. -// Source mixes are real too once their own query has data -- usually already, -// since the queue loads it for candidate names -- and the placeholder is the -// same height, so showing them early causes no layout shift. +// Source mixes are real too once their own query has data -- e.g. when +// returning to a candidate whose details are cached -- and the placeholder is +// the same height, so showing them early causes no layout shift. function ComparisonSkeleton({ resourceIds, sourceDetails }) { return ( ); diff --git a/deployments/stitch-frontend/src/components/Pagination.test.jsx b/deployments/stitch-frontend/src/components/Pagination.test.jsx index 2d206137..0d7013f4 100644 --- a/deployments/stitch-frontend/src/components/Pagination.test.jsx +++ b/deployments/stitch-frontend/src/components/Pagination.test.jsx @@ -236,9 +236,11 @@ describe("Pagination", () => { expect(button.className).toMatch(/(^|\s)shrink-0(\s|$)/); } - for (const span of container.querySelectorAll( - 'span[aria-hidden="true"]', - )) { + const ellipses = [ + ...container.querySelectorAll('span[aria-hidden="true"]'), + ].filter((span) => span.textContent === "…"); + expect(ellipses.length).toBeGreaterThan(0); + for (const span of ellipses) { expect(span.className).toMatch(/(^|\s)shrink-0(\s|$)/); } @@ -247,4 +249,45 @@ describe("Pagination", () => { ).toMatch(/(^|\s)flex-wrap(\s|$)/); }); }); + + describe("compact", () => { + it("shows the page position as text instead of page buttons", () => { + const { container } = renderPagination({ + page: 2, + totalCount: 143, + totalPages: 6, + compact: true, + }); + + expect(screen.getByText("Page 2 of 6")).toBeInTheDocument(); + expect(pageButtons(container)).toHaveLength(0); + expect(screen.getByText("Showing 26–50 of 143")).toBeInTheDocument(); + }); + + it("still steps with the arrows", async () => { + const user = userEvent.setup(); + renderPagination({ + page: 2, + totalCount: 143, + totalPages: 6, + compact: true, + }); + + await user.click(screen.getByLabelText("Previous page")); + await user.click(screen.getByLabelText("Next page")); + + expect(onPageChange.mock.calls).toEqual([[1], [3]]); + }); + + it("formats large page counts with separators", () => { + renderPagination({ + page: 12345, + totalCount: 1_000_000, + totalPages: 40_000, + compact: true, + }); + + expect(screen.getByText("Page 12,345 of 40,000")).toBeInTheDocument(); + }); + }); }); diff --git a/deployments/stitch-frontend/src/components/Select.jsx b/deployments/stitch-frontend/src/components/Select.jsx new file mode 100644 index 00000000..93e62eeb --- /dev/null +++ b/deployments/stitch-frontend/src/components/Select.jsx @@ -0,0 +1,24 @@ +// A native + {children} + + + + ); +} diff --git a/deployments/stitch-frontend/src/config/mergeReviewParams.js b/deployments/stitch-frontend/src/config/mergeReviewParams.js new file mode 100644 index 00000000..9af3d651 --- /dev/null +++ b/deployments/stitch-frontend/src/config/mergeReviewParams.js @@ -0,0 +1,138 @@ +/** + * URL schema for the Merge Review queue. + * + * Same contract as the resources list (config/listParams.js): the query string + * is a complete serialization of the queue view, loading a URL reproduces it + * for any user, and the param names mirror what the API takes, so there is no + * translation layer. See the "Merge review URLs" section of the frontend README + * before changing them. + * + * Rules: + * - Defaults are omitted, so the default view (pending candidates, newest + * first, page 1) is a bare /merge-candidate-review. + * - Serialization order is fixed, so the same view always produces the same URL. + * - Junk is tolerated, never fatal: an unknown status is dropped, and a + * malformed page, page_size or sort falls back to its default. + * - The default status filter is PENDING, so "no status param" cannot also + * mean "every status". Selecting nothing is written as `status=all`. + */ +import { DEFAULT_PAGE, PAGE_SIZE_OPTIONS } from "../queries/resources"; + +export const MERGE_STATUSES = ["PENDING", "APPROVED", "DENIED"]; +export const DEFAULT_STATUSES = ["PENDING"]; +const ALL_STATUSES = "all"; + +export const DEFAULT_QUEUE_PAGE_SIZE = 25; + +// Each option is one sort_by + sort_order pair the API accepts, labelled in +// plain language. The first is the default. +export const QUEUE_SORT_OPTIONS = [ + { + key: "newest", + label: "Newest first", + sortBy: "created", + sortOrder: "desc", + }, + { key: "oldest", label: "Oldest first", sortBy: "created", sortOrder: "asc" }, + { + key: "recently-reviewed", + label: "Recently reviewed", + sortBy: "reviewed_at", + sortOrder: "desc", + }, + { + key: "earliest-reviewed", + label: "Earliest reviewed", + sortBy: "reviewed_at", + sortOrder: "asc", + }, +]; +const DEFAULT_SORT = QUEUE_SORT_OPTIONS[0]; + +function parsePage(raw) { + const value = Number(raw); + return /^\d+$/.test(raw ?? "") && Number.isSafeInteger(value) && value >= 1 + ? value + : DEFAULT_PAGE; +} + +function parsePageSize(raw) { + const value = Number(raw); + return PAGE_SIZE_OPTIONS.includes(value) ? value : DEFAULT_QUEUE_PAGE_SIZE; +} + +// [] means "every status". Returned in MERGE_STATUSES order. +function parseStatuses(rawValues) { + if (rawValues.includes(ALL_STATUSES)) return []; + const statuses = MERGE_STATUSES.filter((status) => + rawValues.includes(status), + ); + return statuses.length ? statuses : DEFAULT_STATUSES; +} + +// sort_by and sort_order are a unit; an unknown pair falls back to the default. +function parseSort(rawSortBy, rawSortOrder) { + return ( + QUEUE_SORT_OPTIONS.find( + (option) => + option.sortBy === rawSortBy && option.sortOrder === rawSortOrder, + ) ?? DEFAULT_SORT + ); +} + +function isDefaultStatuses(statuses) { + return ( + statuses.length === DEFAULT_STATUSES.length && + DEFAULT_STATUSES.every((status) => statuses.includes(status)) + ); +} + +export function parseMergeReviewParams(searchParams) { + return { + page: parsePage(searchParams.get("page")), + pageSize: parsePageSize(searchParams.get("page_size")), + statuses: parseStatuses(searchParams.getAll("status")), + sortKey: parseSort( + searchParams.get("sort_by"), + searchParams.get("sort_order"), + ).key, + }; +} + +export function toMergeReviewParams({ page, pageSize, statuses, sortKey }) { + const params = new URLSearchParams(); + if (page > DEFAULT_PAGE) params.set("page", String(page)); + if (pageSize !== DEFAULT_QUEUE_PAGE_SIZE) { + params.set("page_size", String(pageSize)); + } + + if (statuses.length === 0) { + params.set("status", ALL_STATUSES); + } else if (!isDefaultStatuses(statuses)) { + for (const status of MERGE_STATUSES) { + if (statuses.includes(status)) params.append("status", status); + } + } + + const sort = QUEUE_SORT_OPTIONS.find((option) => option.key === sortKey); + if (sort && sort !== DEFAULT_SORT) { + params.set("sort_by", sort.sortBy); + params.set("sort_order", sort.sortOrder); + } + + return params; +} + +// The list endpoint's query params for a parsed view. An empty status list +// sends no status filter, which the API reads as every status. +export function toMergeCandidateQuery({ page, pageSize, statuses, sortKey }) { + const sort = + QUEUE_SORT_OPTIONS.find((option) => option.key === sortKey) ?? DEFAULT_SORT; + return { + page, + page_size: pageSize, + status: statuses.length ? statuses : undefined, + sort_by: sort.sortBy, + sort_order: sort.sortOrder, + }; +} diff --git a/deployments/stitch-frontend/src/config/mergeReviewParams.test.js b/deployments/stitch-frontend/src/config/mergeReviewParams.test.js new file mode 100644 index 00000000..dc5adc89 --- /dev/null +++ b/deployments/stitch-frontend/src/config/mergeReviewParams.test.js @@ -0,0 +1,112 @@ +import { describe, it, expect } from "vitest"; +import { + parseMergeReviewParams, + toMergeCandidateQuery, + toMergeReviewParams, +} from "./mergeReviewParams"; + +const parse = (search) => parseMergeReviewParams(new URLSearchParams(search)); +const serialize = (state) => toMergeReviewParams(state).toString(); + +const DEFAULT_STATE = { + page: 1, + pageSize: 25, + statuses: ["PENDING"], + sortKey: "newest", +}; + +describe("parseMergeReviewParams", () => { + it("returns the pending queue, newest first, for an empty query string", () => { + expect(parse("")).toEqual(DEFAULT_STATE); + }); + + it("reads every supported param", () => { + expect( + parse( + "page=3&page_size=50&status=DENIED&status=APPROVED&sort_by=reviewed_at&sort_order=asc", + ), + ).toEqual({ + page: 3, + pageSize: 50, + // Canonical order, whatever order the URL used. + statuses: ["APPROVED", "DENIED"], + sortKey: "earliest-reviewed", + }); + }); + + it("reads status=all as every status", () => { + expect(parse("status=all").statuses).toEqual([]); + }); + + it("drops unknown statuses, falling back to pending when none are left", () => { + expect(parse("status=MAYBE&status=DENIED").statuses).toEqual(["DENIED"]); + expect(parse("status=MAYBE").statuses).toEqual(["PENDING"]); + }); + + it("falls back to defaults for malformed page, page_size and sort", () => { + expect(parse("page=0&page_size=7&sort_by=name&sort_order=asc")).toEqual( + DEFAULT_STATE, + ); + expect(parse("page=abc").page).toBe(1); + // A lone sort_order has no meaning without its sort_by. + expect(parse("sort_order=asc").sortKey).toBe("newest"); + }); +}); + +describe("toMergeReviewParams", () => { + it("writes a bare query string for the default view", () => { + expect(serialize(DEFAULT_STATE)).toBe(""); + }); + + it("writes every non-default part in a fixed order", () => { + expect( + serialize({ + page: 2, + pageSize: 10, + statuses: ["DENIED", "PENDING"], + sortKey: "oldest", + }), + ).toBe( + "page=2&page_size=10&status=PENDING&status=DENIED&sort_by=created&sort_order=asc", + ); + }); + + it("writes an empty status selection as status=all", () => { + expect(serialize({ ...DEFAULT_STATE, statuses: [] })).toBe("status=all"); + }); + + it("round-trips through parse", () => { + const state = { + page: 4, + pageSize: 100, + statuses: [], + sortKey: "recently-reviewed", + }; + expect(parse(serialize(state))).toEqual(state); + }); +}); + +describe("toMergeCandidateQuery", () => { + it("maps the view to the list endpoint's params", () => { + expect( + toMergeCandidateQuery({ + page: 2, + pageSize: 50, + statuses: ["PENDING", "DENIED"], + sortKey: "recently-reviewed", + }), + ).toEqual({ + page: 2, + page_size: 50, + status: ["PENDING", "DENIED"], + sort_by: "reviewed_at", + sort_order: "desc", + }); + }); + + it("sends no status filter when every status is wanted", () => { + expect( + toMergeCandidateQuery({ ...DEFAULT_STATE, statuses: [] }).status, + ).toBeUndefined(); + }); +}); diff --git a/deployments/stitch-frontend/src/hooks/useMergeCandidateName.js b/deployments/stitch-frontend/src/hooks/useMergeCandidateName.js deleted file mode 100644 index 43bd957b..00000000 --- a/deployments/stitch-frontend/src/hooks/useMergeCandidateName.js +++ /dev/null @@ -1,10 +0,0 @@ -import { useMergeSourceDetails } from "./useMergeSourceDetails"; -import { pickCandidateName } from "../utils/mergeCandidateName"; - -// Resolves a merge candidate's display name from its source resources, using -// source-priority rules. Returns null while loading or if no source has a name. -export function useMergeCandidateName(endpoint, resourceIds) { - const { data: details } = useMergeSourceDetails(endpoint, resourceIds); - - return details ? pickCandidateName(details) : null; -} diff --git a/deployments/stitch-frontend/src/hooks/useMergeReviewState.js b/deployments/stitch-frontend/src/hooks/useMergeReviewState.js new file mode 100644 index 00000000..99961429 --- /dev/null +++ b/deployments/stitch-frontend/src/hooks/useMergeReviewState.js @@ -0,0 +1,39 @@ +/** + * Reads and writes the Merge Review queue view state from the URL. + * + * Mirrors useListState: the URL is the single source of truth, and every + * setter round-trips the complete state through the schema + * (config/mergeReviewParams.js). Refining the view (statuses, sort) rewrites + * the current history entry; paging pushes, so Back steps through pages. + * Page size is not remembered across visits, unlike the resources list. + */ +import { useSearchParams } from "react-router"; +import { + parseMergeReviewParams, + toMergeReviewParams, +} from "../config/mergeReviewParams"; +import { DEFAULT_PAGE } from "../queries/resources"; + +export function useMergeReviewState() { + const [searchParams, setSearchParams] = useSearchParams(); + const state = parseMergeReviewParams(searchParams); + + function write(changes, { replace }) { + setSearchParams(toMergeReviewParams({ ...state, ...changes }), { + replace, + }); + } + + return { + ...state, + // `replace` is for corrections the reviewer did not ask for (stepping + // back from a page past the end), which should not add a Back step. + setPage: (page, { replace = false } = {}) => write({ page }, { replace }), + setPageSize: (pageSize) => + write({ pageSize, page: DEFAULT_PAGE }, { replace: false }), + setStatuses: (statuses) => + write({ statuses, page: DEFAULT_PAGE }, { replace: true }), + setSortKey: (sortKey) => + write({ sortKey, page: DEFAULT_PAGE }, { replace: true }), + }; +} diff --git a/deployments/stitch-frontend/src/hooks/useMergeSourceDetails.js b/deployments/stitch-frontend/src/hooks/useMergeSourceDetails.js index 7ceeb2c3..12ea8c44 100644 --- a/deployments/stitch-frontend/src/hooks/useMergeSourceDetails.js +++ b/deployments/stitch-frontend/src/hooks/useMergeSourceDetails.js @@ -3,9 +3,8 @@ import { useAuthenticatedQuery } from "./useAuthenticatedQuery"; import { getResourceDetail } from "../queries/api"; // Fetches every source resource in a merge candidate as one aggregate query, so -// all entries arrive (and error) together. Callers share a cache entry keyed by -// endpoint + resource ids, so the source comparison table and the candidate -// name lookup don't issue duplicate requests for the same candidate. +// all entries arrive (and error) together. Cached by endpoint + resource ids, so +// returning to a candidate reuses the earlier result. export function useMergeSourceDetails(endpoint, resourceIds, enabled = true) { const config = useConfig(); const ids = resourceIds ?? []; diff --git a/deployments/stitch-frontend/src/hooks/useMergedResourceDetail.js b/deployments/stitch-frontend/src/hooks/useMergedResourceDetail.js index 1d064807..f7b1dfc9 100644 --- a/deployments/stitch-frontend/src/hooks/useMergedResourceDetail.js +++ b/deployments/stitch-frontend/src/hooks/useMergedResourceDetail.js @@ -2,10 +2,9 @@ import { useConfig } from "../config/useConfig"; import { useAuthenticatedQuery } from "./useAuthenticatedQuery"; import { getResourceDetail } from "../queries/api"; -// Fetches the resource a merge produced. Callers share a cache entry keyed by -// endpoint + resource id, so the merged resource view and the review panel -// heading don't issue duplicate requests. Disabled until a resource id exists -// (i.e. the candidate has actually been merged). +// Fetches the resource a merge produced, cached by endpoint + resource id. +// Disabled until a resource id exists (i.e. the candidate has actually been +// merged). export function useMergedResourceDetail(endpoint, resourceId) { const config = useConfig(); diff --git a/deployments/stitch-frontend/src/hooks/useResources.js b/deployments/stitch-frontend/src/hooks/useResources.js index 14009924..7b79ff4d 100644 --- a/deployments/stitch-frontend/src/hooks/useResources.js +++ b/deployments/stitch-frontend/src/hooks/useResources.js @@ -104,10 +104,14 @@ function useFieldSourceValuesReal( }); } -function useMergeCandidatesReal(endpoint = "oil-gas-fields", enabled = true) { +function useMergeCandidatesReal( + endpoint = "oil-gas-fields", + params = {}, + enabled = true, +) { const config = useConfig(); return useAuthenticatedQuery({ - ...resourceQueries.mergeCandidates(config, endpoint), + ...resourceQueries.mergeCandidates(config, endpoint, params), enabled, }); } @@ -323,10 +327,24 @@ function useFieldSourceValuesMock( }); } -function useMergeCandidatesMock(endpoint = "oil-gas-fields", enabled = true) { +function useMergeCandidatesMock( + endpoint = "oil-gas-fields", + params = {}, + enabled = true, +) { + const page = params.page ?? 1; + const page_size = params.page_size ?? 50; return useQuery({ - ...resourceQueries.mergeCandidates(UNUSED_MOCK_CONFIG, endpoint), - queryFn: () => Promise.resolve([]), + ...resourceQueries.mergeCandidates(UNUSED_MOCK_CONFIG, endpoint, params), + queryFn: () => + Promise.resolve({ + items: [], + total_count: 0, + page, + page_size, + total_pages: 0, + status_counts: { PENDING: 0, APPROVED: 0, DENIED: 0 }, + }), enabled, }); } diff --git a/deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.jsx b/deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.jsx index 4316097e..569b7421 100644 --- a/deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.jsx +++ b/deployments/stitch-frontend/src/pages/MergeCandidateReviewPage.jsx @@ -1,24 +1,34 @@ -import { useRef, useState } from "react"; +import { useEffect, useRef, useState } from "react"; import { Link } from "react-router"; import Button from "../components/Button"; +import FilterDropdown from "../components/FilterDropdown"; import MergeSourceComparison from "../components/MergeSourceComparison"; import MergedResourceView from "../components/MergedResourceView"; +import Pagination from "../components/Pagination"; +import Select from "../components/Select"; +import { + MERGE_STATUSES, + QUEUE_SORT_OPTIONS, + toMergeCandidateQuery, +} from "../config/mergeReviewParams"; import { useDocumentTitle } from "../hooks/useDocumentTitle"; -import { useMergeCandidateName } from "../hooks/useMergeCandidateName"; -import { useMergedResourceDetail } from "../hooks/useMergedResourceDetail"; +import { useMergeReviewState } from "../hooks/useMergeReviewState"; import { useMergeSourceDetails } from "../hooks/useMergeSourceDetails"; import { useMergeCandidates, useMergeCandidate, useReviewMergeCandidate, } from "../hooks/useResources"; -import { pickCompareName } from "../utils/candidateCompare"; -import { isEmptyValue } from "../utils/mergeComparison"; const ENDPOINT = "oil-gas-fields"; // Tailwind's md breakpoint, where the queue moves beside the decision panel // (the md: grid classes in the page layout below). const TWO_COLUMN_LAYOUT_QUERY = "(min-width: 48rem)"; +const STATUS_FILTER_LABELS = { + PENDING: "Pending", + APPROVED: "Approved", + DENIED: "Denied", +}; function getStatusClasses(status) { if (status === "PENDING") { @@ -58,19 +68,14 @@ function candidateSourcesTitle(candidate) { return parts.join(" · "); } +// The API names each candidate (the merged resource's name once approved); +// a candidate with no licensed name falls back to its id. +function candidateDisplayName(candidate) { + return candidate.name ?? `Candidate #${candidate.id}`; +} + function CandidateQueueItem({ candidate, isSelected, onSelect }) { - const sourceName = useMergeCandidateName(ENDPOINT, candidate.resource_ids); - // Post-merge, the source resources are null shells, so the merged resource - // is the authoritative name source. The hook is disabled until an id exists, - // so pending candidates skip the fetch. - const { data: mergedResource } = useMergedResourceDetail( - ENDPOINT, - candidate.merged_resource_id, - ); - const mergedName = isEmptyValue(mergedResource?.data?.name) - ? null - : mergedResource.data.name; - const displayName = mergedName ?? sourceName ?? `Candidate #${candidate.id}`; + const displayName = candidateDisplayName(candidate); return (