feat(dev-portal-web): INFRA-1712 media upload global refactoring#7787
Conversation
|
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 (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds a shared media upload component, wires it into a new B2B media subsection, refactors the B2C icons section to use the shared flow, and updates related constants, translations, and markdown validation. ChangesMediaUpload feature: shared component, B2B logo upload, B2C refactor
Sequence Diagram(s)sequenceDiagram
participant User
participant MediaUpload
participant MediaSubsection
participant uploadFiles as uploadFiles(`@open-condo/files`)
participant useUpdateB2BAppMutation
participant GetB2BAppDocument
User->>MediaUpload: selects logo file
MediaUpload->>MediaUpload: validate and build preview
User->>MediaUpload: clicks Save
MediaUpload->>MediaSubsection: onSave(uploadedFiles)
MediaSubsection->>uploadFiles: upload with sender/user/model/item metadata
uploadFiles-->>MediaSubsection: UploadFileResult[]
MediaSubsection->>useUpdateB2BAppMutation: set logo field
useUpdateB2BAppMutation->>GetB2BAppDocument: refetchQueries(id)
GetB2BAppDocument-->>MediaSubsection: updated app data
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.16][ERROR]: unable to find a config; path apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/MediaSubsection.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.21][ERROR]: unable to find a config; path apps/dev-portal-web/domains/miniapp/components/MediaUpload.tsx┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.21][ERROR]: unable to find a config; path
🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec65375de1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| const handleSave = useCallback(async () => { | ||
| await onSave?.(currentFiles) | ||
| setCurrentFiles([]) |
There was a problem hiding this comment.
If uploadFiles fails, the B2B/B2C save handlers catch the exception, call onError, and return without throwing, but MediaUpload still clears currentFiles and the preview immediately after onSave returns. In a transient upload/server error the error notification is shown and the chosen file disappears, so the user cannot retry Save without reselecting it; reset this state only after a confirmed successful save or make onSave signal failure.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good one, will fix it
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx (1)
53-70: 🩺 Stability & Availability | 🟡 MinorDrop the disconnected
forminstance.IconsSubsectionno longer renders a<Form>, whileMediaUploadmanages its own internal form/state, soform.resetFields()is redundant and can be removed along with the unusedFormimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx` around lines 53 - 70, IconsSubsection still creates a disconnected Ant Design form instance even though it no longer renders a Form and MediaUpload manages its own state. Remove the unused Form.useForm setup and the form.resetFields() call from the onCompleted callback, and clean up the now-unused Form import while keeping useMutationCompletedHandler and useUpdateB2CAppMutation behavior unchanged.
🧹 Nitpick comments (2)
apps/dev-portal-web/domains/miniapp/components/MediaUpload.tsx (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
mimetypesliteral restriction collapses tostring.Because
ALLOWED_MIMETYPESis not declaredas const,typeof ALLOWED_MIMETYPES[number]resolves tostring, soMediaRestrictions.mimetypesaccepts any string rather than the intended'image/webp' | 'image/png'union. Addas constto preserve the literals.♻️ Suggested change
-const ALLOWED_MIMETYPES = ['image/webp', 'image/png'] +const ALLOWED_MIMETYPES = ['image/webp', 'image/png'] as constAlso applies to: 22-23
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dev-portal-web/domains/miniapp/components/MediaUpload.tsx` at line 17, Add as const to ALLOWED_MIMETYPES in MediaUpload so typeof ALLOWED_MIMETYPES[number] preserves the literal union instead of widening to string, and keep MediaRestrictions.mimetypes typed from that constant so it only accepts the intended image/webp and image/png values. Update the ALLOWED_MIMETYPES definition and any related type usage in MediaUpload to rely on the narrowed tuple type.apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx (1)
72-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid shadowing the
uploadedFilesparameter.The
SaveHandlerparameteruploadedFiles(Line 72) is shadowed by the destructured result ofuploadFileson Line 80 (const { files: uploadedFiles } = ...). This is confusing to read and obscures which value is in scope below. Rename the inner binding.♻️ Suggested rename
- const { files: uploadedFiles } = await uploadFiles({ + const { files: resultFiles } = await uploadFiles({ ... }) - files = uploadedFiles + files = resultFiles🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx` around lines 72 - 101, The SaveHandler in IconsSubsection’s handleIconSave is shadowing its uploadedFiles parameter with the destructured uploadFiles result, which makes the scope confusing. Rename the inner binding from uploadFiles to a distinct name (for example, uploadedResultFiles) and keep the rest of the logic in handleIconSave unchanged so it’s clear which files array is being referenced.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/MediaSubsection.tsx`:
- Around line 1-20: The import block in MediaSubsection should be reordered to
match the enforced grouped order and alphabetical sorting. Update the imports in
MediaSubsection.tsx so external, `@open-condo`, internal, and sibling imports are
grouped correctly, and move
GetB2BAppDocument/useGetB2BAppQuery/useUpdateB2BAppMutation into the proper
internal group without leaving an external import after internal/sibling
imports.
In `@apps/dev-portal-web/domains/miniapp/components/MediaUpload.tsx`:
- Around line 114-120: The color separator logic in MediaUpload’s ColorsValue
memo is inverted, so lists with 3+ items render incorrectly. Update the flatMap
in ColorsValue so the separator is added before every non-first ColorSpan (idx
!== 0) rather than after the first item, keeping the existing ColorSpan and
styles.colon usage intact.
- Around line 122-139: Object URLs created in MediaUpload are only revoked
during file change and save, so they can leak if the component unmounts while
previews are still active. Add an unmount cleanup in MediaUpload using useEffect
to revoke every previewUrl currently stored in previewState, and update the
React import to include useEffect; keep the existing cleanup behavior in
handleFileChange and handleSave, and reference the handleFileChange, handleSave,
and previewState logic when making the change.
In `@apps/dev-portal-web/lang/en.json`:
- Line 298: The user-facing copy in the localization entry for
pages.apps.b2c.id.sections.info.icons.items.main.description has a grammar
mistake with an extra “in”; update the string to remove the stray word so it
reads naturally. Keep the change limited to that translation key in en.json and
verify the revised wording still clearly describes that it is used to display on
the resident’s home screen in the mobile and web apps.
In `@apps/dev-portal-web/lang/ru.json`:
- Line 300: The Russian translation for
pages.apps.b2c.id.sections.info.icons.items.main.description contains a
grammatical typo with an extra “в” before “для”; update the string in ru.json to
use the correct phrasing, keeping the meaning intact while fixing the malformed
Russian wording.
---
Outside diff comments:
In
`@apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx`:
- Around line 53-70: IconsSubsection still creates a disconnected Ant Design
form instance even though it no longer renders a Form and MediaUpload manages
its own state. Remove the unused Form.useForm setup and the form.resetFields()
call from the onCompleted callback, and clean up the now-unused Form import
while keeping useMutationCompletedHandler and useUpdateB2CAppMutation behavior
unchanged.
---
Nitpick comments:
In
`@apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsx`:
- Around line 72-101: The SaveHandler in IconsSubsection’s handleIconSave is
shadowing its uploadedFiles parameter with the destructured uploadFiles result,
which makes the scope confusing. Rename the inner binding from uploadFiles to a
distinct name (for example, uploadedResultFiles) and keep the rest of the logic
in handleIconSave unchanged so it’s clear which files array is being referenced.
In `@apps/dev-portal-web/domains/miniapp/components/MediaUpload.tsx`:
- Line 17: Add as const to ALLOWED_MIMETYPES in MediaUpload so typeof
ALLOWED_MIMETYPES[number] preserves the literal union instead of widening to
string, and keep MediaRestrictions.mimetypes typed from that constant so it only
accepts the intended image/webp and image/png values. Update the
ALLOWED_MIMETYPES definition and any related type usage in MediaUpload to rely
on the narrowed tuple type.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b566226f-46b3-4a87-8826-e72d4de76967
⛔ Files ignored due to path filters (1)
apps/dev-portal-web/public/icons/b2b.pngis excluded by!**/*.png
📒 Files selected for processing (12)
apps/dev-portal-web/domains/common/hooks/useMarkdownLengthValidation.tsapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/InfoSection.tsxapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/B2BAppCard.module.cssapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/B2BAppCard.tsxapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/MediaSubsection.tsxapps/dev-portal-web/domains/miniapp/components/B2BApp/edit/info/MediaSubsection/index.tsapps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.module.cssapps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.tsxapps/dev-portal-web/domains/miniapp/components/MediaUpload.module.cssapps/dev-portal-web/domains/miniapp/components/MediaUpload.tsxapps/dev-portal-web/lang/en.jsonapps/dev-portal-web/lang/ru.json
💤 Files with no reviewable changes (1)
- apps/dev-portal-web/domains/miniapp/components/B2CApp/edit/info/IconsSubsection.module.css
|
1 participant
Screen.Recording.2026-06-24.at.17.04.29.mov
Summary by CodeRabbit