Skip to content

fix(transactions): Mirror create's file-upload step on edit #79

Merged
nfebe merged 3 commits intodevfrom
fix/transaction-file-upload-and-list-refresh
May 3, 2026
Merged

fix(transactions): Mirror create's file-upload step on edit #79
nfebe merged 3 commits intodevfrom
fix/transaction-file-upload-and-list-refresh

Conversation

@nfebe
Copy link
Copy Markdown
Contributor

@nfebe nfebe commented May 3, 2026

No description provided.

nfebe added 2 commits May 3, 2026 11:51
Creating a transaction routed picked files through the dedicated
multipart upload endpoint. Editing a transaction accepted files in the
form but never uploaded them, so attachments added during an edit
silently disappeared.

The update flow now performs the same upload step, so files added on
edit actually land on the transaction.
Attachments only appeared as filename chips: picked images had no
thumbnail, saved attachments were invisible during edit, and reopening
the picker replaced the previous selection instead of extending it.

Picked attachments now render as cards with image thumbnails or a label
tile for documents, successive picks append to the selection, and saved
attachments load through the auth-gated file endpoint with a per-file
remove control that deletes them from the transaction.
@sourceant
Copy link
Copy Markdown

sourceant Bot commented May 3, 2026

Code Review Summary

This PR successfully implements a robust file-upload mirroring for the edit state. It introduces image previews, file type labeling, and a clean management of attachment lifecycle (upload/delete).

🚀 Key Improvements

  • Integrated ObjectURL management to prevent memory leaks.
  • Added support for deleting existing attachments directly from the edit form.
  • Improved UI with an attachment grid and file-type icons for non-image files.

💡 Minor Suggestions

  • Enhance file extension parsing logic.
  • Improve error handling visibility for bulk file uploads.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 3, 2026

Deploying webui with  Cloudflare Pages  Cloudflare Pages

Latest commit: 697e407
Status: ✅  Deploy successful!
Preview URL: https://2f315ef0.webui-9fh.pages.dev
Branch Preview URL: https://fix-transaction-file-upload.webui-9fh.pages.dev

View logs

Copy link
Copy Markdown

@sourceant sourceant Bot left a comment

Choose a reason for hiding this comment

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

Review complete. See the overview comment for a summary.

Comment thread components/TransactionForm.vue
Comment thread components/TransactionForm.vue
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 3, 2026

Deploying trakli-dev with  Cloudflare Pages  Cloudflare Pages

Latest commit: 697e407
Status: ✅  Deploy successful!
Preview URL: https://6b989e22.trakli-dev.pages.dev
Branch Preview URL: https://fix-transaction-file-upload.trakli-dev.pages.dev

View logs

The "Max 5 files" hint was advisory only -- the picker happily added
more than five attachments, leaving the count to be rejected later by
the server.

The form now silently drops anything past five total attachments
(counting both saved and newly picked files), matching the hint.
Copy link
Copy Markdown

@sourceant sourceant Bot left a comment

Choose a reason for hiding this comment

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

Review complete. See the overview comment for a summary.


function extensionLabel(name: string): string {
const cleaned = (name || '').split('?')[0].split('#')[0];
const ext = cleaned.split('.').pop();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The logic for extracting the file extension might fail if the filename contains special characters or lacks an extension. Using lastIndexOf('.') is more robust than multiple splits.

Suggested change
const ext = cleaned.split('.').pop();
const ext = name.includes('.') ? name.split('.').pop() : 'FILE';

@nfebe nfebe merged commit b48b285 into dev May 3, 2026
5 checks passed
@nfebe nfebe deleted the fix/transaction-file-upload-and-list-refresh branch May 3, 2026 11:04
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.

1 participant