From 9bdef4d4ae0b41612001a62c2409121b654bf71f Mon Sep 17 00:00:00 2001 From: Andy Zhao <17884966+Zhao-Andy@users.noreply.github.com> Date: Thu, 23 Jul 2020 09:49:54 -0400 Subject: [PATCH] [deploy] Update /ban for Connect (#9459) * Rough MVP of /ban v2 * Make snackbar dismissable * Make unban work * Clean up response for happy and sad paths * Fix tests * Fix policy spec --- app/controllers/chat_channels_controller.rb | 39 +++++++++++++-------- app/javascript/chat/chat.jsx | 20 ++++++----- app/models/role.rb | 1 + app/policies/chat_channel_policy.rb | 6 +++- spec/models/role_spec.rb | 2 +- spec/policies/chat_channel_policy_spec.rb | 2 +- spec/requests/chat_channels_spec.rb | 3 +- 7 files changed, 47 insertions(+), 26 deletions(-) diff --git a/app/controllers/chat_channels_controller.rb b/app/controllers/chat_channels_controller.rb index f93aff92c..89dc348ce 100644 --- a/app/controllers/chat_channels_controller.rb +++ b/app/controllers/chat_channels_controller.rb @@ -82,25 +82,36 @@ class ChatChannelsController < ApplicationController end def moderate - command = chat_channel_params[:command].split - case command[0] + chat_channel = ChatChannel.find_by(id: params[:id]) + authorize chat_channel + command, username = chat_channel_params[:command].split + case command when "/ban" - banned_user = User.find_by(username: command[1]) - if banned_user - banned_user.add_role :banned - banned_user.messages.delete_all - Pusher.trigger(@chat_channel.pusher_channels, "user-banned", { userId: banned_user.id }.to_json) - render json: { status: "success", message: "suspended!" }, status: :ok + user = User.find_by(username: username) + membership = user&.chat_channel_memberships&.find_by(chat_channel: chat_channel) + if user && membership + user.add_role :banned + user.messages.where(chat_channel: chat_channel).delete_all + membership.update(status: "removed_from_channel") + Pusher.trigger(chat_channel.pusher_channels, "user-banned", { userId: user.id }.to_json) + render json: { status: "moderation-success", message: "#{username} was suspended.", userId: user.id, + chatChannelId: chat_channel.id }, status: :ok else - render json: { status: "error", message: "username not found" }, status: :bad_request + render json: { + status: "error", + message: "Ban failed. user with username '#{username}' not found in this channel." + }, status: :bad_request end when "/unban" - banned_user = User.find_by(username: command[1]) - if banned_user - banned_user.remove_role :banned - render json: { status: "success", message: "unsuspended!" }, status: :ok + user = User.find_by(username: username) + if user + user.remove_role :banned + render json: { status: "moderation-success", message: "#{username} was unsuspended." }, status: :ok else - render json: { status: "error", message: "username not found" }, status: :bad_request + render json: { + status: "error", + message: "Unban failed. User with username '#{username}' not found in this channel." + }, status: :bad_request end when "/clearchannel" @chat_channel.clear_channel diff --git a/app/javascript/chat/chat.jsx b/app/javascript/chat/chat.jsx index 849b070bd..b74cd62e6 100644 --- a/app/javascript/chat/chat.jsx +++ b/app/javascript/chat/chat.jsx @@ -8,6 +8,7 @@ import { h, Component } from 'preact'; import PropTypes from 'prop-types'; import { setupPusher } from '../utilities/connect'; import debounceAction from '../utilities/debounceAction'; +import { addSnackbarItem } from '../Snackbar'; import { conductModeration, getAllMessages, @@ -737,6 +738,13 @@ export default class Chat extends Component { path: `/search?q=${message.replace('/s ', '')}`, type_of: 'article', }); + } else if (message.startsWith('/ban ') || message.startsWith('/unban ')) { + conductModeration( + activeChannelId, + message, + this.handleSuccess, + this.handleFailure, + ); } else if (message.startsWith('/')) { this.setActiveContentState(activeChannelId, { type_of: 'loading-post', @@ -748,13 +756,6 @@ export default class Chat extends Component { } else if (message.startsWith('/github')) { const args = message.split('/github ')[1].trim(); this.setActiveContentState(activeChannelId, { type_of: 'github', args }); - } else if (message[0] === '/') { - conductModeration( - activeChannelId, - message, - this.handleSuccess, - this.handleFailure, - ); } else { const messageObject = { activeChannelId, @@ -881,8 +882,10 @@ export default class Chat extends Component { return { messages: newMessages }; }); } + } else if (response.status === 'moderation-success') { + addSnackbarItem({ message: response.message, addCloseButton: true }); } else if (response.status === 'error') { - this.receiveNewMessage(response.message); + addSnackbarItem({ message: response.message, addCloseButton: true }); } }; @@ -1070,6 +1073,7 @@ export default class Chat extends Component { handleFailure = (err) => { // eslint-disable-next-line no-console console.error(err); + addSnackbarItem({ message: err, addCloseButton: true }); }; renderMessages = () => { diff --git a/app/models/role.rb b/app/models/role.rb index 27aa67553..bc7a5d975 100644 --- a/app/models/role.rb +++ b/app/models/role.rb @@ -3,6 +3,7 @@ class Role < ApplicationRecord admin banned chatroom_beta_tester + codeland_admin comment_banned podcast_admin pro diff --git a/app/policies/chat_channel_policy.rb b/app/policies/chat_channel_policy.rb index 8073234b6..d10e6f1c7 100644 --- a/app/policies/chat_channel_policy.rb +++ b/app/policies/chat_channel_policy.rb @@ -12,7 +12,7 @@ class ChatChannelPolicy < ApplicationPolicy end def moderate? - !user_is_banned? && user_admin? + !user_is_banned? && codeland_admin? end def show? @@ -70,4 +70,8 @@ class ChatChannelPolicy < ApplicationPolicy def channel_is_direct record.channel_type == "direct" end + + def codeland_admin? + user.has_role?(:codeland_admin) + end end diff --git a/spec/models/role_spec.rb b/spec/models/role_spec.rb index e3d53628f..444d4802d 100644 --- a/spec/models/role_spec.rb +++ b/spec/models/role_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Role, type: :model do describe "::ROLES" do it "contains the correct values" do expected_roles = %w[ - admin banned chatroom_beta_tester comment_banned podcast_admin pro restricted_liquid_tag + admin banned chatroom_beta_tester codeland_admin comment_banned podcast_admin pro restricted_liquid_tag single_resource_admin super_admin tag_moderator mod_relations_admin tech_admin trusted warned workshop_pass ] diff --git a/spec/policies/chat_channel_policy_spec.rb b/spec/policies/chat_channel_policy_spec.rb index 7ffd12dc2..e3180153b 100644 --- a/spec/policies/chat_channel_policy_spec.rb +++ b/spec/policies/chat_channel_policy_spec.rb @@ -27,7 +27,7 @@ RSpec.describe ChatChannelPolicy, type: :policy do context "when user is an admin but not part of channel" do before { user.add_role(:super_admin) } - it { is_expected.to permit_actions(%i[index moderate update]) } + it { is_expected.to permit_actions(%i[index update]) } it { is_expected.to forbid_actions(%i[show open]) } end end diff --git a/spec/requests/chat_channels_spec.rb b/spec/requests/chat_channels_spec.rb index 1df746990..58229558d 100644 --- a/spec/requests/chat_channels_spec.rb +++ b/spec/requests/chat_channels_spec.rb @@ -251,7 +251,8 @@ RSpec.describe "ChatChannels", type: :request do context "when user is logged-in and authorized" do before do - user.add_role :super_admin + user.add_role :codeland_admin + chat_channel.add_users([user, test_subject]) sign_in user allow(Pusher).to receive(:trigger).and_return(true) end