From 03690ccccf7108a139cee1e8a3fe0f3efbc3651e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Saquetim?= <1108771+megothss@users.noreply.github.com> Date: Wed, 2 Aug 2023 18:44:19 -0300 Subject: [PATCH] DEV: Add :push_notification event and deprecate :post_notification_alert (#22917) This commit introduces the :push_notification event and deprecates :post_notification_alert. The old :post_notification_alert event was not triggered when pushing chat notifications and did not respect when the user was in "do not disturb" mode. The new event fixes these issues. --- app/services/post_alerter.rb | 4 ++++ lib/discourse_event.rb | 12 +++++++++++- spec/services/post_alerter_spec.rb | 26 +++++++++++++++++++++++++- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/app/services/post_alerter.rb b/app/services/post_alerter.rb index b0cb9310a2e..75aa3644a81 100644 --- a/app/services/post_alerter.rb +++ b/app/services/post_alerter.rb @@ -42,6 +42,8 @@ class PostAlerter end push_notification(user, payload) + + # deprecated. use push_notification instead DiscourseEvent.trigger(:post_notification_alert, user, payload) end end @@ -80,6 +82,8 @@ class PostAlerter Jobs.enqueue(:push_notification, clients: clients, payload: payload, user_id: user.id) end end + + DiscourseEvent.trigger(:push_notification, user, payload) end def initialize(default_opts = {}) diff --git a/lib/discourse_event.rb b/lib/discourse_event.rb index 64a467bd77c..49051462cfe 100644 --- a/lib/discourse_event.rb +++ b/lib/discourse_event.rb @@ -13,13 +13,23 @@ class DiscourseEvent end def self.on(event_name, &block) - if event_name == :user_badge_removed + case event_name + when :user_badge_removed Discourse.deprecate( "The :user_badge_removed event is deprecated. Please use :user_badge_revoked instead", since: "3.1.0.beta5", drop_from: "3.2.0.beta1", output_in_test: true, ) + when :post_notification_alert + Discourse.deprecate( + "The :post_notification_alert event is deprecated. Please use :push_notification instead", + since: "3.2.0.beta1", + drop_from: "3.3.0.beta1", + output_in_test: true, + ) + else + # ignore end events[event_name] << block diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 9cfb9c1350f..fe403d31d4a 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -1194,13 +1194,30 @@ RSpec.describe PostAlerter do it "does not send push notifications when a filters returns false" do Plugin::Instance.new.register_push_notification_filter { |user, payload| false } expect { mention_post }.not_to change { Jobs::PushNotification.jobs.count } + + events = DiscourseEvent.track_events { mention_post } + expect(events.find { |event| event[:event_name] == :push_notification }).not_to be_present + DiscoursePluginRegistry.reset! end end + it "triggers the push notification event" do + events = DiscourseEvent.track_events { mention_post } + + push_notification_event = events.find { |event| event[:event_name] == :push_notification } + expect(push_notification_event).to be_present + expect(push_notification_event[:params][0].username).to eq("eviltrout") + expect(push_notification_event[:params][1][:username]).to eq(user.username) + expect(push_notification_event[:params][1][:excerpt]).to eq("Hello @eviltrout ❤") + end + it "pushes nothing to suspended users" do evil_trout.update_columns(suspended_till: 1.year.from_now) expect { mention_post }.to_not change { Jobs::PushNotification.jobs.count } + + events = DiscourseEvent.track_events { mention_post } + expect(events.find { |event| event[:event_name] == :push_notification }).not_to be_present end it "pushes nothing when the user is in 'do not disturb'" do @@ -1212,6 +1229,9 @@ RSpec.describe PostAlerter do ) expect { mention_post }.to_not change { Jobs::PushNotification.jobs.count } + + events = DiscourseEvent.track_events { mention_post } + expect(events.find { |event| event[:event_name] == :push_notification }).not_to be_present end it "correctly pushes notifications if configured correctly" do @@ -1413,7 +1433,11 @@ RSpec.describe PostAlerter do end end - expect(events.size).to eq(2) + expect(events.map { |event| event[:event_name] }).to include( + :pre_notification_alert, + :push_notification, + :post_notification_alert, + ) expect(messages.size).to eq(0) expect(Jobs::PushNotification.jobs.size).to eq(1) end