fix(share): include referenced choreographers in program share bundle so received dances keep author attribution (#412)#418
Merged
Conversation
… so received dances keep author attribution (#412) 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>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes program-share/import author attribution by ensuring referenced choreographers are included in the shared archive and that the receive-side JSON adapter carries those choreographer names through to the import pipeline so author IDs can be resolved/created safely.
Changes:
- App share-bundle builder now embeds only the choreographers referenced by bundled dances (with private contact fields stripped).
- Core
GenericJsonAdapternow preserves referenced choreographers in single-dance payloads and populatesStructuredDraft.authorNamesfrom payload choreographers. - Adds adapter-level, send-side, and end-to-end share/receive tests (including FK-enforced DB round-trip).
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/compendium_core/test/imports/generic_json_adapter_test.dart | Adds regression tests ensuring author names are recovered and commits create/reuse choreographer rows without FK failures. |
| packages/compendium_core/lib/src/imports/generic_json_adapter.dart | Captures archive choreographers during discover, embeds referenced choreographers in fetch payloads, and derives authorNames during parse. |
| app/test/program_share_bundle_test.dart | Expands tests to cover choreographer inclusion, privacy stripping, and full end-to-end receiver attribution behavior. |
| app/lib/src/widgets/program_export_menu.dart | Threads a choreographerFor resolver into the “Share (program + dances)” action. |
| app/lib/src/search/collection_data.dart | Adds a choreographersById map to support share/export author resolution. |
| app/lib/src/screens/program_summary_screen.dart | Supplies choreographerFor from loaded collection data to the export menu. |
| app/lib/src/screens/program_editor_screen.dart | Supplies choreographerFor from loaded collection data to the export menu. |
| app/lib/src/export/program_share_bundle.dart | Builds share bundles that include only referenced choreographers and sanitizes private fields before encoding. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ents (#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>
#412) 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug (#412)
Sharing a program via the OS share sheet (AirDrop, etc.) produced a
.ccsharebundle whose dances kept theirauthorIdsbut whose archive omitted the referenced choreographer records (buildProgramShareBundleleftchoreographersempty). User-visible symptom: a received shared dance imported with no choreographer / author attribution.This is distinct from the already-fixed #410 (slots not resolving to dances): here the dance resolved fine but showed no choreographer.
Root cause — two sides
Send side omitted the choreographers entirely.
Receive side could not have honored them anyway:
GenericJsonAdapterre-serialized each dance into a single-dance archive without choreographers, soparseproduced a draft with the sender's rawauthorIdsbut emptyauthorNames.ImportPipeline.committhen kept those raw ids, which reference choreographer rows the receiver never creates — failing thedance_authorsforeign key (PRAGMA foreign_keys = ON) and dropping the dance. This was a pre-existing, production-affecting bug that also broke the manual "Caller's Compendium JSON" import of any authored dance onto a device lacking those exact ids.Both fixes are required for the program-share round-trip, so they ship together in this one atomic PR.
The fix
Send side (app) —
buildProgramShareBundlecollects the DISTINCT choreographers referenced by the bundled dances'authorIds(deduped by id, stable first-seen order, unresolved id skipped / never fatal) via a new injectedchoreographerForlookup, and embeds only those. Each is sanitized first —email/locationcleared per theChoreographerprivacy contract — so the shared file leaks no private contact data (name/website/notes/deceasedpreserved). Threaded throughProgramExportMenuand both screen call sites; achoreographersByIdmap was added toCollectionData.Receive side (core) —
GenericJsonAdapternow captures the archive's choreographers duringdiscover, embeds the ones a dance references into its self-contained single-dance payload, and populatesStructuredDraft.authorNames(inauthorIdsorder, blanks/unresolved dropped). The existing pipeline name-resolution matches or creates the choreographer row and rewritesauthorIdsto the receiver's own ids — no FK failure.commit's author logic is untouched. Full-DB restore uses a separate path (archive_serviceupserts choreographers directly) and is unaffected.End-to-end verification
A round-trip test builds a bundle from an authored dance, then imports it on a fresh
CompendiumArchiveImporterover the real, FK-enforced database: the dance imports, the program slot resolves (no placeholder), the resolved choreographer name matches the sender, andemail/locationare null.OWASP / untrusted input
The received archive stays untrusted — no
ArchiveIntakeServicevalidation relaxed (size cap, UTF-8, well-formed-archive, forward-schema refusal, never-throw). Choreographer creation now driven by the bundle flows through the same validated decode + parameterized-drift path the manual import already crosses (no new sink): names are data-only, blank/whitespace names dropped, rows bounded by dances×authors under the existing 25 MiB intake cap and deduped by normalized name.Tests
Gates
fvm dart format .— cleanfvm flutter analyze(core + app) — 0 issuesfvm dart test— 1798 passed, 4 skipped (pre-existing)fvm flutter test— 1642 passedRefs #412. Share lineage #298.