Skip to content

feat(messaging): message attachments — upload with progress, preview and secure download (#23) - #135

Merged
afeezorobsco-cyber merged 1 commit into
DocForum:mainfrom
Gentle842:feat/message-attachments-upload-with-progress-preview
Oct 20, 2026
Merged

afeezorobsco-cyber merged 1 commit into
DocForum:mainfrom
Gentle842:feat/message-attachments-upload-with-progress-preview

Conversation

@Gentle842

Copy link
Copy Markdown
Contributor

Overview

Adds message attachments to the consultation composer (Phase W2, issue #23): pick or capture from the camera on mobile, client-side image downscale before upload, real upload progress with cancel + retry, inline image previews and PDF chips, and secure per-click signed downloads.

Implements the PRD §4 core flow: patient ⇄ treating doctor async consultation messaging (Phase W2). Built against a documented assumed API shape, since the dependencies below are not merged yet.

Depends on / context:

Related Issue

Closes #23

Changes

📎 Upload + secure download pipeline

  • [ADD] src/services/api-client.ts — apiUpload() (XMLHttpRequest — the one deliberate exception to the fetch-only client, since fetch can't report upload progress; still the only place HTTP details live) with real progress reporting and abort. ApiError gains a cancelled flag distinct from a network error.
  • [ADD] src/services/messaging-service.ts — thread fetch, message list/send, multipart attachment upload, and per-click signed download URL.
  • [ADD] src/types/models.ts — MessageThread, Message, MessageAttachment, MessagePage, SignedDownloadUrl, SendMessageInput (mirror the assumed docforum-core models).

🖼️ Composer, previews & validation

  • [ADD] src/features/messaging/attachment-validation.ts — type/size caps with clear messages; SVG and executables rejected client-side.
  • [ADD] src/features/messaging/image-utils.ts — best-effort client-side downscale (never scales up, falls back to the original). EXIF is never read or displayed here; stripping stays server-side.
  • [ADD] src/features/messaging/hooks/useAttachmentUploader.ts — staged uploads with progress / cancel / retry / remove; in-memory only, aborts in-flight uploads on unmount.
  • [ADD] src/features/messaging/hooks/useMessageThread.ts — thread fetch + send-message hooks.
  • [ADD] src/features/messaging/components/MessageComposer.tsx — attach-file and take-photo inputs, progress bar, cancel/retry, send gated on upload state.
  • [ADD] src/features/messaging/components/AttachmentPreview.tsx — inline image preview + PDF chip; every click mints a fresh signed URL and opens it.
  • [ADD] src/features/messaging/components/MessageThread.tsx — minimal shared thread shell (text-only rendering; message bodies are PHI).

🔀 Routing / docs

  • [ADD] src/pages/MessageThreadPage.tsx, mounted at /patient/messages/:threadId and /doctor/messages/:threadId.
  • [MODIFY] src/pages/PatientDashboardPage.tsx, src/pages/DoctorDashboardPage.tsx — register the shared route.
  • [MODIFY] ROADMAP.md, ARCHITECTURE_ESSENTIALS.md — item status + changelog, and the assumed API contract recorded under "Known assumptions".

🧪 Tests

  • [ADD] attachment-validation.test.ts, image-utils.test.ts, useAttachmentUploader.test.tsx, AttachmentPreview.test.tsx, MessageComposer.test.tsx — upload states (progress, cancel, retry, validation, cap) and the download flow (fresh URL per click, error handling).

Verification Results

> npm run typecheck
(clean, no errors)

> npm test
 Test Files  16 passed (16)
      Tests  70 passed (70)

> npm run build
 ✓ 166 modules transformed.
 ✓ built in 4.31s
 dist/assets/index-*.js   429.47 kB │ gzip: 140.13 kB
Acceptance Criteria Status
Attachments can be picked or captured (camera on mobile) ✅
Client-side image downscale before upload ✅
Progress bar with cancel and retry ✅
Inline image previews and PDF chips, fresh signed URL per click ✅
Client-side type + size validation with clear messages ✅
Tests for upload states and the download flow ✅
Signed URLs never cached beyond their use ✅
ROADMAP.md + ARCHITECTURE_ESSENTIALS.md updated ✅

Notes

Signed URLs are never stored in React Query's cache: the inline preview holds its URL only in component state, and each click/download mints a new one. All network calls (including multipart upload) stay inside src/services/api-client.ts, holding hard rule 1 and the repo's storage rule (drafts/staged uploads are in-memory only). No screenshots this session (no browser tool available) — same disclosed gap as prior UI PRs in this repo; typecheck/tests/build all pass. Happy to add desktop + mobile screenshots on request.

@drips-wave

drips-wave Bot commented Oct 16, 2026

Copy link
Copy Markdown

@Gentle842 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@afeezorobsco-cyber

afeezorobsco-cyber commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Thanks. The upload pipeline is well thought out: real progress with abort, in-memory staging, SVG and executables rejected, and a fresh signed URL for every download that's never cached. It can't merge yet:

Blocking

  1. CI fails: MessageThread type clash. main already has a MessageThread type, from the doctor inbox, with patientProfileId, patientName, unreadCount, lastPreview, waitingMinutes and closingSoon. This PR replaces it with a different shape (appointmentId, status, createdAt, updatedAt), so DoctorInbox.tsx, its tests, useMessageThreads.test.tsx and messaging-service.ts:43 no longer typecheck. Either merge the two shapes into one type, or name yours separately (for example ConsultationThread), and reconcile both in the assumed API contract in ARCHITECTURE_ESSENTIALS.md.
  2. Rebase needed. The branch is 1 commit behind main and conflicts in src/types/models.ts, ROADMAP.md and ARCHITECTURE_ESSENTIALS.md.

Should fix

  • Downloads may be popup-blocked. AttachmentPreview.handleOpen calls window.open(url) after an await. By then the click's user gesture has expired, so Safari and Chrome on mobile often block the new tab silently. Open a blank tab synchronously in the click handler and set its location once the URL arrives, or render a link once the URL is fetched.
  • Overlap with feat(messaging): consultation message thread UI #139. feat(messaging): consultation message thread UI #139 (consultation message thread UI) builds the full thread view, and this PR adds its own MessageThread shell component. Please coordinate with that author so only one thread component lands.

Once CI is green and the branch is rebased, I'll review again.

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@Gentle842

This PR has merge conflicts with main, which has moved on since this branch was created. Please rebase onto the latest main (or merge main into your branch), resolve the conflicts, and push. CI will then re-run, and the PR can be merged once it's conflict-free, CI is green and it's been reviewed. Thanks!

@afeezorobsco-cyber

Copy link
Copy Markdown
Contributor

@Gentle842 Thanks for the PR. It can't be merged yet. To unblock it:

CI is failing. Here's what broke:

Frontend (typecheck + test + build):

src/features/messaging/components/DoctorInbox.test.tsx(14,5): error TS2353: Object literal may only specify known properties, and 'patientProfileId' does not exist in type 'MessageThread'.
src/features/messaging/components/DoctorInbox.test.tsx(24,5): error TS2353: Object literal may only specify known properties, and 'patientProfileId' does not exist in type 'MessageThread'.
src/features/messaging/components/DoctorInbox.tsx(90,67): error TS2339: Property 'patientName' does not exist on type 'MessageThread'.
src/features/messaging/components/DoctorInbox.tsx(92,65): error TS2339: Property 'lastPreview' does not exist on type 'MessageThread'.

Please fix these errors and run the same steps locally before pushing (npm run typecheck && npm test && npm run build, or cargo test for contracts).

Merge conflicts: this branch conflicts with main. Please rebase onto the latest main (git fetch upstream && git rebase upstream/main), resolve the conflicts (often ROADMAP.md, ARCHITECTURE*.md or prisma/schema.prisma), and force-push.

Push to this same branch and CI will re-run. Once it's green and conflict-free, it'll be reviewed for merge.

@github-actions github-actions Bot added the needs-rebase Has merge conflicts with main; the author needs to rebase label Oct 20, 2026
@Gentle842
Gentle842 force-pushed the feat/message-attachments-upload-with-progress-preview branch from 7bb3549 to 878b847 Compare October 20, 2026 15:01
@github-actions

Copy link
Copy Markdown

@Gentle842 This PR now has merge conflicts with main, most likely because #193 (fix(ci): retry transient GitHub API errors in PR automation) was just merged, so it can't be merged until it's rebased:

git fetch upstream            # upstream = https://github.com/DocForum/docforum-web.git
git rebase upstream/main
# resolve the conflicts, re-run the tests, then:
git push --force-with-lease

The usual culprits are shared files like ROADMAP.md and ARCHITECTURE*.md. CI re-runs when you push, and the needs-rebase label is removed automatically once the conflicts are resolved.

Automated message. It won't repeat unless the PR conflicts again after your next push.

@Gentle842
Gentle842 force-pushed the feat/message-attachments-upload-with-progress-preview branch from 878b847 to b774840 Compare October 20, 2026 16:00
@Gentle842
Gentle842 force-pushed the feat/message-attachments-upload-with-progress-preview branch from b774840 to 386988a Compare October 20, 2026 16:01

@afeezorobsco-cyber afeezorobsco-cyber left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: no malicious code found, CI is green. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Has merge conflicts with main; the author needs to rebase

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Phase W2] Message attachments: upload with progress, preview, and secure download

2 participants