From 925c8c009a180230004ca042e22128499624bc17 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Tue, 5 Oct 2021 20:54:06 +0800 Subject: [PATCH 1/4] DEV: Add notice specs and UI updates --- .../components/subscription-container.js.es6 | 2 +- .../discourse/components/wizard-notice.js.es6 | 10 +- .../custom-wizard-important-notice.hbs | 3 + .../custom-wizard-important-notice.js.es6 | 16 +++ .../custom-wizard-issue-notice.hbs | 3 - .../custom-wizard-issue-notice.js.es6 | 12 -- .../admin-wizards-subscription.js.es6 | 2 +- .../initializers/custom-wizard-edits.js.es6 | 10 +- .../models/custom-wizard-notice.js.es6 | 4 +- .../components/subscription-container.hbs | 6 +- .../templates/components/wizard-notice.hbs | 20 ++- assets/stylesheets/admin/admin.scss | 20 ++- config/locales/client.en.yml | 6 +- config/locales/server.en.yml | 18 ++- config/settings.yml | 5 +- controllers/custom_wizard/admin/notice.rb | 4 +- .../scheduled/custom_wizard/update_notices.rb | 2 +- lib/custom_wizard/notice.rb | 132 +++++++++--------- lib/custom_wizard/notice/connection_error.rb | 82 +++++++++++ plugin.rb | 6 +- .../custom_wizard/notice_serializer.rb | 1 + spec/components/custom_wizard/notice_spec.rb | 69 +++++++-- .../sprockets/require_tree_discourse_empty.js | 2 +- 23 files changed, 306 insertions(+), 129 deletions(-) create mode 100644 assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.hbs create mode 100644 assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.js.es6 delete mode 100644 assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.hbs delete mode 100644 assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.js.es6 create mode 100644 lib/custom_wizard/notice/connection_error.rb diff --git a/assets/javascripts/discourse/components/subscription-container.js.es6 b/assets/javascripts/discourse/components/subscription-container.js.es6 index 08498f6f..620c6d88 100644 --- a/assets/javascripts/discourse/components/subscription-container.js.es6 +++ b/assets/javascripts/discourse/components/subscription-container.js.es6 @@ -18,4 +18,4 @@ export default Component.extend({ subscribedTitle(subscribed) { return `admin.wizard.subscription_container.${subscribed ? 'subscribed' : 'not_subscribed'}.title`; } -}) \ No newline at end of file +}); \ No newline at end of file diff --git a/assets/javascripts/discourse/components/wizard-notice.js.es6 b/assets/javascripts/discourse/components/wizard-notice.js.es6 index 86a82c94..15da3f35 100644 --- a/assets/javascripts/discourse/components/wizard-notice.js.es6 +++ b/assets/javascripts/discourse/components/wizard-notice.js.es6 @@ -1,10 +1,10 @@ import Component from "@ember/component"; import discourseComputed from "discourse-common/utils/decorators"; -import { notEmpty, not } from "@ember/object/computed"; +import { not, notEmpty } from "@ember/object/computed"; import I18n from "I18n"; export default Component.extend({ - classNameBindings: [':wizard-notice', 'notice.type', 'dismissed', 'expired'], + classNameBindings: [':wizard-notice', 'notice.type', 'dismissed', 'expired', 'resolved'], showFull: false, resolved: notEmpty('notice.expired_at'), dismissed: notEmpty('notice.dismissed_at'), @@ -18,14 +18,16 @@ export default Component.extend({ @discourseComputed('notice.type') icon(type) { return { - warning: 'exclamation-circle', + plugin_status_warning: 'exclamation-circle', + plugin_status_connection_error: 'bolt', + subscription_messages_connection_error: 'bolt', info: 'info-circle' }[type]; }, actions: { dismiss() { - this.set('dismissing', true) + this.set('dismissing', true); this.notice.dismiss().then(() => { this.set('dismissing', false); }); diff --git a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.hbs b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.hbs new file mode 100644 index 00000000..9b01c468 --- /dev/null +++ b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.hbs @@ -0,0 +1,3 @@ +{{#if importantNotice}} + {{wizard-notice notice=importantNotice importantOnDashboard=true}} +{{/if}} diff --git a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.js.es6 b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.js.es6 new file mode 100644 index 00000000..43a2152b --- /dev/null +++ b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-important-notice.js.es6 @@ -0,0 +1,16 @@ +import { getOwner } from "discourse-common/lib/get-owner"; + +export default { + shouldRender(attrs, ctx) { + return ctx.siteSettings.wizard_important_notices_on_dashboard; + }, + + setupComponent() { + const controller = getOwner(this).lookup('controller:admin-dashboard'); + const importantNotice = controller.get('customWizardImportantNotice'); + + if (importantNotice) { + this.set('importantNotice', importantNotice); + } + } +}; \ No newline at end of file diff --git a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.hbs b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.hbs deleted file mode 100644 index a8aad815..00000000 --- a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.hbs +++ /dev/null @@ -1,3 +0,0 @@ -{{#if wizardWarningNotice}} - {{wizard-notice notice=wizardWarningNotice}} -{{/if}} \ No newline at end of file diff --git a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.js.es6 b/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.js.es6 deleted file mode 100644 index b92e7897..00000000 --- a/assets/javascripts/discourse/connectors/admin-dashboard-top/custom-wizard-issue-notice.js.es6 +++ /dev/null @@ -1,12 +0,0 @@ -import { getOwner } from "discourse-common/lib/get-owner"; - -export default { - setupComponent() { - const controller = getOwner(this).lookup('controller:admin-dashboard') - const wizardWarningNotice = controller.get('wizardWarningNotice'); - - if (wizardWarningNotice) { - this.set('wizardWarningNotice', wizardWarningNotice); - } - } -} \ No newline at end of file diff --git a/assets/javascripts/discourse/controllers/admin-wizards-subscription.js.es6 b/assets/javascripts/discourse/controllers/admin-wizards-subscription.js.es6 index 844d5a25..76f16119 100644 --- a/assets/javascripts/discourse/controllers/admin-wizards-subscription.js.es6 +++ b/assets/javascripts/discourse/controllers/admin-wizards-subscription.js.es6 @@ -38,7 +38,7 @@ export default Controller.extend({ unauthorize() { this.set("unauthorizing", true); - CustomWizardPro.unauthorize() + CustomWizardSubscription.unauthorize() .then((result) => { if (result.success) { this.setProperties({ diff --git a/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 b/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 index 4bf50fa9..f53dc2cd 100644 --- a/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 +++ b/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 @@ -1,6 +1,5 @@ import DiscourseURL from "discourse/lib/url"; import { withPluginApi } from "discourse/lib/plugin-api"; -import { ajax } from "discourse/lib/ajax"; import CustomWizardNotice from "../models/custom-wizard-notice"; import { A } from "@ember/array"; @@ -31,12 +30,13 @@ export default { }); }, - setupController(controller, model) { + setupController(controller) { if (this.notices) { - let warningNotices = this.notices.filter(n => n.type === 'warning'); + let pluginStatusConnectionError = this.notices.filter(n => n.type === 'plugin_status_connection_error')[0]; + let pluginStatusWarning = this.notices.filter(n => n.type === 'plugin_status_warning')[0]; - if (warningNotices.length) { - controller.set('wizardWarningNotice', warningNotices[0]); + if (pluginStatusConnectionError || pluginStatusWarning) { + controller.set('customWizardImportantNotice', pluginStatusConnectionError || pluginStatusWarning); } } diff --git a/assets/javascripts/discourse/models/custom-wizard-notice.js.es6 b/assets/javascripts/discourse/models/custom-wizard-notice.js.es6 index bae81822..a6b47c40 100644 --- a/assets/javascripts/discourse/models/custom-wizard-notice.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-notice.js.es6 @@ -10,13 +10,13 @@ CustomWizardNotice.reopen({ if (result.success) { this.set('dismissed_at', result.dismissed_at); } - }).catch(popupAjaxError) + }).catch(popupAjaxError); } }); CustomWizardNotice.reopenClass({ list() { - return ajax('/admin/wizards/notice').catch(popupAjaxError) + return ajax('/admin/wizards/notice').catch(popupAjaxError); } }); diff --git a/assets/javascripts/discourse/templates/components/subscription-container.hbs b/assets/javascripts/discourse/templates/components/subscription-container.hbs index 7a02f555..8b012671 100644 --- a/assets/javascripts/discourse/templates/components/subscription-container.hbs +++ b/assets/javascripts/discourse/templates/components/subscription-container.hbs @@ -1,5 +1,5 @@ -
-

{{i18n 'admin.wizard.subscription_container.title'}}

+
+

{{i18n "admin.wizard.subscription_container.title"}}

{{d-icon subscribedIcon}} @@ -9,4 +9,4 @@
{{yield}} -
\ No newline at end of file +
diff --git a/assets/javascripts/discourse/templates/components/wizard-notice.hbs b/assets/javascripts/discourse/templates/components/wizard-notice.hbs index f6f923a3..6a7d0afb 100644 --- a/assets/javascripts/discourse/templates/components/wizard-notice.hbs +++ b/assets/javascripts/discourse/templates/components/wizard-notice.hbs @@ -6,18 +6,26 @@ {{format-date notice.expired_at leaveAgo="true"}}
{{/if}} - +
{{d-icon icon}} {{title}}
- {{d-icon "calendar-alt"}} + {{d-icon "far-clock"}} {{i18n "admin.wizard.notice.issued"}} {{format-date notice.created_at leaveAgo="true"}}
+ {{#if notice.updated_at}} +
+ {{d-icon "calendar-alt"}} + {{i18n "admin.wizard.notice.updated"}} + {{format-date notice.updated_at leaveAgo="true"}} +
+ {{/if}} +
{{d-icon "plug"}} {{i18n "admin.wizard.notice.plugin"}} @@ -28,10 +36,16 @@ {{{notice.message}}}
+{{#if importantOnDashboard}} + + {{i18n "admin.wizard.notice.disable_important_on_dashboard"}} + +{{/if}} + {{#if canDismiss}} {{#if dismissing}} {{loading-spinner size="small"}} {{else}} {{d-icon "times"}} {{/if}} -{{/if}} \ No newline at end of file +{{/if}} diff --git a/assets/stylesheets/admin/admin.scss b/assets/stylesheets/admin/admin.scss index 2eb32bf3..862f6761 100644 --- a/assets/stylesheets/admin/admin.scss +++ b/assets/stylesheets/admin/admin.scss @@ -911,13 +911,18 @@ padding: 1em; margin-bottom: 1em; border: 1px solid var(--primary); - border-radius: 4px; position: relative; &.dismissed { display: none; } + &.resolved .notice-badge:not(.notice-expired-at), + &.resolved a, + &.resolved p { + color: var(--primary-medium) !important; + } + .d-icon { margin-right: .4em; } @@ -931,7 +936,6 @@ display: inline-flex; align-items: center; padding: 0 .5em; - border-radius: 4px; margin-right: 1em; font-size: .9em; line-height: 25px; @@ -957,7 +961,8 @@ } } - .notice-issued { + .notice-issued, + .notice-resolved { margin-right: .3em; } @@ -976,6 +981,13 @@ position: absolute; top: 1em; right: 1em; - color: var(--primary); + color: var(--primary-medium); + } + + .disable-important { + position: absolute; + right: 3em; + top: 1em; + color: var(--primary-medium); } } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index a63677c6..61e8274c 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -485,9 +485,13 @@ en: notice: plugin: Custom Wizard Plugin issued: Issued + update: Updated resolved: Resolved title: - warning: Warning Notice + plugin_status_warning: Warning Notice + plugin_status_connection_error: Connection Notice + subscription_messages_connection_error: Connection Notice + disable_important_on_dashboard: disable wizard_js: group: diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 63996b02..5fd7c076 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -53,16 +53,22 @@ en: subscription: "%{type} %{property} is subscription only" notice: - connection_error: "Failed to connect to [%{server}](http://%{server})" + connection_error: "Failed to connect to http://%{domain}" compatibility_issue: > - The Custom Wizard Plugin may have a compatibility issue with the latest version of Discourse. - Please check the Custom Wizard Plugin status on [%{server}](http://%{server}) before updating Discourse. - plugin_status_connection_error_limit: > - We're unable to connect to the plugin status server to determine whether there are any compatibility issues with the latest version of Discourse. - Please contact support@thepavilion.io for further assistance. + The Custom Wizard Plugin has a compatibility issue with the latest version of Discourse. + Please check the Custom Wizard Plugin status on [%{domain}](http://%{domain}) before updating Discourse. + plugin_status: + connection_error_limit: > + We're unable to connect to the Pavilion Plugin Status Server. Please check the Custom Wizard Plugin status on [%{domain}](http://%{domain}) before updating Discourse or the plugin. + If this connection issue persists please contact support@thepavilion.io for further assistance. + subscription_messages: + connection_error_limit: > + We're unable to connect to the Pavilion Subscription Server. This will not affect the operation of the plugin. + If this connection issue persists please contact support@thepavilion.io for further assistance. site_settings: custom_wizard_enabled: "Enable custom wizards." wizard_redirect_exclude_paths: "Routes excluded from wizard redirects." wizard_recognised_image_upload_formats: "File types which will result in upload displaying an image preview" wizard_apis_enabled: "Enable API features (experimental)." + wizard_important_notices_on_dashboard: "Show important notices about the custom wizard plugin on the admin dashboard." diff --git a/config/settings.yml b/config/settings.yml index 0d93524d..d7b34aa9 100644 --- a/config/settings.yml +++ b/config/settings.yml @@ -17,4 +17,7 @@ plugins: list_type: compact wizard_apis_enabled: client: true - default: false \ No newline at end of file + default: false + wizard_important_notices_on_dashboard: + client: true + default: true \ No newline at end of file diff --git a/controllers/custom_wizard/admin/notice.rb b/controllers/custom_wizard/admin/notice.rb index bb332810..f28240e3 100644 --- a/controllers/custom_wizard/admin/notice.rb +++ b/controllers/custom_wizard/admin/notice.rb @@ -4,7 +4,7 @@ class CustomWizard::AdminNoticeController < CustomWizard::AdminController before_action :find_notice, only: [:dismiss] def index - render_serialized(CustomWizard::Notice.list, CustomWizard::NoticeSerializer) + render_serialized(CustomWizard::Notice.list(include_recently_expired: true), CustomWizard::NoticeSerializer) end def dismiss @@ -19,4 +19,4 @@ class CustomWizard::AdminNoticeController < CustomWizard::AdminController @notice = CustomWizard::Notice.find(params[:notice_id]) raise Discourse::InvalidParameters.new(:notice_id) unless @notice end -end \ No newline at end of file +end diff --git a/jobs/scheduled/custom_wizard/update_notices.rb b/jobs/scheduled/custom_wizard/update_notices.rb index 5194e2b8..25ebdf3b 100644 --- a/jobs/scheduled/custom_wizard/update_notices.rb +++ b/jobs/scheduled/custom_wizard/update_notices.rb @@ -6,4 +6,4 @@ class Jobs::CustomWizardUpdateNotices < ::Jobs::Scheduled def execute(args = {}) CustomWizard::Notice.update end -end \ No newline at end of file +end diff --git a/lib/custom_wizard/notice.rb b/lib/custom_wizard/notice.rb index 096cc579..335ccbd4 100644 --- a/lib/custom_wizard/notice.rb +++ b/lib/custom_wizard/notice.rb @@ -3,7 +3,14 @@ class CustomWizard::Notice include ActiveModel::Serialization + PLUGIN_STATUS_DOMAINS = { + "tests-passed" => "try.thepavilion.io", + "stable" => "stable.try.thepavilion.io" + } + SUBSCRIPTION_MESSAGES_DOMAIN = "thepavilion.io" + LOCALHOST_DOMAIN = "localhost:3000" PLUGIN_STATUSES_TO_WARN = %w(incompatible tests_failing) + CHECK_PLUGIN_STATUS_ON_BRANCH = %w(tests-passed main stable) attr_reader :id, :message, @@ -11,6 +18,7 @@ class CustomWizard::Notice :created_at attr_accessor :retrieved_at, + :updated_at, :dismissed_at, :expired_at @@ -19,6 +27,7 @@ class CustomWizard::Notice @message = attrs[:message] @type = attrs[:type].to_i @created_at = attrs[:created_at] + @updated_at = attrs[:updated_at] @retrieved_at = attrs[:retrieved_at] @dismissed_at = attrs[:dismissed_at] @expired_at = attrs[:expired_at] @@ -52,7 +61,6 @@ class CustomWizard::Notice attrs = { expired_at: expired_at, created_at: created_at, - expired_at: expired_at, message: message, type: type } @@ -67,14 +75,9 @@ class CustomWizard::Notice def self.types @types ||= Enum.new( info: 0, - warning: 1 - ) - end - - def self.connection_types - @connection_types ||= Enum.new( - plugin_status: 0, - subscription: 1 + plugin_status_warning: 1, + plugin_status_connection_error: 2, + subscription_messages_connection_error: 3 ) end @@ -82,23 +85,20 @@ class CustomWizard::Notice notices = [] if !skip_subscription - subscription_messages = request(subscription_messages_url) + subscription_messages = request(:subscription_messages) + if subscription_messages.present? subscription_notices = convert_subscription_messages_to_notices(subscription_messages[:messages]) notices.push(*subscription_notices) end end - if !skip_plugin && (Discourse.git_branch === 'tests-passed' || (Rails.env.test? || Rails.env.development?)) - plugin_status = request(plugin_status_url) + if !skip_plugin && request_plugin_status? + plugin_status = request(:plugin_status) if plugin_status.present? && plugin_status[:status].present? && plugin_status[:status].is_a?(Hash) plugin_notice = convert_plugin_status_to_notice(plugin_status[:status]) notices.push(plugin_notice) if plugin_notice - - expire_connection_errors(connection_types[:plugin_status]) - else - create_connection_error(connection_types[:plugin_status]) end end @@ -107,14 +107,6 @@ class CustomWizard::Notice notice.retrieved_at = Time.now notice.save end - - if reached_connection_error_limit(connection_types[:plugin_status]) - new( - message: I18n.t("wizard.notice.plugin_status_connection_error_limit"), - type: types[:warning], - created_at: Time.now - ) - end end def self.convert_subscription_messages_to_notices(messages) @@ -133,19 +125,46 @@ class CustomWizard::Notice if PLUGIN_STATUSES_TO_WARN.include?(plugin_status[:status]) notice = { - message: PrettyText.cook(I18n.t('wizard.notice.compatibility_issue', server: plugin_status_domain)), - type: types[:warning], + message: PrettyText.cook(I18n.t('wizard.notice.compatibility_issue', domain: plugin_status_domain)), + type: types[:plugin_status_warning], created_at: plugin_status[:status_changed_at] } else - list(types[:warning]).each(&:expire) + expire_notices(types[:plugin_status_warning]) end notice end + def self.notify_connection_errors(connection_type_key) + domain = self.send("#{connection_type_key.to_s}_domain") + message = PrettyText.cook(I18n.t("wizard.notice.#{connection_type_key.to_s}.connection_error_limit", domain: domain)) + notices = list(type: types[:connection_error], message: message) + + if notices.any? + notice = notices.first + notice.updated_at = Time.now + notice.save + else + notice = new( + message: message, + type: types["#{connection_type_key}_connection_error".to_sym], + created_at: Time.now + ) + notice.save + end + end + + def self.expire_notices(type) + list(type: type).each(&:expire) + end + + def self.request_plugin_status? + CHECK_PLUGIN_STATUS_ON_BRANCH.include?(Discourse.git_branch) || Rails.env.test? || Rails.env.development? + end + def self.subscription_messages_domain - "localhost:3000" + (Rails.env.test? || Rails.env.development?) ? LOCALHOST_DOMAIN : SUBSCRIPTION_MESSAGES_DOMAIN end def self.subscription_messages_url @@ -153,25 +172,34 @@ class CustomWizard::Notice end def self.plugin_status_domain - "localhost:4200" + return LOCALHOST_DOMAIN if (Rails.env.test? || Rails.env.development?) + PLUGIN_STATUS_DOMAINS[Discourse.git_branch] end def self.plugin_status_url "http://#{plugin_status_domain}/plugin-manager/status/discourse-custom-wizard" end - - def self.request(url) + + def self.request(type) + url = self.send("#{type.to_s}_url") response = Excon.get(url) + connection_error = CustomWizard::Notice::ConnectionError.new(type) if response.status == 200 + connection_error.expire! + expire_notices(types["#{type}_connection_error".to_sym]) + begin data = JSON.parse(response.body).deep_symbolize_keys rescue JSON::ParserError return nil end - + data else + connection_error.create! + notify_connection_errors(type) if connection_error.reached_limit? + nil end end @@ -180,10 +208,6 @@ class CustomWizard::Notice "#{CustomWizard::PLUGIN_NAME}_notice" end - def self.namespace_connection - "#{CustomWizard::PLUGIN_NAME}_notice_connection" - end - def self.find(id) raw = PluginStore.get(namespace, id) new(raw.symbolize_keys) if raw.present? @@ -193,42 +217,16 @@ class CustomWizard::Notice PluginStore.set(namespace, id, raw_notice) end - def self.plugin_status_connection_error_limit - 5 - end - - def self.list_connection_query(type) - query = PluginStoreRow.where(plugin_name: namespace_connection) - query.where("(value::json->>'type')::integer = ?", type) - end - - def self.expire_connection_errors(type) - list_connection_query(type).update_all("value = jsonb_set(value::jsonb, '{ expired_at }', (to_char(current_timestamp, 'HH12:MI:SS'))::jsonb)") - end - - def self.create_connection_error(type) - id = SecureRandom.hex(16) - attrs = { - message: I18n.t("wizard.notice.connection_error", domain: self.send("#{type}_domain")), - type: type, - created_at: Time.now - } - PluginStore.set(namespace_connection, id, attrs) - end - - def self.reached_connection_error_limit(type) - list_connection_query(type).size >= self.send("#{connection_types.key(type)}_connection_error_limit") - end - - def self.list_query(type = nil) + def self.list_query(type: nil, message: nil, include_recently_expired: false) query = PluginStoreRow.where(plugin_name: namespace) - query = query.where("(value::json->>'expired_at') IS NULL OR (value::json->>'expired_at')::date > now()::date - 1") + query = query.where("(value::json->>'expired_at') IS NULL#{include_recently_expired ? " OR (value::json->>'expired_at')::date > now()::date - 1" : ""}") query = query.where("(value::json->>'type')::integer = ?", type) if type + query = query.where("(value::json->>'message') = ?", message) if message query.order("value::json->>'created_at' DESC") end - def self.list(type = nil) - list_query(type) + def self.list(type: nil, message: nil, include_recently_expired: false) + list_query(type: type, message: message, include_recently_expired: include_recently_expired) .map { |r| self.new(JSON.parse(r.value).symbolize_keys) } end end diff --git a/lib/custom_wizard/notice/connection_error.rb b/lib/custom_wizard/notice/connection_error.rb new file mode 100644 index 00000000..84620c0d --- /dev/null +++ b/lib/custom_wizard/notice/connection_error.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +class CustomWizard::Notice::ConnectionError + + attr_reader :type_key + + def initialize(type_key) + @type_key = type_key + end + + def create! + id = "#{type_key.to_s}_error" + + if attrs = PluginStore.get(namespace, id) + attrs['updated_at'] = Time.now + attrs['count'] = attrs['count'].to_i + 1 + else + domain = CustomWizard::Notice.send("#{type_key.to_s}_domain") + attrs = { + message: I18n.t("wizard.notice.connection_error", domain: domain), + type: self.class.types[type_key], + created_at: Time.now, + count: 1 + } + end + + PluginStore.set(namespace, id, attrs) + + @errors = nil + end + + def expire! + if errors.exists? + errors.each do |error_row| + error = JSON.parse(error_row.value) + error['expired_at'] = Time.now + error_row.value = error + error_row.save + end + end + end + + def self.types + @types ||= Enum.new( + plugin_status: 0, + subscription_messages: 1 + ) + end + + def plugin_status_limit + 5 + end + + def subscription_messages_limit + 10 + end + + def limit + self.send("#{type_key.to_s}_limit") + end + + def reached_limit? + return false unless errors.exists? + current_error['count'].to_i >= limit + end + + def current_error + JSON.parse(errors.first.value) + end + + def namespace + "#{CustomWizard::PLUGIN_NAME}_notice_connection" + end + + def errors + @errors ||= begin + query = PluginStoreRow.where(plugin_name: namespace) + query = query.where("(value::json->>'type')::integer = ?", self.class.types[type_key]) + query.where("(value::json->>'expired_at') IS NULL") + end + end +end diff --git a/plugin.rb b/plugin.rb index c3063d29..97d5472b 100644 --- a/plugin.rb +++ b/plugin.rb @@ -40,7 +40,7 @@ if respond_to?(:register_svg_icon) register_svg_icon "comment-alt" register_svg_icon "far-life-ring" register_svg_icon "arrow-right" - register_svg_icon "shield-virus" + register_svg_icon "bolt" end class ::Sprockets::DirectiveProcessor @@ -98,6 +98,7 @@ after_initialize do ../lib/custom_wizard/template.rb ../lib/custom_wizard/wizard.rb ../lib/custom_wizard/notice.rb + ../lib/custom_wizard/notice/connection_error.rb ../lib/custom_wizard/subscription.rb ../lib/custom_wizard/subscription/subscription.rb ../lib/custom_wizard/subscription/authentication.rb @@ -242,10 +243,9 @@ after_initialize do end AdminDashboardData.add_problem_check do - warning_notices = CustomWizard::Notice.list(CustomWizard::Notice.types[:warning]) + warning_notices = CustomWizard::Notice.list(type: CustomWizard::Notice.types[:plugin_status_warning]) warning_notices.any? ? ActionView::Base.full_sanitizer.sanitize(warning_notices.first.message, tags: %w(a)) : nil end - Jobs.enqueue(:custom_wizard_update_notices) DiscourseEvent.trigger(:custom_wizard_ready) end diff --git a/serializers/custom_wizard/notice_serializer.rb b/serializers/custom_wizard/notice_serializer.rb index 310827f7..48b7b381 100644 --- a/serializers/custom_wizard/notice_serializer.rb +++ b/serializers/custom_wizard/notice_serializer.rb @@ -6,6 +6,7 @@ class CustomWizard::NoticeSerializer < ApplicationSerializer :type, :created_at, :expired_at, + :updated_at, :dismissed_at, :retrieved_at, :dismissable diff --git a/spec/components/custom_wizard/notice_spec.rb b/spec/components/custom_wizard/notice_spec.rb index 373d2e31..ef29ea21 100644 --- a/spec/components/custom_wizard/notice_spec.rb +++ b/spec/components/custom_wizard/notice_spec.rb @@ -33,13 +33,13 @@ describe CustomWizard::Notice do expect(notice.message).to eq(subscription_message[:message]) expect(notice.created_at.to_datetime).to be_within(1.second).of (subscription_message[:created_at].to_datetime) end - + it "expires notice if subscription message is expired" do subscription_message[:expired_at] = Time.now stub_request(:get, described_class.subscription_messages_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) described_class.update(skip_plugin: true) - notice = described_class.list.first + notice = described_class.list(include_recently_expired: true).first expect(notice.expired?).to eq(true) end end @@ -51,20 +51,20 @@ describe CustomWizard::Notice do described_class.update(skip_subscription: true) end - it "converts plugin statuses to warn into notices" do + it "converts warning into notice" do notice = described_class.list.first - expect(notice.type).to eq(described_class.types[:warning]) + expect(notice.type).to eq(described_class.types[:plugin_status_warning]) expect(notice.message).to eq(PrettyText.cook(I18n.t("wizard.notice.compatibility_issue", server: described_class.plugin_status_domain))) expect(notice.created_at.to_datetime).to be_within(1.second).of (plugin_status[:status_changed_at].to_datetime) end - - it "expires unexpired warning notices if status is recommended or compatible" do + + it "expires warning notices if status is recommended or compatible" do plugin_status[:status] = 'compatible' plugin_status[:status_changed_at] = Time.now stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) described_class.update(skip_subscription: true) - notice = described_class.list(described_class.types[:warning]).first + notice = described_class.list(type: described_class.types[:plugin_status_warning], include_recently_expired: true).first expect(notice.expired?).to eq(true) end end @@ -75,6 +75,57 @@ describe CustomWizard::Notice do stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) described_class.update - expect(described_class.list.length).to eq(2) + expect(described_class.list(include_recently_expired: true).length).to eq(2) end -end \ No newline at end of file + + context "connection errors" do + before do + freeze_time + end + + it "creates an error if connection to notice server fails" do + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: { status: plugin_status }.to_json) + described_class.update(skip_subscription: true) + + error = CustomWizard::Notice::ConnectionError.new(:plugin_status) + expect(error.errors.exists?).to eq(true) + end + + it "only creates one connection error per type at a time" do + stub_request(:get, described_class.subscription_messages_url).to_return(status: 400, body: { messages: [subscription_message] }.to_json) + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: { status: plugin_status }.to_json) + + 5.times { described_class.update } + + plugin_status_errors = CustomWizard::Notice::ConnectionError.new(:plugin_status) + subscription_message_errors = CustomWizard::Notice::ConnectionError.new(:subscription_messages) + + expect(plugin_status_errors.errors.length).to eq(1) + expect(subscription_message_errors.errors.length).to eq(1) + end + + it "creates a connection error notice if connection errors reach limit" do + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: { status: plugin_status }.to_json) + + error = CustomWizard::Notice::ConnectionError.new(:plugin_status) + error.limit.times { described_class.update(skip_subscription: true) } + notice = described_class.list(type: described_class.types[:plugin_status_connection_error]).first + + expect(error.current_error['count']).to eq(error.limit) + expect(notice.type).to eq(described_class.types[:plugin_status_connection_error]) + end + + it "expires a connection error notice if connection succeeds" do + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: { status: plugin_status }.to_json) + error = CustomWizard::Notice::ConnectionError.new(:plugin_status) + error.limit.times { described_class.update(skip_subscription: true) } + + stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) + described_class.update(skip_subscription: true) + notice = described_class.list(type: described_class.types[:plugin_status_connection_error], include_recently_expired: true).first + + expect(notice.type).to eq(described_class.types[:plugin_status_connection_error]) + expect(notice.expired_at.present?).to eq(true) + end + end +end diff --git a/spec/fixtures/sprockets/require_tree_discourse_empty.js b/spec/fixtures/sprockets/require_tree_discourse_empty.js index df264ec5..953c5ec4 100644 --- a/spec/fixtures/sprockets/require_tree_discourse_empty.js +++ b/spec/fixtures/sprockets/require_tree_discourse_empty.js @@ -1 +1 @@ -//= require_tree_discourse \ No newline at end of file +//= require_tree_discourse \ No newline at end of file From b475e39ee9698083ddf933ae6e4b0c31029bbaf5 Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Thu, 7 Oct 2021 21:19:19 +0800 Subject: [PATCH 2/4] Fix specs --- coverage/.last_run.json | 2 +- lib/custom_wizard/notice.rb | 6 +++--- serializers/custom_wizard/notice_serializer.rb | 4 ++++ spec/components/custom_wizard/notice_spec.rb | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/coverage/.last_run.json b/coverage/.last_run.json index 1c721888..a5de03be 100644 --- a/coverage/.last_run.json +++ b/coverage/.last_run.json @@ -1,5 +1,5 @@ { "result": { - "line": 92.3 + "line": 92.45 } } diff --git a/lib/custom_wizard/notice.rb b/lib/custom_wizard/notice.rb index 335ccbd4..8d41d634 100644 --- a/lib/custom_wizard/notice.rb +++ b/lib/custom_wizard/notice.rb @@ -125,7 +125,7 @@ class CustomWizard::Notice if PLUGIN_STATUSES_TO_WARN.include?(plugin_status[:status]) notice = { - message: PrettyText.cook(I18n.t('wizard.notice.compatibility_issue', domain: plugin_status_domain)), + message: I18n.t('wizard.notice.compatibility_issue', domain: plugin_status_domain), type: types[:plugin_status_warning], created_at: plugin_status[:status_changed_at] } @@ -138,7 +138,7 @@ class CustomWizard::Notice def self.notify_connection_errors(connection_type_key) domain = self.send("#{connection_type_key.to_s}_domain") - message = PrettyText.cook(I18n.t("wizard.notice.#{connection_type_key.to_s}.connection_error_limit", domain: domain)) + message = I18n.t("wizard.notice.#{connection_type_key.to_s}.connection_error_limit", domain: domain) notices = list(type: types[:connection_error], message: message) if notices.any? @@ -221,7 +221,7 @@ class CustomWizard::Notice query = PluginStoreRow.where(plugin_name: namespace) query = query.where("(value::json->>'expired_at') IS NULL#{include_recently_expired ? " OR (value::json->>'expired_at')::date > now()::date - 1" : ""}") query = query.where("(value::json->>'type')::integer = ?", type) if type - query = query.where("(value::json->>'message') = ?", message) if message + query = query.where("(value::json->>'message')::text = ?", message) if message query.order("value::json->>'created_at' DESC") end diff --git a/serializers/custom_wizard/notice_serializer.rb b/serializers/custom_wizard/notice_serializer.rb index 48b7b381..5564de1f 100644 --- a/serializers/custom_wizard/notice_serializer.rb +++ b/serializers/custom_wizard/notice_serializer.rb @@ -18,4 +18,8 @@ class CustomWizard::NoticeSerializer < ApplicationSerializer def type CustomWizard::Notice.types.key(object.type) end + + def messsage + PrettyText.cook(object.message) + end end diff --git a/spec/components/custom_wizard/notice_spec.rb b/spec/components/custom_wizard/notice_spec.rb index ef29ea21..b51305e1 100644 --- a/spec/components/custom_wizard/notice_spec.rb +++ b/spec/components/custom_wizard/notice_spec.rb @@ -54,7 +54,7 @@ describe CustomWizard::Notice do it "converts warning into notice" do notice = described_class.list.first expect(notice.type).to eq(described_class.types[:plugin_status_warning]) - expect(notice.message).to eq(PrettyText.cook(I18n.t("wizard.notice.compatibility_issue", server: described_class.plugin_status_domain))) + expect(notice.message).to eq(I18n.t("wizard.notice.compatibility_issue", domain: described_class.plugin_status_domain)) expect(notice.created_at.to_datetime).to be_within(1.second).of (plugin_status[:status_changed_at].to_datetime) end From 702ccc868c7d5b845937d5a97ff2405ac57d122e Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Tue, 12 Oct 2021 09:45:50 +0800 Subject: [PATCH 3/4] Update connection_error.rb --- lib/custom_wizard/notice/connection_error.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/custom_wizard/notice/connection_error.rb b/lib/custom_wizard/notice/connection_error.rb index 84620c0d..8b8b944a 100644 --- a/lib/custom_wizard/notice/connection_error.rb +++ b/lib/custom_wizard/notice/connection_error.rb @@ -25,7 +25,6 @@ class CustomWizard::Notice::ConnectionError end PluginStore.set(namespace, id, attrs) - @errors = nil end @@ -34,7 +33,7 @@ class CustomWizard::Notice::ConnectionError errors.each do |error_row| error = JSON.parse(error_row.value) error['expired_at'] = Time.now - error_row.value = error + error_row.value = error.to_json error_row.save end end From 450f7bfc2520829814a9004ffd033e2ecaa6fc9f Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Wed, 13 Oct 2021 19:32:49 +0800 Subject: [PATCH 4/4] Update notice.rb --- lib/custom_wizard/notice.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/custom_wizard/notice.rb b/lib/custom_wizard/notice.rb index 8d41d634..fb10b724 100644 --- a/lib/custom_wizard/notice.rb +++ b/lib/custom_wizard/notice.rb @@ -4,10 +4,10 @@ class CustomWizard::Notice include ActiveModel::Serialization PLUGIN_STATUS_DOMAINS = { - "tests-passed" => "try.thepavilion.io", - "stable" => "stable.try.thepavilion.io" + "tests-passed" => "plugins.thepavilion.io", + "stable" => "stable.plugins.thepavilion.io" } - SUBSCRIPTION_MESSAGES_DOMAIN = "thepavilion.io" + SUBSCRIPTION_MESSAGES_DOMAIN = "test.thepavilion.io" LOCALHOST_DOMAIN = "localhost:3000" PLUGIN_STATUSES_TO_WARN = %w(incompatible tests_failing) CHECK_PLUGIN_STATUS_ON_BRANCH = %w(tests-passed main stable)