add a batch_update method to conversations api

The batch update runs in a background job and updates a
Progress record while it runs. Based on the recently added
course controller's batch_update method.

fixes CNVS-2962

test plan:
 - read and check the api documentation for the conversations
   batch_update call
 - create some conversations for a user using the GUI
 - use the new api call to perform batch updates on that user's
   conversations
 - refresh the user's inbox and verify the conversations were
   updated properly.

Change-Id: I185bb5402675b6f0475efed21a525c5d4d8a39cb
Reviewed-on: https://gerrit.instructure.com/17556
Tested-by: Jenkins <jenkins@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
Reviewed-by: Mark Ericksen <marke@instructure.com>
This commit is contained in:
Jon Willesen 2013-02-05 16:31:21 -07:00
parent fc2982a072
commit 0ebaa016a8
9 changed files with 588 additions and 41 deletions

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 Instructure, Inc.
# Copyright (C) 2011 - 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -23,6 +23,7 @@ class ConversationsController < ApplicationController
include ConversationsHelper
include SearchHelper
include Api::V1::Conversation
include Api::V1::Progress
before_filter :require_user, :except => [:public_feed]
before_filter :reject_student_view_student
@ -554,6 +555,37 @@ class ConversationsController < ApplicationController
end
end
# @API Batch update conversations
# Perform a change on a set of conversations. Operates asynchronously; use the {api:ProgressController#show progress endpoint}
# to query the status of an operation.
#
# @argument conversation_ids[] List of conversations to update. Limited to 500 conversations.
# @argument event The action to take on each conversation. Must be one of 'mark_as_read', 'mark_as_unread', 'star', 'unstar', 'archive', 'destroy'
#
# @example_request
# curl https://<canvas>/api/v1/conversations \
# -X PUT \
# -H 'Authorization: Bearer <token>' \
# -d 'event=mark_as_read' \
# -d 'conversation_ids[]=1' \
# -d 'conversation_ids[]=2'
#
# @returns Progress
def batch_update
conversation_ids = params[:conversation_ids]
update_params = params.slice(:event).with_indifferent_access
allowed_events = %w(mark_as_read mark_as_unread star unstar archive destroy)
return render(:json => {:message => 'conversation_ids not specified'}, :status => :bad_request) unless params[:conversation_ids].is_a?(Array)
return render(:json => {:message => 'conversation batch size limit (500) exceeded'}, :status => :bad_request) unless params[:conversation_ids].size <= 500
return render(:json => {:message => 'event not specified'}, :status => :bad_request) unless update_params[:event]
return render(:json => {:message => 'invalid event'}, :status => :bad_request) unless allowed_events.include? update_params[:event]
progress = ConversationParticipant.batch_update(@current_user, conversation_ids, update_params)
render :json => progress_json(progress, @current_user, session)
end
# @API Find recipients
#
# Deprecated, see the {api:SearchController#recipients Find recipients endpoint} in the Search API

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 Instructure, Inc.
# Copyright (C) 2011 - 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -374,6 +374,56 @@ class ConversationParticipant < ActiveRecord::Base
self.find(:all, :select => 'conversation_id', :order => order).map(&:conversation_id)
end
def update_one(update_params)
case update_params[:event]
when 'mark_as_read'
self.workflow_state = 'read'
when 'mark_as_unread'
self.workflow_state = 'unread'
when 'archive'
self.workflow_state = 'archived'
when 'star'
self.starred = true
when 'unstar'
self.starred = false
when 'destroy'
self.remove_messages(:all)
end
self.save!
end
def self.do_batch_update(progress, user, conversation_ids, update_params)
progress_runner = ProgressRunner.new(progress)
progress_runner.completed_message do |completed_count|
t('batch_update_message', {
:one => "1 conversation processed",
:other => "%{count} conversations processed"
},
:count => completed_count)
end
progress_runner.do_batch_update(conversation_ids) do |conversation_id|
participant = user.all_conversations.find_by_conversation_id(conversation_id)
raise t('not_participating', 'The user is not participating in this conversation') unless participant
participant.update_one(update_params)
end
end
def self.batch_update(user, conversation_ids, update_params)
progress = user.progresses.create! :tag => "conversation_batch_update", :completion => 0.0
job = ConversationParticipant.send_later(:do_batch_update, progress, user, conversation_ids, update_params)
progress.user_id = user.id
progress.delayed_job_id = job.id
progress.save!
progress
end
protected
def message_tags
messages.map(&:tags).inject([], &:concat).uniq

View File

@ -3035,48 +3035,24 @@ class Course < ActiveRecord::Base
end
def self.do_batch_update(progress, user, course_ids, update_params)
begin
account = progress.context
progress.start!
update_every = [course_ids.size / 20, 4].max
completed_count = failed_count = 0
errors = {}
course_ids.each_slice(update_every) do |batch|
batch.each do |course_id|
course = account.associated_courses.find_by_id(course_id)
begin
raise t('course_not_found', "The course was not found") unless course
raise t('access_denied', "Access was denied") unless course.grants_right? user, :update
course.update_one(update_params)
completed_count += 1
rescue Exception => e
message = e.message
(errors[message] ||= []) << course_id
failed_count += 1
end
end
progress.update_completion! 100.0 * (completed_count + failed_count) / course_ids.size
end
progress.completion = 100.0
progress.message = t('batch_update_message', {
account = progress.context
progress_runner = ProgressRunner.new(progress)
progress_runner.completed_message do |completed_count|
t('batch_update_message', {
:one => "1 course processed",
:other => "%{count} courses processed"
},
:count => completed_count)
errors.each do |message, ids|
progress.message += t('batch_update_error', "\n%{error}: %{ids}", :error => message, :ids => ids.join(', '))
end
if completed_count > 0
progress.complete!
else
progress.fail!
end
progress.save
rescue Exception => e
progress.message = e.message
progress.fail!
progress.save
end
progress_runner.do_batch_update(course_ids) do |course_id|
course = account.associated_courses.find_by_id(course_id)
raise t('course_not_found', "The course was not found") unless course
raise t('access_denied', "Access was denied") unless course.grants_right? user, :update
course.update_one(update_params)
end
end
def self.batch_update(account, user, course_ids, update_params)

View File

@ -40,4 +40,9 @@ class Progress < ActiveRecord::Base
def update_completion!(value)
update_attribute(:completion, value)
end
def calculate_completion!(current_value, total)
update_completion!(100.0 * current_value / total)
end
end

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 - 2012 Instructure, Inc.
# Copyright (C) 2011 - 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -168,6 +168,8 @@ class User < ActiveRecord::Base
has_one :profile, :class_name => 'UserProfile'
alias :orig_profile :profile
has_many :progresses, :as => :context
belongs_to :otp_communication_channel, :class_name => 'CommunicationChannel'
include StickySisFields

View File

@ -901,6 +901,7 @@ ActionController::Routing::Routes.draw do |map|
conversations.post 'conversations/:id/add_message', :action => :add_message
conversations.post 'conversations/:id/add_recipients', :action => :add_recipients
conversations.post 'conversations/:id/remove_messages', :action => :remove_messages
conversations.put 'conversations', :action => :batch_update
end
api.with_options(:controller => :communication_channels) do |channels|

114
lib/progress_runner.rb Normal file
View File

@ -0,0 +1,114 @@
#
# Copyright (C) 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
class ProgressRunner
attr_reader :progress, :completed_count, :failed_count, :errors
# Initialize the ProgressRunner
# @param progress [Progress] The Progress record to update during {#do_batch_update}
def initialize(progress)
@progress = progress
@completed_count = @failed_count = 0
@errors = {}
@completed_message = method(:default_completed_message)
@error_message = method(:default_error_message)
end
# Process a list of elements, periodically updating the Progress record and finalizing it when
# processing is complete. If processing raises exceptions, the exception messages are recorded
# as error messages and recorded in the Progress object.
#
# @param elements [Array] The collection of elements to process
# @param process_element A block that performs the actually processing on each element.
# Passed an individual element as a parameter.
def do_batch_update(elements, &process_element)
raise 'block required' unless block_given?
@progress.start!
update_every = [elements.size / 20, 4].max
elements.each_slice(update_every) do |batch|
update_batch(batch, &process_element)
@progress.calculate_completion!(@completed_count + @failed_count, elements.size)
end
finish_update
rescue => e
@progress.message = e.message
@progress.fail!
@progress.save
end
# Provide a custom "X items processed" message. The provided block overrides the default.
# @param block The block to call to format the completed message.
# @see #default_completed_message
def completed_message(&block)
raise 'block required' unless block_given?
@completed_message = block
self
end
# Provide a custom error message formatter. The provided block overrides the default
# @param block The block to call to format an error message. See #default_error_message
def error_message(&block)
raise 'block required' unless block_given?
@error_message = block
self
end
# The default completed message.
# @param [Integer] completed_count The number of items that were processed successfully.
def default_completed_message(completed_count)
I18n.t('lib.progress_runner.completed_message', {
:one => "1 item processed",
:other => "%{count} items processed"
},
:count => completed_count)
end
# The default error message formatter.
# @param message [String] The error message that was encountered for the specified elements.
# @param elements [Array] A list of elements this error message applies to.
def default_error_message(message, elements)
I18n.t('lib.progress_runner.error_message', "%{error}: %{ids}", :error => message, :ids => elements.join(', '))
end
private
def update_batch(batch, &process_element)
batch.each do |element|
update_element(element, &process_element)
end
end
def update_element(element, &process_element)
process_element.call(element)
@completed_count += 1
rescue => e
(@errors[e.message] ||= []) << element
@failed_count += 1
end
def finish_update
@progress.completion = 100.0
@progress.message = @completed_message.call(@completed_count)
@errors.each do |message, elements|
@progress.message += "\n" + @error_message.call(message, elements)
end
@completed_count > 0 ? @progress.complete! : @progress.fail!
@progress.save
end
end

View File

@ -1,5 +1,5 @@
#
# Copyright (C) 2011 Instructure, Inc.
# Copyright (C) 2011 - 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
@ -1007,4 +1007,238 @@ describe ConversationsController, :type => :integration do
json.first['visible'].should be_true
end
end
describe "bulk updates" do
it "should mark conversations as read" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
c1.reload.should be_read
c2.reload.should be_read
@me.reload.unread_conversations_count.should eql(0)
end
it "should mark conversations as unread" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_unread', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
c1.reload.should be_unread
c2.reload.should be_unread
@me.reload.unread_conversations_count.should eql(2)
end
it "should mark conversations as starred" do
c1 = conversation(@me, @bob, :workflow_state => 'unread', :starred => true)
c2 = conversation(@me, @jane, :workflow_state => 'read')
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'star', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
c1.reload.starred.should be_true
c2.reload.starred.should be_true
@me.reload.unread_conversations_count.should eql(1)
end
it "should mark conversations as unstarred" do
c1 = conversation(@me, @bob, :workflow_state => 'unread', :starred => true)
c2 = conversation(@me, @jane, :workflow_state => 'read')
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'unstar', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
c1.reload.starred.should be_false
c2.reload.starred.should be_false
@me.reload.unread_conversations_count.should eql(1)
end
# it "should mark conversations as subscribed"
# it "should mark conversations as unsubscribed"
it "should archive conversations" do
conversations = %w(archived read unread).map do |state|
conversation(@me, @bob, :workflow_state => state)
end
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = conversations.map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'archive', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
conversations.each do |c|
c.reload.should be_archived
end
@me.reload.unread_conversations_count.should eql(0)
end
it "should destroy conversations" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
@me.reload.unread_conversations_count.should eql(1)
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'destroy', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.message.to_s.should include "#{conversation_ids.size} conversations processed"
c1.reload.messages.should be_empty
c2.reload.messages.should be_empty
@me.reload.unread_conversations_count.should eql(0)
end
describe "immediate failures" do
it "should fail if event is invalid" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'NONSENSE', :conversation_ids => conversation_ids },
{}, {:expected_status => 400})
json['message'].should include 'invalid event'
end
it "should fail if event parameter is not specified" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :conversation_ids => conversation_ids },
{}, {:expected_status => 400})
json['message'].should include 'event not specified'
end
it "should fail if conversation_ids is not specified" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read' },
{}, {:expected_status => 400})
json['message'].should include 'conversation_ids not specified'
end
it "should fail if batch size limit is exceeded" do
conversation_ids = (1..501).to_a
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids },
{}, {:expected_status => 400})
json['message'].should include 'exceeded'
end
end
describe "progress" do
it "should create and update a progress object" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
conversation_ids = [c1,c2].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids })
progress = Progress.find(json['id'])
progress.should be_present
progress.should be_queued
progress.completion.should eql(0.0)
run_jobs
progress.reload.should be_completed
progress.completion.should eql(100.0)
end
describe "progress failures" do
it "should not update conversations the current user does not participate in" do
c1 = conversation(@me, @bob, :workflow_state => 'unread')
c2 = conversation(@me, @jane, :workflow_state => 'read')
c3 = conversation(@bob, @jane, :sender => @bob, :workflow_state => 'unread')
conversation_ids = [c1,c2,c3].map {|c| c.conversation.id}
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.should be_completed
progress.completion.should eql(100.0)
c1.reload.should be_read
c2.reload.should be_read
c3.reload.should be_unread
progress.message.should include 'not participating'
progress.message.should include '2 conversations processed'
end
it "should fail if all conversation ids are invalid" do
c1 = conversation(@bob, @jane, :sender => @bob, :workflow_state => 'unread')
conversation_ids = [c1.conversation.id]
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.should be_failed
progress.completion.should eql(100.0)
c1.reload.should be_unread
progress.message.should include 'not participating'
progress.message.should include '0 conversations processed'
end
it "should fail progress if exception is raised in job" do
begin
Progress.any_instance.stubs(:complete!).raises "crazy exception"
c1 = conversation(@me, @jane, :workflow_state => 'unread')
conversation_ids = [c1.conversation.id]
json = api_call(:put, "/api/v1/conversations",
{ :controller => 'conversations', :action => 'batch_update', :format => 'json' },
{ :event => 'mark_as_read', :conversation_ids => conversation_ids })
run_jobs
progress = Progress.find(json['id'])
progress.should be_failed
progress.message.should include 'crazy exception'
ensure
Progress.any_instance.unstub(:complete!)
end
end
end
end
end
end

View File

@ -0,0 +1,133 @@
#
# Copyright (C) 2013 Instructure, Inc.
#
# This file is part of Canvas.
#
# Canvas is free software: you can redistribute it and/or modify it under
# the terms of the GNU Affero General Public License as published by the Free
# Software Foundation, version 3 of the License.
#
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
# details.
#
# You should have received a copy of the GNU Affero General Public License along
# with this program. If not, see <http://www.gnu.org/licenses/>.
#
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
describe ProgressRunner do
before do
@progress = mock("progress")
@progress.stub_everything
end
module ProgressMessages
def message=(m)
@message = m
end
def message
@message
end
end
it "should perform normal processing and update progress" do
@progress.expects(:start!).once
@progress.expects(:calculate_completion!).times(3)
@progress.expects(:complete!).once
@progress.expects(:completion=).with(100.0).once
@progress.expects(:message=).with("foo").once
@progress.expects(:save).once
progress_runner = ProgressRunner.new(@progress)
completed_message_value = nil
progress_runner.completed_message {|completed| completed_message_value = completed; "foo"}
error_callback_called = false
progress_runner.error_message {|message, error_ids| error_callback_called = true; "bar"}
process_callback_count = 0
ids = (0..9).to_a
progress_runner.do_batch_update(ids) do |id|
id.should eql process_callback_count
process_callback_count += 1
end
process_callback_count.should eql ids.size
completed_message_value.should eql ids.size
error_callback_called.should be_false
end
it "should rescue exceptions and record messages as errors" do
@progress.extend(ProgressMessages)
@progress.expects(:complete!).once
@progress.expects(:completion=).with(100.0)
@progress.expects(:save).once
progress_runner = ProgressRunner.new(@progress)
progress_runner.completed_message do |count|
count.should eql 1
"abra"
end
error_callback_count = 0
progress_runner.error_message do |error, ids|
error_callback_count += 1
"#{error}: #{ids.join(', ')}"
end
ids = (1..3).to_a
progress_runner.do_batch_update(ids) do |id|
raise "error processing #{id}" if id >= 2
end
error_callback_count.should eql 2
message_lines = @progress.message.lines.map(&:strip).sort
message_lines.size.should eql 3
message_lines.should eql ["abra", "error processing 2: 2", "error processing 3: 3"]
end
it "should have default completion and error messages" do
@progress.extend(ProgressMessages)
progress_runner = ProgressRunner.new(@progress)
ids = (1..4).to_a
progress_runner.do_batch_update(ids) do |id|
raise "processing error" if id >= 3
end
@progress.message.should eql "2 items processed\nprocessing error: 3, 4"
end
# These are also tested above
#it "should accumulate like errors into a single mesage line"
#it "should complete progress if only some records fail"
it "should fail progress if all records fail" do
@progress.extend(ProgressMessages)
@progress.expects(:completion=).with(100.0)
@progress.expects(:fail!).once
@progress.expects(:save).once
progress_runner = ProgressRunner.new(@progress)
ids = (1..4).to_a
progress_runner.do_batch_update(ids) do |id|
raise "processing error"
end
@progress.message.should eql "0 items processed\nprocessing error: 1, 2, 3, 4"
end
it "updates progress frequency relative to size of input" do
ids = (1..255).to_a
times_update = (ids.size / (ids.size / 20).to_f).ceil
@progress.expects(:calculate_completion!).times(times_update)
progress_runner = ProgressRunner.new(@progress)
progress_runner.do_batch_update(ids) {}
end
end