diff --git a/backend/app/services/relationship_service.py b/backend/app/services/relationship_service.py index f7bbda25..c9de5ff4 100644 --- a/backend/app/services/relationship_service.py +++ b/backend/app/services/relationship_service.py @@ -15,12 +15,23 @@ "hasPart": "dct_isPartOf_sm", "pcdm:hasMember": "pcdm_memberOf_sm", "hasMember": "pcdm_memberOf_sm", + "dct:isSourceOf": "dct_source_sm", + "isSourceOf": "dct_source_sm", +} +RELATIONSHIP_PREDICATE_ALIASES = { + "dct:sourceOf": "dct:isSourceOf", + "sourceOf": "isSourceOf", } PUBLICATION_STATE_PUBLISHED = "published" +def _canonical_relationship_predicate(predicate: Any) -> str: + value = str(predicate) + return RELATIONSHIP_PREDICATE_ALIASES.get(value, value) + + def _relationship_browse_link(resource_id: str, predicate: str) -> str | None: - facet_field = RELATIONSHIP_BROWSE_FACET_FIELDS.get(predicate) + facet_field = RELATIONSHIP_BROWSE_FACET_FIELDS.get(_canonical_relationship_predicate(predicate)) if not facet_field: return None return f"/search?include_filters[{facet_field}][]={quote(str(resource_id), safe='')}" @@ -153,13 +164,21 @@ async def get_resource_relationships_map( ) relationships_by_id: Dict[str, Dict] = {} + seen_relationships: set[tuple[str, str, str]] = set() for rel in db_relationships: subject_id = str(rel["subject_id"]) + predicate = _canonical_relationship_predicate(rel["predicate"]) + object_id = str(rel["object_id"]) + relationship_key = (subject_id, predicate, object_id) + if relationship_key in seen_relationships: + continue + seen_relationships.add(relationship_key) + relationships = relationships_by_id.setdefault(subject_id, {}) - if rel["predicate"] not in relationships: - relationships[rel["predicate"]] = [] - relationships[rel["predicate"]].append( + if predicate not in relationships: + relationships[predicate] = [] + relationships[predicate].append( { "resource_id": rel["object_id"], "resource_title": rel["dct_title_s"], @@ -169,7 +188,7 @@ async def get_resource_relationships_map( logger.debug( "Added relationship for %s: %s -> %s", subject_id, - rel["predicate"], + predicate, rel["object_id"], ) @@ -188,10 +207,12 @@ async def get_resource_relationship_summaries_map( ) summaries_by_id: Dict[str, Dict[str, Any]] = {} + seen_relationships: set[tuple[str, str, str]] = set() for rel in db_relationships: subject_id = str(rel["subject_id"]) - predicate = str(rel["predicate"]) + predicate = _canonical_relationship_predicate(rel["predicate"]) + object_id = str(rel["object_id"]) summary = summaries_by_id.setdefault( subject_id, { @@ -201,16 +222,21 @@ async def get_resource_relationship_summaries_map( }, ) relationships = summary["relationships"].setdefault(predicate, []) - relationships.append( - { - "resource_id": rel["object_id"], - "resource_title": rel["dct_title_s"], - "link": f"/resources/{rel['object_id']}", - } - ) + relationship_key = (subject_id, predicate, object_id) + if relationship_key not in seen_relationships: + seen_relationships.add(relationship_key) + relationships.append( + { + "resource_id": rel["object_id"], + "resource_title": rel["dct_title_s"], + "link": f"/resources/{rel['object_id']}", + } + ) total_count = _record_get(rel, "total_count", len(relationships)) - summary["counts"][predicate] = int(total_count) + summary["counts"][predicate] = max( + summary["counts"].get(predicate, 0), int(total_count) + ) browse_link = _relationship_browse_link(subject_id, predicate) if browse_link: diff --git a/backend/app/services/relationship_sync.py b/backend/app/services/relationship_sync.py index 7913ce71..8c868812 100644 --- a/backend/app/services/relationship_sync.py +++ b/backend/app/services/relationship_sync.py @@ -15,12 +15,17 @@ ("dct_relation_sm", "dct:relation", "dct:relation"), ("dct_isPartOf_sm", "dct:isPartOf", "dct:hasPart"), ("pcdm_memberOf_sm", "pcdm:memberOf", "pcdm:hasMember"), - ("dct_source_sm", "dct:source", "dct:sourceOf"), + ("dct_source_sm", "dct:source", "dct:isSourceOf"), ("dct_isVersionOf_sm", "dct:isVersionOf", "dct:hasVersion"), ("dct_replaces_sm", "dct:replaces", "dct:isReplacedBy"), ("dct_isReplacedBy_sm", "dct:isReplacedBy", "dct:replaces"), ) +# Incremental relationship sync briefly used dct:sourceOf for the inverse of +# dct:source. Keep it in the cleanup set so a subsequent sync removes those +# legacy rows while emitting only the canonical dct:isSourceOf predicate. +LEGACY_RELATIONSHIP_PREDICATES: Tuple[str, ...] = ("dct:sourceOf",) + ALL_RELATIONSHIP_PREDICATES: Tuple[str, ...] = tuple( sorted( { @@ -28,6 +33,7 @@ for _, predicate, inverse in RELATIONSHIP_FAMILIES for predicate in (predicate, inverse) } + | set(LEGACY_RELATIONSHIP_PREDICATES) ) ) diff --git a/backend/tests/services/test_relationship_service.py b/backend/tests/services/test_relationship_service.py index 330b8819..b1aec308 100644 --- a/backend/tests/services/test_relationship_service.py +++ b/backend/tests/services/test_relationship_service.py @@ -150,6 +150,72 @@ async def fake_fetch_relationship_rows(resource_ids, *, limit_per_predicate=None "link": "/resources/part-1", } + @pytest.mark.asyncio + async def test_source_of_aliases_are_canonicalized_and_deduplicated(self, monkeypatch): + async def fake_fetch_relationship_rows(resource_ids, *, limit_per_predicate=None): + assert list(resource_ids) == ["parent-record"] + assert limit_per_predicate is None + return [ + { + "subject_id": "parent-record", + "predicate": predicate, + "object_id": "child-record", + "dct_title_s": "Child record", + } + for predicate in ("dct:isSourceOf", "dct:sourceOf") + ] + + monkeypatch.setattr( + RelationshipService, + "_fetch_relationship_rows", + staticmethod(fake_fetch_relationship_rows), + ) + + relationships = await RelationshipService.get_resource_relationships("parent-record") + + assert relationships == { + "dct:isSourceOf": [ + { + "resource_id": "child-record", + "resource_title": "Child record", + "link": "/resources/child-record", + } + ] + } + + @pytest.mark.asyncio + async def test_source_of_summary_uses_derived_records_browse_filter(self, monkeypatch): + async def fake_fetch_relationship_rows(resource_ids, *, limit_per_predicate=None): + assert list(resource_ids) == ["parent-record"] + assert limit_per_predicate == 5 + return [ + { + "subject_id": "parent-record", + "predicate": predicate, + "object_id": "child-record", + "dct_title_s": "Child record", + "total_count": 20, + } + for predicate in ("dct:isSourceOf", "dct:sourceOf") + ] + + monkeypatch.setattr( + RelationshipService, + "_fetch_relationship_rows", + staticmethod(fake_fetch_relationship_rows), + ) + + summaries = await RelationshipService.get_resource_relationship_summaries_map( + ["parent-record"] + ) + + summary = summaries["parent-record"] + assert len(summary["relationships"]["dct:isSourceOf"]) == 1 + assert summary["counts"] == {"dct:isSourceOf": 20} + assert summary["browse_links"] == { + "dct:isSourceOf": ("/search?include_filters[dct_source_sm][]=parent-record") + } + @pytest.mark.asyncio async def test_get_resource_relationships_with_real_database(self): """Test getting resource relationships using real database connection.""" diff --git a/backend/tests/services/test_relationship_sync.py b/backend/tests/services/test_relationship_sync.py new file mode 100644 index 00000000..3c348367 --- /dev/null +++ b/backend/tests/services/test_relationship_sync.py @@ -0,0 +1,35 @@ +from app.services.relationship_sync import ( + ALL_RELATIONSHIP_PREDICATES, + RELATIONSHIP_FAMILIES, + _build_relationship_rows, +) + + +def test_source_relationships_use_canonical_parent_and_child_predicates(): + source_family = next(family for family in RELATIONSHIP_FAMILIES if family[0] == "dct_source_sm") + + rows = _build_relationship_rows( + { + source_family: [ + { + "id": "child-record", + "dct_source_sm": ["parent-record"], + } + ] + }, + ["child-record", "parent-record"], + ) + + assert rows == [ + { + "subject_id": "child-record", + "predicate": "dct:source", + "object_id": "parent-record", + }, + { + "subject_id": "parent-record", + "predicate": "dct:isSourceOf", + "object_id": "child-record", + }, + ] + assert "dct:sourceOf" in ALL_RELATIONSHIP_PREDICATES diff --git a/docs/backend/relationships.md b/docs/backend/relationships.md index a789a54a..77399cba 100644 --- a/docs/backend/relationships.md +++ b/docs/backend/relationships.md @@ -13,6 +13,10 @@ The field `dct_isPartOf_sm` is defined in the index mapping (text + keyword subf `resource_relationships` automatically for imported records and unchanged resources that point at them. This includes inverse replacement links such as `dct:isReplacedBy` from a new record's `dct_replaces_sm` value. +- **Source direction**: A child stores its parent ID in `dct_source_sm`. The + relationship table exposes that as `dct:source` on the child and the inverse + `dct:isSourceOf` on the parent. Resource pages label these "Source record" + and "Derived records," respectively. - **Make task** (from project root): ```bash make populate-relationships diff --git a/frontend/src/__tests__/components/resource/FullDetailsTable.test.tsx b/frontend/src/__tests__/components/resource/FullDetailsTable.test.tsx index 4c1694e0..efc6c7cf 100644 --- a/frontend/src/__tests__/components/resource/FullDetailsTable.test.tsx +++ b/frontend/src/__tests__/components/resource/FullDetailsTable.test.tsx @@ -53,6 +53,60 @@ describe('FullDetailsTable', () => { expect(screen.getByText('Collection records...')).toBeInTheDocument(); }); + it('labels a child dct:source relationship as its source record', () => { + const data = { + ...baseData, + meta: { + ui: { + relationships: { + 'dct:source': [ + { + resource_id: 'parent-record', + resource_title: 'Parent record', + }, + ], + }, + }, + }, + }; + + renderWithRouter(); + + expect(screen.getByText('Source record...')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: 'Parent record' })).toHaveAttribute( + 'href', + '/resources/parent-record' + ); + }); + + it('labels canonical and legacy inverse source predicates as derived records', () => { + const data = { + ...baseData, + meta: { + ui: { + relationships: { + 'dct:isSourceOf': [ + { + resource_id: 'canonical-child', + resource_title: 'Canonical child', + }, + ], + 'dct:sourceOf': [ + { + resource_id: 'legacy-child', + resource_title: 'Legacy child', + }, + ], + }, + }, + }, + }; + + renderWithRouter(); + + expect(screen.getAllByText('Derived records...')).toHaveLength(2); + }); + it('Browse all link for dct:hasPart uses include_filters[dct_isPartOf_sm][]', () => { const parentId = 'eee6150b-ce2f-4837-9d17-ce72a0c1c26f'; const data = { @@ -117,6 +171,37 @@ describe('FullDetailsTable', () => { ); }); + it('Browse all link for dct:isSourceOf filters children by dct_source_sm', () => { + const parentId = 'parent-record'; + const data = { + ...baseData, + attributes: { + ...baseData.attributes, + ogm: { ...baseData.attributes.ogm, id: parentId }, + }, + meta: { + ui: { + relationships: { + 'dct:isSourceOf': Array.from({ length: 6 }, (_, i) => ({ + resource_id: `child-${i}`, + resource_title: `Child ${i}`, + })), + }, + }, + }, + }; + + renderWithRouter(); + + const browseLink = screen.getByRole('link', { + name: /Browse all 6 records/, + }); + expect(browseLink).toHaveAttribute( + 'href', + expect.stringContaining('include_filters[dct_source_sm]') + ); + }); + it('does not show Browse all link when 5 or fewer items', () => { const data = { ...baseData, diff --git a/frontend/src/components/resource/FullDetailsTable.tsx b/frontend/src/components/resource/FullDetailsTable.tsx index 43289dcf..ceb5ddff 100644 --- a/frontend/src/components/resource/FullDetailsTable.tsx +++ b/frontend/src/components/resource/FullDetailsTable.tsx @@ -47,10 +47,12 @@ const relationshipLabels: { [key: string]: string } = { 'dct:replaces': 'Replaces...', isReplacedBy: 'Is replaced by...', 'dct:isReplacedBy': 'Is replaced by...', - isSourceOf: 'Source records...', - 'dct:isSourceOf': 'Source records...', - source: 'Derived records...', - 'dct:source': 'Derived records...', + isSourceOf: 'Derived records...', + 'dct:isSourceOf': 'Derived records...', + sourceOf: 'Derived records...', + 'dct:sourceOf': 'Derived records...', + source: 'Source record...', + 'dct:source': 'Source record...', isVersionOf: 'Is version of...', 'dct:isVersionOf': 'Is version of...', hasVersion: 'Has version...', @@ -468,6 +470,10 @@ export function FullDetailsTable({ data }: FullDetailsTableProps) { hasPart: 'dct_isPartOf_sm', 'pcdm:hasMember': 'pcdm_memberOf_sm', hasMember: 'pcdm_memberOf_sm', + 'dct:isSourceOf': 'dct_source_sm', + isSourceOf: 'dct_source_sm', + 'dct:sourceOf': 'dct_source_sm', + sourceOf: 'dct_source_sm', }; const relationshipFacetField = relationshipToFacetField[relationshipType] ??