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 47b59317f1a..8ee6b5d46c3 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,7 +1,4 @@
-import {
- default as computed,
- observes
-} from "ember-addons/ember-computed-decorators";
+import { default as computed } from "ember-addons/ember-computed-decorators";
import { url } from "discourse/lib/computed";
import { popupAjaxError } from "discourse/lib/ajax-error";
import showModal from "discourse/lib/show-modal";
@@ -27,7 +24,7 @@ export default Ember.Controller.extend({
},
@computed("model.editedFields")
- editedFieldsFormatted(fields) {
+ editedFieldsFormatted() {
const descriptions = [];
["common", "desktop", "mobile"].forEach(target => {
const fields = this.editedFieldsForTarget(target);
@@ -96,6 +93,12 @@ export default Ember.Controller.extend({
hasSettings(settings) {
return settings.length > 0;
},
+
+ @computed("model.remoteError", "updatingRemote")
+ showRemoteError(errorMessage, updating) {
+ return errorMessage && !updating;
+ },
+
editedFieldsForTarget(target) {
return this.get("model.editedFields").filter(
field => field.target === target
diff --git a/app/assets/javascripts/admin/models/theme.js.es6 b/app/assets/javascripts/admin/models/theme.js.es6
index 413d9a93c61..da881abf9bc 100644
--- a/app/assets/javascripts/admin/models/theme.js.es6
+++ b/app/assets/javascripts/admin/models/theme.js.es6
@@ -54,6 +54,13 @@ const Theme = RestModel.extend({
);
},
+ @computed("remote_theme.last_error_text")
+ remoteError(errorText) {
+ if (errorText && errorText.length > 0) {
+ return errorText;
+ }
+ },
+
getKey(field) {
return `${field.target} ${field.name}`;
},
diff --git a/app/assets/javascripts/admin/routes/admin-customize-themes-index.js.es6 b/app/assets/javascripts/admin/routes/admin-customize-themes-index.js.es6
index e29ef8659e7..d74a4778009 100644
--- a/app/assets/javascripts/admin/routes/admin-customize-themes-index.js.es6
+++ b/app/assets/javascripts/admin/routes/admin-customize-themes-index.js.es6
@@ -17,7 +17,7 @@ const externalResources = [
];
export default Ember.Route.extend({
- setupController(controller, model) {
+ setupController(controller) {
this._super(...arguments);
this.controllerFor("adminCustomizeThemes").set("editingTheme", false);
controller.set("externalResources", externalResources);
diff --git a/app/assets/javascripts/admin/templates/customize-themes-show.hbs b/app/assets/javascripts/admin/templates/customize-themes-show.hbs
index 9cb796793a5..fcddcfa165c 100644
--- a/app/assets/javascripts/admin/templates/customize-themes-show.hbs
+++ b/app/assets/javascripts/admin/templates/customize-themes-show.hbs
@@ -87,10 +87,20 @@
{{/if}}
{{else}}
- {{i18n 'admin.customize.theme.up_to_date'}} {{format-date model.remote_theme.updated_at leaveAgo="true"}}
+ {{#unless showRemoteError}}
+ {{i18n 'admin.customize.theme.up_to_date'}} {{format-date model.remote_theme.updated_at leaveAgo="true"}}
+ {{/unless}}
{{/if}}
{{/if}}
+ {{#if showRemoteError}}
+
+ {{d-icon "exclamation-triangle"}} {{I18n "admin.customize.theme.repo_unreachable"}}
+
+
+ {{model.remoteError}}
+
+ {{/if}}
{{/if}}
diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss
index beefd8de9af..41bee9e411d 100644
--- a/app/assets/stylesheets/common/admin/customize.scss
+++ b/app/assets/stylesheets/common/admin/customize.scss
@@ -88,6 +88,17 @@
.admin-container {
padding: 0;
}
+ .error-message {
+ margin-top: 5px;
+ margin-bottom: 5px;
+ .fa {
+ color: $danger;
+ }
+ }
+ .raw-error {
+ background-color: $primary-very-low;
+ padding: 5px;
+ }
}
.admin-customize {
diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb
index fffdd0e0360..8295934a729 100644
--- a/app/models/admin_dashboard_data.rb
+++ b/app/models/admin_dashboard_data.rb
@@ -102,7 +102,7 @@ class AdminDashboardData
:failing_emails_check,
:subfolder_ends_in_slash_check,
:pop3_polling_configuration, :email_polling_errored_recently,
- :out_of_date_themes
+ :out_of_date_themes, :unreachable_themes
add_problem_check do
sidekiq_check || queue_size_check
@@ -252,11 +252,24 @@ class AdminDashboardData
old_themes = RemoteTheme.out_of_date_themes
return unless old_themes.present?
- html = old_themes.map do |name, id|
+ themes_html_format(old_themes, "dashboard.out_of_date_themes")
+ end
+
+ def unreachable_themes
+ themes = RemoteTheme.unreachable_themes
+ return unless themes.present?
+
+ themes_html_format(themes, "dashboard.unreachable_themes")
+ end
+
+ private
+
+ def themes_html_format(themes, i18n_key)
+ html = themes.map do |name, id|
"#{CGI.escapeHTML(name)}"
end.join("\n")
- message = I18n.t("dashboard.out_of_date_themes")
+ message = I18n.t(i18n_key)
message += ""
message
end
diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb
index 32e53fbec17..cdf1080df95 100644
--- a/app/models/remote_theme.rb
+++ b/app/models/remote_theme.rb
@@ -10,6 +10,9 @@ class RemoteTheme < ActiveRecord::Base
GITHUB_SSH_REGEXP = /^git@github\.com:/
has_one :theme
+ scope :joined_remotes, -> {
+ joins("JOIN themes ON themes.remote_theme_id = remote_themes.id").where.not(remote_url: "")
+ }
def self.update_tgz_theme(filename, user: Discourse.system_user)
importer = ThemeStore::TgzImporter.new(filename)
@@ -61,17 +64,24 @@ class RemoteTheme < ActiveRecord::Base
end
def self.out_of_date_themes
- self.joins("JOIN themes ON themes.remote_theme_id = remote_themes.id")
- .where.not(remote_url: "")
- .where("commits_behind > 0 OR remote_version <> local_version")
+ self.joined_remotes.where("commits_behind > 0 OR remote_version <> local_version")
.pluck("themes.name", "themes.id")
end
+ def self.unreachable_themes
+ self.joined_remotes.where("last_error_text IS NOT NULL").pluck("themes.name", "themes.id")
+ end
+
def update_remote_version
importer = ThemeStore::GitImporter.new(remote_url, private_key: private_key)
- importer.import!
- self.updated_at = Time.zone.now
- self.remote_version, self.commits_behind = importer.commits_since(local_version)
+ begin
+ importer.import!
+ rescue ThemeStore::GitImporter::ImportFailed => err
+ self.last_error_text = err.message
+ else
+ self.updated_at = Time.zone.now
+ self.remote_version, self.commits_behind = importer.commits_since(local_version)
+ end
end
def update_from_remote(importer = nil, skip_update: false)
@@ -81,7 +91,12 @@ class RemoteTheme < ActiveRecord::Base
unless importer
cleanup = true
importer = ThemeStore::GitImporter.new(remote_url, private_key: private_key)
- importer.import!
+ begin
+ importer.import!
+ rescue ThemeStore::GitImporter::ImportFailed => err
+ self.last_error_text = err.message
+ return self
+ end
end
theme_info = JSON.parse(importer["about.json"])
@@ -225,4 +240,5 @@ end
# created_at :datetime not null
# updated_at :datetime not null
# private_key :text
+# last_error_text :text
#
diff --git a/app/serializers/theme_serializer.rb b/app/serializers/theme_serializer.rb
index d8e6f5e6626..0e8291cea77 100644
--- a/app/serializers/theme_serializer.rb
+++ b/app/serializers/theme_serializer.rb
@@ -47,7 +47,7 @@ end
class RemoteThemeSerializer < ApplicationSerializer
attributes :id, :remote_url, :remote_version, :local_version, :about_url,
:license_url, :commits_behind, :remote_updated_at, :updated_at,
- :github_diff_link
+ :github_diff_link, :last_error_text
# wow, AMS has some pretty nutty logic where it tries to find the path here
# from action dispatch, tell it not to
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index c9508a605be..808cc1cc9a4 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -3287,6 +3287,7 @@ en:
one: "Theme is 1 commit behind!"
other: "Theme is {{count}} commits behind!"
compare_commits: "(See new commits)"
+ repo_unreachable: "Couldn't contact the Git repository of this theme. Error message:"
scss:
text: "CSS"
title: "Enter custom CSS, we accept all valid CSS and SCSS styles"
diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml
index 761176b97f1..a7cdca724a1 100644
--- a/config/locales/server.en.yml
+++ b/config/locales/server.en.yml
@@ -1118,6 +1118,7 @@ en:
poll_pop3_auth_error: "Connection to the POP3 server is failing with an authentication error. Please check your POP3 settings."
force_https_warning: "Your website is using SSL. But `force_https` is not yet enabled in your site settings."
out_of_date_themes: "Updates are available for the following themes:"
+ unreachable_themes: "We were unable to check for updates on the following themes:"
site_settings:
censored_words: "Words that will be automatically replaced with ■■■■"
diff --git a/db/migrate/20180907075713_add_last_error_text_to_remote_themes.rb b/db/migrate/20180907075713_add_last_error_text_to_remote_themes.rb
new file mode 100644
index 00000000000..b03498eede9
--- /dev/null
+++ b/db/migrate/20180907075713_add_last_error_text_to_remote_themes.rb
@@ -0,0 +1,5 @@
+class AddLastErrorTextToRemoteThemes < ActiveRecord::Migration[5.2]
+ def change
+ add_column :remote_themes, :last_error_text, :text
+ end
+end
diff --git a/lib/theme_store/git_importer.rb b/lib/theme_store/git_importer.rb
index 761aebbf3f8..9238c4228b7 100644
--- a/lib/theme_store/git_importer.rb
+++ b/lib/theme_store/git_importer.rb
@@ -2,6 +2,7 @@ module ThemeStore; end
class ThemeStore::GitImporter
+ class ImportFailed < StandardError; end
attr_reader :url
def initialize(url, private_key: nil)
@@ -65,7 +66,11 @@ class ThemeStore::GitImporter
protected
def import_public!
- Discourse::Utils.execute_command("git", "clone", @url, @temp_folder)
+ begin
+ Discourse::Utils.execute_command("git", "clone", @url, @temp_folder)
+ rescue => err
+ raise ImportFailed.new(err.message)
+ end
end
def import_private!
@@ -77,9 +82,13 @@ class ThemeStore::GitImporter
FileUtils.chmod(0600, 'id_rsa')
end
- Discourse::Utils.execute_command({
- 'GIT_SSH_COMMAND' => "ssh -i #{ssh_folder}/id_rsa -o StrictHostKeyChecking=no"
- }, "git", "clone", @url, @temp_folder)
+ begin
+ Discourse::Utils.execute_command({
+ 'GIT_SSH_COMMAND' => "ssh -i #{ssh_folder}/id_rsa -o StrictHostKeyChecking=no"
+ }, "git", "clone", @url, @temp_folder)
+ rescue => err
+ raise ImportFailed.new(err.message)
+ end
ensure
FileUtils.rm_rf ssh_folder
end
diff --git a/spec/models/admin_dashboard_data_spec.rb b/spec/models/admin_dashboard_data_spec.rb
index 2f4db5fa470..6d81f623dd5 100644
--- a/spec/models/admin_dashboard_data_spec.rb
+++ b/spec/models/admin_dashboard_data_spec.rb
@@ -351,4 +351,19 @@ describe AdminDashboardData do
expect(dashboard_data.out_of_date_themes).to eq(nil)
end
end
+
+ describe '#unreachable_themes' do
+ let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/testtheme", last_error_text: "can't reach repo :'(") }
+ let!(:theme) { Fabricate(:theme, remote_theme: remote, name: "Test< Theme") }
+
+ it "outputs correctly formatted html" do
+ dashboard_data = described_class.new
+ expect(dashboard_data.unreachable_themes).to eq(
+ I18n.t("dashboard.unreachable_themes") + ""
+ )
+
+ remote.update!(last_error_text: nil)
+ expect(dashboard_data.out_of_date_themes).to eq(nil)
+ end
+ end
end
diff --git a/spec/models/remote_theme_spec.rb b/spec/models/remote_theme_spec.rb
index ab5eb010a97..95950f19c59 100644
--- a/spec/models/remote_theme_spec.rb
+++ b/spec/models/remote_theme_spec.rb
@@ -187,6 +187,20 @@ describe RemoteTheme do
end
end
+ context ".joined_remotes" do
+ it "finds records that are associated with themes" do
+ github_repo
+ gitlab_repo
+ expect(RemoteTheme.joined_remotes).to eq([])
+
+ Fabricate(:theme, remote_theme: github_repo)
+ expect(RemoteTheme.joined_remotes).to eq([github_repo])
+
+ Fabricate(:theme, remote_theme: gitlab_repo)
+ expect(RemoteTheme.joined_remotes).to contain_exactly(github_repo, gitlab_repo)
+ end
+ end
+
context ".out_of_date_themes" do
let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/testtheme") }
let!(:theme) { Fabricate(:theme, remote_theme: remote) }
@@ -199,4 +213,16 @@ describe RemoteTheme do
expect(described_class.out_of_date_themes).to eq([])
end
end
+
+ context ".unreachable_themes" do
+ let(:remote) { RemoteTheme.create!(remote_url: "https://github.com/org/testtheme", last_error_text: "can't contact this repo :(") }
+ let!(:theme) { Fabricate(:theme, remote_theme: remote) }
+
+ it "finds out of date themes" do
+ expect(described_class.unreachable_themes).to eq([[theme.name, theme.id]])
+
+ remote.update!(last_error_text: nil)
+ expect(described_class.unreachable_themes).to eq([])
+ end
+ end
end