return attachment and media comment urls, closes #5850

Change-Id: I971a02d2055778120a0a5df0129f2b145da98843
Reviewed-on: https://gerrit.instructure.com/6045
Reviewed-by: Brian Palmer <[email protected]>
Tested-by: Hudson <[email protected]>

app/coffeescripts/messages.coffee

@@ -852,9 +852,8 @@ I18n.scoped 'conversations', (I18n) ->
     $message
 
   build_media_object = (blank, data) ->
-    $media_object = blank.clone(true).attr('id', 'media_object_' + data.id)
-    $media_object.data('id', data.id)
-    $media_object.find('span.title').html $.h(data.title)
+    $media_object = blank.clone(true).attr('id', 'media_comment_' + data.media_id)
+    $media_object.find('span.title').html $.h(data.display_name)
     $media_object.find('span.media_comment_id').html $.h(data.media_id)
     $media_object
 
@@ -863,7 +862,7 @@ I18n.scoped 'conversations', (I18n) ->
     $attachment.data('id', data.id)
     $attachment.find('span.title').html $.h(data.display_name)
     $link = $attachment.find('a')
-    $link.attr('href', $.replaceTags($link.attr('href'), id: data.id, uuid: data.uuid))
+    $link.attr('href', data.url)
     $link.click (e) ->
       e.stopPropagation()
     $attachment

app/controllers/conversations_controller.rb

@@ -188,9 +188,9 @@ def create
   #   body:: The actual message body
   #   author_id:: The id of the user who sent the message (see audience, participants)
   #   generated:: If true, indicates this is a system-generated message (e.g. "Bob added Alice to the conversation")
-  #   media_comment:: Audio comment data for this message (if applicable). Fields include: id, title, media_id
+  #   media_comment:: Audio comment data for this message (if applicable). Fields include: display_name, content-type, media_id, media_type, url
   #   forwarded_messages:: If this message contains forwarded messages, they will be included here (same format as this list). Note that those messages may have forwarded messages of their own, etc.
-  #   attachments:: Array of attachments for this message. Fields include: id, display_name, uuid
+  #   attachments:: Array of attachments for this message. Fields include: display_name, content-type, filename, url
   # @response_field submissions Array of assignment submissions having
   #   comments relevant to this conversation. These should be interleaved with
   #   the messages when displaying to the user. Fields include:
@@ -767,7 +767,7 @@ def create_message_on_conversation(conversation=@conversation, update_for_sender
   def jsonify_conversation(conversation, options = {})
     result = conversation.as_json(options)
     audience = conversation.participants.reject{ |u| u.id == conversation.user_id }
-    result[:messages] = options[:messages] if options[:messages]
+    result[:messages] = jsonify_messages(options[:messages]) if options[:messages]
     result[:submissions] = options[:submissions] if options[:submissions]
     result[:audience] = audience.map(&:id)
     result[:audience_contexts] = contexts_for(audience)
@@ -776,6 +776,15 @@ def jsonify_conversation(conversation, options = {})
     result
   end
 
+  def jsonify_messages(messages)
+    messages.map{ |message|
+      message = message.as_json
+      message['media_comment'] = media_comment_json(message['media_comment']) if message['media_comment']
+      message['attachments'] = message['attachments'].map{ |attachment| attachment_json(attachment) }
+      message
+    }
+  end
+
   def jsonify_users(users, options = {})
     options = {
       :include_participant_avatars => true,

app/controllers/submissions_api_controller.rb

@@ -260,8 +260,7 @@ def submission_json(submission, assignment, includes = [])
           :include_root => false,
           :only => %w(author_id author_name created_at comment))
         if sc.media_comment?
-          sc_hash['media_comment'] = media_comment_json(sc.media_comment_id,
-                                                        sc.media_comment_type)
+          sc_hash['media_comment'] = media_comment_json(:media_id => sc.media_comment_id, :media_type => sc.media_comment_type)
         end
         sc_hash['attachments'] = sc.attachments.map do |a|
           attachment_json(a)
@@ -308,8 +307,7 @@ def submission_attempt_json(attempt, assignment, version_idx = nil)
       'version' => version_idx)
 
     unless attempt.media_comment_id.blank?
-      hash['media_comment'] = media_comment_json(attempt.media_comment_id,
-                                                 attempt.media_comment_type)
+      hash['media_comment'] = media_comment_json(:media_id => attempt.media_comment_id, :media_type => attempt.media_comment_type)
     end
     if other_fields.include?('attachments')
       attachments = attempt.versioned_attachments.dup
@@ -347,18 +345,6 @@ def submission_attempt_json(attempt, assignment, version_idx = nil)
     hash
   end
 
-  # a media comment looks just like an attachment to the API
-  def media_comment_json(media_comment_id, media_comment_type)
-    {
-      'media_comment_id' => media_comment_id,
-      'media_comment_type' => media_comment_type,
-      'content-type' => "#{media_comment_type}/mp4",
-      'url' => course_media_download_url(:entryId => media_comment_id,
-                                         :type => "mp4",
-                                         :redirect => "1"),
-    }
-  end
-
   def map_user_ids(user_ids)
     Api.map_ids(user_ids, User).compact unless user_ids.blank?
   end

app/models/conversation_message.rb

@@ -152,8 +152,8 @@ def forwarded_messages
   def as_json(options = {})
     super(:only => [:id, :created_at, :body, :generated, :author_id])['conversation_message'].merge({
       'forwarded_messages' => forwarded_messages,
-      'attachments' => attachments.map{ |a| a.as_json(:only => [:id, :display_name], :methods => ['uuid'])['attachment'] },
-      'media_comment' => media_comment.as_json(:only => [:id, :title, :media_id])['media_object']
+      'attachments' => attachments,
+      'media_comment' => media_comment
     })
   end
 

app/views/conversations/index.html.erb

@@ -135,7 +135,7 @@
         </a>
       </li>
       <li class='attachment_blank'>
-        <a href="<%= file_download_path('{{ id }}', :verifier => '{{ uuid }}') %>" title="<%= t('titles.download_attachment', "Download attachment") %>">
+        <a href="#" title="<%= t('titles.download_attachment', "Download attachment") %>">
           <%= image_tag "messages/attach-gray.png" %> 
           <span class='title'></span>
         </a>

config/routes.rb

@@ -511,6 +511,7 @@
     user.resources :zip_file_imports, :only => [:new, :create], :collection => [:import_status]
     user.course_teacher_activity 'teacher_activity/course/:course_id', :controller => 'users', :action => 'teacher_activity'
     user.student_teacher_activity 'teacher_activity/student/:student_id', :controller => 'users', :action => 'teacher_activity'
+    user.media_download 'media_download', :controller => 'users', :action => 'media_download'
   end
   map.resource :profile, :only => [:show, :update], :controller => "profile", :member => { :communication => :get, :update_communication => :post } do |profile|
     profile.resources :pseudonyms, :except => %w(index)

lib/api.rb

@@ -134,6 +134,20 @@ def attachment_json(attachment)
     }
   end
 
+  def media_comment_json(media_object_or_hash)
+    media_object_or_hash = OpenStruct.new(media_object_or_hash) if media_object_or_hash.is_a?(Hash)
+    {
+      'content-type' => "#{media_object_or_hash.media_type}/mp4",
+      'display_name' => media_object_or_hash.title,
+      'media_id' => media_object_or_hash.media_id,
+      'media_type' => media_object_or_hash.media_type,
+      'url' => user_media_download_url(:user_id => @current_user.id,
+                                       :entryId => media_object_or_hash.media_id,
+                                       :type => "mp4",
+                                       :redirect => "1")
+    }
+  end
+
   # stream an array of objects as a json response, without building a string of
   # the whole response in memory.
   def stream_json_array(array, json_opts)

spec/apis/v1/conversations_api_spec.rb

@@ -527,7 +527,21 @@ def student_in_course(options = {})
   context "conversation" do
     it "should return the conversation" do
       conversation = conversation(@bob)
-      conversation.add_message("another")
+      attachment = nil
+      media_object = nil
+      conversation.add_message("another") do |message|
+        attachment = message.attachments.create(:filename => 'test.txt', :display_name => "test.txt", :uploaded_data => StringIO.new('test'))
+        media_object = MediaObject.new
+        media_object.media_id = '0_12345678'
+        media_object.media_type = 'audio'
+        media_object.context = @me
+        media_object.user = @me
+        media_object.title = "test title"
+        media_object.save!
+        message.media_comment = media_object
+        message.save
+      end
+
       conversation.reload
 
       json = api_call(:get, "/api/v1/conversations/#{conversation.conversation_id}",
@@ -545,7 +559,7 @@ def student_in_course(options = {})
         "subscribed" => true,
         "private" => true,
         "label" => nil,
-        "properties" => ["last_author"],
+        "properties" => ["last_author", "attachments", "media_objects"],
         "audience" => [@bob.id],
         "audience_contexts" => {
           "groups" => {},
@@ -556,7 +570,29 @@ def student_in_course(options = {})
           {"id" => @bob.id, "name" => @bob.name, "common_courses" => {@course.id.to_s => ["StudentEnrollment"]}, "common_groups" => {}}
         ],
         "messages" => [
-          {"id" => conversation.messages.first.id, "created_at" => conversation.messages.first.created_at.to_json[1, 20], "body" => "another", "author_id" => @me.id, "generated" => false, "media_comment" => nil, "forwarded_messages" => [], "attachments" => []},
+          {
+            "id" => conversation.messages.first.id,
+            "created_at" => conversation.messages.first.created_at.to_json[1, 20], 
+            "body" => "another", 
+            "author_id" => @me.id, 
+            "generated" => false, 
+            "media_comment" => {
+              "media_type" => "audio",
+              "media_id" => "0_12345678",
+              "display_name" => "test title",
+              "content-type" => "audio/mp4",
+              "url" => "http://www.example.com/users/#{@me.id}/media_download?entryId=0_12345678&redirect=1&type=mp4"
+            }, 
+            "forwarded_messages" => [], 
+            "attachments" => [
+              {
+                "filename" => "test.txt",
+                "url" => "http://www.example.com/files/#{attachment.id}/download?verifier=#{attachment.uuid}",
+                "content-type" => "unknown/unknown",
+                "display_name" => "test.txt",
+              }
+            ]
+          },
           {"id" => conversation.messages.last.id, "created_at" => conversation.messages.last.created_at.to_json[1, 20], "body" => "test", "author_id" => @me.id, "generated" => false, "media_comment" => nil, "forwarded_messages" => [], "attachments" => []}
         ],
         "submissions" => []

spec/apis/v1/submissions_api_spec.rb

@@ -258,6 +258,7 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
 
     a1 = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
     sub1 = submit_homework(a1, student1)
+    media_object(:media_id => "3232", :media_type => "audio")
     a1.grade_student(student1, {:grade => '90%', :comment => "Well here's the thing...", :media_comment_id => "3232", :media_comment_type => "audio"})
     comment = sub1.submission_comments.first
 
@@ -282,10 +283,11 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
         "submission_comments"=>
          [{"comment"=>"Well here's the thing...",
            "media_comment" => {
-             "media_comment_id"=>"3232",
-             "media_comment_type"=>"audio",
+             "media_id"=>"3232",
+             "media_type"=>"audio",
              "content-type" => "audio/mp4",
-             "url" => "http://www.example.com/courses/#{@course.id}/media_download?entryId=3232&redirect=1&type=mp4",
+             "url" => "http://www.example.com/users/#{@user.id}/media_download?entryId=3232&redirect=1&type=mp4",
+             "display_name" => nil
            },
            "created_at"=>comment.created_at.as_json,
            "author_name"=>"User",
@@ -327,7 +329,7 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
     course_with_teacher(:active_all => true)
     @course.enroll_student(student1).accept!
     a1 = @course.assignments.create!(:title => 'assignment1', :grading_type => 'letter_grade', :points_possible => 15)
-    MediaObject.create!(:media_id => "54321", :context => student1, :user => student1)
+    media_object(:media_id => "54321", :context => student1, :user => student1)
     mock_kaltura = mock(Kaltura::ClientV3)
     Kaltura::ClientV3.stub(:new).and_return(mock_kaltura)
     mock_kaltura.should_receive :startSession
@@ -362,11 +364,13 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
     a1.create_rubric_association(:rubric => rubric, :purpose => 'grading', :use_for_grading => true)
 
     submit_homework(a1, student1)
+    media_object(:media_id => "54321", :context => student1, :user => student1)
     submit_homework(a1, student1, :media_comment_id => "54321", :media_comment_type => "video")
     sub1 = submit_homework(a1, student1) { |s| s.attachments = [attachment_model(:context => student1, :folder => nil)] }
 
     sub2 = submit_homework(a1, student2, :url => "http://www.instructure.com") { |s| s.attachment = attachment_model(:context => s, :filename => 'snapshot.png', :content_type => 'image/png'); s.attachments = [attachment_model(:context => a1, :filename => 'ss2.png', :content_type => 'image/png')] }
 
+    media_object(:media_id => "3232", :context => student1, :user => student1, :media_type => "audio")
     a1.grade_student(student1, {:grade => '90%', :comment => "Well here's the thing...", :media_comment_id => "3232", :media_comment_type => "audio"})
     sub1.reload
     sub1.submission_comments.size.should == 1
@@ -411,10 +415,11 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
           {"grade"=>nil,
             "assignment_id" => a1.id,
            "media_comment" =>
-            { "media_comment_type"=>"video",
-              "media_comment_id"=>"54321",
+            { "media_type"=>"video",
+              "media_id"=>"54321",
               "content-type" => "video/mp4",
-              "url" => "http://www.example.com/courses/#{@course.id}/media_download?entryId=54321&redirect=1&type=mp4" },
+              "url" => "http://www.example.com/users/#{@user.id}/media_download?entryId=54321&redirect=1&type=mp4",
+              "display_name" => nil },
            "body"=>"test!",
            "submitted_at"=>"1970-01-01T02:00:00Z",
            "attempt"=>2,
@@ -427,9 +432,10 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
           {"grade"=>"A-",
             "assignment_id" => a1.id,
            "media_comment" =>
-            { "media_comment_type"=>"video",
-              "media_comment_id"=>"54321","content-type" => "video/mp4",
-              "url" => "http://www.example.com/courses/#{@course.id}/media_download?entryId=54321&redirect=1&type=mp4" },
+            { "media_type"=>"video",
+              "media_id"=>"54321","content-type" => "video/mp4",
+              "url" => "http://www.example.com/users/#{@user.id}/media_download?entryId=54321&redirect=1&type=mp4",
+              "display_name" => nil },
            "attachments" =>
             [
               { "content-type" => "application/loser",
@@ -453,19 +459,21 @@ def submit_homework(assignment, student, opts = {:body => "test!"})
         "submission_comments"=>
          [{"comment"=>"Well here's the thing...",
            "media_comment" => {
-             "media_comment_type"=>"audio",
-             "media_comment_id"=>"3232",
+             "media_type"=>"audio",
+             "media_id"=>"3232",
              "content-type" => "audio/mp4",
-             "url" => "http://www.example.com/courses/#{@course.id}/media_download?entryId=3232&redirect=1&type=mp4",
+             "url" => "http://www.example.com/users/#{@user.id}/media_download?entryId=3232&redirect=1&type=mp4",
+             "display_name" => nil
            },
            "created_at"=>comment.created_at.as_json,
            "author_name"=>"User",
            "author_id"=>student1.id}],
         "media_comment" =>
-         { "media_comment_type"=>"video",
-           "media_comment_id"=>"54321",
+         { "media_type"=>"video",
+           "media_id"=>"54321",
            "content-type" => "video/mp4",
-           "url" => "http://www.example.com/courses/#{@course.id}/media_download?entryId=54321&redirect=1&type=mp4" },
+           "url" => "http://www.example.com/users/#{@user.id}/media_download?entryId=54321&redirect=1&type=mp4",
+           "display_name" => nil },
         "score"=>13.5},
        {"grade"=>"F",
         "assignment_id" => a1.id,
@@ -937,6 +945,7 @@ def submit_with_grade(assignment_opts, param, score, grade)
     course_with_teacher(:active_all => true)
     @course.enroll_student(student).accept!
     @assignment = @course.assignments.create!(:title => 'assignment1', :grading_type => 'points', :points_possible => 12)
+    media_object(:media_id => "1234", :media_type => 'audio')
 
     json = api_call(:put,
           "/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}/submissions/#{student.id}.json",
@@ -951,7 +960,7 @@ def submit_with_grade(assignment_opts, param, score, grade)
     json['submission_comments'].size.should == 1
     comment = json['submission_comments'].first
     comment['comment'].should == 'This is a media comment.'
-    comment['media_comment']['url'].should == "http://www.example.com/courses/#{@course.id}/media_download?entryId=1234&redirect=1&type=mp4"
+    comment['media_comment']['url'].should == "http://www.example.com/users/#{@user.id}/media_download?entryId=1234&redirect=1&type=mp4"
     comment['media_comment']["content-type"].should == "audio/mp4"
   end
 

spec/spec_helper.rb

@@ -309,6 +309,15 @@ def conversation(*users)
     @conversation.reload
   end
 
+  def media_object(opts={})
+    mo = MediaObject.new
+    mo.media_id = opts[:media_id] || "1234"
+    mo.media_type = opts[:media_type] || "video"
+    mo.context = opts[:context] || @user || @course
+    mo.user = opts[:user] || @user
+    mo.save!
+  end
+
   def assert_status(status=500)
     response.status.to_i.should eql(status)
   end