Skip to content

Commit

Permalink
mobile css and js overrides
Browse files Browse the repository at this point in the history
closes CNVS-19900

test plan:
 - create a brand_config for an account
 - apply brand_config
 - add ?mobile=1 to an api request that returns a
   body (like a wiki page);
   observe no additional stylesheets in the body
 - go to /accounts/self/, and go to brand
   config and upload a mobile_css_override
 - add ?mobile=1 and check the api result again
   your stylesheet should be there appended to the
   body of the wiki page
 - back in brand_config, remove the uploaded file
 - check your source again - there should not be
   an empty stylesheet tag

Change-Id: Ief728a397d185282a57f8cede0916c7ffe4a2584
Reviewed-on: https://gerrit.instructure.com/61885
Tested-by: Jenkins
Reviewed-by: Ryan Shaw <[email protected]>
QA-Review: August Thornton <[email protected]>
Product-Review: Rob Orton <[email protected]>
  • Loading branch information
roor0 committed Sep 28, 2015
1 parent 1f77c5b commit 5ccb7f6
Show file tree
Hide file tree
Showing 13 changed files with 139 additions and 62 deletions.
10 changes: 4 additions & 6 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1701,13 +1701,11 @@ def brand_config_for_account(opts)
private :brand_config_for_account

def brand_config_includes
return @brand_config_includes if defined? @brand_config_includes
includes = {}
if @domain_root_account.allow_global_includes? && active_brand_config.present?
includes[:js] = active_brand_config[:js_overrides] if active_brand_config[:js_overrides].present?
includes[:css] = active_brand_config[:css_overrides] if active_brand_config[:css_overrides].present?
return {} unless @domain_root_account.allow_global_includes?
@brand_config_includes ||= BrandConfig::OVERRIDE_TYPES.each_with_object({}) do |override_type, hsh|
url = active_brand_config.presence.try(override_type)
hsh[override_type] = url if url.present?
end
includes
end
helper_method :brand_config_includes

Expand Down
19 changes: 8 additions & 11 deletions app/controllers/brand_configs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,15 @@ def default_schema
# indicating the progress of generating the css and pushing it to the CDN
# @returns {BrandConfig, Progress}
def create
parent_config = @account.first_parent_brand_config || BrandConfig.new

variables = process_variables(params[:brand_config][:variables])
js_overrides = process_file(params[:js_overrides])
css_overrides = process_file(params[:css_overrides])
opts = {
parent_md5: @account.first_parent_brand_config.try(:md5),
variables: process_variables(params[:brand_config][:variables])
}
BrandConfig::OVERRIDE_TYPES.each do |override|
opts[override] = process_file(params[override])
end

brand_config = BrandConfig.for(
variables: variables,
js_overrides: js_overrides,
css_overrides: css_overrides,
parent_md5: parent_config.md5
)
brand_config = BrandConfig.for(opts)

if existing_config(brand_config)
render json: { brand_config: brand_config.as_json(include_root: false) }
Expand Down
3 changes: 1 addition & 2 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -690,8 +690,7 @@ def get_global_includes
def include_account_js(options = {})
return if params[:global_includes] == '0'
if use_new_styles?
includes = []
includes << brand_config_includes[:js] if brand_config_includes[:js].present?
includes = brand_config_includes.slice(:js_overrides).values
else
includes = get_global_includes.map do |global_include|
global_include[:js] if global_include[:js].present?
Expand Down
28 changes: 28 additions & 0 deletions app/jsx/theme_editor/ThemeEditor.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,34 @@ define([

</div>
</div>
<div className="Theme__editor-upload-overrides">

<div className="Theme__editor-upload-overrides_header">
{ I18n.t('CSS and JavaScript to load when user content is displayed in the canvas iOS or Android native apps') }
</div>

<div className="Theme__editor-upload-overrides_form">

<ThemeEditorFileUpload
label={I18n.t('Upload a CSS file...')}
accept=".css"
name="mobile_css_overrides"
currentValue={this.props.brandConfig.mobile_css_overrides}
userInput={this.state.changedValues.mobile_css_overrides}
onChange={this.changeSomething.bind(null, 'mobile_css_overrides')}
/>

<ThemeEditorFileUpload
label={I18n.t('Upload a JS file...')}
accept=".js"
name="mobile_js_overrides"
currentValue={this.props.brandConfig.mobile_js_overrides}
userInput={this.state.changedValues.mobile_js_overrides}
onChange={this.changeSomething.bind(null, 'mobile_js_overrides')}
/>

</div>
</div>
</div>
: null}

Expand Down
41 changes: 19 additions & 22 deletions app/models/brand_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ class BrandConfig < ActiveRecord::Base
self.primary_key = 'md5'
serialize :variables, Hash

attr_accessible :variables, :js_overrides, :css_overrides
OVERRIDE_TYPES = [:js_overrides, :css_overrides, :mobile_js_overrides, :mobile_css_overrides].freeze
ATTRS_TO_INCLUDE_IN_MD5 = ([:variables, :parent_md5] + OVERRIDE_TYPES).freeze

attr_accessible(*([:variables] + OVERRIDE_TYPES))

validates :variables, presence: true, unless: :overrides?
validates :md5, length: {is: 32}
Expand All @@ -26,19 +29,14 @@ class BrandConfig < ActiveRecord::Base
shared_scope
}

def self.for(variables:, js_overrides:, css_overrides:, parent_md5:)
if variables.blank? && js_overrides.blank? && css_overrides.blank?
default
else
new_config = new(
variables: variables,
js_overrides: js_overrides,
css_overrides: css_overrides,
)
new_config.parent_md5 = parent_md5
existing_config = where(md5: new_config.generate_md5).first
existing_config || new_config
end
def self.for(attrs)
attrs = attrs.with_indifferent_access.slice(*ATTRS_TO_INCLUDE_IN_MD5)
return default if attrs.values.all?(&:blank?)

new_config = new(attrs)
new_config.parent_md5 = attrs[:parent_md5]
existing_config = where(md5: new_config.generate_md5).first
existing_config || new_config
end

def self.default
Expand All @@ -50,24 +48,23 @@ def self.k12_config
end

def default?
self.variables.blank? && self.js_overrides.blank? && self.css_overrides.blank?
([:variables] + OVERRIDE_TYPES).all? {|a| self[a].blank? }
end

def generate_md5
self.id = Digest::MD5.hexdigest([
self.variables.to_s,
self.css_overrides,
self.js_overrides,
self.parent_md5
].join)
self.id = BrandConfig.md5_for(self)
end

def self.md5_for(brand_config)
Digest::MD5.hexdigest(ATTRS_TO_INCLUDE_IN_MD5.map { |a| brand_config[a] }.join)
end

def get_value(variable_name)
self.variables[variable_name]
end

def overrides?
self.js_overrides.present? || self.css_overrides.present?
OVERRIDE_TYPES.any? { |o| self[o].present? }
end

def effective_variables
Expand Down
2 changes: 2 additions & 0 deletions db/migrate/20150708170103_add_overrides_to_brand_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ class AddOverridesToBrandConfig < ActiveRecord::Migration
def change
add_column :brand_configs, :js_overrides, :text
add_column :brand_configs, :css_overrides, :text
add_column :brand_configs, :mobile_js_overrides, :text
add_column :brand_configs, :mobile_css_overrides, :text
end
end
11 changes: 11 additions & 0 deletions db/migrate/20150826200628_add_mobile_overrides_to_brand_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
class AddMobileOverridesToBrandConfig < ActiveRecord::Migration
tag :predeploy
def up
unless column_exists? :brand_configs, :mobile_js_overrides
add_column :brand_configs, :mobile_js_overrides, :text
end
unless column_exists? :brand_configs, :mobile_css_overrides
add_column :brand_configs, :mobile_css_overrides, :text
end
end
end
4 changes: 3 additions & 1 deletion lib/api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,9 @@ def api_user_content(html, context = @context, user = @current_user, preloaded_a
html = rewriter.translate_content(html)

url_helper = Html::UrlProxy.new(self, context, host, protocol)
Html::Content.rewrite_outgoing(html, url_helper)
account = Context.get_account(context, @domain_root_account)
include_mobile = respond_to?(:mobile_device?, true) && mobile_device?
Html::Content.rewrite_outgoing(html, account, url_helper, include_mobile: include_mobile)
end

# This removes the verifier parameters that are added to attachment links by api_user_content
Expand Down
27 changes: 20 additions & 7 deletions lib/api/html/content.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ def self.process_incoming(html)
content.modified_html
end

def self.rewrite_outgoing(html, url_helper)
def self.rewrite_outgoing(html, account, url_helper, include_mobile: false)
return html if html.blank?
self.new(html).rewritten_html(url_helper)
self.new(html, account, include_mobile: include_mobile).rewritten_html(url_helper)
end

attr_reader :html

def initialize(html_string)
@html = html_string
def initialize(html_string, account = nil, include_mobile: false)
@account, @html, @include_mobile = account, html_string, include_mobile
end

def might_need_modification?
Expand Down Expand Up @@ -88,17 +88,30 @@ def rewritten_html(url_helper)
url_helper.rewrite_api_urls(element, attributes)
end
end

if @include_mobile
if (bc = @account.brand_config) && bc.mobile_css_overrides.present?
css = parsed_html.document.create_element("link",
rel: 'stylesheet',
href: bc.mobile_css_overrides)
parsed_html.prepend_child(css)
end
if (bc = @account.brand_config) && bc.mobile_js_overrides.present?
js = parsed_html.document.create_element("script",
source: bc.mobile_js_overrides)
parsed_html.prepend_child(js)
end
end
parsed_html.to_s
end


private

APPLICABLE_ATTRS = %w{href src}
APPLICABLE_ATTRS = %w{href src}.freeze

def scrub_links!(node)
APPLICABLE_ATTRS.each do |attr|
if link = node[attr]
if (link = node[attr])
node[attr] = corrected_link(link)
end
end
Expand Down
9 changes: 3 additions & 6 deletions lib/brand_config_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,8 @@ def child_accounts_with_config
end

def new_brand_config(new_parent_md5)
BrandConfig.for(
variables: brand_config.variables,
js_overrides: brand_config.js_overrides,
css_overrides: brand_config.css_overrides,
parent_md5: new_parent_md5
)
opts = brand_config.attributes.with_indifferent_access.slice(*BrandConfig::ATTRS_TO_INCLUDE_IN_MD5)
opts[:parent_md5] = new_parent_md5
BrandConfig.for(opts)
end
end
27 changes: 24 additions & 3 deletions spec/lib/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -630,17 +630,38 @@ def course_assignment_url(course, assignment)
end

context ".api_user_content" do
class T
extend Api
let(:klass) do
Class.new do
include Api
end
end

it "should ignore non-kaltura instructure_inline_media_comment links" do
student_in_course
html = %{<div>This is an awesome youtube:
<a href="http://www.example.com/" class="instructure_inline_media_comment">here</a>
</div>}
res = T.api_user_content(html, @course, @student)
res = klass.new.api_user_content(html, @course, @student)
expect(res).to eq html
end

it 'prepends mobile css' do
student_in_course
account = @course.root_account
account.enable_feature!(:use_new_styles)
bc = BrandConfig.create(mobile_css_overrides: 'somewhere.css')
account.brand_config_md5 = bc.md5
account.save!

html = "<p>a</p><p>b</p>"

k = klass.new
k.stubs(:mobile_device?).returns(true)
res = k.api_user_content(html, @course, @student)
expect(res).to eq <<-HTML.strip
<link rel="stylesheet" href="somewhere.css"><p>a</p><p>b</p>
HTML
end
end

context ".process_incoming_html_content" do
Expand Down
12 changes: 10 additions & 2 deletions spec/lib/brand_config_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ def setup_account_family_with_configs
variables: {"ic-brand-primary" => "red"},
js_overrides: nil,
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil,
parent_md5: nil
)
@parent_config.save!
Expand All @@ -38,7 +40,9 @@ def setup_account_family_with_configs
variables: {"ic-brand-global-nav-bgd" => "white"},
parent_md5: @parent_config.md5,
js_overrides: nil,
css_overrides: nil
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil
)
@child_config.save!
@child_account.brand_config_md5 = @child_config.md5
Expand All @@ -49,7 +53,9 @@ def setup_account_family_with_configs
variables: {"ic-brand-global-nav-avatar-border" => "blue"},
parent_md5: @child_config.md5,
js_overrides: nil,
css_overrides: nil
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil
)
@grand_child_config.save!
@grand_child_account.brand_config_md5 = @grand_child_config.md5
Expand Down Expand Up @@ -94,6 +100,8 @@ def get_equivalent_md5(config, new_parent_config_md5)
variables: {"ic-brand-primary" => "purple"},
js_overrides: nil,
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil,
parent_md5: nil
)
@new_parent_config.save!
Expand Down
8 changes: 6 additions & 2 deletions spec/models/brand_config_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ def setup_subaccount_with_config
variables: {"ic-brand-global-nav-bgd" => "#123"},
parent_md5: @parent_config.md5,
js_overrides: nil,
css_overrides: nil
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil
)
@subaccount_bc.save!
end
Expand All @@ -58,7 +60,9 @@ def setup_subaccount_with_config
variables: {"ic-brand-global-nav-bgd" => "#123", "ic-brand-primary" => "red"},
parent_md5: @parent_config.md5,
js_overrides: nil,
css_overrides: nil
css_overrides: nil,
mobile_js_overrides: nil,
mobile_css_overrides: nil
)
@new_sub_bc.save!

Expand Down

0 comments on commit 5ccb7f6

Please sign in to comment.