Better handling of custom SVG sprites in themes when using S3

Better handling of custom SVG sprites in themes when using S3

diff --git a/lib/svg_sprite/svg_sprite.rb b/lib/svg_sprite/svg_sprite.rb
index 88eec16..c5e9074 100644
--- a/lib/svg_sprite/svg_sprite.rb
+++ b/lib/svg_sprite/svg_sprite.rb
@@ -202,11 +202,13 @@ module SvgSprite
     ThemeField.where(type_id: ThemeField.types[:theme_upload_var], name: THEME_SPRITE_VAR_NAME, theme_id: Theme.transform_ids(theme_ids))
       .pluck(:upload_id).each do |upload_id|
 
-      upload = Upload.find(upload_id)
-      original_path = Discourse.store.path_for(upload)
-      if original_path.blank?
+      upload = Upload.find(upload_id) rescue nil
+
+      if Discourse.store.external?
         external_copy = Discourse.store.download(upload) rescue nil
         original_path = external_copy.try(:path)
+      else
+        original_path = Discourse.store.path_for(upload)
       end
 
       custom_sprite_paths << original_path if original_path.present?
diff --git a/spec/components/svg_sprite/svg_sprite_spec.rb b/spec/components/svg_sprite/svg_sprite_spec.rb
index f511e61..287e591 100644
--- a/spec/components/svg_sprite/svg_sprite_spec.rb
+++ b/spec/components/svg_sprite/svg_sprite_spec.rb
@@ -119,6 +119,30 @@ describe SvgSprite do
     expect(SvgSprite.bundle([theme.id])).to match(/my-custom-theme-icon/)
   end
 
+  context "s3" do
+    let(:upload_s3) { Fabricate(:upload_s3) }
+
+    before do
+      SiteSetting.enable_s3_uploads = true
+      SiteSetting.s3_upload_bucket = "s3bucket"
+      SiteSetting.s3_access_key_id = "s3_access_key_id"
+      SiteSetting.s3_secret_access_key = "s3_secret_access_key"
+
+      stub_request(:get, upload_s3.url).to_return(status: 200, body: "Hello world")
+    end
+
+    it 'includes svg sprites in themes stored in s3' do
+      theme = Fabricate(:theme)
+      theme.set_field(target: :common, name: SvgSprite.theme_sprite_variable_name, upload_id: upload_s3.id, type: :theme_upload_var)
+      theme.save!
+
+      sprite_files = SvgSprite.custom_svg_sprites([theme.id]).join("|")
+
+      expect(sprite_files).to match(/#{upload_s3.sha1}/)
+      expect(sprite_files).not_to match(/amazonaws/)
+    end
+  end
+
   it 'includes icons from SiteSettings' do
     SiteSetting.svg_icon_subset = "blender|drafting-compass|fab-bandcamp"

GitHub sha: 42818b81

Not generating an exception is tiny bit faster

upload = Upload.find_by(id: upload_id)
1 Like

Thanks! I’ve included it in the related PR: DEV: Fix store.path_for for both local and external storage by pmusaraj · Pull Request #7640 · discourse/discourse · GitHub

1 Like