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

Allow destroying active storage variants #47150

Merged
merged 1 commit into from
Mar 25, 2023

Conversation

shouichi
Copy link
Contributor

@shouichi shouichi commented Jan 26, 2023

Background:

When creating active storage variants, ActiveStorage::VariantRecord is inserted, then a file is uploaded. Because upload can be failed, the file can be missing even though ActiveStorage::VariantRecord exists.

When a file is missing, we need to delete the corresponding ActiveStorage::VariantRecord but there's no API to delete just one variant e.g., attachable.variant(resize_to_limit: [100, 100]).destroy.

@zzak
Copy link
Member

zzak commented Feb 8, 2023

@shouichi Hello! (Long time @Mekajiki!!) Could you add a changelog as well? 🙇

@shouichi
Copy link
Contributor Author

shouichi commented Feb 8, 2023

Oh, forgot about the CHANGELOG, will do.

@shouichi shouichi changed the title Allow destroying active storage variant Allow destroying active storage variants Feb 8, 2023
@shouichi shouichi force-pushed the destroy-variant branch 2 times, most recently from 76a63a9 to 2e7959a Compare February 8, 2023 07:50
@shouichi
Copy link
Contributor Author

shouichi commented Feb 8, 2023

@zzak Added a changelog. Could you take a look?

Copy link
Contributor

@hahmed hahmed left a comment

Choose a reason for hiding this comment

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

nitpick about making it clear what attachable is, I feel like having User.avatar is clearer than attachable wdyt? Otherwise LGTM

activestorage/CHANGELOG.md Outdated Show resolved Hide resolved
@hahmed hahmed added the ready PRs ready to merge label Feb 8, 2023
Background:

When creating active storage variants, `ActiveStorage::VariantRecord` is
inserted, then a file is uploaded. Because upload can be failed, the
file can be missing even though `ActiveStorage::VariantRecord` exists.

When a file is missing, we need to delete the corresponding
`ActiveStorage::VariantRecord` but there's no API to delete just one
variant e.g., `blob.variant(resize_to_limit: [100, 100]).destroy`.

Co-authored-by: Yuichiro NAKAGAWA <ii.hsif.drows@gmail.com>
Co-authored-by: Ryohei UEDA <ueda@anipos.co.jp>
@rafaelfranca rafaelfranca merged commit febd9ab into rails:main Mar 25, 2023
@shouichi shouichi deleted the destroy-variant branch April 4, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
activestorage ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants