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

Option to conditionally skip a version/transformation #247

Merged
merged 3 commits into from
Oct 4, 2018

Conversation

dmarkow
Copy link
Contributor

@dmarkow dmarkow commented Mar 14, 2018

This introduces a new :skip return value on transformations to allow for conditional versions (see #147).

A common use is a generic attachment field that accepts any file types. While a thumbnail version would be desirable for a PDF or JPG file, you might not want it for a ZIP file. Just uploading the original file as the "thumbnail" (like :noaction does) has several problems, my biggest two being that 1) uploading multiple versions of a large attachment wastes storage space, and 2) there's no indication when fetching URLs that it's really not a thumbnail.

With this change, when returning :skip from transform/2:

  • Nothing gets uploaded for the specified version (whereas :noaction still uploads the original)
  • Retrieving the url of the skipped version returns nil

I added tests, all should be passing (except for the custom S3 bucket test that was already failing before as it's trying to compare a relative path to a full URL).

If this can be merged, I'll have another quick PR for the arc_ecto repository as well so it can accommodate nil urls.

Possible changes I can make if needed:

  • Make it return the default URL instead of nil (but this wouldn't provide any way to say "there is no URL")
  • Add another option, maybe :default, that will return the default URL for that version as a fallback (instead of nil)

@dmarkow dmarkow changed the title Skip versions Option to conditionally skip a version/transformation Mar 14, 2018
@dmarkow
Copy link
Contributor Author

dmarkow commented Mar 14, 2018

🤦‍♂️ I could have sworn I checked existing PRs first before spending time on this, but it looks like #208 is proposing something similar.

However, that PR doesn't address the returned URLs (meaning it'll return a non-existent URL when retrieving a skipped version).

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