ylqjgm
/
canvas-lms
mirror of https://github.com/instructure/canvas-lms.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

update/delete discussion entry apis, refs #7567 

test plan: no UI yet. update an entry, verify it gets the new message
(and updated at time). delete an entry, verify it's deleted. attempt
both as a user without permission on that entry, verify you get a 401
and the content doesn't change. also update/delete a student's entry as
a teacher.

Change-Id: I8433aa78e70b3c0597e76de1b8e1634c9f0dd27b
Reviewed-on: https://gerrit.instructure.com/9366
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
This commit is contained in:
Brian Palmer 2012-03-12 09:58:17 -06:00
parent 38b761ca4d
commit d17286f789
3 changed files with 136 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
app/controllers/discussion_entries_controller.rb
View File

@ -16,6 +16,7 @@
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
# @API Discussion Topics
class DiscussionEntriesController < ApplicationController
before_filter :require_context, :except => :public_feed
@ -69,14 +70,32 @@ class DiscussionEntriesController < ApplicationController
end
end
include Api::V1::DiscussionTopics
# @API
# Update an existing discussion entry.
#
# The entry must have been created by the current user, or the current user
# must have admin rights to the discussion. If the edit is not allowed, a 401 will be returned.
#
# @argument message The updated body of the entry.
#
# @example_request
# curl -X PUT 'http://<canvas>/api/v1/courses/<course_id>/discussion_topics/<topic_id>/entries/<entry_id>' \
# -F 'message=<message>' \
# -H "Authorization: Bearer <token>"
def update
@remove_attachment = (params[:discussion_entry] || {}).delete :remove_attachment
@topic = @context.all_discussion_topics.active.find(params[:topic_id]) if params[:topic_id].present?
params[:discussion_entry] ||= params
@remove_attachment = params[:discussion_entry].delete :remove_attachment
# unused attributes during update
params[:discussion_entry].delete(:discussion_topic_id)
params[:discussion_entry].delete(:parent_id)
@entry = @context.discussion_entries.find(params[:id])
@topic = @entry.discussion_topic
@entry = (@topic || @context).discussion_entries.find(params[:id])
raise(ActiveRecord::RecordNotFound) if @entry.deleted?
@topic ||= @entry.discussion_topic
@entry.current_user = @current_user
@entry.attachment_id = nil if @remove_attachment == '1'
if authorized_action(@entry, @current_user, :update)
@ -86,16 +105,18 @@ class DiscussionEntriesController < ApplicationController
quota_exceeded(named_context_url(@context, :context_discussion_topic_url, @topic.id))
@entry.editor = @current_user
respond_to do |format|
if @entry.update_attributes(params[:discussion_entry])
if @entry.update_attributes(params[:discussion_entry].slice(:message, :plaintext_message))
if params[:attachment] && params[:attachment][:uploaded_data] && params[:attachment][:uploaded_data].size > 0 && @entry.grants_right?(@current_user, session, :attach)
@attachment = @context.attachments.create(params[:attachment])
@entry.attachment = @attachment
@entry.save
end
format.html {
flash[:notice] = t :updated_entry_notice, 'Entry was successfully updated.'
format.html { redirect_to named_context_url(@context, :context_discussion_topic_url, @entry.discussion_topic_id) }
format.json { render :json => @entry.to_json(:include => :attachment, :methods => [:user_name, :read_state], :permissions => {:user => @current_user, :session => session}), :status => :ok }
format.text { render :json => @entry.to_json(:include => :attachment, :methods => [:user_name, :read_state], :permissions => {:user => @current_user, :session => session}), :status => :ok }
redirect_to named_context_url(@context, :context_discussion_topic_url, @entry.discussion_topic_id)
}
format.json { render :json => discussion_entry_api_json([@entry], @context, @current_user, session, false).first }
format.text { render :json => discussion_entry_api_json([@entry], @context, @current_user, session, false).first }
else
format.html { render :action => "edit" }
format.json { render :json => @entry.errors.to_json, :status => :bad_request }
@ -105,14 +126,28 @@ class DiscussionEntriesController < ApplicationController
end
end
# @API
# Delete a discussion entry.
#
# The entry must have been created by the current user, or the current user
# must have admin rights to the discussion. If the delete is not allowed, a 401 will be returned.
#
# The discussion will be marked deleted, and the user_id and message will be cleared out.
#
# @example_request
#
# curl -X DELETE 'http://<canvas>/api/v1/courses/<course_id>/discussion_topics/<topic_id>/entries/<entry_id>' \
# -H "Authorization: Bearer <token>"
def destroy
@entry = @context.discussion_entries.find(params[:id])
@topic = @context.all_discussion_topics.active.find(params[:topic_id]) if params[:topic_id].present?
@entry = (@topic || @context).discussion_entries.find(params[:id])
if authorized_action(@entry, @current_user, :delete)
@entry.editor = @current_user
@entry.destroy
respond_to do |format|
format.html { redirect_to named_context_url(@context, :context_discussion_topic_url, @entry.discussion_topic_id) }
format.json { render :json => @entry.to_json, :status => :ok }
format.json { render :nothing => true, :status => :no_content }
end
end
end

2
config/routes.rb
View File

@ -697,6 +697,8 @@ ActionController::Routing::Routes.draw do |map|
topics.get "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/entries", :action => :entries, :path_name => "#{context}_discussion_entries"
topics.post "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/entries/:entry_id/replies", :action => :add_reply, :path_name => "#{context}_discussion_add_reply"
topics.get "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/entries/:entry_id/replies", :action => :replies, :path_name => "#{context}_discussion_replies"
topics.put "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/entries/:id", :controller => :discussion_entries, :action => :update
topics.delete "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/entries/:id", :controller => :discussion_entries, :action => :destroy
topics.put "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/read", :action => :mark_topic_read, :path_name => "#{context}_discussion_topic_mark_read"
topics.delete "#{context.pluralize}/:#{context}_id/discussion_topics/:topic_id/read", :action => :mark_topic_unread, :path_name => "#{context}_discussion_topic_mark_unread"

89
spec/apis/v1/discussion_topics_api_spec.rb
View File

@ -579,6 +579,90 @@ describe DiscussionTopicsController, :type => :integration do
end
end
context "update entry" do
before do
@topic = create_topic(@course, :title => "topic", :message => "topic")
@entry = create_entry(@topic, :message => "<p>top-level entry</p>")
end
it "should 401 if the user can't update" do
student_in_course(:course => @course, :user => user_with_pseudonym)
api_call(:put, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "update", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, { :message => 'haxor' }, {}, :expected_status => 401)
@entry.reload.message.should == '<p>top-level entry</p>'
end
it "should 404 if the entry is deleted" do
@entry.destroy
api_call(:put, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "update", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, { :message => 'haxor' }, {}, :expected_status => 404)
end
it "should update the message" do
api_call(:put, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "update", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, { :message => '<p>i had a spleling error</p>' })
@entry.reload.message.should == '<p>i had a spleling error</p>'
end
it "should allow passing an plaintext message (undocumented)" do
# undocumented but used by the dashboard right now (this'll go away eventually)
api_call(:put, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "update", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, { :plaintext_message => 'i had a spleling error' })
@entry.reload.message.should == 'i had a spleling error'
end
it "should allow teachers to edit student entries" do
@teacher = @user
student_in_course(:course => @course, :user => user_with_pseudonym)
@student = @user
@user = @teacher
@entry = create_entry(@topic, :message => 'i am a student', :user => @student)
@entry.user.should == @student
@entry.editor.should be_nil
api_call(:put, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "update", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, { :message => '<p>denied</p>' })
@entry.reload.message.should == '<p>denied</p>'
@entry.editor.should == @teacher
end
end
context "delete entry" do
before do
@topic = create_topic(@course, :title => "topic", :message => "topic")
@entry = create_entry(@topic, :message => "top-level entry")
end
it "should 401 if the user can't delete" do
student_in_course(:course => @course, :user => user_with_pseudonym)
api_call(:delete, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "destroy", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, {}, {}, :expected_status => 401)
@entry.reload.should_not be_deleted
end
it "should soft-delete the entry" do
raw_api_call(:delete, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "destroy", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, {}, {}, :expected_status => 204)
response.body.should be_blank
@entry.reload.should be_deleted
end
it "should allow teachers to delete student entries" do
@teacher = @user
student_in_course(:course => @course, :user => user_with_pseudonym)
@student = @user
@user = @teacher
@entry = create_entry(@topic, :message => 'i am a student', :user => @student)
@entry.user.should == @student
@entry.editor.should be_nil
raw_api_call(:delete, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/entries/#{@entry.id}",
{ :controller => "discussion_entries", :action => "destroy", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s, :id => @entry.id.to_s }, {}, {}, :expected_status => 204)
@entry.reload.should be_deleted
@entry.editor.should == @teacher
end
end
context "read/unread state" do
before(:each) do
@topic = create_topic(@course, :title => "topic", :message => "topic")
@ -768,6 +852,9 @@ describe DiscussionTopicsController, :type => :integration do
json.map { |r| r['parent_id'] }.should == [@sub2.id, @entry.id, @sub2.id, @sub1.id, @entry.id]
end
it "should set and return editor_id if editing another user's post" do
end
it "should fail if the max entry depth is reached" do
entry = @entry
(DiscussionEntry.max_depth - 1).times do
@ -816,6 +903,8 @@ describe DiscussionTopicsController, :type => :integration do
@reply_reply1.editor = @teacher
@reply_reply1.update_attributes(:message => '<p>censored</p>')
@all_entries.each &:reload
json = api_call(:get, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}/view",
{ :controller => "discussion_topics_api", :action => "view", :format => "json", :course_id => @course.id.to_s, :topic_id => @topic.id.to_s })