From ab7380b3672a02f48ec9e755be5e9102a7f508e7 Mon Sep 17 00:00:00 2001
From: JR <139925286+fumimowdan@users.noreply.github.com>
Date: Wed, 20 Sep 2023 11:15:43 +0100
Subject: [PATCH 1/3] Revert "Revert "Removed unused methods""

This reverts commit d88a1d1b2a9b3cbdb0b76e08527cdc1ee47fcc7d.
---
 app/controllers/application_controller.rb | 31 -----------------------
 1 file changed, 31 deletions(-)

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 848035d1..ec58aac3 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -5,37 +5,6 @@ class ApplicationController < ActionController::Base
 
   before_action :check_service_open!
 
-  delegate :application_route, to: :current_application
-  helper_method :application_route
-
-  def check_application!
-    return if session["application_id"]
-
-    redirect_to(root_path)
-  end
-
-  def current_application
-    Application.in_progress.find(session["application_id"])
-  end
-  helper_method :current_application
-
-  def current_applicant
-    current_application.applicant
-  end
-  helper_method :current_applicant
-
-  def check_teacher!
-    return unless application_route != "teacher"
-
-    redirect_to(new_applicants_application_route_path)
-  end
-
-  def check_trainee!
-    return unless application_route != "salaried_trainee"
-
-    redirect_to(new_applicants_application_route_path)
-  end
-
   def check_service_open!
     return if request.path == destroy_user_session_path # skip this for log out page
     return if Gatekeeper.application_open?

From 0846d59139ca750c34474520b1a50f22339ed0a4 Mon Sep 17 00:00:00 2001
From: JR <139925286+fumimowdan@users.noreply.github.com>
Date: Wed, 20 Sep 2023 11:15:43 +0100
Subject: [PATCH 2/3] Revert "Revert "Remove concept of submitted application""

This reverts commit 3a3b38b06302a8e1fd37bcedde0585b31b0cf7f4.
---
 .../system_admin/applicants_controller.rb     |  2 +-
 app/models/application.rb                     | 29 ++++----------
 app/models/kpis.rb                            |  2 +-
 app/queries/average_age_query.rb              |  2 +-
 app/queries/gender_breakdown_query.rb         |  2 +-
 app/queries/nationality_breakdown_query.rb    |  2 +-
 .../rejection_reason_breakdown_query.rb       |  2 +-
 app/queries/route_breakdown_query.rb          |  2 +-
 app/queries/subject_breakdown_query.rb        |  2 +-
 app/queries/visa_breakdown_query.rb           |  2 +-
 spec/factories/applications.rb                | 10 +----
 .../admin_console/applications_list_spec.rb   |  6 +--
 .../download_qa_csv_file_spec.rb              |  2 +-
 .../admin_console/duplicates_search_spec.rb   | 30 ++------------
 spec/models/application_spec.rb               | 39 +------------------
 spec/models/kpis_spec.rb                      |  1 -
 spec/queries/average_age_query_spec.rb        |  2 -
 spec/queries/gender_breakdown_query_spec.rb   |  2 -
 .../nationality_breakdown_query_spec.rb       |  2 -
 .../rejection_reason_breakdown_query_spec.rb  |  1 -
 spec/queries/route_breakdown_query_spec.rb    |  1 -
 spec/queries/subject_breakdown_query_spec.rb  |  1 -
 spec/queries/visa_breakdown_query_spec.rb     |  1 -
 23 files changed, 26 insertions(+), 119 deletions(-)

diff --git a/app/controllers/system_admin/applicants_controller.rb b/app/controllers/system_admin/applicants_controller.rb
index 9a868b41..28e3220d 100644
--- a/app/controllers/system_admin/applicants_controller.rb
+++ b/app/controllers/system_admin/applicants_controller.rb
@@ -9,7 +9,7 @@ class ApplicantsController < AdminController
     include Pagy::Backend
 
     def index
-      results = Application.submitted
+      results = Application.all
       .search(params[:search])
       .filter_by_status(params[:status])
       .order(created_at: :desc)
diff --git a/app/models/application.rb b/app/models/application.rb
index 358f13ab..79e54f6e 100644
--- a/app/models/application.rb
+++ b/app/models/application.rb
@@ -30,10 +30,6 @@ class Application < ApplicationRecord
   delegate :sla_breached?, to: :application_progress
   delegate :status, to: :application_progress, allow_nil: false
 
-  default_scope { submitted }
-  scope :submitted, -> { where.not(urn: nil) }
-  scope :in_progress, -> { unscoped.where(urn: nil) }
-
   scope :search, lambda { |term|
                    return if term.blank?
 
@@ -64,22 +60,11 @@ def mark_as_qa!
                        unique_by: %i[application_id status])
   end
 
-  with_options if: :submitted? do
-    validates(:application_date, presence: true)
-    validates(:application_route, presence: true)
-    validates(:date_of_entry, presence: true)
-    validates(:start_date, presence: true)
-    validates(:subject, presence: true)
-    validates(:visa_type, presence: true)
-    validates(:urn, presence: true)
-    validates(:applicant, presence: true)
-  end
-
-  def assign_urn!
-    update!(urn: Urn.generate(application_route))
-  end
-
-  def submitted?
-    urn.present?
-  end
+  validates(:application_date, presence: true)
+  validates(:application_route, presence: true)
+  validates(:date_of_entry, presence: true)
+  validates(:start_date, presence: true)
+  validates(:subject, presence: true)
+  validates(:visa_type, presence: true)
+  validates(:applicant, presence: true)
 end
diff --git a/app/models/kpis.rb b/app/models/kpis.rb
index 10ac1596..8176369c 100644
--- a/app/models/kpis.rb
+++ b/app/models/kpis.rb
@@ -4,7 +4,7 @@ def initialize
   end
 
   def total_applications
-    Application.submitted.count
+    Application.count
   end
 
   def total_rejections
diff --git a/app/queries/average_age_query.rb b/app/queries/average_age_query.rb
index 08240ad1..383d827d 100644
--- a/app/queries/average_age_query.rb
+++ b/app/queries/average_age_query.rb
@@ -1,6 +1,6 @@
 class AverageAgeQuery
   def initialize(relation = Applicant.all)
-    @relation = relation.joins(:application).merge(Application.submitted)
+    @relation = relation.joins(:application).merge(Application.all)
   end
 
   def call
diff --git a/app/queries/gender_breakdown_query.rb b/app/queries/gender_breakdown_query.rb
index 73b354bf..98340ba9 100644
--- a/app/queries/gender_breakdown_query.rb
+++ b/app/queries/gender_breakdown_query.rb
@@ -1,6 +1,6 @@
 class GenderBreakdownQuery
   def initialize(relation = Applicant.all)
-    @relation = relation.joins(:application).merge(Application.submitted)
+    @relation = relation.joins(:application).merge(Application.all)
   end
 
   def call
diff --git a/app/queries/nationality_breakdown_query.rb b/app/queries/nationality_breakdown_query.rb
index b22f006d..6b075920 100644
--- a/app/queries/nationality_breakdown_query.rb
+++ b/app/queries/nationality_breakdown_query.rb
@@ -1,6 +1,6 @@
 class NationalityBreakdownQuery
   def initialize(relation = Applicant.all)
-    @relation = relation.joins(:application).merge(Application.submitted)
+    @relation = relation.joins(:application).merge(Application.all)
   end
 
   def call
diff --git a/app/queries/rejection_reason_breakdown_query.rb b/app/queries/rejection_reason_breakdown_query.rb
index a9d5aea9..ee8a6a07 100644
--- a/app/queries/rejection_reason_breakdown_query.rb
+++ b/app/queries/rejection_reason_breakdown_query.rb
@@ -1,6 +1,6 @@
 class RejectionReasonBreakdownQuery
   def initialize(relation = ApplicationProgress.all)
-    @relation = relation.joins(:application).merge(Application.submitted).where.not(rejection_reason: nil)
+    @relation = relation.joins(:application).merge(Application.all).where.not(rejection_reason: nil)
   end
 
   def call
diff --git a/app/queries/route_breakdown_query.rb b/app/queries/route_breakdown_query.rb
index 2b865dc7..2df254ba 100644
--- a/app/queries/route_breakdown_query.rb
+++ b/app/queries/route_breakdown_query.rb
@@ -1,6 +1,6 @@
 class RouteBreakdownQuery
   def initialize(relation = Application.all)
-    @relation = relation.submitted
+    @relation = relation
   end
 
   def call
diff --git a/app/queries/subject_breakdown_query.rb b/app/queries/subject_breakdown_query.rb
index ea0aa1db..19559899 100644
--- a/app/queries/subject_breakdown_query.rb
+++ b/app/queries/subject_breakdown_query.rb
@@ -1,6 +1,6 @@
 class SubjectBreakdownQuery
   def initialize(relation = Application.all)
-    @relation = relation.submitted
+    @relation = relation
   end
 
   def call
diff --git a/app/queries/visa_breakdown_query.rb b/app/queries/visa_breakdown_query.rb
index 80d815b7..a55afdf1 100644
--- a/app/queries/visa_breakdown_query.rb
+++ b/app/queries/visa_breakdown_query.rb
@@ -1,6 +1,6 @@
 class VisaBreakdownQuery
   def initialize(relation = Application.all)
-    @relation = relation.submitted
+    @relation = relation
   end
 
   def call
diff --git a/spec/factories/applications.rb b/spec/factories/applications.rb
index 08050094..49d86172 100644
--- a/spec/factories/applications.rb
+++ b/spec/factories/applications.rb
@@ -31,7 +31,7 @@
     visa_type { VisaStep::VALID_ANSWERS_OPTIONS.reject { _1 == "Other" }.sample }
     date_of_entry { Time.zone.today }
     start_date { 1.month.from_now.to_date }
-    submitted
+    urn { Urn.generate(application_route) }
 
     factory :teacher_application do
       application_route { "teacher" }
@@ -45,14 +45,6 @@
       applicant strategy: :create, factory: :applicant
     end
 
-    trait :submitted do
-      urn { Urn.generate(application_route) }
-    end
-
-    trait :not_submitted do
-      urn { nil }
-    end
-
     trait :with_initial_checks_completed do
       application_progress strategy: :build, factory: %i[application_progress initial_checks_completed]
     end
diff --git a/spec/features/admin_console/applications_list_spec.rb b/spec/features/admin_console/applications_list_spec.rb
index d00ffb07..85a51dee 100644
--- a/spec/features/admin_console/applications_list_spec.rb
+++ b/spec/features/admin_console/applications_list_spec.rb
@@ -60,13 +60,13 @@
   def given_there_are_few_applications
     # Create 2 specific applications for search tests
     unique_applicant = create(:applicant, given_name: "Unique Given Name", middle_name: "Unique Middle Name", family_name: "Unique Family Name", email_address: "unique@example.com")
-    create(:application, :submitted, applicant: unique_applicant, urn: "Unique Urn 1")
+    create(:application, applicant: unique_applicant, urn: "Unique Urn 1")
 
     another_applicant = create(:applicant, given_name: "Another Given Name", middle_name: "Another Middle Name", family_name: "Another Family Name", email_address: "another@example.com")
-    create(:application, :submitted, applicant: another_applicant, urn: "Unique Urn 2")
+    create(:application, applicant: another_applicant, urn: "Unique Urn 2")
 
     # Create 19 more applications for pagination test
-    create_list(:application, 19, :submitted)
+    create_list(:application, 19)
   end
 
   def given_there_is_an_application_that_breached_sla
diff --git a/spec/features/admin_console/download_qa_csv_file_spec.rb b/spec/features/admin_console/download_qa_csv_file_spec.rb
index 3c7e1548..35fc74a8 100644
--- a/spec/features/admin_console/download_qa_csv_file_spec.rb
+++ b/spec/features/admin_console/download_qa_csv_file_spec.rb
@@ -36,7 +36,7 @@
   end
 
   def given_there_are_few_applications
-    create_list(:application, 5, :submitted)
+    create_list(:application, 5)
   end
 
   def when_i_am_in_the_applications_list_page
diff --git a/spec/features/admin_console/duplicates_search_spec.rb b/spec/features/admin_console/duplicates_search_spec.rb
index 9d21f2a2..c4f64ce3 100644
--- a/spec/features/admin_console/duplicates_search_spec.rb
+++ b/spec/features/admin_console/duplicates_search_spec.rb
@@ -10,10 +10,10 @@
   let!(:applicant_four) { build(:applicant, email_address: "test2@example.com", passport_number: "123456", phone_number: "999999999") }
 
   before do
-    create(:application, :submitted, applicant: applicant_one)
-    create(:application, :submitted, applicant: applicant_two)
-    create(:application, :submitted, applicant: applicant_three)
-    create(:application, :submitted, applicant: applicant_four)
+    create(:application, applicant: applicant_one)
+    create(:application, applicant: applicant_two)
+    create(:application, applicant: applicant_three)
+    create(:application, applicant: applicant_four)
   end
 
   it "Admin can search for duplicates by email" do
@@ -34,20 +34,6 @@
     then_i_see_matching_duplicates_by_passport_number
   end
 
-  it "the view renders even where there are 'in progress' applications" do
-    given_i_am_signed_as_an_admin
-    when_there_are_in_progress_applications
-    when_i_visit_the_duplicates_page
-    then_i_see_the_in_progress_applications
-  end
-
-  it "shows duplicates only once even if they match multiple times" do
-    given_i_am_signed_as_an_admin
-    when_there_are_in_progress_applications
-    when_i_visit_the_duplicates_page
-    then_i_see_the_in_progress_applications_only_once
-  end
-
   def when_i_search_for_a_duplicate_by(type)
     visit duplicates_path
     case type
@@ -60,18 +46,10 @@ def when_i_search_for_a_duplicate_by(type)
     end
   end
 
-  def when_there_are_in_progress_applications
-    create(:application, applicant: nil, urn: nil, application_progress: nil)
-  end
-
   def when_i_visit_the_duplicates_page
     visit duplicates_path
   end
 
-  def then_i_see_the_in_progress_applications
-    expect(page).to have_content("Duplicated applications")
-  end
-
   def then_i_see_matching_duplicates
     expect(page).to have_content("test@example.com")
     expect(page).to have_content("123456")
diff --git a/spec/models/application_spec.rb b/spec/models/application_spec.rb
index 5ce83fb7..5ed916b0 100644
--- a/spec/models/application_spec.rb
+++ b/spec/models/application_spec.rb
@@ -26,7 +26,7 @@
 RSpec.describe Application do
   describe "validations" do
     context "when the application has been submitted" do
-      subject(:application) { build(:teacher_application, :submitted) }
+      subject(:application) { build(:teacher_application) }
 
       it { expect(application).to validate_presence_of(:application_date) }
       it { expect(application).to validate_presence_of(:application_route) }
@@ -36,30 +36,9 @@
       it { expect(application).to validate_presence_of(:visa_type) }
       it { expect(application).to validate_presence_of(:applicant) }
     end
-
-    context "when the application has not been submitted" do
-      it "is valid" do
-        expect(described_class.new).to be_valid
-      end
-    end
   end
 
   describe "scopes" do
-    describe ".submitted" do
-      it "returns applications with a URN" do
-        create_list(:application, 2, :submitted)
-        create(:application, :not_submitted)
-
-        expect(described_class.submitted.count).to eq 2
-      end
-
-      it "does not return applications without a URN" do
-        create(:application, :not_submitted)
-
-        expect(described_class.submitted.count).to eq 0
-      end
-    end
-
     describe ".filter_by_status" do
       it "returns applications with the specified status" do
         initial_checks_application = build(:application)
@@ -95,22 +74,6 @@
     end
   end
 
-  describe "#assign_urn!" do
-    it "matches the required format for a teacher" do
-      application = create(:teacher_application)
-      application.assign_urn!
-
-      expect(application.reload.urn).to match(/^IRPTE[A-Z0-9]{5}$/)
-    end
-
-    it "matches the required format for a trainee" do
-      application = create(:salaried_trainee_application)
-      application.assign_urn!
-
-      expect(application.reload.urn).to match(/^IRPST[A-Z0-9]{5}$/)
-    end
-  end
-
   describe ".search" do
     let(:applicant_john) { create(:applicant, given_name: "John", family_name: "Doe", email_address: "john.doe@example.com", passport_number: "123456") }
     let(:application_john) { create(:application, urn: "APP123", applicant: applicant_john) }
diff --git a/spec/models/kpis_spec.rb b/spec/models/kpis_spec.rb
index f14c4142..c1ed016a 100644
--- a/spec/models/kpis_spec.rb
+++ b/spec/models/kpis_spec.rb
@@ -3,7 +3,6 @@
 RSpec.describe Kpis do
   describe "#total_applications" do
     it "returns the total number of applications" do
-      create(:application, :not_submitted)
       create_list(:application, 5)
 
       kpis = described_class.new
diff --git a/spec/queries/average_age_query_spec.rb b/spec/queries/average_age_query_spec.rb
index 19615279..6960cf71 100644
--- a/spec/queries/average_age_query_spec.rb
+++ b/spec/queries/average_age_query_spec.rb
@@ -4,8 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:application, :not_submitted, applicant: create(:applicant, date_of_birth: 25.years.ago))
-
         create(:application, applicant: create(:applicant, date_of_birth: 35.years.ago))
         create(:application, applicant: create(:applicant, date_of_birth: 45.years.ago))
         create(:application, applicant: create(:applicant, date_of_birth: 52.years.ago))
diff --git a/spec/queries/gender_breakdown_query_spec.rb b/spec/queries/gender_breakdown_query_spec.rb
index 4584948f..603d7269 100644
--- a/spec/queries/gender_breakdown_query_spec.rb
+++ b/spec/queries/gender_breakdown_query_spec.rb
@@ -4,8 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:application, :not_submitted, applicant: create(:applicant, sex: "male"))
-
         create(:application, applicant: create(:applicant, sex: "male"))
         create(:application, applicant: create(:applicant, sex: "female"))
         create(:application, applicant: create(:applicant, sex: "male"))
diff --git a/spec/queries/nationality_breakdown_query_spec.rb b/spec/queries/nationality_breakdown_query_spec.rb
index 9d3b5dc2..565fb718 100644
--- a/spec/queries/nationality_breakdown_query_spec.rb
+++ b/spec/queries/nationality_breakdown_query_spec.rb
@@ -4,8 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:application, :not_submitted, applicant: create(:applicant, nationality: "Nationality 1"))
-
         create(:application, applicant: create(:applicant, nationality: "Nationality 1"))
         create(:application, applicant: create(:applicant, nationality: "Nationality 3"))
         create(:application, applicant: create(:applicant, nationality: "Nationality 4"))
diff --git a/spec/queries/rejection_reason_breakdown_query_spec.rb b/spec/queries/rejection_reason_breakdown_query_spec.rb
index adf14d0c..d6c80bae 100644
--- a/spec/queries/rejection_reason_breakdown_query_spec.rb
+++ b/spec/queries/rejection_reason_breakdown_query_spec.rb
@@ -8,7 +8,6 @@
         create(:application_progress, application: create(:application, applicant:), rejection_reason: "suspected_fraud")
         create(:application_progress, application: create(:application, applicant:), rejection_reason: "suspected_fraud")
         create(:application_progress, application: create(:application, applicant:), rejection_reason: "ineligible_school")
-        create(:application_progress, application: create(:application, :not_submitted, applicant:), rejection_reason: "duplicate_submission") # This one should not be included in the count because the associated application is not submitted
       end
 
       it "returns the correct rejection reason breakdown" do
diff --git a/spec/queries/route_breakdown_query_spec.rb b/spec/queries/route_breakdown_query_spec.rb
index 7391a312..016a32c6 100644
--- a/spec/queries/route_breakdown_query_spec.rb
+++ b/spec/queries/route_breakdown_query_spec.rb
@@ -4,7 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:teacher_application, :not_submitted)
         create(:teacher_application)
         create(:salaried_trainee_application)
         create(:teacher_application)
diff --git a/spec/queries/subject_breakdown_query_spec.rb b/spec/queries/subject_breakdown_query_spec.rb
index 00e8147d..c9d4408b 100644
--- a/spec/queries/subject_breakdown_query_spec.rb
+++ b/spec/queries/subject_breakdown_query_spec.rb
@@ -4,7 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:application, :not_submitted)
         create(:application, subject: :physics)
         create(:application, subject: :languages)
         create(:application, subject: :general_science)
diff --git a/spec/queries/visa_breakdown_query_spec.rb b/spec/queries/visa_breakdown_query_spec.rb
index 7280d223..abeaf581 100644
--- a/spec/queries/visa_breakdown_query_spec.rb
+++ b/spec/queries/visa_breakdown_query_spec.rb
@@ -4,7 +4,6 @@
   describe "#call" do
     context "when there are a few applicants" do
       before do
-        create(:application, :not_submitted, visa_type: "visa_1")
         create(:application, visa_type: "visa_1")
         create_list(:application, 2, visa_type: "visa_2")
         create_list(:application, 4, visa_type: "visa_3")

From 2a2826243792775d001aaa8ca358480cc9974a0e Mon Sep 17 00:00:00 2001
From: JR <139925286+fumimowdan@users.noreply.github.com>
Date: Wed, 20 Sep 2023 11:15:43 +0100
Subject: [PATCH 3/3] Revert "Revert "Avoid extra sql query""

This reverts commit 5450876250359ec8026bd84c791cbdaee260bc58.
---
 app/services/submit_form.rb | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/app/services/submit_form.rb b/app/services/submit_form.rb
index 84f9a20e..939f2756 100644
--- a/app/services/submit_form.rb
+++ b/app/services/submit_form.rb
@@ -36,15 +36,15 @@ def submit_form!
 
   def create_application_records
     ActiveRecord::Base.transaction do
-      school = create_school
-      applicant = create_applicant(school)
-      @application = create_application(applicant)
+      create_school
+      create_applicant
+      create_application
       delete_form
     end
   end
 
   def create_school
-    School.create!(
+    @school = School.create!(
       name: form.school_name,
       headteacher_name: form.school_headteacher_name,
       address_attributes: {
@@ -56,8 +56,8 @@ def create_school
     )
   end
 
-  def create_applicant(school)
-    Applicant.create!(
+  def create_applicant
+    @applicant = Applicant.create!(
       ip_address: ip_address,
       given_name: form.given_name,
       middle_name: form.middle_name,
@@ -75,13 +75,13 @@ def create_applicant(school)
         city: form.city,
         postcode: form.postcode,
       },
-      school: school,
+      school: @school,
     )
   end
 
-  def create_application(applicant)
-    Application.create!(
-      applicant: applicant,
+  def create_application
+    @application = Application.create!(
+      applicant: @applicant,
       application_date: Date.current.to_s,
       application_route: form.application_route,
       application_progress: ApplicationProgress.new,
@@ -100,7 +100,7 @@ def delete_form
   def send_applicant_email
     GovukNotifyMailer
       .with(
-        email: application.applicant.email_address,
+        email: @applicant.email_address,
         urn: application.urn,
       )
       .application_submission