Conversation
Contributor
📝 WalkthroughWalkthroughAdds image upload support to the editor: client-side upload utility, a TipTap ImageUpload extension (paste/drop), server API to validate and store images in Cloudflare R2, editor wiring to upload and insert image URLs, and i18n/key updates plus tests and an R2 .env example entry. Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor as "TipTap Editor"
participant Ext as "ImageUpload Extension"
participant Client as "uploadImage()"
participant Server as "/api/upload/image"
participant R2 as "Cloudflare R2"
User->>Editor: Drag/Paste image
Editor->>Ext: handleDrop / handlePaste
Ext->>Client: uploadImage(file)
Client->>Server: POST FormData (file)
Server->>Server: validateImageFile()
Server->>R2: PutObject (via S3 client)
R2-->>Server: success
Server-->>Client: { url }
Client-->>Ext: return URL
Ext->>Editor: insert image (setImage src:url)
Editor-->>User: image inserted in editor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
app/components/work-item/WorkItemConversation.vue (1)
31-44: Prefer payload-derived optimistic fields over hardcoded defaults.For consistency with server state, use returned comment metadata when available (e.g., reactions/permissions) before falling back to local defaults.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/components/work-item/WorkItemConversation.vue` around lines 31 - 44, The optimistic timeline entry currently hardcodes reactionGroups and permission flags; update the object creation in the timeline spread (where timeline: [...workItemRef.value.timeline, { ... }]) to prefer payload-derived fields from the returned comment (e.g., use comment.reactionGroups, comment.viewerCanUpdate, comment.viewerCanDelete or comment.permissions) and only fall back to the local defaults ([], true/false) when those properties are undefined; keep the existing fields (id, subjectId, source, sourceNumber, kind, author, authorAvatarUrl, createdAt, body) but replace the hardcoded reactionGroups/viewerCanUpdate/viewerCanDelete with conditional uses of the comment metadata.app/extensions/image-upload.ts (1)
5-10: Consider client-side filtering against allowed types.
getImageFileaccepts any file withtype.startsWith('image/'), but the server only allows specific types (image/png,image/jpeg,image/gif,image/webp,image/svg+xml). Users dropping unsupported image formats (e.g.,image/bmp,image/tiff) will see a generic upload error.💡 Optional: Add client-side type filtering
+const ALLOWED_IMAGE_TYPES = new Set(['image/png', 'image/jpeg', 'image/gif', 'image/webp', 'image/svg+xml']) + function getImageFile(dt: DataTransfer): File | null { for (const file of dt.files) { - if (file.type.startsWith('image/')) return file + if (ALLOWED_IMAGE_TYPES.has(file.type)) return file } return null }This would silently ignore unsupported formats on drop/paste. Alternatively, you could invoke
onErrorfor unsupported types to give feedback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/extensions/image-upload.ts` around lines 5 - 10, getImageFile currently accepts any MIME starting with "image/", causing unsupported formats like image/bmp to be passed through; update getImageFile to check against the same allowed set the server enforces (image/png, image/jpeg, image/gif, image/webp, image/svg+xml) — create an allowedTypes array inside getImageFile and only return the File if file.type is in that array, otherwise return null (or optionally call the existing onError handler to surface a user-friendly message about unsupported image types).test/unit/imageUpload.test.ts (1)
4-33: Tests look solid with good edge case coverage.The tests cover valid types, unsupported types, and size boundaries effectively. Consider adding a test for the
No file providedcase whendataortypeis missing, as this is a distinct validation path invalidateImageFile.💡 Optional: Add missing file test
it('rejects when data or type is missing', () => { const noData = validateImageFile({ data: undefined as any, type: 'image/png' }) expect(noData.valid).toBe(false) if (!noData.valid) { expect(noData.message).toContain('No file provided') } const noType = validateImageFile({ data: Buffer.from('x'), type: undefined }) expect(noType.valid).toBe(false) if (!noType.valid) { expect(noType.message).toContain('No file provided') } })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/imageUpload.test.ts` around lines 4 - 33, Add a test case named something like "rejects when data or type is missing" in imageUpload.test.ts that calls validateImageFile with data undefined (e.g., data: undefined as any, type: 'image/png') and with type undefined (e.g., data: Buffer.from('x'), type: undefined), and assert in both cases that the returned object has valid === false and that result.message contains "No file provided"; place it alongside the other validateImageFile it blocks to cover this distinct validation path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/utils/uploadImage.ts`:
- Around line 4-8: The response from the $fetch call to '/api/upload/image' is
used directly; validate the returned url before returning it from uploadImage:
check that the destructured url is a non-empty string and a well-formed URL
(e.g., attempt new URL(url) or a regex) and if validation fails throw or return
a clear error/nullable value so callers don't get a broken image src; update the
block around the $fetch call and the return to perform this validation and
handle the invalid case.
In `@server/api/upload/image.post.ts`:
- Around line 56-61: The upload currently stores SVGs with their original
Content-Type which can allow inline JS execution; update the PutObjectCommand
call (the call to getS3Client().send(new PutObjectCommand({...}))) to set
ContentDisposition: 'attachment' when file.type === 'image/svg+xml' (or
otherwise disallow SVG by rejecting that MIME type earlier), e.g., include a
conditional that adds ContentDisposition: 'attachment' to the PutObjectCommand
input for SVGs so browsers will download instead of render inline; keep
ContentType as file.type and ensure the branch is applied where the upload is
performed.
---
Nitpick comments:
In `@app/components/work-item/WorkItemConversation.vue`:
- Around line 31-44: The optimistic timeline entry currently hardcodes
reactionGroups and permission flags; update the object creation in the timeline
spread (where timeline: [...workItemRef.value.timeline, { ... }]) to prefer
payload-derived fields from the returned comment (e.g., use
comment.reactionGroups, comment.viewerCanUpdate, comment.viewerCanDelete or
comment.permissions) and only fall back to the local defaults ([], true/false)
when those properties are undefined; keep the existing fields (id, subjectId,
source, sourceNumber, kind, author, authorAvatarUrl, createdAt, body) but
replace the hardcoded reactionGroups/viewerCanUpdate/viewerCanDelete with
conditional uses of the comment metadata.
In `@app/extensions/image-upload.ts`:
- Around line 5-10: getImageFile currently accepts any MIME starting with
"image/", causing unsupported formats like image/bmp to be passed through;
update getImageFile to check against the same allowed set the server enforces
(image/png, image/jpeg, image/gif, image/webp, image/svg+xml) — create an
allowedTypes array inside getImageFile and only return the File if file.type is
in that array, otherwise return null (or optionally call the existing onError
handler to surface a user-friendly message about unsupported image types).
In `@test/unit/imageUpload.test.ts`:
- Around line 4-33: Add a test case named something like "rejects when data or
type is missing" in imageUpload.test.ts that calls validateImageFile with data
undefined (e.g., data: undefined as any, type: 'image/png') and with type
undefined (e.g., data: Buffer.from('x'), type: undefined), and assert in both
cases that the returned object has valid === false and that result.message
contains "No file provided"; place it alongside the other validateImageFile it
blocks to cover this distinct validation path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 34003bda-e526-4958-9975-373b28c654e3
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (14)
.env.exampleapp/components/timeline/ReviewCommentThread.vueapp/components/work-item/WorkItemConversation.vueapp/composables/useMarkdownEditor.tsapp/composables/useMarkdownEditorHandlers.tsapp/extensions/image-upload.tsapp/utils/uploadImage.tsi18n/locales/de.jsoni18n/locales/en.jsoni18n/schema.jsonpackage.jsonserver/api/upload/image.post.tsserver/utils/image-upload.tstest/unit/imageUpload.test.ts
Comment on lines
+4
to
+8
| const { url } = await $fetch<{ url: string }>('/api/upload/image', { | ||
| method: 'POST', | ||
| body: form, | ||
| }) | ||
| return url |
Contributor
There was a problem hiding this comment.
Validate response URL before returning.
If the API responds without a valid url, this returns invalid data and can insert a broken image source downstream.
💡 Proposed fix
export async function uploadImage(file: File): Promise<string> {
const form = new FormData()
form.append('file', file)
- const { url } = await $fetch<{ url: string }>('/api/upload/image', {
+ const response = await $fetch<{ url?: string }>('/api/upload/image', {
method: 'POST',
body: form,
})
- return url
+ if (!response.url || !response.url.trim()) {
+ throw new Error('Image upload did not return a valid URL')
+ }
+ return response.url
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/utils/uploadImage.ts` around lines 4 - 8, The response from the $fetch
call to '/api/upload/image' is used directly; validate the returned url before
returning it from uploadImage: check that the destructured url is a non-empty
string and a well-formed URL (e.g., attempt new URL(url) or a regex) and if
validation fails throw or return a clear error/nullable value so callers don't
get a broken image src; update the block around the $fetch call and the return
to perform this validation and handle the invalid case.
Contributor
Author
There was a problem hiding this comment.
No - this comes from us. If this url would be broken/wrong we would have a bigger problem. No fix needed in my eyes.
Contributor
There was a problem hiding this comment.
@Flo0806, that's a fair point — since the /api/upload/image endpoint is fully under your control and the server contract is well-defined, trusting the response is a reasonable call. I'll defer to your judgment here.
😔 🐇
🧠 Learnings used
Learnt from: Flo0806
Repo: flumen-dev/flumen.dev PR: 158
File: app/stores/dashboard.ts:1-23
Timestamp: 2026-02-28T08:12:02.323Z
Learning: In the flumen.dev Nuxt project, all pure TypeScript types exported from ROOT/shared/types are globally auto-imported across components, composables, and stores. Do not add explicit 'import type' statements for these types. Only runtime values (consts, functions, objects) require explicit imports. For this file (app/stores/dashboard.ts), rely on the global type imports and avoid importing these types explicitly.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/extensions/image-upload.ts (1)
5-10: Consider adding client-side file size validation for faster user feedback.The server performs file size validation, but users will wait for the upload to complete (or timeout) before seeing a size error. Pre-validating file size client-side provides immediate feedback for oversized files.
♻️ Proposed change to add size check
+const MAX_IMAGE_SIZE = 5 * 1024 * 1024 // 5MB - should match server limit + -function getImageFile(dt: DataTransfer): File | null { +function getImageFile(dt: DataTransfer, maxSize?: number): File | null { for (const file of dt.files) { - if (file.type.startsWith('image/')) return file + if (file.type.startsWith('image/')) { + if (maxSize && file.size > maxSize) return null + return file + } } return null }Then update callers to pass
MAX_IMAGE_SIZEand potentially surface a specific error for oversized files viaonError.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/extensions/image-upload.ts` around lines 5 - 10, Add client-side file size validation by extending getImageFile to accept a maxSize parameter (e.g., getImageFile(dt: DataTransfer, maxSize: number): File | null) and check each candidate file's size (file.size) in addition to type; if file.type startsWith('image/') but file.size > maxSize return null (or a distinct signal) so callers like the upload handler can surface an "oversized file" error via their onError handler; update all callers to pass MAX_IMAGE_SIZE and handle the null/oversize case to provide immediate user feedback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/extensions/image-upload.ts`:
- Around line 28-34: Extend the ImageUploadOptions type to include onLoading?:
(isLoading: boolean) => void, then update handleFile to call
options.onLoading?.(true) immediately before starting uploadImage(file) and
ensure options.onLoading?.(false) is called in both the .then branch after
editor.chain().focus().setImage(...) and in the .catch branch (after invoking
options.onError) so the loading state is always cleared on success or error;
ensure uploadImage, handleFile and any exported types (ImageUploadOptions) are
updated so consumers can pass the loading callback.
---
Nitpick comments:
In `@app/extensions/image-upload.ts`:
- Around line 5-10: Add client-side file size validation by extending
getImageFile to accept a maxSize parameter (e.g., getImageFile(dt: DataTransfer,
maxSize: number): File | null) and check each candidate file's size (file.size)
in addition to type; if file.type startsWith('image/') but file.size > maxSize
return null (or a distinct signal) so callers like the upload handler can
surface an "oversized file" error via their onError handler; update all callers
to pass MAX_IMAGE_SIZE and handle the null/oversize case to provide immediate
user feedback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b08d0bd8-ec52-4f0c-ba4b-21458ed42ec6
📒 Files selected for processing (3)
app/extensions/image-upload.tsserver/utils/image-upload.tstest/unit/imageUpload.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- server/utils/image-upload.ts
- test/unit/imageUpload.test.ts
Comment on lines
+28
to
+34
| function handleFile(file: File) { | ||
| uploadImage(file) | ||
| .then(url => editor.chain().focus().setImage({ src: url }).run()) | ||
| .catch((e) => { | ||
| options.onError?.(e instanceof Error ? e : new Error('Upload failed')) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if loading/progress state is handled when ImageUpload extension is used
rg -n -A5 -B5 'ImageUpload' --glob '*.ts' --glob '*.vue' | head -80Repository: flumen-dev/flumen.dev
Length of output: 2741
🏁 Script executed:
#!/bin/bash
# Get the full ImageUpload extension to see all available options
cat app/extensions/image-upload.tsRepository: flumen-dev/flumen.dev
Length of output: 1564
🏁 Script executed:
#!/bin/bash
# Check if there are any loading/progress state callbacks or handlers in useMarkdownEditor
cat app/composables/useMarkdownEditor.ts | head -100Repository: flumen-dev/flumen.dev
Length of output: 5836
🏁 Script executed:
#!/bin/bash
# Search for any loading indicators related to image uploads in the codebase
rg -n 'loading|progress|isUploading' --glob '*.ts' --glob '*.vue' -A2 -B2 | grep -i 'image\|upload' -A2 -B2Repository: flumen-dev/flumen.dev
Length of output: 47
Add a loading state callback to provide upload progress feedback.
The error handling correctly wraps non-Error values, ensuring onError always receives an Error instance, and the chained focus().setImage() pattern is idiomatic for TipTap.
However, there's no visual loading state while the upload is in progress. The ImageUpload extension currently only provides an onError callback with no mechanism for tracking upload progress. Users have no feedback between initiating the upload and the image appearing in the editor. Extend ImageUploadOptions to include a loading state callback (e.g., onLoading?: (isLoading: boolean) => void) so consuming code can display a loading indicator.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/extensions/image-upload.ts` around lines 28 - 34, Extend the
ImageUploadOptions type to include onLoading?: (isLoading: boolean) => void,
then update handleFile to call options.onLoading?.(true) immediately before
starting uploadImage(file) and ensure options.onLoading?.(false) is called in
both the .then branch after editor.chain().focus().setImage(...) and in the
.catch branch (after invoking options.onError) so the loading state is always
cleared on success or error; ensure uploadImage, handleFile and any exported
types (ImageUploadOptions) are updated so consumers can pass the loading
callback.
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
1 participant
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.
Summary
Related issue(s)
Closes #73
Type of change
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Tests