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

sub-account branding; closes #9368 

allow sub accounts to include their own global scripts and stylesheets. if
global includes are enabled on the root account, root account administrators
will have an option to enable them for immediate child accounts.  those child
accounts can then choose to enable them for their sub-accounts, and so on down
the chain.

these includes are added to the page in order from highest to lowest account,
so sub-accounts are able to override styles added by their parents.

the logic for which styles to display on which pages is as follows:
- on account pages, include all styles in the chain from this account up to the
  root account.  this ensures that you can always see styles for account
  X without any sub-account overrides on account X's page
- on course/group pages, include all styles in the chain from the account which
  contains that course/group up to the root
- on the dashboard, calendar, user pages, and other pages that don't fall into
  one of the above categories, we find the lowest account that contains all of
  the current user's active classes + groups, and include styles from that
  account up to the root

test plan:
- in a root account, create two sub-accounts, create courses in each of them,
  and create 3 users, one enrolled only in the first course, one only in the
  second course, and one enrolled in both courses.
- enable global includes on the root account (no sub-accounts yet) add files,
  and make sure all three students see them.
- now enable sub-account includes, and add include files to each sub-account
- make sure both users in course 1 see include for sub-account 1
- make sure user 1 sees include for sub-account 1 on her dashboard, but user
  3 does not.

Change-Id: I3d07d4bced39593f3084d5eac6ea3137666e319b
Reviewed-on: https://gerrit.instructure.com/12248
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Simon Williams <simon@instructure.com>
This commit is contained in:
Simon Williams 2012-07-09 15:30:16 -06:00
parent 14917cece8
commit 323283c237
11 changed files with 332 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
app/helpers/application_helper.rb
View File

@ -648,9 +648,33 @@ module ApplicationHelper
end
end
def account_context(context)
if context.is_a?(Account)
context
elsif context.is_a?(Course) || context.is_a?(CourseSection)
account_context(context.account)
elsif context.is_a?(Group)
account_context(context.context)
end
end
def get_include_accounts
return @include_accounts if @include_accounts.present?
@include_accounts = [Account.site_admin, @domain_root_account]
if @domain_root_account.try(:sub_account_includes?)
# get the deepest account to start looking for branding
common_chain = account_context(@context).try(:account_chain)
common_chain ||= @current_user.common_account_chain(@domain_root_account) if @current_user.present?
@include_accounts.concat(common_chain) if common_chain.present?
end
@include_accounts.uniq!
@include_accounts.compact!
@include_accounts
end
def include_account_js
includes = [Account.site_admin, @domain_root_account].uniq.inject([]) do |js_includes, account|
if account && account.settings[:global_includes] && account.settings[:global_javascript].present?
includes = get_include_accounts.inject([]) do |js_includes, account|
if account && account.allow_global_includes? && account.settings[:global_javascript].present?
js_includes << "'#{account.settings[:global_javascript]}'"
end
js_includes
@ -676,6 +700,16 @@ module ApplicationHelper
end
end
def include_account_css
includes = get_include_accounts.inject([]) do |css_includes, account|
if account && account.allow_global_includes? && account.settings[:global_stylesheet].present?
css_includes << account.settings[:global_stylesheet]
end
css_includes
end
includes << { :media => 'all' }
stylesheet_link_tag *includes
end
# this should be the same as friendlyDatetime in handlebars_helpers.coffee
def friendly_datetime(datetime, opts={})

9
app/models/account.rb
View File

@ -130,8 +130,9 @@ class Account < ActiveRecord::Base
# these settings either are or could be easily added to
# the account settings page
add_setting :global_includes, :root_only => true, :boolean => true, :default => false
add_setting :global_javascript, :condition => :global_includes, :root_only => true
add_setting :global_stylesheet, :condition => :global_includes, :root_only => true
add_setting :global_javascript, :condition => :allow_global_includes
add_setting :global_stylesheet, :condition => :allow_global_includes
add_setting :sub_account_includes, :condition => :allow_global_includes, :boolean => true, :default => false
add_setting :error_reporting, :hash => true, :values => [:action, :email, :url, :subject_param, :body_param], :root_only => true
add_setting :custom_help_links, :root_only => true
add_setting :prevent_course_renaming_by_teachers, :boolean => true, :root_only => true
@ -178,6 +179,10 @@ class Account < ActiveRecord::Base
settings
end
def allow_global_includes?
self.global_includes? || self.parent_account.try(:sub_account_includes?)
end
def ip_filters=(params)
filters = {}
require 'ipaddr'

35
app/models/user.rb
View File

@ -1601,6 +1601,41 @@ class User < ActiveRecord::Base
end
memoize :account
# this finds the reverse account chain starting at in_root_account and ending
# at the lowest account such that all of the accounts to which the user is
# associated which descend from in_root_account, descend from one of the
# accounts in the chain. In other words, if the users associated accounts
# made a tree, it would be the chain between the root and the first branching
# point.
#
# NOTE: this is currently used for computing sub-account branding, and as
# such we can afford rudimentary caching. More precise caching may be
# necessary for future uses.
def common_account_chain(in_root_account)
Rails.cache.fetch([self.id, 'common_account_chain', in_root_account.id].cache_key, :expires_in => 15.minutes) do
rid = in_root_account.id
accts = self.associated_accounts.scoped(:conditions => ["accounts.id = ? OR accounts.root_account_id = ?", rid, rid])
return nil if accts.blank?
children = accts.inject({}) do |hash,acct|
pid = acct.parent_account_id
if pid.present?
hash[pid] ||= []
hash[pid] << acct
end
hash
end
longest_chain = [in_root_account]
while true
next_children = children[longest_chain.last.id]
break unless next_children.present? && next_children.count == 1
longest_chain << next_children.first
end
longest_chain
end
end
def courses_with_primary_enrollment(association = :current_and_invited_courses, enrollment_uuid = nil, options = {})
res = Rails.cache.fetch([self, 'courses_with_primary_enrollment', association, options].cache_key, :expires_in => 15.minutes) do
courses = send(association).distinct_on(["courses.id"],

6
app/views/accounts/settings.html.erb
View File

@ -231,7 +231,7 @@
<% end %>
</fieldset>
<fieldset id="account_settings_global_includes_settings" style="<%= hidden unless @account.settings[:global_includes] %>">
<fieldset id="account_settings_global_includes_settings" style="<%= hidden unless @account.allow_global_includes? %>">
<legend><%= t(:global_js_and_css_includes_title, "Global JavaScript and CSS Includes") %></legend>
<p style="font-size: 0.9em;"><%= t(:global_js_and_css_includes_description, "These files will be included on all page loads for your account") %></p>
<% f.fields_for :settings do |settings| %>
@ -245,6 +245,10 @@
<td><%= settings.text_field :global_stylesheet, :value => @account.settings[:global_stylesheet] %></td>
</tr>
</table>
<div>
<%= settings.check_box :sub_account_includes, :checked => @account.settings[:sub_account_includes] %>
<%= settings.label :sub_account_includes, :en => "Let sub-accounts define additional includes" %>
</div>
<% end %>
</fieldset>

3
app/views/layouts/_head.html.erb
View File

@ -20,8 +20,7 @@
<%= include_css_bundles %>
<!--[if lte IE 8]> <%= javascript_include_tag "vendor/html5.js" %> <![endif]-->
<%= yield :stylesheets %>
<%= stylesheet_link_tag(Account.site_admin.settings[:global_stylesheet], :media => 'all') if @domain_root_account != Account.site_admin && Account.site_admin.settings[:global_stylesheet].present? %>
<%= stylesheet_link_tag(@domain_root_account.settings[:global_stylesheet], :media => 'all') if @domain_root_account && @domain_root_account.settings[:global_stylesheet].present? %>
<%= include_account_css %>
<script>
// listen for any clicks on links that have href="#" and queue them to be fired on dom ready.
_earlyClick = function(e){

3
spec/controllers/application_controller_spec.rb
View File

@ -138,7 +138,4 @@ describe ApplicationController do
@controller.instance_variable_get(:@context).should == @section
end
end
end

171
spec/helpers/application_helper_spec.rb
View File

@ -99,12 +99,144 @@ describe ApplicationHelper do
end
end
describe "include account js" do
context "include_account_css" do
before do
@domain_root_account = Account.default
@site_admin = Account.site_admin
Account.stubs(:site_admin).returns(@site_admin)
@settings = { :global_includes => true, :global_javascript => '/path/to/js' }
@domain_root_account = Account.default
end
context "with no custom css" do
it "should be empty" do
include_account_css.should be_empty
end
end
context "with custom css" do
it "should include account css" do
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_stylesheet => '/path/to/css' })
@domain_root_account.save!
output = include_account_css
output.should have_tag 'link'
output.should match %r{/path/to/css}
end
it "should include site admin css" do
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_stylesheet => '/path/to/css' })
@site_admin.save!
output = include_account_css
output.should have_tag 'link'
output.should match %r{/path/to/css}
end
it "should include site admin css once" do
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_stylesheet => '/path/to/css' })
@site_admin.save!
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/css}).length.should eql 1
end
it "should include site admin css first" do
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_stylesheet => '/path/to/admin/css' })
@site_admin.save!
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_stylesheet => '/path/to/root/css' })
@domain_root_account.save!
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(root/|admin/)?css}).should eql [['admin/'], ['root/']]
end
end
context "sub-accounts" do
before do
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_stylesheet => '/path/to/admin/css' })
@site_admin.save!
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :sub_account_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_stylesheet => '/path/to/root/css' })
@domain_root_account.save!
@sub_account1 = account_model(:root_account => @domain_root_account)
@sub_account1.settings = @sub_account1.settings.merge({ :global_stylesheet => '/path/to/sub1/css' })
@sub_account1.save!
@sub_account2 = account_model(:root_account => @domain_root_account)
@sub_account2.settings = @sub_account2.settings.merge({ :global_stylesheet => '/path/to/sub2/css' })
@sub_account2.save!
end
it "should include sub-account css" do
@context = @sub_account1
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/'], ['sub1/']]
end
it "should not include sub-account css when root account is context" do
@context = @domain_root_account
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/']]
end
it "should include sub-account css for course context" do
@context = @sub_account1.courses.create!
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/'], ['sub1/']]
end
it "should include sub-account css for group context" do
@course = @sub_account1.courses.create!
@context = @course.groups.create!
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/'], ['sub1/']]
end
it "should use include sub-account css, if sub-account is lowest common account context" do
@course = @sub_account1.courses.create!
@course.offer!
student_in_course(:active_all => true)
@context = @user
@current_user = @user
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/'], ['sub1/']]
end
it "should not use include sub-account css, if sub-account is not lowest common account context" do
@course1 = @sub_account1.courses.create!
@course1.offer!
@course2 = @sub_account2.courses.create!
@course2.offer!
student_in_course(:active_all => true, :course => @course1)
student_in_course(:active_all => true, :course => @course2, :user => @user)
@context = @user
@current_user = @user
output = include_account_css
output.should have_tag 'link'
output.scan(%r{/path/to/(sub1/|sub2/|root/|admin/)?css}).should eql [['admin/'], ['root/']]
end
end
end
describe "include_account_js" do
before do
@site_admin = Account.site_admin
@domain_root_account = Account.default
end
context "with no custom js" do
@ -115,31 +247,50 @@ describe ApplicationHelper do
context "with custom js" do
it "should include account javascript" do
@domain_root_account.stubs(:settings).returns(@settings)
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_javascript => '/path/to/js' })
@domain_root_account.save!
output = include_account_js
output.should have_tag 'script'
output.should match %r{/path/to/js}
end
it "should include site admin javascript" do
@site_admin.stubs(:settings).returns(@settings)
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_javascript => '/path/to/js' })
@site_admin.save!
output = include_account_js
output.should have_tag 'script'
output.should match %r{/path/to/js}
end
it "should include both site admin and account javascript" do
Account.any_instance.stubs(:settings).returns(@settings)
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_javascript => '/path/to/js' })
@domain_root_account.save!
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_javascript => '/path/to/js' })
@site_admin.save!
output = include_account_js
output.should have_tag 'script'
output.scan(%r{/path/to/js}).length.should eql 2
end
it "should include site admin javascript first" do
@site_admin.stubs(:settings).returns({ :global_includes => true, :global_javascript => '/path/to/admin/js' })
@domain_root_account.stubs(:settings).returns(@settings)
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_includes => true })
@domain_root_account.settings = @domain_root_account.settings.merge({ :global_javascript => '/path/to/root/js' })
@domain_root_account.save!
@site_admin.settings = @site_admin.settings.merge({ :global_includes => true })
@site_admin.settings = @site_admin.settings.merge({ :global_javascript => '/path/to/admin/js' })
@site_admin.save!
output = include_account_js
output.scan(%r{/path/to/(admin/)?js})[0].should eql ['admin/']
output.scan(%r{/path/to/(admin/|root/)?js}).should eql [['admin/'], ['root/']]
end
end
end

4
spec/models/conversation_spec.rb
View File

@ -237,11 +237,11 @@ describe Conversation do
subscription_guy.reload.unread_conversations_count.should eql 1
subscription_guy.conversations.unread.size.should eql 1
subscription_guy.conversations.first.last_message_at.should eql last_message_at
subscription_guy.conversations.first.last_message_at.to_i.should eql last_message_at.to_i
archive_guy.reload.unread_conversations_count.should eql 1
archive_guy.conversations.unread.size.should eql 1
subscription_guy.conversations.first.last_message_at.should eql last_message_at
subscription_guy.conversations.first.last_message_at.to_i.should eql last_message_at.to_i
end
it "should not toggle read/unread until the subscription change is saved" do

54
spec/models/user_spec.rb
View File

@ -1711,4 +1711,58 @@ describe User do
profile.save!
user.profile.should == profile
end
describe "common_account_chain" do
before do
user_with_pseudonym
end
it "work for just root accounts" do
root_acct1 = Account.create!
root_acct2 = Account.create!
@user.user_account_associations.create!(:account_id => root_acct2.id)
@user.reload
@user.common_account_chain(root_acct1).should be_nil
@user.common_account_chain(root_acct2).should eql [root_acct2]
end
it "should work for one level of sub accounts" do
root_acct = Account.create!
sub_acct1 = Account.create!(:parent_account => root_acct)
sub_acct2 = Account.create!(:parent_account => root_acct)
@user.user_account_associations.create!(:account_id => root_acct.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct]
@user.user_account_associations.create!(:account_id => sub_acct1.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct, sub_acct1]
@user.user_account_associations.create!(:account_id => sub_acct2.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct]
end
it "should work for two levels of sub accounts" do
root_acct = Account.create!
sub_acct1 = Account.create!(:parent_account => root_acct)
sub_sub_acct1 = Account.create!(:parent_account => sub_acct1)
sub_sub_acct2 = Account.create!(:parent_account => sub_acct1)
sub_acct2 = Account.create!(:parent_account => root_acct)
@user.user_account_associations.create!(:account_id => root_acct.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct]
@user.user_account_associations.create!(:account_id => sub_acct1.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct, sub_acct1]
@user.user_account_associations.create!(:account_id => sub_sub_acct1.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct, sub_acct1, sub_sub_acct1]
@user.user_account_associations.create!(:account_id => sub_sub_acct2.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct, sub_acct1]
@user.user_account_associations.create!(:account_id => sub_acct2.id)
@user.reload.common_account_chain(root_acct).should eql [root_acct]
end
end
end

31
spec/selenium/admin/admin_settings_tab_spec.rb
View File

@ -119,6 +119,37 @@ describe "admin settings tab" do
end
end
context "global includes" do
it "should not have a global includes section by default" do
fj('#account_settings_global_includes_settings:visible').should be_nil
end
it "should have a global includes section if enabled" do
Account.default.settings = Account.default.settings.merge({ :global_includes => true })
Account.default.save!
section = f('#account_settings_global_includes_settings')
section.find_element(:id, 'account_settings_sub_account_includes').should_not be_nil
end
it "a sub-account should not have a global includes section by default" do
Account.default.settings = Account.default.settings.merge({ :global_includes => true })
Account.default.save!
acct = account_model(:root_account => Account.default)
get "/accounts/#{acct.id}/settings"
fj('#account_settings_global_includes_settings:visible').should be_nil
end
it "a sub-account should have a global includes section if enabled by the parrent" do
Account.default.settings = Account.default.settings.merge({ :global_includes => true })
Account.default.settings = Account.default.settings.merge({ :sub_account_includes => true })
Account.default.save!
acct = account_model(:root_account => Account.default)
get "/accounts/#{acct.id}/settings"
section = f('#account_settings_global_includes_settings')
section.find_element(:id, 'account_settings_sub_account_includes').should_not be_nil
end
end
context "quiz ip address filter" do
def add_quiz_filter name ="www.canvas.instructure.com", value="192.168.217.1/24"

21
spec/views/layouts/application.html.erb_spec.rb
View File

@ -23,25 +23,4 @@ describe "layouts/application" do
assigns[:domain_root_account] = Account.default
render "layouts/application"
end
it "should include site admin css" do
account = Account.site_admin
account.settings[:global_includes] = true
account.settings[:global_stylesheet] = 'somewhereoutthere'
account.save!
assigns[:domain_root_account] = Account.default
render "layouts/application"
response.body.should match /somewhereoutthere/
end
it "should include site admin css once" do
account = Account.site_admin
account.settings[:global_includes] = true
account.settings[:global_stylesheet] = 'somewhereoutthere'
account.save!
assigns[:domain_root_account] = Account.site_admin
render "layouts/application"
response.body.should match /somewhereoutthere/
response.body.scan(/somewhereoutthere/).length.should == 1
end
end