Raise custom error for missing organisations

app/controllers/api/application_controller.rb

@@ -9,16 +9,4 @@ def set_headers
   def verify_json_request
     not_found unless request.format.json?
   end
-
-  def render_server_error(exception)
-    render json: { error: "Internal server error", message: exception.message }, status: :internal_server_error
-  end
-
-  def render_not_found
-    render json: { error: "The given ID does not match any vacancy for your ATS" }, status: :not_found
-  end
-
-  def render_bad_request
-    render json: { error: "Request body could not be read properly" }, status: :bad_request
-  end
 end

app/controllers/publishers/ats_api/v1/create_vacancy_service.rb

@@ -2,6 +2,9 @@ module Publishers
   module AtsApi
     module V1
       class CreateVacancyService
+
+        InvalidOrganisationError = Class.new(StandardError)
+
         def initialize(params)
           @params = params
         end
@@ -26,7 +29,7 @@ def call
 
         def permitted_params
           organisations = fetch_organisations(params[:schools])
-          raise ActiveRecord::RecordNotFound, "No valid organisations found" if organisations.blank?
+          raise InvalidOrganisationError, "No valid organisations found" if organisations.blank?
 
           params[:publish_on] ||= Time.zone.today.to_s
           params[:working_patterns] ||= []
@@ -39,7 +42,7 @@ def fetch_organisations(school_params)
           return [] unless school_params
 
           if school_params[:trust_uid].present?
-            SchoolGroup.trusts.find_by(uid: school_params[:trust_uid]).schools.where(urn: school_params[:school_urns])
+            SchoolGroup.trusts.find_by(uid: school_params[:trust_uid]).organisation_urns.where(urn: school_params[:school_urns])
           else
             School.where(urn: school_params[:school_urns])
           end

app/controllers/publishers/ats_api/v1/update_vacancy_service.rb

@@ -23,10 +23,6 @@ def permitted_params
           organisations = fetch_organisations(params[:schools])
           raise ActiveRecord::RecordNotFound, "No valid organisations found" if organisations.blank?
 
-          params[:publish_on] ||= Time.zone.today.to_s
-          params[:working_patterns] ||= []
-          params[:phases] ||= []
-
           params.except(:schools, :trust_uid).merge(organisations: organisations)
         end
 

app/controllers/publishers/ats_api/v1/vacancies_controller.rb

@@ -4,6 +4,7 @@ class Publishers::AtsApi::V1::VacanciesController < Api::ApplicationController
   rescue_from StandardError, with: :render_server_error
   rescue_from ActiveRecord::RecordNotFound, with: :render_not_found
   rescue_from ActionController::ParameterMissing, with: :render_bad_request
+  rescue_from Publishers::AtsApi::V1::CreateVacancyService::InvalidOrganisationError, with: :render_bad_request
 
   def index
     @pagy, @vacancies = pagy(vacancies.where(publisher_ats_api_client: client), items: 100)
@@ -67,9 +68,11 @@ def permitted_vacancy_params
     missing_keys = required_vacancy_keys - params.fetch(:vacancy, {}).keys.map(&:to_sym)
     raise ActionController::ParameterMissing, "Missing required parameters: #{missing_keys.join(', ')}" if missing_keys.any?
 
-    params.fetch(:vacancy).permit(:external_advert_url, :external_reference, :visa_sponsorship_available, :is_job_share,
-                                  :expires_at, :job_title, :skills_and_experience, :is_parental_leave_cover, :salary, :job_advert, :contract_type,
-                                  job_roles: [], working_patterns: [], phases: [], schools: [:trust_uid, { school_urns: [] }])
+    params.fetch(:vacancy)
+          .permit(:external_advert_url, :external_reference, :visa_sponsorship_available, :is_job_share,
+                  :expires_at, :job_title, :skills_and_experience, :is_parental_leave_cover, :salary, :job_advert, :contract_type,
+                  job_roles: [], working_patterns: [], phases: [], schools: [:trust_uid, { school_urns: [] }])
+          .merge(publisher_ats_api_client_id: client.id)
   end
 
   def vacancies
@@ -91,4 +94,16 @@ def authenticate_client!
            json: { error: "Invalid API key" },
            content_type: "application/json"
   end
+
+  def render_server_error(exception)
+    render json: { error: "Internal server error", message: exception.message }, status: :internal_server_error
+  end
+
+  def render_not_found
+    render json: { error: "The given ID does not match any vacancy for your ATS" }, status: :not_found
+  end
+
+  def render_bad_request(exception = nil)
+    render json: { error: exception&.message.presence || "Request body could not be read properly" }, status: :bad_request
+  end
 end

app/models/vacancy.rb

@@ -129,7 +129,7 @@ def self.array_enums
   end
 
   def external?
-    external_source.present? || external_reference.present? || external_advert_url.present?
+    external_source.present? || publisher_ats_api_client.present?
   end
 
   def organisation
@@ -138,7 +138,7 @@ def organisation
     organisations.find(&:trust?) || publisher_organisation || organisations.first&.school_groups&.first
   end
 
-  def schools
+  def organisation_urns
     organisations.filter_map do |organisation|
       if organisation.is_a?(School)
         { school_urns: [organisation.urn] }

config/analytics_blocklist.yml

@@ -95,7 +95,6 @@
   - in_progress_steps
   - employment_history_section_completed
   - qualifications_section_completed
-  - following_religion
   - religious_reference_type
   - faith
   - place_of_worship

spec/factories/vacancies.rb

@@ -208,6 +208,7 @@
       how_to_apply { Faker::Lorem.paragraph(sentence_count: 4) }
       job_advert { Faker::Lorem.paragraph(sentence_count: factory_rand(50..300)) }
       personal_statement_guidance { Faker::Lorem.paragraph(sentence_count: factory_rand(5..10)) }
+      publisher_ats_api_client
       external_source { "may_the_feed_be_with_you" }
       external_reference { "J3D1" }
       external_advert_url { "https://example.com/jobs/123" }

spec/jobs/import_from_vacancy_source_job_spec.rb

@@ -101,7 +101,7 @@ def each(...)
         expect(FailedImportedVacancy.first.external_reference).to eq("J3D1")
         expect(FailedImportedVacancy.first.import_errors.first).to eq("job_title:[can't be blank]")
         expect(FailedImportedVacancy.first.import_errors.last).to eq("phases:[can't be blank]")
-        expect(FailedImportedVacancy.first.vacancy).to include(
+        expect(FailedImportedVacancy.first.vacancy).to eq(
           "about_school" => "test",
           "actual_salary" => "",
           "application_email" => nil,
@@ -151,6 +151,7 @@ def each(...)
           "phases" => [],
           "publish_on" => Date.today.strftime("%Y-%m-%d"),
           "publisher_id" => nil,
+          "publisher_ats_api_client_id" => nil,
           "publisher_organisation_id" => nil,
           "readable_job_location" => nil,
           "readable_phases" => [],

spec/models/vacancy_spec.rb

@@ -551,14 +551,10 @@
       it { is_expected.to be true }
     end
 
-    context "when external_reference is present" do
-      before { vacancy.external_reference = "some_reference" }
+    context "when ats api client id is present" do
+      let(:publisher_api_client) { build(:publisher_ats_api_client) }
 
-      it { is_expected.to be true }
-    end
-
-    context "when external_advert_url is present" do
-      before { vacancy.external_advert_url = "https://example.com" }
+      before { vacancy.publisher_ats_api_client = publisher_api_client }
 
       it { is_expected.to be true }
     end
@@ -568,15 +564,15 @@
     end
   end
 
-  describe "#schools" do
+  describe "#organisation_urns" do
     let(:vacancy) { create(:vacancy, organisations: organisations) }
 
     context "when the organisation is a school" do
       let(:school) { create(:school, urn: "100001") }
       let(:organisations) { [school] }
 
       it "returns an array with school_urns" do
-        expect(vacancy.schools).to eq([{ school_urns: ["100001"] }])
+        expect(vacancy.organisation_urns).to eq([{ school_urns: ["100001"] }])
       end
     end
 
@@ -590,7 +586,7 @@
       end
 
       it "returns an array with trust_uid and school_urns" do
-        expect(vacancy.schools.sort).to eq([{ trust_uid: "12345", school_urns: %w[100002 100003].sort }])
+        expect(vacancy.organisation_urns.sort).to eq([{ trust_uid: "12345", school_urns: %w[100002 100003].sort }])
       end
     end
   end

spec/requests/publishers/ats_api/v1/create_vacancy_service_spec.rb

@@ -4,6 +4,7 @@
   subject(:service) { described_class.new(params) }
 
   let(:school) { create(:school) }
+  let(:publisher_ats_api_client_id) { create(:publisher_ats_api_client).id }
   let(:external_reference) { "new-ref" }
   let(:school_urns) { { school_urns: [school.urn] } }
   let(:job_title) { "A job title" }
@@ -24,6 +25,7 @@
       skills_and_experience: "Expert in teaching",
       salary: "£30,000 - £40,000",
       schools: school_urns,
+      publisher_ats_api_client_id: publisher_ats_api_client_id,
     }
   end
 
@@ -61,7 +63,11 @@
       let(:school_urns) { { school_urns: [9999] } }
 
       it "raises ActiveRecord::RecordNotFound" do
-        expect { service.call }.to raise_error(ActiveRecord::RecordNotFound, "No valid organisations found")
+        expect { service.call }
+          .to raise_error(
+            Publishers::AtsApi::V1::CreateVacancyService::InvalidOrganisationError,
+            "No valid organisations found",
+          )
       end
     end
 

spec/requests/publishers/ats_api/v1/vacancies_spec.rb

@@ -85,8 +85,8 @@
     it "only returns vacancies for the authenticated client" do
       other_client = create(:publisher_ats_api_client)
       school = create(:school)
-      create_list(:vacancy, 2, publisher_ats_api_client: client, organisations: [school])
-      create_list(:vacancy, 3, publisher_ats_api_client: other_client, organisations: [school])
+      create_list(:vacancy, 2, :external, publisher_ats_api_client: client, organisations: [school])
+      create_list(:vacancy, 3, :external, publisher_ats_api_client: other_client, organisations: [school])
 
       get "/ats-api/v1/vacancies", headers: { "X-Api-Key" => client.api_key, "Accept" => "application/json" }
 
@@ -96,7 +96,7 @@
     end
 
     it "returns paginated results" do
-      create_list(:vacancy, 10, publisher_ats_api_client: client)
+      create_list(:vacancy, 10, :external, publisher_ats_api_client: client)
 
       get "/ats-api/v1/vacancies", headers: { "X-Api-Key" => client.api_key, "Accept" => "application/json" }
 
@@ -297,6 +297,7 @@
               salary: source.salary,
               visa_sponsorship_available: source.visa_sponsorship_available,
               external_reference: source.external_reference,
+              publisher_ats_api_client_id: client.id,
               is_job_share: source.is_job_share,
               job_roles: source.job_roles,
               working_patterns: source.working_patterns,

spec/swagger_helper.rb

@@ -64,11 +64,11 @@
               job_title: { type: :string, example: "Teacher of Geography" },
               skills_and_experience: { type: :string, example: "We're looking for a dedicated Teacher of Geography" },
               salary: { type: :string, example: "£12,345 to £67, 890" },
-              benefits_details: { type: :string, example: "TLR2a", nullable: true },
-              starts_on: { type: :string, example: "Summer Term", nullable: true },
-              external_reference: { type: :string, example: "123GTZY", nullable: true },
-              visa_sponsorship_available: { type: :boolean, nullable: true },
-              is_job_share: { type: :boolean, nullable: true },
+              benefits_details: { type: :string, example: "TLR2a" },
+              starts_on: { type: :string, example: "Summer Term" },
+              external_reference: { type: :string, example: "123GTZY" },
+              visa_sponsorship_available: { type: :boolean },
+              is_job_share: { type: :boolean },
               schools: {
                 oneOf: [
                   {
@@ -277,5 +277,5 @@
   # the key, this may want to be changed to avoid putting yaml in json files.
   # Defaults to json. Accepts ':json' and ':yaml'.
   config.openapi_format = :yaml
-  config.openapi_no_additional_properties = false # Allow additional properties
+  # config.openapi_no_additional_properties = false # Allow additional properties
 end