New Feature: Block Users (#4411)

* Prevent need for eager loading

* Add initial implementation of user block

* Remove debugger oops

* Add index and foreign keys for user_block table

* Use private method instead of exists

* Move channel handling logic to service object

* Update styling a bit

* Use profileDropdown file for future proofing

* Remove commented out code

* Render json: { result } for all endpoints

* Add statuses for sad paths

* Add better sad path handling in the JS

* Remove accidentally committed spec file

* Don't wait for DOM to load block button

* Use equality for accuracy in slug query

* Add status code for unauthorized requests

* Add missing comma sigh
This commit is contained in:
Andy Zhao 2019-10-23 17:14:28 -04:00 committed by Ben Halpern
parent 789149e01e
commit 73caa9a864
22 changed files with 501 additions and 3 deletions

View file

@ -517,6 +517,18 @@
&.showing {
display: block;
}
button {
font-size: 0.75em;
color: $black;
font-weight: bold;
width: 100%;
text-align: left;
padding-left: 10px;
display: block;
border: none;
text-overflow: ellipsis;
overflow: hidden;
}
}
}
}

View file

@ -0,0 +1,58 @@
class UserBlocksController < ApplicationController
before_action :check_sign_in_status
after_action :verify_authorized
def show
skip_authorization
if current_user.blocking?(params[:blocked_id].to_i)
render json: { result: "blocking" }
else
render json: { result: "not-blocking" }
end
end
def create
authorize UserBlock
@user_block = UserBlock.new(permitted_attributes(UserBlock))
@user_block.blocker_id = current_user.id
@user_block.config = "default"
if @user_block.save
UserBlocks::ChannelHandler.new(@user_block).block_chat_channel
current_user.stop_following(@user_block.blocked)
@user_block.blocked.stop_following(current_user)
render json: { result: "blocked" }
else
render json: { error: @user_block.errors.full_messages.join(", "), status: 422 }, status: :unprocessable_entity
end
end
def destroy
if current_user.blocking_others_count.zero?
skip_authorization
render json: { result: "not-blocking-anyone" }
return
end
@user_block = UserBlock.find_by(blocked_id: permitted_attributes(UserBlock)[:blocked_id], blocker: current_user)
authorize @user_block
if @user_block.destroy
UserBlocks::ChannelHandler.new(@user_block).unblock_chat_channel
render json: { result: "unblocked" }
else
render json: { error: @user_block.errors.full_messages.join(", "), status: 422 }, status: :unprocessable_entity
end
end
private
def check_sign_in_status
unless current_user
skip_authorization
render json: { result: "not-logged-in", status: 401 }, status: :unauthorized
return
end
end
end

View file

@ -0,0 +1,7 @@
import initBlock from '../profileDropdown/blockButton';
window.InstantClick.on('change', () => {
initBlock();
});
initBlock();

View file

@ -0,0 +1,90 @@
export default function initBlock() {
const blockButton = document.getElementById(
'user-profile-dropdownmenu-block-button',
);
const { profileUserId } = blockButton.dataset;
function unblock() {
fetch(`/user_blocks/${profileUserId}`, {
method: 'DELETE',
headers: {
Accept: 'application/json',
'X-CSRF-Token': window.csrfToken,
'Content-Type': 'application/json',
},
body: JSON.stringify({
user_block: {
blocked_id: profileUserId,
},
}),
})
.then(response => response.json())
.then(response => {
if (response.result === 'unblocked') {
blockButton.innerText = 'Block';
blockButton.addEventListener('click', block, { once: true });
} else if (response.status === 422) {
window.alert(`Something went wrong: ${e} -- Please refresh the page to try again.`)
}
})
.catch(e => {
window.alert(`Something went wrong: ${e}. -- Please refresh the page to try again.`);
});
}
function block() {
const confirmBlock = window.confirm(
`Are you sure you want to block this person? This will:
- prevent them from commenting on your posts
- block all notifications from them
- prevent them from messaging you via DEV Connect`,
);
if (confirmBlock) {
fetch(`/user_blocks/${profileUserId}`, {
method: 'POST',
headers: {
Accept: 'application/json',
'X-CSRF-Token': window.csrfToken,
'Content-Type': 'application/json',
},
body: JSON.stringify({
user_block: {
blocked_id: profileUserId,
},
}),
})
.then(response => response.json())
.then(response => {
if (response.result === 'blocked') {
blockButton.innerText = 'Unblock';
blockButton.addEventListener('click', unblock, { once: true });
} else if (response.status === 422) {
window.alert(`Something went wrong: ${e}. -- Please refresh the page to try again.`)
}
})
.catch(e => {
window.alert(`Something went wrong: ${e}. -- Please refresh the page to try again.`);
});
} else {
blockButton.addEventListener('click', block, { once: true });
}
}
// userData() is a global function
const currentUserId = userData().id;
if (currentUserId === parseInt(profileUserId, 10)) {
blockButton.style.display = 'none';
} else {
fetch(`/user_blocks/${profileUserId}`)
.then(response => response.json())
.then(response => {
if (response.result === 'blocking') {
blockButton.innerText = 'Unblock';
blockButton.addEventListener('click', unblock, { once: true });
} else {
blockButton.addEventListener('click', block, { once: true });
}
});
}
}

View file

@ -77,6 +77,7 @@ class Message < ApplicationRecord
return if channel.open?
errors.add(:base, "You are not a participant of this chat channel.") unless channel.has_member?(user)
errors.add(:base, "Something went wrong") if channel.status == "blocked"
end
def rich_link_article(link)

View file

@ -29,6 +29,7 @@ class Notification < ApplicationRecord
class << self
def send_new_follower_notification(follow, is_read = false)
return unless Follow.need_new_follower_notification_for?(follow.followable_type)
return if follow.followable_type == "User" && UserBlock.blocking?(follow.followable_id, follow.follower_id)
follow_data = follow.attributes.slice("follower_id", "followable_id", "followable_type").symbolize_keys
Notifications::NewFollowerJob.perform_later(follow_data, is_read)
@ -36,6 +37,7 @@ class Notification < ApplicationRecord
def send_new_follower_notification_without_delay(follow, is_read = false)
return unless Follow.need_new_follower_notification_for?(follow.followable_type)
return if follow.followable_type == "User" && UserBlock.blocking?(follow.followable_id, follow.follower_id)
follow_data = follow.attributes.slice("follower_id", "followable_id", "followable_type").symbolize_keys
Notifications::NewFollowerJob.perform_now(follow_data, is_read)
@ -47,12 +49,14 @@ class Notification < ApplicationRecord
def send_new_comment_notifications(comment)
return if comment.commentable_type == "PodcastEpisode"
return if UserBlock.blocking?(comment.commentable.user_id, comment.user_id)
Notifications::NewCommentJob.perform_later(comment.id)
end
def send_new_comment_notifications_without_delay(comment)
return if comment.commentable_type == "PodcastEpisode"
return if UserBlock.blocking?(comment.commentable.user_id, comment.user_id)
Notifications::NewCommentJob.perform_now(comment.id)
end
@ -67,17 +71,21 @@ class Notification < ApplicationRecord
def send_reaction_notification(reaction, receiver)
return if reaction.skip_notification_for?(receiver)
return if UserBlock.blocking?(receiver, reaction.user_id)
Notifications::NewReactionJob.perform_later(*reaction_notification_attributes(reaction, receiver))
end
def send_reaction_notification_without_delay(reaction, receiver)
return if reaction.skip_notification_for?(receiver)
return if UserBlock.blocking?(receiver, reaction.user_id)
Notifications::NewReactionJob.perform_now(*reaction_notification_attributes(reaction, receiver))
end
def send_mention_notification(mention)
return if mention.mentionable_type == "User" && UserBlock.blocking?(mention.mentionable_id, mention.user_id)
Notifications::MentionJob.perform_later(mention.id)
end
@ -86,6 +94,9 @@ class Notification < ApplicationRecord
end
def send_moderation_notification(notifiable)
# TODO: make this work for articles in the future. only works for comments right now
return if UserBlock.blocking?(notifiable.commentable.user_id, notifiable.user_id)
Notifications::ModerationNotificationJob.perform_later(notifiable.id)
end

View file

@ -49,6 +49,7 @@ class User < ApplicationRecord
has_many :access_grants, class_name: "Doorkeeper::AccessGrant", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all
has_many :access_tokens, class_name: "Doorkeeper::AccessToken", foreign_key: :resource_owner_id, inverse_of: :resource_owner, dependent: :delete_all
has_many :webhook_endpoints, class_name: "Webhook::Endpoint", foreign_key: :user_id, inverse_of: :user, dependent: :delete_all
has_many :user_blocks
has_one :pro_membership, dependent: :destroy
mount_uploader :profile_image, ProfileImageUploader
@ -376,6 +377,24 @@ class User < ApplicationRecord
OrganizationMembership.exists?(user: user, organization: organization, type_of_user: "admin")
end
def block; end
def all_blocking
UserBlock.where(blocker_id: id)
end
def all_blocked_by
UserBlock.where(blocked_id: id)
end
def blocking?(blocked_id)
UserBlock.blocking?(id, blocked_id)
end
def blocked_by?(blocker_id)
UserBlock.blocking?(blocker_id, id)
end
def unique_including_orgs_and_podcasts
errors.add(:username, "is taken.") if Organization.find_by(slug: username) || Podcast.find_by(slug: username) || Page.find_by(slug: username)
end

21
app/models/user_block.rb Normal file
View file

@ -0,0 +1,21 @@
class UserBlock < ApplicationRecord
belongs_to :blocker, foreign_key: "blocker_id", class_name: "User", inverse_of: :user_blocks
belongs_to :blocked, foreign_key: "blocked_id", class_name: "User", inverse_of: :user_blocks
validates :blocked_id, :blocker_id, :config, presence: true
validates :blocked_id, uniqueness: { scope: %i[blocker_id] }
validates :config, inclusion: { in: %w[default] }
validate :blocker_cannot_be_same_as_blocked
counter_culture :blocker, column_name: "blocking_others_count"
counter_culture :blocked, column_name: "blocked_by_count"
class << self
def blocking?(blocker_id, blocked_id)
exists?(blocker_id: blocker_id, blocked_id: blocked_id)
end
end
def blocker_cannot_be_same_as_blocked
errors.add(:blocker_id, "can't be the same as the blocked_id") if blocker_id == blocked_id
end
end

View file

@ -4,7 +4,7 @@ class CommentPolicy < ApplicationPolicy
end
def create?
!user_is_banned? && !user_is_comment_banned?
!user_is_banned? && !user_is_comment_banned? && !user_is_blocked?
end
def update?
@ -48,4 +48,10 @@ class CommentPolicy < ApplicationPolicy
def user_is_author?
record.user_id == user.id
end
def user_is_blocked?
return false if user.blocked_by_count.zero?
UserBlock.blocking?(record.commentable.user_id, user.id)
end
end

View file

@ -0,0 +1,13 @@
class UserBlockPolicy < ApplicationPolicy
def create?
!user_is_banned?
end
def destroy?
!user_is_banned?
end
def permitted_attributes
%i[id blocked_id]
end
end

View file

@ -0,0 +1,38 @@
module UserBlocks
class ChannelHandler
attr_reader :user_block
def initialize(user_block)
@user_block = user_block
end
def get_potential_chat_channel
blocked_user = User.select(:id, :username).find(user_block.blocked_id)
blocker = User.select(:id, :username).find(user_block.blocker_id)
potential_slugs = ["#{blocked_user.username}/#{blocker.username}", "#{blocker.username}/#{blocked_user.username}"]
blocker.chat_channels.where(slug: potential_slugs, channel_type: "direct").first
end
def block_chat_channel
chat_channel = get_potential_chat_channel
return if chat_channel.blank?
chat_channel.update(status: "blocked")
chat_channel.chat_channel_memberships.each do |membership|
membership.update(status: "left_channel")
membership.remove_from_index!
end
end
def unblock_chat_channel
chat_channel = get_potential_chat_channel
return if chat_channel.blank?
chat_channel.update(status: "active")
chat_channel.chat_channel_memberships.each do |membership|
membership.update(status: "active")
membership.index!
end
end
end
end

View file

@ -1,5 +1,5 @@
json.messages @chat_channel.messages.order("created_at DESC").limit(50).reverse do |message|
json.user_id message.user.id
json.user_id message.user_id
json.username message.user.username
json.profile_image_url ProfileImage.new(message.user).get(90)
json.message message.message_html

View file

@ -150,6 +150,8 @@
<%= image_tag("three-dots.svg", class: "dropdown-icon", alt: "Toggle dropdown menu") %>
</button>
<div id="user-profile-dropdownmenu" class="profile-dropdownmenu">
<button id="user-profile-dropdownmenu-block-button" data-profile-user-id="<%= @user.id %>" class="block-button" type="button">Block @<%= @user.username %></button>
<%= javascript_pack_tag "profileDropdown", defer: true %>
<a href="/report-abuse?url=https://dev.to/<%= @user.username %>">Report Abuse</a>
</div>
</div>

View file

@ -178,6 +178,9 @@ Rails.application.routes.draw do
resources :display_ad_events, only: [:create]
resources :badges, only: [:index]
resource :pro_membership, path: :pro, only: %i[show create update]
resources :user_blocks, param: :blocked_id, only: %i[show destroy] do
post ":blocked_id", on: :collection, to: "user_blocks#create"
end
resolve("ProMembership") { [:pro_membership] } # see https://guides.rubyonrails.org/routing.html#using-resolve
get "/chat_channel_memberships/find_by_chat_channel_id" => "chat_channel_memberships#find_by_chat_channel_id"

View file

@ -0,0 +1,15 @@
class CreateUserBlocks < ActiveRecord::Migration[5.2]
def change
create_table :user_blocks do |t|
t.bigint :blocked_id, null: false
t.bigint :blocker_id, null: false
t.string :config, null: false, default: "default"
t.timestamps null: false
end
add_index :user_blocks, %i[blocked_id blocker_id], unique: true
add_foreign_key :user_blocks, :users, column: :blocker_id
add_foreign_key :user_blocks, :users, column: :blocked_id
end
end

View file

@ -0,0 +1,6 @@
class AddBlockedCountToUsers < ActiveRecord::Migration[5.2]
def change
add_column :users, :blocked_by_count, :bigint, null: false, default: 0
add_column :users, :blocking_others_count, :bigint, null: false, default: 0
end
end

View file

@ -12,7 +12,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_09_18_104106) do
ActiveRecord::Schema.define(version: 2019_09_25_193205) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@ -1015,6 +1015,15 @@ ActiveRecord::Schema.define(version: 2019_09_18_104106) do
t.boolean "user_is_verified"
end
create_table "user_blocks", force: :cascade do |t|
t.bigint "blocked_id", null: false
t.bigint "blocker_id", null: false
t.string "config", default: "default", null: false
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["blocked_id", "blocker_id"], name: "index_user_blocks_on_blocked_id_and_blocker_id", unique: true
end
create_table "users", id: :serial, force: :cascade do |t|
t.integer "articles_count", default: 0, null: false
t.string "available_for"
@ -1022,6 +1031,8 @@ ActiveRecord::Schema.define(version: 2019_09_18_104106) do
t.text "base_cover_letter"
t.string "behance_url"
t.string "bg_color_hex"
t.bigint "blocked_by_count", default: 0, null: false
t.bigint "blocking_others_count", default: 0, null: false
t.boolean "checked_code_of_conduct", default: false
t.boolean "checked_terms_and_conditions", default: false
t.integer "comments_count", default: 0, null: false
@ -1204,6 +1215,8 @@ ActiveRecord::Schema.define(version: 2019_09_18_104106) do
add_foreign_key "push_notification_subscriptions", "users"
add_foreign_key "sponsorships", "organizations"
add_foreign_key "sponsorships", "users"
add_foreign_key "user_blocks", "users", column: "blocked_id"
add_foreign_key "user_blocks", "users", column: "blocker_id"
add_foreign_key "webhook_endpoints", "oauth_applications"
add_foreign_key "webhook_endpoints", "users"
end

View file

@ -0,0 +1,7 @@
FactoryBot.define do
factory :user_block do
blocker { user }
blocked { user }
config { "default" }
end
end

View file

@ -0,0 +1,16 @@
require "rails_helper"
RSpec.describe UserBlock, type: :model do
let(:blocker) { create(:user) }
let(:blocked) { create(:user) }
describe "validations" do
it { is_expected.to validate_inclusion_of(:config).in_array(%w[default]) }
it "prevents the blocker from blocking itself" do
user_block = UserBlock.new(blocker_id: 1, blocked_id: 1, config: "default")
expect(user_block.valid?).to eq false
expect(user_block.errors.full_messages).to include "Blocker can't be the same as the blocked_id"
end
end
end

View file

@ -35,6 +35,17 @@ RSpec.describe "Messages", type: :request do
end
end
context "when user is blocked" do
before do
sign_in user
chat_channel.update(status: "blocked")
end
it "raises an error" do
expect { post "/messages", params: { message: new_message } }.to raise_error
end
end
# context "when Pusher isn't cooperating" do
# before do
# allow(Pusher).to receive(:trigger).and_raise(Pusher::Error)

View file

@ -0,0 +1,105 @@
require "rails_helper"
RSpec.describe "UserBlock", type: :request do
let(:blocker) { create(:user) }
let(:blocked) { create(:user) }
before { sign_in blocker }
describe "GET /user_blocks/:blocked_id or #show" do
it "rejects when not-logged-in" do
sign_out(blocker)
get "/user_blocks/#{blocked.id}"
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(response.status).to eq 401
expect(json_response["result"]).to eq "not-logged-in"
end
it "returns 'not-blocking' when the user is not blocked" do
get "/user_blocks/#{blocked.id}"
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(json_response["result"]).to eq "not-blocking"
end
it "returns 'blocking' when blocking" do
create(:user_block, blocker: blocker, blocked: blocked)
get "/user_blocks/#{blocked.id}"
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(json_response["result"]).to eq "blocking"
end
end
describe "POST /user_blocks/:blocked_id or #create" do
it "renders 'not-logged-in' when not logged in" do
sign_out blocker
post "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(response.status).to eq 401
expect(json_response["result"]).to eq "not-logged-in"
end
it "creates the correct user_block" do
post "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
expect(UserBlock.count).to eq 1
expect(UserBlock.first.blocker_id).to eq blocker.id
expect(UserBlock.first.blocked_id).to eq blocked.id
end
it "returns a JSON response with blocked" do
post "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(json_response["result"]).to eq "blocked"
end
it "blocks the potential chat channel" do
chat_channel = create(:chat_channel, channel_type: "direct", slug: "#{blocker.username}/#{blocked.username}", status: "active")
create(:chat_channel_membership, chat_channel_id: chat_channel.id, user_id: blocker.id)
create(:chat_channel_membership, chat_channel_id: chat_channel.id, user_id: blocked.id)
post "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
expect(chat_channel.reload.status).to eq "blocked"
end
end
describe "DELETE /user_blocks/:blocked_id or #delete" do
before do
create(:user_block, blocker: blocker, blocked: blocked)
blocker.update(blocking_others_count: 1)
end
it "renders 'not-logged-in' when not logged in" do
sign_out blocker
delete "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(response.status).to eq 401
expect(json_response["result"]).to eq "not-logged-in"
end
it "renders 'not-blocking-anyone' if there is no one to unblock" do
UserBlock.delete_all
blocker.update(blocking_others_count: 0)
delete "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
json_response = JSON.parse(response.body)
expect(response.content_type).to eq "application/json"
expect(json_response["result"]).to eq "not-blocking-anyone"
end
it "removes the correct user_block" do
delete "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
expect(blocker.blocking?(blocked)).to eq false
end
it "unblocks the direct chat channel" do
chat_channel = create(:chat_channel, channel_type: "direct", slug: "#{blocker.username}/#{blocked.username}", status: "blocked")
create(:chat_channel_membership, chat_channel_id: chat_channel.id, user_id: blocker.id)
create(:chat_channel_membership, chat_channel_id: chat_channel.id, user_id: blocked.id)
delete "/user_blocks/#{blocked.id}", params: { user_block: { blocked_id: blocked.id } }
expect(chat_channel.reload.status).to eq "active"
end
end
end

View file

@ -0,0 +1,44 @@
require "rails_helper"
RSpec.describe UserBlocks::ChannelHandler, type: :service do
before do
create_list(:user, 2)
blocker = User.first
blocked = User.second
chat_channel = create(:chat_channel, channel_type: "direct", status: "active", slug: "#{blocker.username}/#{blocked.username}")
create(:chat_channel_membership, user: blocker, chat_channel: chat_channel)
create(:chat_channel_membership, user: blocked, chat_channel: chat_channel)
create(:user_block, blocker: blocker, blocked: blocked)
end
describe ".get_potential_chat_channel" do
it "returns the correct channel" do
expect(described_class.new(UserBlock.first).get_potential_chat_channel).to eq(ChatChannel.first)
end
end
describe ".block_chat_channel" do
it "updates the chat channel status to blocked" do
described_class.new(UserBlock.first).block_chat_channel
expect(ChatChannel.first.status).to eq "blocked"
end
it "removes the related chat channel memberships" do
expect { described_class.new(UserBlock.first).block_chat_channel }.
to change(ChatChannelMembership.where(status: "active"), :count).to 0
end
end
describe ".unblock_chat_channel" do
it "updates the chat channel status to be active" do
described_class.new(UserBlock.first).unblock_chat_channel
expect(ChatChannel.first.status).to eq "active"
end
it "updates the related chat channel memberships" do
ChatChannelMembership.update_all(status: "left-channel")
expect { described_class.new(UserBlock.first).unblock_chat_channel }.
to change(ChatChannelMembership.where(status: "left-channel"), :count).to 0
end
end
end