CSV Uploads#2134
Open
GelatoGenesis wants to merge 2 commits intomainfrom
Open
Conversation
Signed-off-by: GelatoGenesis <tarmokalling@gmail.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR introduces comprehensive CSV file support to the drop/wave upload system. Changes include new file type detection for CSV files, conditional CSV acceptance based on upload mode (chat mode allows CSV, drop mode rejects it), a new DropMediaAttachmentCard component for CSV rendering, MIME type normalization for CSV files, and corresponding test coverage across multiple components. Changes
Sequence DiagramsequenceDiagram
actor User
participant CreateDropActions as CreateDropActions<br/>(Chat/Drop Mode)
participant CreateDropContent as CreateDropContent<br/>(File Handler)
participant multipartUploadCore as multipartUploadCore<br/>(MIME Normalization)
participant MediaDisplay as MediaDisplay<br/>(CSV Rendering)
participant DropMediaAttachmentCard as DropMediaAttachmentCard
User->>CreateDropActions: Selects file
alt Chat Mode (isDropMode=false)
CreateDropActions->>CreateDropActions: fileInputAccept includes .csv, text/csv
CreateDropActions->>User: Allows CSV selection
else Drop Mode (isDropMode=true)
CreateDropActions->>CreateDropActions: fileInputAccept excludes .csv
CreateDropActions->>User: Rejects CSV selection
end
alt File Upload in Drop Mode
User->>CreateDropContent: Uploads file
CreateDropContent->>CreateDropContent: Check isCsvFile()
alt Is CSV
CreateDropContent->>User: Show error toast, reject
else Not CSV
CreateDropContent->>multipartUploadCore: Upload file
multipartUploadCore->>multipartUploadCore: Normalize MIME to text/csv if CSV
multipartUploadCore->>CreateDropContent: Return with normalized type
end
end
CreateDropContent->>MediaDisplay: Display file
alt Media MIME Type is text/csv
MediaDisplay->>DropMediaAttachmentCard: Render CSV attachment
DropMediaAttachmentCard->>User: Show CSV icon + download
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
components/drops/view/item/content/media/MediaDisplay.tsx (1)
11-19: Consider consolidatingMediaTypeenum andgetMediaTypelogic.The
MediaTypeenum andgetMediaTypefunction are duplicated between this file andDropListItemContentMedia.tsx. While the current implementation is correct, this duplication could lead to inconsistencies if future media types are added to one file but not the other.♻️ Suggested approach
Extract to a shared utility:
// e.g., helpers/mediaType.helpers.ts export enum MediaType { IMAGE = "IMAGE", VIDEO = "VIDEO", AUDIO = "AUDIO", GLB = "GLB", HTML = "HTML", CSV = "CSV", UNKNOWN = "UNKNOWN", } export const getMediaType = ( mimeType: string, url: string ): MediaType => { // ... shared detection logic };Both
MediaDisplay.tsxandDropListItemContentMedia.tsxcan then import and use this shared utility.Also applies to: 47-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplay.tsx` around lines 11 - 19, The MediaType enum and getMediaType detection logic are duplicated between MediaDisplay.tsx and DropListItemContentMedia.tsx; extract both into a shared helper (e.g., helpers/mediaType.helpers.ts) exporting the MediaType enum and getMediaType function, update MediaDisplay.tsx to import MediaType and getMediaType (references: MediaType, getMediaType) and remove the local enum/function, and update DropListItemContentMedia.tsx to import the same shared symbols so both components use the single source of truth for media type detection.components/drops/create/utils/file/CreateDropSelectedFilePreview.tsx (1)
15-29: Consider extracting sharedgetFileTypelogic to reduce duplication.This
getFileTypefunction is nearly identical to the one inCreateDropSelectedFileIcon.tsx(see context snippet 1), with the only difference being the return value for unknown types (FILE_TYPES.UNKNOWNvsnull). Having two separate implementations increases the risk of them diverging when new file types are added.♻️ Proposed consolidation approach
Create a shared utility:
// e.g., in helpers/file.helpers.ts or a new file export const detectFileType = (file: File | null): FILE_TYPES | null => { if (!file) return null; if (file.type.includes("image")) return FILE_TYPES.IMAGE; if (file.type.includes("video")) return FILE_TYPES.VIDEO; if (file.type.includes("audio")) return FILE_TYPES.AUDIO; if (file.type === "text/csv" || file.name.toLowerCase().endsWith(".csv")) { return FILE_TYPES.CSV; } return null; };Then each consumer can handle the
nullcase as needed (returningUNKNOWNor keepingnull).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/create/utils/file/CreateDropSelectedFilePreview.tsx` around lines 15 - 29, The getFileType implementation in CreateDropSelectedFilePreview.tsx duplicates logic found in CreateDropSelectedFileIcon.tsx; extract that logic into a single exported helper (e.g., detectFileType in a new helpers/file.helpers.ts) that accepts File | null and returns FILE_TYPES | null, update both components to import and call detectFileType, and in CreateDropSelectedFilePreview.tsx map a null result to FILE_TYPES.UNKNOWN (while the other consumer can keep null) and remove the local getFileType implementation so the shared helper is the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drops/create/utils/CreateDropActionsRow.tsx`:
- Line 46: The accept attribute in CreateDropActionsRow currently
unconditionally includes ".csv,text/csv", which bypasses the drop-mode
restriction; modify CreateDropActionsRow to conditionally omit CSV MIME/types
when in drop mode by mirroring the mode gating used in CreateDropActions (use
the same mode prop or isDropMode flag passed from CreateDropActions), build the
accept string dynamically (exclude ".csv" and "text/csv" when mode === 'drop' or
isDropMode is true), and ensure the component receives the mode value from
CreateDropActions so the CSV types are only allowed outside drop mode.
---
Nitpick comments:
In `@components/drops/create/utils/file/CreateDropSelectedFilePreview.tsx`:
- Around line 15-29: The getFileType implementation in
CreateDropSelectedFilePreview.tsx duplicates logic found in
CreateDropSelectedFileIcon.tsx; extract that logic into a single exported helper
(e.g., detectFileType in a new helpers/file.helpers.ts) that accepts File | null
and returns FILE_TYPES | null, update both components to import and call
detectFileType, and in CreateDropSelectedFilePreview.tsx map a null result to
FILE_TYPES.UNKNOWN (while the other consumer can keep null) and remove the local
getFileType implementation so the shared helper is the single source of truth.
In `@components/drops/view/item/content/media/MediaDisplay.tsx`:
- Around line 11-19: The MediaType enum and getMediaType detection logic are
duplicated between MediaDisplay.tsx and DropListItemContentMedia.tsx; extract
both into a shared helper (e.g., helpers/mediaType.helpers.ts) exporting the
MediaType enum and getMediaType function, update MediaDisplay.tsx to import
MediaType and getMediaType (references: MediaType, getMediaType) and remove the
local enum/function, and update DropListItemContentMedia.tsx to import the same
shared symbols so both components use the single source of truth for media type
detection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61454309-cd22-4d80-a6fd-fbcb8092760a
📒 Files selected for processing (14)
__tests__/components/drops/create/utils/CreateDropActionsRow.test.tsx__tests__/components/drops/view/item/content/media/DropListItemContentMedia.test.tsx__tests__/components/drops/view/item/content/media/MediaDisplay.test.tsx__tests__/components/waves/CreateDropActions.test.tsx__tests__/services/uploads/multipartUploadCore.test.tscomponents/drops/create/utils/CreateDropActionsRow.tsxcomponents/drops/create/utils/file/CreateDropSelectedFileIcon.tsxcomponents/drops/create/utils/file/CreateDropSelectedFilePreview.tsxcomponents/drops/view/item/content/media/DropListItemContentMedia.tsxcomponents/drops/view/item/content/media/DropMediaAttachmentCard.tsxcomponents/drops/view/item/content/media/MediaDisplay.tsxcomponents/waves/CreateDropActions.tsxcomponents/waves/CreateDropContent.tsxservices/uploads/multipartUploadCore.ts
Signed-off-by: GelatoGenesis <tarmokalling@gmail.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
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 by CodeRabbit
New Features
Tests