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

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,

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;

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

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
 

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"

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

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
 

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

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
       expect(@theme.theme_fields.length).to eq(7)
 
       mapped = Hash[*@theme.theme_fields.map { |f| ["#{f.target_id}-#{f.name}", f.value] }.flatten]
-
       expect(mapped["0-header"]).to eq("I AM HEADER")
       expect(mapped["1-scss"]).to eq(scss_data)
       expect(mapped["0-embedded_scss"]).to eq("EMBED")
@@ -117,7 +116,6 @@ describe RemoteTheme do
 
       File.delete("#{initial_repo}/settings.yaml")
       File.delete("#{initial_repo}/scss/file.scss")
-
       `cd #{initial_repo} && git commit -am "update"`
 
       time = Time.new('2001')
@@ -160,6 +158,13 @@ describe RemoteTheme do
 
       scheme_count = ColorScheme.where(theme_id: @theme.id).count
       expect(scheme_count).to eq(1)
+
+      # It should detect local changes
+      @theme.set_field(target: :common, name: :scss, value: 'body {background-color: blue};')
+      @theme.save
+      @theme.reload
+
+      expect(remote.diff_local_changes[:diff]).to include("background-color: blue")
     end
   end
 

spec/requests/admin/themes_controller_spec.rb

@@ -378,4 +378,13 @@ describe Admin::ThemesController do
       expect(response.status).to eq(400)
     end
   end
+
+  describe '#diff_local_changes' do
+    let(:theme) { Fabricate(:theme) }
+
+    it "should return empty for a default theme" do
+      get "/admin/themes/#{theme.id}/diff_local_changes.json"
+      expect(response.body).to eq("{}")
+    end
+  end
 end