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: Make file format attachment less rigid #5883

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

diegolmello
Copy link
Member

@diegolmello diegolmello commented Sep 27, 2024

Proposed changes

In some cases, the URL returned by formatAttachmentUrl wouldn't consider local files and the code would get to this line

return setParamInUrl({ url: `${server}${attachmentUrl}`, token, userId });

The result would be an invalid URL like ${server}${localFile} -> https://open.rocket.chatfile://var/mobile...

I was not able to reproduce the issue, but since the code on this function is fairly straightforward and the only condition that should consider local files was based on a specific local directory (FileSystem.documentDirectory), I've decided to relax this rule a little bit by just checking file://.

Issue(s)

How to test or reproduce

Screenshots

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Further comments

NATIVE-655

Copy link
Contributor

@OtavioStasiak OtavioStasiak left a comment

Choose a reason for hiding this comment

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

The code is well-written, the purpose of the change is clearly defined, and it didn't break anything on either platform.

@diegolmello diegolmello merged commit b8b76bc into develop Oct 1, 2024
13 of 18 checks passed
@diegolmello diegolmello deleted the fix.format-attachment-url branch October 1, 2024 11:37
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.

2 participants