WIP: Improve student creation

Author: chrisroos

lib/concepts/school_student/create.rb

@@ -9,7 +9,13 @@ def call(school:, school_student_params:, token:)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
-        response[:error] = "Error creating school student: #{e}"
+
+        response[:error] = "Error creating school student: "
+        if e.is_a?(ProfileApiClient::CreateStudentError) && e.duplicate_username?
+          response[:error] += "username has already been taken"
+        else
+          response[:error] += e.to_s
+        end
         response
       end
 

lib/profile_api_client.rb

@@ -6,6 +6,20 @@ class ProfileApiClient
     owner: 'school:owner'
   }.freeze
 
+  class Error < StandardError; end
+
+  class CreateStudentError < Error
+    def initialize(response)
+      @status_code = response.status
+      @body = response.body
+      super "Student not created in Profile API (status code #{@status_code})"
+    end
+
+    def duplicate_username?
+      (@body['errors']&.first || {})['error'] == 'ERR_USER_EXISTS'
+    end
+  end
+
   class << self
     # TODO: Replace with HTTP requests once the profile API has been built.
 
@@ -158,13 +172,7 @@ def create_school_student(token:, username:, password:, name:, school_id:)
         }]
       end
 
-      if response.status == 422
-        error_code = JSON.parse(response.body)['errors'].first['error']
-        message = error_code == 'ERR_USER_EXISTS' ? 'Username already exists' : "Unknown error code: #{error_code}"
-        raise "Student not created in Profile API (status code #{response.status}). #{message}"
-      end
-
-      raise "Student not created in Profile API (status code #{response.status})" unless response.status == 201
+      raise CreateStudentError.new(response) unless response.status == 201
 
       response.body.deep_symbolize_keys
     end

spec/concepts/school_student/create_spec.rb

@@ -84,4 +84,17 @@
       expect(response[:error]).to match(/school is not verified/)
     end
   end
+
+  context 'when the student cannot be created in profile api' do
+    let(:duplicate_username) { true }
+    let(:exception) { instance_double(ProfileApiClient::CreateStudentError, duplicate_username?: duplicate_username) }
+
+    before do
+      allow(ProfileApiClient).to receive(:create_school_student).and_raise(exception)
+    end
+
+    it 'adds a useful error message' do
+      # TODO
+    end
+  end
 end

spec/lib/profile_api_client_spec.rb

@@ -12,6 +12,34 @@
     allow(ENV).to receive(:fetch).with('PROFILE_API_KEY').and_return(api_key)
   end
 
+  describe ProfileApiClient::CreateStudentError do
+    subject { described_class.new(response) }
+
+    let(:status) { '404' }
+    let(:body) { '' }
+    let(:response) { instance_double(Faraday::Response, status:, body:) }
+
+    it 'includes status code in the message' do
+      expect(subject.message).to eq('Student not created in Profile API (status code 404)')
+    end
+
+    describe '#duplicate_username?' do
+      context 'when body includes duplicate username error code' do
+        let(:body) { { 'errors' => [{ 'username' => 'jdoe', 'error' => 'ERR_USER_EXISTS' }] } }
+
+        it 'returns true' do
+          expect(subject.duplicate_username?).to be(true)
+        end
+      end
+
+      context 'when body does not include duplicate username error code' do
+        it 'returns false' do
+          expect(subject.duplicate_username?).to be(false)
+        end
+      end
+    end
+  end
+
   describe '.create_school' do
     let(:school) { build(:school, id: SecureRandom.uuid, code: SecureRandom.uuid) }
     let(:create_school_url) { "#{api_url}/api/v1/schools" }
@@ -331,25 +359,11 @@ def delete_safeguarding_flag
       expect(create_school_student).to eq(response)
     end
 
-    it 'raises exception with details of the error if 422 response indicates that the user already exists' do
-      response = { 'errors' => [{ 'username' => 'jdoe', 'error' => 'ERR_USER_EXISTS' }] }
-      stub_request(:post, create_students_url)
-        .to_return(status: 422, body: response.to_json)
-      expect { create_school_student }.to raise_error('Student not created in Profile API (status code 422). Username already exists')
-    end
-
-    it 'raises exception including the error code if 422 response indicates that some other error occurred' do
-      response = { 'errors' => [{ 'username' => 'jdoe', 'error' => 'ERR_UNKNOWN' }] }
-      stub_request(:post, create_students_url)
-        .to_return(status: 422, body: response.to_json)
-      expect { create_school_student }.to raise_error('Student not created in Profile API (status code 422). Unknown error code: ERR_UNKNOWN')
-    end
-
     it 'raises exception if anything other than a 201 status code is returned' do
       stub_request(:post, create_students_url)
         .to_return(status: 200)
 
-      expect { create_school_student }.to raise_error(RuntimeError)
+      expect { create_school_student }.to raise_error(ProfileApiClient::CreateStudentError)
     end
 
     it 'includes details of underlying response when exception is raised' do