From e23e69d4b2cbc836f5466f16c9cf44d483532964 Mon Sep 17 00:00:00 2001 From: Vaidehi Joshi Date: Wed, 1 Apr 2020 11:57:50 -0700 Subject: [PATCH] Prompt users to ask a question/start a discussion (#7007) [deploy] * Add a discuss/ask notification(s) to seeds + factories * Add a start discussion/ask question welcome notification * Add a scope for finding articles published by a user + with a certain tag. * Some clean up/refactoring in the generator * Reorganize welcome notification sending, add specs for discuss/ask notification --- app/models/article.rb | 7 + .../welcome_notification/generator.rb | 47 +++++-- db/seeds.rb | 5 +- spec/factories/broadcasts.rb | 18 +++ .../welcome_notification/generator_spec.rb | 122 ++++++++++++------ 5 files changed, 152 insertions(+), 47 deletions(-) diff --git a/app/models/article.rb b/app/models/article.rb index 1c171e06b..a00eef547 100644 --- a/app/models/article.rb +++ b/app/models/article.rb @@ -97,6 +97,13 @@ class Article < ApplicationRecord tagged_with(tag_name) } + scope :user_published_with, lambda { |user_id, tag_name| + published. + where(user_id: user_id). + order(published_at: :desc). + tagged_with(tag_name) + } + scope :cached_tagged_with, ->(tag) { where("cached_tag_list ~* ?", "^#{tag},| #{tag},|, #{tag}$|^#{tag}$") } scope :cached_tagged_by_approval_with, ->(tag) { cached_tagged_with(tag).where(approved: true) } diff --git a/app/services/broadcasts/welcome_notification/generator.rb b/app/services/broadcasts/welcome_notification/generator.rb index 6de98d020..f415d71d8 100644 --- a/app/services/broadcasts/welcome_notification/generator.rb +++ b/app/services/broadcasts/welcome_notification/generator.rb @@ -16,8 +16,9 @@ module Broadcasts send_welcome_notification unless notification_enqueued send_authentication_notification unless notification_enqueued - send_ux_customization_notification unless notification_enqueued send_feed_customization_notification unless notification_enqueued + send_ux_customization_notification unless notification_enqueued + send_discuss_and_ask_notification unless notification_enqueued end private @@ -38,6 +39,12 @@ module Broadcasts @notification_enqueued = true end + def send_feed_customization_notification + return if user_is_following_tags? || received_notification?(customize_feed_broadcast) || user.created_at > 3.days.ago + + Notification.send_welcome_notification(user.id, customize_feed_broadcast.id) + end + def send_ux_customization_notification return if received_notification?(customize_ux_broadcast) || user.created_at > 5.days.ago @@ -45,10 +52,11 @@ module Broadcasts @notification_enqueued = true end - def send_feed_customization_notification - return if user_is_following_tags? || received_notification?(customize_feed_broadcast) || user.created_at > 3.days.ago + def send_discuss_and_ask_notification + return if (asked_a_question && started_a_discussion) || received_notification?(discuss_and_ask_broadcast) || user.created_at > 6.days.ago - Notification.send_welcome_notification(user.id, customize_feed_broadcast.id) + Notification.send_welcome_notification(user.id, discuss_and_ask_broadcast.id) + @notification_enqueued = true end def received_notification?(broadcast) @@ -80,20 +88,43 @@ module Broadcasts @customize_feed_broadcast ||= Broadcast.find_by(title: "Welcome Notification: customize_feed") end - def identities - @identities ||= user.identities.where(provider: SiteConfig.authentication_providers) - end - def authentication_broadcast @authentication_broadcast ||= find_auth_broadcast end + def discuss_and_ask_broadcast + @discuss_and_ask_broadcast ||= find_discuss_ask_broadcast + end + + def identities + @identities ||= user.identities.where(provider: SiteConfig.authentication_providers) + end + def find_auth_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 + + def find_discuss_ask_broadcast + type = if !asked_a_question && started_a_discussion + "ask_question" + elsif !started_a_discussion && asked_a_question + "start_discussion" + else + "discuss_and_ask" + end + Broadcast.find_by(title: "Welcome Notification: #{type}") + end + + def asked_a_question + @asked_a_question ||= Article.user_published_with(user.id, "explainlikeimfive").any? + end + + def started_a_discussion + @started_a_discussion ||= Article.user_published_with(user.id, "discuss").any? + end end end end diff --git a/db/seeds.rb b/db/seeds.rb index 9bc6f840c..b321ad678 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -251,7 +251,10 @@ broadcast_messages = { twitter_connect: "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. 🤖", github_connect: "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. 🤖", customize_feed: "Hi, it's me again! 👋 Now that you're a part of the DEV community, let's focus on personalizing your content. You can start by following some tags to help customize your feed! 🎉", - customize_experience: "Sloan here! 👋 Did you know that that you can customize your DEV experience? Try changing your font and theme and find the best style for you!" + customize_experience: "Sloan here! 👋 Did you know that that you can customize your DEV experience? Try changing your font and theme and find the best style for you!", + start_discussion: "Sloan here! 👋 I noticed that you haven't started a discussion yet. Starting a discussion is easy to do; just click on 'Write a Post' in the sidebar of the tag page to get started!", + ask_question: "Sloan here! 👋 I noticed that you haven't asked a question yet. Asking a question is easy to do; just click on 'Write a Post' in the sidebar of the tag page to get started!", + discuss_and_ask: "Sloan here! 👋 I noticed that you haven't asked a question or started a discussion yet. It's easy to do both of these; just click on 'Write a Post' in the sidebar of the tag page to get started!" } broadcast_messages.each do |type, message| diff --git a/spec/factories/broadcasts.rb b/spec/factories/broadcasts.rb index 7bb4557b0..6cb761ad6 100644 --- a/spec/factories/broadcasts.rb +++ b/spec/factories/broadcasts.rb @@ -32,6 +32,24 @@ FactoryBot.define do processed_html { "Hi, it's me again! 👋 Now that you're a part of the DEV community, let's focus on personalizing your content. You can start by following some tags to help customize your feed! 🎉" } end + factory :start_discussion_broadcast do + title { "Welcome Notification: start_discussion" } + type_of { "Welcome" } + processed_html { "Sloan here! 👋 I noticed that you haven't started a discussion yet. Starting a discussion is easy to do; just click on 'Write a Post' in the sidebar of the tag page to get started!" } + end + + factory :ask_question_broadcast do + title { "Welcome Notification: ask_question" } + type_of { "Welcome" } + processed_html { "Sloan here! 👋 I noticed that you haven't asked a question yet. Asking a question is easy to do; just click on 'Write a Post' in the sidebar of the tag page to get started!" } + end + + factory :discuss_and_ask_broadcast do + title { "Welcome Notification: discuss_and_ask" } + type_of { "Welcome" } + processed_html { "Sloan here! 👋 I noticed that you haven't asked a question or started a discussion yet. It's easy to do both of these; just click on 'Write a Post' in the sidebar of the tag page to get started!" } + end + # TODO: [@thepracticaldev/delightful] Remove onboarding factory once welcome notifications are live. factory :onboarding_broadcast do title { "Welcome Notification" } diff --git a/spec/services/broadcasts/welcome_notification/generator_spec.rb b/spec/services/broadcasts/welcome_notification/generator_spec.rb index af8529ab3..293699be2 100644 --- a/spec/services/broadcasts/welcome_notification/generator_spec.rb +++ b/spec/services/broadcasts/welcome_notification/generator_spec.rb @@ -4,10 +4,6 @@ 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) } - let!(:customize_ux_broadcast) { create(:customize_ux_broadcast) } - let!(:customize_feed_broadcast) { create(:customize_feed_broadcast) } before do allow(Notification).to receive(:send_welcome_notification).and_call_original @@ -68,6 +64,9 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do end describe "#send_authentication_notification" do + let!(:twitter_connect_broadcast) { create(:twitter_connect_broadcast) } + let!(:github_connect_broadcast) { create(:github_connect_broadcast) } + 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) } @@ -109,37 +108,9 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do end end - describe "#send_ux_customization_notification" do - # Ensure that the user is already authenticated with all providers and has commented on the - # welcome thread so that we can actually trigger just the customize notification only. - let(:user) { create(:user, :with_identity, identities: %w[twitter github], created_at: 5.days.ago) } - - before do - create(:comment, commentable: welcome_thread, commentable_type: "Article", user: user) - end - - 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_ux_customization_notification) } - expect(Notification).not_to have_received(:send_welcome_notification) - 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_ux_customization_notification) } - expect(user.notifications.count).to eq(1) - expect(user.notifications.first.notifiable).to eq(customize_ux_broadcast) - end - - it "does not send duplicate notifications" do - 2.times do - sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_ux_customization_notification) } - end - expect(user.notifications.count).to eq(1) - end - end - describe "#send_feed_customization_notification" do let!(:user) { create(:user, :with_identity, identities: %w[twitter github], created_at: 3.days.ago) } + let!(:customize_feed_broadcast) { create(:customize_feed_broadcast) } it "does not send a notification to a newly-created user" do user.update!(created_at: Time.zone.now) @@ -167,10 +138,85 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do 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" + describe "#send_ux_customization_notification" do + let!(:user) { create(:user, :with_identity, identities: %w[twitter github], created_at: 5.days.ago) } + let!(:customize_ux_broadcast) { create(:customize_ux_broadcast) } + + 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_ux_customization_notification) } + expect(Notification).not_to have_received(:send_welcome_notification) + 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_ux_customization_notification) } + expect(user.notifications.count).to eq(1) + expect(user.notifications.first.notifiable).to eq(customize_ux_broadcast) + end + + it "does not send duplicate notifications" do + 2.times do + sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_ux_customization_notification) } + end + expect(user.notifications.count).to eq(1) + end + end + + describe "#send_discuss_and_ask_notification" do + let!(:user) { create(:user, :with_identity, identities: %w[twitter github], created_at: 6.days.ago) } + let!(:ask_question_broadcast) { create(:ask_question_broadcast) } + let!(:start_discussion_broadcast) { create(:start_discussion_broadcast) } + let!(:discuss_and_ask_broadcast) { create(:discuss_and_ask_broadcast) } + + context "with a user who has asked a question" do + it "generates the correct broadcast type and sends the notification to the user" do + create(:article, tags: "explainlikeimfive", user: user) + + sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_discuss_and_ask_notification) } + expect(user.notifications.count).to eq(1) + expect(user.notifications.first.notifiable).to eq(start_discussion_broadcast) + end + end + + context "with a user who has started a discussion" do + it "generates the correct broadcast type and sends the notification to the user" do + create(:article, tags: "discuss", user: user) + + sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_discuss_and_ask_notification) } + expect(user.notifications.count).to eq(1) + expect(user.notifications.first.notifiable).to eq(ask_question_broadcast) + end + end + + context "with a user who has neither asked a question and started a discussion" do + 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_discuss_and_ask_notification) } + expect(user.notifications.count).to eq(1) + expect(user.notifications.first.notifiable).to eq(discuss_and_ask_broadcast) + end + end + + context "with a user who has asked a question and started a discussion" do + it "does not send a notification" do + create(:article, tags: "discuss", user: user) + create(:article, tags: "explainlikeimfive", user: user) + + sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_discuss_and_ask_notification) } + expect(Notification).not_to have_received(:send_welcome_notification) + end + end + + 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_discuss_and_ask_notification) } + expect(Notification).not_to have_received(:send_welcome_notification) + end + + it "does not send duplicate notifications" do + 2.times do + sidekiq_perform_enqueued_jobs { described_class.new(user.id).send(:send_discuss_and_ask_notification) } + end + expect(user.notifications.count).to eq(1) + end end end