Return 401 instead of 500 when a channel is blocked (#4721)

app/controllers/messages_controller.rb

@@ -11,6 +11,7 @@ class MessagesController < ApplicationController
       message_json = create_pusher_payload(@message)
       Pusher.trigger(@message.chat_channel.pusher_channels, "message-created", message_json)
     end
+
     if @message.save
       begin
         @message.send_push

app/models/message.rb

@@ -6,7 +6,7 @@ class Message < ApplicationRecord
   validates :message_markdown, presence: true, length: { maximum: 1024 }
   validate :channel_permission
 
-  before_save       :determine_user_validity
+  before_validation :determine_user_validity
   before_validation :evaluate_markdown
   after_create      :send_email_if_appropriate
   after_create      :update_chat_channel_last_message_at
@@ -17,10 +17,6 @@ class Message < ApplicationRecord
     HexComparer.new(color_options).brightness(0.9)
   end
 
-  def determine_user_validity
-    raise unless chat_channel.status == "active" && (chat_channel.has_member?(user) || chat_channel.channel_type == "open")
-  end
-
   def send_push
     Messages::SendPushJob.perform_later(user.id, chat_channel.id, message_html)
   end
@@ -70,6 +66,13 @@ class Message < ApplicationRecord
     html
   end
 
+  def determine_user_validity
+    return unless chat_channel
+
+    user_ok = chat_channel.status == "active" && (chat_channel.has_member?(user) || chat_channel.channel_type == "open")
+    errors.add(:base, "You are not a participant of this chat channel.") unless user_ok
+  end
+
   def channel_permission
     errors.add(:base, "Must be part of channel.") if chat_channel_id.blank?
 

spec/models/message_spec.rb

@@ -7,8 +7,6 @@ RSpec.describe Message, type: :model do
   let(:long_text) { Faker::Hipster.words(number: 1500) }
 
   describe "validations" do
-    subject { build(:message, :ignore_after_callback) }
-
     before do
       allow(ChatChannel).to receive(:find).and_return(ChatChannel.new)
     end

spec/requests/messages_spec.rb

@@ -17,7 +17,6 @@ RSpec.describe "Messages", type: :request do
       post "/messages", params: { message: {} }
       expect(response.status).to eq(302)
     end
-    # Pusher::Error
 
     context "when user is signed in" do
       before do
@@ -41,21 +40,10 @@ RSpec.describe "Messages", type: :request do
         chat_channel.update(status: "blocked")
       end
 
-      it "raises an error" do
-        expect { post "/messages", params: { message: new_message } }.to raise_error
+      it "return unauthorized" do
+        post "/messages", params: { message: new_message }
+        expect(response).to have_http_status(:unauthorized)
       end
     end
-
-    # context "when Pusher isn't cooperating" do
-    #   before do
-    #     allow(Pusher).to receive(:trigger).and_raise(Pusher::Error)
-    #     sign_in user
-    #     post "/messages", params: { message: new_message }
-    #   end
-
-    #   it "returns proper message" do
-    #     expect(response.body).to include("could not trigger Pusher")
-    #   end
-    # end
   end
 end