Add inline gallery controls to set featured image and remove images#1545
Add inline gallery controls to set featured image and remove images#1545maebeale wants to merge 1 commit into
Conversation
Stakeholders need to manage gallery images without digging into the edit form. Each gallery image now exposes an inline "set as featured" and "remove" control for users who can manage the owning record, so changing the hero image or pruning the gallery is a one-click action from the public show page. The controls reuse the existing PrimaryAsset/GalleryAsset STI swap that the "Change photo" picker already relies on, keeping a single source of truth for what "featured" means. Closes #1510 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| # end | ||
| resources :primary_assets | ||
| resources :rich_text_assets | ||
| resources :gallery_assets, only: [ :destroy ] do |
There was a problem hiding this comment.
Routed under /gallery_assets rather than resources :assets on purpose: the Rails asset pipeline owns the /assets/* URL prefix and returns 405 for non-GET requests, which would shadow these DELETE/POST routes.
| authorize! @asset.owner, to: :manage? | ||
| ActiveRecord::Base.transaction do | ||
| if existing_primary = @asset.owner.assets.find_by(type: "PrimaryAsset") | ||
| existing_primary.update!(type: "GalleryAsset") |
There was a problem hiding this comment.
Promotion is a type swap, not a new upload: the current PrimaryAsset is demoted back to GalleryAsset and the chosen gallery image becomes the PrimaryAsset. This mirrors the existing PrimaryAssetsController#create flow used by the "Change photo" picker, so there's a single definition of "featured".
| <% gallery_assets = resource.gallery_assets.select { |img| img.file.attached? } %> | ||
| <% align = (defined?(align) && align.present?) ? align.to_s : "center" %> | ||
| <% if gallery_assets.any? %> | ||
| <% can_manage = allowed_to?(:manage?, resource) %> |
There was a problem hiding this comment.
Controls are gated by allowed_to?(:manage?, resource) so they only render for users who can manage the owning record. The controller re-checks the same policy server-side, so this is purely a UI gate. Because it lives in the shared gallery partial, the feature works for every asset owner (stories, story ideas, workshops, events, …), not just stories.
There was a problem hiding this comment.
Pull request overview
Adds inline “set featured” and “remove” controls to gallery images across asset-owning resources, backed by new routes and a controller that performs the existing PrimaryAsset ⇄ GalleryAsset STI swap and deletion.
Changes:
- Introduces
GalleryAssetsControllerwithdestroyandmake_primaryactions to remove/promote gallery assets. - Adds
/gallery_assetsroutes (DELETE + member POSTmake_primary) and renders per-image overlay controls from the gallery display partial. - Adds request specs covering admin vs non-admin behavior for both endpoints.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/requests/gallery_assets_spec.rb | Adds request coverage for deleting and promoting gallery assets. |
| config/routes.rb | Adds gallery_assets routes for destroy and make_primary. |
| app/views/assets/_gallery_item_controls.html.erb | New overlay UI with featured/remove buttons per gallery image. |
| app/views/assets/_display_gallery_media.html.erb | Wraps gallery items with a hover/focus overlay when user can manage the resource. |
| app/controllers/gallery_assets_controller.rb | Implements destroy + featured-image promotion via STI type swap in a transaction. |
| context "as a non-admin user" do | ||
| before { sign_in regular_user } | ||
|
|
||
| it "does not change the asset type" do | ||
| post make_primary_gallery_asset_url(gallery_asset) | ||
|
|
||
| expect(Asset.find(gallery_asset.id).type).to eq("GalleryAsset") | ||
| end | ||
| end | ||
| end |
| <%= button_to make_primary_gallery_asset_path(asset), | ||
| method: :post, | ||
| class: "bg-white text-gray-700 rounded-full w-8 h-8 flex items-center justify-center shadow hover:bg-gray-100 transition", | ||
| title: "Set as featured image", | ||
| data: { turbo_confirm: "Make this the featured image?" } do %> | ||
| <i class="fa-solid fa-star"></i> | ||
| <% end %> | ||
| <%= button_to gallery_asset_path(asset), | ||
| method: :delete, | ||
| class: "bg-white text-red-600 rounded-full w-8 h-8 flex items-center justify-center shadow hover:bg-gray-100 transition", | ||
| title: "Remove image", | ||
| data: { turbo_confirm: "Remove this image from the gallery?" } do %> |
| def make_primary | ||
| authorize! @asset.owner, to: :manage? | ||
| ActiveRecord::Base.transaction do | ||
| if existing_primary = @asset.owner.assets.find_by(type: "PrimaryAsset") | ||
| existing_primary.update!(type: "GalleryAsset") | ||
| end | ||
|
|
||
| @asset.update!(type: "PrimaryAsset") | ||
| end | ||
|
|
||
| redirect_back_or_to polymorphic_path(@asset.owner), notice: "Featured image updated." | ||
| end |
Closes #1510
What is the goal of this PR and why is this important?
How did you approach the change?
GalleryAssetsControllerwithdestroy(remove an image) andmake_primary(promote a gallery image to the featured/primary image) actionsmake_primaryreuses the samePrimaryAsset⇄GalleryAssetSTI swap the existing "Change photo" picker already uses, so there is one definition of what "featured" means — the old featured image is demoted back into the galleryassets/_gallery_item_controlspartial, overlaid on each image and revealed on hover; they only render for users who canmanage?the owning record (viaallowed_to?), and every action re-checks the owner's policy server-sideassets/_display_gallery_mediapartial, this works for any asset owner (stories, story ideas, workshops, events, etc.), not just stories/gallery_assetsrather than/assetsto avoid colliding with the Rails asset-pipeline URL prefix (which 405s non-GET requests)UI Testing Checklist
Anything else to add?