Merge pull request #557 from trade-tariff/HOTT-2237-session-integrity

HOTT-2237: Handle session integrity issues when fetching schemes

app/controllers/steps/base_controller.rb

@@ -3,6 +3,7 @@ class BaseController < ::ApplicationController
     include CommodityHelper
     include ServiceHelper
 
+    rescue_from Errors::SessionIntegrityError, with: :handle_session_integrity_error
     rescue_from StandardError, with: :handle_exception
 
     default_form_builder GOVUKDesignSystemFormBuilder::FormBuilder
@@ -53,14 +54,20 @@ def track_session
     end
 
     def handle_exception(exception)
-      Sentry.set_user(user_session.session.to_h.except('_csrf_token'))
-      track_session
+      with_session_tracking do
+        raise exception
+      end
+    end
 
-      raise exception
+    def handle_session_integrity_error(exception)
+      with_session_tracking do
+        Sentry.capture_exception(exception)
+        redirect_to sections_url, allow_other_host: true
+      end
     end
 
     def ensure_session_integrity
-      return redirect_to sections_url, allow_other_host: true if commodity_code.blank?
+      handle_session_integrity_error(Errors::SessionIntegrityError.new('commodity_code')) if commodity_code.blank?
     end
 
     def initialize_commodity_context_service
@@ -70,5 +77,11 @@ def initialize_commodity_context_service
     def title
       t("page_titles.#{@step.class.try(:id)}", default: super)
     end
+
+    def with_session_tracking
+      Sentry.set_user(user_session.session.to_h.except('_csrf_token'))
+      track_session
+      yield
+    end
   end
 end

app/models/api/commodity.rb

@@ -61,6 +61,8 @@ def applicable_excise_measures
     end
 
     def rules_of_origin_schemes
+      raise Errors::SessionIntegrityError, 'origin_country_code' if user_session.origin_country_code.blank?
+
       RulesOfOriginScheme.build_collection(
         source,
         nil,

config/application.rb

@@ -1,20 +1,22 @@
-require_relative "boot"
+require_relative 'boot'
 
-require "rails"
+require 'rails'
 # Pick the frameworks you want:
-require "active_model/railtie"
+require 'active_model/railtie'
 # require "active_job/railtie"
 require 'active_record/attribute_assignment'
 # require "active_record/railtie"
 # require "active_storage/engine"
-require "action_controller/railtie"
+require 'action_controller/railtie'
 # require "action_mailer/railtie"
 # require "action_mailbox/engine"
 # require "action_text/engine"
-require "action_view/railtie"
+require 'action_view/railtie'
 # require "action_cable/engine"
 # require "rails/test_unit/railtie"
 
+require_relative '../lib/errors'
+
 # Require the gems listed in Gemfile, including any gems
 # you've limited to :test, :development, or :production.
 Bundler.require(*Rails.groups)
@@ -32,9 +34,9 @@ class Application < Rails::Application
     # config.time_zone = "Central Time (US & Canada)"
     # config.eager_load_paths << Rails.root.join("extras")
     config.middleware.use Rack::Deflater
-    config.time_zone                 = "London"
+    config.time_zone                 = 'London'
     config.exceptions_app            = routes
-    config.trade_tariff_frontend_url = ENV["TRADE_TARIFF_FRONTEND_URL"]
-    config.duty_calculator_host      = ENV.fetch("DUTY_CALCULATOR_HOST", "http://localhost")
+    config.trade_tariff_frontend_url = ENV['TRADE_TARIFF_FRONTEND_URL']
+    config.duty_calculator_host      = ENV.fetch('DUTY_CALCULATOR_HOST', 'http://localhost')
   end
 end

lib/errors.rb

@@ -0,0 +1,13 @@
+module Errors
+  class SessionIntegrityError < StandardError
+    def initialize(expected_attribute)
+      super
+
+      @expected_attribute = expected_attribute
+    end
+
+    def message
+      "User session state error. You are missing #{@expected_attribute}. Do you have multiple tabs open?"
+    end
+  end
+end

spec/controllers/steps/duty_controller_spec.rb

@@ -4,7 +4,7 @@
     allow(Api::MonetaryExchangeRate).to receive(:for).with('GBP').and_call_original
   end
 
-  let(:user_session) { build(:user_session, import_destination: 'XI', commodity_code: '0103921100') }
+  let(:user_session) { build(:user_session, import_destination: 'XI', commodity_code: '0103921100', country_of_origin: 'AF') }
   let(:duty_calculator) { instance_double('DutyCalculator', options: []) }
 
   describe 'GET #show' do

spec/models/api/commodity_spec.rb

@@ -197,16 +197,24 @@
       allow(Api::RulesOfOriginScheme).to receive(:build_collection).and_call_original
     end
 
-    let(:user_session) { build(:user_session, :with_row_to_gb_route) }
-
-    it 'passes the correct arguments to the RulesOfOriginScheme collection builder' do
-      commodity.rules_of_origin_schemes
+    context 'when the user session has no origin country code in it' do
+      let(:user_session) { build(:user_session) }
 
-      expect(Api::RulesOfOriginScheme).to have_received(:build_collection).with('uk', nil, { country_code: 'AR', heading_code: '070200' })
+      it { expect { commodity.rules_of_origin_schemes }.to raise_error(Errors::SessionIntegrityError, /origin_country_code/) }
     end
 
-    it 'returns a collection of RulesOfOriginScheme records' do
-      expect(commodity.rules_of_origin_schemes.first).to be_a(Api::RulesOfOriginScheme)
+    context 'when the user session has an origin country code in it' do
+      let(:user_session) { build(:user_session, :with_row_to_gb_route) }
+
+      it 'passes the correct arguments to the RulesOfOriginScheme collection builder' do
+        commodity.rules_of_origin_schemes
+
+        expect(Api::RulesOfOriginScheme).to have_received(:build_collection).with('uk', nil, { country_code: 'AR', heading_code: '070200' })
+      end
+
+      it 'returns a collection of RulesOfOriginScheme records' do
+        expect(commodity.rules_of_origin_schemes.first).to be_a(Api::RulesOfOriginScheme)
+      end
     end
   end