Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/controllers/course/gradebook_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
6 changes: 2 additions & 4 deletions app/models/components/course/gradebook_ability_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
24 changes: 24 additions & 0 deletions app/models/course/assessment/tab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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<Hash>] 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.
#
Expand Down
4 changes: 4 additions & 0 deletions app/views/course/gradebook/index.json.jbuilder
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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|
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@

resource :gradebook, only: [] do
get '/' => 'gradebook#index'
patch '/weights' => 'gradebook#update_weights'
end

scope module: :discussion do
Expand Down
143 changes: 143 additions & 0 deletions spec/controllers/course/gradebook_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
41 changes: 41 additions & 0 deletions spec/models/course/assessment/tab_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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