Create students in Profile API (#370)

Author: chrisroos

app/controllers/api/school_students_controller.rb

@@ -5,6 +5,7 @@ class SchoolStudentsController < ApiController
     before_action :authorize_user
     load_and_authorize_resource :school
     authorize_resource :school_student, class: false
+    before_action :create_safeguarding_flags
 
     def index
       result = SchoolStudent::List.call(school: @school, token: current_user.token)
@@ -62,5 +63,28 @@ def destroy
     def school_student_params
       params.require(:school_student).permit(:username, :password, :name)
     end
+
+    def create_safeguarding_flags
+      create_teacher_safeguarding_flag
+      create_owner_safeguarding_flag
+    end
+
+    def create_teacher_safeguarding_flag
+      return unless current_user.school_teacher?(@school)
+
+      ProfileApiClient.create_safeguarding_flag(
+        token: current_user.token,
+        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher]
+      )
+    end
+
+    def create_owner_safeguarding_flag
+      return unless current_user.school_owner?(@school)
+
+      ProfileApiClient.create_safeguarding_flag(
+        token: current_user.token,
+        flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner]
+      )
+    end
   end
 end

app/services/school_verification_service.rb

@@ -13,7 +13,7 @@ def verify(token:)
       school.verify!
       Role.owner.create!(user_id: school.creator_id, school:)
       Role.teacher.create!(user_id: school.creator_id, school:)
-      ProfileApiClient.create_school(token:, school:)
+      ProfileApiClient.create_school(token:, id: school.id, code: school.code)
     end
   rescue StandardError => e
     Sentry.capture_exception(e)

lib/concepts/school_student/create.rb

@@ -5,7 +5,11 @@ class Create
     class << self
       def call(school:, school_student_params:, token:)
         response = OperationResponse.new
-        create_student(school, school_student_params, token)
+        response[:student_id] = create_student(school, school_student_params, token)
+        response
+      rescue ProfileApiClient::CreateStudent422Error => e
+        Sentry.capture_exception(e)
+        response[:error] = "Error creating school student: #{e.error}"
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -16,14 +20,17 @@ def call(school:, school_student_params:, token:)
       private
 
       def create_student(school, school_student_params, token)
-        organisation_id = school.id
+        school_id = school.id
         username = school_student_params.fetch(:username)
         password = school_student_params.fetch(:password)
         name = school_student_params.fetch(:name)
 
         validate(school:, username:, password:, name:)
 
-        ProfileApiClient.create_school_student(token:, username:, password:, name:, organisation_id:)
+        response = ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id:)
+        user_id = response[:created].first
+        Role.student.create!(school:, user_id:)
+        user_id
       end
 
       def validate(school:, username:, password:, name:)

lib/concepts/school_student/create_batch.rb

@@ -23,7 +23,7 @@ def create_batch(school, uploaded_file, token)
         validate(school:, sheet:)
 
         non_header_rows_with_content(sheet:).each do |name, username, password|
-          ProfileApiClient.create_school_student(token:, username:, password:, name:, organisation_id: school.id)
+          ProfileApiClient.create_school_student(token:, username:, password:, name:, school_id: school.id)
         end
       end
 

lib/profile_api_client.rb

@@ -1,23 +1,48 @@
 # frozen_string_literal: true
 
 class ProfileApiClient
+  SAFEGUARDING_FLAGS = {
+    teacher: 'school:teacher',
+    owner: 'school:owner'
+  }.freeze
+
+  class Error < StandardError; end
+
+  class CreateStudent422Error < 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 created in Profile API (status code 422, username '#{@username}', error '#{@error}')"
+    end
+  end
+
   class << self
     # TODO: Replace with HTTP requests once the profile API has been built.
 
-    def create_school(token:, school:)
-      return { 'id' => school.id, 'schoolCode' => school.code } if ENV['BYPASS_OAUTH'].present?
+    def create_school(token:, id:, code:)
+      return { 'id' => id, 'schoolCode' => code } if ENV['BYPASS_OAUTH'].present?
 
-      response = connection.post('/api/v1/schools') do |request|
-        apply_default_headers(request, token)
+      response = connection(token).post('/api/v1/schools') do |request|
         request.body = {
-          id: school.id,
-          schoolCode: school.code
-        }.to_json
+          id:,
+          schoolCode: code
+        }
       end
 
-      raise "School not created in Profile API. HTTP response code: #{response.status}" unless response.status == 201
+      raise "School not created in Profile API (status code #{response.status})" unless response.status == 201
 
-      JSON.parse(response.body)
+      response.body
     end
 
     # The API should enforce these constraints:
@@ -142,19 +167,24 @@ def list_school_students(token:, organisation_id:)
     # The API should respond:
     # - 404 Not Found if the user doesn't exist
     # - 422 Unprocessable if the constraints are not met
-    def create_school_student(token:, username:, password:, name:, organisation_id:)
+    # rubocop:disable Metrics/AbcSize
+    def create_school_student(token:, username:, password:, name:, school_id:)
       return nil if token.blank?
 
-      _ = username
-      _ = password
-      _ = name
-      _ = organisation_id
+      response = connection(token).post("/api/v1/schools/#{school_id}/students") do |request|
+        request.body = [{
+          name: name.strip,
+          username: username.strip,
+          password: password.strip
+        }]
+      end
 
-      # TODO: We should make Faraday raise a Ruby error for a non-2xx status
-      # code so that SchoolStudent::Create propagates the error in the response.
-      response = {}
-      response.deep_symbolize_keys
+      raise CreateStudent422Error, response.body['errors'].first if response.status == 422
+      raise "Student not created in Profile API (status code #{response.status})" unless response.status == 201
+
+      response.body.deep_symbolize_keys
     end
+    # rubocop:enable Metrics/AbcSize
 
     # The API should enforce these constraints:
     # - The token has the school-owner or school-teacher role for the given organisation ID
@@ -198,18 +228,46 @@ def delete_school_student(token:, student_id:, organisation_id:)
       response.deep_symbolize_keys
     end
 
-    private
+    def safeguarding_flags(token:)
+      response = connection(token).get('/api/v1/safeguarding-flags')
 
-    def connection
-      Faraday.new(ENV.fetch('IDENTITY_URL'))
+      unless response.status == 200
+        raise "Safeguarding flags cannot be retrieved from Profile API (status code #{response.status})"
+      end
+
+      response.body.map(&:deep_symbolize_keys)
+    end
+
+    def create_safeguarding_flag(token:, flag:)
+      response = connection(token).post('/api/v1/safeguarding-flags') do |request|
+        request.body = { flag: }
+      end
+
+      return if response.status == 201 || response.status == 303
+
+      raise "Safeguarding flag not created in Profile API (status code #{response.status})"
     end
 
-    def apply_default_headers(request, token)
-      request.headers['Accept'] = 'application/json'
-      request.headers['Authorization'] = "Bearer #{token}"
-      request.headers['Content-Type'] = 'application/json'
-      request.headers['X-API-KEY'] = ENV.fetch('PROFILE_API_KEY')
-      request
+    def delete_safeguarding_flag(token:, flag:)
+      response = connection(token).delete("/api/v1/safeguarding-flags/#{flag}")
+
+      return if response.status == 204
+
+      raise "Safeguarding flag not deleted from Profile API (status code #{response.status})"
+    end
+
+    private
+
+    def connection(token)
+      Faraday.new(ENV.fetch('IDENTITY_URL')) do |faraday|
+        faraday.request :json
+        faraday.response :json
+        faraday.headers = {
+          'Accept' => 'application/json',
+          'Authorization' => "Bearer #{token}",
+          'X-API-KEY' => ENV.fetch('PROFILE_API_KEY')
+        }
+      end
     end
   end
 end

spec/concepts/school_student/create_batch_spec.rb

@@ -21,15 +21,15 @@
 
     # TODO: Replace with WebMock assertion once the profile API has been built.
     expect(ProfileApiClient).to have_received(:create_school_student)
-      .with(token:, username: 'jane123', password: 'secret123', name: 'Jane Doe', organisation_id: school.id)
+      .with(token:, username: 'jane123', password: 'secret123', name: 'Jane Doe', school_id: school.id)
   end
 
   it "makes a profile API call to create John Doe's account" do
     described_class.call(school:, uploaded_file: file, token:)
 
     # TODO: Replace with WebMock assertion once the profile API has been built.
     expect(ProfileApiClient).to have_received(:create_school_student)
-      .with(token:, username: 'john123', password: 'secret456', name: 'John Doe', organisation_id: school.id)
+      .with(token:, username: 'john123', password: 'secret456', name: 'John Doe', school_id: school.id)
   end
 
   context 'when an .xlsx file is provided' do

spec/concepts/school_student/create_spec.rb

@@ -5,6 +5,7 @@
 RSpec.describe SchoolStudent::Create, type: :unit do
   let(:token) { UserProfileMock::TOKEN }
   let(:school) { create(:verified_school) }
+  let(:user_id) { SecureRandom.uuid }
 
   let(:school_student_params) do
     {
@@ -15,7 +16,7 @@
   end
 
   before do
-    stub_profile_api_create_school_student
+    stub_profile_api_create_school_student(user_id:)
   end
 
   it 'returns a successful operation response' do
@@ -28,13 +29,23 @@
 
     # TODO: Replace with WebMock assertion once the profile API has been built.
     expect(ProfileApiClient).to have_received(:create_school_student)
-      .with(token:, username: 'student-to-create', password: 'at-least-8-characters', name: 'School Student', organisation_id: school.id)
+      .with(token:, username: 'student-to-create', password: 'at-least-8-characters', name: 'School Student', school_id: school.id)
+  end
+
+  it 'creates a role associating the student with the school' do
+    described_class.call(school:, school_student_params:, token:)
+    expect(Role.student.where(school:, user_id:)).to exist
+  end
+
+  it 'returns the ID of the student created in Profile API' do
+    response = described_class.call(school:, school_student_params:, token:)
+    expect(response[:student_id]).to eq(user_id)
   end
 
   context 'when creation fails' do
     let(:school_student_params) do
       {
-        username: ' ',
+        username: '',
         password: 'at-least-8-characters',
         name: 'School Student'
       }
@@ -56,7 +67,7 @@
 
     it 'returns the error message in the operation response' do
       response = described_class.call(school:, school_student_params:, token:)
-      expect(response[:error]).to match(/username ' ' is invalid/)
+      expect(response[:error]).to match(/username '' is invalid/)
     end
 
     it 'sent the exception to Sentry' do
@@ -73,4 +84,30 @@
       expect(response[:error]).to match(/school is not verified/)
     end
   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(:exception) { ProfileApiClient::CreateStudent422Error.new(error) }
+
+    before do
+      allow(ProfileApiClient).to receive(:create_school_student).and_raise(exception)
+    end
+
+    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')
+    end
+  end
+
+  context 'when the student cannot be created in profile api because of a response other than 422' do
+    before do
+      allow(ProfileApiClient).to receive(:create_school_student)
+        .and_raise('Student not created in Profile API (status code 401)')
+    end
+
+    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: Student not created in Profile API (status code 401)')
+    end
+  end
 end

spec/features/school_student/creating_a_batch_of_school_students_spec.rb

@@ -6,6 +6,7 @@
   before do
     authenticated_in_hydra_as(owner)
     stub_profile_api_create_school_student
+    stub_profile_api_create_safeguarding_flag
   end
 
   let(:headers) { { Authorization: UserProfileMock::TOKEN } }
@@ -15,6 +16,16 @@
 
   let(:file) { fixture_file_upload('students.csv') }
 
+  it 'creates the school owner safeguarding flag' do
+    post("/api/schools/#{school.id}/students/batch", headers:, params: { file: })
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner])
+  end
+
+  it 'does not create the school teacher safeguarding flag' do
+    post("/api/schools/#{school.id}/students/batch", headers:, params: { file: })
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher])
+  end
+
   it 'responds 204 No Content' do
     post("/api/schools/#{school.id}/students/batch", headers:, params: { file: })
     expect(response).to have_http_status(:no_content)
@@ -28,6 +39,22 @@
     expect(response).to have_http_status(:no_content)
   end
 
+  it 'does not create the school owner safeguarding flag when the user is a school-teacher' do
+    teacher = create(:teacher, school:)
+    authenticated_in_hydra_as(teacher)
+
+    post("/api/schools/#{school.id}/students/batch", headers:, params: { file: })
+    expect(ProfileApiClient).not_to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:owner])
+  end
+
+  it 'creates the school teacher safeguarding flag when the user is a school-teacher' do
+    teacher = create(:teacher, school:)
+    authenticated_in_hydra_as(teacher)
+
+    post("/api/schools/#{school.id}/students/batch", headers:, params: { file: })
+    expect(ProfileApiClient).to have_received(:create_safeguarding_flag).with(token: UserProfileMock::TOKEN, flag: ProfileApiClient::SAFEGUARDING_FLAGS[:teacher])
+  end
+
   it 'responds 422 Unprocessable Entity when params are invalid' do
     post("/api/schools/#{school.id}/students/batch", headers:, params: {})
     expect(response).to have_http_status(:unprocessable_entity)