allow users to view question banks without edit rights

also solidify question bank rights around :read_question_banks

test plan:
* create a teacher who is restricted from editing a course
 through term enrollment dates
* they should see a link to view question banks through
 the quizzes page
* they should be able to view question banks

* this should also apply to account admin roles with
 "View course content" and "View and link to
 question banks" enabled but not "Manage assignments"

closes #CNVS-22457

Change-Id: Iea5fe2e7a4dfcee5f7a68e307bf3d9e33ee14c9a
Reviewed-on: https://gerrit.instructure.com/62894
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Charles Kimball <ckimball@instructure.com>
Product-Review: Cosme Salazar <cosme@instructure.com>

app/controllers/question_banks_controller.rb

@@ -23,12 +23,12 @@ class QuestionBanksController < ApplicationController
   include Api::V1::Outcome
 
   def index
-    if @context == @current_user || authorized_action(@context, @current_user, :manage_assignments)
+    if @context == @current_user || authorized_action(@context, @current_user, :read_question_banks)
       @question_banks = @context.assessment_question_banks.active.except(:preload).to_a
       if params[:include_bookmarked] == '1'
         @question_banks += @current_user.assessment_question_banks.active
       end
-      if params[:inherited] == '1' && @context != @current_user && @context.grants_right?(@current_user, :read_question_banks)
+      if params[:inherited] == '1' && @context != @current_user
         @question_banks += @context.inherited_assessment_question_banks.active
       end
       @question_banks = @question_banks.select{|b| b.grants_right?(@current_user, :manage) } if params[:managed] == '1'

app/controllers/quizzes/quizzes_controller.rb

@@ -117,7 +117,8 @@ class Quizzes::QuizzesController < ApplicationController
       },
       :PERMISSIONS => {
         create: can_do(@context.quizzes.scoped.new, @current_user, :create),
-        manage: can_manage
+        manage: can_manage,
+        read_question_banks: can_manage || can_do(@context, @current_user, :read_question_banks)
       },
       :FLAGS => {
         question_banks: feature_enabled?(:question_banks)

app/models/assessment_question_bank.rb

@@ -39,9 +39,12 @@ class AssessmentQuestionBank < ActiveRecord::Base
   end
 
   set_policy do
-    given{|user, session| self.context.grants_right?(user, session, :manage_assignments) }
+    given{|user, session| self.context.grants_right?(user, session, :read_question_banks) && self.context.grants_right?(user, session, :manage_assignments) }
     can :read and can :create and can :update and can :delete and can :manage
 
+    given{|user, session| self.context.grants_right?(user, session, :read_question_banks) }
+    can :read
+
     given{|user| user && self.assessment_question_bank_users.where(:user_id => user).exists? }
     can :read
   end

app/models/course.rb

@@ -1130,6 +1130,19 @@ class Course < ActiveRecord::Base
     end
     can [:read, :read_as_admin, :read_roster, :read_prior_roster, :read_forum, :use_student_view, :read_outcomes, :view_unpublished_items]
 
+    # overrideable permissions for concluded admins
+    [:read_question_banks].each do |permission|
+      given do |user|
+        !self.deleted? && user &&
+          (prior_enrollments.for_user(user).any?{|e| e.admin? && e.has_permission_to?(permission)} ||
+            user.cached_not_ended_enrollments.any? do |e|
+              e.course_id == self.id && e.admin? && e.completed? && e.has_permission_to?(permission)
+            end
+          )
+      end
+      can permission
+    end
+
     given do |user|
       !self.deleted? && user &&
         (prior_enrollments.for_user(user).any?{|e| e.instructor? } ||

app/views/jst/quizzes/IndexView.handlebars

@@ -21,7 +21,7 @@
       </a>
 
       <ul id="toolbar-1" class="al-options" role="menu" tabindex="0" aria-hidden="true" aria-expanded="false">
-        {{#ifAll permissions.manage flags.question_banks}}
+        {{#ifAll permissions.read_question_banks flags.question_banks}}
         <li role="presentation">
           <a href="{{urls.question_banks_url}}" class="icon-question" tabindex="-1" role="menuitem">
             {{#t "links.manage_question_banks"}}Manage Question Banks{{/t}}
@@ -39,6 +39,16 @@
       </ul>
     </div>
   </div>
+  {{else}}
+    {{#if permissions.read_question_banks}}
+      <div class="header-bar-right form-inline">
+        <a href="{{urls.question_banks_url}}" class="btn view_question_banks" role="button"
+           title='{{#t}}View Question Banks{{/t}}'
+           aria-label='{{#t}}View Question Banks{{/t}}'>
+          {{#t}}View Question Banks{{/t}}
+        </a>
+      </div>
+    {{/if}}
   {{/ifAny}}
 </div>
 

app/views/question_banks/_question_bank.html.erb

@@ -9,7 +9,7 @@
       <% not_yet_bookmarked = question_bank && !question_bank.bookmarked_for?(@current_user) %>
       <a href="<%= context_url(context, :context_question_bank_bookmark_url, question_bank ? question_bank.id : "{{ id }}") %>" class="bookmark_bank_link no-hover" title="<%= t('bookmark_bank', 'Bookmark this Question Bank') %>" style="<%= hidden unless not_yet_bookmarked %>"><%= image_tag "bookmark_gray.png" %></a>
       <a href="<%= context_url(context, :context_question_bank_bookmark_url, question_bank ? question_bank.id : "{{ id }}", :unbookmark => 1) %>" class="bookmark_bank_link no-hover" title="<%= t('unbookmark_bank', 'Unbookmark this Question Bank') %>" style="<%= hidden if not_yet_bookmarked %>"><%= image_tag "bookmark.png" %></a>
-      <% if context == @context %>
+      <% if context == @context && @context.grants_right?(@current_user, session, :manage_assignments) %>
         <% edit_bank_text = t('edit_bank', 'Edit this Question Bank') %>
         <% delete_bank_text = t('delete_bank', 'Delete this Question Bank') %>
         <span style="<%= hidden if question_bank && !can_do(question_bank, @current_user, :update) %>">

app/views/question_banks/index.html.erb

@@ -38,7 +38,7 @@
 
 <% content_for :right_side do %>
   <div class="rs-margin-all">
-    <% if !@context.is_a?(User) %>
+    <% if !@context.is_a?(User) && @context.grants_right?(@current_user, session, :manage_assignments) %>
       <a href="#" class="add_bank_link btn button-sidebar-wide"><i class="icon-add"></i> <%= t 'add_bank', 'Add Question Bank' %></a>
     <% end %>
     <% if @current_user && @context != @current_user %>

app/views/quizzes/quizzes/_find_question_from_bank.html.erb

@@ -3,7 +3,7 @@
   <a href="/<%= @quiz.context_url_prefix %>/question_banks/{{ question_bank_id }}/questions?inherited=1" class="question_bank_questions_url" style="display: none;">&nbsp;</a>
   <a href="<%= context_url(@context, :context_quiz_quiz_questions_url, @quiz.id) %>" class="add_questions_url" style="display: none;">&nbsp;</a>
   <div style="float: right; padding-right: 5px; line-height: 2.5em;">
-    <a href="<%= context_url(@context, :context_question_banks_url) %>"><%= t('links.manage_question_banks', "Manage Course Question Banks") %></a>
+    <a href="<%= context_url(@context, :context_question_banks_url) %>"><%= t('links.view_question_banks', "View Course Question Banks") %></a>
   </div>
   <div class="clear"></div>
   <div class="message" style="display: none;"></div>
@@ -28,7 +28,7 @@
         <div class="side_tabs_content">
           <div class="question_message"></div>
           <div class="question_list_holder" style="display: none;">
-            <a href="#" class="select_all_link"><%= t('links.select_all_banks', "Select All") %></a> | 
+            <a href="#" class="select_all_link"><%= t('links.select_all_banks', "Select All") %></a> |
             <a href="#" class="clear_all_link"><%= t('links.clear_all_banks', "Clear All") %></a>
             <ul class="question_list unstyled_list">
               <li class="found_question blank" style="display: none;">
@@ -59,7 +59,7 @@
 <div id="find_bank_dialog" style="display: none;">
   <a href="<%= context_url(@context, :context_question_banks_url, :include_bookmarked => '1', :inherited => '1') %>" class="find_question_banks_url" style="display: none;">&nbsp;</a>
   <div style="float: right; padding-right: 5px; line-height: 2.5em;">
-    <a href="<%= context_url(@context, :context_question_banks_url) %>"><%= t('links.manage_question_banks', "Manage Course Question Banks") %></a>
+    <a href="<%= context_url(@context, :context_question_banks_url) %>"><%= t('links.view_question_banks', "View Course Question Banks") %></a>
   </div>
   <div class="clear"></div>
   <div class="message" style="display: none;"></div>

app/views/quizzes/quizzes/_question_group.html.erb

@@ -36,7 +36,7 @@
               '#' => "<input name='quiz_group[question_points]' class='float_value' aria-label='#{h(t('labels.points_per_question', "Points per Question"))}' type='text' style='width: 20px;' value='\1'/>"
             }) %>
     </span>
-    <% if !group %>
+    <% if !group && @context.grants_right?(@current_user, :read_question_banks) %>
       <div class="group_edit">
         <a href="#" class="icon-search find_bank_link"><%= t('links.link_to_a_question_bank', "Link to a Question Bank") %></a>
       </div>

app/views/quizzes/quizzes/new.html.erb

@@ -416,7 +416,7 @@
           <i class="icon-add"><span class="screenreader-only"><%= new_question_group_title %></span></i>
           <%= new_question_group_title %>
         </a>
-        <% if feature_enabled?(:question_banks) %>
+        <% if feature_enabled?(:question_banks) && @context.grants_right?(@current_user, :read_question_banks) %>
           <% find_questions_title = t('links.find_questions', "Find Questions") %>
           <a href="#" class="find_question_link btn">
             <i class="icon-search"><span class="screenreader-only"><%= find_questions_title %></span></i>

spec/selenium/quizzes/quizzes_question_banks_spec.rb

@@ -154,23 +154,13 @@ describe 'quizzes question banks' do
       get "/courses/#{@course.id}/quizzes/#{quiz.id}/edit"
       click_questions_tab
 
-      keep_trying_until do
-        f('.find_question_link').click
-        wait_for_ajaximations
-        expect(f('#find_question_dialog')).to be_displayed
-        expect(f('.select_all_link')).to be_displayed
-      end
-      expect(ffj('#find_question_dialog .bank:visible').size).to eq 1
+      expect(f('.find_question_link')).to be_nil
 
-      close_visible_dialog
       keep_trying_until do
         f('.add_question_group_link').click
         wait_for_ajaximations
-        expect(f('.find_bank_link')).to be_displayed
+        expect(f('.find_bank_link')).to be_nil
       end
-      f('.find_bank_link').click
-      wait_for_ajaximations
-      expect(ffj('#find_bank_dialog .bank:visible').size).to eq 1
     end
 
     it 'should create a question group from a question bank', priority: "1", test_id: 319907 do
@@ -249,5 +239,76 @@ describe 'quizzes question banks' do
       expect(f("#question_#{@quest1.id}")).to include_text new_name
       expect(f("#question_#{@quest1.id}")).to include_text new_question_text
     end
+
+    it "should let teachers view question banks in a soft-concluded course (but not edit)" do
+      term = Account.default.enrollment_terms.create!
+      term.set_overrides(Account.default, 'TeacherEnrollment' => {:end_at => 3.days.ago})
+      @course.enrollment_term = term
+      @course.save!
+      @bank = @course.assessment_question_banks.create!(title: 'Test Bank')
+
+      get "/courses/#{@course.id}/quizzes"
+
+      view_banks_link = f('.view_question_banks')
+      expect(view_banks_link).to be_displayed
+
+      expect_new_page_load { view_banks_link.click }
+
+      expect(f('.add_bank_link')).to be_nil
+      expect(f('.edit_bank_link')).to be_nil
+      expect(f('.delete_bank_link')).to be_nil
+
+      view_bank_link = f("#question_bank_#{@bank.id} a.title")
+      expect(view_bank_link).to be_displayed
+
+      expect_new_page_load { view_bank_link.click }
+    end
+
+    it "should let account admins view question banks without :manage_assignments (but not edit)" do
+      user(:active_all => true)
+      user_session(@user)
+      @role = custom_account_role 'weakling', :account => @course.account
+      @course.account.role_overrides.create!(:permission => 'read_course_content', :enabled => true, :role => @role)
+      @course.account.role_overrides.create!(:permission => 'read_question_banks', :enabled => true, :role => @role)
+      @course.account.account_users.create!(user: @user, role: @role)
+
+      @bank = @course.assessment_question_banks.create!(title: 'Test Bank')
+
+      get "/courses/#{@course.id}/quizzes"
+
+      view_banks_link = f('.view_question_banks')
+      expect(view_banks_link).to be_displayed
+
+      expect_new_page_load { view_banks_link.click }
+
+      expect(f('.add_bank_link')).to be_nil
+      expect(f('.edit_bank_link')).to be_nil
+      expect(f('.delete_bank_link')).to be_nil
+
+      view_bank_link = f("#question_bank_#{@bank.id} a.title")
+      expect(view_bank_link).to be_displayed
+
+      expect_new_page_load { view_bank_link.click }
+    end
+
+    it "should lock out teachers when :read_question_banks is disabled" do
+      term = Account.default.enrollment_terms.create!
+      term.set_overrides(Account.default, 'TeacherEnrollment' => {:end_at => 3.days.ago})
+      @course.enrollment_term = term
+      @course.save!
+
+      @bank = @course.assessment_question_banks.create!(title: 'Test Bank')
+
+      Account.default.role_overrides.create(:permission => 'read_question_banks', :role => teacher_role, :enabled => false)
+
+      get "/courses/#{@course.id}/quizzes"
+      expect(f('.view_question_banks')).to be_nil
+
+      get "/courses/#{@course.id}/question_banks"
+      expect(f('#unauthorized_message')).to be_displayed
+
+      get "/courses/#{@course.id}/question_banks/#{@bank.id}"
+      expect(f('#unauthorized_message')).to be_displayed
+    end
   end
 end