Fix API fields filtering

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)

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
 

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|

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

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

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

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