♻️(frontend) centralize allowed conversion formats in ContentTypes#2215
Conversation
StephanMeijer
force-pushed
the
feature/centralize-allowed-conversion-formats-frontend
branch
from
71534c2 to
b5440f9
Compare
StephanMeijer
changed the title
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughReplaced the exported Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
StephanMeijer
changed the title
There was a problem hiding this comment.
Pull request overview
Centralizes the mapping between importable MIME types and their allowed file extensions by moving extension knowledge into ContentTypes, and refactors the dropzone accept-map generation to be derived from that centralized definition.
Changes:
- Replaces
ContentTypesfrom a string enum to a{ mime, extensions }object model. - Refactors
useImportto generate thereact-dropzoneacceptmap by iterating overContentTypesand the configured allowed extensions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx |
Builds the dropzone accept map from centralized ContentTypes + config-driven extension allowlist. |
src/frontend/apps/impress/src/features/docs/docs-grid/api/useImportDoc.tsx |
Redefines ContentTypes to include both MIME and extension metadata for each supported import format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx`:
- Around line 43-46: The rejection message is hardcoded to "only .docx and .md…"
while allowed extensions are driven by config in the ACCEPT memo; update the
rejection copy generation in useImport (same file/useImport.tsx) to derive the
displayed allowed list from the computed allowedExtensions
(config.CONVERSION_FILE_EXTENSIONS_ALLOWED fallback array used in ACCEPT), e.g.,
build a string by mapping toLowerCase() and joining with commas and use that
string wherever the rejection/error message (around the current rejection
message location) is composed so the UI always reflects runtime config.
- Around line 48-56: The accept map currently assigns the entire
contentType.extensions when any single extension is allowed, which can re-enable
disallowed extensions; in useImport.tsx refine the assignment inside the reduce
so that for the matched contentType you filter contentType.extensions against
allowedExtensions and assign only the filtered list to acc[contentType.mime]
(e.g., compute allowed = contentType.extensions.filter(ext =>
allowedExtensions.includes(ext)) and set acc[contentType.mime] = allowed only
when allowed.length > 0), keeping the rest of the reduce logic and types
(AcceptedMap, ContentTypes) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4a1e0655-9a7f-4c98-b739-2e2b4e77f555
📒 Files selected for processing (2)
src/frontend/apps/impress/src/features/docs/docs-grid/api/useImportDoc.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx
StephanMeijer
force-pushed
the
feature/centralize-allowed-conversion-formats-frontend
branch
from
b5440f9 to
ee0bfcc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
StephanMeijer
force-pushed
the
feature/centralize-allowed-conversion-formats-frontend
branch
2 times, most recently
from
052bec6 to
be9aac0
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx (1)
98-107: Optionally de-duplicate displayed allowed extensions.Current output can show duplicates if future MIME groups share extensions. A tiny dedupe keeps the toast cleaner.
♻️ Suggested tweak
- const allowedExtensions = Object.values(ACCEPT).flat().join(', '); + const allowedExtensions = [ + ...new Set(Object.values(ACCEPT).flat()), + ].join(', ');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx` around lines 98 - 107, The toast builds allowedExtensions from Object.values(ACCEPT).flat().join(', ') which can produce duplicate extensions; de-duplicate before joining (e.g., convert the flattened array to a Set or otherwise unique it, optionally normalize case) so allowedExtensions is a clean, unique list; update the expression that creates allowedExtensions (referencing ACCEPT and allowedExtensions used in the toast/t(...) call and rejection.file.name) to dedupe the extensions prior to .join.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx`:
- Around line 98-107: The toast builds allowedExtensions from
Object.values(ACCEPT).flat().join(', ') which can produce duplicate extensions;
de-duplicate before joining (e.g., convert the flattened array to a Set or
otherwise unique it, optionally normalize case) so allowedExtensions is a clean,
unique list; update the expression that creates allowedExtensions (referencing
ACCEPT and allowedExtensions used in the toast/t(...) call and
rejection.file.name) to dedupe the extensions prior to .join.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c3de3618-20dd-4286-af6c-fe2fa1b75c92
📒 Files selected for processing (3)
CHANGELOG.mdsrc/frontend/apps/impress/src/features/docs/docs-grid/api/useImportDoc.tsxsrc/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx
✅ Files skipped from review due to trivial changes (1)
- CHANGELOG.md
StephanMeijer
force-pushed
the
feature/centralize-allowed-conversion-formats-frontend
branch
from
be9aac0 to
248e934
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace ContentTypes enum with a structured object mapping MIME types to their file extensions, removing the manual switch-case in useImport Signed-off-by: Stephan Meijer <me@stephanmeijer.com>
StephanMeijer
force-pushed
the
feature/centralize-allowed-conversion-formats-frontend
branch
from
248e934 to
12f7f6c
Compare
AntoLC
left a comment
AntoLC
left a comment
There was a problem hiding this comment.
We still have the 400 error, react-dropzone's accept prop works by MIME type apparently, not by extension.
I suppose we should have a double check here so:
https://github.com/StephanMeijer/docs/blob/12f7f6c04f70e9dfca29d719253cbfba3144f2bc/src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx#L70
That will display this toast as well, it should stay DRY.:
https://github.com/StephanMeijer/docs/blob/12f7f6c04f70e9dfca29d719253cbfba3144f2bc/src/frontend/apps/impress/src/features/docs/docs-grid/hooks/useImport.tsx#L99-L107
|
Strange. Because functionally this PR should not change anything. |
Purpose / Proposal
To centralize configuration surrounding acceptable formats to be converted and imported into La Suite Docs. This PR will be followed up by others to further centralize configuration.
External contributions
General requirements
CI requirements
git commit --signoff(DCO compliance)git commit -S)<gitmoji>(type) title description## [Unreleased]section (if noticeable change)AI requirements
I have used AI only for the description of this pull request and the commit message of my commit(s) and applying PR feedback.