Fix attachment type #14741
Fix attachment type #14741
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Overview
Summary
This PR standardizes attachment type values across the codebase by converting from PascalCase to uppercase underscore format (e.g., TextDocument → TEXT_DOCUMENT). The changes ensure consistency between frontend and backend representations.
Key Changes:
- Updated
AttachmentTypeenum to use uppercase constants with underscores - Modified
getFileTypeutility function to return standardized type values - Updated backend workspace entity with proper SELECT field options and labels
- Added comprehensive migration command to clean existing database values
- Enhanced timeline activity entity with attachment relationship support
- Updated all tests to match new type format
Migration Strategy:
- Database migration handles conversion of legacy values to new format
- Supports multiple legacy formats (PascalCase, file extensions, etc.)
- Sets unrecognized types to
OTHERas fallback - Comprehensive logging for tracking conversion results
Confidence Score: 5/5
- This PR is safe to merge with minimal risk
- The changes are well-structured with comprehensive migration strategy, updated tests, and consistent type standardization across frontend/backend. The migration command safely handles legacy data conversion with proper error handling and logging.
- No files require special attention
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/twenty-front/src/modules/activities/files/types/Attachment.ts | 5/5 | Updated AttachmentType enum to use uppercase constants with underscores, standardizing file type values |
| packages/twenty-front/src/modules/activities/files/utils/getFileType.ts | 5/5 | Updated file extension mapping to use new uppercase attachment type constants, maintaining complete coverage |
| packages/twenty-server/src/modules/attachment/standard-objects/attachment.workspace-entity.ts | 5/5 | Updated attachment type field options to use uppercase values with proper labels and colors |
| packages/twenty-server/src/database/commands/upgrade-version-command/1-7/1-7-clean-attachment-type-values.command.ts | 4/5 | Migration command to clean up existing attachment type values, converting legacy formats to new uppercase standards |
Sequence Diagram
sequenceDiagram
participant User as User
participant UI as Frontend UI
participant API as Backend API
participant DB as Database
participant Migration as Migration Command
Note over Migration, DB: Database Migration Phase
Migration->>DB: Check attachment table exists
Migration->>DB: Query existing attachment types
Migration->>DB: UPDATE type='ARCHIVE' WHERE type IN (old formats)
Migration->>DB: UPDATE type='AUDIO' WHERE type IN (old formats)
Migration->>DB: UPDATE type='IMAGE' WHERE type IN (old formats)
Migration->>DB: UPDATE type='PRESENTATION' WHERE type IN (old formats)
Migration->>DB: UPDATE type='SPREADSHEET' WHERE type IN (old formats)
Migration->>DB: UPDATE type='TEXT_DOCUMENT' WHERE type IN (old formats)
Migration->>DB: UPDATE type='VIDEO' WHERE type IN (old formats)
Migration->>DB: UPDATE type='OTHER' WHERE type NOT IN (valid types)
Note over User, DB: Runtime Operations
User->>UI: Upload file
UI->>UI: Call getFileType(fileName)
UI->>UI: Map file extension to AttachmentType
UI->>API: Create attachment with standardized type
API->>DB: INSERT attachment with uppercase type value
API->>UI: Return attachment with type
UI->>User: Display file with correct type icon
10 files reviewed, no comments
![seer-by-sentry[bot]](https://avatars.githubusercontent.com/in/801464?s=60&v=4){kind=small}
| rar: 'ARCHIVE', | ||
| '7z': 'ARCHIVE', | ||
| }; | ||
|
|
||
| export const getFileType = (fileName: string): AttachmentType => { | ||
| const fileExtension = fileName.split('.').at(-1); | ||
| if (!fileExtension) { | ||
| return 'Other'; | ||
| return 'OTHER'; | ||
| } | ||
| return FileExtensionMapping[fileExtension.toLowerCase()] ?? 'Other'; | ||
| return FileExtensionMapping[fileExtension.toLowerCase()] ?? 'OTHER'; | ||
| }; |
There was a problem hiding this comment.
Potential bug: The getFileType function returns SCREAMING_SNAKE_CASE values, but IconMapping and getIconColors expect PascalCase keys, causing lookups to fail and icons not to render.
-
Description: The
getFileTypefunction is being updated to returnSCREAMING_SNAKE_CASEvalues (e.g.,TEXT_DOCUMENT) to align with theAttachmentTypetype definition. However, theIconMappingandgetIconColorsobjects, which are used to retrieve UI components and color styles for files, were not updated and still usePascalCasekeys (e.g.,TextDocument). As a result, any lookup using the value fromgetFileType, such asIconMapping[fileType], will returnundefined. This will cause file icons to fail to render throughout the application, breaking a key part of the file display functionality. -
Suggested fix: Update the keys in the
IconMappingandgetIconColorsobjects to useSCREAMING_SNAKE_CASEto match the values returned by thegetFileTypefunction. For example, the keyTextDocumentshould be changed toTEXT_DOCUMENT.
severity: 0.65, confidence: 0.98
Did we get this right? 👍 / 👎 to inform future reviews.
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:52296 This environment will automatically shut down when the PR is closed or after 5 hours. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
FelixMalfait
force-pushed
the
fix-attachment-type
branch
from
7764bb4 to
afbf07f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[log] [log] ✖ Field Attachment.type changed type from String to AttachmentTypeEnum REST API Breaking Changes
|
| ThemeColor | ||
| >; | ||
|
|
||
| if (typeField?.options !== undefined && typeField.options !== null) { |
| @WorkspaceField({ | ||
| standardId: ATTACHMENT_STANDARD_FIELD_IDS.type, | ||
| type: FieldMetadataType.TEXT, | ||
| type: FieldMetadataType.SELECT, |
There was a problem hiding this comment.
not sure this is well supported with the current sync (nor with the new one actually 🤔)
| `Migrating attachment author to createdBy for workspace ${workspaceId} in schema ${schemaName}`, | ||
| ); | ||
|
|
||
| const attachmentTableExists = (await this.coreDataSource.query(` |
There was a problem hiding this comment.
I think looking for the object existence should be enough but why not!
|
|
||
| try { | ||
| // Get attachments that have authorId but no createdBy data | ||
| const attachmentsWithAuthor = (await this.coreDataSource.query(` |
There was a problem hiding this comment.
nit: we usually prefer to use workspace repository API instead of raws. I guess for commands that makes sense though since you can use the same datasource for all the mutations 🤔
There was a problem hiding this comment.
Yes I thought we might not want to pollute the logs/timeline/workflow events also
2 participants
Preparing for Attachments being displayed as a standard object, let's start by fixing the Type field
Note
Standardizes attachment type to UPPERCASE SELECT enum with color mapping, migrates data, adds createdBy and timeline relations, and updates frontend components/tests accordingly.
AttachmentTypevalues to UPPERCASE; updategetFileTypemappings/tests and default toOTHER.useFileTypeColorswithuseAttachmentTypeColorssourced from metadata; map totheme.colorand updateIconMappingkeys to UPPERCASE.FileBlockdefaultfileTypetoOTHER; adjustAgentChatFilePreviewandFileIconto new color hook; useCoreObjectNameSingularingetIconColorForObjectType.Attachment.typefrom TEXT to SELECT with options/default'OTHER'; addcreatedBy(ACTOR), mark fields UI read-only; rename labels to “File/Files”; addtimelineActivitiesrelation.typevalues to new UPPERCASE set.typecolumn to enum (SELECT) and set default.authorIddata intocreatedBy.createdBy,timelineActivities, and timeline activity ↔ attachment relation; wire commands into upgrade module/runner.Written by Cursor Bugbot for commit ecebf40. This will update automatically on new commits. Configure here.