🌸✨ Marketplace: Seller may add Photo to Product
#1441
Conversation
| return unless photo.attached? && photo.blob.persisted? | ||
| return if photo.blob.filename.to_s.start_with?(space.id.to_s) | ||
|
|
||
| new_name = "#{space.id}-#{photo.blob.filename}" |
There was a problem hiding this comment.
It turns out that my naive approach to make this space.id/... didn't work, because Active Storage strips out the directories.
@zspencer is pre-pending the filenames with the space id (but having all files be together in the bucket) enough for our needs right now?
There was a problem hiding this comment.
I think so, I still think bucket-per-space is a better strategy for segmenting data but I can live with prefixing with space-id for now.
There was a problem hiding this comment.
PS. If we end up going with this approach, and liking it, I imagine that this callback method could be extracted into a module to be shared by any model that has attachments.
| return unless photo.attached? && photo.blob.persisted? | ||
| return if photo.blob.filename.to_s.start_with?(space.id.to_s) | ||
|
|
||
| new_name = "#{space.id}-#{photo.blob.filename}" |
There was a problem hiding this comment.
I think so, I still think bucket-per-space is a better strategy for segmenting data but I can live with prefixing with space-id for now.
Marketplace: Add basic support for Product photosMarketplace: Add basic support for Product photos
de50486 to
f53dd91
Compare
|
I probably won't be able to move this forward until next Wednesday's ensemble. If anyone else feels like picking up this PR and moving it forward before then, please feel free! |
|
For now, I'm thinking we take the photos as the maximum size they are; and we add variants on a per-location basis. I'm not sure if this is a long-term good idea; but in the near term it should give us flexibility for figuring out what the sizing should be. |
Marketplace: Add basic support for Product photosMarketplace: Seller may add Photo to Product
|
I have a set of PRs that are ready to merge into this that fulfill the remaining checkboxes. @anaulin - Do you want to review / merge them before we merge this or can I merge this now? |
|
@zspencer brilliant! feel free to merge ALL the things! |
Marketplace:Product::Photos #1428Utility:AWS::S3#1430This adds very basic support for adding photos to a
Marketplace::Product, including:space.idspace.id/<filename>, but the default Active Storage behavior is to strip all directories, so that didn't workvipsimage library installation as part of setupWIP screenshots
Still todo: