Skip to content
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 attachment upload #6574

Merged
merged 2 commits into from
Aug 7, 2024
Merged

fix attachment upload #6574

merged 2 commits into from
Aug 7, 2024

Conversation

Weiko
Copy link
Member

@Weiko Weiko commented Aug 7, 2024

Context

UploadFile now returns the file token which we don't want when we save the attachment in the DB due to its non-persistence so now the FE removes the token query param before saving the attachment. This is most likely not the right way to do it, we will need to refactor this part before.

Also made sure we don't save the token in the DB for rich_text as well and remove it before saving.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

The pull request focuses on removing non-persistent tokens from file URLs before saving them to the database, affecting both the RichTextEditor component and the useUploadAttachmentFile hook.

  • RichTextEditor.tsx: Introduced prepareBody function to strip token query parameters from image URLs before saving.
  • useUploadAttachmentFile.tsx: Added computePathWithoutToken function to remove token query parameters from attachment URLs.
  • useUploadAttachmentFile.test.tsx: Added unit tests for computePathWithoutToken to ensure correct removal of token query parameters.

These changes ensure non-persistent tokens are not stored, enhancing security and data integrity.

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@Weiko Weiko merged commit 1b9f63b into main Aug 7, 2024
3 of 9 checks passed
@Weiko Weiko deleted the c--fix-attachment-upload branch August 7, 2024 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant