return file's uploading user in api

test plan:
 - consult api docs for files index and show actions
 - upload some files via the files page, putting files
   up with at least two different users
 - do index with ?include[]=user
  * user who uploaded each file should be returned
 - do show with ?include[]=user
  * user who uploaded the file should be returned

fixes CNVS-11534

Change-Id: I973f96e67d80a5c000482c60d57f928004901f2a
Reviewed-on: https://gerrit.instructure.com/33622
Tested-by: Jenkins <[email protected]>
Reviewed-by: Mark Severson <[email protected]>
QA-Review: Clare Strong <[email protected]>
Reviewed-by: Ryan Shaw <[email protected]>
Product-Review: Ryan Shaw <[email protected]>

app/controllers/files_controller.rb

@@ -211,6 +211,11 @@ def index
   # @argument search_term [Optional, String]
   #   The partial name of the files to match and return.
   #
+  # @argument include[] [Optional, "user"]
+  #   Array of additional information to include.
+  #
+  #   "user":: the user who uploaded the file or last edited its content
+  #
   # @example_request
   #
   #   curl 'https://<canvas>/api/v1/folders/<folder_id>/files?content_types[]=image&content_types[]=text/plain \
@@ -231,6 +236,7 @@ def api_index
     if authorized_action(folder, @current_user, :read_contents)
       @context = folder.context unless context_index
       can_manage_files = @context.grants_right?(@current_user, session, :manage_files)
+      params[:include] = Array(params[:include])
 
       if context_index
         if can_manage_files
@@ -246,6 +252,7 @@ def api_index
           scope = folder.visible_file_attachments.not_hidden.not_locked
         end
       end
+      scope = scope.includes(:user) if params[:include].include? 'user'
       scope = Attachment.search_by_attribute(scope, :display_name, params[:search_term])
       if params[:sort_by] == 'position'
         scope = scope.by_position_then_display_name
@@ -259,7 +266,7 @@ def api_index
 
       url = context_index ? context_files_url : api_v1_list_files_url(folder)
       @files = Api.paginate(scope, self, url)
-      render :json => attachments_json(@files, @current_user, {}, :can_manage_files => can_manage_files)
+      render :json => attachments_json(@files, @current_user, {}, :can_manage_files => can_manage_files, :include => params[:include])
     end
   end
 
@@ -356,6 +363,10 @@ def public_url
   # @API Get file
   # Returns the standard attachment json object
   #
+  # @argument include[] [Optional, "user"]
+  #   Array of additional information to include.
+  #
+  #   "user":: the user who uploaded the file or last edited its content
   #
   # @example_request
   #
@@ -366,8 +377,9 @@ def public_url
   def api_show
     @attachment = Attachment.find(params[:id])
     raise ActiveRecord::RecordNotFound if @attachment.deleted?
+    params[:include] = Array(params[:include])
     if authorized_action(@attachment,@current_user,:read)
-      render :json => attachment_json(@attachment, @current_user)
+      render :json => attachment_json(@attachment, @current_user, {}, { include: params[:include] })
     end
   end
 

lib/api/v1/attachment.rb

@@ -19,6 +19,7 @@
 module Api::V1::Attachment
   include Api::V1::Json
   include Api::V1::Locked
+  include Api::V1::User
 
   def attachments_json(files, user, url_options = {}, options = {})
     files.map do |f|
@@ -81,6 +82,7 @@ def attachment_json(attachment, user, url_options = {}, options = {})
       'thumbnail_url' => thumbnail_download_url,
     }
     locked_json(hash, attachment, user, 'file')
+    hash['user'] = user_display_json(attachment.user, attachment.context) if (options[:include] || []).include? 'user'
     hash
   end
 

spec/apis/v1/files_controller_api_spec.rb

@@ -263,6 +263,21 @@ def call_create_success
       json = api_call(:get, @files_path + "?search_term=fir", @files_path_options.merge(:search_term => 'fir'), {})
       json.map{|h| h['id']}.sort.should == atts.map(&:id).sort
     end
+
+    it "should include user if requested" do
+      @a1.update_attribute(:user, @user)
+      json = api_call(:get, @files_path + "?include[]=user", @files_path_options.merge(include: ['user']))
+      json.map{|f|f['user']}.should eql [
+        {},
+        {},
+        {
+          "id" => @user.id,
+          "display_name" => @user.short_name,
+          "avatar_image_url" => User.avatar_fallback_url,
+          "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}"
+        }
+      ]
+    end
   end
 
   describe "#index for courses" do
@@ -472,6 +487,17 @@ def should_be_locked(json)
       @att.save!
       api_call(:get, @file_path, @file_path_options, {}, {}, :expected_status => 200)
     end
+
+    it "should return user if requested" do
+      @att.update_attribute(:user, @user)
+      json = api_call(:get, @file_path + "?include[]=user", @file_path_options.merge(include: ['user']))
+      json['user'].should eql({
+        "id" => @user.id,
+        "display_name" => @user.short_name,
+        "avatar_image_url" => User.avatar_fallback_url,
+        "html_url" => "http://www.example.com/courses/#{@course.id}/users/#{@user.id}"
+      })
+    end
   end
 
   describe "#destroy" do