From 98061c14e8ef84e974f1e8162d1ecbc2038e56bf Mon Sep 17 00:00:00 2001 From: angusmcleod Date: Tue, 2 Nov 2021 15:29:31 +0800 Subject: [PATCH] Fix spec (mostly) --- .../initializers/custom-wizard-edits.js.es6 | 1 + lib/custom_wizard/notice.rb | 49 ++++++------ lib/custom_wizard/notice/connection_error.rb | 17 ++-- spec/components/custom_wizard/notice_spec.rb | 80 +++++++++++++------ spec/jobs/update_notices_spec.rb | 2 +- .../admin/notice_controller_spec.rb | 57 +++++++++++-- 6 files changed, 141 insertions(+), 65 deletions(-) diff --git a/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 b/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 index 8208cd20..774acca3 100644 --- a/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 +++ b/assets/javascripts/discourse/initializers/custom-wizard-edits.js.es6 @@ -41,6 +41,7 @@ export default { subscribe() { this.unsubscribe(); this.messageBus.subscribe("/custom-wizard/notices", (data) => { + if (isPresent(data.active_notice_count)) { this.loadCriticalNotices(); } diff --git a/lib/custom_wizard/notice.rb b/lib/custom_wizard/notice.rb index 7266178d..a07e1443 100644 --- a/lib/custom_wizard/notice.rb +++ b/lib/custom_wizard/notice.rb @@ -43,24 +43,30 @@ class CustomWizard::Notice def dismiss! if dismissable? self.dismissed_at = Time.now - self.save - self.class.publish_notice_count + self.save_and_publish end end def hide! if can_hide? self.hidden_at = Time.now - self.save - self.class.publish_notice_count + self.save_and_publish end end def expire! if !expired? self.expired_at = Time.now - self.save + self.save_and_publish + end + end + + def save_and_publish + if self.save self.class.publish_notice_count + true + else + false end end @@ -95,7 +101,6 @@ class CustomWizard::Notice updated_at: updated_at, retrieved_at: retrieved_at, created_at: created_at, - hidden_at: hidden_at, title: title, message: message, type: type, @@ -104,6 +109,7 @@ class CustomWizard::Notice if current = self.class.find(self.id) attrs[:dismissed_at] = current.dismissed_at || self.dismissed_at + attrs[:hidden_at] = current.hidden_at || self.hidden_at end self.class.store(id, attrs) @@ -146,7 +152,6 @@ class CustomWizard::Notice end if notices.any? - notices.each do |notice_data| notice = new(notice_data) notice.retrieved_at = Time.now @@ -166,18 +171,16 @@ class CustomWizard::Notice def self.convert_subscription_messages_to_notices(messages) messages.reduce([]) do |result, message| - notice_id = generate_notice_id(message[:title], message[:created_at]) - - unless exists?(notice_id) - result.push( - title: message[:title], - message: message[:message], - type: types[message[:type].to_sym], - archetype: archetypes[:subscription_message], - created_at: message[:created_at], - expired_at: message[:expired_at] - ) - end + id = generate_notice_id(message[:title], message[:created_at]) + result.push( + id: id, + title: message[:title], + message: message[:message], + type: types[message[:type].to_sym], + archetype: archetypes[:subscription_message], + created_at: message[:created_at], + expired_at: message[:expired_at] + ) result end end @@ -298,11 +301,11 @@ class CustomWizard::Notice PluginStore.set(namespace, id, raw_notice) end - def self.list_query(type: nil, archetype: nil, title: nil, include_all: false, include_recently_expired: false, page: nil, visible: false) + def self.list_query(type: nil, archetype: nil, title: nil, include_all: false, page: nil, visible: false) query = PluginStoreRow.where(plugin_name: namespace) query = query.where("(value::json->>'hidden_at') IS NULL") if visible query = query.where("(value::json->>'dismissed_at') IS NULL") unless include_all - query = query.where("(value::json->>'expired_at') IS NULL#{include_recently_expired ? " OR (value::json->>'expired_at')::date > now()::date - 1" : ""}") unless include_all + query = query.where("(value::json->>'expired_at') IS NULL") unless include_all query = query.where("(value::json->>'archetype')::integer = ?", archetype) if archetype if type type_query_str = type.is_a?(Array) ? "(value::json->>'type')::integer IN (?)" : "(value::json->>'type')::integer = ?" @@ -313,8 +316,8 @@ class CustomWizard::Notice query.order("value::json->>'expired_at' DESC, value::json->>'updated_at' DESC,value::json->>'dismissed_at' DESC, value::json->>'created_at' DESC") end - def self.list(type: nil, archetype: nil, title: nil, include_all: false, include_recently_expired: false, page: 0, visible: false) - list_query(type: type, archetype: archetype, title: title, include_all: include_all, include_recently_expired: include_recently_expired, page: page, visible: visible) + def self.list(type: nil, archetype: nil, title: nil, include_all: false, page: 0, visible: false) + list_query(type: type, archetype: archetype, title: title, include_all: include_all, page: page, visible: visible) .map { |r| self.new(JSON.parse(r.value).symbolize_keys) } end diff --git a/lib/custom_wizard/notice/connection_error.rb b/lib/custom_wizard/notice/connection_error.rb index 9f78a09e..a1d834c6 100644 --- a/lib/custom_wizard/notice/connection_error.rb +++ b/lib/custom_wizard/notice/connection_error.rb @@ -10,9 +10,9 @@ class CustomWizard::Notice::ConnectionError def create! if attrs = current_error - key = "#{archetype.to_s}_error_#{attrs["id"]}" - attrs['updated_at'] = Time.now - attrs['count'] = attrs['count'].to_i + 1 + key = "#{archetype.to_s}_error_#{attrs[:id]}" + attrs[:updated_at] = Time.now + attrs[:count] = attrs[:count].to_i + 1 else domain = CustomWizard::Notice.send("#{archetype.to_s}_domain") id = SecureRandom.hex(8) @@ -27,7 +27,8 @@ class CustomWizard::Notice::ConnectionError end PluginStore.set(namespace, key, attrs) - @errors = nil + + @current_error = nil end def expire! @@ -54,7 +55,7 @@ class CustomWizard::Notice::ConnectionError def reached_limit? return false unless current_error.present? - current_error['count'].to_i >= limit + current_error[:count].to_i >= limit end def namespace @@ -64,11 +65,13 @@ class CustomWizard::Notice::ConnectionError def current_error(query_only: false) @current_error ||= begin query = PluginStoreRow.where(plugin_name: namespace) - query = query.where("(value::json->>'archetype')::integer = ?", CustomWizard::Notice.archetypes[archetype]) + query = query.where("(value::json->>'archetype')::integer = ?", CustomWizard::Notice.archetypes[archetype.to_sym]) query = query.where("(value::json->>'expired_at') IS NULL") + return nil if !query.exists? return query if query_only - JSON.parse(query.first.value) + + JSON.parse(query.first.value).deep_symbolize_keys end end end diff --git a/spec/components/custom_wizard/notice_spec.rb b/spec/components/custom_wizard/notice_spec.rb index b51305e1..0b34664d 100644 --- a/spec/components/custom_wizard/notice_spec.rb +++ b/spec/components/custom_wizard/notice_spec.rb @@ -6,6 +6,7 @@ describe CustomWizard::Notice do fab!(:user) { Fabricate(:user) } let(:subscription_message) { { + title: "Title of message about subscription", message: "Message about subscription", type: "info", created_at: Time.now - 3.day, @@ -23,7 +24,7 @@ describe CustomWizard::Notice do context "subscription message" do before do freeze_time - stub_request(:get, described_class.subscription_messages_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) + stub_request(:get, described_class.subscription_message_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) described_class.update(skip_plugin: true) end @@ -36,46 +37,73 @@ describe CustomWizard::Notice do 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) + stub_request(:get, described_class.subscription_message_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) described_class.update(skip_plugin: true) - notice = described_class.list(include_recently_expired: true).first + notice = described_class.list(include_all: true).first expect(notice.expired?).to eq(true) end + + it "dismisses informational subscription notices" do + notice = described_class.list(include_all: true).first + expect(notice.dismissed?).to eq(false) + + notice.dismiss! + expect(notice.dismissed?).to eq(true) + end + + it "dismisses all informational subscription notices" do + 4.times do |index| + subscription_message[:title] += " #{index}" + stub_request(:get, described_class.subscription_message_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) + described_class.update(skip_plugin: true) + end + expect(described_class.list.count).to eq(5) + described_class.dismiss_all + expect(described_class.list.count).to eq(0) + end end context "plugin status" do before do freeze_time - stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) + stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: plugin_status.to_json) described_class.update(skip_subscription: true) end 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(I18n.t("wizard.notice.compatibility_issue", domain: described_class.plugin_status_domain)) + expect(notice.type).to eq(described_class.types[:warning]) + expect(notice.message).to eq(I18n.t("wizard.notice.compatibility_issue.message", 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 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) + stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: plugin_status.to_json) described_class.update(skip_subscription: true) - notice = described_class.list(type: described_class.types[:plugin_status_warning], include_recently_expired: true).first + notice = described_class.list(type: described_class.types[:warning], include_all: true).first expect(notice.expired?).to eq(true) end + + it "hides plugin status warnings" do + notice = described_class.list.first + expect(notice.hidden?).to eq(false) + + notice.hide! + expect(notice.hidden?).to eq(true) + end end it "lists notices not expired more than a day ago" do subscription_message[:expired_at] = Time.now - 8.hours - stub_request(:get, described_class.subscription_messages_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) - stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) + stub_request(:get, described_class.subscription_message_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) + stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: plugin_status.to_json) described_class.update - expect(described_class.list(include_recently_expired: true).length).to eq(2) + expect(described_class.list(include_all: true).length).to eq(2) end context "connection errors" do @@ -84,47 +112,47 @@ describe CustomWizard::Notice do 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) + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: plugin_status.to_json) described_class.update(skip_subscription: true) error = CustomWizard::Notice::ConnectionError.new(:plugin_status) - expect(error.errors.exists?).to eq(true) + expect(error.current_error.present?).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) + stub_request(:get, described_class.subscription_message_url).to_return(status: 400, body: { messages: [subscription_message] }.to_json) + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: 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) + subscription_message_errors = CustomWizard::Notice::ConnectionError.new(:subscription_message) - expect(plugin_status_errors.errors.length).to eq(1) - expect(subscription_message_errors.errors.length).to eq(1) + expect(plugin_status_errors.current_error[:count]).to eq(5) + expect(subscription_message_errors.current_error[:count]).to eq(5) 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) + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: 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 + notice = described_class.list(type: described_class.types[:connection_error]).first - expect(error.current_error['count']).to eq(error.limit) - expect(notice.type).to eq(described_class.types[:plugin_status_connection_error]) + expect(error.current_error[:count]).to eq(error.limit) + expect(notice.type).to eq(described_class.types[: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) + stub_request(:get, described_class.plugin_status_url).to_return(status: 400, body: 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) + stub_request(:get, described_class.plugin_status_url).to_return(status: 200, body: 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 + notice = described_class.list(type: described_class.types[:connection_error], include_all: true).first - expect(notice.type).to eq(described_class.types[:plugin_status_connection_error]) + expect(notice.type).to eq(described_class.types[:connection_error]) expect(notice.expired_at.present?).to eq(true) end end diff --git a/spec/jobs/update_notices_spec.rb b/spec/jobs/update_notices_spec.rb index d0e5a468..8ba5587f 100644 --- a/spec/jobs/update_notices_spec.rb +++ b/spec/jobs/update_notices_spec.rb @@ -20,7 +20,7 @@ describe Jobs::CustomWizardUpdateNotices do } it "updates the notices" do - stub_request(:get, CustomWizard::Notice.subscription_messages_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) + stub_request(:get, CustomWizard::Notice.subscription_message_url).to_return(status: 200, body: { messages: [subscription_message] }.to_json) stub_request(:get, CustomWizard::Notice.plugin_status_url).to_return(status: 200, body: { status: plugin_status }.to_json) described_class.new.execute diff --git a/spec/requests/custom_wizard/admin/notice_controller_spec.rb b/spec/requests/custom_wizard/admin/notice_controller_spec.rb index bd174e90..33d03432 100644 --- a/spec/requests/custom_wizard/admin/notice_controller_spec.rb +++ b/spec/requests/custom_wizard/admin/notice_controller_spec.rb @@ -3,29 +3,70 @@ require_relative '../../../plugin_helper' describe CustomWizard::AdminNoticeController do fab!(:admin_user) { Fabricate(:user, admin: true) } + let(:subscription_message_notice) { + { + title: "Title of message about subscription", + message: "Message about subscription", + type: 0, + created_at: Time.now.iso8601(3), + expired_at: nil + } + } + let(:plugin_status_notice) { + { + title: "The Custom Wizard Plugin is incompatibile with the latest version of Discourse.", + message: "Please check the Custom Wizard Plugin status on [localhost:3000](http://localhost:3000) before updating Discourse.", + type: 1, + archetype: 1, + created_at: Time.now.iso8601(3), + expired_at: nil + } + } before do sign_in(admin_user) - @notice = CustomWizard::Notice.new( - message: "Message about subscription", - type: "info", - created_at: Time.now - 3.day, - expired_at: nil - ) - @notice.save end it "lists notices" do + @notice = CustomWizard::Notice.new(subscription_message_notice) + @notice.save + get "/admin/wizards/notice.json" expect(response.status).to eq(200) expect(response.parsed_body.length).to eq(1) end it "dismisses notices" do - put "/admin/wizards/notice/#{@notice.id}.json" + @notice = CustomWizard::Notice.new(subscription_message_notice) + @notice.save + + put "/admin/wizards/notice/#{@notice.id}/dismiss.json" expect(response.status).to eq(200) updated = CustomWizard::Notice.find(@notice.id) expect(updated.dismissed?).to eq(true) end + + it "dismisses all notices" do + 5.times do |index| + subscription_message_notice[:title] += " #{index}" + @notice = CustomWizard::Notice.new(subscription_message_notice) + @notice.save + end + + put "/admin/wizards/notice/dismiss.json" + expect(response.status).to eq(200) + expect(CustomWizard::Notice.list.size).to eq(0) + end + + it "hides notices" do + @notice = CustomWizard::Notice.new(plugin_status_notice) + @notice.save + + put "/admin/wizards/notice/#{@notice.id}/hide.json" + expect(response.status).to eq(200) + + updated = CustomWizard::Notice.find(@notice.id) + expect(updated.hidden?).to eq(true) + end end