Add logo and favicon to stores by IslamElsayed · Pull Request #6657 · solidusio/solidus

IslamElsayed · 2026-10-03T10:04:25Z

Summary

Fixes #6092, with the scope agreed there with @jarednorman: a logo and a favicon per store, uploaded in solidus_admin, and used by the storefront.

Spree::Store declares has_one_attached :logo and has_one_attached :favicon, validated against Spree::Config.allowed_image_mime_types like product images. There's no Paperclip variant and no change to the image adapter, so Spree::Image and Spree::Taxon are untouched.
solidus_admin: a "Logo and favicon" panel on the store form, with a file field for each and a thumbnail of the current image. The input component now allows type: :file, and the field component gets a file_field helper.
Storefront: the layout's favicon and the header and footer logos come from the current store when it has them, otherwise they stay the Solidus ones.

Admin uploads only replace an image; there's no way to remove one yet. Happy to add that here or in a follow-up.

The first version added Paperclip support and split the Active Storage adapter to allow two attachments; that's been removed after @tvdeyen's review (63cdfad).

Tests: model specs for the attachments and their validation, request specs for the admin form and uploads, the admin stores feature spec, and storefront specs for the default and the store's images. I ran the core specs with Active Storage and with DISABLE_ACTIVE_STORAGE=true, the admin specs, and the storefront specs in an app generated from this branch with the starter template. standardrb reports no offenses.

I used an AI assistant while preparing this PR; I reviewed and tested the changes myself.

Checklist

I agree that my PR will be published under the same license as Solidus.
I have written a thorough PR description.
I have kept my commits small and atomic.
I have localized any and all user-facing strings that I added to the source code.
I have used clear, explanatory commit messages.
✅ I have added automated tests to cover my changes.

tvdeyen

I like the feature. Thanks for contributing.

But we should not add the extra complexity for the legacy paperclip adapter. The extra module that was extracted for that reason should not be necessary. Let's just add has_one_attached :logo and :favicon directly into the store model.

codecov · 2026-10-11T10:00:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.47%. Comparing base (4dd5312) to head (0904e82).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6657      +/-   ##
==========================================
+ Coverage   92.46%   92.47%   +0.01%     
==========================================
  Files        1058     1059       +1     
  Lines       21493    21527      +34     
==========================================
+ Hits        19873    19907      +34     
  Misses       1620     1620

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

IslamElsayed · 2026-10-11T11:32:52Z

@tvdeyen thanks, that makes sense. Done in 63cdfad: Spree::Store now just declares has_one_attached :logo and has_one_attached :favicon, with the same content-type validation as product images. The Paperclip variant, the store_attachment_module setting, its migration and the HasAttachments extraction are gone, so the image adapter is back to how it is on main. The admin preview and the storefront helpers use the Active Storage attachments directly. I also merged main, which fixes the Lint failure: those were rubocop:disable comments already removed on main.

(I used AI assistance while working on this; I reviewed and tested the changes myself.)

tvdeyen · 2026-10-14T05:33:20Z

      <% end %>
+      <%= render component('ui/panel').new(title: t('.branding')) do |panel| %>
+        <% panel.with_section(class: "flex flex-col gap-4") do %>
+          <% %i[logo favicon].each do |attachment| %>


Render a component for each attachment separately. We can then remove the helper.

tvdeyen · 2026-10-14T05:34:29Z


+  # Only a saved attachment has a URL: after a failed save the form is shown
+  # again with a new file assigned but not stored yet.
+  def attachment_preview_url(name)


this helper takes every method name as an argument and simply sends it. it will fail if we call with anything different from the the attachment names. we dont need this helper if we render each attachment component separately.

tvdeyen · 2026-10-14T05:35:22Z

          </div>
        <% end %>
      <% end %>
+      <%= render component('ui/panel').new(title: t('.branding')) do |panel| %>


we need a conditional for stores not using active storage. it will fail otherwise

tvdeyen · 2026-10-14T05:36:51Z

+  describe "logo and favicon" do
+    let(:admin_user) { create(:admin_user) }
+    let(:store) { create(:store, default: false) }
+    let(:image) { Rack::Test::UploadedFile.new(Spree::Core::Engine.root.join("lib/spree/testing_support/fixtures/blank.jpg"), "image/jpeg") }


there is a fixture_file_upload helper we could use.

tvdeyen · 2026-10-14T05:38:38Z

  # hosted by a single Solidus implementation can be built.
  #
  class Store < Spree::Base
+    has_one_attached :logo


we need to safe guard these for stores not using active storage adapter. it will fail without active storage available

IslamElsayed · 2026-10-14T13:57:17Z

Thanks, all five are in 9ab97f0:

Spree::Store.attachments_available? (the same check SolidusAdmin::OrdersController uses for image preloading) gates has_one_attached :logo/:favicon and the image validation, so a store on the Paperclip adapter doesn't define them at all. The admin form and the storefront logo/favicon helpers check it too.
Each attachment is rendered by its own stores/form/attachment component, which takes @store.logo / @store.favicon directly, so the attachment_preview_url helper is gone.
The request spec uses fixture_file_upload.

Specs pass with Active Storage and with DISABLE_ACTIVE_STORAGE=true (which covers the guard).

AlistairNorman

Thanks for all the work on this. Can you clean up the commit history? It looks like there is a lot of code here that is added and then deleted in a later commit. That obscures the actual approach being taken here. Please use small and atomic commits that represent a coherent series of changes.

Stores can now have their own logo and favicon, so that each store of a multi-store setup can be branded differently. They are declared with has_one_attached and are only defined when images use the Active Storage adapter; Spree::Store.attachments_available? tells callers whether they exist.

IslamElsayed · 2026-10-17T09:25:33Z

Thanks @AlistairNorman, fair point. I've rebuilt the branch on current main as three commits:

Add logo and favicon attachments to Spree::Store (core model, attachments_available?, specs)
Allow uploading them in solidus_admin
Use them in the storefront templates

The Paperclip variant and the merge commits are gone; the resulting diff is the same as before.

(AI-assisted: prepared with Claude, reviewed by me.)

tvdeyen

Thanks. One change and we should be ready

tvdeyen · 2026-10-17T12:18:06Z

        <% end %>
      <% end %>
+      <%# Only stores on the Active Storage adapter, with a Solidus that has them, have a logo and favicon. %>
+      <% if Spree::Store.reflect_on_attachment(:logo) %>


Why aren't we using Spree::Store.attachments_available? here as well?

tvdeyen · 2026-10-17T12:19:57Z

      ]
      # This check can be removed once we drop support for Solidus 4.7 and earlier.
      permitted << {address_attributes: Spree::PermittedAttributes.address_attributes} if Spree::Store.reflect_on_association(:address)
+      permitted.push(:logo, :favicon) if Spree::Store.reflect_on_attachment(:logo)


Should we use Spree::Store.attachments_available? here as well?

The store form gets a file field for each attachment, showing the current image when there is one, and skips them when the store has no attachments. The form input component now accepts the file type, and the field component gets a file_field helper to render it.

The layout, header and footer use the current store's favicon and logo when it has them, and keep the Solidus ones otherwise.

IslamElsayed · 2026-10-18T11:17:13Z

Done: both now use Spree::Store.attachments_available?, through try, because solidus_admin still supports solidus_core >= 4.6, which doesn't define it. That was also why I'd used reflect_on_attachment there. I folded the change into the admin commit to keep the history atomic.

(AI-assisted: prepared with Claude, reviewed by me.)

jarednorman

I think this is good now.

IslamElsayed · 2026-10-20T10:08:47Z

@AlistairNorman the history is now three atomic commits (core, admin, storefront) as you asked. Could you take another look? The one red job (solidus_promotions / Rails 7.2, Ruby 3.3, postgres, activestorage) fails Promotion Categories allows to bulk delete promo category, which this PR doesn't touch; the same spec passes in the other three promotions jobs on this head, so it looks flaky. Could someone re-run it?

(AI-assisted: drafted with Claude, reviewed by me.)

IslamElsayed requested a review from a team as a code owner October 3, 2026 10:04

github-actions Bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus_admin changelog:solidus_storefront Changes to the storefront labels Oct 3, 2026

github-project-automation Bot added this to Solidus Admin Oct 3, 2026

tvdeyen requested changes Oct 11, 2026

View reviewed changes

IslamElsayed requested a review from tvdeyen October 11, 2026 11:38

tvdeyen reviewed Oct 14, 2026

View reviewed changes

AlistairNorman requested changes Oct 16, 2026

View reviewed changes

IslamElsayed force-pushed the store-logo-and-favicon branch from 2b331ae to d9d7b38 Compare October 17, 2026 09:25

tvdeyen reviewed Oct 17, 2026

View reviewed changes

IslamElsayed added 2 commits October 18, 2026 14:16

Use the store's logo and favicon in the storefront

0904e82

The layout, header and footer use the current store's favicon and logo when it has them, and keep the Solidus ones otherwise.

IslamElsayed force-pushed the store-logo-and-favicon branch from d9d7b38 to 0904e82 Compare October 18, 2026 11:17

jarednorman approved these changes Oct 20, 2026

View reviewed changes

Conversation

IslamElsayed commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

tvdeyen left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Oct 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

IslamElsayed commented Oct 11, 2026

Uh oh!

tvdeyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

IslamElsayed commented Oct 14, 2026

Uh oh!

AlistairNorman left a comment

Choose a reason for hiding this comment

Uh oh!

IslamElsayed commented Oct 17, 2026

Uh oh!

tvdeyen left a comment

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

tvdeyen Oct 17, 2026

Choose a reason for hiding this comment

Uh oh!

IslamElsayed commented Oct 18, 2026

Uh oh!

jarednorman left a comment

Choose a reason for hiding this comment

Uh oh!

IslamElsayed commented Oct 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

IslamElsayed commented Oct 3, 2026 •

edited

Loading

codecov Bot commented Oct 11, 2026 •

edited

Loading