Implement authentication welcome notification (#6846)

This commit is contained in:
Mac Siri 2020-03-27 15:21:40 -04:00 committed by GitHub
parent 263baeb2f6
commit 2c5e12268f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 178 additions and 88 deletions

View file

@ -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

View file

@ -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 <a href='/welcome'>the welcome thread</a>!" }
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 <a href='/settings'> Twitter account</a> 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 <a href='/settings'> GitHub account</a> 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 <a href='/welcome'>welcome thread!</a>" }
end
end
trait :active do
active { true }
end
end

View file

@ -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 }

View file

@ -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

View file

@ -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 }