Skip to content

Commit

Permalink
Student create/update validation handling (#435)
Browse files Browse the repository at this point in the history
closes #434

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Lois Wells <[email protected]>
Co-authored-by: Scott Adams <[email protected]>
  • Loading branch information
3 people authored Sep 16, 2024
1 parent d272788 commit dee5721
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 41 deletions.
4 changes: 0 additions & 4 deletions lib/concepts/school_student/create.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ def call(school:, school_student_params:, token:)
response = OperationResponse.new
response[:student_id] = create_student(school, school_student_params, token)
response
rescue ProfileApiClient::Student422Error => e
Sentry.capture_exception(e)
response[:error] = "Error creating school student: #{e.error}"
response
rescue StandardError => e
Sentry.capture_exception(e)
response[:error] = "Error creating school student: #{e}"
Expand Down
20 changes: 4 additions & 16 deletions lib/profile_api_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,9 @@ class ProfileApiClient
class Error < StandardError; end

class Student422Error < Error
DEFAULT_ERROR = 'unknown error'
ERRORS = {
'ERR_USER_EXISTS' => 'username has already been taken',
'ERR_INVALID' => 'unknown validation error',
'ERR_INVALID_PASSWORD' => 'password is invalid',
'ERR_UNKNOWN' => DEFAULT_ERROR
}.freeze

attr_reader :username, :error

def initialize(error)
@username = error['username']
@error = ERRORS.fetch(error['error'], DEFAULT_ERROR)

super "Student not saved in Profile API (status code 422, username '#{@username}', error '#{@error}')"
@message = error['message']
super @message
end
end

Expand Down Expand Up @@ -109,7 +97,7 @@ def create_school_student(token:, username:, password:, name:, school_id:)
raise UnexpectedResponse, response unless response.status == 201

response.body.deep_symbolize_keys
rescue Faraday::UnprocessableEntityError => e
rescue Faraday::BadRequestError => e
raise Student422Error, JSON.parse(e.response_body)['errors'].first
end

Expand All @@ -127,7 +115,7 @@ def update_school_student(token:, school_id:, student_id:, name: nil, username:
raise UnexpectedResponse, response unless response.status == 200

Student.new(**response.body)
rescue Faraday::UnprocessableEntityError => e
rescue Faraday::BadRequestError => e
raise Student422Error, JSON.parse(e.response_body)['errors'].first
end

Expand Down
4 changes: 2 additions & 2 deletions spec/concepts/school_student/create_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@
end

context 'when the student cannot be created in profile api because of a 422 response' do
let(:error) { { 'username' => 'username', 'error' => 'ERR_USER_EXISTS' } }
let(:error) { { 'message' => "something's up with the username" } }
let(:exception) { ProfileApiClient::Student422Error.new(error) }

before do
Expand All @@ -95,7 +95,7 @@

it 'adds a useful error message' do
response = described_class.call(school:, school_student_params:, token:)
expect(response[:error]).to eq('Error creating school student: username has already been taken')
expect(response[:error]).to eq("Error creating school student: something's up with the username")
end
end

Expand Down
30 changes: 11 additions & 19 deletions spec/lib/profile_api_client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,10 @@
subject(:exception) { described_class.new(error) }

let(:error_code) { 'ERR_USER_EXISTS' }
let(:error) { { 'username' => 'username', 'error' => error_code } }
let(:error) { { 'message' => "Something's wrong with the password" } }

it 'includes status code, username and translated error code in the message' do
expect(exception.message).to eq("Student not saved in Profile API (status code 422, username 'username', error 'username has already been taken')")
end

context "when the error isn't recognised" do
let(:error_code) { 'unrecognised-code' }

it 'includes a default error message' do
expect(exception.message).to match(/error 'unknown error'/)
end
it 'includes the message from the error' do
expect(exception.message).to eq("Something's wrong with the password")
end
end

Expand Down Expand Up @@ -379,13 +371,13 @@ def delete_safeguarding_flag
expect(create_school_student).to eq(response)
end

it 'raises 422 exception if 422 status code is returned' do
response = { errors: [username: 'username', error: 'ERR_USER_EXISTS'] }
it 'raises 422 exception with the relevant message if 400 status code is returned' do
response = { errors: [message: 'The password is well dodgy'] }
stub_request(:post, create_students_url)
.to_return(status: 422, body: response.to_json, headers: { 'content-type' => 'application/json' })
.to_return(status: 400, body: response.to_json, headers: { 'content-type' => 'application/json' })

expect { create_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')")
.with_message('The password is well dodgy')
end

it 'raises exception if anything other that 201 status code is returned' do
Expand Down Expand Up @@ -548,13 +540,13 @@ def list_school_students
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'] }
it 'raises 422 exception with the relevant message if 400 status code is returned' do
response = { errors: [message: 'The username is well dodgy'] }
stub_request(:patch, update_student_url)
.to_return(status: 422, body: response.to_json, headers: { 'content-type' => 'application/json' })
.to_return(status: 400, 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')")
.with_message('The username is well dodgy')
end

it 'raises exception if anything other that 200 status code is returned' do
Expand Down

0 comments on commit dee5721

Please sign in to comment.