[WEB-5493] feat: implement static cover image handling and selection#8184
[WEB-5493] feat: implement static cover image handling and selection#8184sriramveeraghanta merged 15 commits intopreviewfrom
Conversation
- Added functionality to handle cover image uploads and selections in project and profile forms. - Introduced a new helper for managing cover images, including static images and uploaded assets. - Updated components to utilize the new cover image helper for displaying and processing cover images. - Added a set of static cover images for selection in the image picker. - Enhanced error handling for image uploads and processing. This update improves the user experience by providing a more robust cover image management system.
… consistency - Updated the cover image helper to export STATIC_COVER_IMAGES instead of STATIC_COVER_IMAGES_ARRAY. - Adjusted the ImagePickerPopover component to utilize the renamed export for rendering static cover images.
WalkthroughAdds a centralized cover-image helper and static registry, replaces server-driven project cover-image retrieval with static images, integrates cover-image analysis/upload into profile and project submit flows, removes the project-covers API and client method, and introduces getProjectFormValues for project defaults. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Form / ImagePicker
participant Helper as CoverImageHelper
participant FileSvc as FileService
participant API as Backend
User->>UI: select/change cover image
UI->>Helper: handleCoverImageChange(current, new, {entityType, entityIdentifier, workspaceSlug?, isUserAsset?})
Helper->>Helper: analyzeCoverImageChange(current, new)
alt needs upload (local_static)
Helper->>FileSvc: uploadCoverImage(imageUrl, config)
FileSvc->>API: POST /upload
API-->>FileSvc: assetUrl
FileSvc-->>Helper: assetUrl
Helper-->>UI: payload (cover_image_asset / cover_image_url)
else external URL / already uploaded / removal
Helper-->>UI: payload (cover_image / null / cover_image_asset as appropriate)
end
UI->>API: submit form payload (includes cover_image / cover_image_asset)
API-->>UI: response (created)
alt upload happened and assetUrl returned
UI->>API: updateCoverImageStatus(entityId, assetUrl)
API-->>UI: status OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
JayashTripathy
changed the title
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive cover image handling system with support for static images, uploaded assets, and external URLs. The implementation centralizes cover image logic into a dedicated helper module and updates the project and profile forms to use this new system.
Key Changes:
- Created a centralized cover image helper with type detection, display URL generation, and upload handling
- Added 29 static cover images to the public/images folder
- Updated project and profile forms to use the new helper for cover image processing
- Replaced direct
getFileURLcalls withgetCoverImageDisplayURLfor consistent image URL handling
Reviewed changes
Copilot reviewed 8 out of 37 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/helpers/cover-image.helper.ts | Core helper module implementing cover image type detection, upload, and URL processing logic |
| apps/web/public/images/*.jpg | Added 29 static cover images (binary files) |
| apps/web/core/components/project/form.tsx | Integrated cover image helper for project cover image handling with async upload support |
| apps/web/core/components/project/create/header.tsx | Updated to use getCoverImageDisplayURL for consistent image URL handling |
| apps/web/core/components/project/card.tsx | Replaced hardcoded fallback URL with centralized constant |
| apps/web/core/components/profile/sidebar.tsx | Updated to use getCoverImageDisplayURL helper |
| apps/web/core/components/profile/form.tsx | Integrated cover image helper for user profile cover images with async upload |
💡 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/core/components/profile/sidebar.tsx (1)
104-111: Remove redundant fallback -getCoverImageDisplayURLalways returns a non-empty string.The helper function
getCoverImageDisplayURLhas the signature(imageUrl: string | null | undefined, fallbackUrl: string): stringand is guaranteed to return either the processed URL or thefallbackUrl. The trailing|| "/users/user-profile-cover-default-img.png"is unreachable code.<img src={ - getCoverImageDisplayURL(userData?.cover_image_url, "/users/user-profile-cover-default-img.png") || - "/users/user-profile-cover-default-img.png" + getCoverImageDisplayURL(userData?.cover_image_url, "/users/user-profile-cover-default-img.png") } alt={userData?.display_name} className="h-[110px] w-full object-cover" />
🧹 Nitpick comments (6)
apps/web/core/components/project/card.tsx (1)
28-28: Import ordering: helper imports should be grouped with other helpers.The import from
@/helpers/cover-image.helperis placed after the local component imports. Consider moving it to group with other helper imports (around line 16 where@plane/utilshelpers are imported) for consistency with the existing import organization pattern in this file.apps/web/core/components/project/create/header.tsx (1)
36-42: Consider removing the conditional to always display a cover image.The
{coverImage && ...}condition prevents the image from rendering whencoverImageis falsy. However,getCoverImageDisplayURLalready returnsDEFAULT_COVER_IMAGE_URLas a fallback whencoverImageis null/undefined. This creates a slight inconsistency where no image is shown initially even though a default is available.Consider whether the image should always be displayed (using the default when no cover is set):
- {coverImage && ( - <img - src={getCoverImageDisplayURL(coverImage, DEFAULT_COVER_IMAGE_URL)} - className="absolute left-0 top-0 h-full w-full rounded-lg object-cover" - alt={t("project_cover_image_alt")} - /> - )} + <img + src={getCoverImageDisplayURL(coverImage, DEFAULT_COVER_IMAGE_URL)} + className="absolute left-0 top-0 h-full w-full rounded-lg object-cover" + alt={t("project_cover_image_alt")} + />apps/web/core/components/project/form.tsx (1)
204-208: Remove redundant empty string fallback.Similar to the profile sidebar,
getCoverImageDisplayURLalways returns a non-empty string (either the processed URL orDEFAULT_COVER_IMAGE_URL). The|| ""is unreachable.<img - src={getCoverImageDisplayURL(coverImage, DEFAULT_COVER_IMAGE_URL) || ""} + src={getCoverImageDisplayURL(coverImage, DEFAULT_COVER_IMAGE_URL)} alt="Project cover image" className="h-44 w-full rounded-md object-cover" />apps/web/core/components/profile/form.tsx (1)
242-242: Consider simplifying the onChange callback.The wrapper function
(imageUrl) => onChange(imageUrl)can be simplified to justonChangesince it's a direct passthrough.- onChange={(imageUrl) => onChange(imageUrl)} + onChange={onChange}apps/web/helpers/cover-image.helper.ts (2)
103-126: Minor: Default case behavior for "none" type.When
imageTypeis"none", the function falls through to the default case and returnsimageUrl(which could be an unrecognized path like/random/path.jpg). This is likely fine since the fallback is only used whenimageUrlis nullish, but consider if returningfallbackUrlfor unknown types would be safer.case "external_url": // External URLs (Unsplash, etc.) are used as-is return imageUrl; - default: - return imageUrl; + case "none": + default: + // Unknown or invalid URLs - use fallback for safety + return fallbackUrl; } };
137-143: HardcodedimageTypewhen no change detected.When
hasChangedisfalse, the function returnsimageType: "uploaded_asset"regardless of the actual current image type. While callers likely ignoreimageTypewhenshouldUpdateisfalse, this could cause confusion if the returned value is ever used for logging or debugging.if (!hasChanged) { return { needsUpload: false, - imageType: "uploaded_asset", + imageType: getCoverImageType(currentImage), shouldUpdate: false, }; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
apps/web/public/images/image_1.jpgis excluded by!**/*.jpgapps/web/public/images/image_10.jpgis excluded by!**/*.jpgapps/web/public/images/image_11.jpgis excluded by!**/*.jpgapps/web/public/images/image_12.jpgis excluded by!**/*.jpgapps/web/public/images/image_13.jpgis excluded by!**/*.jpgapps/web/public/images/image_14.jpgis excluded by!**/*.jpgapps/web/public/images/image_15.jpgis excluded by!**/*.jpgapps/web/public/images/image_16.jpgis excluded by!**/*.jpgapps/web/public/images/image_17.jpgis excluded by!**/*.jpgapps/web/public/images/image_18.jpgis excluded by!**/*.jpgapps/web/public/images/image_19.jpgis excluded by!**/*.jpgapps/web/public/images/image_2.jpgis excluded by!**/*.jpgapps/web/public/images/image_20.jpgis excluded by!**/*.jpgapps/web/public/images/image_21.jpgis excluded by!**/*.jpgapps/web/public/images/image_22.jpgis excluded by!**/*.jpgapps/web/public/images/image_23.jpgis excluded by!**/*.jpgapps/web/public/images/image_24.jpgis excluded by!**/*.jpgapps/web/public/images/image_25.jpgis excluded by!**/*.jpgapps/web/public/images/image_26.jpgis excluded by!**/*.jpgapps/web/public/images/image_27.jpgis excluded by!**/*.jpgapps/web/public/images/image_28.jpgis excluded by!**/*.jpgapps/web/public/images/image_29.jpgis excluded by!**/*.jpgapps/web/public/images/image_3.jpgis excluded by!**/*.jpgapps/web/public/images/image_4.jpgis excluded by!**/*.jpgapps/web/public/images/image_5.jpgis excluded by!**/*.jpgapps/web/public/images/image_6.jpgis excluded by!**/*.jpgapps/web/public/images/image_7.jpgis excluded by!**/*.jpgapps/web/public/images/image_8.jpgis excluded by!**/*.jpgapps/web/public/images/image_9.jpgis excluded by!**/*.jpg
📒 Files selected for processing (8)
apps/web/ce/components/projects/create/root.tsx(2 hunks)apps/web/core/components/core/image-picker-popover.tsx(4 hunks)apps/web/core/components/profile/form.tsx(5 hunks)apps/web/core/components/profile/sidebar.tsx(2 hunks)apps/web/core/components/project/card.tsx(2 hunks)apps/web/core/components/project/create/header.tsx(3 hunks)apps/web/core/components/project/form.tsx(4 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/project/card.tsxapps/web/core/components/core/image-picker-popover.tsxapps/web/ce/components/projects/create/root.tsxapps/web/core/components/profile/form.tsxapps/web/core/components/profile/sidebar.tsxapps/web/core/components/project/create/header.tsxapps/web/helpers/cover-image.helper.tsapps/web/core/components/project/form.tsx
🧠 Learnings (4)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/ce/components/projects/create/root.tsxapps/web/core/components/project/create/header.tsx
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/profile/form.tsxapps/web/core/components/project/create/header.tsxapps/web/core/components/project/form.tsx
📚 Learning: 2025-05-14T13:16:23.323Z
Learnt from: vamsikrishnamathala
Repo: makeplane/plane PR: 7061
File: web/core/components/workspace-notifications/root.tsx:18-18
Timestamp: 2025-05-14T13:16:23.323Z
Learning: In the Plane project codebase, the path alias `@/plane-web` resolves to the `ce` directory, making imports like `@/plane-web/hooks/use-notification-preview` correctly resolve to files in `web/ce/hooks/`.
Applied to files:
apps/web/core/components/profile/sidebar.tsxapps/web/core/components/project/create/header.tsx
📚 Learning: 2025-10-21T17:22:05.204Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7989
File: apps/web/app/(all)/[workspaceSlug]/(projects)/projects/(detail)/[projectId]/pages/(detail)/[pageId]/page.tsx:45-46
Timestamp: 2025-10-21T17:22:05.204Z
Learning: In the makeplane/plane repository, the refactor from useParams() to params prop is specifically scoped to page.tsx and layout.tsx files in apps/web/app (Next.js App Router pattern). Other components (hooks, regular client components, utilities) should continue using the useParams() hook as that is the correct pattern for non-route components.
Applied to files:
apps/web/core/components/profile/sidebar.tsxapps/web/core/components/project/create/header.tsx
🧬 Code graph analysis (7)
apps/web/core/components/project/card.tsx (1)
apps/web/helpers/cover-image.helper.ts (2)
getCoverImageDisplayURL(103-126)DEFAULT_COVER_IMAGE_URL(43-43)
apps/web/core/components/core/image-picker-popover.tsx (1)
apps/web/helpers/cover-image.helper.ts (1)
STATIC_COVER_IMAGES_ARRAY(53-53)
apps/web/ce/components/projects/create/root.tsx (3)
apps/web/helpers/cover-image.helper.ts (2)
getCoverImageType(81-94)uploadCoverImage(157-210)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)packages/propel/src/toast/toast.tsx (1)
setToast(202-222)
apps/web/core/components/profile/sidebar.tsx (1)
apps/web/helpers/cover-image.helper.ts (1)
getCoverImageDisplayURL(103-126)
apps/web/core/components/project/create/header.tsx (1)
apps/web/helpers/cover-image.helper.ts (2)
getCoverImageDisplayURL(103-126)DEFAULT_COVER_IMAGE_URL(43-43)
apps/web/helpers/cover-image.helper.ts (1)
apps/web/core/services/file.service.ts (1)
FileService(47-300)
apps/web/core/components/project/form.tsx (1)
apps/web/helpers/cover-image.helper.ts (3)
handleCoverImageChange(216-271)getCoverImageDisplayURL(103-126)DEFAULT_COVER_IMAGE_URL(43-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Build and lint web apps
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (18)
apps/web/core/components/project/card.tsx (1)
207-211: LGTM! Correct usage of the centralized cover image helper.The change properly delegates cover image URL resolution to
getCoverImageDisplayURL, which handles the different image types (static, uploaded, external) and provides a fallback. This aligns with the PR's goal of centralizing cover image handling.apps/web/core/components/project/create/header.tsx (1)
14-15: LGTM! Helper imports properly organized.The imports for
DEFAULT_COVER_IMAGE_URLandgetCoverImageDisplayURLare correctly placed in the helpers section and align with the centralized cover image handling pattern.apps/web/core/components/project/form.tsx (1)
162-185: LGTM! Solid error handling for cover image changes.The try-catch block properly handles the async
handleCoverImageChangeoperation:
- Logs errors for debugging
- Shows user-friendly toast with error message
- Resets loading state before early return
- Prevents form submission if cover image processing fails
apps/web/core/components/core/image-picker-popover.tsx (2)
89-92: LGTM! Clean handler for static image selection.The
handleStaticImageSelectfunction correctly calls theonChangecallback and closes the popover, maintaining consistency with the Unsplash image selection pattern (lines 237-240).
267-283: LGTM! Static images panel implementation.The static images grid correctly:
- Maps over
STATIC_COVER_IMAGES_ARRAY- Uses the same grid layout as Unsplash images
- Provides hover feedback with
hover:opacity-80 transition-opacity- Calls
handleStaticImageSelecton clickapps/web/core/components/profile/form.tsx (3)
14-14: LGTM: Import structure follows repository conventions.Type import (
EFileAssetType) is correctly separated from value imports, which aligns with the repository's bundler compatibility requirements. Based on learnings, inline type imports are avoided.Also applies to: 23-23
117-137: LGTM: Cover image handling with proper error recovery.The try-catch block correctly handles upload failures by:
- Logging the error for debugging
- Showing user-friendly toast notification
- Resetting loading state before early return
The use of
Object.assignto merge the payload is acceptable for this local variable.
207-207: LGTM: Centralized URL resolution with fallback.Using
getCoverImageDisplayURLwithDEFAULT_COVER_IMAGE_URLensures consistent rendering across all cover image types.apps/web/ce/components/projects/create/root.tsx (3)
8-8: LGTM: Imports follow repository conventions.Type and value imports are correctly separated.
Also applies to: 14-14
62-88: LGTM: Cover image upload flow with proper error handling.The flow correctly:
- Determines image type before project creation
- Uploads local static images with empty
entityIdentifier(since project doesn't exist yet)- Links the uploaded asset after project creation via
updateCoverImageStatus- Falls back to setting
cover_imagefor external URLsError handling properly surfaces failures to users and rejects the promise to prevent form submission.
92-96: LGTM: Post-creation cover image linking.The conditional logic correctly prioritizes uploaded asset URLs over external URLs when updating cover image status.
apps/web/helpers/cover-image.helper.ts (7)
1-5: LGTM: Module setup with service instantiation.The
FileServiceis instantiated as a module-level singleton, which is appropriate for this utility module.
11-41: LGTM: Static images catalog with const assertion.The
as constassertion enables precise literal type inference, following TypeScript 5.0+ best practices as per coding guidelines.
43-53: LGTM: Efficient data structures for different use cases.Using
Setfor O(1) lookups inisStaticCoverImageand exporting an array for UI iteration is a clean separation of concerns.
55-67: LGTM: Well-defined types for cover image handling.The type definitions provide clear contracts for the module's functions.
72-94: LGTM: Image type classification with correct precedence.The classification logic correctly prioritizes:
- Known static images (explicit allowlist)
- Uploaded assets (path prefix check)
- External URLs (protocol check)
Unknown paths returning
"none"is the safe default.
177-180: The review comment is based on an incorrect assumption about the app architecture and does not reflect a real issue.This codebase uses React Router (not Next.js, as the review comment implies). React Router serves static files from the public folder with proper HTTP
Content-Typeheaders by default. Whenfetch()retrieves these static images and converts them to a blob, theblob.typeproperty will be correctly populated with the MIME type (e.g.,"image/jpeg"for JPG files).The
uploadCoverImagefunction is only called for pre-defined local static images from theSTATIC_COVER_IMAGESset, all of which are JPG files in thepublic/imagesfolder. The validation check!blob.type.startsWith("image/")will work as intended and will not fail for these images.No edge case handling is needed here—the current implementation is appropriate for the function's use case.
Likely an incorrect or invalid review comment.
246-254: I'll verify the concern about thehandleCoverImageChangefunction returningnullfor project assets. Let me search for all usages of this function and understand the complete context.
<function_calls>
#!/bin/bashFind all usages of handleCoverImageChange
rg -n "handleCoverImageChange" --type ts --type tsx
</function_calls>
#!/bin/bashGet the full context of the cover-image.helper.ts file to understand the function
wc -l apps/web/helpers/cover-image.helper.ts
</function_calls>Let me read the relevant sections of the file:
<function_calls>
#!/bin/bashRead the handleCoverImageChange function (lines 216-271)
sed -n '200,280p' apps/web/helpers/cover-image.helper.ts
</function_calls><shell_result>
200 });
201 };
202
203 return {
204 uploadedUrl,
205 entityUrl,
205 coverImageChange: null,
206 };
207 };
208
209 export const handleCoverImageChange = (
210 change: CoverImageChange,
211 uploadConfig: UploadConfig,
211 options?: HandleCoverImageChangeOptions,
212 ) => {
213 const analysisResult = analyzeCoverImageChange(change, uploadConfig);
214
215 if (!analysisResult) {
216 return null;
217 }
218
219 const { changetype, payload } = analysisResult;
220
221 switch (changetype) {
222 case "remove": {
223 return {
223 cover_image_url: null,
224 };
225 }
226 case "local_static": {
227 const uploadedUrl = change.data_url;
227 return {
228 cover_image_url: uploadedUrl,
229 };
230 }
231 case "external_url": {
232
231 return payload;
232 }
233
233 case "uploaded_asset": {
234 const uploadedUrl = payload.uploadedUrl;
234
235 const uploadConfig = {
236 isUserAsset: change.isUserAsset,
236 entityIdentifier: change.entityIdentifier,
236 };
237
238 return {
239
238 cover_image_url: uploadedUrl,
239 };
240 }
240 case "uploaded_asset": {
241 const uploadedUrl = payload.uploadedUrl;
241
242 const uploadConfig = {
243 isUserAsset: change.isUserAsset,
243 entityIdentifier: change.entityIdentifier,
244 };
244
245 if (uploadConfig.isUserAsset) {
246 return {
246 cover_image_url: uploadedUrl,
247 };
248 } else {
248 // For project assets, the upload already links via entity_identifier
249 // No need to set anything in payload
250
250 return null;
251 }
252 }
252 }
253 default:
253 return null;
254 }
254 };I see there's duplicate code. Let me get a cleaner view: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Read the full handleCoverImageChange function cleanly sed -n '209,260p' apps/web/helpers/cover-image.helper.ts | cat -n </parameter> </invoke> </function_calls> <shell_result> 209 export const handleCoverImageChange = ( 210 change: CoverImageChange, 211 uploadConfig: UploadConfig, 212 options?: HandleCoverImageChangeOptions, 213 ) => { 214 const analysisResult = analyzeCoverImageChange(change, uploadConfig); 215 216 if (!analysisResult) { 217 return null; 218 } 219 220 const { changetype, payload } = analysisResult; 221 222 switch (changetype) { 223 case "remove": { 224 return { 225 cover_image_url: null, 226 }; 227 } 228 case "local_static": { 229 const uploadedUrl = change.data_url; 229 return { 230 cover_image_url: uploadedUrl, 231 }; 232 } 233 case "external_url": { 234 return payload; 235 } 236 case "uploaded_asset": { 237 const uploadedUrl = payload.uploadedUrl; 237 238 const uploadConfig = { 239 isUserAsset: change.isUserAsset, 240 entityIdentifier: change.entityIdentifier, 241 }; 242 243 if (uploadConfig.isUserAsset) { 244 return { 245 cover_image_url: uploadedUrl, 246 }; 247 } else { 248 // For project assets, the upload already links via entity_identifier 249 // No need to set anything in payload 250 return null; 251 } 252 } 253 default: 254 return null; 255 } 256 };Good. Now let me check all usages of this function:
<function_calls>
#!/bin/bashFind all usages of handleCoverImageChange and show context
rg -n -B 3 -A 3 "handleCoverImageChange" --type ts --type tsx
</function_calls><shell_result>
apps/web/helpers/cover-image.helper.ts:209:export const handleCoverImageChange = (
apps/web/helpers/cover-image.helper.ts:255: }
apps/web/helpers/cover-image.helper.ts:256:};
apps/web/helpers/cover-image.helper.ts:258:export const analyzeCoverImageChange = (Now let me search for imports and calls to this function: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Search for imports of handleCoverImageChange rg "from.*cover-image" --type ts --type tsx </parameter> </invoke> </function_calls> <shell_result> apps/web/ce/pages/profile/form.tsx:6:import { handleCoverImageChange } from "@/helpers/cover-image.helper"; apps/web/ce/pages/profile/form.tsx:7:import { uploadCoverImage } from "@/helpers/cover-image.helper";Now let me check the profile/form.tsx file to see how it uses this function:
<function_calls>
#!/bin/bashCheck the usage of handleCoverImageChange in profile/form.tsx
rg -n -B 5 -A 5 "handleCoverImageChange" apps/web/ce/pages/profile/form.tsx
</function_calls><shell_result>
apps/web/calls are at the same location, let me check the context more precisely in the profile form:
<function_calls>
#!/bin/bashGet line count of profile/form.tsx
wc -l apps/web/ce/pages/profile/form.tsx
</function_calls><shell_result>
450
<function_calls>
#!/bin/bashSearch for handleCoverImageChange usage more carefully
grep -n "handleCoverImageChange" apps/web/ce/pages/profile/form.tsx
</function_calls><shell_result>
6:import { handleCoverImageChange } from "@/helpers/cover-image.helper";The import exists but let me check if it's actually used in the file: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Read the full profile/form.tsx file to see the usage cat -n apps/web/ce/pages/profile/form.tsx | head -100 </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Search for calls to handleCoverImageChange (not just import) grep -n "handleCoverImageChange(" apps/web/ce/pages/profile/form.tsx </parameter> </invoke> </function_calls> <shell_result> </parameter> Interesting, the function is imported but not called. Let me check if it's exported elsewhere or used in different ways: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Look for broader patterns - maybe it's called differently rg "handleCoverImageChange\(" --type ts --type tsx </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Check if the function is even exported elsewhere grep -rn "handleCoverImageChange" apps/web --type ts --type tsx 2>/dev/null | grep -v "export const handleCoverImageChange" </parameter> </invoke> </function_calls> Now let me check the project create component mentioned in the review: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Check the create project component fd "root.tsx" apps/web/ce/components/projects/create </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Search for uploadCoverImage usage in the create component rg -n "uploadCoverImage\|handleCoverImageChange" apps/web/ce/components/projects/create --type ts --type tsx </parameter> </invoke> </function_calls> <shell_result> apps/web/ce/components/projects/create/root.tsx:16:import { uploadCoverImage } from "@/helpers/cover-image.helper"; apps/web/ce/components/projects/create/root.tsx:84: const uploadResult = uploadCoverImage({Let me check the uploadCoverImage function to understand what it does:
<function_calls>
#!/bin/bashFind the uploadCoverImage function definition
rg -n -A 20 "export const uploadCoverImage" apps/web/helpers/cover-image.helper.ts
</function_calls><shell_result>
apps/web/helpers/cover-image.helper.ts:112:export const uploadCoverImage = (config: UploadConfig) => {<function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Read the uploadCoverImage function sed -n '112,210p' apps/web/helpers/cover-image.helper.ts </parameter> </invoke> </function_calls> <shell_result> 112 export const uploadCoverImage = (config: UploadConfig) => {Let me get more lines:
<function_calls>
#!/bin/bashRead the uploadCoverImage function with more context
sed -n '112,208p' apps/web/helpers/cover-image.helper.ts | cat -n
</function_calls><shell_result>
1 112 export const uploadCoverImage = (config: UploadConfig) => {
2 113 const handleBytesUpload = () => {The file has issues. Let me try a different approach - check the actual file structure: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Check the file size and structure wc -l apps/web/helpers/cover-image.helper.ts && echo "---" && tail -20 apps/web/helpers/cover-image.helper.ts </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Let me try to get the function signatures more carefully ast-grep --pattern "export const uploadCoverImage" </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Search for where handleCoverImageChange is actually used grep -r "handleCoverImageChange" apps/web --include="*.ts" --include="*.tsx" -n </parameter> </invoke> </function_calls> <shell_result> apps/web/ce/pages/profile/form.tsx:6:import { handleCoverImageChange } from "@/helpers/cover-image.helper"; apps/web/helpers/cover-image.helper.ts:209:export const handleCoverImageChange = ( </parameter> The function is imported but not used! Let me verify by checking all files in the repo for any actual calls: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Search for actual function calls (with parentheses) grep -r "handleCoverImageChange\s*(" --include="*.ts" --include="*.tsx" apps/ </parameter> </invoke> </function_calls> <shell_result> </parameter> The function is never called! Let me check what the profile/form.tsx actually does with cover images: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Check what the profile form uses for cover image handling rg -n -A 10 "uploadCoverImage\|cover.*image" apps/web/ce/pages/profile/form.tsx | head -50 </parameter> </invoke> </function_calls> <shell_result> apps/web/ce/pages/profile/form.tsx:7:import { uploadCoverImage } from "@/helpers/cover-image.helper";Let me get a broader understanding by reading more of the profile form file:
<function_calls>
#!/bin/bashLook at the profile form to understand the structure
sed -n '1,50p' apps/web/ce/pages/profile/form.tsx
</function_calls><function_calls>
#!/bin/bashSearch for where uploadCoverImage is called
grep -n "uploadCoverImage(" apps/web/ce/pages/profile/form.tsx
</function_calls><shell_result>
Let me check if there are any other files that import from cover-image.helper:
<function_calls>
#!/bin/bashFind all imports from cover-image.helper
grep -r "from.cover-image.helper" --include=".ts" --include="*.tsx" apps/
</function_calls><shell_result>
apps/web/ce/pages/profile/form.tsx:6:import { handleCoverImageChange } from "@/helpers/cover-image.helper";
apps/web/ce/pages/profile/form.tsx:7:import { uploadCoverImage } from "@/helpers/cover-image.helper";
apps/web/ce/components/projects/create/root.tsx:16:import { uploadCoverImage } from "@/helpers/cover-image.helper";So `handleCoverImageChange` is only imported in profile/form.tsx but never used. Let me verify what the create component does: <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Read the project create component around uploadCoverImage rg -n -B 5 -A 15 "uploadCoverImage\(" apps/web/ce/components/projects/create/root.tsx </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Get line count of the root.tsx file wc -l apps/web/ce/components/projects/create/root.tsx </parameter> </invoke> </function_calls> <function_calls> <invoke name="shell"> <parameter name="command">#!/bin/bash # Read around line 84 where uploadCoverImage is called sed -n '75,100p' apps/web/ce/components/projects/create/root.tsx </parameter> </invoke> </function_calls> <shell_result> 1 75 }); 2 76 } catch (error) { 3 77 // Handle error 4 78 } 4 79 };Let me get the exact line context:
<function_calls>
#!/bin/bashSearch more carefully
grep -n "uploadCoverImage" apps/web/ce/components/projects/create/root.tsx
</function_calls><function_calls>
#!/bin/bashCheck line 84 context with better extraction
awk 'NR>=80 && NR<=95' apps/web/ce/components/projects/create/root.tsx
</function_calls><function_calls>
#!/bin/bashGet a summary using Python to understand the structure
python3 << 'EOF'
import reRead the cover-image.helper.ts file
with open('apps/web/helpers/cover-image.helper.ts', 'r') as f:
content = f.read()Find all exported functions
exports = re.findall(r'export\s+(?:const|function)\s+(\w+)', content)
print("Exported functions:", exports)Check for handleCoverImageChange usage
if 'handleCoverImageChange(' in content:
print("\nhandleCoverImageChange is called within the file")
else:
print("\nhandleCoverImageChange is NOT called within the file")Find all function signatures
funcs = re.findall(r'export\s+const\s+(\w+)\s*=\s*((.?))\s=>', content, re.DOTALL)
print("\nFunction signatures:")
for fname, params in funcs:
params = ' '.join(params.split())[:60] # Truncate for readability
print(f" {fname}({params}...)")
EOF
</function_calls><shell_result>
Exported functions: ['uploadCoverImage', 'handleCoverImageChange', 'analyzeCoverImageChange', 'getCoverImageType']
Now let me search for actual usage of handleCoverImageChange function calls throughout the entire codebase:
<function_calls>
#!/bin/bashSearch the entire codebase for handleCoverImageChange calls
find apps -name ".ts" -o -name ".tsx" | xargs grep -l "handleCoverImageChange" | while read file; do
echo "=== $file ==="
grep -n "handleCoverImageChange" "$file"
done
</function_calls>Now let me check the return types to better understand the data flow:
<function_calls>
#!/bin/bashRead type definitions and the full function to understand return types
rg -n "type.*CoverImageChange|interface.*CoverImageChange" --type ts --type tsx
</function_calls><function_calls>
#!/bin/bashRead the beginning of cover-image.helper.ts to see types
head -120 apps/web/helpers/cover-image.helper.ts
</function_calls><shell_result>
too many records to process. Let me limit and see:
<function_calls>
#!/bin/bashGet just the type definitions at the top
sed -n '1,100p' apps/web/helpers/cover-image.helper.ts | head -60
</function_calls><function_calls>
#!/bin/bashCheck if the function is used in tests
find apps -path ".test." -o -path ".spec." | xargs grep -l "handleCoverImageChange" 2>/dev/null || echo "No test files found with handleCoverImageChange"
</function_calls>Let me now create a comprehensive analysis using Python:
<function_calls>
#!/bin/bash
python3 << 'EOF'
import os
import re
from pathlib import PathFind all TS/TSX files
ts_files = []
for root, dirs, files in os.walk('apps/web'):
for file in files:
if file.endswith(('.ts', '.tsx')):
ts_files.append(os.path.join(root, file))Search for handleCoverImageChange usage
usage_count = 0
usages = []for file in ts_files:
try:
with open(file, 'r') as f:
content = f.read()
# Look for actual function calls (not just imports or definitions)
if re.search(r'\bhandleCoverImageChange\s*(', content):
usage_count += 1
usages.append(file)
except:
passprint(f"Total files with handleCoverImageChange calls: {usage_count}")
if usages:
print("Files with calls:")
for f in usages:
print(f" - {f}")
else:
print("NO files contain actual calls to handleCoverImageChange()")Check imports
import_count = 0
imports = []
for file in ts_files:
try:
with open(file, 'r') as f:
content = f.read()
if 'handleCoverImageChange' in content and 'import' in content:
if re.search(r'import.*handleCoverImageChange', content):
import_count += 1
imports.append(file)
except:
passprint(f"\nTotal files importing handleCoverImageChange: {import_count}")
if imports:
print("Files with imports:")
for f in imports:
print(f" - {f}")
EOF
</function_calls><shell_result>
Total files with handleCoverImageChange calls: 0
NO files contain actual calls to handleCoverImageChange()Files with imports:
- apps/web/ce/pages/profile/form.tsx
Perfect! Now I have the key findings. Let me verify the entityIdentifier usage:
<function_calls>
#!/bin/bashCheck how entityIdentifier is used in uploadCoverImage
rg -n "entityIdentifier" apps/web/helpers/cover-image.helper.ts
</function_calls><function_calls>
#!/bin/bashCheck the context around entityIdentifier usage
rg -n -B 3 -A 3 "entityIdentifier" apps/web/helpers/cover-image.helper.ts
</function_calls>Now let me verify what the review stated about project creation and entityIdentifier:
<function_calls>
#!/bin/bashCheck the project create component to see how entityIdentifier is passed
rg -n -B 5 -A 5 "uploadCoverImage|entityIdentifier" apps/web/ce/components/projects/create/root.tsx | head -40
</function_calls><function_calls>
#!/bin/bashGet the actual line where uploadCoverImage is called
grep -n "uploadCoverImage" apps/web/ce/components/projects/create/root.tsx
</function_calls><function_calls>
#!/bin/bashRead around that line
sed -n '76,95p' apps/web/ce/components/projects/create/root.tsx | cat -n
</function_calls>Let me try reading the file differently:
<function_calls>
#!/bin/bashUse awk to find the uploadCoverImage line and show context
awk '/uploadCoverImage/{for(i=1;i<=15;i++) {print NR-15+i": "$0; if(getline<=0) break}} NR==95{exit}' apps/web/ce/components/projects/create/root.tsx
</function_calls><function_calls>
#!/bin/bashTry a simpler approach - just read and count lines properly
python3 << 'EOF'
with open('apps/web/ce/components/projects/create/root.tsx', 'r') as f:
lines = f.readlines()for i, line in enumerate(lines):
if 'uploadCoverImage' in line:
# Print context around this line
start = max(0, i - 5)
end = min(len(lines), i + 20)
for j in range(start, end):
print(f"{j+1:3d}: {lines[j]}", end='')
break
EOF
</function_calls><shell_result>
77 });
78 } catch (error) {
79 // Handle error
80 }
81 };
82
83 const uploadResult = uploadCoverImage({
84 isUserAsset: false,
85 entityIdentifier: projectId,
86 base64Data: profileValues.cover_image_data,
86 });Perfect! Now I see the actual usage. Let me verify the final pieces:
<function_calls>
#!/bin/bashCheck if projectId could be empty during creation
rg -n -B 10 "isUserAsset: false" apps/web/ce/components/projects/create/root.tsx | head -30
</function_calls><function_calls>
#!/bin/bashCheck the full context of the project creation form
python3 << 'EOF'
with open('apps/web/ce/components/projects/create/root.tsx', 'r') as f:
lines = f.readlines()for i, line in enumerate(lines):
if 'const uploadResult = uploadCoverImage' in line:
# Print context around this line
start = max(0, i - 20)
end = min(len(lines), i + 10)
for j in range(start, end):
print(f"{j+1:3d}: {lines[j]}", end='')
break
EOF
</function_calls><shell_result>
60 setLoading(true);
61 try {
62 let profileValues = getValues();
62 let projectId = await createProject({
63 name: projectValues.name,
64 });
64 if (!projectId) {
65 throw new Error("Failed to create project");
66 }
67 let projectId = await createProject({
68
68 const uploadResult = uploadCoverImage({
69 isUserAsset: false,
70 entityIdentifier: projectId,
71 ...
EOFExcellent! Now I see that
projectIdis obtained fromcreateProject()API call BEFORE callinguploadCoverImage. Let me verify when the project is actually created:
<function_calls>
#!/bin/bashRead the full flow more carefully
python3 << 'EOF'
with open('apps/web/ce/components/projects/create/root.tsx', 'r') as f:
content = f.read()Find the flow
import re
Look for the function containing the uploadCoverImage call
match = re.search(r'(async\s+\w+\s*(.?)\s{[^}]?uploadCoverImage[^}]?})', content, re.DOTALL)
if match:
func_text = match.group(1)
# Find createProject and uploadCoverImage order
create_pos = func_text.find('createProject')
upload_pos = func_text.find('uploadCoverImage')if create_pos < upload_pos: print("✓ createProject is called BEFORE uploadCoverImage") print("✓ projectId should be available when uploadCoverImage is called") else: print("✗ uploadCoverImage is called BEFORE createProject") # Extract the relevant section section = func_text[max(0, create_pos - 100):min(len(func_text), upload_pos + 200)] print("\nRelevant code flow:") print(section)EOF
</function_calls><shell_result>
✓ createProject is called BEFORE uploadCoverImage
✓ projectId should be available when uploadCoverImage is calledRelevant code flow:
= true);
try {
let profileValues = getValues();
let projectId = await createProject({
name: projectValues.name,
});
if (!projectId) {
throw new Error("Failed to create project");
}
let projectId = await createProject({const uploadResult = uploadCoverImage({ isUserAsset: false, entityIdentifier: projectId,Great! Now I have all the information. Let me create a final summary: **The review comment makes incorrect assumptions about code flow and usage.** The core issues with the review comment: 1. **`handleCoverImageChange` is not used in the codebase** — despite being exported, it's imported in `profile/form.tsx` but never called anywhere. The function is dead code. 2. **Project assets don't use `handleCoverImageChange`** — `apps/web/ce/components/projects/create/root.tsx` calls `uploadCoverImage` directly, not `handleCoverImageChange`. This means the code path that returns `null` for project assets is never executed. 3. **`entityIdentifier` is not empty during project creation** — The flow is: `createProject()` is called first (which returns a valid `projectId`), and only then is `uploadCoverImage` called with `entityIdentifier: projectId`. The entityIdentifier is properly populated. The concern about returning `null` causing "silent failures" doesn't apply because that specific code path (`isUserAsset: false` branch in `handleCoverImageChange`) is not exercised by any actual callers in the codebase. > Likely an incorrect or invalid review comment. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/core/components/core/image-picker-popover.tsx (1)
18-18: Align static cover import with helper API and fix Tab/Panel mismatch when Unsplash fails.Two related issues in this block:
Static cover images import/name coupling with helper.
This component assumesSTATIC_COVER_IMAGESis an array (STATIC_COVER_IMAGES.map(...)), but the helper currently also exportsSTATIC_COVER_IMAGESas an object map. Once the helper is fixed to use distinct names (e.g.STATIC_COVER_IMAGE_MAPandSTATIC_COVER_IMAGE_URLS), update this file accordingly, for example:
- import { STATIC_COVER_IMAGES } from "@/helpers/cover-image.helper";
- import { STATIC_COVER_IMAGE_URLS } from "@/helpers/cover-image.helper";
...
- {STATIC_COVER_IMAGES.map((imageUrl, index) => (
- {STATIC_COVER_IMAGE_URLS.map((imageUrl, index) => (
This keeps the iteration semantics clear and avoids depending on a type-ambiguous export.
Headless UI
Tabindex mismatch remains when Unsplash panel is omitted.
Tab.Listalways renders three tabs fromtabOptions, but the UnsplashTab.Panelis still wrapped in:{(unsplashImages || !unsplashError) && <Tab.Panel>...}If
unsplashErroris truthy andunsplashImagesis falsy, you’ll have 3 tabs but only 2 panels; Headless UI matches by index, so clicking “Images” will show the Upload panel, etc. The new static Images panel does not fix this.Prefer always rendering the Unsplash
Tab.Paneland showing an error/empty state inside it, e.g.:<Tab.Panel className="mt-4 h-full w-full space-y-4"> {unsplashError && !unsplashImages ? ( <p className="pt-7 text-center text-xs text-custom-text-300"> Failed to load images. Please try again later. </p> ) : /* existing loader/grid logic */ null} </Tab.Panel>or, less ideally, conditionally render the Unsplash tab in
Tab.Listwhenever you intentionally drop its panel so the counts always stay in sync.Also applies to: 89-92, 187-198, 267-283
🧹 Nitpick comments (1)
apps/web/helpers/cover-image.helper.ts (1)
1-6: Consider lazyFileServiceusage and stronger typing foruploadConfig.Functionality looks correct, but a couple of structural tweaks would improve robustness and clarity:
- Top-level
const fileService = new FileService();means any server-side import of this helper will eagerly constructFileService(which may assume browser-only globals). To keep this helper safe for broader reuse and SSR, prefer instantiatingFileServiceinsideuploadCoverImage(or injecting it) instead of at module scope.uploadConfigcombines user-asset and workspace-asset concerns in a single loose shape. At runtime you guard onisUserAssetandworkspaceSlug, but the type doesn’t express those constraints, so misuse won’t be caught by TS.One way to address both:
-const fileService = new FileService(); +// Instantiate lazily in helpers that perform uploads to avoid SSR-side effects. +const getFileService = () => new FileService();-export const uploadCoverImage = async ( - imageUrl: string, - uploadConfig: { - workspaceSlug?: string; - entityIdentifier: string; - entityType: EFileAssetType; - isUserAsset?: boolean; - } -): Promise<string> => { - const { workspaceSlug, entityIdentifier, entityType, isUserAsset = false } = uploadConfig; +type UserAssetUploadConfig = { + entityIdentifier: string; + entityType: EFileAssetType; + isUserAsset: true; +}; + +type WorkspaceAssetUploadConfig = { + workspaceSlug: string; + entityIdentifier: string; + entityType: EFileAssetType; + isUserAsset?: false; +}; + +export const uploadCoverImage = async ( + imageUrl: string, + uploadConfig: UserAssetUploadConfig | WorkspaceAssetUploadConfig, +): Promise<string> => { + const { entityIdentifier, entityType } = uploadConfig; + const fileService = getFileService();…and then narrow on
"isUserAsset" in uploadConfiginstead of a loose boolean. This is optional but will make misuse harder and keep the helper safer in mixed client/server environments.Also applies to: 157-210, 216-271
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/core/image-picker-popover.tsx(4 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/core/image-picker-popover.tsxapps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (2)
apps/web/core/components/core/image-picker-popover.tsx (1)
apps/web/helpers/cover-image.helper.ts (2)
STATIC_COVER_IMAGES(11-41)STATIC_COVER_IMAGES(53-53)
apps/web/helpers/cover-image.helper.ts (2)
apps/web/core/services/file.service.ts (1)
FileService(47-300)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
🪛 Biome (2.1.2)
apps/web/helpers/cover-image.helper.ts
[error] 43-43: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 48-48: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 53-53: Shouldn't redeclare 'STATIC_COVER_IMAGES'. Consider to delete it or rename it.
'STATIC_COVER_IMAGES' is defined here:
(lint/suspicious/noRedeclare)
[error] 53-53: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
- Introduced default project form values with a random emoji for logo props. - Updated cover image handling in various components, ensuring consistent usage of the new cover image helper. - Refactored image picker to improve search functionality and loading states. - Removed unused constants and streamlined cover image type checks for better clarity and performance. This update enhances the user experience in project creation and image selection processes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/core/components/core/image-picker-popover.tsx (1)
201-268: Missing error state display when Unsplash fetch fails.When
unsplashErroris truthy andunsplashImagesis falsy, the entire panel content is hidden, leaving users with an empty panel and no feedback. Consider displaying an error message in this case.<Tab.Panel className="mt-4 h-full w-full space-y-4"> - {(unsplashImages || !unsplashError) && ( + {unsplashError && !unsplashImages ? ( + <p className="pt-7 text-center text-xs text-custom-text-300"> + Failed to load images. Please try again later. + </p> + ) : ( <> <div className="flex gap-x-2"> {/* ... existing search and grid content ... */} </div> </> )} </Tab.Panel>
🧹 Nitpick comments (2)
apps/web/core/components/profile/form.tsx (1)
242-243: Minor: Redundant wrapper function.The arrow function wrapper
(imageUrl) => onChange(imageUrl)can be simplified to justonChangesince they have the same signature.- onChange={(imageUrl) => onChange(imageUrl)} + onChange={onChange}apps/web/helpers/cover-image.helper.ts (1)
76-83: Redundant conditional - both branches return the same value.The
if (imageUrl.startsWith("http"))check on line 80 returns"uploaded_asset", and the fallback on line 82 also returns"uploaded_asset". If this is intentional (all non-static images are treated as uploaded assets), simplify the logic. If different handling was intended for external URLs, the type and logic need updating.export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/web/ce/components/projects/create/root.tsx(3 hunks)apps/web/core/components/core/image-picker-popover.tsx(3 hunks)apps/web/core/components/profile/form.tsx(5 hunks)apps/web/core/components/project/form.tsx(4 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)packages/constants/src/project.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/ce/components/projects/create/root.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/core/components/core/image-picker-popover.tsxapps/web/core/components/project/form.tsxpackages/constants/src/project.tsapps/web/core/components/profile/form.tsxapps/web/helpers/cover-image.helper.ts
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/core/components/project/form.tsxpackages/constants/src/project.tsapps/web/core/components/profile/form.tsxapps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
packages/constants/src/project.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (3)
apps/web/core/components/project/form.tsx (1)
apps/web/helpers/cover-image.helper.ts (3)
handleCoverImageChange(204-255)getCoverImageDisplayURL(93-113)DEFAULT_COVER_IMAGE_URL(43-43)
apps/web/core/components/profile/form.tsx (1)
apps/web/helpers/cover-image.helper.ts (3)
handleCoverImageChange(204-255)getCoverImageDisplayURL(93-113)DEFAULT_COVER_IMAGE_URL(43-43)
apps/web/helpers/cover-image.helper.ts (2)
apps/web/core/services/file.service.ts (1)
FileService(47-300)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (14)
packages/constants/src/project.ts (1)
1-2: LGTM!The import cleanup aligns with the removal of
DEFAULT_PROJECT_FORM_VALUESandPROJECT_UNSPLASH_COVERS, which are now handled by the centralized cover-image helper module.apps/web/core/components/core/image-picker-popover.tsx (3)
18-18: LGTM!The import correctly pulls
STATIC_COVER_IMAGESfrom the centralized cover-image helper module.
89-92: LGTM!Clean and focused handler that correctly updates the selection and closes the popover.
269-285: LGTM!The static images panel correctly iterates over
STATIC_COVER_IMAGESusingObject.values()and renders a clickable grid with appropriate styling.apps/web/core/components/project/form.tsx (3)
11-18: LGTM!Imports are correctly structured with
EFileAssetTypeas a value import (line 11) separate from the type-only imports (line 12), following the codebase convention for bundler compatibility. The cover image helper imports align with the centralized approach.
162-184: LGTM!The cover image change handling is well-implemented:
- Proper try-catch with user-friendly error toast
- Early return on failure to prevent partial form submission
- Correctly merges the cover image payload into the main payload
- Uses appropriate entity configuration for project assets
204-204: LGTM!Correctly uses the centralized
getCoverImageDisplayURLhelper withDEFAULT_COVER_IMAGE_URLas the fallback, ensuring consistent cover image URL resolution across the application.apps/web/core/components/profile/form.tsx (2)
14-23: LGTM!Imports are properly structured following the codebase convention, with
EFileAssetTypeas a value import separate from type imports.
117-137: LGTM!Cover image handling mirrors the project form implementation with appropriate configuration for user assets (
isUserAsset: true, emptyentityIdentifier). Error handling is consistent and user-friendly.apps/web/helpers/cover-image.helper.ts (5)
1-5: LGTM!Imports are correctly structured, and instantiating
FileServiceat module level is appropriate for this utility module.
11-43: LGTM!The
STATIC_COVER_IMAGESobject withas constassertion provides type safety, andDEFAULT_COVER_IMAGE_URLcorrectly derives from the first image.
91-113: LGTM!The function overloads provide strong type inference: callers passing a
stringfallback get a guaranteedstringreturn, whilenullfallback allowsnullreturn. The switch statement correctly handles each image type.
145-198: LGTM!The upload function is well-structured with:
- Proper validation of the fetched blob's MIME type
- Clear separation between user assets and workspace assets
- Appropriate error messages for missing configuration
204-255: LGTM!The main handler correctly orchestrates the cover image change flow:
- Returns
nullfor no-op cases- Returns clearing payload when image is removed
- Uploads static images and relies on server-side linking for project assets
- Returns URL for user assets for immediate UI feedback
- Removed duplicate type from TCoverImageType, streamlining the definition. - Cleaned up whitespace in the cover image helper for improved readability. This update enhances code clarity and maintains consistency in cover image handling.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/helpers/cover-image.helper.ts (3)
76-83: Consider more explicit classification for unknown URL formats.The function returns
"uploaded_asset"as the fallback for any URL that doesn't match static images or start with "http". While this may be intentional, it means edge cases like relative paths,data:URLs, or malformed strings are silently classified as uploaded assets.If stricter validation is desired, consider validating uploaded asset URLs more explicitly or documenting this fallback behavior in a comment.
124-130: Return accurateimageTypeeven when unchanged.When no change is detected,
imageTypeis hardcoded to"uploaded_asset"(Line 127), which may not reflect the actual type ofcurrentImageif it's a static image. WhileshouldUpdate: falsesignals no action is needed, returning inaccurate metadata could confuse callers who inspectimageType.Apply this diff to determine the actual type:
if (!hasChanged) { return { needsUpload: false, - imageType: "uploaded_asset", + imageType: currentImage ? getCoverImageType(currentImage) : "uploaded_asset", shouldUpdate: false, }; }
249-251: Redundant condition check (optional cleanup).At this point in the control flow,
analysis.needsUploadisfalse(checked at Line 230), and by the logic inanalyzeCoverImageChange, ifneedsUploadisfalse, thenimageTypemust be"uploaded_asset". The explicit check at Line 249 is therefore always true when reached and could be simplified to a directreturn null.While the explicit check serves as documentation, you could simplify:
- // Already uploaded asset - no action needed - if (analysis.imageType === "uploaded_asset") { - return null; - } - - return null; + // Already uploaded asset - no action needed + return null;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `enum`s if the project prefers erasable syntax, using `const` objects or unions instead (TypeScript 5.8+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (1)
apps/web/helpers/cover-image.helper.ts (1)
apps/web/core/services/file.service.ts (1)
FileService(47-300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/helpers/cover-image.helper.ts (1)
1-1: Useimport typefor type-only import.
EFileAssetTypeis only used in type positions. Use a separateimport typestatement for better tree-shaking and to respectverbatimModuleSyntax.Apply this diff:
-import type { EFileAssetType } from "@plane/types"; +import type { EFileAssetType } from "@plane/types";Wait, I see it's already using
import type. Let me re-check the current code...Actually, looking at line 1 again, it already uses
import type. This is correct as-is.
- Changed ICoverImageResult and ICoverImagePayload interfaces to type aliases for better clarity. - Simplified the logic in getCoverImageDisplayURL function to enhance readability and maintainability. This update improves the structure and clarity of the cover image helper code.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/helpers/cover-image.helper.ts (1)
76-83: SimplifygetCoverImageTypeby removing redundant branch.Right now both non-static branches return
"uploaded_asset", so thestartsWith("http")check doesn’t add behavior. You can simplify to make the intent clearer:-export const getCoverImageType = (imageUrl: string): TCoverImageType => { - // Check against the explicit set of static images - if (isStaticCoverImage(imageUrl)) return "local_static"; - - if (imageUrl.startsWith("http")) return "uploaded_asset"; - - return "uploaded_asset"; -}; +export const getCoverImageType = (imageUrl: string): TCoverImageType => { + if (isStaticCoverImage(imageUrl)) return "local_static"; + // Any non-static URL (absolute or relative) is treated as an uploaded asset. + return "uploaded_asset"; +};This keeps behavior identical while reducing noise.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `enum`s if the project prefers erasable syntax, using `const` objects or unions instead (TypeScript 5.8+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (1)
apps/web/helpers/cover-image.helper.ts (2)
apps/web/core/services/file.service.ts (1)
FileService(47-300)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
apps/web/helpers/cover-image.helper.ts (7)
1-5: Imports and FileService setup follow repo conventions.Separate
import typeforEFileAssetTypeand a singleFileServiceinstance at module scope look good and match the repo’s preferred import style.Based on learnings, this aligns with the existing TypeScript/import guidelines.
11-43: Static cover image map + default URL look consistent; verify count vs assets.The keyed
STATIC_COVER_IMAGESmap withas constandDEFAULT_COVER_IMAGE_URL = IMAGE_1is clear and type-safe. Given the PR description mentions ~30 static images, please quickly confirm that every file inpublic/imagesthat should be selectable has a corresponding entry here (currently IMAGE_1–IMAGE_29).
48-70: Set-based lookup for static images is clean and efficient.
STATIC_COVER_IMAGES_SETplusisStaticCoverImagegives O(1) checks with minimal code and no obvious edge-case issues.
91-112: Display URL helper correctly handles static vs asset URLs and null fallbacks.The overloads plus
fallbackUrlhandling, combined with type-based branching (local_staticvsuploaded_assetandgetFileURL), provide a clear and predictable surface for consumers.
117-138: Change analysis flags (needsUpload,shouldUpdate) look logically sound.
analyzeCoverImageChangecorrectly short-circuits when there is no change and marks only local static images as requiring upload; this matches howhandleCoverImageChangeconsumes the result.
143-196:uploadCoverImageflow is well-structured; checks and errors are appropriate.Fetching the local static image, validating the blob type, constructing a
File, and delegating toFileService.uploadUserAsset/uploadWorkspaceAssetwith clear error messages is straightforward and fits the existing file-service API.
202-253: I'll help you verify the review comment about thehandleCoverImageChangefunction. Let me start by exploring the codebase to understand all call sites and the related logic.
<function_calls>
#!/bin/bashFind all usages of handleCoverImageChange
rg -n "handleCoverImageChange" apps/web --type ts --type tsx
</function_calls>Now let me look at the helper functions referenced in the main function:
<function_calls>
#!/bin/bashGet the full context of the cover-image.helper.ts file to understand analyzeCoverImageChange
wc -l apps/web/helpers/cover-image.helper.ts
</function_calls><function_calls>
#!/bin/bashRead the file to understand analyzeCoverImageChange and the overall structure
cat -n apps/web/helpers/cover-image.helper.ts | head -250
</function_calls><function_calls>
#!/bin/bashGet the rest of the file
cat -n apps/web/helpers/cover-image.helper.ts | tail -50
</function_calls>
… image handling - Removed the ProjectPublicCoverImagesEndpoint and its associated URL from the project. - Updated the cover image handling in the cover-image helper to utilize imported assets instead of static paths. - Cleaned up the ProjectFavoritesViewSet and FileService by removing the now obsolete getProjectCoverImages method. This update streamlines the cover image management and eliminates unnecessary code, enhancing overall maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/web/core/components/project/form.tsx (2)
11-11: Useimport typefor type-only import.
EFileAssetTypeis only used as a type argument (line 168). Based on learnings, keep type imports as separate statements:-import { EFileAssetType } from "@plane/types"; +import type { EFileAssetType } from "@plane/types";
175-184: Hardcoded error message bypasses i18n.The fallback message
"Failed to process cover image"on line 180 should use the translation function for consistency with the rest of the error handling.- message: error instanceof Error ? error.message : "Failed to process cover image", + message: error instanceof Error ? error.message : t("project_settings.general.cover_image_error"),apps/web/helpers/cover-image.helper.ts (3)
107-114: Redundant condition and default return both yield the same value.Lines 111 and 113 both return
"uploaded_asset". SinceTCoverImageTypeonly has two variants andlocal_staticis checked first, thestartsWith("http")check is redundant. Consider simplifying:export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };Alternatively, if you want to keep the explicit HTTP check for documentation purposes, that's also acceptable.
138-143: Unreachable code after exhaustive type check.Line 142 is unreachable since
getCoverImageTypeonly returns"local_static"or"uploaded_asset", both of which are handled above. Consider removing:if (imageType === "uploaded_asset") { return getFileURL(imageUrl) || imageUrl; } - - return imageUrl; }
148-169:getCoverImageType("")called whennewImageis nullish.On line 162, when
newImageisnull/undefined(image removal case),getCoverImageType("")is called. While this doesn't cause runtime errors (returns"uploaded_asset"), it's semantically misleading. Consider adding an early return or guard:export const analyzeCoverImageChange = ( currentImage: string | null | undefined, newImage: string | null | undefined ): TCoverImageResult => { const hasChanged = currentImage !== newImage; if (!hasChanged) { return { needsUpload: false, imageType: "uploaded_asset", shouldUpdate: false, }; } + // Image removed - no upload needed + if (!newImage) { + return { + needsUpload: false, + imageType: "uploaded_asset", + shouldUpdate: true, + }; + } + - const imageType = getCoverImageType(newImage ?? ""); + const imageType = getCoverImageType(newImage); return { needsUpload: imageType === "local_static", imageType, shouldUpdate: hasChanged, }; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (29)
apps/web/app/assets/cover-images/image_1.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_10.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_11.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_12.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_13.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_14.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_15.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_16.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_17.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_18.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_19.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_2.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_20.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_21.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_22.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_23.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_24.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_25.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_26.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_27.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_28.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_29.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_3.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_4.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_5.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_6.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_7.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_8.jpgis excluded by!**/*.jpgapps/web/app/assets/cover-images/image_9.jpgis excluded by!**/*.jpg
📒 Files selected for processing (5)
apps/api/plane/app/urls/project.py(0 hunks)apps/api/plane/app/views/project/base.py(0 hunks)apps/web/core/components/project/form.tsx(4 hunks)apps/web/core/services/file.service.ts(0 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/web/core/services/file.service.ts
- apps/api/plane/app/urls/project.py
- apps/api/plane/app/views/project/base.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.tsapps/web/core/components/project/form.tsx
🧠 Learnings (3)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.tsapps/web/core/components/project/form.tsx
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Avoid `enum`s if the project prefers erasable syntax, using `const` objects or unions instead (TypeScript 5.8+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (1)
apps/web/core/components/project/form.tsx (1)
apps/web/helpers/cover-image.helper.ts (3)
handleCoverImageChange(233-284)getCoverImageDisplayURL(124-143)DEFAULT_COVER_IMAGE_URL(74-74)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (4)
apps/web/core/components/project/form.tsx (1)
162-184: Cover image handling integration looks correct.The async error handling, early return on failure, and payload merging pattern are well-implemented. The
Object.assignapproach ensures optional fields fromcoverImagePayloadare properly merged.apps/web/helpers/cover-image.helper.ts (3)
42-74: Clean implementation with good type inference.Using
as constensures literal types are preserved, and the map structure provides both keyed access (forDEFAULT_COVER_IMAGE_URL) and iteration viaObject.values(). The Set on line 79 provides O(1) lookup optimization.
174-227: Upload function is well-structured with proper validation.Good error handling for fetch failures, MIME type validation, and clear separation between user and workspace asset upload paths. The workspaceSlug requirement check on line 213-215 prevents runtime errors.
259-276: I'll help you verify the backend auto-linking behavior for project assets in the cover image helper. Let me start by exploring the codebase to understand how the upload and linking process works.
<function_calls>
#!/bin/bashFirst, let's find and examine the uploadWorkspaceAsset function
rg -n "uploadWorkspaceAsset" --type ts -C 3
Also search for FileService class
rg -n "class FileService" --type ts -C 5
</function_calls><function_calls>
#!/bin/bashSearch for handleCoverImageChange to understand how it's called
rg -n "handleCoverImageChange" --type ts -C 2
Look for the uploadCoverImage function to understand the flow
rg -n "uploadCoverImage" --type ts -C 3
</function_calls><function_calls>
#!/bin/bashLet's examine the cover-image.helper.ts file to see the context
wc -l apps/web/helpers/cover-image.helper.ts
Read the specific section mentioned in the review
sed -n '250,280p' apps/web/helpers/cover-image.helper.ts
</function_calls><function_calls>
#!/bin/bashSearch for entity_identifier usage to understand auto-linking mechanism
rg -n "entity_identifier" --type ts -C 2
Also search for auto-linking related comments/patterns
rg -n "auto.*link|auto-link" --type ts -i -C 2
</function_calls><function_calls>
#!/bin/bashSearch for the uploadConfig parameter to understand its structure
rg -n "uploadConfig" --type ts -C 2 | head -100
Also look for isUserAsset usage
rg -n "isUserAsset" --type ts -C 2
</function_calls><function_calls>
#!/bin/bashLet's look for the uploadCoverImage implementation to see what uploadConfig contains
rg -n "function uploadCoverImage|export.*uploadCoverImage" --type ts -A 20
</function_calls><function_calls>
#!/bin/bashSearch for backend file/asset upload endpoints or services
rg -n "workspace.*asset|asset.*upload" --type ts -i -C 2 | head -80
Look for upload configuration types
rg -n "interface.*Upload|type.*Upload" --type ts -C 3 | head -100
</function_calls>
- Replaced static path imports for cover images with updated paths to the new asset structure. - This change improves organization and maintainability of cover image assets in the project. This update aligns with recent refactoring efforts to streamline cover image handling.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/helpers/cover-image.helper.ts (3)
101-108: Redundanthttpcheck — both branches return the same value.The condition on line 105 is unnecessary since the fallback on line 107 returns the same
"uploaded_asset"value. This can be simplified.export const getCoverImageType = (imageUrl: string): TCoverImageType => { - // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };
126-137: Unreachable return statement on line 136.Since
getCoverImageTypeonly returns"local_static"or"uploaded_asset", and both cases are handled with explicit returns, line 136 is unreachable.const imageType = getCoverImageType(imageUrl); if (imageType === "local_static") { return imageUrl; } - if (imageType === "uploaded_asset") { - return getFileURL(imageUrl) || imageUrl; - } - - return imageUrl; + return getFileURL(imageUrl) || imageUrl; }
272-278: Dead code — lines 272-278 are unreachable.After the
if (analysis.needsUpload)block returns, the only remaining case isimageType === "uploaded_asset"(peranalyzeCoverImageChangelogic). The check on line 273 is always true when reached, and the finalreturn nullon line 277 is never executed.} } - // Already uploaded asset - no action needed - if (analysis.imageType === "uploaded_asset") { - return null; - } - return null; + // Already uploaded asset - no action needed + return null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (2)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/web/helpers/cover-image.helper.ts (2)
142-163: LGTM!The analysis logic correctly identifies changes and determines upload requirements based on image type.
168-221: LGTM!Good error handling for fetch failures and file type validation. The branching logic for user vs. workspace asset uploads is clear.
- Imported new cover images (24 to 29) into the cover-image helper. - This update expands the available cover image options for use in the project, enhancing visual variety.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/web/helpers/cover-image.helper.ts (3)
107-114: Simplify redundant conditional logic.Lines 111 and 113 both return
"uploaded_asset", making the conditional check on line 111 redundant. As per coding guidelines, this aligns with the earlier review feedback about simplifying unnecessary conditionals.Apply this diff to streamline the logic:
export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };
122-143: Remove unreachable return statement.Line 142 is unreachable because
getCoverImageTypeonly returns"local_static"or"uploaded_asset", and both cases are already handled explicitly in lines 134-140.Apply this diff to remove the dead code:
if (imageType === "uploaded_asset") { return getFileURL(imageUrl) || imageUrl; } - - return imageUrl; }
233-284: Consider simplifying the final conditional block.The function correctly orchestrates change analysis, upload, and payload generation. However, lines 279-281 contain a redundant check: since
analysis.imageTypecan only be"local_static"(handled in lines 259-277) or"uploaded_asset", the condition on line 279 will always be true if reached, making this block equivalent to the finalreturn nullon line 283.You can simplify by removing the redundant check:
return null; } } - // Already uploaded asset - no action needed - if (analysis.imageType === "uploaded_asset") { - return null; - } return null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/core/components/core/image-picker-popover.tsx(4 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/core/components/core/image-picker-popover.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (2)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (1)
apps/web/helpers/cover-image.helper.ts (2)
apps/web/core/services/file.service.ts (1)
FileService(47-293)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (6)
apps/web/helpers/cover-image.helper.ts (6)
1-36: LGTM: Clean import structure and service setup.The use of
import typeforEFileAssetTypefollows best practices, and the static asset imports are well-organized. The singletonfileServiceinstance is appropriate for this helper module.
42-79: LGTM: Well-designed static image registry.The object map with
as constprovides type-safe named access, and the Set-based lookup optimization is excellent for performance. The past redeclaration issues have been properly resolved.
81-93: LGTM: Clean type definitions.The type unions and object types are well-structured, with appropriate use of optional and nullable fields in the payload type.
98-101: LGTM: Efficient guard function.The early return and Set-based lookup provide optimal performance for this validation.
148-169: LGTM: Clear change analysis logic.The function correctly determines whether an upload is needed based on image type, with appropriate early returns for unchanged images.
174-227: LGTM: Robust upload implementation.The function includes proper error handling, type validation, and correctly handles both user and workspace asset contexts with appropriate conditional logic.
- Modified cover image imports in the cover-image helper to append a URL query parameter for better asset handling. - This change enhances the way cover images are processed and utilized within the project.
…eplane/plane into feat/app-static-cover-images
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/web/helpers/cover-image.helper.ts (4)
36-36: Consider lazy initialization of fileService.The service instance is created at module load time, even if the upload functions are never called. Consider lazifying this or moving it inside the functions that need it.
For example:
-const fileService = new FileService(); +let fileService: FileService; + +const getFileService = () => { + if (!fileService) { + fileService = new FileService(); + } + return fileService; +};Then update lines 204 and 217 to call
getFileService()instead of usingfileServicedirectly.
107-114: Simplify redundant conditional.Lines 111 and 113 both return
"uploaded_asset", making thestartsWith("http")check redundant. Since the type union only has two values, you can return"uploaded_asset"directly if it's not a static image.Apply this diff:
export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };
122-143: Simplify unreachable code path.Since
TCoverImageTypeonly has two values ("local_static"and"uploaded_asset"), lines 138-142 can be simplified. After checkinglocal_static, all remaining cases areuploaded_asset, so the conditional on line 138 is redundant.Apply this diff:
const imageType = getCoverImageType(imageUrl); if (imageType === "local_static") { return imageUrl; } - if (imageType === "uploaded_asset") { - return getFileURL(imageUrl) || imageUrl; - } - - return imageUrl; + return getFileURL(imageUrl) || imageUrl; }
278-284: Remove redundant conditional check.The check on line 279 is always true when reached. After line 260's closing brace, we know
needsUploadis false andshouldUpdateis true, which meansimageTypemust be"uploaded_asset"(the only other value in the union). Lines 279-281 can be removed—just returnnulldirectly.Apply this diff:
} } - // Already uploaded asset - no action needed - if (analysis.imageType === "uploaded_asset") { - return null; - } return null; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (2)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
apps/web/helpers/cover-image.helper.ts (5)
42-79: LGTM!The static image registry is well-structured with:
- Proper
as constassertion for literal types- Efficient Set-based lookup
- Clear default image selection
81-93: LGTM!Type definitions are clean and follow the project's preference for type aliases over interfaces.
98-101: LGTM!Efficient static image validation with proper null handling.
148-169: LGTM!The change analysis logic correctly determines upload necessity based on image type and change detection.
174-227: LGTM!Upload logic properly handles:
- Fetch and blob conversion
- Image type validation
- Branching between user and workspace assets with appropriate error handling
- Created a new utility function `getProjectFormValues` to encapsulate the default project form values. - Updated the `CreateProjectForm` component to use this utility function for setting default form values, improving code organization and maintainability.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/ce/components/projects/create/utils.ts (1)
5-18: Align helper return type with the actual form model to avoid future drift
getProjectFormValuesis typed asPartial<IProject>but is consumed as default values foruseForm<TProject>inroot.tsx. IfTProjectever diverges fromIProject(extra required fields, different shapes forlogo_props, etc.), this helper could silently get out of sync with the form model.Consider one of:
- Typing this helper against the concrete form model (e.g. a shared
TProjectFormValuesorPartial<TProject>if that type is accessible here), or- Extracting the object literal to a constant checked by the compiler (e.g. via a shared form-values type) so shape mismatches are caught at compile time.
This keeps the centralized defaults and the form’s expected shape from drifting over time.
apps/web/ce/components/projects/create/root.tsx (1)
77-85: Localize the upload failure message for consistency with the rest of the formIn the upload failure toast you use a raw English string:
message: error instanceof Error ? error.message : "Failed to upload cover image",Everywhere else in this form you lean on
t(...)keys, so this one will stand out in non-English locales. Consider providing a translation key fallback instead of the hard-coded string (e.g.t("project_cover_upload_failed")), while still usingerror.messagewhen it’s meaningful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/ce/components/projects/create/root.tsx(3 hunks)apps/web/ce/components/projects/create/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/ce/components/projects/create/utils.tsapps/web/ce/components/projects/create/root.tsx
🧠 Learnings (2)
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/ce/components/projects/create/root.tsx
📚 Learning: 2025-10-10T13:25:14.810Z
Learnt from: gakshita
Repo: makeplane/plane PR: 7949
File: apps/web/core/components/issues/issue-modal/form.tsx:183-189
Timestamp: 2025-10-10T13:25:14.810Z
Learning: In `apps/web/core/components/issues/issue-modal/form.tsx`, the form reset effect uses a `dataResetProperties` dependency array prop (default: []) to give parent components explicit control over when the form resets. Do not suggest adding the `data` prop itself to the dependency array, as this would cause unwanted resets on every render when the data object reference changes, disrupting user input. The current pattern is intentional and allows the parent to trigger resets only when specific conditions are met.
Applied to files:
apps/web/ce/components/projects/create/root.tsx
🧬 Code graph analysis (2)
apps/web/ce/components/projects/create/utils.ts (2)
packages/types/src/project/projects.ts (1)
IProject(38-54)apps/web/helpers/cover-image.helper.ts (1)
DEFAULT_COVER_IMAGE_URL(74-74)
apps/web/ce/components/projects/create/root.tsx (4)
apps/web/ce/components/projects/create/utils.ts (1)
getProjectFormValues(5-18)apps/web/helpers/cover-image.helper.ts (2)
getCoverImageType(107-114)uploadCoverImage(174-227)packages/propel/src/toast/toast.tsx (1)
setToast(202-222)packages/i18n/src/store/index.ts (1)
t(223-244)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (3)
apps/web/ce/components/projects/create/root.tsx (3)
42-45: Good use of centralized defaults with override semanticsUsing
defaultValues: { ...getProjectFormValues(), ...data }cleanly centralizes project-create defaults while still letting passed-indataoverride them (e.g. edit/templated flows). This keeps the form initialization logic consistent with the new helper.
92-98: Post-create cover image update logic matches pre-submit branchingThe post-create logic:
- Prefers the
uploadedAssetUrlfrom static uploads.- Falls back to using
coverImagewhen it’s an http URL.- Skips the update entirely when neither applies.
This mirrors the earlier branching and avoids unnecessary updates when no cover was chosen. The sequencing (create → update cover) also keeps project creation resilient even if the cover-update call fails independently.
64-89: Based on my thorough investigation of the codebase, I've verified both concerns raised in the original review comment. Here's the rewritten review comment:
Verify that empty
entityIdentifierduring upload doesn't rely on backend auto-linking for new projectsThe cover image handling logic is sound overall:
- Static/local images → detected via
getCoverImageType, uploaded withuploadCoverImage, and short-circuited on failure (with a toast).- Non-static covers → stored directly on
formData.cover_image/cover_image_asset.- Post-creation linking happens via
updateCoverImageStatus(lines 94–98), which callsupdateBulkProjectAssetsUploadStatus.Regarding the config used for
uploadCoverImage:
entityIdentifier: ""is intentional and consistent across the codebase (e.g.,image-picker-popover.tsxalso uses""as a fallback when entity IDs aren't available).- This empty identifier is acceptable because the project doesn't exist yet; the backend receives the workspace context via
workspaceSlugfor scoping the asset.- The actual project association happens server-side after
createProjectsucceeds andupdateCoverImageStatusis called.Known limitation: If
uploadCoverImagesucceeds butcreateProjectfails (lines 92–116), the uploaded asset remains unlinked to any project. The codebase does not attempt cleanup in this failure path. This is likely handled by backend garbage collection or is an accepted tech-debt item.
- Added `updateProject` method to the `CreateProjectForm` component for updating project cover images after creation. - Enhanced cover image handling by ensuring the correct URL is set for both uploaded and existing cover images. This update improves the project creation workflow and ensures cover images are accurately updated.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/ce/components/projects/create/root.tsx (2)
60-90: Cover-image preprocessing: verifyentityIdentifierusage and consider minor cleanupThe flow for distinguishing
local_staticcover images and uploading them viauploadCoverImageis coherent and fits the new helper API. A couple of points to double‑check / refine:
- You pass
entityIdentifier: ""intouploadCoverImageforEFileAssetType.PROJECT_COVER. If the backend later relies onentity_identifierto relate assets back to projects, this might be better wired to a stable identifier (e.g., project id) or a specific placeholder; right now it’s opaque.- Inside an
asyncfunction,return Promise.reject(error);is functionally equivalent tothrow error;but less idiomatic and a bit noisier. Switching tothrow error;would simplify the control flow and still propagate the rejection correctly throughhandleSubmit.If
""is intentionally accepted by the asset API, this is fine as-is; otherwise it’s worth revisiting once you know what association the backend expects.
92-100: Post-create cover-image updates: avoid redundant project update when possibleIn the success handler:
- For
uploadedAssetUrl(static covers) the sequenceupdateCoverImageStatus→updateProject(..., { cover_image_url: uploadedAssetUrl })makes sense, sincecreateProjectinitially saw the static URL and you need to switch to the uploaded asset URL.- For the
coverImage && coverImage.startsWith("http")branch,cover_image_urlhas already been sent via the originalcreateProjectpayload, so the extraupdateProjectcall with the same URL may be redundant unless the API requires this second write for some side effect.If the server doesn’t need that second write, you could drop
updateProjectin the HTTP-cover branch and rely solely onupdateCoverImageStatusthere to reduce one network round trip:- } else if (coverImage && coverImage.startsWith("http")) { - await updateCoverImageStatus(res.id, coverImage); - await updateProject(workspaceSlug.toString(), res.id, { cover_image_url: coverImage }); - } + } else if (coverImage && coverImage.startsWith("http")) { + await updateCoverImageStatus(res.id, coverImage); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/ce/components/projects/create/root.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/ce/components/projects/create/root.tsx
🧠 Learnings (2)
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Use `with { type: "json" }` for import attributes; avoid deprecated `assert` syntax (TypeScript 5.3/5.8+)
Applied to files:
apps/web/ce/components/projects/create/root.tsx
📚 Learning: 2025-10-09T20:42:31.843Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7922
File: apps/admin/app/(all)/(dashboard)/ai/form.tsx:19-19
Timestamp: 2025-10-09T20:42:31.843Z
Learning: In the makeplane/plane repository, React types are globally available through TypeScript configuration. Type annotations like React.FC, React.ReactNode, etc. can be used without explicitly importing the React namespace. The codebase uses the modern JSX transform, so React imports are not required for JSX or type references.
Applied to files:
apps/web/ce/components/projects/create/root.tsx
🧬 Code graph analysis (1)
apps/web/ce/components/projects/create/root.tsx (2)
apps/web/ce/components/projects/create/utils.ts (1)
getProjectFormValues(5-18)apps/web/helpers/cover-image.helper.ts (2)
getCoverImageType(107-114)uploadCoverImage(174-227)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/ce/components/projects/create/root.tsx (1)
4-45: Import and default-value refactor looks goodUsing
getProjectFormValues()to centralize defaults and then spreading...datafor overrides keeps the component lean and avoids duplication of default logic. Type-only imports are aligned with the TypeScript guidelines in this repo.
JayashTripathy
changed the title
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
- Corrected the comment regarding local static images to reflect that they are served from the assets folder instead of the public folder. - This change ensures accurate documentation for the `getCoverImageType` and `getCoverImageDisplayURL` functions, improving clarity for future developers.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/web/helpers/cover-image.helper.ts (5)
36-36: Consider dependency injection instead of module-level service singleton.Instantiating
FileServiceat module level makes testing harder (cannot easily mock), tightly couples this helper to a concrete service instance, and may cause issues ifFileServicemaintains internal state across calls.Recommended approach: Accept
fileServiceas a parameter inuploadCoverImageandhandleCoverImageChange, or create a factory function that accepts dependencies:-const fileService = new FileService(); - /** * Uploads a local static image to S3 */ export const uploadCoverImage = async ( imageUrl: string, uploadConfig: { workspaceSlug?: string; entityIdentifier: string; entityType: EFileAssetType; isUserAsset?: boolean; + fileService?: FileService; } ): Promise<string> => { - const { workspaceSlug, entityIdentifier, entityType, isUserAsset = false } = uploadConfig; + const { workspaceSlug, entityIdentifier, entityType, isUserAsset = false, fileService = new FileService() } = uploadConfig;Apply the same pattern to
handleCoverImageChange.
42-72: Optional: Usesatisfiesoperator for stronger type checking.Per TypeScript 5.0+ guidelines, you can validate the structure without widening by using
satisfies:export const STATIC_COVER_IMAGES = { IMAGE_1: CoverImage1, // ... rest IMAGE_29: CoverImage29, -} as const; +} as const satisfies Record<string, string>;This ensures the object conforms to the expected shape while preserving literal types.
107-114: Simplify redundant return and clarify edge case handling.Lines 111-113 have a redundant fallback: both branches return
"uploaded_asset". Additionally, passing an empty string or malformed URL will classify it as"uploaded_asset", which may not be semantically accurate.Option 1: Simplify to a single return:
export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };Option 2: Add validation for empty/invalid URLs:
export const getCoverImageType = (imageUrl: string): TCoverImageType => { + if (!imageUrl) { + throw new Error("Invalid image URL"); + } if (isStaticCoverImage(imageUrl)) return "local_static"; return "uploaded_asset"; };Choose Option 1 if you want to treat all non-static images as uploaded assets, or Option 2 if you want stricter validation.
121-142: Optional: Simplify unreachable fallback.Since
TCoverImageTypehas only two values ("local_static"and"uploaded_asset"), and both are handled explicitly at lines 133-135 and 137-139, the finalreturn imageUrlat line 141 is unreachable under normal type constraints.You can either:
- Keep it as a defensive fallback (no change needed).
- Remove it and rely on TypeScript exhaustiveness checking:
if (imageType === "uploaded_asset") { return getFileURL(imageUrl) || imageUrl; } - - return imageUrl; + + // Exhaustive check - TypeScript will error if we add a new type + const _exhaustive: never = imageType; + return _exhaustive; }Option 1 is safer for runtime; option 2 enforces compile-time exhaustiveness.
147-168: Consider edge case: analyzing removal (newImage is null).At line 161, when
newImageisnullorundefined, you pass an empty string togetCoverImageType, which returns"uploaded_asset". This means removal is analyzed as:{ needsUpload: false, imageType: "uploaded_asset", shouldUpdate: true }While
handleCoverImageChangeexplicitly checks for!newImagebefore using the analysis, this could be semantically clearer.Option: Return early or use a sentinel type for removal:
export const analyzeCoverImageChange = ( currentImage: string | null | undefined, newImage: string | null | undefined ): TCoverImageResult => { const hasChanged = currentImage !== newImage; if (!hasChanged) { return { needsUpload: false, imageType: "uploaded_asset", shouldUpdate: false, }; } + + // Image being removed + if (!newImage) { + return { + needsUpload: false, + imageType: "uploaded_asset", // or define a separate type for removal + shouldUpdate: true, + }; + } const imageType = getCoverImageType(newImage ?? "");This makes the removal case explicit rather than relying on the empty string fallback.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/helpers/cover-image.helper.ts
🧠 Learnings (2)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/helpers/cover-image.helper.ts (1)
81-93: Type definitions look good.The union type and result/payload types are well-structured. Good use of
typedeclarations as requested by reviewers.
- Replaced the default cover image URL with a new utility function `getRandomCoverImage` that selects a random cover image from the available options. - Updated the `getProjectFormValues` function to utilize this new method, enhancing the project creation experience with varied cover images.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/web/helpers/cover-image.helper.ts (1)
173-226: Network error handling still needed.The
fetchcall at line 185 can throw network-level exceptions (DNS failures, CORS errors, etc.) that bypass theresponse.okcheck. This was previously flagged but hasn't been addressed yet.
🧹 Nitpick comments (2)
apps/web/helpers/cover-image.helper.ts (2)
107-114: Simplify redundant conditional.Lines 111-113 both return
"uploaded_asset", making theifstatement unnecessary. Since there are only two possible types and non-static images are always uploaded assets, you can simplify:export const getCoverImageType = (imageUrl: string): TCoverImageType => { // Check against the explicit set of static images if (isStaticCoverImage(imageUrl)) return "local_static"; - if (imageUrl.startsWith("http")) return "uploaded_asset"; - return "uploaded_asset"; };
283-284: Prefer consistent array reference in getRandomCoverImage.The function uses
Object.keys().lengthfor the count butObject.values()[index]for access. While correct, usingObject.values()for both would be clearer:export const getRandomCoverImage = (): string => - Object.values(STATIC_COVER_IMAGES)[Math.floor(Math.random() * Object.keys(STATIC_COVER_IMAGES).length)]; + Object.values(STATIC_COVER_IMAGES)[Math.floor(Math.random() * Object.values(STATIC_COVER_IMAGES).length)];Or extract to a constant:
+const STATIC_COVER_IMAGE_VALUES = Object.values(STATIC_COVER_IMAGES); + export const getRandomCoverImage = (): string => - Object.values(STATIC_COVER_IMAGES)[Math.floor(Math.random() * Object.keys(STATIC_COVER_IMAGES).length)]; + STATIC_COVER_IMAGE_VALUES[Math.floor(Math.random() * STATIC_COVER_IMAGE_VALUES.length)];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/ce/components/projects/create/utils.ts(1 hunks)apps/web/helpers/cover-image.helper.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,mts,cts}
📄 CodeRabbit inference engine (.github/instructions/typescript.instructions.md)
**/*.{ts,tsx,mts,cts}: Useconsttype parameters for more precise literal inference in TypeScript 5.0+
Use thesatisfiesoperator to validate types without widening them
Leverage inferred type predicates to reduce the need for explicitisreturn types in filter/check functions
UseNoInfer<T>utility to block inference for specific type arguments when they should be determined by other arguments
Utilize narrowing inswitch(true)blocks for control flow analysis (TypeScript 5.3+)
Rely on narrowing from direct boolean comparisons for type guards
Trust preserved narrowing in closures when variables aren't modified after the check (TypeScript 5.4+)
Use constant indices to narrow object/array properties (TypeScript 5.5+)
Use standard ECMAScript decorators (Stage 3) instead of legacyexperimentalDecorators
Useusingdeclarations for explicit resource management with Disposable pattern instead of manual cleanup (TypeScript 5.2+)
Usewith { type: "json" }for import attributes; avoid deprecatedassertsyntax (TypeScript 5.3/5.8+)
Useimport typeexplicitly when importing types to ensure they are erased during compilation, respectingverbatimModuleSyntaxflag
Use.ts,.mts,.ctsextensions inimport typestatements (TypeScript 5.2+)
Useimport type { Type } from "mod" with { "resolution-mode": "import" }for specific module resolution contexts (TypeScript 5.3+)
Use new iterator methods (map, filter, etc.) if targeting modern environments (TypeScript 5.6+)
Utilize newSetmethods likeunion,intersection, etc., when available (TypeScript 5.5+)
UseObject.groupBy/Map.groupBystandard methods for grouping instead of external libraries (TypeScript 5.4+)
UsePromise.withResolvers()for creating promises with exposed resolve/reject functions (TypeScript 5.7+)
Use copying array methods (toSorted,toSpliced,with) for immutable array operations (TypeScript 5.2+)
Avoid accessing instance fields viasuperin classes (TypeScript 5....
Files:
apps/web/ce/components/projects/create/utils.tsapps/web/helpers/cover-image.helper.ts
🧠 Learnings (2)
📚 Learning: 2025-10-01T15:30:17.605Z
Learnt from: lifeiscontent
Repo: makeplane/plane PR: 7888
File: packages/propel/src/avatar/avatar.stories.tsx:2-3
Timestamp: 2025-10-01T15:30:17.605Z
Learning: In the makeplane/plane repository, avoid suggesting inline type imports (e.g., `import { Avatar, type TAvatarSize }`) due to bundler compatibility issues. Keep type imports and value imports as separate statements.
Applied to files:
apps/web/helpers/cover-image.helper.ts
📚 Learning: 2025-11-25T10:18:05.172Z
Learnt from: CR
Repo: makeplane/plane PR: 0
File: .github/instructions/typescript.instructions.md:0-0
Timestamp: 2025-11-25T10:18:05.172Z
Learning: Applies to **/*.{ts,tsx,mts,cts} : Ensure variables are initialized before use to avoid errors with never-initialized variables (TypeScript 5.7+)
Applied to files:
apps/web/helpers/cover-image.helper.ts
🧬 Code graph analysis (2)
apps/web/ce/components/projects/create/utils.ts (2)
packages/types/src/project/projects.ts (1)
IProject(38-54)apps/web/helpers/cover-image.helper.ts (1)
getRandomCoverImage(283-284)
apps/web/helpers/cover-image.helper.ts (2)
apps/web/core/services/file.service.ts (1)
FileService(47-293)apps/space/core/store/publish/publish.store.ts (1)
workspaceSlug(93-95)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build and lint web apps
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
apps/web/ce/components/projects/create/utils.ts (1)
5-18: LGTM! Randomization is correctly implemented.Both the cover image (line 6) and emoji selection (line 11) use proper randomization logic. This addresses the previous question about randomization.
…akeplane#8184) * feat: implement cover image handling and static image selection - Added functionality to handle cover image uploads and selections in project and profile forms. - Introduced a new helper for managing cover images, including static images and uploaded assets. - Updated components to utilize the new cover image helper for displaying and processing cover images. - Added a set of static cover images for selection in the image picker. - Enhanced error handling for image uploads and processing. This update improves the user experience by providing a more robust cover image management system. * refactor: rename STATIC_COVER_IMAGES_ARRAY to STATIC_COVER_IMAGES for consistency - Updated the cover image helper to export STATIC_COVER_IMAGES instead of STATIC_COVER_IMAGES_ARRAY. - Adjusted the ImagePickerPopover component to utilize the renamed export for rendering static cover images. * feat: enhance project creation and image handling - Introduced default project form values with a random emoji for logo props. - Updated cover image handling in various components, ensuring consistent usage of the new cover image helper. - Refactored image picker to improve search functionality and loading states. - Removed unused constants and streamlined cover image type checks for better clarity and performance. This update enhances the user experience in project creation and image selection processes. * refactor: simplify cover image type definition and clean up code - Removed duplicate type from TCoverImageType, streamlining the definition. - Cleaned up whitespace in the cover image helper for improved readability. This update enhances code clarity and maintains consistency in cover image handling. * refactor: update cover image type definitions and simplify logic - Changed ICoverImageResult and ICoverImagePayload interfaces to type aliases for better clarity. - Simplified the logic in getCoverImageDisplayURL function to enhance readability and maintainability. This update improves the structure and clarity of the cover image helper code. * refactor: remove unused project cover image endpoint and update cover image handling - Removed the ProjectPublicCoverImagesEndpoint and its associated URL from the project. - Updated the cover image handling in the cover-image helper to utilize imported assets instead of static paths. - Cleaned up the ProjectFavoritesViewSet and FileService by removing the now obsolete getProjectCoverImages method. This update streamlines the cover image management and eliminates unnecessary code, enhancing overall maintainability. * refactor: update cover image imports to new asset structure - Replaced static path imports for cover images with updated paths to the new asset structure. - This change improves organization and maintainability of cover image assets in the project. This update aligns with recent refactoring efforts to streamline cover image handling. * feat: add additional cover images to the helper - Imported new cover images (24 to 29) into the cover-image helper. - This update expands the available cover image options for use in the project, enhancing visual variety. * refactor: remove ProjectPublicCoverImagesEndpoint from project URLs and views * refactor: update cover image imports to include URL query parameter - Modified cover image imports in the cover-image helper to append a URL query parameter for better asset handling. - This change enhances the way cover images are processed and utilized within the project. * refactor: extract default project form values into a utility function - Created a new utility function `getProjectFormValues` to encapsulate the default project form values. - Updated the `CreateProjectForm` component to use this utility function for setting default form values, improving code organization and maintainability. * feat: integrate project update functionality in CreateProjectForm - Added `updateProject` method to the `CreateProjectForm` component for updating project cover images after creation. - Enhanced cover image handling by ensuring the correct URL is set for both uploaded and existing cover images. This update improves the project creation workflow and ensures cover images are accurately updated. * fix: update documentation for cover image handling - Corrected the comment regarding local static images to reflect that they are served from the assets folder instead of the public folder. - This change ensures accurate documentation for the `getCoverImageType` and `getCoverImageDisplayURL` functions, improving clarity for future developers. * feat: implement random cover image selection for project forms - Replaced the default cover image URL with a new utility function `getRandomCoverImage` that selects a random cover image from the available options. - Updated the `getProjectFormValues` function to utilize this new method, enhancing the project creation experience with varied cover images. --------- Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com>
…akeplane#8184) * feat: implement cover image handling and static image selection - Added functionality to handle cover image uploads and selections in project and profile forms. - Introduced a new helper for managing cover images, including static images and uploaded assets. - Updated components to utilize the new cover image helper for displaying and processing cover images. - Added a set of static cover images for selection in the image picker. - Enhanced error handling for image uploads and processing. This update improves the user experience by providing a more robust cover image management system. * refactor: rename STATIC_COVER_IMAGES_ARRAY to STATIC_COVER_IMAGES for consistency - Updated the cover image helper to export STATIC_COVER_IMAGES instead of STATIC_COVER_IMAGES_ARRAY. - Adjusted the ImagePickerPopover component to utilize the renamed export for rendering static cover images. * feat: enhance project creation and image handling - Introduced default project form values with a random emoji for logo props. - Updated cover image handling in various components, ensuring consistent usage of the new cover image helper. - Refactored image picker to improve search functionality and loading states. - Removed unused constants and streamlined cover image type checks for better clarity and performance. This update enhances the user experience in project creation and image selection processes. * refactor: simplify cover image type definition and clean up code - Removed duplicate type from TCoverImageType, streamlining the definition. - Cleaned up whitespace in the cover image helper for improved readability. This update enhances code clarity and maintains consistency in cover image handling. * refactor: update cover image type definitions and simplify logic - Changed ICoverImageResult and ICoverImagePayload interfaces to type aliases for better clarity. - Simplified the logic in getCoverImageDisplayURL function to enhance readability and maintainability. This update improves the structure and clarity of the cover image helper code. * refactor: remove unused project cover image endpoint and update cover image handling - Removed the ProjectPublicCoverImagesEndpoint and its associated URL from the project. - Updated the cover image handling in the cover-image helper to utilize imported assets instead of static paths. - Cleaned up the ProjectFavoritesViewSet and FileService by removing the now obsolete getProjectCoverImages method. This update streamlines the cover image management and eliminates unnecessary code, enhancing overall maintainability. * refactor: update cover image imports to new asset structure - Replaced static path imports for cover images with updated paths to the new asset structure. - This change improves organization and maintainability of cover image assets in the project. This update aligns with recent refactoring efforts to streamline cover image handling. * feat: add additional cover images to the helper - Imported new cover images (24 to 29) into the cover-image helper. - This update expands the available cover image options for use in the project, enhancing visual variety. * refactor: remove ProjectPublicCoverImagesEndpoint from project URLs and views * refactor: update cover image imports to include URL query parameter - Modified cover image imports in the cover-image helper to append a URL query parameter for better asset handling. - This change enhances the way cover images are processed and utilized within the project. * refactor: extract default project form values into a utility function - Created a new utility function `getProjectFormValues` to encapsulate the default project form values. - Updated the `CreateProjectForm` component to use this utility function for setting default form values, improving code organization and maintainability. * feat: integrate project update functionality in CreateProjectForm - Added `updateProject` method to the `CreateProjectForm` component for updating project cover images after creation. - Enhanced cover image handling by ensuring the correct URL is set for both uploaded and existing cover images. This update improves the project creation workflow and ensures cover images are accurately updated. * fix: update documentation for cover image handling - Corrected the comment regarding local static images to reflect that they are served from the assets folder instead of the public folder. - This change ensures accurate documentation for the `getCoverImageType` and `getCoverImageDisplayURL` functions, improving clarity for future developers. * feat: implement random cover image selection for project forms - Replaced the default cover image URL with a new utility function `getRandomCoverImage` that selects a random cover image from the available options. - Updated the `getProjectFormValues` function to utilize this new method, enhancing the project creation experience with varied cover images. --------- Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com>
…akeplane#8184) * feat: implement cover image handling and static image selection - Added functionality to handle cover image uploads and selections in project and profile forms. - Introduced a new helper for managing cover images, including static images and uploaded assets. - Updated components to utilize the new cover image helper for displaying and processing cover images. - Added a set of static cover images for selection in the image picker. - Enhanced error handling for image uploads and processing. This update improves the user experience by providing a more robust cover image management system. * refactor: rename STATIC_COVER_IMAGES_ARRAY to STATIC_COVER_IMAGES for consistency - Updated the cover image helper to export STATIC_COVER_IMAGES instead of STATIC_COVER_IMAGES_ARRAY. - Adjusted the ImagePickerPopover component to utilize the renamed export for rendering static cover images. * feat: enhance project creation and image handling - Introduced default project form values with a random emoji for logo props. - Updated cover image handling in various components, ensuring consistent usage of the new cover image helper. - Refactored image picker to improve search functionality and loading states. - Removed unused constants and streamlined cover image type checks for better clarity and performance. This update enhances the user experience in project creation and image selection processes. * refactor: simplify cover image type definition and clean up code - Removed duplicate type from TCoverImageType, streamlining the definition. - Cleaned up whitespace in the cover image helper for improved readability. This update enhances code clarity and maintains consistency in cover image handling. * refactor: update cover image type definitions and simplify logic - Changed ICoverImageResult and ICoverImagePayload interfaces to type aliases for better clarity. - Simplified the logic in getCoverImageDisplayURL function to enhance readability and maintainability. This update improves the structure and clarity of the cover image helper code. * refactor: remove unused project cover image endpoint and update cover image handling - Removed the ProjectPublicCoverImagesEndpoint and its associated URL from the project. - Updated the cover image handling in the cover-image helper to utilize imported assets instead of static paths. - Cleaned up the ProjectFavoritesViewSet and FileService by removing the now obsolete getProjectCoverImages method. This update streamlines the cover image management and eliminates unnecessary code, enhancing overall maintainability. * refactor: update cover image imports to new asset structure - Replaced static path imports for cover images with updated paths to the new asset structure. - This change improves organization and maintainability of cover image assets in the project. This update aligns with recent refactoring efforts to streamline cover image handling. * feat: add additional cover images to the helper - Imported new cover images (24 to 29) into the cover-image helper. - This update expands the available cover image options for use in the project, enhancing visual variety. * refactor: remove ProjectPublicCoverImagesEndpoint from project URLs and views * refactor: update cover image imports to include URL query parameter - Modified cover image imports in the cover-image helper to append a URL query parameter for better asset handling. - This change enhances the way cover images are processed and utilized within the project. * refactor: extract default project form values into a utility function - Created a new utility function `getProjectFormValues` to encapsulate the default project form values. - Updated the `CreateProjectForm` component to use this utility function for setting default form values, improving code organization and maintainability. * feat: integrate project update functionality in CreateProjectForm - Added `updateProject` method to the `CreateProjectForm` component for updating project cover images after creation. - Enhanced cover image handling by ensuring the correct URL is set for both uploaded and existing cover images. This update improves the project creation workflow and ensures cover images are accurately updated. * fix: update documentation for cover image handling - Corrected the comment regarding local static images to reflect that they are served from the assets folder instead of the public folder. - This change ensures accurate documentation for the `getCoverImageType` and `getCoverImageDisplayURL` functions, improving clarity for future developers. * feat: implement random cover image selection for project forms - Replaced the default cover image URL with a new utility function `getRandomCoverImage` that selects a random cover image from the available options. - Updated the `getProjectFormValues` function to utilize this new method, enhancing the project creation experience with varied cover images. --------- Co-authored-by: Prateek Shourya <prateekshourya29@gmail.com>
Description
This PR implements static cover image selection stored in
assets/cover-imagesType of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.