From d8bd3c32ca43407499dea414c4f17e5c37d6d71d Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Wed, 30 Jan 2019 14:17:04 +0000
Subject: [PATCH] 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    |  6 ++--
 app/models/remote_theme.rb                    |  5 +--
 spec/requests/admin/themes_controller_spec.rb | 36 ++++++++++++++++++-
 3 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
index 15027da7ff4..f927eee3374 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 5e74f325df9..9e8f3ef7cd7 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 55204bf3705..dc25e125138 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