From c87229209add7d3d09ddc5d7794317e17b4da564 Mon Sep 17 00:00:00 2001 From: Guflly <145608489+Guflly@users.noreply.github.com> Date: Sun, 2 Aug 2026 14:11:18 -0700 Subject: [PATCH] Fix API fields filtering --- Changelog.md | 1 + app/controllers/api/courses_controller.rb | 10 ++++---- app/controllers/api/main_api_controller.rb | 14 +++++++++++ app/controllers/api/users_controller.rb | 10 ++++---- doc/markus-contributors.txt | 1 + .../api/courses_controller_spec.rb | 18 +++++++++++++++ spec/controllers/api/users_controller_spec.rb | 23 +++++++++++++++++++ 7 files changed, 69 insertions(+), 8 deletions(-) diff --git a/Changelog.md b/Changelog.md index f056c9c6f0..212a983bd0 100644 --- a/Changelog.md +++ b/Changelog.md @@ -23,6 +23,7 @@ - Added GET /test_runs API route (#8055) ### šŸ› Bug fixes +- Fixed API user and course endpoints ignoring the optional fields parameter - Ensured random grader assignment excludes ineligible roles and recalculates weights using eligible graders (#8073) - Prevented grader assignment and unassignment operations from modifying groupings belonging to other assignments (#8072) - Fixed the "Fix" link being clipped off the screen for long QR-scan error messages on the exam scan log table (#8070) diff --git a/app/controllers/api/courses_controller.rb b/app/controllers/api/courses_controller.rb index 579882d499..f6fde7f38d 100644 --- a/app/controllers/api/courses_controller.rb +++ b/app/controllers/api/courses_controller.rb @@ -11,17 +11,19 @@ def index else courses = get_collection(current_user.visible_courses) end + fields = get_fields || return respond_to do |format| - format.xml { render xml: courses.to_xml(only: DEFAULT_FIELDS, root: 'courses', skip_types: 'true') } - format.json { render json: courses.to_json(only: DEFAULT_FIELDS) } + format.xml { render xml: courses.to_xml(only: fields, root: 'courses', skip_types: 'true') } + format.json { render json: courses.to_json(only: fields) } end end def show course = current_course + fields = get_fields || return respond_to do |format| - format.xml { render xml: course.to_xml(only: DEFAULT_FIELDS, root: 'course', skip_types: 'true') } - format.json { render json: course.to_json(only: DEFAULT_FIELDS) } + format.xml { render xml: course.to_xml(only: fields, root: 'course', skip_types: 'true') } + format.json { render json: course.to_json(only: fields) } end end diff --git a/app/controllers/api/main_api_controller.rb b/app/controllers/api/main_api_controller.rb index a410ce0ca8..6595744ca3 100644 --- a/app/controllers/api/main_api_controller.rb +++ b/app/controllers/api/main_api_controller.rb @@ -81,6 +81,20 @@ def get_collection(collection) end end + def get_fields + return self.class::DEFAULT_FIELDS if params[:fields].blank? + + fields = params.permit(fields: [])[:fields]&.map { |field| field.to_s.to_sym } + fields = fields&.intersection(self.class::DEFAULT_FIELDS) + if fields.blank? + render 'shared/http_status', locals: { code: '422', message: + 'Invalid or malformed parameter values' }, status: :unprocessable_content + false + else + fields + end + end + # Checks that the symbols provided in the array aren't blank in the params def has_missing_params?(required_params) required_params.each do |param| diff --git a/app/controllers/api/users_controller.rb b/app/controllers/api/users_controller.rb index 9e11effc58..a7541a4a07 100644 --- a/app/controllers/api/users_controller.rb +++ b/app/controllers/api/users_controller.rb @@ -9,11 +9,12 @@ class UsersController < MainApiController # Optional: filter, fields def index users = get_collection(visible_users) || return + fields = get_fields || return respond_to do |format| - format.xml { render xml: users.to_xml(only: DEFAULT_FIELDS, root: :users, skip_types: true) } + format.xml { render xml: users.to_xml(only: fields, root: :users, skip_types: true) } format.json do - render json: users.pluck_to_hash(*DEFAULT_FIELDS) + render json: users.pluck_to_hash(*fields) end end end @@ -70,9 +71,10 @@ def show render 'shared/http_status', locals: { code: '404', message: 'No user exists with that id' }, status: :not_found else + fields = get_fields || return respond_to do |format| - format.xml { render xml: user.to_xml(only: DEFAULT_FIELDS, root: :user, skip_types: true) } - format.json { render json: user.to_json(only: DEFAULT_FIELDS) } + format.xml { render xml: user.to_xml(only: fields, root: :user, skip_types: true) } + format.json { render json: user.to_json(only: fields) } end end end diff --git a/doc/markus-contributors.txt b/doc/markus-contributors.txt index db558aa574..d3dbf2bed4 100644 --- a/doc/markus-contributors.txt +++ b/doc/markus-contributors.txt @@ -88,6 +88,7 @@ GaĆ«tan Girin Geoffrey Flores Ghislain Guiot Gillian Chesnais +Guflly Hannah Li Hanson Wu Haohan David Jiang diff --git a/spec/controllers/api/courses_controller_spec.rb b/spec/controllers/api/courses_controller_spec.rb index 3b9dc5b439..b3ee8038ff 100644 --- a/spec/controllers/api/courses_controller_spec.rb +++ b/spec/controllers/api/courses_controller_spec.rb @@ -33,6 +33,12 @@ keys = Hash.from_xml(response.body).dig('courses', 'course').keys.map(&:to_sym) expect(keys).to match_array Api::CoursesController::DEFAULT_FIELDS end + + it 'should return only requested fields' do + get :index, params: { fields: %w[id name] } + keys = Hash.from_xml(response.body).dig('courses', 'course').keys + expect(keys).to match_array(%w[id name]) + end end context 'with multiple courses' do @@ -93,6 +99,12 @@ keys = response.parsed_body&.first&.keys&.map(&:to_sym) expect(keys).to match_array Api::CoursesController::DEFAULT_FIELDS end + + it 'should return only requested fields' do + get :index, params: { fields: %w[id name] } + keys = response.parsed_body&.first&.keys + expect(keys).to match_array(%w[id name]) + end end context 'with multiple courses' do @@ -186,6 +198,12 @@ expect(Time.zone.parse(response.parsed_body['start_at'])).to be_within(1.second).of(start_time) expect(Time.zone.parse(response.parsed_body['end_at'])).to be_within(1.second).of(end_time) end + + it 'returns only requested fields' do + get :show, params: { id: course.id, fields: %w[id name] } + + expect(response.parsed_body.keys).to match_array(%w[id name]) + end end it 'should fail to authenticate a POST create request' do diff --git a/spec/controllers/api/users_controller_spec.rb b/spec/controllers/api/users_controller_spec.rb index 484e714574..249362ba6f 100644 --- a/spec/controllers/api/users_controller_spec.rb +++ b/spec/controllers/api/users_controller_spec.rb @@ -70,6 +70,12 @@ info = Hash.from_xml(response.body).dig('users', 'user')[0] expect(Set.new(info.keys.map(&:to_sym))).to eq Set.new(Api::UsersController::DEFAULT_FIELDS) end + + it 'should return only requested fields' do + get :index, params: { fields: %w[id user_name] } + info = Hash.from_xml(response.body).dig('users', 'user')[0] + expect(info.keys).to match_array(%w[id user_name]) + end end context 'expecting an json response' do @@ -98,6 +104,12 @@ info = response.parsed_body[0] expect(Set.new(info.keys.map(&:to_sym))).to eq Set.new(Api::UsersController::DEFAULT_FIELDS) end + + it 'should return only requested fields' do + get :index, params: { fields: %w[id user_name] } + info = response.parsed_body[0] + expect(info.keys).to match_array(%w[id user_name]) + end end end @@ -134,6 +146,12 @@ info = Hash.from_xml(response.body)['user'] expect(Set.new(info.keys.map(&:to_sym))).to eq Set.new(Api::UsersController::DEFAULT_FIELDS) end + + it 'should return only requested fields' do + get :show, params: { id: end_users[0].id, fields: %w[id user_name] } + info = Hash.from_xml(response.body)['user'] + expect(info.keys).to match_array(%w[id user_name]) + end end context 'expecting an json response' do @@ -156,6 +174,11 @@ info = response.parsed_body expect(Set.new(info.keys.map(&:to_sym))).to eq Set.new(Api::UsersController::DEFAULT_FIELDS) end + + it 'should return only requested fields' do + get :show, params: { id: end_users[0].id, fields: %w[id user_name] } + expect(response.parsed_body.keys).to match_array(%w[id user_name]) + end end end