Implement ProfileApiClient.update_school_student

I've chosen to make ProfileApiClient responsible for stripping leading
and trailing spaces from name, username and password, and for not
sending them to the API unless they're defined.

API docs: https://my.raspberrypi.org/api/v1/documentation/#/Students/patch_schools__schoolId__students__studentId_

Author: chrisroos

app/controllers/api/school_students_controller.rb

@@ -39,7 +39,9 @@ def create_batch
     end
 
     def update
-      result = SchoolStudent::Update.call(school: @school, school_student_params:, token: current_user.token)
+      result = SchoolStudent::Update.call(
+        school: @school, student_id: params[:id], school_student_params:, token: current_user.token
+      )
 
       if result.success?
         head :no_content

lib/concepts/school_student/update.rb

@@ -3,9 +3,9 @@
 module SchoolStudent
   class Update
     class << self
-      def call(school:, school_student_params:, token:)
+      def call(school:, student_id:, school_student_params:, token:)
         response = OperationResponse.new
-        update_student(school, school_student_params, token)
+        update_student(school, student_id, school_student_params, token)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -15,17 +15,16 @@ def call(school:, school_student_params:, token:)
 
       private
 
-      def update_student(school, school_student_params, token)
+      def update_student(school, student_id, school_student_params, token)
         username = school_student_params.fetch(:username, nil)
         password = school_student_params.fetch(:password, nil)
         name = school_student_params.fetch(:name, nil)
 
         validate(username:, password:, name:)
 
-        attributes_to_update = { username:, password:, name: }.compact
-        return if attributes_to_update.empty?
-
-        ProfileApiClient.update_school_student(token:, attributes_to_update:, organisation_id: school.id)
+        ProfileApiClient.update_school_student(
+          token:, school_id: school.id, student_id:, username:, password:, name:
+        )
       end
 
       def validate(username:, password:, name:)

lib/profile_api_client.rb

@@ -109,14 +109,25 @@ def create_school_student(token:, username:, password:, name:, school_id:)
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
 
-    def update_school_student(token:, attributes_to_update:, organisation_id:)
+    # rubocop:disable Metrics/AbcSize
+    def update_school_student(token:, school_id:, student_id:, name: nil, username: nil, password: nil) # rubocop:disable Metrics/ParameterLists
       return nil if token.blank?
 
-      _ = attributes_to_update
-      _ = organisation_id
+      response = connection(token).patch("/api/v1/schools/#{school_id}/students/#{student_id}") do |request|
+        request.body = {
+          name: name&.strip,
+          username: username&.strip,
+          password: password&.strip
+        }.compact
+      end
 
-      {}
+      raise UnexpectedResponse, response unless response.status == 200
+
+      Student.new(**response.body)
+    rescue Faraday::UnprocessableEntityError => e
+      raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
+    # rubocop:enable Metrics/AbcSize
 
     def delete_school_student(token:, student_id:, organisation_id:)
       return nil if token.blank?

spec/concepts/school_student/update_spec.rb

@@ -20,16 +20,16 @@
   end
 
   it 'returns a successful operation response' do
-    response = described_class.call(school:, school_student_params:, token:)
+    response = described_class.call(school:, student_id:, school_student_params:, token:)
     expect(response.success?).to be(true)
   end
 
   it 'makes a profile API call' do
-    described_class.call(school:, school_student_params:, token:)
+    described_class.call(school:, student_id:, school_student_params:, token:)
 
     # TODO: Replace with WebMock assertion once the profile API has been built.
     expect(ProfileApiClient).to have_received(:update_school_student)
-      .with(token:, attributes_to_update: school_student_params, organisation_id: school.id)
+      .with(token:, username: 'new-username', password: 'new-password', name: 'New Name', school_id: school.id, student_id:)
   end
 
   context 'when updating fails' do
@@ -46,22 +46,22 @@
     end
 
     it 'does not make a profile API request' do
-      described_class.call(school:, school_student_params:, token:)
+      described_class.call(school:, student_id:, school_student_params:, token:)
       expect(ProfileApiClient).not_to have_received(:update_school_student)
     end
 
     it 'returns a failed operation response' do
-      response = described_class.call(school:, school_student_params:, token:)
+      response = described_class.call(school:, student_id:, school_student_params:, token:)
       expect(response.failure?).to be(true)
     end
 
     it 'returns the error message in the operation response' do
-      response = described_class.call(school:, school_student_params:, token:)
+      response = described_class.call(school:, student_id:, school_student_params:, token:)
       expect(response[:error]).to match(/username ' ' is invalid/)
     end
 
     it 'sent the exception to Sentry' do
-      described_class.call(school:, school_student_params:, token:)
+      described_class.call(school:, student_id:, school_student_params:, token:)
       expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
     end
   end

spec/lib/profile_api_client_spec.rb

@@ -495,4 +495,111 @@ def list_school_students
       described_class.list_school_students(token:, school_id: school.id, student_ids:)
     end
   end
+
+  # rubocop:disable RSpec/MultipleMemoizedHelpers
+  describe '.update_school_student' do
+    let(:username) { 'username' }
+    let(:password) { 'password' }
+    let(:name) { 'name' }
+    let(:school) { build(:school, id: SecureRandom.uuid) }
+    let(:student) { create(:student, school:) }
+    let(:update_student_url) { "#{api_url}/api/v1/schools/#{school.id}/students/#{student.id}" }
+
+    before do
+      stub_request(:patch, update_student_url)
+        .to_return(
+          status: 200,
+          body: '{"id":"","schoolId":"","name":"","username":"","createdAt":"","updatedAt":"","discardedAt":""}',
+          headers: { 'content-type' => 'application/json' }
+        )
+    end
+
+    it 'makes a request to the profile api host' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url)
+    end
+
+    it 'includes token in the authorization request header' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url).with(headers: { authorization: "Bearer #{token}" })
+    end
+
+    it 'includes the profile api key in the x-api-key request header' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url).with(headers: { 'x-api-key' => api_key })
+    end
+
+    it 'sets content-type of request to json' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url).with(headers: { 'content-type' => 'application/json' })
+    end
+
+    it 'sets accept header to json' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url).with(headers: { 'accept' => 'application/json' })
+    end
+
+    it 'sends the student details in the request body' do
+      update_school_student
+      expect(WebMock).to have_requested(:patch, update_student_url).with(body: { name:, username:, password: }.to_json)
+    end
+
+    it 'returns the updated student if successful' do
+      response = { id: 'id', schoolId: 'school-id', name: 'new-name', username: 'new-username', createdAt: '', updatedAt: '', discardedAt: '' }
+      expected = ProfileApiClient::Student.new(**response)
+      stub_request(:patch, update_student_url)
+        .to_return(status: 200, body: response.to_json, headers: { 'content-type' => 'application/json' })
+      expect(update_school_student).to eq(expected)
+    end
+
+    it 'raises 422 exception if 422 status code is returned' do
+      response = { errors: [username: 'username', error: 'ERR_USER_EXISTS'] }
+      stub_request(:patch, update_student_url)
+        .to_return(status: 422, body: response.to_json, headers: { 'content-type' => 'application/json' })
+
+      expect { update_school_student }.to raise_error(ProfileApiClient::Student422Error)
+        .with_message("Student not saved in Profile API (status code 422, username 'username', error 'username has already been taken')")
+    end
+
+    it 'raises exception if anything other that 200 status code is returned' do
+      stub_request(:patch, update_student_url)
+        .to_return(status: 201)
+
+      expect { update_school_student }.to raise_error(ProfileApiClient::UnexpectedResponse)
+    end
+
+    it 'raises faraday exception for 4xx and 5xx responses' do
+      stub_request(:patch, update_student_url)
+        .to_return(status: 401)
+
+      expect { update_school_student }.to raise_error(Faraday::Error)
+    end
+
+    context 'when there are extraneous leading and trailing spaces in the student params' do
+      let(:username) { '  username  ' }
+      let(:password) { '  password  ' }
+      let(:name) { '  name  ' }
+
+      it 'strips the extraneous spaces' do
+        update_school_student
+        expect(WebMock).to have_requested(:patch, update_student_url).with(body: { name: 'name', username: 'username', password: 'password' }.to_json)
+      end
+    end
+
+    context 'when optional values are nil' do
+      let(:username) { nil }
+      let(:password) { nil }
+      let(:name) { nil }
+
+      it 'does not send empty values' do
+        update_school_student
+        expect(WebMock).to have_requested(:patch, update_student_url).with(body: {}.to_json)
+      end
+    end
+
+    def update_school_student
+      described_class.update_school_student(token:, username:, password:, name:, school_id: school.id, student_id: student.id)
+    end
+  end
+  # rubocop:enable RSpec/MultipleMemoizedHelpers
 end