FEATURE: Show diff of local changes before updating remote theme (#7443)

FEATURE: Show diff of local changes before updating remote theme (#7443)

diff --git a/app/assets/javascripts/admin/models/theme.js.es6 b/app/assets/javascripts/admin/models/theme.js.es6
index 0e2ff37..1de1a6a 100644
--- a/app/assets/javascripts/admin/models/theme.js.es6
+++ b/app/assets/javascripts/admin/models/theme.js.es6
@@ -1,6 +1,9 @@
 import RestModel from "discourse/models/rest";
 import { default as computed } from "ember-addons/ember-computed-decorators";
 import { popupAjaxError } from "discourse/lib/ajax-error";
+import { ajax } from "discourse/lib/ajax";
+import { escapeExpression } from "discourse/lib/utilities";
+import highlightSyntax from "discourse/lib/highlight-syntax";
 
 const THEME_UPLOAD_VAR = 2;
 
@@ -277,9 +280,36 @@ const Theme = RestModel.extend({
   },
 
   updateToLatest() {
-    return this.save({ remote_update: true }).then(() =>
-      this.set("changed", false)
-    );
+    return ajax(`/admin/themes/${this.id}/diff_local_changes`).then(json => {
+      if (json && json.error) {
+        bootbox.alert(
+          I18n.t("generic_error_with_reason", {
+            error: json.error
+          })
+        );
+      } else if (json && json.diff) {
+        bootbox.confirm(
+          I18n.t("admin.customize.theme.update_confirm") +
+            `<pre><code class="diff">${escapeExpression(
+              json.diff
+            )}</code></pre>`,
+          I18n.t("cancel"),
+          I18n.t("admin.customize.theme.update_confirm_yes"),
+          result => {
+            if (result) {
+              return this.save({ remote_update: true }).then(() =>
+                this.set("changed", false)
+              );
+            }
+          }
+        );
+        highlightSyntax();
+      } else {
+        return this.save({ remote_update: true }).then(() =>
+          this.set("changed", false)
+        );
+      }
+    });
   },
 
   changed: false,
diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss
index b200c55..fbe69ed 100644
--- a/app/assets/stylesheets/common/base/modal.scss
+++ b/app/assets/stylesheets/common/base/modal.scss
@@ -265,6 +265,15 @@
         }
       }
     }
+
+    pre {
+      background-color: blend-primary-secondary(5%);
+      max-height: 300px;
+      padding: 0.5em;
+      code {
+        max-height: none;
+      }
+    }
   }
   .password-confirmation {
     display: none;
diff --git a/app/controllers/admin/themes_controller.rb b/app/controllers/admin/themes_controller.rb
index 84ad22f..9e4d78d 100644
--- a/app/controllers/admin/themes_controller.rb
+++ b/app/controllers/admin/themes_controller.rb
@@ -250,6 +250,15 @@ class Admin::ThemesController < Admin::AdminController
     exporter.cleanup!
   end
 
+  def diff_local_changes
+    theme = Theme.find_by(id: params[:id])
+    raise Discourse::InvalidParameters.new(:id) unless theme
+    changes = theme.remote_theme&.diff_local_changes
+    respond_to do |format|
+      format.json { render json: changes || {} }
+    end
+  end
+
   private
 
   def update_default_theme
diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb
index 8bda923..c15f71c 100644
--- a/app/models/remote_theme.rb
+++ b/app/models/remote_theme.rb
@@ -170,6 +170,21 @@ class RemoteTheme < ActiveRecord::Base
     end
   end
 
+  def diff_local_changes
+    return unless is_git?
+    importer = ThemeStore::GitImporter.new(remote_url, private_key: private_key, branch: branch)
+    begin
+      importer.import!
+    rescue RemoteTheme::ImportError => err
+      { error: err.message }
+    else
+      changes = importer.diff_local_changes(self.id)
+      return nil if changes.blank?
+
+      { diff: changes }
+    end
+  end
+
   def normalize_override(hex)
     return unless hex
 
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index e8ec3f2..4bf5f19 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3424,6 +3424,8 @@ en:
           long_title: "Amend colors, CSS and HTML contents of your site"
           edit: "Edit"
           edit_confirm: "This is a remote theme, if you edit CSS/HTML your changes will be erased next time you update the theme."
+          update_confirm: "These local changes will be erased by the update. Are you sure you want to continue?"
+          update_confirm_yes: "Yes, continue with the update"
           common: "Common"
           desktop: "Desktop"
           mobile: "Mobile"
diff --git a/config/routes.rb b/config/routes.rb
index e30d782..33cb6ea 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -206,6 +206,7 @@ Discourse::Application.routes.draw do
     post "themes/upload_asset" => "themes#upload_asset"
     post "themes/generate_key_pair" => "themes#generate_key_pair"
     get "themes/:id/preview" => "themes#preview"
+    get "themes/:id/diff_local_changes" => "themes#diff_local_changes"
 
     scope "/customize", constraints: AdminConstraint.new do
       resources :user_fields, constraints: AdminConstraint.new
diff --git a/lib/theme_store/git_importer.rb b/lib/theme_store/git_importer.rb
index f622f70..5b7a026 100644
--- a/lib/theme_store/git_importer.rb
+++ b/lib/theme_store/git_importer.rb
@@ -1,3 +1,5 @@
+require_dependency 'theme_store/tgz_exporter'
+
 module ThemeStore; end
 
 class ThemeStore::GitImporter
@@ -22,6 +24,27 @@ class ThemeStore::GitImporter
     end
   end
 
+  def diff_local_changes(remote_theme_id)
+    theme = Theme.find_by(remote_theme_id: remote_theme_id)
+    raise Discourse::InvalidParameters.new(:id) unless theme
+    local_version = theme.remote_theme&.local_version
+
+    exporter = ThemeStore::TgzExporter.new(theme)
+    local_temp_folder = exporter.export_to_folder
+
+    Dir.chdir(@temp_folder) do
+      Discourse::Utils.execute_command("git", "checkout", local_version)
+      Discourse::Utils.execute_command("rm -rf ./*/")
+      Discourse::Utils.execute_command("cp", "-rf", "#{local_temp_folder}/#{exporter.export_name}/", @temp_folder)
+      Discourse::Utils.execute_command("git", "checkout", "about.json")
+      # adding and diffing on staged so that we catch uploads
+      Discourse::Utils.execute_command("git", "add", "-A")
+      return Discourse::Utils.execute_command("git", "diff", "--staged")
+    end
+  ensure
+    FileUtils.rm_rf local_temp_folder
+  end
+
   def commits_since(hash)
     commit_hash, commits_behind = nil
 
diff --git a/lib/theme_store/tgz_exporter.rb b/lib/theme_store/tgz_exporter.rb
index e1799e3..7359829 100644
--- a/lib/theme_store/tgz_exporter.rb
+++ b/lib/theme_store/tgz_exporter.rb
@@ -9,6 +9,10 @@ class ThemeStore::TgzExporter
     @export_name = "discourse-#{@export_name}" unless @export_name.starts_with?("discourse")
   end
 
+  def export_name
+    @export_name
+  end
+
   def package_filename
     export_package
   end
@@ -17,7 +21,6 @@ class ThemeStore::TgzExporter
     FileUtils.rm_rf(@temp_folder)
   end
 
-  private
   def export_to_folder
     FileUtils.mkdir(@temp_folder)
 
@@ -50,6 +53,7 @@ class ThemeStore::TgzExporter
     @temp_folder
   end
 
+  private
   def export_package
     export_to_folder
     Dir.chdir(@temp_folder) do
diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb
index 018fc6f6..efdf798 100644
--- a/spec/models/remote_theme_spec.rb
+++ b/spec/models/remote_theme_spec.rb
@@ -29,7 +29,7 @@ describe RemoteTheme do
           "theme_version": "1.0",
           "minimum_discourse_version": "1.0.0",
           "assets": {
-            "font": "assets/awesome.woff2"
+            "font": "assets/font.woff2"
           },
           "color_schemes": {
             "#{color_scheme_name}": {
@@ -53,7 +53,7 @@ describe RemoteTheme do
         "common/header.html" => "I AM HEADER",
         "common/random.html" => "I AM SILLY",
         "common/embedded.scss" => "EMBED",
-        "assets/awesome.woff2" => "FAKE FONT",
+        "assets/font.woff2" => "FAKE FONT",
         "settings.yaml" => "boolean_setting: true",
         "locales/en.yml" => "sometranslations"
       )
@@ -84,7 +84,6 @@ describe RemoteTheme do

[... diff too long, it was truncated ...]

GitHub sha: b948d97c

1 Like

I noticed a very minor thing on dev… if you have a real old install it appears to pick up every single file I had on local as a change and pop up the dialog.

Not a huge deal as long as this mistake only happens once, I recall @davidtaylorhq changed it so we associate more files now with imports than we used to in the past

2 Likes

Any chance you remember which theme/component triggered this on dev?