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

Add API for admins to access user messages 

There are three parts to this:
 - API access for site admins and account admins. Account
   admins can only see messages for their account.
 - Includes start and end time parameters for basic searching
 - Summary notification are gathered up per account now, so
   account admins will not see a summary notification from
   multiple accounts.

fixes CNVS-3695

test plan:
 - Verify Admin Access:
   - Have a user enrolled in two courses in separate root
     accounts.
   - Send the user ASAP notification from each account.
   - Go to /users/:id/messages (the old interface) and verify
     the messages appear there.
   - As a site admin, get /api/v1/comm_messages for the user
     (see documentation for the api).
   - Verify that data for both messages are returned.
   - Use the console to give an account admin on one of the
     accounts :read_messages permissions.
   - As the account admin, get /api/v1/comm_messages for the
     user.
   - Verify that only the message for the admin's account is
     returned.
 - Verify start_time and end_time parameters
   - Use the console to modify the created_at field for the
     user's messages
   - As a site admin, get /api/v1/comm_messages for the user,
     specifying various combinations of start_time and
     end_time, and make sure the appropriate messages are
     returned.
 - Verify account based summaries
   - Set the notification policies for the user to a summary
     setting (daily or weekly)
   - Send multiple notifications to the user from each account.
   - Use the console to run the
     SummaryMessageConsolidator::process function.
   - View /messages for the user and verify that separate
     summaries are sent for each account.

Change-Id: Ie33ec02cc2033a1cc2f1fcbe538b76792aab0e6c
Reviewed-on: https://gerrit.instructure.com/18586
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Mark Ericksen <marke@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
This commit is contained in:
Jon Willesen 2013-03-13 09:11:14 -06:00 committed by Mark Ericksen
parent fb7c1d0508
commit 495db783d5
13 changed files with 398 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

100
app/controllers/comm_messages_api_controller.rb Normal file
View File

@ -0,0 +1,100 @@
#
# Copyright (C) 2011-12 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/>.
#
# @API CommMessages
# @beta
#
# API for accessing the messages (emails, sms, facebook, twitter, etc) that have
# been sent to a user.
#
# @object CommMessage
# {
# // The ID of the CommMessage.
# id: 42,
#
# // The date and time this message was created
# created_at: "2013-03-19T21:00:00Z"
#
# // The date and time this message was sent
# sent_at: "2013-03-20T22:42:00Z"
#
# // The workflow state of the message.
# // One of "created", "staged", "sending", "sent", "bounced",
# // "dashboard", "cancelled", or "closed"
# workflow_state: "sent"
#
# // The address that was put in the "from" field of the message
# from: "notifications@example.com"
#
# // The address the message was sent to:
# to: "someone@example.com"
#
# // The reply_to header of the message
# reply_to: "notifications+specialdata@example.com"
#
# // The message subject
# subject: "example subject line"
#
# // The plain text body of the message
# body: "This is the body of the message"
#
# // The HTML body of the message.
# html_body: "<html><body>This is the body of the message</body></html>"
# }
class CommMessagesApiController < ApplicationController
include Api::V1::CommMessage
before_filter :require_user
# @API List of CommMessages for a user
#
# Retrieve messages sent to a user.
#
# @argument user_id The user id for whom you want to retrieve CommMessages
#
# @argument start_time [optional] The beginning of the time range you want to
# retrieve meesage from.
#
# @argument end_time [optional] The end of the time range you want to retrieve
# messages for.
#
# @returns [CommMessage]
def index
user = api_find(User, params[:user_id])
start_time = TimeHelper.try_parse(params[:start_time])
end_time = TimeHelper.try_parse(params[:end_time])
conditions = case
when Account.site_admin.grants_right?(@current_user, :read_messages)
{} # No further restrictions, site admins see all
when @domain_root_account.grants_right?(@current_user, :read_messages)
{ :root_account_id => @domain_root_account.id }
else
return render_unauthorized_action
end
query = user.messages.scoped(:order => 'created_at DESC')
query = query.scoped(:conditions => conditions) unless conditions.empty?
query = query.scoped(:conditions => ['created_at >= ?', start_time]) if start_time
query = query.scoped(:conditions => ['created_at <= ?', end_time]) if end_time
messages = Api.paginate(query, self, api_v1_comm_messages_url)
messages_json = messages.map { |m| comm_message_json(m) }
render :json => messages_json
end
end

7
app/models/communication_channel.rb
View File

@ -260,13 +260,6 @@ class CommunicationChannel < ActiveRecord::Base
def can_notify?
self.notification_policies.any? { |np| np.frequency == 'never' } ? false : true
end
def self.ids_with_pending_delayed_messages
CommunicationChannel.connection.select_values(
"SELECT distinct communication_channel_id
FROM delayed_messages
WHERE workflow_state = 'pending' AND send_at <= '#{Time.now.to_s(:db)}'")
end
def move_to_user(user, migrate=true)
return unless user

17
app/models/delayed_message.rb
View File

@ -21,10 +21,11 @@ class DelayedMessage < ActiveRecord::Base
belongs_to :notification_policy
belongs_to :context, :polymorphic => true
belongs_to :communication_channel
attr_accessible :notification, :notification_policy, :frequency, :communication_channel, :linked_name, :name_of_topic,
:link, :summary, :notification_id, :notification_policy_id, :context_id, :context_type, :communication_channel_id,
:context, :workflow_state
attr_accessible :notification, :notification_policy, :frequency,
:communication_channel, :linked_name, :name_of_topic, :link, :summary,
:notification_id, :notification_policy_id, :context_id, :context_type,
:communication_channel_id, :context, :workflow_state, :root_account_id
validates_length_of :summary, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true
validates_presence_of :communication_channel_id
@ -73,13 +74,6 @@ class DelayedMessage < ActiveRecord::Base
where(:workflow_state => 'pending').order(:send_at).limit(1)
}
def self.ids_for_messages_with_communication_channel_id(cc_id)
dm_ids = DelayedMessage.connection.select_values(
"SELECT id
FROM delayed_messages
WHERE workflow_state = 'pending' AND send_at <= '#{Time.now.to_s(:db)}' AND communication_channel_id = #{cc_id}")
end
include Workflow
workflow do
@ -131,6 +125,7 @@ class DelayedMessage < ActiveRecord::Base
message.delayed_messages = delayed_messages
message.context = context
message.asset_context = context.context(user) rescue context
message.root_account_id = delayed_messages.first.try(:root_account_id)
message.delay_for = 0
message.parse!
message.save

19
app/models/message.rb
View File

@ -37,7 +37,7 @@ class Message < ActiveRecord::Base
attr_accessible :to, :from, :subject, :body, :delay_for, :context, :path_type,
:from_name, :sent_at, :notification, :user, :communication_channel,
:notification_name, :asset_context, :data
:notification_name, :asset_context, :data, :root_account_id
attr_writer :delayed_messages
@ -478,10 +478,10 @@ class Message < ActiveRecord::Base
current_context = context
until current_context.respond_to?(:root_account) do
return nil if unbounded_loop_paranoia_counter <= 0 || current_context.nil?
return nil unless current_context.respond_to?(:context)
current_context = current_context.context
unbounded_loop_paranoia_counter -= 1
return nil if unbounded_loop_paranoia_counter <= 0 || context.nil?
end
current_context.root_account
@ -502,7 +502,10 @@ class Message < ActiveRecord::Base
self.to_email = true if %w[email sms].include?(path_type)
self.from_name = context_root_account.settings[:outgoing_email_default_name] rescue nil
root_account = context_root_account
self.root_account_id ||= root_account.try(:id)
self.from_name = root_account.settings[:outgoing_email_default_name] rescue nil
self.from_name = HostUrl.outgoing_email_default_name if from_name.blank?
self.from_name = asset_context.name if (asset_context &&
!asset_context.is_a?(Account) && asset_context.name &&
@ -566,6 +569,14 @@ class Message < ActiveRecord::Base
nil
end
# Public: Return the message as JSON filtered to selected fields and
# flattened appropriately.
#
# Returns json hash.
def as_json(options = {})
super(:only => [:id, :created_at, :sent_at, :workflow_state, :from, :to, :reply_to, :subject, :body, :html_body])['message']
end
protected
# Internal: Deliver the message through email.
#

4
config/routes.rb
View File

@ -933,6 +933,10 @@ ActionController::Routing::Routes.draw do |map|
channels.delete 'users/:user_id/communication_channels/:id', :action => :destroy
end
api.with_options(:controller => :comm_messages_api) do |comm_messages|
comm_messages.get 'comm_messages', :action => :index, :path_name => 'comm_messages'
end
api.with_options(:controller => :services_api) do |services|
services.get 'services/kaltura', :action => :show_kaltura_config
services.post 'services/kaltura_session', :action => :start_kaltura_session

22
db/migrate/20130312231026_add_message_account_id.rb Normal file
View File

@ -0,0 +1,22 @@
class AddMessageAccountId < ActiveRecord::Migration
tag :predeploy
self.transactional = false
def self.up
add_column :messages, :root_account_id, :integer, :limit => 8
add_column :delayed_messages, :root_account_id, :integer, :limit => 8
add_index :messages, :root_account_id, :concurrently => true
add_index :delayed_messages, [:communication_channel_id, :root_account_id, :workflow_state, :send_at], :concurrently => true, :name => "ccid_raid_ws_sa"
remove_index :delayed_messages, :name => "ccid_ws_sa"
end
def self.down
add_index :delayed_messages, [:communication_channel_id, :workflow_state, :send_at], :concurrently => true, :name => "ccid_ws_sa"
remove_index :delayed_messages, :name => "ccid_raid_ws_sa"
remove_index :messages, :column => :root_account_id
remove_column :messages, :root_account_id
remove_column :delayed_messages, :root_account_id
end
end

27
lib/api/v1/comm_message.rb Normal file
View File

@ -0,0 +1,27 @@
#
# 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/>.
#
module Api::V1::CommMessage
include Api::V1::Json
def comm_message_json(message, options = {})
result = message.as_json(options)
result
end
end

1
lib/notification_message_creator.rb
View File

@ -129,6 +129,7 @@ class NotificationMessageCreator
:notification_policy => policy,
:frequency => policy.frequency,
:communication_channel_id => policy.communication_channel_id,
:root_account_id => message.context_root_account.try(:id),
:linked_name => 'work on this link!!!',
:name_of_topic => message.subject,
:link => message.url,

30
lib/summary_message_consolidator.rb
View File

@ -26,14 +26,11 @@ class SummaryMessageConsolidator
end
def process
cc_ids = ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
CommunicationChannel.ids_with_pending_delayed_messages
end
dm_id_batches = []
cc_ids.each do |cc_id|
dm_ids = DelayedMessage.ids_for_messages_with_communication_channel_id(cc_id)
dm_id_batches << dm_ids
@logger.info("Scheduled summary with #{dm_ids.length} messages for communication channel id #{cc_id}")
batch_ids = delayed_message_batch_ids
dm_id_batches = batch_ids.map do |batch_id|
dm_ids = delayed_message_ids_for_batch(batch_id)
@logger.info("Scheduled summary with #{dm_ids.length} messages for communication channel id #{batch_id['communication_channel_id']} and root account id #{batch_id['root_account_id'] || 'null'}")
dm_ids
end
dm_id_batches.in_groups_of(Setting.get('summary_message_consolidator_batch_size', '500').to_i, false) do |batches|
@ -48,4 +45,21 @@ class SummaryMessageConsolidator
end
dm_id_batches.size
end
def delayed_message_batch_ids
ActiveRecord::Base::ConnectionSpecification.with_environment(:slave) do
DelayedMessage.connection.select_all(
DelayedMessage.scoped.select('communication_channel_id').select('root_account_id').uniq.
where("workflow_state = ? AND send_at <= ?", 'pending', Time.now.to_s(:db)).
to_sql)
end
end
def delayed_message_ids_for_batch(batch)
DelayedMessage.
where("workflow_state = ? AND send_at <= ?", 'pending', Time.now.to_s(:db)).
where(batch). # hash condition will properly handle the case where root_account_id is null
all.map(&:id)
end
end

160
spec/apis/v1/comm_messages_api_spec.rb Normal file
View File

@ -0,0 +1,160 @@
#
# 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__) + '/../api_spec_helper')
describe CommMessagesApiController, :type => :integration do
describe "index" do
context "a site admin" do
context "with permission" do
before do
@test_user = user(:active_all => true)
site_admin_user
user_session(@admin)
end
it "should be able to see all messages" do
Message.create!(:user => @test_user, :body => "site admin message", :root_account_id => Account.site_admin.id)
Message.create!(:user => @test_user, :body => "account message", :root_account_id => Account.default.id)
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param })
json.size.should eql 2
json.map {|m| m['body'] }.sort.should eql ['account message', 'site admin message']
end
it "should require a valid user_id parameter" do
raw_api_call(:get, "/api/v1/comm_messages", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json'})
response.code.should eql '404'
raw_api_call(:get, "/api/v1/comm_messages?user_id=0", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => '0' })
response.code.should eql '404'
end
it "should use start_time and end_time parameters to limit results" do
m = Message.new(:user => @test_user, :body => "site admin message", :root_account_id => Account.site_admin.id)
m.write_attribute(:created_at, Time.zone.now - 1.day)
m.save!
Message.create!(:user => @test_user, :body => "account message", :root_account_id => Account.default.id)
m = Message.new(:user => @test_user, :body => "account message", :root_account_id => Account.default.id)
m.write_attribute(:created_at, Time.zone.now + 1.day)
m.save!
start_time = (Time.zone.now - 1.hour).iso8601
end_time = (Time.zone.now + 1.hour).iso8601
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}&start_time=#{start_time}&end_time=#{end_time}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param, :start_time => start_time, :end_time => end_time })
end
it "should paginate results" do
5.times do |v|
Message.create!(:user => @test_user, :body => "body #{v}", :root_account_id => Account.default.id)
end
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}&per_page=2", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param, :per_page => '2' })
json.size.should eql 2
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}&per_page=2&page=2", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param, :per_page => '2', :page => '2' })
json.size.should eql 2
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}&per_page=2&page=3", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param, :per_page => '2', :page => '3' })
json.size.should eql 1
end
end
context "without permission" do
before do
@test_user = user(:active_all => true)
account_admin_user_with_role_changes(:account => Account.site_admin, :role_changes => {:read_messages => false})
user_session(@admin)
end
it "should receive unauthorized" do
raw_api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param })
response.code.should eql '401'
end
end
end
context "an account admin" do
context "with permission" do
before do
@test_user = user(:active_all => true)
account_admin_user_with_role_changes(:account => Account.default, :role_changes => {:read_messages => true})
user_session(@admin)
end
it "should only be able to see associated account's messages" do
Message.create!(:user => @test_user, :body => "site admin message", :root_account_id => Account.site_admin.id)
Message.create!(:user => @test_user, :body => "account message", :root_account_id => Account.default.id)
json = api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param })
json.size.should eql 1
json.map {|m| m['body'] }.sort.should eql ['account message']
end
end
context "without permission" do
before do
@test_user = user(:active_all => true)
account_admin_user_with_role_changes(:account => Account.default, :role_changes => {:read_messages => false})
user_session(@admin)
end
it "should receive unauthorized" do
raw_api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param })
response.code.should eql '401'
end
end
end
context "an unauthorized user" do
before do
@test_user = user(:active_all => true)
@user = user(:active_all => true)
user_session(@user)
end
it "should receive unauthorized" do
raw_api_call(:get, "/api/v1/comm_messages?user_id=#{@test_user.id}", {
:controller => 'comm_messages_api', :action => 'index', :format => 'json',
:user_id => @test_user.to_param })
response.code.should eql '401'
end
end
end
end

3
spec/lib/notification_message_creator_spec.rb
View File

@ -171,7 +171,7 @@ describe NotificationMessageCreator do
cc = u1.communication_channels.create(:path => "active@example.com")
cc.confirm!
@a = Assignment.create
@a = assignment_model
messages = NotificationMessageCreator.new(@notification, @a, :to_list => u1).create_message
messages.should_not be_empty
messages.length.should eql(1)
@ -180,6 +180,7 @@ describe NotificationMessageCreator do
DelayedMessage.last.should_not be_nil
DelayedMessage.last.notification_id.should eql(@notification.id)
DelayedMessage.last.communication_channel_id.should eql(cc.id)
DelayedMessage.last.root_account_id.should eql Account.default.id
DelayedMessage.last.send_at.should > Time.now.utc
end

23
spec/lib/summary_message_consolidator_spec.rb
View File

@ -32,4 +32,27 @@ describe "SummaryMessageConsolidator" do
queued = created_jobs.map { |j| j.payload_object.jobs.map { |j| j.payload_object.args } }.flatten
queued.map(&:to_i).sort.should == messages.map(&:id).sort
end
it "should send summaries from different accounts in separate messages" do
users = (0..3).map { user_with_communication_channel }
dms = []
account_ids = [1, 2, 3]
delayed_messages_per_account = 2
account_id_iter = (account_ids * delayed_messages_per_account).sort
users.each do |u|
account_id_iter.each do |rai|
dms << delayed_message_model(
:cc => u.communication_channels.first,
:root_account_id => rai,
:send_at => 1.day.ago)
end
end
SummaryMessageConsolidator.process
dm_summarize_expectation = DelayedMessage.expects(:summarize)
dms.each_slice(delayed_messages_per_account) do |dms|
dm_summarize_expectation.with(dms.map(&:id))
end
run_jobs
end
end

16
spec/models/message_spec.rb
View File

@ -138,5 +138,21 @@ describe Message do
@message.deliver.should == false
end
end
describe "infer_defaults" do
it "should not break if there is no context" do
message_model.root_account_id.should be_nil
end
it "should not break if the context does not have an account" do
user_model
message_model(:context => @user).root_account_id.should be_nil
end
it "should populate root_account_id if the context can chain back to a root account" do
message_model(:context => course_model).root_account_id.should eql Account.default.id
end
end
end
end