From 86904e9cd6c7fbfb582bb61fa281b89d4c420b0e Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Mon, 17 Apr 2017 16:55:53 -0400
Subject: [PATCH] FIX: better error handling for theme import

---
 .../admin-customize-themes-show.js.es6          | 17 +++++++++++------
 .../admin/templates/customize-themes-show.hbs   |  2 +-
 app/controllers/admin/themes_controller.rb      | 13 ++++++++++---
 app/models/remote_theme.rb                      |  2 +-
 config/locales/server.en.yml                    |  3 +++
 5 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6 b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6
index e94bde4f8d2..3b2c2d9683e 100644
--- a/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6
+++ b/app/assets/javascripts/admin/controllers/admin-customize-themes-show.js.es6
@@ -1,5 +1,6 @@
 import { default as computed } from 'ember-addons/ember-computed-decorators';
 import { url } from 'discourse/lib/computed';
+import { popupAjaxError } from 'discourse/lib/ajax-error';
 
 export default Ember.Controller.extend({
 
@@ -79,16 +80,20 @@ export default Ember.Controller.extend({
 
     updateToLatest() {
       this.set("updatingRemote", true);
-      this.get("model").updateToLatest().finally(()=>{
-        this.set("updatingRemote", false);
-      });
+      this.get("model").updateToLatest()
+        .catch(popupAjaxError)
+        .finally(()=>{
+          this.set("updatingRemote", false);
+        });
     },
 
     checkForThemeUpdates() {
       this.set("updatingRemote", true);
-      this.get("model").checkForUpdates().finally(()=>{
-        this.set("updatingRemote", false);
-      });
+      this.get("model").checkForUpdates()
+        .catch(popupAjaxError)
+        .finally(()=>{
+          this.set("updatingRemote", false);
+        });
     },
 
     cancelChangeScheme() {
diff --git a/app/assets/javascripts/admin/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/templates/customize-themes-show.hbs
index e23a231b85f..0bc970e2534 100644
--- a/app/assets/javascripts/admin/templates/customize-themes-show.hbs
+++ b/app/assets/javascripts/admin/templates/customize-themes-show.hbs
@@ -65,7 +65,7 @@
   <p>
     {{#if model.remote_theme}}
       {{#if model.remote_theme.commits_behind}}
-      {{#d-button action="updateToLatest" icon="download"}}{{i18n "admin.customize.theme.update_to_latest"}}{{/d-button}}
+      {{#d-button action="updateToLatest" icon="download" class='btn-primary'}}{{i18n "admin.customize.theme.update_to_latest"}}{{/d-button}}
       {{else}}
       {{#d-button action="checkForThemeUpdates" icon="refresh"}}{{i18n "admin.customize.theme.check_for_updates"}}{{/d-button}}
       {{/if}}
diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
index 63f160f1ba3..700df88b654 100644
--- a/app/controllers/admin/themes_controller.rb
+++ b/app/controllers/admin/themes_controller.rb
@@ -101,25 +101,32 @@ class Admin::ThemesController < Admin::AdminController
 
     set_fields
 
+    save_remote = false
     if params[:theme][:remote_check]
       @theme.remote_theme.update_remote_version
-      @theme.remote_theme.save!
+      save_remote = true
     end
 
     if params[:theme][:remote_update]
       @theme.remote_theme.update_from_remote
-      @theme.remote_theme.save!
+      save_remote = true
     end
 
     respond_to do |format|
       if @theme.save
 
+        @theme.remote_theme.save! if save_remote
+
         update_default_theme
 
         log_theme_change(original_json, @theme)
         format.json { render json: @theme, status: :created}
       else
-        format.json { render json: @theme.errors, status: :unprocessable_entity }
+        format.json {
+
+          error = @theme.errors[:color_scheme] ? I18n.t("themes.bad_color_scheme") : I18n.t("themes.other_error")
+          render json: {errors: [ error ]}, status: :unprocessable_entity
+        }
       end
     end
   end
diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb
index a22192a0e4a..da46d9efd11 100644
--- a/app/models/remote_theme.rb
+++ b/app/models/remote_theme.rb
@@ -82,7 +82,7 @@ class RemoteTheme < ActiveRecord::Base
     return unless hex
 
     override = hex.downcase
-    if override !~ /[0-9a-f]{6}/
+    if override !~ /^[0-9a-f]{6}$/
       override = nil
     end
     override
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 1859a7239d3..940db10b9f9 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -55,6 +55,9 @@ en:
   anonymous: "Anonymous"
   remove_posts_deleted_by_author: "Deleted by author"
 
+  themes:
+    bad_color_scheme: "Can not update theme, invalid color scheme"
+    other_error: "Something went wrong updating theme"
   emails:
     incoming:
       default_subject: "This topic needs a title"