diff --git a/.rubocop.yml b/.rubocop.yml index 9e23adcb..10e624cf 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -55,6 +55,10 @@ Rails/HasManyOrHasOneDependent: Exclude: - 'app/models/application.rb' +Rails/UniqueValidationWithoutIndex: + Exclude: + - 'app/models/application.rb' + Performance/MethodObjectAsBlock: Exclude: - 'app/models/summary.rb' diff --git a/app/models/application.rb b/app/models/application.rb index 79e54f6e..01ef7e2c 100644 --- a/app/models/application.rb +++ b/app/models/application.rb @@ -67,4 +67,5 @@ def mark_as_qa! validates(:subject, presence: true) validates(:visa_type, presence: true) validates(:applicant, presence: true) + validates(:urn, presence: true, uniqueness: true) end diff --git a/app/models/urn.rb b/app/models/urn.rb index 56330c1c..9a56dfd6 100644 --- a/app/models/urn.rb +++ b/app/models/urn.rb @@ -11,19 +11,40 @@ # Urn.generate('salaried_trainee') # => "IRPST12345" # class Urn + class GenerationError < StandardError; end + + CHARSET = %w[0 1 2 3 4 5 6 7 8 9].freeze + PREFIX = "IRP" + LENGTH = 5 + + MAX_URN_SET_PER_ROUTE = ("9" * LENGTH).to_i + INVALID_SUFFIX = "0" * LENGTH + + private_constant :CHARSET, :PREFIX, :LENGTH + attr_reader :value attr_writer :suffix + def self.next_available(route, used_urns) + raise(GenerationError, "No more urn available for route #{route}") if used_urns.size >= MAX_URN_SET_PER_ROUTE + + urn = nil + + loop do + urn = generate(route) + + next if urn.include?(INVALID_SUFFIX) + break unless used_urns.include?(urn) + end + + urn + end + def self.generate(applicant_type) code = applicant_type_code(applicant_type) PREFIX + code + Array.new(LENGTH) { CHARSET.sample }.join end - CHARSET = %w[0 1 2 3 4 5 6 7 8 9].freeze - PREFIX = "IRP" - LENGTH = 5 - private_constant :CHARSET, :PREFIX, :LENGTH - def self.applicant_type_code(applicant_type) case applicant_type when "teacher" diff --git a/app/services/submit_form.rb b/app/services/submit_form.rb index 939f2756..5cbd5a15 100644 --- a/app/services/submit_form.rb +++ b/app/services/submit_form.rb @@ -11,8 +11,9 @@ def initialize(form, ip_address) @form = form @ip_address = ip_address @success = false + @counter = 0 end - attr_reader :form, :ip_address, :application + attr_reader :form, :ip_address, :application, :counter delegate :errors, to: :form @@ -30,6 +31,9 @@ def submit_form! create_application_records send_applicant_email @success = true + rescue StandardError => e + Sentry.capture_exception(e) + errors.add(:base, :technical_error, message: "We are unable to submit your application at the moment. Please try again later") end private @@ -75,20 +79,20 @@ def create_applicant city: form.city, postcode: form.postcode, }, - school: @school, + school: school, ) end def create_application @application = Application.create!( - applicant: @applicant, + applicant: applicant, application_date: Date.current.to_s, application_route: form.application_route, application_progress: ApplicationProgress.new, date_of_entry: form.date_of_entry, start_date: form.start_date, subject: SubjectStep.new(form).answer.formatted_value, - urn: Urn.generate(form.application_route), + urn: Urn.next_available(form.application_route, used_urns), visa_type: form.visa_type, ) end @@ -100,10 +104,18 @@ def delete_form def send_applicant_email GovukNotifyMailer .with( - email: @applicant.email_address, + email: applicant.email_address, urn: application.urn, ) .application_submission .deliver_later end + + def used_urns + Application + .where(application_route: form.application_route) + .pluck(:urn) + end + + attr_reader :school, :applicant end diff --git a/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb b/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb new file mode 100644 index 00000000..7a308142 --- /dev/null +++ b/db/migrate/20230919103917_add_uniqueness_to_application_urn.rb @@ -0,0 +1,9 @@ +class AddUniquenessToApplicationUrn < ActiveRecord::Migration[7.0] + def up + drop_view :duplicate_applications + add_index(:applications, :urn) + create_view :duplicate_applications, version: 3 + end + + def down; end +end diff --git a/db/schema.rb b/db/schema.rb index 2873ecf2..b9037b41 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2023_09_15_100841) do +ActiveRecord::Schema[7.0].define(version: 2023_09_19_103917) do # These are extensions that must be enabled in order to support this database enable_extension "citext" enable_extension "plpgsql" @@ -84,6 +84,7 @@ t.datetime "standing_data_csv_downloaded_at" t.datetime "payroll_csv_downloaded_at" t.index ["applicant_id"], name: "index_applications_on_applicant_id" + t.index ["urn"], name: "index_applications_on_urn" end create_table "audits", force: :cascade do |t| diff --git a/spec/models/application_spec.rb b/spec/models/application_spec.rb index 5ed916b0..032333a4 100644 --- a/spec/models/application_spec.rb +++ b/spec/models/application_spec.rb @@ -35,6 +35,8 @@ it { expect(application).to validate_presence_of(:subject) } it { expect(application).to validate_presence_of(:visa_type) } it { expect(application).to validate_presence_of(:applicant) } + it { expect(application).to validate_presence_of(:urn) } + it { expect(application).to validate_uniqueness_of(:urn) } end end diff --git a/spec/models/urn_spec.rb b/spec/models/urn_spec.rb index 4df75ded..fb2316b2 100644 --- a/spec/models/urn_spec.rb +++ b/spec/models/urn_spec.rb @@ -3,9 +3,9 @@ require "rails_helper" RSpec.describe Urn do - subject(:urn) { described_class.generate(applicant_type) } - describe ".generate" do + subject(:urn) { described_class.generate(applicant_type) } + context 'when applicant type is "teacher"' do let(:applicant_type) { "teacher" } @@ -36,4 +36,34 @@ end end end + + describe ".next_available" do + subject(:urn) { described_class.next_available(application_route, used_urns) } + + let(:application_route) { "teacher" } + let(:used_urns) { %w[IRPTE45632 IRPTE74125] } + + context "when all urns possible have been used" do + let(:used_urns) { Array.new(Urn::MAX_URN_SET_PER_ROUTE) } + + it "raises an GenerationError" do + expect { urn }.to raise_error(Urn::GenerationError, "No more urn available for route #{application_route}") + end + end + + context "never returns an urn with only 0 in suffix" do + before do + allow(described_class).to receive(:generate).and_return(invalid_urn, valid_urn) + end + + let(:invalid_urn) { "IRPTE00000" } + let(:valid_urn) { "IRPTE22222" } + + it { expect(urn).to eq(valid_urn) } + end + + context "returns an unused urn" do + it { expect(urn).to include("IRPTE") } + end + end end diff --git a/spec/services/submit_form_spec.rb b/spec/services/submit_form_spec.rb index 3c110afe..76cd12f9 100644 --- a/spec/services/submit_form_spec.rb +++ b/spec/services/submit_form_spec.rb @@ -133,7 +133,7 @@ context "applicant email" do before do - allow(Urn).to receive(:generate).and_return(urn) + allow(Urn).to receive(:next_available).and_return(urn) end let(:urn) { "SOMEURN" }