content share index/show/delete/add_users endpoints

test plan:
 - render API docs
 - test API endpoints to
   * list sent content shares
     - includes type of item shared
     - admins can list other users' shares
     - avoids N+1 query
   * list received content shares
     - should not show other users who have received the same share
     - also avoids N+1 query
   * show a single content share
     - admins can see another user's share
     - scoped to the user in the path (/users/X/content_shares/Y
       404s if share Y does not belong to user X)
   * delete a content share
     - only the owner of the share can do this; user X cannot delete
       /users/Y/content_shares/...
   * add users to a content share
     - only the owner of the share can do this
     - returns 400 if you try to add yourself
     - silently ignores if you try to add someone who has already
       received the share
     - returns 400 if you try to add users to a content share
       that somebody else created and shared with you
   * change read state of a content share
     - only works with your own content share
     - rejects invalid read state

closes ADMIN-2810

flag=direct_share

Change-Id: I014f23c2aff1ba5a655e47148c8c859cfcd75d03
Reviewed-on: https://gerrit.instructure.com/205672
Tested-by: Jenkins
Reviewed-by: Mysti Lilla <mysti@instructure.com>
QA-Review: Carl Kibler <ckibler@instructure.com>
Product-Review: Carl Kibler <ckibler@instructure.com>
This commit is contained in:
Jeremy Stanley 2019-08-19 12:42:30 -06:00
parent 21dcd20aa6
commit d4f8f5e14e
5 changed files with 452 additions and 19 deletions

View File

@ -24,7 +24,7 @@
# @model ContentShare
# {
# "id": "ContentShare",
# "description": "Content shared between two users",
# "description": "Content shared between users",
# "properties": {
# "id": {
# "description": "The id of the content share for the current user",
@ -36,6 +36,11 @@
# "example": "War of 1812 homework",
# "type": "string"
# },
# "content_type": {
# "description": "The type of content that was shared. Can be assignment, discussion_topic, page, quiz, module, or module_item.",
# "example": "assignment",
# "type": "string"
# },
# "created_at": {
# "description": "The datetime the content was shared with this user.",
# "example": "2017-05-09T10:12:00Z",
@ -52,16 +57,21 @@
# "type": "integer"
# },
# "sender": {
# "description": "The user who shared the content. No sender information will be given for the sharing user.",
# "description": "The user who shared the content. This field is provided only to receivers; it is not populated in the sender's list of sent content shares.",
# "example": {"id": 1, "display_name": "Matilda Vargas", "avatar_image_url": "http:\/\/localhost:3000\/image_url", "html_url": "http:\/\/localhost:3000\/users\/1"},
# "type": "object"
# },
# "receivers": {
# "description": "An Array of users the content is shared with. An empty array will be returned for the receiving users.",
# "description": "An Array of users the content is shared with. This field is provided only to senders; an empty array will be returned for the receiving users.",
# "example": [{"id": 1, "display_name": "Jon Snow", "avatar_image_url": "http:\/\/localhost:3000\/image_url2", "html_url": "http:\/\/localhost:3000\/users\/2"}],
# "type": "array",
# "items": {"type": "object"}
# },
# "source_course": {
# "description": "The course the content was originally shared from.",
# "example": {"id": 787, "name": "History 105"},
# "type": "object"
# },
# "read_state": {
# "description": "Whether the recipient has viewed the content share.",
# "example": "read",
@ -79,26 +89,36 @@ class ContentSharesController < ApplicationController
page: WikiPage,
quiz: Quizzes::Quiz,
module: ContextModule,
module_item: ContentTag,
content_share: ContentShare
module_item: ContentTag
}.freeze
before_action :require_user
before_action :require_direct_share_enabled
before_action :require_user
before_action :get_user_param
before_action :require_current_user, :except => %w(show index)
before_action :get_receivers, :only => %w(create add_users)
def require_direct_share_enabled
render json: { message: "Feature disabled" }, status: :forbidden unless @domain_root_account.feature_enabled?(:direct_share)
end
def get_user_param
@user = api_find(User, params[:user_id])
end
def require_current_user
render json: { message: "You cannot create or modify other users' content shares" }, status: :forbidden unless @user == @current_user
end
# @API Create a content share
# Share content directly between two or more users
#
# @argument receiver_ids [Array]
# @argument receiver_ids [Required, Array]
# IDs of users to share the content with.
#
# @argument content_type [Required, String, "assignment"|"discussion_topic"|"page"|"quiz"|"module"|"module_item"]
# Type of content you are sharing. 'content_share' allows you to re-share content that is already shared.
# Type of content you are sharing.
#
# @argument content_id [Required, Integer]
# The id of the content that you are sharing
@ -106,22 +126,16 @@ class ContentSharesController < ApplicationController
#
# @example_request
#
# curl 'https://<canvas>/api/v1/content_shares \
# curl 'https://<canvas>/api/v1/users/self/content_shares \
# -d 'content_type=assignment' \
# -d 'content_id=1' \
# -H 'Authorization: Bearer <token>' \
# -X POST
#
# @returns ContentShare
#
def create
unless @current_user == api_find(User, params[:user_id])
return render(json: { message: 'Cannot create content shares for other users'}, status: :forbidden)
end
create_params = params.permit(:content_type, :content_id, receiver_ids: [])
create_params = params.permit(:content_type, :content_id)
allowed_types = ['assignment', 'discussion_topic', 'page', 'quiz', 'module', 'module_item']
receivers = User.active.where(id: create_params[:receiver_ids])
return render(json: { message: 'No valid receiving users found' }, status: :bad_request) unless receivers.any?
unless create_params[:content_type] && create_params[:content_id]
return render(json: { message: 'Content type and id required'}, status: :bad_request)
end
@ -148,10 +162,124 @@ class ContentSharesController < ApplicationController
name = Context.asset_name(content)
sender_share = @current_user.sent_content_shares.create(content_export: export, name: name, read_state: 'read')
receivers.each do |receiver|
receiver.received_content_shares.create(content_export: export, sender: @current_user, name: name, read_state: 'unread')
end
create_receiver_shares(sender_share, @receivers)
render json: content_share_json(sender_share, @current_user, session), status: :created
end
# @API List content shares
# Return a paginated list of content shares a user has sent or received. Use +self+ as the user_id
# to retrieve your own content shares. Only linked observers and administrators may view other users'
# content shares.
#
# @example_request
#
# curl 'https://<canvas>/api/v1/users/self/content_shares/received'
#
# @returns [ContentShare]
def index
if authorized_action(@user, @current_user, :read)
if params[:list] == 'received'
shares = Api.paginate(@user.received_content_shares.by_date, self, api_v1_user_received_content_shares_url)
render json: received_content_shares_json(shares, @current_user, session)
else
shares = Api.paginate(@user.sent_content_shares.by_date, self, api_v1_user_sent_content_shares_url)
render json: sent_content_shares_json(shares, @current_user, session)
end
end
end
# @API Get content share
# Return information about a single content share. You may use +self+ as the user_id to retrieve your own content share.
#
# @example_request
#
# curl 'https://<canvas>/api/v1/users/self/content_shares/123'
#
# @returns ContentShare
def show
if authorized_action(@user, @current_user, :read)
@content_share = @user.content_shares.find(params[:id])
render json: content_share_json(@content_share, @current_user, session)
end
end
# @API Remove content share
# Remove a content share from your list. Use +self+ as the user_id. Note that this endpoint does not delete other users'
# copies of the content share.
#
# @example_request
#
# curl -X DELETE 'https://<canvas>/api/v1/users/self/content_shares/123'
def destroy
@content_share = @current_user.content_shares.find(params[:id])
@content_share.destroy
render json: { message: 'content share deleted' }
end
# @API Add users to content share
# Send a previously created content share to additional users
#
# @argument receiver_ids [Array]
# IDs of users to share the content with.
#
# @example_request
#
# curl -X POST 'https://<canvas>/api/v1/users/self/content_shares/123/add_users?receiver_ids[]=789'
#
# @returns ContentShare
def add_users
@content_share = @current_user.content_shares.find(params[:id])
reject!('Content share not owned by you') unless @content_share.is_a?(SentContentShare)
create_receiver_shares(@content_share, @receivers - @content_share.receivers)
@content_share.reload
render json: content_share_json(@content_share, @current_user, session)
end
# @API Update a content share
# Mark a content share read or unread
#
# @argument read_state [String, "read"|"unread"]
# Read state for the content share
#
# @example_request
#
# curl -X PUT 'https://<canvas>/api/v1/users/self/content_shares/123?read_state=read'
#
# @returns ContentShare
def update
@content_share = @current_user.content_shares.find(params[:id])
update_params = params.permit(:read_state)
@content_share.update_attributes(update_params)
if @content_share.save
render json: content_share_json(@content_share, @current_user, session)
else
render json: @content_share.errors.to_json, :status => 400
end
end
private
def get_receivers
receiver_ids = params.require(:receiver_ids)
@receivers = api_find_all(User, Array(receiver_ids))
unless @receivers.any?
render(json: { message: 'No valid receiving users found' }, status: :bad_request)
return false
end
if @receivers.include?(@current_user)
render(json: { message: 'You cannot share with yourself' }, status: :bad_request)
return false
end
# TODO verify we're allowed to send content to these users, once we decide how to do that
end
def create_receiver_shares(sender_share, receivers)
receivers.each do |receiver|
sender_share.clone_for(receiver)
end
end
end

View File

@ -21,4 +21,16 @@ class ContentShare < ActiveRecord::Base
belongs_to :user
belongs_to :content_export
validates :read_state, inclusion: { in: %w(read unread) }
scope :by_date, -> { order(created_at: :desc) }
def clone_for(receiver)
receiver.received_content_shares.create!(sender_id: self.user_id,
content_export_id: self.content_export_id,
name: self.name,
read_state: 'unread')
end
end

View File

@ -2186,6 +2186,12 @@ CanvasRails::Application.routes.draw do
scope(controller: :content_shares) do
post 'users/:user_id/content_shares', action: :create
get 'users/:user_id/content_shares/sent', action: :index, defaults: { list: 'sent' }, as: :user_sent_content_shares
get 'users/:user_id/content_shares/received', action: :index, defaults: { list: 'received' }, as: :user_received_content_shares
get 'users/:user_id/content_shares/:id', action: :show
delete 'users/:user_id/content_shares/:id', action: :destroy
post 'users/:user_id/content_shares/:id/add_users', action: :add_users
put 'users/:user_id/content_shares/:id', action: :update
end
scope(:controller => :csp_settings) do

View File

@ -19,10 +19,52 @@ module Api::V1::ContentShare
include Api::V1::Json
include Api::V1::ContentExport
# map content export selected-content collection to API-visible content type
EXPORT_TYPES = {
'assignments' => 'assignment',
'discussion_topics' => 'discussion_topic',
'wiki_pages' => 'page',
'quizzes' => 'quiz',
'context_modules' => 'module',
'content_tags' => 'module_item'
}.freeze
def content_share_json(content_share, user, session, opts = {})
json = api_json(content_share, user, session, opts.merge(only: %w(id name created_at updated_at user_id read_state)))
json['sender'] = content_share.respond_to?(:sender) ? user_display_json(content_share.sender) : nil
json['receivers'] = content_share.respond_to?(:receivers) ? content_share.receivers.map {|rec| user_display_json(rec)} : []
json['content_type'] = get_content_type_from_export_settings(content_share.content_export.settings)
if content_share.content_export.context_type == 'Course'
json['source_course'] = {
id: content_share.content_export.context.id,
name: content_share.content_export.context.nickname_for(user)
}
end
json
end
def sent_content_shares_json(content_shares, user, session, opts = {})
ActiveRecord::Associations::Preloader.new.preload(content_shares, [{:content_export => :context}, :receivers])
content_shares_json(content_shares, user, session, opts)
end
def received_content_shares_json(content_shares, user, session, opts = {})
ActiveRecord::Associations::Preloader.new.preload(content_shares, [{:content_export => :context}, :sender])
content_shares_json(content_shares, user, session, opts)
end
def content_shares_json(content_shares, user, session, opts = {})
content_shares.map do |content_share|
content_share_json(content_share, user, session, opts)
end
end
private
def get_content_type_from_export_settings(settings)
return nil unless settings.key?('selected_content')
selected_types = settings['selected_content'].keys.map { |k| EXPORT_TYPES[k] }.compact
%w(module module_item).each { |k| return k if selected_types.include?(k) }
# otherwise there should be only one selected type...
selected_types.first
end
end

View File

@ -83,4 +83,249 @@ describe ContentSharesController do
expect(response).to have_http_status(:forbidden)
end
end
describe "rest of CRUD" do
before :once do
@export = @course_1.content_exports.create!(settings: {"selected_content" => {"assignments" => {CC::CCHelper.create_key(@assignment) => '1'}}})
@sent_share = @teacher_1.sent_content_shares.create! name: 'booga', content_export: @export, read_state: 'read'
@received_share = @teacher_2.received_content_shares.create! name: 'booga', content_export: @export, sender: @teacher_1, read_state: 'unread'
end
describe "GET #index" do
it "lists sent content shares" do
user_session @teacher_1
get :index, params: { user_id: @teacher_1.id, list: 'sent' }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['id']).to eq @sent_share.id
expect(json[0]['name']).to eq 'booga'
expect(json[0]['read_state']).to eq 'read'
expect(json[0]['sender']).to be_nil
expect(json[0]['receivers'].length).to eq 1
expect(json[0]['receivers'][0]['id']).to eq @teacher_2.id
expect(json[0]['content_type']).to eq 'assignment'
expect(json[0]['source_course']).to eq({'id' => @course_1.id, 'name' => @course_1.name})
end
it "paginates sent content shares" do
Timecop.travel(1.hour.ago) do
export2 = @course_1.content_exports.create!(settings: {"selected_content" => {"assignments" => {'foo' => '1'}, "content_tags" => {'bar' => '1'}}})
sent_share2 = @teacher_1.sent_content_shares.create! name: 'ooga', content_export: export2, read_state: 'read'
end
user_session @teacher_1
get :index, params: { user_id: 'self', list: 'sent', per_page: 1 }
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['name']).to eq 'booga'
expect(json[0]['content_type']).to eq 'assignment'
links = Api.parse_pagination_links(response.headers['Link'])
link = links.detect { |link| link[:rel] == 'next' }
expect(link[:uri].path).to eq '/api/v1/users/self/content_shares/sent'
expect(link[:uri].query).to include 'page=2'
expect(link[:uri].query).to include 'per_page=1'
get :index, params: { user_id: 'self', list: 'sent', per_page: 1, page: 2 }
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['name']).to eq 'ooga'
expect(json[0]['content_type']).to eq 'module_item'
links = Api.parse_pagination_links(response.headers['Link'])
expect(links.detect { |link| link[:rel] == 'next' }).to be_nil
end
it "lists received content shares" do
user_session @teacher_2
get :index, params: { user_id: @teacher_2.id, list: 'received' }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['id']).to eq @received_share.id
expect(json[0]['name']).to eq 'booga'
expect(json[0]['read_state']).to eq 'unread'
expect(json[0]['sender']['id']).to eq @teacher_1.id
expect(json[0]['receivers']).to eq([])
expect(json[0]['content_type']).to eq 'assignment'
expect(json[0]['source_course']).to eq({'id' => @course_1.id, 'name' => @course_1.name})
end
it "paginates received content shares" do
Timecop.travel(1.hour.ago) do
export2 = @course_1.content_exports.create!(settings: {"selected_content" => {"quizzes" => {'foo' => '1'}, "content_tags" => {'bar' => '1'}, "context_modules" => {'baz' => '1'}}})
received_share2 = @teacher_2.received_content_shares.create! name: 'ooga', content_export: export2, sender_id: user_with_pseudonym, read_state: 'unread'
end
user_session @teacher_2
get :index, params: { user_id: 'self', list: 'received', per_page: 1 }
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['name']).to eq 'booga'
expect(json[0]['content_type']).to eq 'assignment'
links = Api.parse_pagination_links(response.headers['Link'])
link = links.detect { |link| link[:rel] == 'next' }
expect(link[:uri].path).to eq '/api/v1/users/self/content_shares/received'
expect(link[:uri].query).to include 'page=2'
expect(link[:uri].query).to include 'per_page=1'
get :index, params: { user_id: 'self', list: 'received', per_page: 1, page: 2 }
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json.length).to eq 1
expect(json[0]['name']).to eq 'ooga'
expect(json[0]['content_type']).to eq 'module'
links = Api.parse_pagination_links(response.headers['Link'])
expect(links.detect { |link| link[:rel] == 'next' }).to be_nil
end
it "requires permission on user" do
user_session @teacher_1
get :index, params: { user_id: @teacher_2.id, list: 'received' }
expect(response).to have_http_status(:unauthorized)
end
end
describe "GET #show" do
it "returns a content share" do
user_session @teacher_1
get :show, params: { user_id: @teacher_1.id, id: @sent_share.id }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json['id']).to eq @sent_share.id
expect(json['name']).to eq 'booga'
expect(json['read_state']).to eq 'read'
expect(json['sender']).to be_nil
expect(json['receivers'].length).to eq 1
expect(json['receivers'][0]['id']).to eq @teacher_2.id
expect(json['content_type']).to eq 'assignment'
expect(json['source_course']).to eq({'id' => @course_1.id, 'name' => @course_1.name})
end
it "scopes to user" do
user_session @teacher_1
get :show, params: { user_id: @teacher_1.id, id: @received_share.id }
expect(response).to have_http_status(:not_found)
end
end
describe "DELETE #destroy" do
it "deletes a content share" do
user_session @teacher_1
delete :destroy, params: { user_id: @teacher_1.id, id: @sent_share.id }
expect(response).to be_successful
expect(ContentShare.where(id: @sent_share.id).exists?).to eq false
end
it "scopes to user" do
user_session @teacher_2
delete :destroy, params: { user_id: @teacher_2.id, id: @sent_share.id }
expect(response).to have_http_status(:not_found)
end
it "requires user=self" do
user_session @teacher_1
delete :destroy, params: { user_id: @teacher_2.id, id: @sent_share.id }
expect(response).to have_http_status(:forbidden)
end
end
describe "POST #add_users" do
before :once do
@teacher_3 = user_with_pseudonym(active_user: true)
end
it "adds users" do
user_session @teacher_1
post :add_users, params: { user_id: @teacher_1.id, id: @sent_share.id, receiver_ids: [@teacher_3.id] }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json['receivers'].length).to eq 2
expect(json['receivers'].map { |r| r['id'] }).to match_array([@teacher_2.id, @teacher_3.id])
expect(@sent_share.receivers.pluck(:id)).to match_array([@teacher_2.id, @teacher_3.id])
end
it "ignores users already shared" do
user_session @teacher_1
post :add_users, params: { user_id: @teacher_1.id, id: @sent_share.id, receiver_ids: [@teacher_2.id, @teacher_3.id] }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json['receivers'].length).to eq 2
expect(json['receivers'].map { |r| r['id'] }).to match_array([@teacher_2.id, @teacher_3.id])
expect(@sent_share.receivers.pluck(:id)).to match_array([@teacher_2.id, @teacher_3.id])
end
it "disallows sharing with yourself" do
user_session @teacher_1
post :add_users, params: { user_id: @teacher_1.id, id: @sent_share.id, receiver_ids: [@teacher_1.id, @teacher_3.id] }
expect(response).to have_http_status(:bad_request)
expect(response.body).to include 'You cannot share with yourself'
end
it "complains if no valid users are included" do
user_session @teacher_1
post :add_users, params: { user_id: @teacher_1.id, id: @sent_share.id, receiver_ids: [0] }
expect(response).to have_http_status(:bad_request)
expect(response.body).to include 'No valid receiving users found'
end
it "disallows resharing somebody else's share" do
user_session @teacher_2
post :add_users, params: { user_id: @teacher_2.id, id: @received_share.id, receiver_ids: [@teacher_3.id] }
expect(response).to have_http_status(:bad_request)
expect(response.body).to include 'Content share not owned by you'
end
it "scopes to user" do
user_session @teacher_2
post :add_users, params: { user_id: @teacher_2.id, id: @sent_share.id, receiver_ids: [@teacher_3.id] }
expect(response).to have_http_status(:not_found)
end
it "requires user=self" do
user_session @teacher_2
post :add_users, params: { user_id: @teacher_1.id, id: @sent_share.id, receiver_ids: [@teacher_3.id] }
expect(response).to have_http_status(:forbidden)
end
end
describe "PUT #update" do
it "marks a content share read" do
user_session @teacher_2
put :update, params: { user_id: @teacher_2.id, id: @received_share.id, read_state: 'read' }
expect(response).to be_successful
json = JSON.parse(response.body.sub(/^while\(1\);/, ''))
expect(json['read_state']).to eq 'read'
expect(@received_share.reload.read_state).to eq 'read'
end
it "rejects an invalid read state" do
user_session @teacher_2
put :update, params: { user_id: @teacher_2.id, id: @received_share.id, read_state: 'malarkey' }
expect(response).to have_http_status(:bad_request)
end
it "ignores invalid attributes" do
user_session @teacher_2
put :update, params: { user_id: @teacher_2.id, id: @received_share.id, content_export_id: 0, read_state: 'read' }
expect(response).to be_successful
expect(@received_share.reload.read_state).to eq 'read'
expect(@received_share.content_export_id).to eq @export.id
end
it "scopes to user" do
user_session @teacher_1
put :update, params: { user_id: @teacher_1.id, id: @received_share.id, read_state: 'read' }
expect(response).to have_http_status(:not_found)
end
it "requires user=self" do
user_session @teacher_1
put :update, params: { user_id: @teacher_2.id, id: @received_share.id, read_state: 'read' }
expect(response).to have_http_status(:forbidden)
end
end
end
end