diff --git a/app/services/broadcasts/welcome_notification/generator.rb b/app/services/broadcasts/welcome_notification/generator.rb
index 584df3b9b..143ef48fe 100644
--- a/app/services/broadcasts/welcome_notification/generator.rb
+++ b/app/services/broadcasts/welcome_notification/generator.rb
@@ -1,9 +1,9 @@
-# Generates a broadcast to be delivered as a notification.
module Broadcasts
module WelcomeNotification
class Generator
def initialize(receiver_id)
@user = User.find(receiver_id)
+ @notification_enqueued = false
end
def self.call(*args)
@@ -13,13 +13,31 @@ module Broadcasts
def call
# TODO: [@thepracticaldev/delightful] Move this check into the rake task logic once it has been implemented.
return unless user.welcome_notifications
- return if commented_on_welcome_thread? || received_notification?
- Notification.send_welcome_notification(user.id, welcome_broadcast.id)
+ send_welcome_notification unless notification_enqueued
+ send_authentication_notification unless notification_enqueued
end
- def received_notification?
- Notification.exists?(notifiable: welcome_broadcast, user: user)
+ private
+
+ attr_reader :user, :notification_enqueued
+
+ def send_welcome_notification
+ return if received_notification?(welcome_broadcast) || commented_on_welcome_thread? || user.created_at > 3.hours.ago
+
+ Notification.send_welcome_notification(user.id, welcome_broadcast.id)
+ @notification_enqueued = true
+ end
+
+ def send_authentication_notification
+ return if authenticated_with_all_providers? || received_notification?(authentication_broadcast) || user.created_at > 1.day.ago
+
+ Notification.send_welcome_notification(user.id, authentication_broadcast.id)
+ @notification_enqueued = true
+ end
+
+ def received_notification?(broadcast)
+ Notification.exists?(notifiable: broadcast, user: user)
end
def commented_on_welcome_thread?
@@ -27,13 +45,28 @@ module Broadcasts
Comment.where(commentable: welcome_thread, user: user).any?
end
- private
+ def authenticated_with_all_providers?
+ identities.count == SiteConfig.authentication_providers.count
+ end
def welcome_broadcast
@welcome_broadcast ||= Broadcast.find_by(title: "Welcome Notification: welcome_thread")
end
- attr_reader :user
+ def identities
+ @identities ||= user.identities.where(provider: SiteConfig.authentication_providers)
+ end
+
+ def authentication_broadcast
+ @authentication_broadcast ||= find_broadcast
+ end
+
+ def find_broadcast
+ missing_identities = SiteConfig.authentication_providers.map do |provider|
+ identities.exists?(provider: provider) ? nil : "#{provider}_connect"
+ end.compact
+ Broadcast.find_by(title: "Welcome Notification: #{missing_identities.first}")
+ end
end
end
end
diff --git a/spec/factories/broadcasts.rb b/spec/factories/broadcasts.rb
index a91005a63..3547eb6cf 100644
--- a/spec/factories/broadcasts.rb
+++ b/spec/factories/broadcasts.rb
@@ -1,22 +1,30 @@
FactoryBot.define do
factory :broadcast do
- active { false }
+ active { true }
factory :welcome_broadcast do
- title { "Welcome Notification: welcome_thread" }
- type_of { "Welcome" }
+ title { "Welcome Notification: welcome_thread" }
+ type_of { "Welcome" }
processed_html { "Sloan here again! 👋 DEV is a friendly community. Why not introduce yourself by leaving a comment in the welcome thread!" }
end
+ factory :twitter_connect_broadcast do
+ title { "Welcome Notification: twitter_connect" }
+ type_of { "Welcome" }
+ processed_html { "You're on a roll! 🎉 Let's connect your Twitter account to complete your identity so that we don't think you're a robot. 🤖" }
+ end
+
+ factory :github_connect_broadcast do
+ title { "Welcome Notification: github_connect" }
+ type_of { "Welcome" }
+ processed_html { "You're on a roll! 🎉 Let's connect your GitHub account to complete your identity so that we don't think you're a robot. 🤖" }
+ end
+
# TODO: [@thepracticaldev/delightful] Remove onboarding factory once welcome notifications are live.
factory :onboarding_broadcast do
- title { "Welcome Notification" }
- type_of { "Onboarding" }
+ title { "Welcome Notification" }
+ type_of { "Onboarding" }
processed_html { "Welcome! Introduce yourself in our welcome thread!" }
end
end
-
- trait :active do
- active { true }
- end
end
diff --git a/spec/requests/notifications_spec.rb b/spec/requests/notifications_spec.rb
index f2fbbe1e7..cb78a6993 100644
--- a/spec/requests/notifications_spec.rb
+++ b/spec/requests/notifications_spec.rb
@@ -593,8 +593,8 @@ RSpec.describe "NotificationsIndex", type: :request do
context "when a user has a new welcome notification" do
# TODO: [@thepracticaldev/delightful] Only test against type_of Welcome once Onbarding notifications have been removed.
- let(:active_broadcast) { create(:onboarding_broadcast, :active) }
- let(:inactive_broadcast) { create(:onboarding_broadcast) }
+ let(:active_broadcast) { create(:onboarding_broadcast) }
+ let(:inactive_broadcast) { create(:onboarding_broadcast, active: false) }
before { sign_in user }
diff --git a/spec/services/broadcasts/welcome_notification/generator_spec.rb b/spec/services/broadcasts/welcome_notification/generator_spec.rb
index a29fcb882..7bf68b495 100644
--- a/spec/services/broadcasts/welcome_notification/generator_spec.rb
+++ b/spec/services/broadcasts/welcome_notification/generator_spec.rb
@@ -1,77 +1,126 @@
require "rails_helper"
-# TODO: [@thepracticaldev/delightful] Reuse this shared example across all notifications,
-# since it should be tested against every kind of broadcast we could send.
-RSpec.shared_examples "unsubscribed from welcome notifications" do |_broadcast|
- it "does not send a notification to an unsubscribed user" do
- expect do
- sidekiq_perform_enqueued_jobs { described_class.call(unsubscribed_user.id) }
- end.to not_change(unsubscribed_user.notifications, :count)
- end
-end
-
RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
+ let(:mascot_account) { create(:user) }
+ let!(:welcome_thread) { create(:article, user: mascot_account, published: true, tags: "welcome") }
+ let!(:welcome_broadcast) { create(:welcome_broadcast) }
+ let!(:twitter_connect_broadcast) { create(:twitter_connect_broadcast) }
+ let!(:github_connect_broadcast) { create(:github_connect_broadcast) }
+
+ before do
+ allow(User).to receive(:mascot_account).and_return(mascot_account)
+ SiteConfig.staff_user_id = mascot_account.id
+ end
+
+ after do
+ # SiteConfig.clear_cache should work here but for some reason it isn't
+ SiteConfig.staff_user_id = 1
+ end
+
+ it "requires a valid user id" do
+ expect { described_class.call(1) }.to raise_error(ActiveRecord::RecordNotFound)
+ end
+
describe "::call" do
- let(:user) { create(:user) }
- let(:unsubscribed_user) { create(:user, welcome_notifications: false) }
- let(:mascot_account) { create(:user) }
- let!(:welcome_broadcast) { create(:welcome_broadcast, :active) }
-
- before do
- allow(User).to receive(:mascot_account).and_return(mascot_account)
- SiteConfig.staff_user_id = mascot_account.id
+ it "does not send a notification to an unsubscribed user" do
+ user = create(:user, :with_identity, identities: ["github"], created_at: 1.week.ago, welcome_notifications: false)
+ expect do
+ sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
+ end.to not_change(user.notifications, :count)
end
- after do
- SiteConfig.staff_user_id = 1
- end
-
- context "when sending a set_up_profile notification" do
- xit "generates the appropriate broadcast to be sent to a user"
- xit "it sends a welcome notification for that broadcast"
- xit "it does not send duplicate welcome notification for that broadcast"
- xit "does not send a notification to a user who has set up their profile"
- end
-
- context "when sending a welcome_thread notification" do
- it "generates the correct broadcast type and sends the notification to the user", :aggregate_failures do
- expect do
- sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
- end.to change(user.notifications, :count).by(1)
-
- expect(user.notifications.first.notifiable).to eq(welcome_broadcast)
- end
-
- it "does not send a notification to a user who has commented in a welcome thread", :aggregate_failures do
- welcome_thread_article = create(:article, title: "Welcome Thread - v0", published: true, tags: "welcome", user: mascot_account)
- create(:comment, commentable: welcome_thread_article, commentable_type: "Article", user: user)
-
- expect do
- sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
- end.to not_change(user.notifications, :count)
- end
-
- it "does not send a duplicate notification" do
- 2.times { sidekiq_perform_enqueued_jobs { described_class.call(user.id) } }
-
- expect(user.notifications.count).to eq(1)
- end
-
- it_behaves_like "unsubscribed from welcome notifications"
- end
-
- context "when sending a twitter_connect notification" do
- xit "generates the appropriate broadcast to be sent to a user"
- xit "it sends a welcome notification for that broadcast"
- xit "it does not send duplicate welcome notification for that broadcast"
- xit "does not send a notification to a user who is connected via twitter"
- end
-
- context "when sending a github_connect notification" do
- xit "generates the appropriate broadcast to be sent to a user"
- xit "it sends a welcome notification for that broadcast"
- xit "it does not send duplicate welcome notification for that broadcast"
- xit "does not send a notification to a user who is connected via github"
+ it "sends only 1 notification at a time" do
+ user = create(:user, :with_identity, identities: ["github"], created_at: 1.week.ago)
+ expect do
+ sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
+ end.to change(user.notifications, :count).by(1)
end
end
+
+ describe "#send_welcome_notification" do
+ let(:user) { create(:user, created_at: 4.hours.ago) }
+
+ it "does not send a notification to a newly-created user" do
+ user.update!(created_at: Time.zone.now)
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_welcome_notification) }
+ expect(user.notifications.count).to eq(0)
+ end
+
+ it "generates the correct broadcast type and sends the notification to the user" do
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_welcome_notification) }
+ expect(user.notifications.first.notifiable).to eq(welcome_broadcast)
+ end
+
+ it "does not send to a user who has commented in a welcome thread" do
+ create(:comment, commentable: welcome_thread, commentable_type: "Article", user: user)
+ expect do
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_welcome_notification) }
+ end.not_to change(user.notifications, :count)
+ end
+
+ it "does not send a duplicate notifications" do
+ 2.times do
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_welcome_notification) }
+ end
+
+ expect(user.notifications.count).to eq(1)
+ end
+ end
+
+ describe "#send_authentication_notification" do
+ before do
+ allow(Notification).to receive(:send_welcome_notification)
+ end
+
+ it "does not send notification if user is created less than a day ago" do
+ user = create(:user, :with_identity, identities: ["github"])
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ expect(Notification).not_to have_received(:send_welcome_notification)
+ end
+
+ it "generates and sends the appropriate broadcast (twitter)" do
+ user = create(:user, :with_identity, identities: ["github"], created_at: 1.day.ago)
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ expect(Notification).to have_received(:send_welcome_notification).with(user.id, twitter_connect_broadcast.id)
+ end
+
+ it "generates and sends the appropriate broadcast (github)" do
+ user = create(:user, :with_identity, identities: ["twitter"], created_at: 1.day.ago)
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ expect(Notification).to have_received(:send_welcome_notification).with(user.id, github_connect_broadcast.id)
+ end
+
+ it "does not send notification if user is authenticated with both services" do
+ user = create(:user, :with_identity, identities: %w[twitter github], created_at: 1.day.ago)
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ expect(Notification).not_to have_received(:send_welcome_notification).with(user.id, github_connect_broadcast.id)
+ end
+
+ it "does not send a duplicate notifications (github)" do
+ allow(Notification).to receive(:send_welcome_notification).and_call_original
+
+ user = create(:user, :with_identity, identities: ["twitter"], created_at: 1.day.ago)
+ 2.times do
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ end
+ expect(Notification).to have_received(:send_welcome_notification).with(user.id, github_connect_broadcast.id).exactly(:once)
+ end
+
+ it "does not send a duplicate notifications (twitter)" do
+ allow(Notification).to receive(:send_welcome_notification).and_call_original
+
+ user = create(:user, :with_identity, identities: ["github"], created_at: 1.day.ago)
+ 2.times do
+ sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_authentication_notification) }
+ end
+ expect(Notification).to have_received(:send_welcome_notification).with(user.id, twitter_connect_broadcast.id).exactly(:once)
+ end
+ end
+
+ context "when sending a set_up_profile notification" do
+ xit "generates the appropriate broadcast to be sent to a user"
+ xit "it sends a welcome notification for that broadcast"
+ xit "it does not send duplicate welcome notification for that broadcast"
+ xit "does not send a notification to a user who has set up their profile"
+ end
end
diff --git a/spec/workers/notifications/welcome_notification_worker_spec.rb b/spec/workers/notifications/welcome_notification_worker_spec.rb
index 7187cf4f6..665fd4f0d 100644
--- a/spec/workers/notifications/welcome_notification_worker_spec.rb
+++ b/spec/workers/notifications/welcome_notification_worker_spec.rb
@@ -1,8 +1,8 @@
require "rails_helper"
RSpec.describe Notifications::WelcomeNotificationWorker, type: :worker do
describe "#perform" do
- let(:broadcast) { create(:onboarding_broadcast, :active) }
- let(:inactive_broadcast) { create(:onboarding_broadcast) }
+ let(:broadcast) { create(:onboarding_broadcast) }
+ let(:inactive_broadcast) { create(:onboarding_broadcast, active: false) }
let(:user) { create(:user) }
let(:service) { Notifications::WelcomeNotification::Send }
let(:worker) { subject }