FEATURE: Allow themes to define screenshots (#29079)

This commit allows themes to define up to 2 screenshots
in about.json. These should be paths within the theme's
git repository, images with a 1MB max file size and max width 3840x2160.

These screenshots will be downloaded and stored against a theme
field, and we will use these in the redesigned theme grid UI.

These screenshots will be updated when the theme is updated
in the same way the additional theme files are.

For now this is gated behind a hidden `theme_download_screenshots`
site setting, to allow us to test this on a small number of sites without
making other sites make unnecessary uploads.

**Future considerations:**

* We may want to have a specialized naming system for screenshots. E.g. having light.png/dark.png/some_palette.png
* We may want to show more than one screenshot for the theme, maybe in a carousel or reacting to dark mode or color palette changes
* We may want to allow clicking on the theme screenshot to show a lightbox
* We may want to make an optimized thumbnail image for the theme grid

---------

Co-authored-by: Ted Johansson <ted@discourse.org>

app/assets/javascripts/admin/addon/components/themes-grid-card.gjs

@@ -82,10 +82,10 @@ export default class ThemeCard extends Component {
     >
       <:content>
         <div class="theme-card__image-wrapper">
-          {{#if @theme.screenshot}}
+          {{#if @theme.screenshot_url}}
             <img
               class="theme-card__image"
-              src={{htmlSafe @theme.screenshot}}
+              src={{@theme.screenshot_url}}
               alt={{@theme.name}}
             />
           {{else}}

app/models/remote_theme.rb

@@ -31,6 +31,9 @@ class RemoteTheme < ActiveRecord::Base
   MAX_ASSET_FILE_SIZE = 8.megabytes
   MAX_THEME_FILE_COUNT = 1024
   MAX_THEME_SIZE = 256.megabytes
+  MAX_THEME_SCREENSHOT_FILE_SIZE = 1.megabyte
+  MAX_THEME_SCREENSHOT_DIMENSIONS = [3840, 2160] # 4K resolution
+  THEME_SCREENSHOT_ALLOWED_FILE_TYPES = %w[.jpg .jpeg .gif .png].freeze
 
   has_one :theme, autosave: false
   scope :joined_remotes,
@@ -250,15 +253,7 @@ class RemoteTheme < ActiveRecord::Base
 
     theme_info["assets"]&.each do |name, relative_path|
       if path = importer.real_path(relative_path)
-        new_path = "#{File.dirname(path)}/#{SecureRandom.hex}#{File.extname(path)}"
+        upload = create_upload(path, relative_path)
-        File.rename(path, new_path) # OptimizedImage has strict file name restrictions, so rename temporarily
-        upload =
-          UploadCreator.new(
-            File.open(new_path),
-            File.basename(relative_path),
-            for_theme: true,
-          ).create_for(theme.user_id)
-
         if !upload.errors.empty?
           raise ImportError,
                 I18n.t(
@@ -277,6 +272,67 @@ class RemoteTheme < ActiveRecord::Base
       end
     end
 
+    # TODO (martin): Until we are ready to roll this out more
+    # widely, let's avoid doing this work for most sites.
+    if SiteSetting.theme_download_screenshots
+      theme_info["screenshots"] = Array.wrap(theme_info["screenshots"]).take(2)
+      theme_info["screenshots"].each_with_index do |relative_path, idx|
+        if path = importer.real_path(relative_path)
+          if !THEME_SCREENSHOT_ALLOWED_FILE_TYPES.include?(File.extname(path))
+            raise ImportError,
+                  I18n.t(
+                    "themes.import_error.screenshot_invalid_type",
+                    file_name: File.basename(path),
+                    accepted_formats: THEME_SCREENSHOT_ALLOWED_FILE_TYPES.join(","),
+                  )
+          end
+
+          if File.size(path) > MAX_THEME_SCREENSHOT_FILE_SIZE
+            raise ImportError,
+                  I18n.t(
+                    "themes.import_error.screenshot_invalid_size",
+                    file_name: File.basename(path),
+                    max_size:
+                      ActiveSupport::NumberHelper.number_to_human_size(
+                        MAX_THEME_SCREENSHOT_FILE_SIZE,
+                      ),
+                  )
+          end
+
+          screenshot_width, screenshot_height = FastImage.size(path)
+          if (screenshot_width.nil? || screenshot_height.nil?) ||
+               screenshot_width > MAX_THEME_SCREENSHOT_DIMENSIONS[0] ||
+               screenshot_height > MAX_THEME_SCREENSHOT_DIMENSIONS[1]
+            raise ImportError,
+                  I18n.t(
+                    "themes.import_error.screenshot_invalid_dimensions",
+                    file_name: File.basename(path),
+                    width: screenshot_width.to_i,
+                    height: screenshot_height.to_i,
+                    max_width: MAX_THEME_SCREENSHOT_DIMENSIONS[0],
+                    max_height: MAX_THEME_SCREENSHOT_DIMENSIONS[1],
+                  )
+          end
+
+          upload = create_upload(path, relative_path)
+          if !upload.errors.empty?
+            raise ImportError,
+                  I18n.t(
+                    "themes.import_error.screenshot",
+                    errors: upload.errors.full_messages.join(","),
+                  )
+          end
+
+          updated_fields << theme.set_field(
+            target: :common,
+            name: "screenshot_#{idx + 1}",
+            type: :theme_screenshot_upload_var,
+            upload_id: upload.id,
+          )
+        end
+      end
+    end
+
     # Update all theme attributes if this is just a placeholder
     if self.remote_url.present? && !self.local_version && !self.commits_behind
       self.theme.name = theme_info["name"]
@@ -466,6 +522,19 @@ class RemoteTheme < ActiveRecord::Base
   def is_git?
     remote_url.present?
   end
+
+  def create_upload(path, relative_path)
+    new_path = "#{File.dirname(path)}/#{SecureRandom.hex}#{File.extname(path)}"
+
+    # OptimizedImage has strict file name restrictions, so rename temporarily
+    File.rename(path, new_path)
+
+    UploadCreator.new(
+      File.open(new_path),
+      File.basename(relative_path),
+      for_theme: true,
+    ).create_for(theme.user_id)
+  end
 end
 
 # == Schema Information

app/models/theme_field.rb

@@ -16,11 +16,31 @@ class ThemeField < ActiveRecord::Base
   validate :migration_filename_is_valid, if: :migration_field?
 
   after_save do
-    if self.type_id == ThemeField.types[:theme_upload_var] && saved_change_to_upload_id?
+    if (
+         self.type_id == ThemeField.types[:theme_screenshot_upload_var] ||
+           self.type_id == ThemeField.types[:theme_upload_var]
+       ) && saved_change_to_upload_id?
       UploadReference.ensure_exist!(upload_ids: [self.upload_id], target: self)
     end
   end
 
+  after_save do
+    dependent_fields.each(&:invalidate_baked!)
+
+    if upload && svg_sprite_field?
+      upsert_svg_sprite!
+      SvgSprite.expire_cache
+    end
+  end
+
+  after_destroy do
+    if svg_sprite_field?
+      ThemeSvgSprite.where(theme_id: theme_id).delete_all
+
+      SvgSprite.expire_cache
+    end
+  end
+
   scope :find_by_theme_ids,
         ->(theme_ids) do
           return none if theme_ids.blank?
@@ -69,6 +89,7 @@ class ThemeField < ActiveRecord::Base
         theme_var: 4, # No longer used
         yaml: 5,
         js: 6,
+        theme_screenshot_upload_var: 7,
       )
   end
 
@@ -717,21 +738,8 @@ class ThemeField < ActiveRecord::Base
     end
   end
 
-  after_save do
+  def upload_url
-    dependent_fields.each(&:invalidate_baked!)
+    self.upload&.url
-
-    if upload && svg_sprite_field?
-      upsert_svg_sprite!
-      SvgSprite.expire_cache
-    end
-  end
-
-  after_destroy do
-    if svg_sprite_field?
-      ThemeSvgSprite.where(theme_id: theme_id).delete_all
-
-      SvgSprite.expire_cache
-    end
   end
 
   private

app/serializers/theme_serializer.rb

@@ -13,7 +13,8 @@ class ThemeSerializer < BasicThemeSerializer
              :description,
              :enabled?,
              :disabled_at,
-             :theme_fields
+             :theme_fields,
+             :screenshot_url
 
   has_one :color_scheme, serializer: ColorSchemeSerializer, embed: :object
   has_one :user, serializer: UserNameSerializer, embed: :object
@@ -47,6 +48,13 @@ class ThemeSerializer < BasicThemeSerializer
     @include_theme_field_values || object.remote_theme_id.nil?
   end
 
+  def screenshot_url
+    object
+      .theme_fields
+      .find { |field| field.type_id == ThemeField.types[:theme_screenshot_upload_var] }
+      &.upload_url
+  end
+
   def child_themes
     object.child_themes
   end

config/locales/server.en.yml

@@ -85,6 +85,10 @@ en:
     import_error:
       generic: An error occurred while importing that theme
       upload: "Error creating upload asset: %{name}. %{errors}"
+      screenshot: "Error importing theme screenshots. %{errors}"
+      screenshot_invalid_type: "The theme screenshots must be in one of the following formats: %{accepted_formats}. The screenshot %{file_name} has an invalid format."
+      screenshot_invalid_size: "The theme screenshots must be less than %{max_size}. The screenshot %{file_name} is too large."
+      screenshot_invalid_dimensions: "The theme screenshots must be maximum %{max_width}x%{max_height}. The screenshot %{file_name} exceeds this. Its dimensions are %{width}x%{height}."
       about_json: "Import Error: about.json does not exist, or is invalid. Are you sure this is a Discourse Theme?"
       about_json_too_big: "Import Error: about.json is bigger than the %{limit} limit."
       about_json_values: "about.json contains invalid values: %{errors}"

config/site_settings.yml

@@ -1525,6 +1525,9 @@ files:
     default: "wasm|jpg|jpeg|png|woff|woff2|svg|eot|ttf|otf|gif|webp|avif|js"
     type: list
     list_type: file_types
+  theme_download_screenshots:
+    default: false
+    hidden: true
   authorized_extensions:
     client: true
     default: "jpg|jpeg|png|gif|heic|heif|webp|avif"

spec/models/remote_theme_spec.rb

@@ -34,7 +34,8 @@ RSpec.describe RemoteTheme do
               "type": "setting",
               "value": "list_setting"
             }
-          }
+          },
+          "screenshots": ["screenshots/1.jpeg", "screenshots/2.jpeg"]
         }
       JSON
     end
@@ -72,6 +73,8 @@ RSpec.describe RemoteTheme do
         "settings.yaml" => settings,
         "locales/en.yml" => "sometranslations",
         "migrations/settings/0001-some-migration.js" => migration_js,
+        "screenshots/1.jpeg" => file_from_fixtures("logo.jpg", "images"),
+        "screenshots/2.jpeg" => file_from_fixtures("logo.jpg", "images"),
       )
     end
 
@@ -355,7 +358,7 @@ RSpec.describe RemoteTheme do
       stub_const(RemoteTheme, "MAX_THEME_FILE_COUNT", 1) do
         expect { RemoteTheme.import_theme(initial_repo_url) }.to raise_error(
           RemoteTheme::ImportError,
-          I18n.t("themes.import_error.too_many_files", count: 15, limit: 1),
+          I18n.t("themes.import_error.too_many_files", count: 17, limit: 1),
         )
       end
     end
@@ -384,6 +387,85 @@ RSpec.describe RemoteTheme do
         )
       end
     end
+
+    describe "screenshots" do
+      before { SiteSetting.theme_download_screenshots = true }
+
+      it "fails if any of the provided screenshots is not an accepted file type" do
+        stub_const(RemoteTheme, "THEME_SCREENSHOT_ALLOWED_FILE_TYPES", [".bmp"]) do
+          expect { RemoteTheme.import_theme(initial_repo_url) }.to raise_error(
+            RemoteTheme::ImportError,
+            I18n.t(
+              "themes.import_error.screenshot_invalid_type",
+              file_name: "1.jpeg",
+              accepted_formats: ".bmp",
+            ),
+          )
+        end
+      end
+
+      it "fails if any of the provided screenshots is too big" do
+        stub_const(RemoteTheme, "MAX_THEME_SCREENSHOT_FILE_SIZE", 1.byte) do
+          expect { RemoteTheme.import_theme(initial_repo_url) }.to raise_error(
+            RemoteTheme::ImportError,
+            I18n.t(
+              "themes.import_error.screenshot_invalid_size",
+              file_name: "1.jpeg",
+              max_size: "1 Bytes",
+            ),
+          )
+        end
+      end
+
+      it "fails if any of the provided screenshots has dimensions that are too big" do
+        FastImage
+          .expects(:size)
+          .with { |arg| arg.match(%r{/screenshots/1\.jpeg}) }
+          .returns([512, 512])
+        stub_const(RemoteTheme, "MAX_THEME_SCREENSHOT_DIMENSIONS", [1, 1]) do
+          expect { RemoteTheme.import_theme(initial_repo_url) }.to raise_error(
+            RemoteTheme::ImportError,
+            I18n.t(
+              "themes.import_error.screenshot_invalid_dimensions",
+              file_name: "1.jpeg",
+              width: 512,
+              height: 512,
+              max_width: 1,
+              max_height: 1,
+            ),
+          )
+        end
+      end
+
+      it "creates uploads and associated theme fields for all theme screenshots" do
+        FastImage
+          .stubs(:size)
+          .with { |arg| arg.match(%r{/screenshots/1\.jpeg}) }
+          .returns([800, 600])
+        FastImage
+          .stubs(:size)
+          .with { |arg| arg.match(%r{/screenshots/2\.jpeg}) }
+          .returns([1024, 768])
+
+        theme = RemoteTheme.import_theme(initial_repo_url)
+
+        screenshot_1 = theme.theme_fields.find_by(name: "screenshot_1")
+        screenshot_2 = theme.theme_fields.find_by(name: "screenshot_2")
+
+        expect(screenshot_1).to be_present
+        expect(screenshot_1.type_id).to eq(ThemeField.types[:theme_screenshot_upload_var])
+        expect(screenshot_2).to be_present
+        expect(screenshot_2.type_id).to eq(ThemeField.types[:theme_screenshot_upload_var])
+        expect(screenshot_1.upload).to be_present
+        expect(screenshot_2.upload).to be_present
+
+        expect(UploadReference.exists?(target: screenshot_1)).to eq(true)
+        expect(UploadReference.exists?(target: screenshot_2)).to eq(true)
+
+        expect(screenshot_1.upload.original_filename).to eq("1.jpeg")
+        expect(screenshot_2.upload.original_filename).to eq("2.jpeg")
+      end
+    end
   end
 
   let(:github_repo) do

spec/serializers/theme_serializer_spec.rb

@@ -2,8 +2,9 @@
 
 RSpec.describe ThemeSerializer do
   describe "load theme settings" do
+    fab!(:theme)
+
     it "should add error message when settings format is invalid" do
-      theme = Fabricate(:theme)
       Theme
         .any_instance
         .stubs(:settings)
@@ -16,11 +17,51 @@ RSpec.describe ThemeSerializer do
 
     it "should add errors messages from theme fields" do
       error = "error when compiling theme field"
-      theme = Fabricate(:theme)
       theme_field = Fabricate(:theme_field, error: error, theme: theme)
       serialized = ThemeSerializer.new(theme.reload).as_json[:theme]
       expect(serialized[:errors].count).to eq(1)
       expect(serialized[:errors][0]).to eq(error)
     end
   end
+
+  describe "screenshot_url" do
+    fab!(:theme)
+    let(:serialized) { ThemeSerializer.new(theme.reload).as_json[:theme] }
+
+    it "should include screenshot_url when there is a theme field with screenshot upload type" do
+      Fabricate(
+        :theme_field,
+        theme: theme,
+        type_id: ThemeField.types[:theme_screenshot_upload_var],
+        name: "theme_screenshot_1",
+        upload: Fabricate(:upload),
+      )
+      expect(serialized[:screenshot_url]).to be_present
+    end
+
+    it "should not include screenshot_url when there is no theme field with screenshot upload type" do
+      expect(serialized[:screenshot_url]).to be_nil
+    end
+
+    it "should handle multiple screenshot fields and use the first one" do
+      first_upload = Fabricate(:upload)
+      second_upload = Fabricate(:upload)
+      Fabricate(
+        :theme_field,
+        theme: theme,
+        type_id: ThemeField.types[:theme_screenshot_upload_var],
+        name: "theme_screenshot_1",
+        upload: first_upload,
+      )
+      Fabricate(
+        :theme_field,
+        theme: theme,
+        type_id: ThemeField.types[:theme_screenshot_upload_var],
+        name: "theme_screenshot_2",
+        upload: second_upload,
+      )
+
+      expect(serialized[:screenshot_url]).to eq(first_upload.url)
+    end
+  end
 end