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.

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

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"

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