From 60bc38e6a8914a10341a32ff9909e69faa65ffef Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 23 Nov 2020 15:29:22 +0200 Subject: [PATCH] FIX: Gracefully handle force pushes for remote themes (#11325) Force pushing a commit to a theme repository used to break the updater, because the system was not able to count the commits behind the old and new version. This operation failed because a force push deleted the old commits. The user was prompted with a simple "500 server error" message. --- .../admin-customize-themes-show.js | 9 ++++++++ .../addon/templates/customize-themes-show.hbs | 6 ++++- config/locales/client.en.yml | 1 + lib/theme_store/git_importer.rb | 2 +- spec/models/remote_theme_spec.rb | 23 +++++++++++++++++++ 5 files changed, 39 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js index 770f541f6e..9e9afda71d 100644 --- a/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js +++ b/app/assets/javascripts/admin/addon/controllers/admin-customize-themes-show.js @@ -151,6 +151,15 @@ export default Controller.extend({ hasTranslations: notEmpty("translations"), + @discourseComputed( + "model.remote_theme.local_version", + "model.remote_theme.remote_version", + "model.remote_theme.commits_behind" + ) + hasOverwrittenHistory(localVersion, remoteVersion, commitsBehind) { + return localVersion !== remoteVersion && commitsBehind === -1; + }, + @discourseComputed("model.remoteError", "updatingRemote") showRemoteError(errorMessage, updating) { return errorMessage && !updating; diff --git a/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs index dba011faac..fcb9bd2acd 100644 --- a/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs +++ b/app/assets/javascripts/admin/addon/templates/customize-themes-show.hbs @@ -105,7 +105,11 @@ {{i18n "admin.customize.theme.updating"}} {{else}} {{#if model.remote_theme.commits_behind}} - {{i18n "admin.customize.theme.commits_behind" count=model.remote_theme.commits_behind}} + {{#if hasOverwrittenHistory}} + {{i18n "admin.customize.theme.has_overwritten_history"}} + {{else}} + {{i18n "admin.customize.theme.commits_behind" count=model.remote_theme.commits_behind}} + {{/if}} {{#if model.remote_theme.github_diff_link}} {{i18n "admin.customize.theme.compare_commits"}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b12f6774ba..de1483da0b 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -4073,6 +4073,7 @@ en: check_for_updates: "Check for Updates" updating: "Updating..." up_to_date: "Theme is up-to-date, last checked:" + has_overwritten_history: "Current theme version no longer exists because the Git history has been overwritten by a force push." add: "Add" theme_settings: "Theme Settings" no_settings: "This theme has no settings." diff --git a/lib/theme_store/git_importer.rb b/lib/theme_store/git_importer.rb index b58724277d..37fb49e6cd 100644 --- a/lib/theme_store/git_importer.rb +++ b/lib/theme_store/git_importer.rb @@ -35,7 +35,7 @@ class ThemeStore::GitImporter Discourse::Utils.execute_command(chdir: @temp_folder) do |runner| commit_hash = runner.exec("git", "rev-parse", "HEAD").strip - commits_behind = runner.exec("git", "rev-list", "#{hash}..HEAD", "--count").strip + commits_behind = runner.exec("git", "rev-list", "#{hash}..HEAD", "--count").strip rescue -1 end [commit_hash, commits_behind] diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb index 6e63427842..9bb82c9d5c 100644 --- a/spec/models/remote_theme_spec.rb +++ b/spec/models/remote_theme_spec.rb @@ -158,6 +158,29 @@ describe RemoteTheme do scheme = ColorScheme.find_by(theme_id: @theme.id) expect(scheme.colors.find_by(name: 'tertiary_low_color')).to eq(nil) end + + it "can update themes with overwritten history" do + theme = RemoteTheme.import_theme(initial_repo) + remote = theme.remote_theme + + old_version = `cd #{initial_repo} && git rev-parse HEAD`.strip + expect(theme.name).to eq('awesome theme') + expect(remote.remote_url).to eq(initial_repo) + expect(remote.local_version).to eq(old_version) + expect(remote.remote_version).to eq(old_version) + + `cd #{initial_repo} && git commit --amend -m "amended commit"` + new_version = `cd #{initial_repo} && git rev-parse HEAD`.strip + + # make sure that the amended commit does not exist anymore + `cd #{initial_repo} && git reflog expire --all --expire=now` + `cd #{initial_repo} && git prune` + + remote.update_remote_version + expect(remote.reload.local_version).to eq(old_version) + expect(remote.reload.remote_version).to eq(new_version) + expect(remote.reload.commits_behind).to eq(-1) + end end let(:github_repo) do