uploadcare / uploadcare-ruby

Ruby API client that handles uploads and further operations with files by wrapping Uploadcare Upload and REST APIs.
https://uploadcare.com
MIT License
39 stars 28 forks source link

add support for UPLOADCARE_STORE flag #36

Closed vizvamitra closed 7 years ago

vizvamitra commented 7 years ago

Now gem users can define wheather to store their files on upload.

This could be done in two different ways: either setting global :store_files_upon_upload option or setting :store option on each upload request.

# accepts `true`, `false` or `:auto`
api = Uploadcare::Api.new(store_files_upon_upload: true)

api.upload(file) # request will be sent with `UPLOADCARE_STORE=1` param
api.upload(url) # request will be sent with `store=1` param

api.upload(url, store: false) # will override global setting, sending `store=0`
api.upload(url, store: :auto) # will override global setting, sending `store=auto`

Also:

vizvamitra commented 7 years ago

@dmitry-mukhin, Setting :autostore to true by default feels strange for me. Consider this:

  1. API docs say that default behaviour (when UPLOADCARE_STORE http param is not provided in an upload request) is equal to setting UPLOADCARE_STORE=0
  2. Same docs say that UPLOADCARE_STORE=1 "Requires “automatic file storing” setting to be enabled", otherwise you'll receive [HTTP 403] Autostore is disabled in response. So if we'll set the default to true than some users will receive errors because their projects has autostore setting set to disallow.

Are you sure?

dmitry-mukhin commented 7 years ago

of course you're right. we need to default this to "auto".

vizvamitra commented 7 years ago

@dmitry-mukhin before this update the default behaviour of the #upload method used to be "don't store files". If i'll set default to auto then those ho have autostore setting allowed will get their files stored by default, others will get them not stored. "Do not store" will change to "Store according your project settings". Is it ok?

dmitry-mukhin commented 7 years ago

As I've said earlier it's ok if we change major versions and write about this potentially breaking change in changelog.

vizvamitra commented 7 years ago

@dmitry-mukhin seems that I've made all requestd changes. Note the change in spec/resources/file_spec.rb, it reflects the change in a default behaviour

vizvamitra commented 7 years ago

@dmitry-mukhin it looks that there are some heisenbugs in Uploadcare::Api::File tests (example). Don't merge this PR untill I'll find the reason

vizvamitra commented 7 years ago

@dmitry-mukhin please note my changes in the last commit. If you are ok with them, we can finally merge this PR

vizvamitra commented 7 years ago

@dmitry-mukhin I've thrown out caveats section from readme and fixed changelog. Approve please