feat(desktop): implement favicon discovery and icon management for pr…#1168
Closed
Ipriyankrajai wants to merge 12 commits intosuperset-sh:mainfrom
Closed
feat(desktop): implement favicon discovery and icon management for pr…#1168Ipriyankrajai wants to merge 12 commits intosuperset-sh:mainfrom
Ipriyankrajai wants to merge 12 commits intosuperset-sh:mainfrom
Conversation
…ojects - Added `discoverFavicon` utility to locate and retrieve favicons from project directories. - Enhanced `upsertProject` function to trigger favicon discovery for both new and existing projects. - Introduced `setProjectIcon` and `discoverProjectIcon` procedures for managing project icons via TRPC. - Created `IconUploader` component for user-uploaded icons in project settings. - Updated database schema to include `iconUrl` and `iconOverride` fields for projects. - Modified various components to display project icons, including sidebar and project settings.
Contributor
📝 WalkthroughWalkthroughThis PR implements project icon discovery and management functionality. It introduces a favicon discovery utility that searches repositories for icon files, adds database columns for storing discovered and custom icons, creates TRPC mutations for icon operations, integrates favicon discovery into project create/clone/open workflows, and adds UI components for icon upload and display in project settings with sidebar propagation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Settings as Project Settings UI
participant Uploader as IconUploader Component
participant Window as Window Router
participant TRPC as Projects TRPC
participant Repo as Repository
participant DB as Database
User->>Settings: Opens project settings
Note over Settings: Displays Project Icon section
User->>Uploader: Clicks Upload/Change Icon
Uploader->>Window: selectImageFile()
Window-->>Uploader: Returns image data URL
Uploader->>TRPC: setProjectIcon(projectId, dataUrl)
TRPC->>DB: Update project.iconOverride
DB-->>TRPC: Success
TRPC-->>Uploader: Success response
Uploader->>Settings: Invalidate caches
Settings->>Settings: Refresh UI with new icon
alt User clicks Auto-Discover
User->>Uploader: Trigger discovery
Uploader->>TRPC: discoverProjectIcon(projectId)
TRPC->>Repo: Scan for favicon files
Repo-->>TRPC: Favicon file found
TRPC->>DB: Update project.iconUrl
DB-->>TRPC: Success
TRPC-->>Uploader: Returns {success, found}
Uploader->>Settings: Invalidate caches
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/project/`$projectId/components/ProjectSettings/ProjectSettings.tsx:
- Around line 21-26: The setIcon mutation
(electronTrpc.projects.setProjectIcon.useMutation) lacks error handling and
should show a toast on failure; add an onError handler to the useMutation
options that calls toast.error with a helpful message and the error details, and
keep the existing onSuccess logic (invalidating utils.projects.get and
utils.workspaces.getAllGrouped). Also import toast from "@superset/ui/sonner" at
the top of the file so the onError handler can display the notification.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/ProjectSection/ProjectThumbnail/ProjectThumbnail.tsx`:
- Around line 46-50: The iconError state can become stale when the source
changes; in the ProjectThumbnail component, add an effect that watches
effectiveIcon (the derived value using iconOverride || iconUrl) and resets
iconError via setIconError(false) whenever effectiveIcon changes so a newly
provided icon will be retried/rendered; reference the iconError/setIconError
state and the effectiveIcon variable when implementing this effect.
🧹 Nitpick comments (5)
apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/ProjectSettings.tsx (1)
47-55:projectIdprop is passed but unused inIconUploader.The
IconUploadercomponent declaresprojectIdin its props interface but never uses it (it's not destructured from props at line 22-29 ofIconUploader.tsx). Either remove it from both places or use it if intended.♻️ Option 1: Remove unused prop
In
ProjectSettings.tsx:<IconUploader - projectId={projectId} projectName={project.name}In
IconUploader.tsxprops interface:interface IconUploaderProps { - projectId: string; projectName: string;apps/desktop/src/lib/trpc/routers/projects/utils/favicon-discovery.ts (1)
28-29: Path length heuristic may not always select the closest favicon to root.Sorting by string length assumes shorter paths are closer to root, but this isn't always true (e.g.,
/a/b/x.pngis shorter than/abc/x.pngbut deeper). Consider sorting by path segment count instead.♻️ Proposed fix using segment count
-// Pick shortest path (closest to project root) -const shortest = matches.sort((a, b) => a.length - b.length)[0]; +// Pick path closest to project root (fewest directory segments) +const shortest = matches.sort( + (a, b) => a.split("/").length - b.split("/").length +)[0];apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
1070-1076: Consider adding server-side validation for the icon data URL.The
iconinput accepts any string without validating its length or format. While the client-sideprocessIconFilelimits the size, a malicious client could bypass this and send arbitrarily large data. Consider adding a reasonable max length constraint.🛡️ Proposed fix to add input validation
+const MAX_ICON_DATA_URL_LENGTH = 1024 * 1024; // ~750KB base64 encoded + setProjectIcon: publicProcedure .input( z.object({ id: z.string(), - icon: z.string().nullable(), // Base64 data URL or null to remove + icon: z + .string() + .max(MAX_ICON_DATA_URL_LENGTH, "Icon data exceeds maximum size") + .nullable(), // Base64 data URL or null to remove }), )apps/desktop/src/renderer/routes/_authenticated/settings/project/$projectId/components/ProjectSettings/components/IconUploader/IconUploader.tsx (2)
69-83: Silent failure when dropping non-image files.When a user drops files that don't include an image, no feedback is provided. Consider showing an error message to guide the user.
♻️ Proposed fix to add feedback
const handleDrop = useCallback( async (e: React.DragEvent) => { e.preventDefault(); e.stopPropagation(); setIsDragOver(false); const files = Array.from(e.dataTransfer.files); const imageFile = files.find((f) => f.type.startsWith("image/")); if (imageFile) { await handleFileSelect(imageFile); + } else if (files.length > 0) { + setError("Please drop an image file"); } }, [handleFileSelect], );
18-20: Consider extractinggetGitHubAvatarUrlto a shared utility.This function appears in multiple places (here, and similar logic exists in
projects.tsviagetGitHubAvatarUrlfrom./utils/github). For consistency and maintainability, consider importing from a shared location.
- Added new dependencies: `@anthropic-ai/sdk`, `@slack/types`, `@slack/web-api`, and `@headless-tree/core` to improve functionality. - Updated existing dependencies to their latest versions for better performance and security. - Modified `postinstall` script to use a shell script for improved installation process. - Enhanced environment variable management by adding Slack and Anthropic API keys to the configuration. - Removed obsolete SEO audit plan file to streamline project documentation.
- Introduced new fields `icon_url` and `icon_override` in the `projects` table to support project icon management. - Updated migration journal and created a snapshot for versioning.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 1079-1142: The setProjectIcon input currently allows any string
for the icon field; update the Zod schema for setProjectIcon to validate that
icon is either null or a valid Base64 data URL and enforce a reasonable max
length (e.g., ensure input.icon matches
/^data:image\/(png|jpeg|webp);base64,[A-Za-z0-9+/=]+$/ and is below your chosen
byte/char limit). Modify the publicProcedure input definition in setProjectIcon
to replace z.string().nullable() with a refined/regex-checked
z.string().nullable() (or z.union with z.null()) and add a .max(...) check so
oversized or non-data-URL strings are rejected before updating
projects.iconOverride in the mutation. Ensure error messages are descriptive for
validation failures so callers know why their payload was rejected.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
37-37: Prefer a params object forupsertProjectargumentsThis helper still takes positional args; switching to a params object aligns with the repo guideline and reduces mis-ordered call risk.
♻️ Proposed refactor
-function upsertProject(mainRepoPath: string, defaultBranch: string): Project { +function upsertProject({ + mainRepoPath, + defaultBranch, +}: { + mainRepoPath: string; + defaultBranch: string; +}): Project { const name = basename(mainRepoPath); … }-const project = upsertProject(mainRepoPath, defaultBranch); +const project = upsertProject({ mainRepoPath, defaultBranch });-const project = upsertProject(mainRepoPath, defaultBranch); +const project = upsertProject({ mainRepoPath, defaultBranch });-const project = upsertProject(input.path, defaultBranch); +const project = upsertProject({ mainRepoPath: input.path, defaultBranch });As per coding guidelines: “Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.”
Also applies to: 57-129
- Introduced a new markdown file with instructions on how to resolve active comments on pull requests using the `gh` CLI. - Included commands to enumerate comments, view details, and mark comments as resolved.
- Implemented error logging in the icon update process within ProjectSettings component to improve debugging and user feedback.
- Added a useRef hook to track the previous icon and reset the error state when the icon source changes. - Removed unnecessary flex-shrink class from the ProjectThumbnail component for improved layout consistency.
- Added regex validation for base64 image data URLs and a maximum length constraint for project icons. - Updated the input schema for project creation to enforce these new validation rules, ensuring better error handling and user feedback.
Contributor
Author
|
@Kitenite Can you review this? |
- Added toast notification to display error messages when icon update fails, enhancing user feedback and debugging capabilities.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/projects/projects.ts`:
- Around line 226-228: The current ICON_DATA_URL_REGEX only checks the data URL
prefix and allows empty payloads; update ICON_DATA_URL_REGEX to require a
non-empty valid base64 payload (e.g. require at least one base64 character group
and optional = padding after the comma) so malformed/empty data URLs are
rejected, and ensure callers that use ICON_DATA_URL_REGEX (references around
projects.ts and the other block at lines 1083-1116) use the updated constant;
keep MAX_ICON_DATA_URL_LENGTH unchanged.
🧹 Nitpick comments (1)
apps/desktop/src/lib/trpc/routers/projects/projects.ts (1)
60-63: Prefer a params object forupsertProject.
Line 62 uses positional arguments; switching to a named params object reduces call-site ambiguity as this function grows.
As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
saddlepaddle
reviewed
Feb 3, 2026
- Remove unrelated .claude/commands/respond-to-pr-comments.md - Tighten ICON_DATA_URL_REGEX to require non-empty base64 payload - Extract duplicated favicon discovery logic into triggerFaviconDiscovery helper - Add favicon discovery to cloneRepo path (was missing) - Remove unused projectId prop from IconUploader component - Remove redundant comments across all changed files - Consolidate SVG/ICO early-return in processIconFile utils
…-and-drop Replace the custom drag-and-drop UI and client-side canvas processing with the established selectImageFile pattern (native OS file picker via Electron dialog). This aligns project icon upload with the existing avatar/logo upload flow used elsewhere in the app.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…ojects
discoverFaviconutility to locate and retrieve favicons from project directories.upsertProjectfunction to trigger favicon discovery for both new and existing projects.setProjectIconanddiscoverProjectIconprocedures for managing project icons via TRPC.IconUploadercomponent for user-uploaded icons in project settings.iconUrlandiconOverridefields for projects.Description
Screen.Recording.2026-02-03.at.4.26.34.PM.mov
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
Release Notes
New Features