Fix login issue #9012
Fix login issue #9012
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR addresses login issues by enhancing validation and authentication flows across the frontend and backend components.
- Improved object metadata loading with new hooks
useLoadMockedObjectMetadataItemsanduseRefreshObjectMetadataItemsin/packages/twenty-front/src/modules/object-metadata/hooks/ - Enhanced logo URL validation using
isNonEmptyStringinstead ofisDefinedin/packages/twenty-front/src/modules/auth/components/Logo.tsxandImageInput.tsx - Added token-based authentication for workspace logos in
/packages/twenty-server/src/engine/core-modules/workspace/workspace.resolver.ts - Modified environment variable handling in
vite.config.tsto use_env_instead ofprocess.env - Added validation in
RecordActionMenuEntriesSetter.tsxto prevent null reference errors with object metadata
10 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
| import { useFindManyObjectMetadataItems } from '@/object-metadata/hooks/useFindManyObjectMetadataItems'; | ||
| import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; | ||
| import { useLoadMockedObjectMetadataItems } from '@/object-metadata/hooks/useLoadMockedObjectMetadataItems'; | ||
| import { useRefreshObjectMetadataItems } from '@/object-metadata/hooks/useRefreshObjectMetadataItem'; |
There was a problem hiding this comment.
syntax: typo in import path - 'useRefreshObjectMetadataItem' should be 'useRefreshObjectMetadataItems' to match the actual hook name
| ) { | ||
| loadMockedObjectMetadataItems(); | ||
| } else { | ||
| refreshObjectMetadataItems(); |
There was a problem hiding this comment.
logic: refreshObjectMetadataItems is an async function but the await is missing here
| const primaryLogoUrl = getImageAbsoluteURI( | ||
| props.primaryLogo ?? defaultPrimaryLogoUrl, | ||
| ); |
There was a problem hiding this comment.
style: check if primaryLogo could also be an empty string - may need similar validation
| snapshot.getLoadable(objectMetadataItemsState).getValue(), | ||
| generatedMockObjectMetadataItems, |
There was a problem hiding this comment.
style: getLoadable().getValue() may throw if atom is in error state. Consider using try/catch.
| const result = await client.query<ObjectMetadataItemsQuery>({ | ||
| query: FIND_MANY_OBJECT_METADATA_ITEMS, | ||
| variables: {}, | ||
| }); |
There was a problem hiding this comment.
logic: Missing error handling for failed GraphQL queries. Add try/catch block.
| const result = await client.query<ObjectMetadataItemsQuery>({ | ||
| query: FIND_MANY_OBJECT_METADATA_ITEMS, | ||
| variables: {}, | ||
| }); |
There was a problem hiding this comment.
style: Consider adding fetchPolicy to prevent stale cache data during refresh
|
|
||
| const objectMetadataItems = | ||
| mapPaginatedObjectMetadataItemsToObjectMetadataItems({ | ||
| pagedObjectMetadataItems: result.data, |
There was a problem hiding this comment.
logic: result.data could be undefined here, causing runtime error
| @@ -27,6 +27,7 @@ export class FileController { | |||
| @Req() req: Request, | |||
| ) { | |||
| const folderPath = checkFilePath(params[0]); | |||
|
|
|||
| const filename = checkFilename(params['filename']); | |||
|
|
|||
| const workspaceId = (req as any)?.workspaceId; | |||
There was a problem hiding this comment.
logic: Unsafe type assertion. Consider proper typing of the Request object or middleware to ensure workspaceId exists
| } catch (e) { | ||
| workspaceLogoWithToken = workspace.logo; | ||
| } |
There was a problem hiding this comment.
style: Consider logging the error before falling back to raw logo URL
No description provided.