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

store full message in materialized view, not summary 

test plan: pull a materialized view from the /view api (making sure it
was generated with this changeset), verify you get the message back
rather than the summary

Change-Id: Ifd61218d40c67f43b85ed0171f3b90f8a20dd291
Reviewed-on: https://gerrit.instructure.com/9632
Reviewed-by: Zach Wily <zach@instructure.com>
Tested-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Brian Palmer 2012-03-27 12:41:55 -06:00
parent 93a76a37d6
commit 0a2e19fb8d
10 changed files with 52 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/controllers/discussion_entries_controller.rb
View File

@ -115,8 +115,8 @@ class DiscussionEntriesController < ApplicationController
flash[:notice] = t :updated_entry_notice, 'Entry was successfully updated.'
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 }
format.json { render :json => discussion_entry_api_json([@entry], @context, @current_user, session, [:user_name]).first }
format.text { render :json => discussion_entry_api_json([@entry], @context, @current_user, session, [:user_name]).first }
else
format.html { render :action => "edit" }
format.json { render :json => @entry.errors.to_json, :status => :bad_request }

25
app/controllers/discussion_topics_api_controller.rb
View File

@ -27,14 +27,14 @@ class DiscussionTopicsApiController < ApplicationController
before_filter :require_initial_post, :except => [:add_entry, :mark_topic_read, :mark_topic_unread]
# @API
# Return a summary structure of the discussion topic, containing all entries,
# their authors, and a summarized version of their message bodies.
# Return a cached structure of the discussion topic, containing all entries,
# their authors, and their message bodies.
#
# May require (depending on the topic) that the user has posted in the topic.
# If it is required, and the user has not posted, will respond with a 403
# Forbidden status and the body 'require_initial_post'.
#
# In some rare situations, this summary structure may not be available yet. In
# In some rare situations, this cached structure may not be available yet. In
# that case, the server will respond with a 503 error, and the caller should
# try again soon.
#
@ -45,8 +45,7 @@ class DiscussionTopicsApiController < ApplicationController
# * "unread_entries": a list of entry ids that are unread by the current
# user. this implies that any entry not in this list is read.
# * "view": a threaded view of all the entries in the discussion, containing
# the id, user_id, and summary of the message. to get the full message text,
# use the other discussion api calls.
# the id, user_id, and message.
#
# @example_request
#
@ -61,11 +60,11 @@ class DiscussionTopicsApiController < ApplicationController
# { "id": 11, "display_name": "user 2", "avatar_url": "https://..." }
# ],
# "view": [
# { "id": 1, "user_id": 10, "parent_id": null, "summary": "...html text...", "replies": [
# { "id": 3, "user_id": 11, "parent_id": 1, "summary": "...html....", "replies": [...] }
# { "id": 1, "user_id": 10, "parent_id": null, "message": "...html text...", "replies": [
# { "id": 3, "user_id": 11, "parent_id": 1, "message": "...html....", "replies": [...] }
# ]},
# { "id": 2, "user_id": 11, "parent_id": null, "summary": "...html..." },
# { "id": 4, "user_id": 10, "parent_id": null, "summary": "...html..." }
# { "id": 2, "user_id": 11, "parent_id": null, "message": "...html..." },
# { "id": 4, "user_id": 10, "parent_id": null, "message": "...html..." }
# ]
# }
def view
@ -189,7 +188,7 @@ class DiscussionTopicsApiController < ApplicationController
def entries
if authorized_action(@topic, @current_user, :read)
@entries = Api.paginate(root_entries(@topic).newest_first, self, entry_pagination_path(@topic))
render :json => discussion_entry_api_json(@entries, @context, @current_user, session, true)
render :json => discussion_entry_api_json(@entries, @context, @current_user, session)
end
end
@ -266,7 +265,7 @@ class DiscussionTopicsApiController < ApplicationController
@parent = root_entries(@topic).find(params[:entry_id])
if authorized_action(@topic, @current_user, :read)
@replies = Api.paginate(reply_entries(@parent).newest_first, self, reply_pagination_path(@parent))
render :json => discussion_entry_api_json(@replies, @context, @current_user, session, true)
render :json => discussion_entry_api_json(@replies, @context, @current_user, session)
end
end
@ -311,7 +310,7 @@ class DiscussionTopicsApiController < ApplicationController
ids = Array(params[:ids])
entries = @topic.discussion_entries.find(ids, :order => :id)
@entries = Api.paginate(entries, self, entry_pagination_path(@topic))
render :json => discussion_entry_api_json(@entries, @context, @current_user, session, false)
render :json => discussion_entry_api_json(@entries, @context, @current_user, session, [])
end
end
@ -453,7 +452,7 @@ class DiscussionTopicsApiController < ApplicationController
@entry.attachment = @attachment
@entry.save
end
render :json => discussion_entry_api_json([@entry], @context, @current_user, session, false).first, :status => :created
render :json => discussion_entry_api_json([@entry], @context, @current_user, session, [:user_name]).first, :status => :created
else
render :json => @entry.errors, :status => :bad_request
end

4
app/models/discussion_topic/materialized_view.rb
View File

@ -81,9 +81,7 @@ class DiscussionTopic::MaterializedView < ActiveRecord::Base
user_ids = Set.new
discussion_entries = self.discussion_topic.discussion_entries
discussion_entries.find_each do |entry|
json = discussion_entry_api_json([entry], @context, nil, nil, false).first
json.delete(:user_name) # this can get out of date in the cached view
json[:summary] = entry.summary unless entry.deleted?
json = discussion_entry_api_json([entry], discussion_topic.context, nil, nil, []).first
entry_lookup[entry.id] = json
user_ids << entry.user_id
user_ids << entry.editor_id if entry.editor_id

10
lib/api.rb
View File

@ -214,11 +214,15 @@ module Api
def api_user_content(html, context = @context, user = @current_user)
return html if html.blank?
# if we're a controller, use the host of the request, otherwise let HostUrl
# figure out what host is appropriate
host = HostUrl.context_host(context, @account_domain) unless self.is_a?(ApplicationController)
rewriter = UserContent::HtmlRewriter.new(context, user)
rewriter.set_handler('files') do |match|
obj = match.obj_class.find_by_id(match.obj_id)
next unless obj && rewriter.user_can_view_content?(obj)
file_download_url(obj.id, :verifier => obj.uuid, :download => '1')
file_download_url(obj.id, :verifier => obj.uuid, :download => '1', :host => host)
end
html = rewriter.translate_content(html)
@ -229,8 +233,8 @@ module Api
doc.css('a.instructure_inline_media_comment').each do |anchor|
media_id = anchor['id'].try(:gsub, /^media_comment_/, '')
next if media_id.blank?
media_redirect = polymorphic_url([context, :media_download], :entryId => media_id, :type => 'mp4', :redirect => '1')
thumbnail = media_object_thumbnail_url(media_id, :width => 550, :height => 448, :type => 3)
media_redirect = polymorphic_url([context, :media_download], :entryId => media_id, :type => 'mp4', :redirect => '1', :host => host)
thumbnail = media_object_thumbnail_url(media_id, :width => 550, :height => 448, :type => 3, :host => host)
video_node = Nokogiri::XML::Node.new('video', doc)
video_node['controls'] = 'controls'
video_node['poster'] = thumbnail

22
lib/api/v1/discussion_topics.rb
View File

@ -63,31 +63,29 @@ module Api::V1::DiscussionTopics
# there is no specific user attached to this view of the discussion, the same
# json is returned to all users who can access the discussion, so it's a bit
# different than our normal api_json helpers
#
# the message body will only be included if context and @current_user are present
def discussion_entry_api_json(entries, context, user, session, include_subentries)
def discussion_entry_api_json(entries, context, user, session, includes = [:user_name, :subentries])
entries.map do |entry|
if entry.deleted?
json = api_json(entry, user, session, :only => %w(id created_at updated_at parent_id))
json[:deleted] = true
else
json = api_json(entry, user, session,
:only => %w(id user_id created_at updated_at parent_id),
:methods => [:user_name, :summary])
:only => %w(id user_id created_at updated_at parent_id))
json[:user_name] = entry.user_name if includes.include?(:user_name)
json[:editor_id] = entry.editor_id if entry.editor_id && entry.editor_id != entry.user_id
if context.present? && user.present?
json[:message] = api_user_content(entry.message, context, user)
json[:message] = api_user_content(entry.message, context, user)
if entry.attachment
json[:attachment] = attachment_json(entry.attachment, :host => HostUrl.context_host(context))
# this is for backwards compatibility, and can go away if we make an api v2
json[:attachments] = [json[:attachment]]
end
json[:attachment] = attachment_json(entry.attachment, :host => HostUrl.context_host(context)) if entry.attachment
# this is for backwards compatibility, and can go away if we make an api v2
json[:attachments] = [json[:attachment]] if entry.attachment
end
json[:read_state] = entry.read_state(user) if user
if include_subentries && entry.root_entry_id.nil?
if includes.include?(:subentries) && entry.root_entry_id.nil?
replies = entry.flattened_discussion_subentries.active.newest_first.find(:all, :limit => 11).to_a
unless replies.empty?
json[:recent_replies] = discussion_entry_api_json(replies.first(10), context, user, session, false)
json[:recent_replies] = discussion_entry_api_json(replies.first(10), context, user, session, includes)
json[:has_more_replies] = replies.size > 10
end
end

2
lib/api/v1/submission.rb
View File

@ -118,7 +118,7 @@ module Api::V1::Submission
else
entries = assignment.discussion_topic.discussion_entries.active.for_user(attempt.user_id)
end
hash['discussion_entries'] = discussion_entry_api_json(entries, assignment.discussion_topic.context, user, session, true)
hash['discussion_entries'] = discussion_entry_api_json(entries, assignment.discussion_topic.context, user, session)
end
hash

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

@ -224,7 +224,6 @@ describe DiscussionTopicsController, :type => :integration do
"user_name" => @user.name,
"read_state" => "read",
"message" => @message,
"summary" => @entry.summary,
"created_at" => @entry.created_at.utc.iso8601,
"updated_at" => @entry.updated_at.as_json,
}
@ -930,7 +929,7 @@ describe DiscussionTopicsController, :type => :integration do
'id' => @root1.id,
'parent_id' => nil,
'user_id' => @student.id,
'summary' => "root1",
'message' => "root1",
'created_at' => @root1.created_at.as_json,
'updated_at' => @root1.updated_at.as_json,
'replies' => [
@ -944,7 +943,7 @@ describe DiscussionTopicsController, :type => :integration do
'id' => @reply_reply2.id,
'parent_id' => @reply1.id,
'user_id' => @student.id,
'summary' => 'reply_reply2',
'message' => 'reply_reply2',
'created_at' => @reply_reply2.created_at.as_json,
'updated_at' => @reply_reply2.updated_at.as_json,
} ],
@ -952,7 +951,7 @@ describe DiscussionTopicsController, :type => :integration do
{ 'id' => @reply2.id,
'parent_id' => @root1.id,
'user_id' => @teacher.id,
'summary' => 'reply2',
'message' => "<p><a href=\"http://localhost/files/#{@reply2_attachment.id}/download?verifier=#{@reply2_attachment.uuid}\">This is a file link</a></p>\n <p>This is a video:\n <video controls=\"controls\" poster=\"http://localhost/media_objects/0_abcde/thumbnail?height=448&amp;type=3&amp;width=550\" src=\"http://localhost/courses/#{@course.id}/media_download?entryId=0_abcde&amp;redirect=1&amp;type=mp4\" width=\"550\" height=\"448\"></video>\n </p>",
'created_at' => @reply2.created_at.as_json,
'updated_at' => @reply2.updated_at.as_json,
'replies' => [ {
@ -960,7 +959,7 @@ describe DiscussionTopicsController, :type => :integration do
'parent_id' => @reply2.id,
'user_id' => @student.id,
'editor_id' => @teacher.id,
'summary' => 'censored',
'message' => '<p>censored</p>',
'created_at' => @reply_reply1.created_at.as_json,
'updated_at' => @reply_reply1.updated_at.as_json,
'attachment' => reply_reply1_attachment_json,
@ -973,7 +972,7 @@ describe DiscussionTopicsController, :type => :integration do
'id' => @root2.id,
'parent_id' => nil,
'user_id' => @student.id,
'summary' => 'root2',
'message' => 'root2',
'created_at' => @root2.created_at.as_json,
'updated_at' => @root2.updated_at.as_json,
'replies' => [
@ -981,7 +980,7 @@ describe DiscussionTopicsController, :type => :integration do
'id' => @reply3.id,
'parent_id' => @root2.id,
'user_id' => @student.id,
'summary' => 'reply3',
'message' => 'reply3',
'created_at' => @reply3.created_at.as_json,
'updated_at' => @reply3.updated_at.as_json,
},

4
spec/apis/v1/submissions_api_spec.rb
View File

@ -240,7 +240,6 @@ describe 'Submissions API', :type => :integration do
[{
'id' => se1.id,
'message' => 'sub 1',
'summary' => 'sub 1',
'user_id' => @student.id,
'read_state' => 'unread',
'parent_id' => e1.id,
@ -251,7 +250,6 @@ describe 'Submissions API', :type => :integration do
{
'id' => se2.id,
'message' => 'student 1',
'summary' => 'student 1',
'user_id' => @student.id,
'read_state' => 'unread',
'parent_id' => nil,
@ -307,7 +305,6 @@ describe 'Submissions API', :type => :integration do
[{
'id' => se1.id,
'message' => 'sub 1',
'summary' => 'sub 1',
'user_id' => @student.id,
'user_name' => 'User',
'read_state' => 'unread',
@ -318,7 +315,6 @@ describe 'Submissions API', :type => :integration do
{
'id' => se2.id,
'message' => 'student 1',
'summary' => 'student 1',
'user_id' => @student.id,
'user_name' => 'User',
'read_state' => 'unread',

8
spec/factories/discussion_topic_factory.rb
View File

@ -44,7 +44,13 @@ def topic_with_nested_replies(opts = {})
@root1 = @topic.reply_from(:user => @student, :html => "root1")
@root2 = @topic.reply_from(:user => @student, :html => "root2")
@reply1 = @root1.reply_from(:user => @teacher, :html => "reply1")
@reply2 = @root1.reply_from(:user => @teacher, :html => "reply2")
@reply2_attachment = attachment_model(:context => @course)
@reply2 = @root1.reply_from(:user => @teacher, :html => <<-HTML)
<p><a href="/courses/#{@course.id}/files/#{@reply2_attachment.id}/download">This is a file link</a></p>
<p>This is a video:
<a class='instructure_inline_media_comment' id='media_comment_0_abcde' href='#'>link</a>
</p>
HTML
@reply_reply1 = @reply2.reply_from(:user => @student, :html => "reply_reply1")
@reply_reply1.update_attribute(:attachment, attachment_model)
@reply_reply2 = @reply1.reply_from(:user => @student, :html => "reply_reply2")

8
spec/models/discussion_topic/materialized_view_spec.rb
View File

@ -68,12 +68,16 @@ describe DiscussionTopic::MaterializedView do
json.size.should == 2
json.map { |e| e['id'] }.should == [@root1.id, @root2.id]
json.map { |e| e['parent_id'] }.should == [nil, nil]
json.map { |e| e['summary'] }.should == ['root1', 'root2']
deleted = json[0]['replies'][0]
deleted['deleted'].should == true
deleted['user_id'].should be_nil
deleted['summary'].should be_nil
deleted['message'].should be_nil
json[0]['replies'][1]['replies'][0]['attachment']['url'].should == "http://localhost/files/#{@attachment.id}/download?download_frd=1&verifier=#{@attachment.uuid}"
# verify the api_user_content functionality in a non-request context
html_message = json[0]['replies'][1]['message']
html = Nokogiri::HTML::DocumentFragment.parse(html_message)
html.at_css('a')['href'].should == "http://localhost/files/#{@reply2_attachment.id}/download?verifier=#{@reply2_attachment.uuid}"
html.at_css('video')['src'].should == "http://localhost/courses/#{@course.id}/media_download?entryId=0_abcde&redirect=1&type=mp4"
# the deleted entry will be marked deleted and have no summary
simple_json = map_to_ids_and_replies(json)