Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions backend/app/api/v1/strong_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
"fq[b1g_code_s][]",
"fq[dct_accessRights_s][]",
"fq[gbl_georeferenced_b][]",
"fq[b1g_georeferenced_allmaps_b][]",
# Spatial facets
"fq[geo_country][]",
"fq[geo_region][]",
Expand Down Expand Up @@ -62,6 +63,7 @@
"fq[b1g_code_s][]",
"fq[dct_accessRights_s][]",
"fq[gbl_georeferenced_b][]",
"fq[b1g_georeferenced_allmaps_b][]",
# Spatial facets
"fq[geo_country][]",
"fq[geo_region][]",
Expand Down
29 changes: 28 additions & 1 deletion backend/app/elasticsearch/index.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,11 @@ async def process_resource(resource_dict):

date_fields = {"gbl_mdmodified_dt", "b1g_dateAccessioned_s", "b1g_dateRetired_s"}
integer_fields = {"gbl_indexYear_im"}
boolean_fields = {"gbl_georeferenced_b", "b1g_child_record_b"}
boolean_fields = {
"gbl_georeferenced_b",
"b1g_child_record_b",
"b1g_georeferenced_allmaps_b",
}

for key, value in resource_dict.items():
if isinstance(value, (list, tuple)):
Expand Down Expand Up @@ -320,6 +324,10 @@ async def process_resource(resource_dict):
first = summaries[0]
processed_dict["summary"] = first.get("summary") or None

processed_dict["b1g_georeferenced_allmaps_b"] = await get_allmaps_overlay_status(
processed_dict["id"]
)

# Calculate and add time_period facet
time_period = _calculate_time_period_from_year(processed_dict.get("gbl_indexYear_im"))
if time_period:
Expand Down Expand Up @@ -389,6 +397,25 @@ async def get_resource_summaries(resource_id):
return []


async def get_allmaps_overlay_status(resource_id):
"""Return whether a resource has an annotated Allmaps overlay."""
try:
query = """
SELECT annotated
FROM resource_allmaps
WHERE resource_id = :resource_id
ORDER BY id DESC
LIMIT 1
"""
result = await database.fetch_one(query, {"resource_id": resource_id})
if not result:
return False
return bool(dict(result).get("annotated"))
except Exception as e:
logger.error(f"Error getting Allmaps status for resource {resource_id}: {str(e)}")
return False


async def get_spatial_facets(resource_id):
"""Get spatial facets for a resource."""
try:
Expand Down
1 change: 1 addition & 0 deletions backend/app/elasticsearch/mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"fields": {"keyword": {"type": "keyword", "ignore_above": 8191}},
},
"gbl_georeferenced_b": {"type": "boolean"},
"b1g_georeferenced_allmaps_b": {"type": "boolean"},
"dct_alternative_sm": {
"type": "text",
"fields": {
Expand Down
12 changes: 12 additions & 0 deletions backend/app/elasticsearch/search.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,10 @@ def get_facet_aggregation_config(facet_name: str) -> dict:
"field": "gbl_georeferenced_b",
"size": DEFAULT_FACET_SIZE,
},
"b1g_georeferenced_allmaps_b": {
"field": "b1g_georeferenced_allmaps_b",
"size": DEFAULT_FACET_SIZE,
},
"dct_subject_sm": {
"field": "dct_subject_sm.keyword",
"size": DEFAULT_FACET_SIZE,
Expand Down Expand Up @@ -285,6 +289,12 @@ def _build_search_aggregations() -> dict:
"gbl_georeferenced_b": {
"terms": {"field": "gbl_georeferenced_b", "size": DEFAULT_FACET_SIZE}
},
"b1g_georeferenced_allmaps_b": {
"terms": {
"field": "b1g_georeferenced_allmaps_b",
"size": DEFAULT_FACET_SIZE,
}
},
"geo_country": {"terms": {"field": "geo_country.keyword", "size": GEO_COUNTRY_FACET_SIZE}},
"geo_region": {"terms": {"field": "geo_region.keyword", "size": GEO_REGION_FACET_SIZE}},
"geo_county": {"terms": {"field": "geo_county.keyword", "size": GEO_COUNTY_FACET_SIZE}},
Expand Down Expand Up @@ -2555,6 +2565,8 @@ def process_aggregations(aggregations, search_context: dict):
"ogm_repo": "OGM Repo",
"access_rights_agg": "Access Rights",
"georeferenced_agg": "Georeferenced",
"gbl_georeferenced_b": "Georeferenced",
"b1g_georeferenced_allmaps_b": "Map Overlay",
"geo_country_agg": "Country",
"geo_region_agg": "Region",
"geo_county_agg": "County",
Expand Down
1 change: 1 addition & 0 deletions backend/app/services/search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ def extract_filter_queries(self, params: str) -> Dict:
"b1g_code_agg": "b1g_code_s",
"access_rights_agg": "dct_accessRights_s",
"georeferenced_agg": "gbl_georeferenced_b",
"map_overlay_agg": "b1g_georeferenced_allmaps_b",
# Spatial facet fields
"geo_country_agg": "geo_country",
"geo_region_agg": "geo_region",
Expand Down
26 changes: 25 additions & 1 deletion backend/scripts/reindex_atomic.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

Performance:
- Bulk indexing via Elasticsearch `_bulk`
- Chunk-level prefetch of summaries/spatial facets to avoid N+1 DB queries
- Chunk-level prefetch of summaries/spatial/Allmaps facets to avoid N+1 DB queries
- Optional temporary fast index settings during ingest
"""

Expand Down Expand Up @@ -249,22 +249,45 @@ async def _prefetch_spatial_facets_by_id(ids: list[str]) -> dict[str, dict[str,
return {str(row["resource_id"]): _format_spatial_facets_row(dict(row)) for row in rows}


async def _prefetch_allmaps_overlay_status_by_id(ids: list[str]) -> dict[str, bool]:
if not ids:
return {}

placeholders = ", ".join([f":id{i}" for i, _ in enumerate(ids)])
params = {f"id{i}": rid for i, rid in enumerate(ids)}
sql = f"""
SELECT resource_id, annotated
FROM resource_allmaps
WHERE resource_id IN ({placeholders})
ORDER BY id ASC
"""
rows = await database.fetch_all(sql, params)

return {str(row["resource_id"]): bool(row["annotated"]) for row in rows}


async def _prepare_documents_for_chunk(ids: list[str]) -> list[tuple[str, dict[str, Any]]]:
rows_by_id = await _db_rows_for_ids(ids)
summaries_by_id = await _prefetch_summaries_by_id(ids)
facets_by_id = await _prefetch_spatial_facets_by_id(ids)
allmaps_overlay_by_id = await _prefetch_allmaps_overlay_status_by_id(ids)

async def _cached_get_resource_summaries(resource_id: str):
return summaries_by_id.get(str(resource_id), [])

async def _cached_get_spatial_facets(resource_id: str):
return facets_by_id.get(str(resource_id))

async def _cached_get_allmaps_overlay_status(resource_id: str):
return allmaps_overlay_by_id.get(str(resource_id), False)

original_get_resource_summaries = index_module.get_resource_summaries
original_get_spatial_facets = index_module.get_spatial_facets
original_get_allmaps_overlay_status = index_module.get_allmaps_overlay_status

index_module.get_resource_summaries = _cached_get_resource_summaries
index_module.get_spatial_facets = _cached_get_spatial_facets
index_module.get_allmaps_overlay_status = _cached_get_allmaps_overlay_status

documents: list[tuple[str, dict[str, Any]]] = []
try:
Expand All @@ -278,6 +301,7 @@ async def _cached_get_spatial_facets(resource_id: str):
finally:
index_module.get_resource_summaries = original_get_resource_summaries
index_module.get_spatial_facets = original_get_spatial_facets
index_module.get_allmaps_overlay_status = original_get_allmaps_overlay_status

return documents

Expand Down
1 change: 1 addition & 0 deletions backend/tests/api/v1/test_facet_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ async def test_adv_q_parameter(
"b1g_code_s",
"dct_accessRights_s",
"gbl_georeferenced_b",
"b1g_georeferenced_allmaps_b",
"geo_country",
"geo_region",
"geo_county",
Expand Down
1 change: 1 addition & 0 deletions backend/tests/api/v1/test_strong_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ def test_search_allowed_params_facet_filters(self):
assert "fq[b1g_code_s][]" in SEARCH_ALLOWED_PARAMS
assert "fq[dct_accessRights_s][]" in SEARCH_ALLOWED_PARAMS
assert "fq[gbl_georeferenced_b][]" in SEARCH_ALLOWED_PARAMS
assert "fq[b1g_georeferenced_allmaps_b][]" in SEARCH_ALLOWED_PARAMS

def test_search_allowed_params_spatial_facets(self):
"""Test that SEARCH_ALLOWED_PARAMS contains spatial facet parameters."""
Expand Down
5 changes: 5 additions & 0 deletions backend/tests/elasticsearch/test_facet_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def test_valid_facet_names(self):
"b1g_localCollectionLabel_sm",
"dct_accessRights_s",
"gbl_georeferenced_b",
"b1g_georeferenced_allmaps_b",
"geo_country",
"geo_region",
"geo_county",
Expand Down Expand Up @@ -76,6 +77,10 @@ def test_non_keyword_fields(self):
assert config["field"] == "gbl_georeferenced_b"
assert ".keyword" not in config["field"]

config = get_facet_aggregation_config("b1g_georeferenced_allmaps_b")
assert config["field"] == "b1g_georeferenced_allmaps_b"
assert ".keyword" not in config["field"]

def test_direct_keyword_field(self):
"""Test that keyword-mapped fields can facet without a .keyword suffix."""
config = get_facet_aggregation_config("b1g_code_s")
Expand Down
41 changes: 41 additions & 0 deletions backend/tests/elasticsearch/test_index.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""
Tests for Elasticsearch indexing transformations.
"""

import pytest

import app.elasticsearch.index as index_module


@pytest.mark.asyncio
async def test_process_resource_adds_allmaps_overlay_status(monkeypatch):
async def fake_get_resource_summaries(resource_id):
return []

async def fake_get_spatial_facets(resource_id):
return None

async def fake_get_allmaps_overlay_status(resource_id):
return resource_id == "allmaps-map"

monkeypatch.setattr(
index_module,
"get_resource_summaries",
fake_get_resource_summaries,
)
monkeypatch.setattr(index_module, "get_spatial_facets", fake_get_spatial_facets)
monkeypatch.setattr(
index_module,
"get_allmaps_overlay_status",
fake_get_allmaps_overlay_status,
)

indexed = await index_module.process_resource(
{
"id": "allmaps-map",
"dct_title_s": "Annotated map",
"gbl_indexYear_im": "1929",
}
)

assert indexed["b1g_georeferenced_allmaps_b"] is True
7 changes: 6 additions & 1 deletion backend/tests/elasticsearch/test_mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def test_btaa_specific_field_mappings(self):
"b1g_dateAccessioned_s",
"b1g_dateRetired_s",
"b1g_child_record_b",
"b1g_georeferenced_allmaps_b",
"b1g_dct_mediator_sm",
"b1g_access_s",
"b1g_image_ss",
Expand Down Expand Up @@ -194,7 +195,11 @@ def test_field_type_consistency(self):
assert properties[field]["fields"]["keyword"]["type"] == "keyword"

# Boolean fields should be configured as booleans
boolean_fields = ["gbl_georeferenced_b", "b1g_child_record_b"]
boolean_fields = [
"gbl_georeferenced_b",
"b1g_child_record_b",
"b1g_georeferenced_allmaps_b",
]

for field in boolean_fields:
if field in properties:
Expand Down
2 changes: 2 additions & 0 deletions backend/tests/services/test_search_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,7 @@ def test_extract_filter_queries_all_aggregation_fields(self):
"fq[b1g_code_s][]=BTAA&"
"fq[access_rights_agg][]=Public&"
"fq[georeferenced_agg][]=true&"
"fq[map_overlay_agg][]=true&"
"fq[geo_country_agg][]=USA&"
"fq[geo_region_agg][]=Midwest&"
"fq[geo_county_agg][]=Hennepin"
Expand All @@ -708,6 +709,7 @@ def test_extract_filter_queries_all_aggregation_fields(self):
assert result["b1g_code_s"] == ["BTAA"]
assert result["dct_accessRights_s"] == ["Public"]
assert result["gbl_georeferenced_b"] == ["true"]
assert result["b1g_georeferenced_allmaps_b"] == ["true"]
assert result["geo_country"] == ["USA"]
assert result["geo_region"] == ["Midwest"]
assert result["geo_county"] == ["Hennepin"]
Expand Down
49 changes: 43 additions & 6 deletions frontend/src/__tests__/components/FacetList.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { render, screen, within } from '@testing-library/react';
import { axeWithWCAG22 } from '../../test-utils/axe';
import userEvent from '@testing-library/user-event';
import { BrowserRouter } from 'react-router';
import { FacetList } from '../../components/FacetList';
import { FacetList, type JsonApiFacet } from '../../components/FacetList';
import { vi } from 'vitest';

// Mock useSearchParams
Expand All @@ -24,9 +24,11 @@ vi.mock('../../utils/facetLabels', () => ({
dc_publisher_sm: 'Publisher',
dct_temporal_sm: 'Year',
dct_spatial_sm: 'Location',
b1g_georeferenced_allmaps_b: 'Map Overlay',
},
// For unit tests we keep facet IDs stable (no remapping).
normalizeFacetId: (id: string) => id,
normalizeFacetId: (id: string) =>
id === 'map_overlay_agg' ? 'b1g_georeferenced_allmaps_b' : id,
}));

vi.mock('../../constants/facets', () => ({
Expand All @@ -37,6 +39,7 @@ vi.mock('../../constants/facets', () => ({
'year_histogram',
'dct_spatial_sm',
'georeferenced_agg',
'b1g_georeferenced_allmaps_b',
],
}));

Expand Down Expand Up @@ -204,7 +207,7 @@ const mockFacetData = [
},
];

const mockTimelineFacetData: any = [
const mockTimelineFacetData: JsonApiFacet[] = [
{
type: 'timeline' as const,
id: 'year_histogram',
Expand Down Expand Up @@ -296,7 +299,7 @@ describe('FacetList Component', () => {

render(
<TestWrapper>
<FacetList facets={compactFacetData as any} />
<FacetList facets={compactFacetData as JsonApiFacet[]} />
</TestWrapper>
);

Expand Down Expand Up @@ -351,7 +354,7 @@ describe('FacetList Component', () => {
it('displays message when facets is null', () => {
render(
<TestWrapper>
<FacetList facets={null as any} />
<FacetList facets={null as unknown as JsonApiFacet[]} />
</TestWrapper>
);

Expand Down Expand Up @@ -663,6 +666,38 @@ describe('FacetList Component', () => {
).toBeInTheDocument(); // Value "false" renamed
});

it('renders map overlay facet with true and false values', () => {
const mapOverlayFacet = [
{
type: 'facet' as const,
id: 'b1g_georeferenced_allmaps_b',
attributes: {
label: 'Map Overlay',
items: [
[1, 10],
[0, 5],
] as [number, number][],
},
},
];

render(
<TestWrapper>
<FacetList facets={mapOverlayFacet} />
</TestWrapper>
);

expect(
screen.getByRole('heading', { level: 3, name: 'Map Overlay' })
).toBeInTheDocument();
expect(
screen.getByRole('button', { name: /true\s*\(\d+\)/ })
).toBeInTheDocument();
expect(
screen.getByRole('button', { name: /false\s*\(\d+\)/ })
).toBeInTheDocument();
});

it('handles facets with missing items attribute', () => {
const facetsWithMissingItems = [
{
Expand All @@ -677,7 +712,9 @@ describe('FacetList Component', () => {

render(
<TestWrapper>
<FacetList facets={facetsWithMissingItems as any} />
<FacetList
facets={facetsWithMissingItems as unknown as JsonApiFacet[]}
/>
</TestWrapper>
);

Expand Down
Loading
Loading