From 48ff07c356c56aefe319a942e5e03c4e622996a7 Mon Sep 17 00:00:00 2001 From: Adrian Diaz Date: Wed, 12 May 2021 09:18:34 -0500 Subject: [PATCH] Translate item banks tab label and apply permissions closes QUIZ-8300 QUIZ-8301 flag = new_quizzes_account_course_level_item_banks Please, take in count that cache may affect the result. Perhaps, you will need to test this in a clean environment or wait 1 hour before cache refresh tabs. Also, You'll need an updated lti version running that supports item banks and course_placement and account placement. Test plan: - Log in as Admin - Go to /uuid.quiz.next and provision - Enable all "New Quiz"/"New Quizzes" feature options in Site Admin and your CANVAS_LMS_ACCOUNT_NAME - Create a course and verify "Item Banks" appear - Add a Teacher role user to the course - Enroll a Student to the course - Log out and log in as a Teacher - Go to the course and verify "Item Banks" appear - Log out and log in as a Student - Go to the course and verify "Item Banks" doesn't appear Extra: - Switch user language in settings - Then go back to a Course and see "Item Banks" is translated Change-Id: I296c5ac1a00947530da77cb648bbf0e0953aaff3 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/264787 Product-Review: Susan Sorensen Tested-by: Service Cloud Jenkins Reviewed-by: Stephen Kacsmark QA-Review: Stephen Kacsmark --- app/models/account.rb | 1 + app/models/course.rb | 4 ++ app/models/lti/resource_placement.rb | 8 ++++ spec/models/course_spec.rb | 36 ++++++++++++++++++ spec/models/lti/resource_placement_spec.rb | 44 ++++++++++++++++++++++ 5 files changed, 93 insertions(+) diff --git a/app/models/account.rb b/app/models/account.rb index ddc872636c3..ebdeada5705 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -1756,6 +1756,7 @@ class Account < ActiveRecord::Base tabs += external_tool_tabs(opts, user) tabs += Lti::MessageHandler.lti_apps_tabs(self, [Lti::ResourcePlacement::ACCOUNT_NAVIGATION], opts) + Lti::ResourcePlacement.update_tabs_and_return_item_banks_tab(tabs) tabs << { :id => TAB_ADMIN_TOOLS, :label => t('#account.tab_admin_tools', "Admin Tools"), :css_class => 'admin_tools', :href => :account_admin_tools_path } if can_see_admin_tools_tab?(user) if user && grants_right?(user, :moderate_user_content) tabs << { diff --git a/app/models/course.rb b/app/models/course.rb index bfbebe1041e..1d028d2c0a4 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -3016,6 +3016,8 @@ class Course < ActiveRecord::Base else [] end + item_banks_tab = Lti::ResourcePlacement.update_tabs_and_return_item_banks_tab(external_tabs) + tabs = tabs.map do |tab| default_tab = default_tabs.find {|t| t[:id] == tab[:id] } || external_tabs.find{|t| t[:id] == tab[:id] } if default_tab @@ -3125,6 +3127,8 @@ class Course < ActiveRecord::Base delete_unless.call([TAB_ANNOUNCEMENTS], :read_announcements) delete_unless.call([TAB_RUBRICS], :read_rubrics, :manage_rubrics) + tabs -= [item_banks_tab] if item_banks_tab && !check_for_permission.call(:manage_content, :manage_assignments) + if self.root_account.feature_enabled?(:granular_permissions_course_files) delete_unless.call([TAB_FILES], :read, *RoleOverride::GRANULAR_FILE_PERMISSIONS) else diff --git a/app/models/lti/resource_placement.rb b/app/models/lti/resource_placement.rb index 90d7a0d6c03..0b6aaab4e30 100644 --- a/app/models/lti/resource_placement.rb +++ b/app/models/lti/resource_placement.rb @@ -96,5 +96,13 @@ module Lti p.delete(:conference_selection) unless Account.site_admin.feature_enabled?(:conference_selection_lti_placement) end end + + def self.update_tabs_and_return_item_banks_tab(tabs, new_label = nil) + item_banks_tab = tabs.find {|t| t[:label] == 'Item Banks'} + if item_banks_tab + item_banks_tab[:label] = new_label || t('#tabs.item_banks', 'Item Banks') + end + item_banks_tab + end end end diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index be18358c21d..08192c828aa 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -2737,6 +2737,24 @@ describe Course, "tabs_available" do expect(available_tabs.select{|t| t[:hidden]}).to be_empty end + it "should include item banks tab for active external tools" do + @course.context_external_tools.create!( + :url => "http://example.com/ims/lti", + :consumer_key => "asdf", + :shared_secret => "hjkl", + :name => "external tool 1", + :course_navigation => { + :text => "Item Banks", + :url => "http://example.com/ims/lti", + :default => false, + } + ) + + tabs = @course.tabs_available(@user,include_external: true).map { |tab| tab[:label] } + + expect(tabs).to be_include("Item Banks") + end + describe "with canvas_for_elementary account setting on" do context "homeroom course" do before :once do @@ -2910,6 +2928,24 @@ describe Course, "tabs_available" do expect(tabs).not_to be_include(t2.asset_string) end + it "should not include item banks tab for active external tools" do + @course.context_external_tools.create!( + :url => "http://example.com/ims/lti", + :consumer_key => "asdf", + :shared_secret => "hjkl", + :name => "external tool 1", + :course_navigation => { + :text => "Item Banks", + :url => "http://example.com/ims/lti", + :default => false, + } + ) + + tabs = @course.tabs_available(@user,include_external: true).map { |tab| tab[:label] } + + expect(tabs).not_to be_include("Item Banks") + end + it 'sets the target value on the tab if the external tool has a windowTarget' do tool = @course.context_external_tools.create!( :url => "http://example.com/ims/lti", diff --git a/spec/models/lti/resource_placement_spec.rb b/spec/models/lti/resource_placement_spec.rb index 5e2991db86f..22a1ed6fc28 100644 --- a/spec/models/lti/resource_placement_spec.rb +++ b/spec/models/lti/resource_placement_spec.rb @@ -53,5 +53,49 @@ module Lti expect(described_class.valid_placements(Account.default)).to include(:submission_type_selection) end end + + describe 'update_tabs_and_return_item_banks_tab' do + let(:tabs_with_item_banks) { + [ + { + :id=>"context_external_tool_1", + :label=>"Item Banks", + :css_class=>"context_external_tool_1", + :visibility=>nil, + :href=>:course_external_tool_path, + :external=>true, + :hidden=>false, + :args=>[2, 1] + } + ] + } + + let(:tabs_without_item_banks) { + [ + { + :id=>"context_external_tool_1", + :label=>"Another", + :css_class=>"context_external_tool_1", + :visibility=>nil, + :href=>:course_external_tool_path, + :external=>true, + :hidden=>false, + :args=>[2, 1] + } + ] + } + + it 'updates item banks tab label' do + tabs = tabs_with_item_banks + described_class.update_tabs_and_return_item_banks_tab(tabs, :new_label) + expect(tabs[0][:label]).to eq :new_label + end + + it 'let tabs as the same' do + tabs = tabs_without_item_banks + described_class.update_tabs_and_return_item_banks_tab(tabs) + expect(tabs).to eq tabs_without_item_banks + end + end end end