Update Attachment Table on RICH_TEXT Field Edits (#6565) #7617
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 pull request implements a feature to update the attachment table when new images are added to RICH_TEXT fields, enhancing data consistency in the application's file management system.
- Added new 'AddAttachment' mutation in
packages/twenty-front/src/generated/graphql.tsxfor creating attachment records - Updated
RichTextEditorcomponent inpackages/twenty-front/src/modules/activities/components/RichTextEditor.tsxto handle image uploads and update attachment table - Modified
UPLOAD_FILEmutation inpackages/twenty-front/src/modules/attachments/graphql/queries/uploadFile.tsto return file id and url - Added
ADD_ATTACHMENTmutation inuploadFile.tsto create attachment records with file details
3 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| if (!uploadedFileData) { | ||
| throw new Error("Couldn't upload Image"); | ||
| } | ||
|
|
||
| if (!result?.data?.uploadFile) { | ||
| throw new Error("Couldn't upload Image"); | ||
| } |
There was a problem hiding this comment.
logic: This check is redundant as it's already covered by the previous condition.
There was a problem hiding this comment.
Interesting one, thanks for raising it @yadavshubham01
Here are my thought on this one:
- Product behavior: I agree it would be great to store uploaded files as attachments @Bonapara @FelixMalfait what do you think of the product behavior
- Eng strategy: we are doing it in two calls: uploadFile, then addAttachment. These two calls belong to the same atomic operations and I would prefer having only one uploadAttachment customResolver doing both. However, taking the look at useUploadAttachment (which is used on Files tab), I can see we are already doing two calls there so I guess we can keep doing it
- Could we try to re-use useUploadAttachment logic instead?
- In your PR I don't see the code for addAttachment custom resolver, but with 3 in mind I could prefer that we reuse useUploadAttachment which is doing createAttachment
Putting this on hold the time to get an answer from Product on this
|
Hey @charlesBochet , |
|
@charlesBochet yes this is the right product behavior, here's the related issue: #6565 |
|
Ok @yadavshubham01, could you re-use the useUploadAttachment hook in this case? |
|
hey @charlesBochet Is there anything wrong in this PR and i don't understand re-use of useUploadAttachment hook |
|
hey @charlesBochet i just create new PR #7690 that re-use the useUploadAttachment hook in this case |
charlesBochet
added
-PR: awaiting review
and removed
blocked: product specs needed
labels
|
Ok, perfect, closing this one |
Reusing the useUploadAttachment Hook In the implementation of the feature to ensure the attachment table is updated whenever new images are added to a RICH_TEXT field #7617 , it is likely that the useUploadAttachment hook is reused. The useUploadAttachment hook is responsible for handling the upload of attachments, including images, and returning the uploaded file URL. By reusing this hook, you can leverage its existing functionality to handle image uploads within the RICH_TEXT field. In this case, the modified image handling logic would utilize the useUploadAttachment hook to upload new images added to the RICH_TEXT content. The hook would then return the uploaded file URL, which would be used to update the attachment table with the details of the newly added images. By reusing the useUploadAttachment hook, you can avoid duplicating code and ensure consistency in the way attachments are handled throughout the application. Fixes #6565 --------- Co-authored-by: Charles Bochet <[email protected]> Co-authored-by: Lucas Bordeau <[email protected]>
i implements a feature that ensures the attachment table is updated whenever new images are added to a RICH_TEXT field. This enhancement improves the data consistency and integrity of our application's file management system, ensuring that all relevant attachments are correctly linked to their respective entries.
Changes Made:
Image Handling Logic :-
Modified the existing functionality for handling image uploads within the RICH_TEXT field.
Implemented a mechanism to detect when new images are added to the RICH_TEXT content.
Attachment Table Update :-
Added a function that updates the attachment table with the details of newly added images.
Each image upload will now trigger an insert operation in the attachment table, including fields such as fileId, fileName, and fileUrl.
Integration with GraphQL :--
Updated the relevant GraphQL mutations to support the addition of attachments in conjunction with RICH_TEXT field edits.
Ensured proper error handling and logging for failed uploads or attachment inserts.