maverick/docbrown
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Allow users to opt-out of welcome notifications (#6852) [deploy]

Browse source 
* Update some test descriptions in notifications_spec

The descriptions of these tests didn't actually match the test's assertions!

* Add welcome_notifications column to users table

Also ensures that welcome_notifications cannot be set to `nil`.

* Show an opt-out message for Welcome broadcast notifications

* Add opt-out link to notifications + welcome notification box to settings

* Allow welcome notification subscription to be updated

* Add tests for unsubscribed to welcome notification users

Also refactor generator specs a bit.

* Fix up paths, copy, tests

Use rails path helpers instead of hardcoding paths. Improve copy in a few locations.
This commit is contained in:
Vaidehi Joshi 2020-03-26 11:26:19 -07:00 • committed by GitHub
parent d20a50aa2b
commit c12cea9b02
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 73 additions and 26 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

7
app/assets/stylesheets/notifications.scss
View file

@ -52,6 +52,13 @@
font-weight: 500;
}
}
&.broadcast-content {
.opt-out {
padding-top: 24px;
font-size: 12px;
font-style: italic;
}
}
&.reaction-content {
width: calc(100% - 145px);
}

1
app/policies/user_policy.rb
View file

@ -44,6 +44,7 @@ class UserPolicy < ApplicationPolicy
medium_url
mobile_comment_notifications
mod_roundrobin_notifications
welcome_notifications
mostly_work_with
name
password

2
app/services/broadcasts/welcome_notification/generator.rb
View file

@ -11,6 +11,8 @@ module Broadcasts
end
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)

3
app/services/notifications/welcome_notification/send.rb
View file

@ -19,7 +19,8 @@ module Notifications
user: user_data(mascot_account),
broadcast: {
title: welcome_broadcast.title,
processed_html: welcome_broadcast.processed_html
processed_html: welcome_broadcast.processed_html,
type_of: welcome_broadcast.type_of
}
}
Notification.create!(

4
app/views/notifications/_broadcast.html.erb
View file

@ -8,5 +8,9 @@
</a>
<div class="content notification-content broadcast-content">
<%= json_data["broadcast"]["processed_html"].html_safe %>
<% if notification.json_data['broadcast']['type_of'] == "Welcome" %>
<div class="opt-out"><a href="<%= user_settings_path(tab: :notifications) %>">Go to your settings to manage your notification settings.</a></<div>
<% end %>
</div>
<% end %>

12
app/views/users/_notifications.html.erb
View file

@ -49,7 +49,7 @@
</div>
<div class="sub-field">
<%= f.check_box :email_unread_notifications %>
<%= f.label :email_unread_notifications, "Send me occasional reminders that I have unread notifications on dev.to" %>
<%= f.label :email_unread_notifications, "Send me occasional reminders that I have unread notifications on DEV" %>
</div>
</div>
<h3>Mobile Notification Settings (Beta)</h3>
@ -62,6 +62,16 @@
<%= f.label :mobile_comment_notifications, "Notify me when someone replies to me in a comment thread" %>
</div>
</div>
<h3>Platform Notification Settings</h3>
<p>
<em>Notifications that only appear on the <a href="<%= notifications_path %>">notifications page</a>.</em>
</p>
<div class="checkbox-field">
<div class="sub-field">
<%= f.check_box :welcome_notifications %>
<%= f.label :welcome_notifications, "Send me occasional tips on how to enhance my DEV experience" %>
</div>
</div>
<% if current_user.trusted %>
<h3>Mod Notification Settings</h3>
<div class="checkbox-field">

5
db/migrate/20200324170819_add_welcome_notifications_to_users.rb Normal file
View file

@ -0,0 +1,5 @@
class AddWelcomeNotificationsToUsers < ActiveRecord::Migration[5.2]
def change
add_column :users, :welcome_notifications, :boolean, default: true, null: false
end
end

3
db/schema.rb
View file

@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2020_03_24_113133) do
ActiveRecord::Schema.define(version: 2020_03_24_170819) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -1263,6 +1263,7 @@ ActiveRecord::Schema.define(version: 2020_03_24_113133) do
t.datetime "updated_at", null: false
t.string "username"
t.string "website_url"
t.boolean "welcome_notifications", default: true, null: false
t.datetime "workshop_expiration"
t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true
t.index ["created_at"], name: "index_users_on_created_at"

12
spec/requests/notifications_spec.rb
View file

@ -538,15 +538,15 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
it "renders the proper message" do
it "does not render the notification message" do
expect(response.body).not_to include "Since they are new to the community, could you leave a nice reply"
end
it "renders the article's path" do
it "does not render the article's path" do
expect(response.body).not_to include article.path
end
it "renders the comment's processed HTML" do
it "does not render the comment's processed HTML" do
expect(response.body).not_to include comment.processed_html
end
end
@ -566,15 +566,15 @@ RSpec.describe "NotificationsIndex", type: :request do
get "/notifications"
end
it "renders the proper message" do
it "does not render the proper message" do
expect(response.body).not_to include "Since they are new to the community, could you leave a nice reply"
end
it "renders the article's path" do
it "does not render the article's path" do
expect(response.body).not_to include article.path
end
it "renders the comment's processed HTML" do
it "does not render the comment's processed HTML" do
expect(response.body).not_to include comment.processed_html
end
end

8
spec/requests/user/user_settings_spec.rb
View file

@ -98,6 +98,14 @@ RSpec.describe "UserSettings", type: :request do
expect(user.reload.mod_roundrobin_notifications).to be(false)
end
it "can toggle welcome notifications" do
put "/users/#{user.id}", params: { user: { tab: "notifications", welcome_notifications: 0 } }
expect(user.reload.welcome_notifications).to be(false)
put "/users/#{user.id}", params: { user: { tab: "notifications", welcome_notifications: 1 } }
expect(user.reload.welcome_notifications).to be(true)
end
it "updates username to too short username" do
put "/users/#{user.id}", params: { user: { tab: "profile", username: "h" } }
expect(response.body).to include("Username is too short")

42
spec/services/broadcasts/welcome_notification/generator_spec.rb
View file

@ -1,9 +1,19 @@
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
describe "::call" do
let(:receiving_user) { create(:user) }
let(:user) { create(:user) }
let(:unsubscribed_user) { create(:user, welcome_notifications: false) }
let(:mascot_account) { create(:user) }
let!(:welcome_broadcast) { create(:welcome_broadcast, :active) }
@ -24,32 +34,30 @@ RSpec.describe Broadcasts::WelcomeNotification::Generator, type: :service do
end
context "when sending a welcome_thread notification" do
before 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)
end
it "generates the correct broadcast type and sends the notification to the user", :aggregate_failures do
expect(receiving_user.notifications.count).to eq(0)
sidekiq_perform_enqueued_jobs { described_class.call(receiving_user.id) }
expect do
sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
end.to change(user.notifications, :count).by(1)
expect(receiving_user.notifications.count).to eq(1)
expect(receiving_user.notifications.first.notifiable).to eq(welcome_broadcast)
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
expect(user.notifications.count).to eq(0)
sidekiq_perform_enqueued_jobs { described_class.call(user.id) }
expect(user.notifications.count).to eq(0)
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 do
sidekiq_perform_enqueued_jobs { described_class.call(receiving_user.id) }
end
2.times { sidekiq_perform_enqueued_jobs { described_class.call(user.id) } }
expect(receiving_user.notifications.count).to eq(1)
expect(user.notifications.count).to eq(1)
end
it_behaves_like "unsubscribed from welcome notifications"
end
context "when sending a twitter_connect notification" do