-
Notifications
You must be signed in to change notification settings - Fork 4.2k
fix(server): enhance logo handling in UserWorkspaceService
#14581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(server): enhance logo handling in UserWorkspaceService
#14581
Conversation
Updated the `castWorkspaceToAvailableWorkspace` method to sign the workspace logo URL if it exists. Added error handling to fallback to the original logo URL in case of signing failure. This improves the reliability of logo display in user workspaces.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR fixes workspace icons not appearing in the workspace switcher menu by enhancing logo URL handling in the UserWorkspaceService
. The main change is in the castWorkspaceToAvailableWorkspace
method, which now signs workspace logo URLs using the FileService.signFileUrl
method when a logo exists.
The implementation adds defensive programming practices by first checking if a workspace logo exists using isDefined(workspace.logo)
, then attempting to sign the URL with proper workspace authentication tokens. If the signing process fails for any reason (network issues, token generation problems, etc.), the code gracefully falls back to the original logo URL rather than breaking the workspace display.
This change addresses the core issue where workspace logos stored as file URLs needed to be signed with JWT tokens to be properly accessible by the frontend. The signFileUrl
method creates the necessary authentication tokens and builds a signed path that allows the frontend to display logo images securely. The solution maintains backward compatibility while adding the security layer required for proper file access in the workspace switcher interface.
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it implements proper error handling and fallback mechanisms
- Score reflects well-structured defensive programming with try-catch blocks that prevent breaking changes
- Pay close attention to the logo URL signing logic in the UserWorkspaceService to ensure it integrates properly with existing file service patterns
1 file reviewed, no comments
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:42626 This environment will automatically shut down when the PR is closed or after 5 hours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello there, thanks for you contribution
Unfortunately too many force cast, also it does not seem necessary to try catch the signFileUrl
Closing this for the moment
Have a nice day
workspaceId: workspace.id, | ||
}); | ||
} catch { | ||
signedLogo = workspace.logo as string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be undefined, wrong force cast
if (isDefined(workspace.logo)) { | ||
try { | ||
signedLogo = this.fileService.signFileUrl({ | ||
url: workspace.logo as string, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
redundant force cast
Close #14572