diff --git a/app/controllers/api/application_controller.rb b/app/controllers/api/application_controller.rb index abccefbaaf..56d157f5b4 100644 --- a/app/controllers/api/application_controller.rb +++ b/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 diff --git a/app/controllers/publishers/ats_api/v1/create_vacancy_service.rb b/app/controllers/publishers/ats_api/v1/create_vacancy_service.rb index 7bed0a4785..4fc0343412 100644 --- a/app/controllers/publishers/ats_api/v1/create_vacancy_service.rb +++ b/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 diff --git a/app/controllers/publishers/ats_api/v1/update_vacancy_service.rb b/app/controllers/publishers/ats_api/v1/update_vacancy_service.rb index 6a4f0a99f3..03418ef7fb 100644 --- a/app/controllers/publishers/ats_api/v1/update_vacancy_service.rb +++ b/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 diff --git a/app/controllers/publishers/ats_api/v1/vacancies_controller.rb b/app/controllers/publishers/ats_api/v1/vacancies_controller.rb index f2b06de95d..8e0f29508e 100644 --- a/app/controllers/publishers/ats_api/v1/vacancies_controller.rb +++ b/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 diff --git a/app/models/vacancy.rb b/app/models/vacancy.rb index 8e9d96c1b1..81004652a3 100644 --- a/app/models/vacancy.rb +++ b/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] } diff --git a/config/analytics_blocklist.yml b/config/analytics_blocklist.yml index 260f846943..242b0d78ff 100644 --- a/config/analytics_blocklist.yml +++ b/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 diff --git a/spec/factories/vacancies.rb b/spec/factories/vacancies.rb index 2b59415a53..9d3a450b71 100644 --- a/spec/factories/vacancies.rb +++ b/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" } diff --git a/spec/jobs/import_from_vacancy_source_job_spec.rb b/spec/jobs/import_from_vacancy_source_job_spec.rb index 6469c72042..c538ff253b 100644 --- a/spec/jobs/import_from_vacancy_source_job_spec.rb +++ b/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" => [], diff --git a/spec/models/vacancy_spec.rb b/spec/models/vacancy_spec.rb index e67364a8be..c47197cdc6 100644 --- a/spec/models/vacancy_spec.rb +++ b/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,7 +564,7 @@ end end - describe "#schools" do + describe "#organisation_urns" do let(:vacancy) { create(:vacancy, organisations: organisations) } context "when the organisation is a school" do @@ -576,7 +572,7 @@ 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 diff --git a/spec/requests/publishers/ats_api/v1/create_vacancy_service_spec.rb b/spec/requests/publishers/ats_api/v1/create_vacancy_service_spec.rb index f0b791d84f..5365c5dab5 100644 --- a/spec/requests/publishers/ats_api/v1/create_vacancy_service_spec.rb +++ b/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 diff --git a/spec/requests/publishers/ats_api/v1/vacancies_spec.rb b/spec/requests/publishers/ats_api/v1/vacancies_spec.rb index e10520db07..9f54360bb0 100644 --- a/spec/requests/publishers/ats_api/v1/vacancies_spec.rb +++ b/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, diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index a23a6bd283..d0f2727123 100644 --- a/spec/swagger_helper.rb +++ b/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