fix 500 level errors in discussion helpers

fixes VICE-1426
fixes VICE-1427

flag=none

test plan:
- specs, old and new, pass

it seems like this is only a helper
method for when quickly writing specs
and stuff. so risk is very low

Change-Id: I4fe474ade6b1acfba0074135489add4d0bf464c6
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/264850
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
This commit is contained in:
Caleb Guanzon 2021-05-12 13:57:34 -06:00
parent 3d636e2005
commit a215e0f4fc
5 changed files with 35 additions and 4 deletions

View File

@ -170,9 +170,9 @@ class DiscussionEntry < ActiveRecord::Base
end
user = nil unless user && self.context.users.include?(user)
if !user
raise "Only context participants may reply to messages"
raise IncomingMail::Errors::InvalidParticipant
elsif !message || message.empty?
raise "Message body cannot be blank"
raise IncomingMail::Errors::BlankMessage
else
self.shard.activate do
entry = discussion_topic.discussion_entries.new(message: message,

View File

@ -1044,9 +1044,9 @@ class DiscussionTopic < ActiveRecord::Base
end
user = nil unless user && self.context.users.include?(user)
if !user
raise "Only context participants may reply to messages"
raise IncomingMail::Errors::InvalidParticipant
elsif !message || message.empty?
raise "Message body cannot be blank"
raise IncomingMail::Errors::BlankMessage
elsif !self.grants_right?(user, :read)
nil
else

View File

@ -24,5 +24,7 @@ module IncomingMail
class UnknownSender < ReplyFrom; end
class ReplyToLockedTopic < ReplyFrom; end
class ReplyToDeletedDiscussion < ReplyFrom; end
class InvalidParticipant < ReplyFrom; end
class BlankMessage < ReplyFrom; end
end
end

View File

@ -640,6 +640,15 @@ describe DiscussionEntry do
student_in_course(:course => @course)
expect { @entry.reply_from(:user => @student, :text => "reply") }.to raise_error(IncomingMail::Errors::ReplyToLockedTopic)
end
it 'raises InvalidParticipant for invalid participants' do
u = user_with_pseudonym(:active_user => true, :username => 'test1@example.com', :password => 'test1234')
expect { @topic.reply_from(user: u, text: "entry 1") }.to raise_error IncomingMail::Errors::InvalidParticipant
end
it 'raises BlankMessage for blank message' do
expect { @topic.reply_from(user: @teacher, text: '') }.to raise_error IncomingMail::Errors::BlankMessage
end
end
context 'stream items' do

View File

@ -2515,6 +2515,26 @@ describe DiscussionTopic do
end
end
describe 'reply_from' do
before(:once) do
@topic = @course.discussion_topics.create!(user: @teacher, message: 'topic')
end
it 'returns entry for valid arguments' do
val = @topic.reply_from(:user => @teacher, :text => "entry 1")
expect(val).to be_a_kind_of DiscussionEntry
end
it 'raises InvalidParticipant for invalid participants' do
u = user_with_pseudonym(:active_user => true, :username => 'test1@example.com', :password => 'test1234')
expect { @topic.reply_from(user: u, text: "entry 1") }.to raise_error IncomingMail::Errors::InvalidParticipant
end
it 'raises BlankMessage for empty message' do
expect { @topic.reply_from(user: @teacher, text: '') }.to raise_error IncomingMail::Errors::BlankMessage
end
end
describe 'to_podcast' do
it "includes media extension in enclosure url even though it is a redirect (for itunes)" do
@topic = @course.discussion_topics.create!(