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

Feature: add customizable asset delete callbacks #601

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ddink
Copy link
Contributor

@ddink ddink commented Oct 7, 2024

What will it allow you to do that you can't do today?

It will allow users to customize a soft_delete/1 callback that's used when Beacon.MediaLibrary.soft_delete/1 gets called.

Why do you need this feature and how will it benefit other users?

This feature is necessary to be able to programmatically delete stored assets where they are stored when utilizing custom provider modules for the media library's asset upload functionality.

Are there any drawbacks to this feature?

This feature does not standardize the customizable callback. As many or as few callbacks can be added. In other words, this callback is not a required implementation.

@ddink ddink force-pushed the customizable_asset_delete_callbacks_feature branch from 3b5fe1f to 751b35f Compare October 7, 2024 17:43
@ddink ddink force-pushed the customizable_asset_delete_callbacks_feature branch from 751b35f to dda1bfa Compare October 7, 2024 18:10
@leandrocp
Copy link
Contributor

Hey @ddink thanks for the PR! I believe we can actually move that logic into the provider itself, similar to send_to_cdn/1 that providers already implement. So we would not introduce a new global lifecycle, but rather a new Provider callback to soft delete an asset.

So MediaLibrary.soft_delete/1 can call Lifecycle.Asset.delete_uploaded_asset(asset) which fetches the list of providers from the media type config, and call Beacon.MediaLibrary.Provider.soft_delete/1 (to be implemented, similar to Provider.send_to_cdns/1)

And also move that repo(asset).update_all from MediaLibrary.soft_delete/1 into the Repo.soft_delete/1 provider function.

There are still some improvements and fixes we need to do on the Media Library API, but this PR is moving into that direction, giving more capabilities to the providers, which is great!

@ddink
Copy link
Contributor Author

ddink commented Oct 8, 2024

Hey @leandrocp! Of course, happy to be able to help 🫡 

I think I have now moved around things the way that you want. The only thing that sticks out to me is the way we’re updating the asset before the soft delete.

In order to have the media library updated when the asset is soft deleted, I also had to add the repo(asset).update_all block into my custom provider’s soft_delete callback. It’s not a big deal, but I’m wondering if there’s a way to do that without having to include it into the custom provider’s logic. What do you think about putting the repo(asset).update_all block in Beacon.MediaLibrary.Provider.soft_delete/2 before the list of providers call their own soft_delete/1 callbacks?

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