search

fringd / zipline

A gem that lets you stream a zip file from rails
MIT License
289 stars 68 forks source link

Fixes condition for ActiveStorage #45

Closed mapreal19 closed 5 years ago

mapreal19 commented 5 years ago

This works in one of my apps using ActiveStorage and Amazon S3

fringd commented 5 years ago

Thanks for the PR by the way. Can you address the comments above? I think we want Attached::One or Attachment.

Attached in general doesn't make sense because Attached::Many wouldn't make sense. If we had many, like user.photos, we'd want accept user.photos[0] which would be an Attachment not an Attached.

Does that make sense?

mapreal19 commented 5 years ago

Good catch on the Many thing. I'll take a look and update the PR this Monday.

mapreal19 commented 5 years ago

Just pushed last commit. Looking good now?

I'm also thinking in long term would be best to have a dummy app and have better tests there instead of too much mocking. But for now is fine I guess

fringd commented 5 years ago

Thanks for the PR!

mapreal19 commented 5 years ago

@fringd my pleasure! thank you for this gem!