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 cropped banner images being overwritten #981

Merged
merged 2 commits into from
Mar 4, 2024

Conversation

kirkkwang
Copy link
Contributor

When cropper.js intercepts an updated banner image file and crops it, the original filename gets changed to cropped.jpg which gets saved to disk. When another tenant crops their banner image, it also gets saved as cropped.jpg and overwrites the previous tenant's banner image. In this commit, we prepend the tenant name so it would look something like shared-cropped.jpg and uindy-cropped.jpg instead and avoids the files from being overwritten.

Ref:

Expected Behavior Before Changes

Banner images were being overwritten.

Expected Behavior After Changes

Banner images have unique names based off the tenant so overwriting the name shouldn't be possible.

Screenshots / Video

Before

UIndy tenant has the banner image with filename being cropped.jpg
image

Search tenant also has the same filename of cropped.jpg
image

After

The tenant is prepended to the filename
image

Another example
image

When cropper.js intercepts an updated banner image file and crops it,
the original filename gets changed to cropped.jpg which gets saved to
disk.  When another tenant crops their banner image, it also gets saved
as cropped.jpg and overwrites the previous tenant's banner image.  In
this commit, we prepend the tenant name so it would look something like
`shared-cropped.jpg` and `uindy-cropped.jpg` instead and avoids the
files from being overwritten.

Ref:
  - #948
  - #746
Copy link
Contributor

@ShanaLMoore ShanaLMoore left a comment

Choose a reason for hiding this comment

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

Nice catch. We'll definitely want to contribute this fix back.

@kirkkwang kirkkwang merged commit cc71d30 into main Mar 4, 2024
7 checks passed
@kirkkwang kirkkwang deleted the i948-banner-image-overwritten branch March 4, 2024 19:44
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