From 8748d005938eb65a41da7d789c7008d7e475d0ad Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 16:03:55 -0700 Subject: [PATCH 1/3] fix(share): include referenced choreographers in program share bundle so received dances keep author attribution (#412) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a program is shared via the OS share sheet (AirDrop, etc.), `buildProgramShareBundle` built a CompendiumArchive with the referenced dances but left `choreographers` empty. Bundled dances carried `authorIds` pointing at choreographer records the archive omitted, so on the receiving device a shared dance imported with no choreographer/author attribution. Two coordinated fixes, both required for the round-trip: Send side (app): buildProgramShareBundle now collects the DISTINCT choreographers referenced by the bundled dances' authorIds (deduped by id, stable first-seen order, unresolved id skipped/never fatal) via a new injected `choreographerFor` lookup, and embeds only those. Each is SANITIZED first — email/location are cleared per the Choreographer privacy contract (choreographer.dart:6-9) — so the shared file leaks no private contact data. Threaded through ProgramExportMenu and both screen call sites (a `choreographersById` map was added to CollectionData). Receive side (core): GenericJsonAdapter previously re-serialized each dance into a single-dance archive WITHOUT choreographers, so parse produced a draft with the sender's raw authorIds but empty authorNames; commit then kept those raw ids, which reference choreographer rows the receiver never creates — failing the dance_authors foreign key (PRAGMA foreign_keys = ON) and dropping the dance (a pre-existing, production-affecting bug that also broke manual "Caller's Compendium JSON" import of any authored dance). The adapter now captures the archive's choreographers, embeds the ones a dance references in its self-contained payload, and populates StructuredDraft.authorNames (in authorIds order, blanks/unresolved dropped). The existing pipeline name-resolution then matches or creates the choreographer row and rewrites authorIds to the receiver's own ids — no FK failure. commit's author logic is untouched. Full-DB restore uses a separate path and is unaffected. OWASP: the received archive stays untrusted — no ArchiveIntakeService validation relaxed. Choreographer creation is driven only through the same validated decode + parameterized-drift path the manual import already uses; blank/whitespace names are dropped, rows are bounded by dances x authors under the existing 25 MiB intake cap and deduped by normalized name. Tests: send-side (referenced-only, dedupe-once, no-authors, unresolved-id skipped, email/location stripped), adapter-level class fix (name recovery, commit creates/points-at receiver row with no FK error, reuse-by-name), and full end-to-end program-share round-trip on a fresh CompendiumArchiveImporter over the real FK-enforced database (dance imports, slot resolves, author name matches, email/location null). Refs #412. Share lineage #298. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- app/lib/src/export/program_share_bundle.dart | 31 ++ .../src/screens/program_editor_screen.dart | 1 + .../src/screens/program_summary_screen.dart | 1 + app/lib/src/search/collection_data.dart | 7 + app/lib/src/widgets/program_export_menu.dart | 13 +- app/test/program_share_bundle_test.dart | 324 +++++++++++++++++- .../lib/src/imports/generic_json_adapter.dart | 83 ++++- .../imports/generic_json_adapter_test.dart | 130 +++++++ 8 files changed, 564 insertions(+), 26 deletions(-) diff --git a/app/lib/src/export/program_share_bundle.dart b/app/lib/src/export/program_share_bundle.dart index 009837c4..cb0f12c7 100644 --- a/app/lib/src/export/program_share_bundle.dart +++ b/app/lib/src/export/program_share_bundle.dart @@ -20,11 +20,27 @@ import 'package:compendium_core/compendium_core.dart'; /// program itself (its `slots` carry their own `text`), so nothing extra is /// needed for them here. /// +/// [choreographerFor] resolves an author id (from a bundled dance's `authorIds`) +/// to its full [Choreographer], so author attribution survives the round-trip: +/// the receive-side importer reads incoming author *names* from the bundle's own +/// `CompendiumArchive.choreographers` (a receiver cannot resolve the sender's +/// author ids). Only the choreographers actually referenced by the bundled +/// dances are included (deduped by id, stable first-seen order) — the bundle +/// stays minimal and never leaks unrelated authors. An id that can't be resolved +/// is skipped (best-effort, never fatal — mirrors [danceFor]). +/// +/// Privacy (issue #412, and the [Choreographer] model's contract): a +/// choreographer's `email`/`location` are private contact data that MUST NOT +/// leave the device in a shareable export. Each included choreographer is +/// therefore sanitized here — `email`/`location` are cleared — before it is +/// embedded, so sharing carries only public attribution (name/website/notes). +/// /// [now] stamps the archive's `exportedAt`; it defaults to the current time and /// is injectable for deterministic tests. String buildProgramShareBundle( Program program, { required Dance? Function(String danceId) danceFor, + required Choreographer? Function(String id) choreographerFor, DateTime? now, }) { final dances = []; @@ -36,11 +52,26 @@ String buildProgramShareBundle( if (dance != null) dances.add(dance); } + final choreographers = []; + final seenAuthors = {}; + for (final dance in dances) { + for (final authorId in dance.authorIds) { + if (!seenAuthors.add(authorId)) continue; + final choreographer = choreographerFor(authorId); + if (choreographer == null) continue; + // Strip private contact fields before the record leaves the device. + choreographers.add( + choreographer.copyWith(clearEmail: true, clearLocation: true), + ); + } + } + return encodeArchive( CompendiumArchive( exportedAt: (now ?? DateTime.now()).toUtc(), programs: [program], dances: dances, + choreographers: choreographers, ), ); } diff --git a/app/lib/src/screens/program_editor_screen.dart b/app/lib/src/screens/program_editor_screen.dart index acc21468..a2cae9aa 100644 --- a/app/lib/src/screens/program_editor_screen.dart +++ b/app/lib/src/screens/program_editor_screen.dart @@ -676,6 +676,7 @@ class _ProgramEditorScreenState extends State program: _draftProgram!, titleFor: _titleForDance, danceFor: (id) => _data?.dancesById[id], + choreographerFor: (id) => _data?.choreographersById[id], ), if (!widget.isNew && _existing != null) ...[ if (_slots.any((s) => s.danceId != null)) diff --git a/app/lib/src/screens/program_summary_screen.dart b/app/lib/src/screens/program_summary_screen.dart index 27f272a3..b7f17b44 100644 --- a/app/lib/src/screens/program_summary_screen.dart +++ b/app/lib/src/screens/program_summary_screen.dart @@ -343,6 +343,7 @@ class _ProgramSummaryPaneState extends State { program: program, titleFor: (id) => _danceTitles[id], danceFor: (id) => _dances[id], + choreographerFor: (id) => _collectionData?.choreographersById[id], ), if (program.slots.any((s) => s.danceId != null)) IconButton( diff --git a/app/lib/src/search/collection_data.dart b/app/lib/src/search/collection_data.dart index d8628aeb..b2f887ea 100644 --- a/app/lib/src/search/collection_data.dart +++ b/app/lib/src/search/collection_data.dart @@ -13,6 +13,7 @@ import '../models/dance_list_entry.dart'; class CollectionData { CollectionData({ required this.dancesById, + required this.choreographersById, required this.choreographerNames, required this.tagNames, required this.customFieldDefs, @@ -38,6 +39,10 @@ class CollectionData { }); final Map dancesById; + + /// All choreographers keyed by id, so a share/export path can resolve a + /// dance's `authorIds` to full [Choreographer] records (mirrors [dancesById]). + final Map choreographersById; final Map choreographerNames; final Map tagNames; final List customFieldDefs; @@ -90,6 +95,7 @@ class CollectionData { final callCounts = await repos.programs.countByDance(); final dancesById = {for (final d in dances) d.id: d}; + final choreographersById = {for (final c in choreographers) c.id: c}; final choreographerNames = {for (final c in choreographers) c.id: c.name}; final tagNames = {for (final t in tags) t.id: t.name}; @@ -133,6 +139,7 @@ class CollectionData { return CollectionData( dancesById: dancesById, + choreographersById: choreographersById, choreographerNames: choreographerNames, tagNames: tagNames, customFieldDefs: defs, diff --git a/app/lib/src/widgets/program_export_menu.dart b/app/lib/src/widgets/program_export_menu.dart index 0635e95a..5b6fb5c6 100644 --- a/app/lib/src/widgets/program_export_menu.dart +++ b/app/lib/src/widgets/program_export_menu.dart @@ -83,6 +83,7 @@ class ProgramExportMenu extends StatelessWidget { required this.program, required this.titleFor, this.danceFor, + this.choreographerFor, this.shareInvoker, this.bundleFileWriter, this.pdfLayouter, @@ -96,6 +97,12 @@ class ProgramExportMenu extends StatelessWidget { /// bundle. When `null`, that action is omitted (there is nothing to embed). final Dance? Function(String danceId)? danceFor; + /// Resolves a dance's author id to its full [Choreographer], so the "Share + /// (program + dances)" action can embed the choreographers its bundled dances + /// reference and author attribution survives the round-trip. Optional and + /// best-effort: an unresolved id is simply omitted from the bundle. + final Choreographer? Function(String id)? choreographerFor; + /// Test seam for the share call; defaults to [SharePlus.instance.share]. final ShareInvoker? shareInvoker; @@ -142,7 +149,11 @@ class ProgramExportMenu extends StatelessWidget { final resolveDance = danceFor; if (resolveDance == null) return; - final json = buildProgramShareBundle(program, danceFor: resolveDance); + final json = buildProgramShareBundle( + program, + danceFor: resolveDance, + choreographerFor: choreographerFor ?? (_) => null, + ); final fileName = programShareBundleFileName(program.title); final writeFile = bundleFileWriter ?? writeBundleTempFile; diff --git a/app/test/program_share_bundle_test.dart b/app/test/program_share_bundle_test.dart index b5d8a4be..ea16fdc7 100644 --- a/app/test/program_share_bundle_test.dart +++ b/app/test/program_share_bundle_test.dart @@ -1,21 +1,35 @@ +import 'dart:convert'; +import 'dart:typed_data'; + +import 'package:compendium_app/src/data/archive_intake_service.dart'; import 'package:compendium_app/src/export/program_share_bundle.dart'; import 'package:compendium_core/compendium_core.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'support/test_repositories.dart'; + final _now = DateTime.utc(2026, 1, 1); -Dance _dance(String id, String title) => Dance( - id: id, - title: title, - authorIds: const [], - figures: [ - Figure(move: 'swing', params: {'beats': 16, 'who': 'partners'}), - ], - sourceCitations: const [], - customFields: const [], - createdAt: _now, - updatedAt: _now, -); +Dance _dance(String id, String title, {List authorIds = const []}) => + Dance( + id: id, + title: title, + authorIds: authorIds, + figures: [ + Figure(move: 'swing', params: {'beats': 16, 'who': 'partners'}), + ], + sourceCitations: const [], + customFields: const [], + createdAt: _now, + updatedAt: _now, + ); + +Choreographer _choreographer( + String id, + String name, { + String? email, + String? location, +}) => Choreographer(id: id, name: name, email: email, location: location); ProgramSlot _slot( int position, { @@ -63,6 +77,7 @@ void main() { 'd3': _dance('d3', 'Chinese New Year'), }; Dance? danceFor(String id) => catalog[id]; + Choreographer? choreographerFor(String id) => null; test('embeds every referenced dance, deduped, and preserves the program', () { final program = _program( @@ -77,6 +92,7 @@ void main() { final json = buildProgramShareBundle( program, danceFor: danceFor, + choreographerFor: choreographerFor, now: _now, ); final archive = decodeArchive(json).archive; @@ -107,7 +123,11 @@ void main() { ); final archive = decodeArchive( - buildProgramShareBundle(program, danceFor: danceFor), + buildProgramShareBundle( + program, + danceFor: danceFor, + choreographerFor: choreographerFor, + ), ).archive; expect(archive.dances.map((d) => d.id), ['d1']); @@ -122,7 +142,11 @@ void main() { ); final archive = decodeArchive( - buildProgramShareBundle(program, danceFor: danceFor), + buildProgramShareBundle( + program, + danceFor: danceFor, + choreographerFor: choreographerFor, + ), ).archive; expect(archive.dances, isEmpty); @@ -144,6 +168,7 @@ void main() { final json = buildProgramShareBundle( program, danceFor: danceFor, + choreographerFor: choreographerFor, now: _now, ); final imported = await _importedDances(json); @@ -157,6 +182,140 @@ void main() { ); }, ); + + group('choreographers (author attribution, #412)', () { + final authored = { + 'd1': _dance('d1', 'Rory O\'More', authorIds: ['c1']), + 'd2': _dance('d2', 'The Nice Combination', authorIds: ['c1', 'c2']), + 'd3': _dance('d3', 'Chinese New Year', authorIds: ['c-missing']), + 'd4': _dance('d4', 'Anonymous Reel'), + }; + Dance? authoredDanceFor(String id) => authored[id]; + + final choreographers = { + 'c1': _choreographer( + 'c1', + 'Cary Ravitz', + email: 'cary@example.com', + location: 'Lexington, KY', + ), + 'c2': _choreographer('c2', 'Tom Hinds'), + }; + Choreographer? choreographerCatalogFor(String id) => choreographers[id]; + + test('includes only choreographers referenced by bundled dances', () { + final program = _program( + slots: [ + _slot(0, danceId: 'd1'), + _slot(1, danceId: 'd2'), + ], + ); + + final archive = decodeArchive( + buildProgramShareBundle( + program, + danceFor: authoredDanceFor, + choreographerFor: choreographerCatalogFor, + now: _now, + ), + ).archive; + + expect(archive.choreographers.map((c) => c.id).toSet(), {'c1', 'c2'}); + }); + + test('a choreographer shared by several dances appears exactly once', () { + // d1 -> c1, d2 -> c1 + c2. c1 is referenced twice but must not double. + final program = _program( + slots: [ + _slot(0, danceId: 'd1'), + _slot(1, danceId: 'd2'), + _slot(2, danceId: 'd1'), // repeated dance id too + ], + ); + + final archive = decodeArchive( + buildProgramShareBundle( + program, + danceFor: authoredDanceFor, + choreographerFor: choreographerCatalogFor, + now: _now, + ), + ).archive; + + expect( + archive.choreographers.where((c) => c.id == 'c1').length, + 1, + reason: 'a choreographer referenced by multiple dances is deduped', + ); + expect(archive.choreographers.length, 2); + }); + + test('a dance with no authors bundles fine with no choreographers', () { + final program = _program(slots: [_slot(0, danceId: 'd4')]); + + final archive = decodeArchive( + buildProgramShareBundle( + program, + danceFor: authoredDanceFor, + choreographerFor: choreographerCatalogFor, + now: _now, + ), + ).archive; + + expect(archive.dances.map((d) => d.id), ['d4']); + expect(archive.choreographers, isEmpty); + }); + + test('an unresolvable author id is skipped, never fatal', () { + // d3 references 'c-missing', which choreographerFor cannot resolve. + final program = _program( + slots: [ + _slot(0, danceId: 'd1'), + _slot(1, danceId: 'd3'), + ], + ); + + final archive = decodeArchive( + buildProgramShareBundle( + program, + danceFor: authoredDanceFor, + choreographerFor: choreographerCatalogFor, + now: _now, + ), + ).archive; + + expect(archive.dances.map((d) => d.id).toSet(), {'d1', 'd3'}); + expect( + archive.choreographers.map((c) => c.id), + ['c1'], + reason: 'the unresolved author id is dropped, the dance still ships', + ); + }); + + test('strips private email/location from shared choreographers', () { + final program = _program(slots: [_slot(0, danceId: 'd1')]); + + final json = buildProgramShareBundle( + program, + danceFor: authoredDanceFor, + choreographerFor: choreographerCatalogFor, + now: _now, + ); + final shared = decodeArchive(json).archive.choreographers.single; + + expect(shared.id, 'c1'); + expect(shared.name, 'Cary Ravitz', reason: 'attribution is preserved'); + expect(shared.email, isNull, reason: 'private contact is not shared'); + expect( + shared.location, + isNull, + reason: 'private contact is not shared', + ); + // Defense in depth: the raw JSON must not carry the private fields. + expect(json.contains('cary@example.com'), isFalse); + expect(json.contains('Lexington, KY'), isFalse); + }); + }); }); group('programShareBundleFileName', () { @@ -190,4 +349,141 @@ void main() { expect(name, endsWith('.ccshare')); }); }); + + // The whole point of #412: verify on the RECEIVE side that the choreographers + // the builder now embeds actually restore author attribution end-to-end, via + // the real shared receive path (ArchiveIntakeService -> CompendiumArchive + // importer -> ImportPipeline) over the real CompendiumDatabase (FK enforced). + group('end-to-end author attribution on the receiver (#412)', () { + Future receive( + CompendiumRepositories repos, + String json, + ) async { + final intake = ArchiveIntakeService( + repositories: repos, + now: () => DateTime.utc(2026, 7, 20), + ); + return intake.importBytes(Uint8List.fromList(utf8.encode(json))); + } + + Future danceByTitle( + CompendiumRepositories repos, + String title, + ) async => + (await repos.dances.listAll()).firstWhere((d) => d.title == title); + + Future> authorNamesOf( + CompendiumRepositories repos, + Dance dance, + ) async { + final names = []; + for (final id in dance.authorIds) { + final c = await repos.choreographers.getById(id); + if (c != null) names.add(c.name); + } + return names; + } + + test('a received authored dance keeps its choreographer', () async { + final repos = openTestRepositories(); + addTearDown(repos.db.close); + + final program = _program(slots: [_slot(0, danceId: 'd1')]); + final json = buildProgramShareBundle( + program, + danceFor: (id) => + id == 'd1' ? _dance('d1', 'Rory O\'More', authorIds: ['c1']) : null, + choreographerFor: (id) => id == 'c1' + ? _choreographer( + 'c1', + 'Cary Ravitz', + email: 'cary@example.com', + location: 'Lexington, KY', + ) + : null, + now: _now, + ); + + final result = await receive(repos, json); + expect(result.isImported, isTrue, reason: result.message); + + final imported = await danceByTitle(repos, 'Rory O\'More'); + expect( + await authorNamesOf(repos, imported), + ['Cary Ravitz'], + reason: 'the received dance is attributed, not authorless', + ); + + // The program slot resolves to the imported dance (no placeholder). + final importedProgram = (await repos.programs.listAll()).single; + expect(importedProgram.slots.single.danceId, imported.id); + expect( + result.issues.where( + (i) => i.code == 'archive_program_unresolved_dance', + ), + isEmpty, + ); + + // The received choreographer carries no private contact data. + final author = await repos.choreographers.getById( + imported.authorIds.single, + ); + expect(author!.email, isNull); + expect(author.location, isNull); + }); + + test('reuses a choreographer the receiver already has, by name', () async { + final repos = openTestRepositories(); + addTearDown(repos.db.close); + // The receiver already knows this author under a DIFFERENT id. + await repos.choreographers.upsert( + Choreographer(id: 'local-cary', name: 'Cary Ravitz'), + ); + + final program = _program(slots: [_slot(0, danceId: 'd1')]); + final json = buildProgramShareBundle( + program, + danceFor: (id) => id == 'd1' + ? _dance('d1', 'Rory O\'More', authorIds: ['sender-cary']) + : null, + choreographerFor: (id) => + id == 'sender-cary' ? _choreographer(id, 'Cary Ravitz') : null, + now: _now, + ); + + final result = await receive(repos, json); + expect(result.isImported, isTrue, reason: result.message); + + final imported = await danceByTitle(repos, 'Rory O\'More'); + expect(imported.authorIds, ['local-cary'], reason: 'matched by name'); + expect( + await repos.choreographers.listAll(), + hasLength(1), + reason: 'no duplicate choreographer created', + ); + }); + + test( + 'a received dance with no authors imports fine, unattributed', + () async { + final repos = openTestRepositories(); + addTearDown(repos.db.close); + + final program = _program(slots: [_slot(0, danceId: 'd1')]); + final json = buildProgramShareBundle( + program, + danceFor: (id) => id == 'd1' ? _dance('d1', 'Anonymous Reel') : null, + choreographerFor: (_) => null, + now: _now, + ); + + final result = await receive(repos, json); + expect(result.isImported, isTrue, reason: result.message); + + final imported = await danceByTitle(repos, 'Anonymous Reel'); + expect(imported.authorIds, isEmpty); + expect(await repos.choreographers.listAll(), isEmpty); + }, + ); + }); } diff --git a/packages/compendium_core/lib/src/imports/generic_json_adapter.dart b/packages/compendium_core/lib/src/imports/generic_json_adapter.dart index 22667bba..0fefa065 100644 --- a/packages/compendium_core/lib/src/imports/generic_json_adapter.dart +++ b/packages/compendium_core/lib/src/imports/generic_json_adapter.dart @@ -1,3 +1,4 @@ +import '../model/choreographer.dart'; import '../model/dance.dart'; import '../model/enums.dart'; import '../serialization/archive_codec.dart'; @@ -50,6 +51,16 @@ class GenericJsonAdapter implements SourceAdapter { /// consults this — it works from the [RawRecord] alone. final Map _dancesById = {}; + /// Choreographers seen during [discover], keyed by their archive id, so + /// [fetch] can embed the ones a dance references into its self-contained + /// single-dance payload. A received bundle credits its dances by author id, + /// but the receiver cannot resolve the sender's ids — so [parse] turns these + /// into author *names* on the draft, which the [ImportPipeline] resolves to + /// (or creates) real [Choreographer] rows at commit. Without this the dance's + /// `authorIds` would reference choreographer rows that never get created, + /// failing the `dance_authors` foreign key and dropping the dance (#412). + final Map _choreographersById = {}; + /// The archive schema version seen during [discover], echoed onto each /// [RawRecord.sourceVersion] and used to re-serialize single-dance archives. int _schemaVersion = archiveSchemaVersion; @@ -59,6 +70,7 @@ class GenericJsonAdapter implements SourceAdapter { // Reset discovery state up front so a failed attempt never leaves stale // records fetchable from a prior successful discover on this instance. _dancesById.clear(); + _choreographersById.clear(); _schemaVersion = archiveSchemaVersion; final payload = request.payload; @@ -85,6 +97,9 @@ class GenericJsonAdapter implements SourceAdapter { } _dancesById.addEntries(result.archive.dances.map((d) => MapEntry(d.id, d))); + _choreographersById.addEntries( + result.archive.choreographers.map((c) => MapEntry(c.id, c)), + ); _schemaVersion = result.archive.schemaVersion; return [ @@ -121,11 +136,31 @@ class GenericJsonAdapter implements SourceAdapter { source: source, externalId: record.externalId, sourceVersion: '$_schemaVersion', - payload: _encodeSingleDance(dance, _schemaVersion), + payload: _encodeSingleDance( + dance, + _referencedChoreographers(dance), + _schemaVersion, + ), contentType: 'application/json', ); } + /// The choreographers this [dance] credits, in its `authorIds` order, deduped + /// by id and skipping ids that were not present in the discovered archive + /// (best-effort — an unresolved author id is simply dropped, never fatal). The + /// single-dance payload carries ONLY these, keeping it minimal and leaking no + /// unrelated authors. + List _referencedChoreographers(Dance dance) { + final referenced = []; + final seen = {}; + for (final id in dance.authorIds) { + if (!seen.add(id)) continue; + final choreographer = _choreographersById[id]; + if (choreographer != null) referenced.add(choreographer); + } + return referenced; + } + @override StructuredDraft parse(RawRecord raw) { final result = decodeArchive(raw.payload); @@ -170,12 +205,32 @@ class GenericJsonAdapter implements SourceAdapter { ), ]; + // Recover author display names from the payload's own choreographers (a + // received bundle credits by author id, but the receiver cannot resolve the + // sender's ids). Names are emitted in the dance's `authorIds` order, deduped + // by id, skipping ids with no choreographer and blank/whitespace-only names + // (the archive is untrusted input; a blank name would throw in the + // Choreographer model, and parse must never fail a dance). The pipeline + // resolves each name to an existing row or creates one at commit. + final dance = dances.single; + final nameById = { + for (final c in result.archive.choreographers) c.id: c.name, + }; + final authorNames = []; + final seenAuthorIds = {}; + for (final id in dance.authorIds) { + if (!seenAuthorIds.add(id)) continue; + final name = nameById[id]?.trim(); + if (name != null && name.isNotEmpty) authorNames.add(name); + } + // The draft carries no provenance — the pipeline attaches it at commit, // derived from `raw` (including the externalId that keys exact dedupe). return StructuredDraft( - dance: _withoutProvenance(dances.single), + dance: _withoutProvenance(dance), raw: raw, issues: issues, + authorNames: authorNames, ); } @@ -191,15 +246,21 @@ class GenericJsonAdapter implements SourceAdapter { } /// Encodes [dance] as a minimal, self-contained single-dance archive so the - /// payload is fully decodable by [parse] on its own. - static String _encodeSingleDance(Dance dance, int schemaVersion) => - encodeArchive( - CompendiumArchive( - schemaVersion: schemaVersion, - exportedAt: DateTime.fromMillisecondsSinceEpoch(0, isUtc: true), - dances: [dance], - ), - ); + /// payload is fully decodable by [parse] on its own. [choreographers] are the + /// dance's referenced authors, carried alongside so [parse] can recover their + /// display names (the pipeline resolves names to ids at commit). + static String _encodeSingleDance( + Dance dance, + List choreographers, + int schemaVersion, + ) => encodeArchive( + CompendiumArchive( + schemaVersion: schemaVersion, + exportedAt: DateTime.fromMillisecondsSinceEpoch(0, isUtc: true), + dances: [dance], + choreographers: choreographers, + ), + ); /// Strips any embedded provenance from an imported dance; the pipeline owns /// provenance and re-derives it from the [RawRecord] at commit time. diff --git a/packages/compendium_core/test/imports/generic_json_adapter_test.dart b/packages/compendium_core/test/imports/generic_json_adapter_test.dart index ebeae236..e5f94dfc 100644 --- a/packages/compendium_core/test/imports/generic_json_adapter_test.dart +++ b/packages/compendium_core/test/imports/generic_json_adapter_test.dart @@ -10,6 +10,11 @@ import '../storage/test_database.dart'; final _now = DateTime.utc(2026, 7, 15); +String Function() sequentialIds(String prefix) { + var n = 0; + return () => '$prefix-${++n}'; +} + Dance _dance( String id, String title, { @@ -278,5 +283,130 @@ void main() { expect(verdict.kind, DedupeKind.reimport); expect(verdict.targetDanceId, 'existing-99'); }); + + // #412: the adapter must carry the referenced choreographers' display names + // through parse so the pipeline can resolve/create author rows on commit. + // Without this, a received authored dance's raw authorIds reference + // choreographer rows the receiver never creates -> FK 787 -> dance dropped. + group('author attribution round-trip (#412)', () { + test( + 'parse recovers author names from the payload choreographers', + () async { + final json = encodeArchive( + _archive([ + _dance('d1', 'Give and Take', authorIds: ['c1']), + ]), + ); + final adapter = GenericJsonAdapter(); + final discovered = await adapter.discover( + ImportRequest(payload: json), + ); + final draft = await _importOne(adapter, discovered.single); + + // The draft credits by NAME (resolvable on any receiver), not the + // sender's opaque id; dance.authorIds is left untouched. + expect(draft.authorNames, ['Cary Ravitz']); + expect(draft.dance.authorIds, ['c1']); + }, + ); + + test( + 'an author id with no matching choreographer yields no name', + () async { + // authorIds references 'ghost', absent from the archive's choreographers. + final json = encodeArchive( + _archive([ + _dance('d1', 'Orphaned', authorIds: ['ghost']), + ]), + ); + final adapter = GenericJsonAdapter(); + final discovered = await adapter.discover( + ImportRequest(payload: json), + ); + final draft = await _importOne(adapter, discovered.single); + + expect(draft.authorNames, isEmpty); + expect( + draft.dance.title, + 'Orphaned', + reason: 'still parses, never fatal', + ); + }, + ); + + test('committing creates the choreographer row and points authorIds at it ' + 'with no FK failure', () async { + final db = openTestDatabase(); + addTearDown(db.close); + final dances = DanceRepository(db, contraTaxonomy); + final choreographers = ChoreographerRepository(db); + final pipeline = ImportPipeline(dances, choreographers); + + final json = encodeArchive( + _archive([ + _dance('d1', 'Give and Take', authorIds: ['c1']), + ]), + ); + final batch = await pipeline.plan( + GenericJsonAdapter(), + ImportRequest(payload: json), + ); + final session = await pipeline.commit( + batch, + now: _now, + newId: sequentialIds('new'), + ); + + expect(session.committedCount, 1); + final imported = await dances.getById(session.insertedDanceIds.single); + // authorIds now reference the RECEIVER's own row (not the sender's 'c1'). + expect(imported!.authorIds, isNot(contains('c1'))); + expect(imported.authorIds, hasLength(1)); + final author = await choreographers.getById(imported.authorIds.single); + expect(author!.name, 'Cary Ravitz'); + }); + + test( + 'reuses a choreographer the receiver already has, matched by name', + () async { + final db = openTestDatabase(); + addTearDown(db.close); + final dances = DanceRepository(db, contraTaxonomy); + final choreographers = ChoreographerRepository(db); + final pipeline = ImportPipeline(dances, choreographers); + // Receiver knows this author under a DIFFERENT id than the sender. + await choreographers.upsert( + Choreographer(id: 'local-cary', name: 'Cary Ravitz'), + ); + + final json = encodeArchive( + _archive([ + _dance('d1', 'Give and Take', authorIds: ['c1']), + ]), + ); + final batch = await pipeline.plan( + GenericJsonAdapter(), + ImportRequest(payload: json), + ); + final session = await pipeline.commit( + batch, + now: _now, + newId: sequentialIds('new'), + ); + + final imported = await dances.getById( + session.insertedDanceIds.single, + ); + expect(imported!.authorIds, [ + 'local-cary', + ], reason: 'matched by name'); + expect( + await choreographers.listAll(), + hasLength(1), + reason: 'no duplicate', + ); + }, + ); + }); }); } From afa57cac60406ac3bbea42c398ba1eabf609d897 Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 16:09:26 -0700 Subject: [PATCH 2/3] docs(share): correct choreographer ordering note in share-bundle comments (#412) encodeArchive canonicalizes entities by id, so the emitted bundle's choreographer order is by id, not reference/first-seen order. Only the SET of included choreographers is significant (parse recovers author names in the dance's authorIds order directly from the decoded dance). Doc-only; no behavior change. Addresses Copilot review feedback on #418. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- app/lib/src/export/program_share_bundle.dart | 9 ++++++--- .../lib/src/imports/generic_json_adapter.dart | 12 +++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/lib/src/export/program_share_bundle.dart b/app/lib/src/export/program_share_bundle.dart index cb0f12c7..ec143e7a 100644 --- a/app/lib/src/export/program_share_bundle.dart +++ b/app/lib/src/export/program_share_bundle.dart @@ -25,9 +25,12 @@ import 'package:compendium_core/compendium_core.dart'; /// the receive-side importer reads incoming author *names* from the bundle's own /// `CompendiumArchive.choreographers` (a receiver cannot resolve the sender's /// author ids). Only the choreographers actually referenced by the bundled -/// dances are included (deduped by id, stable first-seen order) — the bundle -/// stays minimal and never leaks unrelated authors. An id that can't be resolved -/// is skipped (best-effort, never fatal — mirrors [danceFor]). +/// dances are included (deduped by id) — the bundle stays minimal and never +/// leaks unrelated authors. (The emitted archive is canonicalized by +/// [encodeArchive], which sorts entities by id, so the serialized order is by +/// id rather than reference order — only the set of included choreographers is +/// significant.) An id that can't be resolved is skipped (best-effort, never +/// fatal — mirrors [danceFor]). /// /// Privacy (issue #412, and the [Choreographer] model's contract): a /// choreographer's `email`/`location` are private contact data that MUST NOT diff --git a/packages/compendium_core/lib/src/imports/generic_json_adapter.dart b/packages/compendium_core/lib/src/imports/generic_json_adapter.dart index 0fefa065..60d780fd 100644 --- a/packages/compendium_core/lib/src/imports/generic_json_adapter.dart +++ b/packages/compendium_core/lib/src/imports/generic_json_adapter.dart @@ -145,11 +145,13 @@ class GenericJsonAdapter implements SourceAdapter { ); } - /// The choreographers this [dance] credits, in its `authorIds` order, deduped - /// by id and skipping ids that were not present in the discovered archive - /// (best-effort — an unresolved author id is simply dropped, never fatal). The - /// single-dance payload carries ONLY these, keeping it minimal and leaking no - /// unrelated authors. + /// The choreographers this [dance] credits — the ones whose ids appear in its + /// `authorIds`, deduped by id, skipping ids that were not present in the + /// discovered archive (best-effort — an unresolved author id is simply + /// dropped, never fatal). The single-dance payload carries ONLY these, keeping + /// it minimal and leaking no unrelated authors. ([parse] recovers names in + /// `authorIds` order directly from the decoded dance, so the order of this + /// list — which [encodeArchive] canonicalizes by id anyway — is irrelevant.) List _referencedChoreographers(Dance dance) { final referenced = []; final seen = {}; From 7c291ba5401b5d2523f6d214a145aa976a7c87d2 Mon Sep 17 00:00:00 2001 From: Isaac Banner Date: Mon, 20 Jul 2026 16:27:06 -0700 Subject: [PATCH 3/3] refactor(share): extract reusable sanitizeChoreographerForShare helper (#412) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The send-side redaction was an inline copyWith(clearEmail, clearLocation) in buildProgramShareBundle. The Choreographer privacy contract (choreographer.dart:6-9) covers all program/dance share paths, and the forthcoming dance-share path (#298) must apply the identical redaction — an inline copyWith is exactly the kind of guarantee that gets silently forgotten when that path is added. Extract it into a top-level, reusable `sanitizeChoreographerForShare` in a new export/share_sanitization.dart, doc-commented against the privacy contract (clears email+location; preserves name/website/notes/deceased/identity). buildProgramShareBundle now calls it. Add a direct unit test pinning the guarantee independently of the bundle path (email+location cleared; public fields preserved; no-op when no contact data). No behavior change to the bundle. Addresses tech-lead review on #418. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- app/lib/src/export/program_share_bundle.dart | 6 +-- app/lib/src/export/share_sanitization.dart | 18 +++++++ app/test/share_sanitization_test.dart | 51 ++++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) create mode 100644 app/lib/src/export/share_sanitization.dart create mode 100644 app/test/share_sanitization_test.dart diff --git a/app/lib/src/export/program_share_bundle.dart b/app/lib/src/export/program_share_bundle.dart index ec143e7a..5876d13b 100644 --- a/app/lib/src/export/program_share_bundle.dart +++ b/app/lib/src/export/program_share_bundle.dart @@ -1,5 +1,7 @@ import 'package:compendium_core/compendium_core.dart'; +import 'share_sanitization.dart'; + /// Builds a self-contained "share this program + its dances" bundle (ROADMAP /// §4.3, issue #298 — AirDrop/OS share-sheet sharing, send side). /// @@ -63,9 +65,7 @@ String buildProgramShareBundle( final choreographer = choreographerFor(authorId); if (choreographer == null) continue; // Strip private contact fields before the record leaves the device. - choreographers.add( - choreographer.copyWith(clearEmail: true, clearLocation: true), - ); + choreographers.add(sanitizeChoreographerForShare(choreographer)); } } diff --git a/app/lib/src/export/share_sanitization.dart b/app/lib/src/export/share_sanitization.dart new file mode 100644 index 00000000..bf8b9617 --- /dev/null +++ b/app/lib/src/export/share_sanitization.dart @@ -0,0 +1,18 @@ +import 'package:compendium_core/compendium_core.dart'; + +/// Redacts a [Choreographer]'s private contact data for inclusion in a +/// **shareable** export (any program/dance share path). +/// +/// The [Choreographer] model documents (choreographer.dart:6-9) that [email] +/// and [location] are private contact data that are safe in the user's own +/// full-DB snapshot/backup but MUST NOT be emitted in any shareable export. +/// This helper enforces that at the single send-side choke point: it clears +/// `email` and `location` while preserving the public attribution fields +/// (`name`, `website`, `notes`, `deceased`, and identity/timestamps). +/// +/// It is intentionally a top-level, reusable function (not private to any one +/// builder): the program-share bundle uses it today, and the forthcoming +/// dance-share path (issue #298) must apply the identical redaction rather than +/// re-deriving an inline copyWith that could silently drift. +Choreographer sanitizeChoreographerForShare(Choreographer choreographer) => + choreographer.copyWith(clearEmail: true, clearLocation: true); diff --git a/app/test/share_sanitization_test.dart b/app/test/share_sanitization_test.dart new file mode 100644 index 00000000..88a35dc4 --- /dev/null +++ b/app/test/share_sanitization_test.dart @@ -0,0 +1,51 @@ +import 'package:compendium_app/src/export/share_sanitization.dart'; +import 'package:compendium_core/compendium_core.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() { + group('sanitizeChoreographerForShare', () { + test('clears private contact fields (email + location)', () { + final sanitized = sanitizeChoreographerForShare( + Choreographer( + id: 'c1', + name: 'Cary Ravitz', + email: 'cary@example.com', + location: 'Lexington, KY', + ), + ); + + expect(sanitized.email, isNull); + expect(sanitized.location, isNull); + }); + + test('preserves public attribution fields and identity', () { + final sanitized = sanitizeChoreographerForShare( + Choreographer( + id: 'c1', + name: 'Cary Ravitz', + website: 'https://ravitz.example', + notes: 'Prolific New England composer.', + email: 'cary@example.com', + location: 'Lexington, KY', + deceased: true, + ), + ); + + expect(sanitized.id, 'c1'); + expect(sanitized.name, 'Cary Ravitz'); + expect(sanitized.website, 'https://ravitz.example'); + expect(sanitized.notes, 'Prolific New England composer.'); + expect(sanitized.deceased, isTrue); + }); + + test('is a no-op for a choreographer with no contact data', () { + final sanitized = sanitizeChoreographerForShare( + Choreographer(id: 'c1', name: 'Anonymous'), + ); + + expect(sanitized.email, isNull); + expect(sanitized.location, isNull); + expect(sanitized.name, 'Anonymous'); + }); + }); +}