Skip to content

NEEDS DISCUSSION: Add admin asset library (/asset_library) with editable filename + caption#1953

Draft
maebeale wants to merge 12 commits into
mainfrom
maebeale/admin-image-index
Draft

NEEDS DISCUSSION: Add admin asset library (/asset_library) with editable filename + caption#1953
maebeale wants to merge 12 commits into
mainfrom
maebeale/admin-image-index

Conversation

@maebeale

@maebeale maebeale commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

🤖 suggested review level: 3 Read 📖 new admin library with contained filter/search logic, a decorator, an admin-only policy, and inline edits that mutate title + Active Storage blob filename; no schema changes

What is the goal of this PR and why is this important?

  • Admins had no single place to see every uploaded asset. This adds an admin-only Asset library (/asset_library) listing all assets — PrimaryAsset, GalleryAsset, RichTextAsset, DownloadableAsset — including PDFs/docs.
  • Two things about a file were previously hard/impossible to edit, now both are inline-editable per asset:
    • CaptionAsset#title (metadata label)
    • Filename — the Active Storage blob filename (what downloads)
  • Results can be narrowed by type, attached-to record, file type (jpg/png/pdf/…), and search visibility.
  • A "New resource" button opens the resource form with "Hidden from search" pre-checked, so newly created resources start hidden.

How did you approach the change?

  • Asset.search matches a keyword against title, attached filename (join active_storage_blobs), and the owner model name; present_types/present_owner_types/present_content_types power the dropdown filters. File-type filter keys off the blob content_type (not the filename string), so a mislabeled extension can't slip through.
  • Search-visibility filter keys off the owning Resource's hidden_from_search (a Resource-only concept): "Hidden from search" returns only assets attached to a hidden resource; "Searchable" returns everything else. No asset-level column — the filter composes as an id-membership subquery so it stacks with the other filters.
  • AssetsController#index follows the standard lazy turbo_frame + filter pattern. #update handles the two inline fields independently — each is its own turbo frame.
  • The "New resource" button links to new_resource_path(hidden_from_search: true); resources#new now honors that param to pre-check the box.
  • Served at /asset_library, not /assets: the sprockets pipeline owns the /assets/* prefix and would shadow /assets/:id.
  • AssetPolicy gates index + inline edits to admins (with: AssetPolicy, so it holds across STI subtypes).
  • AssetDecorator centralizes type label, owner label/link, filename, content-type badge, and thumbnail.
  • The library container spans the full content width (max-w-none), and its admin-home card now sits above Banners.

Anything else to add?

  • Non-image files show a file-type badge instead of a thumbnail; assets with no attached file show "No file attached" (no filename field).
  • Specs: model search + attached_to_hidden_resource + present_content_types, request (auth, filters incl. file type + resource visibility, caption edit, filename rename), policy, page_bg alignment.

Admins had no way to see every uploaded asset in one place, and Primary/
Gallery asset titles were never editable in the UI (only RichTextAsset was).
This surfaces all assets in one searchable index and makes the label
editable inline for every type.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 13:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread app/controllers/images_controller.rb Outdated

private

def set_image

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Authorized with with: AssetPolicy explicitly so the rule is admin-only for every STI subtype. Without it, ActionPolicy would resolve to the subtype policy (e.g. PrimaryAssetPolicy#update? = authenticated?), which is looser than we want for the index.

Comment thread app/models/asset.rb
like = "%#{sanitize_sql_like(query)}%"
left_joins(:file_blob).where(
"assets.title LIKE :like OR assets.owner_type LIKE :like OR active_storage_blobs.filename LIKE :like",
like: like

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: left_joins(:file_blob) is safe from row multiplication here because has_one_attached :file means at most one attachment per asset — so no distinct needed, and counts/pagination stay correct.

Renames the admin index from /images to /asset_library (the sprockets
pipeline owns /assets, so a literal /assets resource would be shadowed) and
reframes it around "assets" now that PDFs/docs are included. Adds a second
inline-editable field for the download filename (blob), distinct from the
caption (Asset#title), and orders filename above the attached-to record.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maebeale maebeale changed the title Add admin Images index with searchable, inline-editable labels Add admin asset library (/asset_library) with editable filename + caption Jul 8, 2026
Comment thread config/routes.rb
Rails.application.routes.draw do
resources :primary_assets
resources :rich_text_assets
# Admin asset library. Served at /asset_library rather than /assets because the

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Routed at /asset_library rather than /assets on purpose: the sprockets pipeline claims the /assets/* prefix, so a resources :assets member route (/assets/:id) would be shadowed by asset serving, and the generated asset_path helper would collide with ActionView's asset_path.

authorize! @asset, to: :update?, with: AssetPolicy

@asset.title = params.dig(:asset, :title) if params[:asset]&.key?(:title)
rename_file(params.dig(:asset, :filename))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Caption (title) and filename (blob) are edited independently — each field is its own turbo frame. The response renders both frames (_editable_fields) and Turbo swaps only the one that submitted, so editing the filename never clobbers an in-progress caption edit and vice versa.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 13:44

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Renders a visual thumbnail for previewable files (PDFs via the Poppler
previewer, images via variants) using file.representation, matching how
resources display PDFs. Non-previewable files (e.g. Word docs) keep the
file-type badge; PDFs without a previewer fall back to a red PDF icon.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 13:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Renders PDFs in an <iframe> pointed at the same-origin inline blob URL so the
browser's native viewer shows all pages scrollably, rather than a static
first-page image. This also drops the Poppler dependency for PDF display.
Images still use an image variant; other files keep the file-type badge.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 14:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@maebeale

maebeale commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author
Screenshot 2026-07-08 at 10 15 17 AM

Wire the search form to the collection Stimulus controller so the keyword box
debounce-submits (400ms) and the type/attached-to selects submit on change,
matching the other index pages — no more hitting Enter. Clear filters resets
via collection#clearAndSubmit, and results blur while the next page loads.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 14:17

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

The keyword search now matches the specific owner record's title/name (e.g.
searching a workshop title like "Befriending" finds its assets), not just the
owner type. Owner-name matches are resolved per owner_type since owners are
polymorphic across models using title vs name columns; id-only queries strip
the controller's polymorphic :owner include to avoid an eager-load JOIN.

Attached-to line uses line-clamp-2 so longer owner titles are visible.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 14:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Admins searching the library often want just PDFs, or just images. A
content-type filter narrows results without leaning on keyword guesses.
Filters on the blob's content type (not the filename string) so a
mislabeled extension can't slip through.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 14:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

base = Asset.includes(:owner, file_attachment: :blob)
base = base.where(type: params[:type]) if params[:type].present?
base = base.where(owner_type: params[:owner_type]) if params[:owner_type].present?
base = base.where(id: Asset.joins(:file_blob).where(active_storage_blobs: { content_type: params[:content_type] })) if params[:content_type].present?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Filtered via a where(id: …) subquery on the blob join rather than chaining .joins(:file_blob) onto basebase already includes(file_attachment: :blob), and a second join referencing active_storage_blobs would collide with the eager-load alias. The subquery keeps the two blob references independent.

Move the Asset library card above Banners so the new library is easier
to find among the system tools.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 14:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Admins need to stage assets in the library without immediately exposing
them to search, and a fast way to add one-off assets that aren't tied to
a specific owner record. This adds a hidden_from_search flag (default
false), a search-visibility filter, and a New asset form that defaults
the flag on so uploads stay hidden until deliberately surfaced.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 15:02

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread app/controllers/assets_controller.rb Outdated

# STI subclass to instantiate, constrained to the known TYPES so a stray param
# can't constantize into an arbitrary class.
def asset_class

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: create instantiates the STI subclass via presence_in(Asset::TYPES) before constantize — a stray type param can't constantize into an arbitrary class, and an unknown/blank type safely falls back to the first known type. The permitted :type is stripped from the attributes hash since the subclass already fixes it.

Assets are created in the context of an owner record, and the standalone
asset-create form couldn't set attributes reliably. Resources natively
support hidden_from_search, so the library's create button now opens the
resource form with "Hidden from search" pre-checked, and the unused asset
new/create flow is removed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 15:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Drop the unused asset-level hidden_from_search column and instead scope
the visibility filter through the owning Resource: "Hidden from search"
returns only assets on a hidden resource, "Searchable" returns everything
else. hidden_from_search stays a Resource-only concept.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 8, 2026 16:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

base = base.where(type: params[:type]) if params[:type].present?
base = base.where(owner_type: params[:owner_type]) if params[:owner_type].present?
base = base.where(id: Asset.joins(:file_blob).where(active_storage_blobs: { content_type: params[:content_type] })) if params[:content_type].present?
hidden_ids = Asset.attached_to_hidden_resource.select(:id)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 From Claude: Visibility keys off the owning Resource's hidden_from_search (a Resource-only flag), via an id-membership subquery so it composes with the type/owner/file-type filters. "searchable" is the complement (where.not(id: hidden_ids)), which correctly includes non-resource-owned assets and assets on non-hidden resources.

@maebeale maebeale changed the title Add admin asset library (/asset_library) with editable filename + caption NEEDS DISCUSSION: Add admin asset library (/asset_library) with editable filename + caption Jul 9, 2026
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