diff --git a/app/controllers/course/gradebook_controller.rb b/app/controllers/course/gradebook_controller.rb index fe739dc11e..c54fe2d6f7 100644 --- a/app/controllers/course/gradebook_controller.rb +++ b/app/controllers/course/gradebook_controller.rb @@ -5,6 +5,7 @@ class Course::GradebookController < Course::ComponentController def index respond_to do |format| format.json do + @weighted_view_enabled = @settings.weighted_view_enabled @published_assessments = fetch_published_assessments @categories, @tabs = fetch_categories_and_tabs @students = fetch_students @@ -18,12 +19,27 @@ def index end end + def update_weights + authorize! :manage_gradebook_weights, current_course + updates = update_weights_params[:weights].map do |entry| + { tab_id: entry[:tabId].to_i, weight: entry[:weight].to_i } + end + Course::Assessment::Tab.update_gradebook_weights(course: current_course, updates: updates) + render json: { weights: updates.map { |u| { tabId: u[:tab_id], weight: u[:weight] } } } + rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotFound => e + render json: { errors: { base: e.message } }, status: :unprocessable_entity + end + private def authorize_read_gradebook! authorize! :read_gradebook, current_course end + def update_weights_params + params.permit(weights: [:tabId, :weight]) + end + def component current_component_host[:course_gradebook_component] end diff --git a/app/models/components/course/gradebook_ability_component.rb b/app/models/components/course/gradebook_ability_component.rb index e084be9509..d54a56cca6 100644 --- a/app/models/components/course/gradebook_ability_component.rb +++ b/app/models/components/course/gradebook_ability_component.rb @@ -4,10 +4,8 @@ module Course::GradebookAbilityComponent def define_permissions can :read_gradebook, Course, id: course.id if course_user&.staff? - if course_user&.manager_or_owner? - can :manage_gradebook_weights, Course, id: course.id # Reserved for weights-editing endpoint in follow-on PR - can :manage_gradebook_settings, Course, id: course.id - end + can :manage_gradebook_weights, Course, id: course.id if course_user&.manager_or_owner? + can :manage_gradebook_settings, Course, id: course.id if course_user&.manager_or_owner? super end end diff --git a/app/models/course/assessment/tab.rb b/app/models/course/assessment/tab.rb index 305bfc5e1c..4013558f34 100644 --- a/app/models/course/assessment/tab.rb +++ b/app/models/course/assessment/tab.rb @@ -29,6 +29,30 @@ class Course::Assessment::Tab < ApplicationRecord select('(array_agg(title))[0:3]') end) + # Bulk-updates the gradebook_weight for a set of tabs belonging to the given course. + # Raises ActiveRecord::RecordNotFound if any tab_id does not belong to the course. + # Raises ActiveRecord::RecordInvalid if any weight fails validation; the transaction is rolled back. + # + # @param course [Course] + # @param updates [Array] array of { tab_id: Integer, weight: Integer } + def self.update_gradebook_weights(course:, updates:) + course_tab_ids = course.assessment_tabs.pluck(:id).to_set + tab_ids_to_update = updates.map { |e| e[:tab_id] } + + tab_ids_to_update.each do |tab_id| + raise ActiveRecord::RecordNotFound unless course_tab_ids.include?(tab_id) + end + + tabs_by_id = where(id: tab_ids_to_update).index_by(&:id) + + transaction do + updates.each do |entry| + tab = tabs_by_id[entry[:tab_id]] + tab.update!(gradebook_weight: entry[:weight]) + end + end + end + # Returns a boolean value indicating if there are other tabs # besides this one remaining in its category. # diff --git a/app/views/course/gradebook/index.json.jbuilder b/app/views/course/gradebook/index.json.jbuilder index 8c67f0a070..edd6daa4d1 100644 --- a/app/views/course/gradebook/index.json.jbuilder +++ b/app/views/course/gradebook/index.json.jbuilder @@ -1,4 +1,7 @@ # frozen_string_literal: true +json.weightedViewEnabled @weighted_view_enabled +json.canManageWeights can?(:manage_gradebook_weights, current_course) + json.categories @categories do |cat| json.id cat.id json.title cat.title @@ -8,6 +11,7 @@ json.tabs @tabs do |tab| json.id tab.id json.title tab.title json.categoryId tab.category_id + json.gradebookWeight tab.gradebook_weight if @weighted_view_enabled end json.assessments @published_assessments do |assessment| diff --git a/config/routes.rb b/config/routes.rb index 0b6f16f368..6838d77f24 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -501,6 +501,7 @@ resource :gradebook, only: [] do get '/' => 'gradebook#index' + patch '/weights' => 'gradebook#update_weights' end scope module: :discussion do diff --git a/spec/controllers/course/gradebook_controller_spec.rb b/spec/controllers/course/gradebook_controller_spec.rb index 4b21113c31..01fc03c37d 100644 --- a/spec/controllers/course/gradebook_controller_spec.rb +++ b/spec/controllers/course/gradebook_controller_spec.rb @@ -185,5 +185,148 @@ end end end + + describe 'PATCH update_weights' do + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:student) { create(:course_student, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab1) { create(:course_assessment_tab, category: category) } + let!(:tab2) { create(:course_assessment_tab, category: category) } + + let(:valid_payload) do + { weights: [{ tabId: tab1.id, weight: 60 }, { tabId: tab2.id, weight: 40 }] } + end + + context 'as manager' do + before { controller_sign_in(controller, manager.user) } + + it 'updates and returns 200' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'accepts sum < 100' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 30] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'accepts sum > 100' do + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 70 }, { tabId: tab2.id, weight: 70 }] }, + format: :json + expect(response).to have_http_status(:ok) + end + + it 'rejects negative with 422 and no partial write' do + tab1.update!(gradebook_weight: 10) + patch :update_weights, + params: { course_id: course.id, + weights: [{ tabId: tab1.id, weight: 50 }, { tabId: tab2.id, weight: -1 }] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + expect(tab1.reload.gradebook_weight).to eq(10) + end + + it 'rejects >100 with 422' do + patch :update_weights, + params: { course_id: course.id, weights: [tabId: tab1.id, weight: 101] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'rejects foreign tab id with 422' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + patch :update_weights, + params: { course_id: course.id, weights: [tabId: other_tab.id, weight: 50] }, + format: :json + expect(response).to have_http_status(:unprocessable_entity) + end + end + + context 'as TA' do + before { controller_sign_in(controller, ta.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'as student' do + before { controller_sign_in(controller, student.user) } + it 'is denied' do + expect do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + end.to raise_error(CanCan::AccessDenied) + end + end + + context 'when setting is disabled' do + before { controller_sign_in(controller, manager.user) } + + it 'still allows update (storage independent of display)' do + patch :update_weights, params: { course_id: course.id, **valid_payload }, format: :json + expect(response).to have_http_status(:ok) + expect(tab1.reload.gradebook_weight).to eq(60) + end + end + end + + describe 'GET index — weighted view fields' do + render_views + let(:manager) { create(:course_manager, course: course) } + let(:ta) { create(:course_teaching_assistant, course: course) } + let(:category) { create(:course_assessment_category, course: course) } + let!(:tab) { create(:course_assessment_tab, category: category, gradebook_weight: 30) } + let!(:assessment) do + create(:course_assessment_assessment, :published_with_mcq_question, + course: course, tab: tab) + end + + context 'when setting is disabled (default)' do + before { controller_sign_in(controller, manager.user) } + + it 'returns weightedViewEnabled false and omits gradebookWeight per tab' do + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(false) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json).not_to have_key('gradebookWeight') + end + end + + context 'when setting is enabled' do + before do + ctx = Struct.new(:current_course, :key).new(course, Course::GradebookComponent.key) + Course::Settings::GradebookComponent.new(ctx).weighted_view_enabled = true + course.save! + end + + it 'includes weightedViewEnabled true and gradebookWeight per tab for manager' do + controller_sign_in(controller, manager.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['weightedViewEnabled']).to eq(true) + expect(body['canManageWeights']).to eq(true) + tab_json = body['tabs'].find { |t| t['id'] == tab.id } + expect(tab_json['gradebookWeight']).to eq(30) + end + + it 'returns canManageWeights false for TA' do + controller_sign_in(controller, ta.user) + get :index, params: { course_id: course.id }, format: :json + body = JSON.parse(response.body) + expect(body['canManageWeights']).to eq(false) + end + end + end end end diff --git a/spec/models/course/assessment/tab_spec.rb b/spec/models/course/assessment/tab_spec.rb index 6eaea2f517..b546671032 100644 --- a/spec/models/course/assessment/tab_spec.rb +++ b/spec/models/course/assessment/tab_spec.rb @@ -53,5 +53,46 @@ expect(tab).not_to be_valid end end + + describe '.update_gradebook_weights' do + let(:course) { create(:course) } + let(:category) { create(:course_assessment_category, course: course) } + let(:tab1) { create(:course_assessment_tab, category: category) } + let(:tab2) { create(:course_assessment_tab, category: category) } + + it 'updates given tabs' do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 60 }, { tab_id: tab2.id, weight: 40 }] + ) + expect(tab1.reload.gradebook_weight).to eq(60) + expect(tab2.reload.gradebook_weight).to eq(40) + end + + it 'is transactional — invalid value rolls back everything' do + tab1.update!(gradebook_weight: 10) + tab2.update!(gradebook_weight: 20) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [{ tab_id: tab1.id, weight: 50 }, { tab_id: tab2.id, weight: 999 }] + ) + end.to raise_error(ActiveRecord::RecordInvalid) + expect(tab1.reload.gradebook_weight).to eq(10) + expect(tab2.reload.gradebook_weight).to eq(20) + end + + it 'rejects foreign tab_id' do + other_course = create(:course) + other_tab = create(:course_assessment_tab, + category: create(:course_assessment_category, course: other_course)) + expect do + described_class.update_gradebook_weights( + course: course, + updates: [tab_id: other_tab.id, weight: 50] + ) + end.to raise_error(ActiveRecord::RecordNotFound) + end + end end end