AP-5524:Let rails handle legal_aid_application RecordNotFound as 404

A question on how we want to handle Not found legal_aid_application objects.

Currently we swallow the fact that a legal_aid_application from the id in params is not found,
returning nil for it instead. It then handles it being nil by redirecting to the page_not_found page.

The rails way would be to let it raise RecordNotFound, which on this branch, will then be handled as
a 404 transparently by the ErrorController (via the exceptions_app  config).

The Advantage of letting rails handle it is that be there is no redirect so the URL remains the same,
which is more typical behaviour for not found, and logs will reflect the true cause of the 404. A
Disadvantage is I do not know what potential side affects it could have on the rest of the appsince
this code underpins virtually every page. But cannot think of any other than that.

app/controllers/concerns/providers/application_dependable.rb

@@ -16,24 +16,20 @@ def legal_aid_application_not_required?
       before_action :encode_upload_header
       before_action :set_legal_aid_application
 
+      # Let ActiveRecord::RecordNotFound bubble up for handling by exceptions_app as a 404
       def legal_aid_application
-        @legal_aid_application ||= LegalAidApplication.find_by(id: params[:legal_aid_application_id])
+        @legal_aid_application ||= LegalAidApplication.find(params[:legal_aid_application_id])
       end
       delegate :applicant, to: :legal_aid_application
 
     private
 
       def set_legal_aid_application
         return if self.class.legal_aid_application_not_required?
-        return process_invalid_application if legal_aid_application.blank?
 
         legal_aid_application.update!(provider_step:, provider_step_params:) unless provider_step == :delete
       end
 
-      def process_invalid_application
-        redirect_to error_path(:page_not_found)
-      end
-
       def provider_step_params
         params.except(:action, :controller, :legal_aid_application_id)
       end

spec/policies/provider_access_spec.rb

@@ -27,11 +27,12 @@
     expect(response).to redirect_to(error_path(:access_denied))
   end
 
-  it "allows missing application to be caught by not found" do
+  it "allows missing application to be caught by not found", :show_exceptions do
     login_as other_provider
 
     get providers_legal_aid_application_correspondence_address_lookup_path(SecureRandom.uuid)
 
-    expect(response).to redirect_to(error_path(:page_not_found))
+    expect(response).to have_http_status(:not_found)
+    expect(response).to render_template("errors/show/_page_not_found")
   end
 end

spec/requests/errors_controller_spec.rb

@@ -57,7 +57,7 @@
     let(:get_invalid_id) { get feedback_path(SecureRandom.uuid) }
 
     context "with default locale" do
-      it "responds with http status" do
+      it "responds with expected http status" do
         get_invalid_id
         expect(response).to have_http_status(:not_found)
       end
@@ -77,6 +77,23 @@
     end
   end
 
+  context "when page not found due to legal_aid_application not found" do
+    let(:get_invalid_id) { get providers_legal_aid_application_previous_references_path(legal_aid_application_id: SecureRandom.uuid) }
+    let(:provider) { create(:provider) }
+
+    before { sign_in provider }
+
+    it "responds with expected http status" do
+      get_invalid_id
+      expect(response).to have_http_status(:not_found)
+    end
+
+    it "renders page not found" do
+      get_invalid_id
+      expect(response).to render_template("errors/show/_page_not_found")
+    end
+  end
+
   context "when internal server error/500 due to code fault" do
     let(:get_invalid_id) { get feedback_path(SecureRandom.uuid) }
 

spec/requests/providers/about_the_financial_assessments_controller_spec.rb

@@ -33,11 +33,13 @@
         expect(unescaped_response_body).to include(I18n.t("providers.about_the_financial_assessments.show.title"))
       end
 
-      context "when the application does not exist" do
+      context "when the application does not exist", :show_exceptions do
         let(:application_id) { SecureRandom.uuid }
 
-        it "redirects to an error" do
-          expect(response).to redirect_to(error_path(:page_not_found))
+        it "renders page not found page" do
+          expect(response)
+            .to have_http_status(:not_found)
+            .and render_template("errors/show/_page_not_found")
         end
       end
 
@@ -83,15 +85,17 @@
         login_as application.provider
       end
 
-      context "when the application does not exist" do
+      context "when the application does not exist", :show_exceptions do
         let(:application_id) { SecureRandom.uuid }
 
         it "redirects to and error page without calling the email service" do
           expect(CitizenEmailService).not_to receive(:new)
 
           submit_patch
 
-          expect(response).to redirect_to(error_path(:page_not_found))
+          expect(response)
+            .to have_http_status(:not_found)
+            .and render_template("errors/show/_page_not_found")
         end
       end
 

spec/requests/providers/check_merits_answers_spec.rb

@@ -228,7 +228,7 @@
       context "when there are required document categories" do
         before do
           allow(LegalFramework::MeritsTasksService).to receive(:call).with(application).and_return(smtl)
-          allow(LegalAidApplication).to receive(:find_by).and_return(application)
+          allow(LegalAidApplication).to receive(:find).and_return(application)
           allow(application).to receive(:evidence_is_required?).and_return(true)
         end