Skip to content

Commit

Permalink
validate default_view on courses
Browse files Browse the repository at this point in the history
don't let UI restrictions on 'wiki' type be bypassed via API

test plan:
* use the courses update API endpoint to
 update the 'default_view' value
* should only be able to set to 'wiki' if a wiki front
 page exists
* should not be able to set to values other
 than those specified in the API documentation

closes #ADMIN-828

Change-Id: Ib18e8cc1df4cda7de3b8b639589a6d46a6e7c031
Reviewed-on: https://gerrit.instructure.com/142832
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <[email protected]>
QA-Review: Deepeeca Soundarrajan <[email protected]>
Product-Review: James Williams  <[email protected]>
  • Loading branch information
maneframe committed Mar 7, 2018
1 parent f1b0c86 commit d56dbc0
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 14 deletions.
20 changes: 15 additions & 5 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,6 @@ def inherited_assessment_question_banks(include_self = false)
prepend Profile::Association

before_save :assign_uuid
before_save :assign_default_view
before_validation :assert_defaults
before_save :update_enrollments_later
before_save :update_show_total_grade_as_on_weighting_scheme_change
Expand All @@ -218,6 +217,7 @@ def inherited_assessment_question_banks(include_self = false)
before_validation :verify_unique_ids
validate :validate_course_dates
validate :validate_course_image
validate :validate_default_view
validates_presence_of :account_id, :root_account_id, :enrollment_term_id, :workflow_state
validates_length_of :syllabus_body, :maximum => maximum_long_text_length, :allow_nil => true, :allow_blank => true
validates_length_of :name, :maximum => maximum_string_length, :allow_nil => true, :allow_blank => true
Expand Down Expand Up @@ -403,6 +403,19 @@ def valid_course_image_url?(image_url)
URI.parse(image_url) rescue false
end

def validate_default_view
if self.default_view_changed?
if !%w{assignments feed modules syllabus wiki}.include?(self.default_view)
self.errors.add(:default_view, t("Home page is not valid"))
return false
elsif self.default_view == 'wiki' && !(self.wiki_id && self.wiki.has_front_page?)
self.errors.add(:default_view, t("A Front Page is required"))
return false
end
end
true
end

def image
if self.image_id.present?
self.shard.activate do
Expand Down Expand Up @@ -916,6 +929,7 @@ def assert_defaults
if self.course_format && !['on_campus', 'online', 'blended'].include?(self.course_format)
self.course_format = nil
end
self.default_view ||= default_home_page
true
end

Expand Down Expand Up @@ -2814,10 +2828,6 @@ def user_list_search_mode_for(user)
:closed
end

def assign_default_view
self.default_view ||= default_home_page
end

def default_home_page
"modules"
end
Expand Down
1 change: 1 addition & 0 deletions app/models/importers/course_content_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,7 @@ def self.import_settings_from_migration(course, data, migration)

# superhax to force new wiki front page if home view changed (or is master course sync)
if settings['default_view'] && data[:wikis] && (migration.for_master_course_import? || (settings['default_view'] != course.default_view))
course.wiki # ensure that it exists already
if page_hash = data[:wikis].detect{|h| h[:front_page]}
if page = migration.find_imported_migration_item(WikiPage, page_hash[:migration_id])
page.set_as_front_page!
Expand Down
24 changes: 21 additions & 3 deletions spec/apis/v1/courses_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ def method_missing(method_name, *args)
course_with_student(:user => @user, :active_all => true)
@course2 = @course
@course2.update_attribute(:sis_source_id, 'TEST-SIS-ONE.2011')
@course2.update_attribute(:default_view, 'wiki')
@course2.update_attributes(:default_view => 'assignments')
@user.pseudonym.update_attribute(:sis_user_id, 'user1')
end

Expand Down Expand Up @@ -1005,7 +1005,7 @@ def method_missing(method_name, *args)
'hide_final_grades' => false,
'apply_assignment_group_weights' => true,
'restrict_enrollments_to_course_dates' => true,
'default_view' => 'new default view',
'default_view' => 'syllabus',
'course_format' => 'on_campus',
'time_zone' => 'Pacific/Honolulu'
}, 'offer' => true }
Expand Down Expand Up @@ -1049,11 +1049,29 @@ def method_missing(method_name, *args)
expect(@course.restrict_enrollments_to_course_dates).to be_truthy
expect(@course.workflow_state).to eq 'available'
expect(@course.apply_group_weights?).to eq true
expect(@course.default_view).to eq 'new default view'
expect(@course.default_view).to eq 'syllabus'
expect(@course.course_format).to eq 'on_campus'
expect(@course.time_zone.tzinfo.name).to eq 'Pacific/Honolulu'
end

it "should not be able to update default_view to arbitrary values" do
json = api_call(:put, @path, @params, {'course' => {'default_view' => 'somethingsilly'}}, {}, {:expected_status => 400})
expect(json["errors"]["default_view"].first['message']).to eq "Home page is not valid"
end

it "should not be able to update default_view to 'wiki' without a front page" do
expect(@course.wiki.front_page).to be_nil
json = api_call(:put, @path, @params, {'course' => {'default_view' => 'wiki'}}, {}, {:expected_status => 400})
expect(json["errors"]["default_view"].first['message']).to eq "A Front Page is required"
end

it "should be able to update default_view to 'wiki' with a front page" do
wp = @course.wiki_pages.create!(:title => "something")
wp.set_as_front_page!
json = api_call(:put, @path, @params, {'course' => {'default_view' => 'wiki'}}, {}, {:expected_status => 200})
expect(@course.reload.default_view).to eq 'wiki'
end

it "should not change dates that aren't given" do
@course.update_attribute(:conclude_at, '2013-01-01T23:59:59Z')
@new_values['course'].delete('end_at')
Expand Down
3 changes: 2 additions & 1 deletion spec/controllers/application_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1082,11 +1082,12 @@ def tool_settings(setting, include_class=false)
describe "set_js_wiki_data" do
before :each do
course_with_teacher_logged_in :active_all => true
@course.wiki_pages.create!(:title => 'blah').set_as_front_page!
@course.reload
@course.default_view = "wiki"
@course.show_announcements_on_home_page = true
@course.home_page_announcement_limit = 5
@course.save!
@course.wiki_pages.create!(:title => 'blah').set_as_front_page!
end

it "should populate js_env with course_home setting" do
Expand Down
9 changes: 6 additions & 3 deletions spec/controllers/courses_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -898,34 +898,37 @@ def check_course_show(should_show)
end

it "should work for wiki view with draft state enabled" do
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
@course1.reload
@course1.default_view = "wiki"
@course1.save!
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
get 'show', params: {:id => @course1.id}
expect(controller.js_env[:WIKI_RIGHTS].symbolize_keys).to eql({:read => true})
expect(controller.js_env[:PAGE_RIGHTS].symbolize_keys).to eql({:read => true})
expect(controller.js_env[:COURSE_TITLE]).to eql @course1.name
end

it "should work for wiki view with home page announcements enabled" do
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
@course1.reload
@course1.default_view = "wiki"
@course1.show_announcements_on_home_page = true
@course1.home_page_announcement_limit = 3
@course1.save!
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
get 'show', params: {:id => @course1.id}
expect(controller.js_env[:COURSE_HOME]).to be_truthy
expect(controller.js_env[:SHOW_ANNOUNCEMENTS]).to be_truthy
expect(controller.js_env[:ANNOUNCEMENT_LIMIT]).to eq(3)
end

it "should not show announcements for public users" do
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
@course1.reload
@course1.default_view = "wiki"
@course1.show_announcements_on_home_page = true
@course1.home_page_announcement_limit = 3
@course1.is_public = true
@course1.save!
@course1.wiki_pages.create!(:title => 'blah').set_as_front_page!
remove_user_session
get 'show', params: {:id => @course1.id}
expect(response).to be_success
Expand Down
2 changes: 1 addition & 1 deletion spec/models/content_migration/course_copy_wiki_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,9 @@
end

it "should set default view to modules if wiki front page is missing" do
@copy_from.wiki.set_front_page_url!('haha not here')
@copy_from.default_view = 'wiki'
@copy_from.save!
@copy_from.wiki.set_front_page_url!('haha not here')

run_course_copy

Expand Down
4 changes: 3 additions & 1 deletion spec/selenium/courses_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,13 @@ def enroll_student(student, accept_invitation)
html = "<p>#{text}</p>"
@course.announcements.create!(:title => "something", :message => html)

@course.wiki_pages.create!(:title => 'blah').set_as_front_page!

@course.reload
@course.default_view = "wiki"
@course.show_announcements_on_home_page = true
@course.home_page_announcement_limit = 5
@course.save!
@course.wiki_pages.create!(:title => 'blah').set_as_front_page!

get "/courses/#{@course.id}"

Expand Down

0 comments on commit d56dbc0

Please sign in to comment.