Skip to content

Commit cd4f95e

Browse files
committed
survey notification support
Special survey account notifications (announcements) can be set up on the site_admin account. These survey notifications will only appear for accounts that have the "Account Surveys" setting enabled in their account settings, and they'll only show up for 1/N users in those accounts each month. N is configurable, defaults to 9. closes CNVS-6036 test plan: * On a regular account, create an announcement in account settings. There shouldn't be any options related to surveys available. * On the site admin account, create an announcement. Select to make it a survey. You can leave N at 9 or change it. * Verify that the survey doesn't show up for any users on accounts that don't have "Account Surveys" enabled. * Enable the "Account Surveys" setting on an account. Verify that the survey shows up for (roughly) 1 out of every N users in the account, on their dashboard. Change the time on the computer running canvas to another month. Verify that the survey shows up for a different set of 1/N users in the account. Change-Id: If11467d2153acee24a010ba45d516b0b320a4634 Reviewed-on: https://gerrit.instructure.com/21432 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Cody Cutrer <cody@instructure.com> QA-Review: Jeremy Putnam <jeremyp@instructure.com> Product-Review: Brian Palmer <brianp@instructure.com>
1 parent 851c5ef commit cd4f95e

11 files changed

Lines changed: 181 additions & 27 deletions

File tree

app/controllers/accounts_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ def update
258258
:default_group_storage_quota, :default_group_storage_quota_mb].each { |key| params[:account].delete key }
259259
end
260260
if params[:account][:services]
261-
params[:account][:services].slice(*Account.services_exposed_to_ui_hash(nil, @current_user).keys).each do |key, value|
261+
params[:account][:services].slice(*Account.services_exposed_to_ui_hash(nil, @current_user, @account).keys).each do |key, value|
262262
@account.set_service_availability(key, value == '1')
263263
end
264264
params[:account].delete :services

app/models/account.rb

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,7 +1123,14 @@ def self.allowable_services
11231123
:description => "",
11241124
:default => false,
11251125
:expose_to_ui => :setting
1126-
}
1126+
},
1127+
:account_survey_notifications => {
1128+
:name => "Account Surveys",
1129+
:description => "",
1130+
:default => false,
1131+
:expose_to_ui => :setting,
1132+
:expose_to_ui_proc => proc { |user, account| user && account && account.grants_right?(user, :manage_site_settings) },
1133+
},
11271134
}.merge(@plugin_services || {}).freeze
11281135
end
11291136

@@ -1198,12 +1205,12 @@ def allowed_services_hash
11981205

11991206
# if expose_as is nil, all services exposed in the ui are returned
12001207
# if it's :service or :setting, then only services set to be exposed as that type are returned
1201-
def self.services_exposed_to_ui_hash(expose_as = nil, current_user = nil)
1208+
def self.services_exposed_to_ui_hash(expose_as = nil, current_user = nil, account = nil)
12021209
if expose_as
12031210
self.allowable_services.reject { |key, setting| setting[:expose_to_ui] != expose_as }
12041211
else
12051212
self.allowable_services.reject { |key, setting| !setting[:expose_to_ui] }
1206-
end.reject { |key, setting| setting[:expose_to_ui_proc] && !setting[:expose_to_ui_proc].call(current_user) }
1213+
end.reject { |key, setting| setting[:expose_to_ui_proc] && !setting[:expose_to_ui_proc].call(current_user, account) }
12071214
end
12081215

12091216
def service_enabled?(service)

app/models/account_notification.rb

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,39 +1,82 @@
11
class AccountNotification < ActiveRecord::Base
22
attr_accessible :subject, :icon, :message,
3-
:account, :user, :start_at, :end_at
3+
:account, :user, :start_at, :end_at,
4+
:required_account_service, :months_in_display_cycle
45

56
validates_presence_of :start_at, :end_at, :account_id
67
before_validation :infer_defaults
78
belongs_to :account, :touch => true
89
belongs_to :user
910
validates_length_of :message, :maximum => maximum_text_length, :allow_nil => false, :allow_blank => false
1011
sanitize_field :message, Instructure::SanitizeField::SANITIZE
11-
12+
13+
ACCOUNT_SERVICE_NOTIFICATION_FLAGS = %w[account_survey_notifications]
14+
validates_inclusion_of :required_account_service, in: ACCOUNT_SERVICE_NOTIFICATION_FLAGS, allow_nil: true
15+
16+
validates_inclusion_of :months_in_display_cycle, in: 1..48, allow_nil: true
17+
1218
def infer_defaults
1319
self.start_at ||= Time.now.utc
1420
self.end_at ||= self.start_at + 2.weeks
1521
self.end_at = [self.end_at, self.start_at].max
1622
end
1723

1824
def self.for_user_and_account(user, account)
19-
closed_ids = user.preferences[:closed_notifications] || []
20-
now = Time.now.utc
21-
# Refreshes every 10 minutes at the longest
22-
current = Rails.cache.fetch(['account_notifications2', account].cache_key, :expires_in => 10.minutes) do
23-
Shard.partition_by_shard([Account.site_admin, account]) do |accounts|
24-
AccountNotification.where("account_id IN (?) AND start_at <? AND end_at>?", accounts, now, now).order('start_at DESC').all
25-
end
26-
end
25+
current = self.for_account(account)
2726

2827
user.shard.activate do
28+
closed_ids = user.preferences[:closed_notifications] || []
2929
# If there are ids marked as 'closed' that are no longer
3030
# applicable, they probably need to be cleared out.
3131
current_ids = current.map(&:id)
3232
if !(closed_ids - current_ids).empty?
3333
closed_ids = user.preferences[:closed_notifications] &= current_ids
3434
user.save!
3535
end
36-
current.reject { |announcement| closed_ids.include?(announcement.id) }
36+
current.reject! { |announcement| closed_ids.include?(announcement.id) }
37+
38+
# filter out announcements that have a periodic cycle of display,
39+
# and the user isn't in the set of users to display it to this month (based
40+
# on user id)
41+
current.reject! do |announcement|
42+
if months_in_period = announcement.months_in_display_cycle
43+
!self.display_for_user?(user.id, months_in_period)
44+
end
45+
end
3746
end
47+
48+
current
49+
end
50+
51+
def self.for_account(account)
52+
# Refreshes every 10 minutes at the longest
53+
Rails.cache.fetch(['account_notifications2', account].cache_key, :expires_in => 10.minutes) do
54+
now = Time.now.utc
55+
# we always check the given account for the flag, even if the announcement is from the site_admin account
56+
# this allows us to make a global announcement that is filtered to only accounts with this flag
57+
enabled_flags = ACCOUNT_SERVICE_NOTIFICATION_FLAGS & account.allowed_services_hash.keys.map(&:to_s)
58+
59+
Shard.partition_by_shard([Account.site_admin, account]) do |accounts|
60+
AccountNotification.where("account_id IN (?) AND start_at <? AND end_at>?", accounts, now, now).
61+
where("required_account_service IS NULL OR required_account_service IN (?)", enabled_flags).
62+
order('start_at DESC').all
63+
end
64+
end
65+
end
66+
67+
def self.default_months_in_display_cycle
68+
Setting.get_cached("account_notification_default_months_in_display_cycle", "9").to_i
69+
end
70+
71+
# private
72+
def self.display_for_user?(user_id, months_in_period, current_time = Time.now.utc)
73+
# we just need a stable reference point, doesn't matter what it is, so
74+
# let's use unix epoch
75+
start_time = Time.at(0).utc
76+
months_since_start_time = (current_time.year - start_time.year) * 12 + (current_time.month - start_time.month)
77+
periods_since_start_time = months_since_start_time / months_in_period
78+
months_into_current_period = months_since_start_time % months_in_period
79+
mod_value = (Random.new(periods_since_start_time).rand(months_in_period) + months_into_current_period) % months_in_period
80+
user_id % months_in_period == mod_value
3881
end
3982
end

app/stylesheets/account_settings.sass

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,10 @@
9393
legend
9494
font-size: 15px
9595
font-weight: bold
96+
97+
#survey_announcement_field
98+
line-height: 30px
99+
input#account_notification_months_in_display_cycle
100+
width: 25px
101+
padding: 0
102+
margin-top: 6px

app/views/accounts/settings.html.erb

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ TEXT
282282
<% end %>
283283
<% end %>
284284
<% f.fields_for :services do |services| %>
285-
<% Account.services_exposed_to_ui_hash(:setting, @current_user).sort_by { |k,h| h[:name] }.each do |key, service| %>
285+
<% Account.services_exposed_to_ui_hash(:setting, @current_user, @account).sort_by { |k,h| h[:name] }.each do |key, service| %>
286286
<div>
287287
<%= services.check_box key, :checked => @account.service_enabled?(key) %>
288288
<%= services.label key, service[:name] + " " %>
@@ -652,6 +652,16 @@ TEXT
652652
</span>&nbsp;&nbsp;
653653
<%= link_to(context_user_name(@account, announcement.user_id), user_path(announcement.user_id)) %>
654654
</div>
655+
<% if announcement.required_account_service %>
656+
<div>
657+
<%= Account.allowable_services[announcement.required_account_service.to_sym].try(:[], :name) %>
658+
</div>
659+
<% end %>
660+
<% if announcement.months_in_display_cycle %>
661+
<div>
662+
<%= t :announcement_sent_to_subset, "Sent to 1 / %{denominator} users each month", denominator: announcement.months_in_display_cycle %>
663+
</div>
664+
<% end %>
655665
<div class="message user_content">
656666
<%= user_content(announcement.message) %>
657667
</div>
@@ -684,8 +694,16 @@ TEXT
684694
</td>
685695
</tr>
686696
<% if @account.site_admin? %>
697+
<tr id="confirm_global_announcement_field">
698+
<td colspan=2><input type="checkbox" id="confirm_global_announcement" name="confirm_global_announcement"> <%= label_tag :confirm_global_announcement, t(:confirm_global_announcement, "This announcement will be shown to *all* Canvas users. Confirm that you want to create it.", :wrapper => '<b>\1</b>') %></td>
699+
</tr>
687700
<tr>
688-
<td colspan=2><input type=checkbox id=confirm_global_announcement name=confirm_global_announcement> <%= label_tag :confirm_global_announcement, t(:confirm_global_announcement, "This announcement will be shown to *all* Canvas users. Confirm that you want to create it.", :wrapper => '<b>\1</b>') %></td>
701+
<td colspan=2>
702+
<div id="survey_announcement_field">
703+
<%# don't use the rails check_box helper here, because we don't want the extra hidden element when not checked %>
704+
<input type="checkbox" name="account_notification[required_account_service]" id="account_notification_required_account_service" value="account_survey_notifications" />
705+
<label><%= t :survey_announcement, "This is a survey announcement. It will be sent to 1 / %{denominator} of users in enabled accounts for each month that it's active.", denominator: f.text_field(:months_in_display_cycle, value: AccountNotification.default_months_in_display_cycle, disabled: true) %></label>
706+
</label>
689707
</tr>
690708
<% end %>
691709
<tr>
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
class AddSurveyFunctionalityToAccountNotifications < ActiveRecord::Migration
2+
tag :predeploy
3+
4+
def self.up
5+
add_column :account_notifications, :required_account_service, :string
6+
add_column :account_notifications, :months_in_display_cycle, :int
7+
# this table is small enough for transactional index creation
8+
add_index :account_notifications, [:account_id, :end_at, :start_at], name: "index_account_notifications_by_account_and_timespan"
9+
remove_index :account_notifications, [:account_id, :start_at]
10+
end
11+
12+
def self.down
13+
remove_column :account_notifications, :required_account_setting
14+
remove_column :account_notifications, :months_in_display_cycle
15+
add_index :account_notifications, [:account_id, :start_at]
16+
remove_index :account_notifications, name: "index_account_notifications_by_account_and_timespan"
17+
end
18+
end

public/javascripts/account_settings.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,30 @@ define([
3737
});
3838
$("#add_notification_form").submit(function(event) {
3939
var $this = $(this);
40-
var $confirmation = $this.find('#confirm_global_announcement:not(:checked)');
40+
var $confirmation = $this.find('#confirm_global_announcement:visible:not(:checked)');
4141
if ($confirmation.length > 0) {
4242
$confirmation.errorBox(I18n.t('confirms.global_announcement', "You must confirm the global announcement"));
4343
return false;
4444
}
45-
var result = $this.validateForm({
45+
var validations = {
4646
object_name: 'account_notification',
4747
required: ['start_at', 'end_at', 'subject', 'message'],
48-
date_fields: ['start_at', 'end_at']
49-
});
48+
date_fields: ['start_at', 'end_at'],
49+
numbers: []
50+
};
51+
if ($('#account_notification_months_in_display_cycle').length > 0) {
52+
validations.numbers.push('months_in_display_cycle');
53+
}
54+
var result = $this.validateForm(validations);
5055
if(!result) {
5156
return false;
5257
}
5358
});
59+
$("#account_notification_required_account_service").click(function(event) {
60+
$this = $(this);
61+
$("#confirm_global_announcement_field").showIf(!$this.is(":checked"));
62+
$("#account_notification_months_in_display_cycle").prop("disabled", !$this.is(":checked"));
63+
});
5464
$(".delete_notification_link").click(function(event) {
5565
event.preventDefault();
5666
var $link = $(this);

spec/controllers/accounts_controller_spec.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -293,19 +293,22 @@ def cross_listed_course
293293

294294
it "should allow updating services that appear in the ui for the current user" do
295295
Account.register_service(:test1, { name: 'test1', description: '', expose_to_ui: :setting, default: false })
296-
Account.register_service(:test2, { name: 'test2', description: '', expose_to_ui: :setting, default: false, expose_to_ui_proc: proc { |user| false } })
296+
Account.register_service(:test2, { name: 'test2', description: '', expose_to_ui: :setting, default: false, expose_to_ui_proc: proc { |user, account| false } })
297297
user_session(user)
298298
@account = Account.create!
299+
Account.register_service(:test3, { name: 'test3', description: '', expose_to_ui: :setting, default: false, expose_to_ui_proc: proc { |user, account| account == @account } })
299300
Account.site_admin.add_user(@user)
300301
post 'update', id: @account.id, account: {
301302
services: {
302303
'test1' => '1',
303304
'test2' => '1',
305+
'test3' => '1',
304306
}
305307
}
306308
@account.reload
307309
@account.allowed_services.should match(%r{\+test1})
308310
@account.allowed_services.should_not match(%r{\+test2})
311+
@account.allowed_services.should match(%r{\+test3})
309312
end
310313

311314
describe "quotas" do

spec/models/account_notification_spec.rb

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,41 @@
4949
@user.preferences[:closed_notifications].should == []
5050
end
5151

52+
describe "survey notifications" do
53+
it "should only display for flagged accounts" do
54+
flag = AccountNotification::ACCOUNT_SERVICE_NOTIFICATION_FLAGS.first
55+
@announcement = Account.site_admin.announcements.create!(message: "hello", required_account_service: flag)
56+
@a1 = account_model
57+
@a2 = account_model
58+
@a2.enable_service(flag)
59+
@a2.save!
60+
AccountNotification.for_account(@a1).should == []
61+
AccountNotification.for_account(@a2).should == [@announcement]
62+
end
63+
64+
describe "display_for_user?" do
65+
it "should select each mod value once throughout the cycle" do
66+
AccountNotification.display_for_user?(5, 3, Time.zone.parse('2012-04-02')).should == false
67+
AccountNotification.display_for_user?(6, 3, Time.zone.parse('2012-04-02')).should == false
68+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-04-02')).should == true
69+
70+
AccountNotification.display_for_user?(5, 3, Time.zone.parse('2012-05-05')).should == true
71+
AccountNotification.display_for_user?(6, 3, Time.zone.parse('2012-05-05')).should == false
72+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-05-05')).should == false
73+
74+
AccountNotification.display_for_user?(5, 3, Time.zone.parse('2012-06-04')).should == false
75+
AccountNotification.display_for_user?(6, 3, Time.zone.parse('2012-06-04')).should == true
76+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-06-04')).should == false
77+
end
78+
79+
it "should shift the mod values each new cycle" do
80+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-04-02')).should == true
81+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-07-02')).should == false
82+
AccountNotification.display_for_user?(7, 3, Time.zone.parse('2012-09-02')).should == true
83+
end
84+
end
85+
end
86+
5287
context "sharding" do
5388
specs_require_sharding
5489

spec/models/account_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -266,19 +266,19 @@
266266
Account.services_exposed_to_ui_hash(:setting).keys.should == Account.allowable_services.reject { |h,k| k[:expose_to_ui] != :setting || (k[:expose_to_ui_proc] && !k[:expose_to_ui_proc].call(nil)) }.keys
267267
end
268268

269-
it "should filter based on user if a proc is specified" do
269+
it "should filter based on user and account if a proc is specified" do
270270
user1 = User.create!
271271
user2 = User.create!
272272
Account.register_service(:myservice, {
273273
name: "My Test Service",
274274
description: "Nope",
275275
expose_to_ui: :setting,
276276
default: false,
277-
expose_to_ui_proc: proc { |user| user == user2 },
277+
expose_to_ui_proc: proc { |user, account| user == user2 && account == Account.default },
278278
})
279279
Account.services_exposed_to_ui_hash(:setting).keys.should_not be_include(:myservice)
280-
Account.services_exposed_to_ui_hash(:setting, user1).keys.should_not be_include(:myservice)
281-
Account.services_exposed_to_ui_hash(:setting, user2).keys.should be_include(:myservice)
280+
Account.services_exposed_to_ui_hash(:setting, user1, Account.default).keys.should_not be_include(:myservice)
281+
Account.services_exposed_to_ui_hash(:setting, user2, Account.default).keys.should be_include(:myservice)
282282
end
283283
end
284284

0 commit comments

Comments
 (0)