Skip to content

fix(formulus): attachment synchronization refactor#612

Merged
najuna-brian merged 6 commits intoOpenDataEnsemble:devfrom
HelloSapiens:fix/attachment-handling-refactor
Apr 17, 2026
Merged

fix(formulus): attachment synchronization refactor#612
najuna-brian merged 6 commits intoOpenDataEnsemble:devfrom
HelloSapiens:fix/attachment-handling-refactor

Conversation

@r0ssing
Copy link
Copy Markdown
Contributor

@r0ssing r0ssing commented Apr 17, 2026

Description

Restores attachment synchronization end-to-end and hardens a few adjacent regressions that surfaced alongside it.

Primary bug — drafts never promoted. FormplayerModal.handleSubmission stopped calling commitDraftAttachmentsAfterSave after a prior refactor, so freshly captured attachments remained in attachments/draft/ and were never queued for upload. Submission now flows through a new persistObservationWithAttachments helper that saves the observation and promotes draft attachments atomically. A best-effort stale-draft sweep also runs on app start.

Attachment folder layout v2. The on-disk layout is now explicitly attachments/{draft,pending,synced}/ with a one-shot migration (runAttachmentLayoutMigrationV2) that relocates legacy top-level committed files into synced/ and pending_upload/ into pending/. All path helpers, the WebView URL resolver (draft → synced → pending lookup order), RepositoryRecoveryService, ServerSwitchService, AttachmentExportService, and SyncScreen pending-upload counter were updated accordingly. FormulusInterfaceDefinition docstrings for getAttachmentUri / getAttachmentsUri and the generated formulus-api.js shim reflect the new layout (breaking change for getAttachmentsUri, which now returns synced/).

Efficiency & robustness.

  • Upload path is now idempotent via a HEAD-before-PUT check, safely recovering from crash-before-unlink.
  • Download path uses the existing monotonic @last_attachment_version cursor so the happy path never re-scans synced files, but downloadRawFile(s) now accepts { overwrite: true } so corrupted/stale locals are always replaced.
  • Dead code removed (saveNewAttachment, downloadAttachments).

Side quest 1 — false "server was reset" on fresh install. Formulus now omits x-repository-generation when the AsyncStorage key is missing and adopts the server's value on first response. Synkronus handlers (sync, attachment_manifest, attachment) use a new ParseClientRepositoryGenerationSent helper to distinguish "not sent" from "explicit 1" and skip the 409 for fresh clients. SyncScreen also silently recovers on 409 when local state is empty.

Side quest 2 — missing server-URL change warning. SettingsScreen.handleServerSwitchIfNeeded now reads the previous URL from serverConfigService.getServerUrl() (authoritative) instead of un-hydrated component state. New ServerSwitchDecision helpers (resolvePreviousServerUrl, classifyServerChange) encapsulate the logic and are unit tested.

E2E design artifacts (no wired tests yet). Adds e2e/README.md (three-layer plan: Synkronus contract, headless Formulus, full-stack RN), e2e/docker-compose.e2e.yml skeleton (Postgres + Synkronus with test-only creds on distinct ports), and a workflow_dispatch-only e2e-attachments.yml GitHub Actions stub.

Type of Change

  • Bug Fix
  • New Feature / Enhancement
  • Refactor / Code Cleanup
  • Documentation Update
  • Maintenance / Chore
  • Other (please specify):

Component(s) Affected

  • formulus (React Native mobile app)
  • formulus-formplayer (React web app)
  • synkronus (Go backend server)
  • synkronus-cli (Command-line utility)
  • Documentation
  • DevOps / CI/CD
  • Other: WebView bridge contract (FormulusInterfaceDefinition + formulus-api.js)

Related Issue(s)

Closes/Fixes/Resolves:


Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manually tested
  • Tested on multiple platforms (if applicable)
  • Not applicable

Details:

  • Formulus Jest suite (34 tests) — attachmentStorage (incl. v2 migration), ServerSwitchDecision, FormplayerModal submission promotion.
  • Synkronus Go tests — pkg/sync/generation_test.go (ParseClientRepositoryGenerationSent), internal/handlers/sync_repository_generation_test.go (missing header → 200), pkg/attachment/manifest_split_cursor_test.go (latest-op, split cursor, hard reset wipes ops+files).
  • go build ./... clean; no new TypeScript or lint errors introduced (pre-existing config.basePath and uploadAttachment TS errors noted but unchanged).

Breaking Changes

  • This PR introduces breaking changes
  • This PR does NOT introduce breaking changes

If breaking changes, please describe migration steps:

  • FormulusAPI.getAttachmentsUri now points at attachments/synced/ rather than the root attachments/ folder. Custom apps that enumerated the old root must update to the new lookup order (draft → synced → pending) via getAttachmentUri(id).
  • On-device attachment layout changes from flat attachments/* + attachments/pending_upload/ to attachments/{draft,pending,synced}/. A one-shot migration runs automatically on next app start (guarded by ATTACHMENTS_LAYOUT_V2_KEY); no user action required.

Documentation Updates

  • Documentation has been updated
  • Documentation update is not required

(FormulusInterfaceDefinition docstrings, formulus-api.js JSDoc, e2e/README.md design doc.)


Checklist

  • Code follows project style guidelines
  • All existing tests pass
  • New tests added for new functionality
  • PR title follows Conventional Commits format

Thank you for contributing to Open Data Ensemble (ODE)!

@r0ssing r0ssing requested a review from najuna-brian April 17, 2026 08:46
@najuna-brian najuna-brian merged commit 56974be into OpenDataEnsemble:dev Apr 17, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants