Skip to content

Commit

Permalink
Fixes issue with updating tab via api
Browse files Browse the repository at this point in the history
fixes CNVS-22124

Test Plan:
* Add multiple tools to a course with "course_navigation[default]"
  set to "disabled" (using create external_tool api)
* Update one of the tools with "hidden=false" with tabs
* Go to the course's settings page -> navigation tab
* Only the tab you marked as "hidden=false" should be unhidden
  for the external tools you created

Change-Id: I106996778284a3b69d5f82f4c772497524681285
Reviewed-on: https://gerrit.instructure.com/59496
Reviewed-by: Jeremy Stanley <[email protected]>
Tested-by: Jenkins
QA-Review: Clare Strong <[email protected]>
Product-Review: Dan Minkevitch <[email protected]>
  • Loading branch information
Dan Minkevitch committed Jul 31, 2015
1 parent 386d953 commit 726f8f1
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
8 changes: 7 additions & 1 deletion app/controllers/tabs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,13 @@ def update
tabs = context_tabs
tab = (tabs.find { |t| t.with_indifferent_access[:css_class] == css_class }).with_indifferent_access
tab_config = @context.tab_configuration
tab_config = tabs.map { |t| {'id' => t.with_indifferent_access['id']} } if tab_config.blank?
tab_config = tabs.map do |t|
{
'id' => t.with_indifferent_access['id'],
'hidden' => t.with_indifferent_access['hidden'],
'position' => t.with_indifferent_access['position']
}
end if tab_config.blank?
if [@context.class::TAB_HOME, @context.class::TAB_SETTINGS].include?(tab[:id])
render json: {error: t(:tab_unmanagable_error, "%{css_class} is not manageable", css_class: css_class)}, status: :bad_request
elsif new_pos && (new_pos <= 1 || new_pos >= tab_config.count + 1)
Expand Down
2 changes: 1 addition & 1 deletion app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ def update_show_total_grade_as_on_weighting_scheme_change
# to ensure permissions on the root folder are updated after hiding or showing the files tab
def touch_root_folder_if_necessary
if tab_configuration_changed?
files_tab_was_hidden = tab_configuration_was && tab_configuration_was.any? { |h| h['id'] == TAB_FILES && h['hidden'] }
files_tab_was_hidden = tab_configuration_was && tab_configuration_was.any? { |h| !h.blank? && h['id'] == TAB_FILES && h['hidden'] }
Folder.root_folders(self).each { |f| f.touch } if files_tab_was_hidden != tab_hidden?(TAB_FILES)
end
true
Expand Down
30 changes: 30 additions & 0 deletions spec/apis/v1/tabs_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,36 @@
expect(@course.reload.tab_configuration[json['position'] - 1]['hidden']).to eq true
end

it 'only unhides one tab and not all when first updating' do
course_with_teacher(:active_all => true)
tools = []

3.times do |i|
tool = @course.context_external_tools.new({
:name => "Example #{i}",
:url => 'http://www.example.com',
:consumer_key => 'key',
:shared_secret => 'secret'
})
tool.settings.merge!({
:course_navigation => {
:default => 'disabled',
:url => 'http://www.example.com',
},
})
tool.save!
tools << tool.reload
end

tab_id = "context_external_tool_#{tools.first.id}"
json = api_call(:put, "/api/v1/courses/#{@course.id}/tabs/#{tab_id}", {:controller => 'tabs', :action => 'update',
:course_id => @course.to_param, :tab_id => tab_id,
:format => 'json', :hidden => false})
expect(json['hidden']).to be_nil
expect(@course.reload.tab_configuration[json['position'] - 1]['hidden']).to be_nil
expect(@course.reload.tab_configuration.select { |t| t['hidden'] }.count).to eql(tools.count - 1)
end

it 'allows updating new tabs not in the configuration yet' do
course_with_teacher(:active_all => true)
tab_ids = [0, 1, 3, 8, 5, 6, 14, 2, 11, 15, 4, 10, 13]
Expand Down

0 comments on commit 726f8f1

Please sign in to comment.