DEV: Allow theme CLI to specify which theme to synchronize (#6963)

DEV: Allow theme CLI to specify which theme to synchronize (#6963)

Currently the theme is matched by name, which can be fragile when there are many themes with the same name. This functionality will be used by the next version of theme CLI.

diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
index 15027da..f927eee 100644
--- a/app/controllers/admin/themes_controller.rb
+++ b/app/controllers/admin/themes_controller.rb
@@ -84,11 +84,13 @@ class Admin::ThemesController < Admin::AdminController
       rescue RemoteTheme::ImportError => e
         render_json_error e.message
       end
-    elsif params[:bundle] || params[:theme] && ["application/x-gzip", "application/gzip"].include?(params[:theme].content_type)
+    elsif params[:bundle] || (params[:theme] && ["application/x-gzip", "application/gzip"].include?(params[:theme].content_type))
       # params[:bundle] used by theme CLI. params[:theme] used by admin UI
       bundle = params[:bundle] || params[:theme]
+      theme_id = params[:theme_id]
+      match_theme_by_name = !!params[:bundle] && !params.key?(:theme_id) # Old theme CLI behavior, match by name. Remove Jan 2020
       begin
-        @theme = RemoteTheme.update_tgz_theme(bundle.path, match_theme: !!params[:bundle], user: current_user)
+        @theme = RemoteTheme.update_tgz_theme(bundle.path, match_theme: match_theme_by_name, user: current_user, theme_id: theme_id)
         log_theme_change(nil, @theme)
         render json: @theme, status: :created
       rescue RemoteTheme::ImportError => e
diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb
index 5e74f32..9e8f3ef 100644
--- a/app/models/remote_theme.rb
+++ b/app/models/remote_theme.rb
@@ -32,12 +32,13 @@ class RemoteTheme < ActiveRecord::Base
     raise ImportError.new I18n.t("themes.import_error.about_json")
   end
 
-  def self.update_tgz_theme(filename, match_theme: false, user: Discourse.system_user)
+  def self.update_tgz_theme(filename, match_theme: false, user: Discourse.system_user, theme_id: nil)
     importer = ThemeStore::TgzImporter.new(filename)
     importer.import!
 
     theme_info = RemoteTheme.extract_theme_info(importer)
-    theme = Theme.find_by(name: theme_info["name"]) if match_theme
+    theme = Theme.find_by(name: theme_info["name"]) if match_theme # Old theme CLI method, remove Jan 2020
+    theme = Theme.find_by(id: theme_id) if theme_id # New theme CLI method
     theme ||= Theme.new(user_id: user&.id || -1, name: theme_info["name"])
 
     theme.component = theme_info["component"].to_s == "true"
diff --git a/spec/requests/admin/themes_controller_spec.rb b/spec/requests/admin/themes_controller_spec.rb
index 55204bf..dc25e12 100644
--- a/spec/requests/admin/themes_controller_spec.rb
+++ b/spec/requests/admin/themes_controller_spec.rb
@@ -113,7 +113,8 @@ describe Admin::ThemesController do
       expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
     end
 
-    it 'updates an existing theme from an archive' do
+    it 'updates an existing theme from an archive by name' do
+      # Old theme CLI method, remove Jan 2020
       existing_theme = Fabricate(:theme, name: "Header Icons")
 
       expect do
@@ -126,6 +127,39 @@ describe Admin::ThemesController do
       expect(json["theme"]["theme_fields"].length).to eq(5)
       expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
     end
+
+    it 'updates an existing theme from an archive by id' do
+      # Used by theme CLI
+      existing_theme = Fabricate(:theme, name: "Header Icons")
+      other_existing_theme = Fabricate(:theme, name: "Some other name")
+
+      expect do
+        post "/admin/themes/import.json", params: { bundle: theme_archive, theme_id: other_existing_theme.id }
+      end.to change { Theme.count }.by (0)
+      expect(response.status).to eq(201)
+      json = ::JSON.parse(response.body)
+
+      expect(json["theme"]["name"]).to eq("Some other name")
+      expect(json["theme"]["id"]).to eq(other_existing_theme.id)
+      expect(json["theme"]["theme_fields"].length).to eq(5)
+      expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
+    end
+
+    it 'creates a new theme when id specified as nil' do
+      # Used by theme CLI
+      existing_theme = Fabricate(:theme, name: "Header Icons")
+
+      expect do
+        post "/admin/themes/import.json", params: { bundle: theme_archive, theme_id: nil }
+      end.to change { Theme.count }.by (1)
+      expect(response.status).to eq(201)
+      json = ::JSON.parse(response.body)
+
+      expect(json["theme"]["name"]).to eq("Header Icons")
+      expect(json["theme"]["id"]).not_to eq(existing_theme.id)
+      expect(json["theme"]["theme_fields"].length).to eq(5)
+      expect(UserHistory.where(action: UserHistory.actions[:change_theme]).count).to eq(1)
+    end
   end
 
   describe '#index' do

GitHub sha: d8bd3c32

1 Like