Merge pull request #153 from epimorphics/task/rc-2.0.1

Task: Release Candidate v2.0.2

CHANGELOG.md

@@ -1,5 +1,22 @@
 # Standard Reports UI: change log
 
+## 2.0.2 - 2024-12
+
+- (Jon) Myriad of tweaks to ensure variables either fail quietly via safe
+  navigation or are set to a default value to prevent errors in the application
+- (Jon) Updated the error template path to use `Rails.public_path` as well as
+  contain the `html` extension to ensure the correct template is rendered
+- (Jon) Improves error metrics reporting to ensure that logging always happens
+  with the appropriate severity depending on the exception status while reducing
+  the types of errors that can trigger a an error metric and therefore a
+  notification in slack
+  [GH-149](https://github.com/epimorphics/hmlr-linked-data/issues/149)
+
+## 2.0.1 - 2024-12
+
+- (Bogdan) Fixed a bug that was causing an internal application error in
+  `ReportManagerApi`
+
 ## 2.0.0 - 2024-12
 
 - (Bogdan) Updated all gems by regenerating `Gemfile.lock`
@@ -9,7 +26,8 @@
 
 ## 1.6.0 - 2024-10
 
-- (Dan) Updates ruby version to 2.7.8 and alpine version to 3.16 [GH-143](https://github.com/epimorphics/standard-reports-ui/issues/143)
+- (Dan) Updates ruby version to 2.7.8 and alpine version to 3.16
+  [GH-143](https://github.com/epimorphics/standard-reports-ui/issues/143)
 
 ## 1.5.4 - 2024-10
 

Makefile

@@ -37,8 +37,9 @@ ${GITHUB_TOKEN}:
 all: image
 
 assets: auth
-	@./bin/bundle config set --local without 'development test'
+	@echo "Installing all packages ..."
 	@./bin/bundle install
+	@echo "Cleaning up precompiled assets ..."
 	@./bin/rails assets:clean assets:precompile
 
 auth: ${GITHUB_TOKEN} ${BUNDLE_CFG}
@@ -68,12 +69,6 @@ image: auth
 lint: assets
 	@./bin/bundle exec rubocop
 
-local:
-	@echo "Installing all packages ..."
-	@./bin/bundle install
-	@echo "Starting local server ..."
-	@./bin/rails server -p ${PORT}
-
 publish: image
 	@echo Publishing image: ${REPO}:${TAG} ...
 	@docker push ${REPO}:${TAG} 2>&1
@@ -86,7 +81,7 @@ run: start
 	@if docker network inspect dnet > /dev/null 2>&1; then echo "Using docker network dnet"; else echo "Create docker network dnet"; docker network create dnet; sleep 2; fi
 	@docker run -p ${PORT}:3000 -e API_SERVICE_URL=${API_SERVICE_URL} --network dnet --rm --name ${SHORTNAME} ${REPO}:${TAG}
 
-server: assets start
+server:
 	@export SECRET_KEY_BASE=$(./bin/rails secret)
 	@API_SERVICE_URL=${API_SERVICE_URL} ./bin/rails server -p ${PORT}
 

app/controllers/application_controller.rb

@@ -27,10 +27,6 @@ def log_request_result
   # or attempt to render a generic error page if no specific error page exists
   unless Rails.application.config.consider_all_requests_local
     rescue_from StandardError do |e|
-      # Instrument ActiveSupport::Notifications for internal errors but only for non-404 errors:
-      unless e.is_a?(ActionController::RoutingError) || e.is_a?(ActionView::MissingTemplate)
-        ActiveSupport::Notifications.instrument('internal_error.application', exception: e)
-      end
       # Trigger the appropriate error handling method based on the exception
       case e.class
       when ActionController::RoutingError, ActionView::MissingTemplate
@@ -51,6 +47,8 @@ def handle_internal_error(exception)
       render_error(400)
     else
       Rails.logger.warn "No explicit error page for exception #{exception} - #{exception.class}"
+      # Instrument ActiveSupport::Notifications for internal server errors only:
+      instrument_internal_error(exception)
       render_error(500)
     end
   end
@@ -83,7 +81,7 @@ def render_error(status)
 
   def render_html_error_page(status)
     render(layout: true,
-           file: Rails.root.join('public', 'landing', status.to_s),
+           file: Rails.public_path + "landing/#{status}.html",
            status: status)
   end
 
@@ -120,4 +118,26 @@ def detailed_request_log(duration)
     end
   end
   # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
+
+  # Notify subscriber(s) of an internal error event with the payload of the
+  # exception once done
+  # @param [exc] exp the exception that caused the error
+  # @return [ActiveSupport::Notifications::Event] provides an object-oriented
+  # interface to the event
+  def instrument_internal_error(exc) # rubocop:disable Metrics/AbcSize, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
+    err = {
+      message: exc&.message || exc,
+      status: exc&.status || Rack::Utils::SYMBOL_TO_STATUS_CODE[exc]
+    }
+    err[:type] = exc.class&.name if exc&.class
+    err[:cause] = exc&.cause if exc&.cause
+    err[:backtrace] = exc&.backtrace if exc&.backtrace && Rails.env.development?
+    # Log the exception to the Rails logger with the appropriate severity
+    Rails.logger.send(err[:status] < 500 ? :warn : :error, JSON.generate(err))
+    # Return unless the status code is 500 or greater to ensure subscribers are NOT notified
+    return unless err[:status] >= 500
+
+    # Instrument the internal error event to notify subscribers of the error
+    ActiveSupport::Notifications.instrument('internal_error.application', exception: err)
+  end
 end

app/helpers/download_request_helper.rb

@@ -12,7 +12,7 @@ def render_report_request(request)
     end
   end
 
-  def request_status(request)
+  def request_status(request) # rubocop:disable Metrics/MethodLength
     capture do
       if request.unknown?
         concat render_unknown_request(request)

app/helpers/report_design_helper.rb

@@ -51,7 +51,7 @@ def layout_values_as_toggle_buttons(step, values, radio, cls = '')
     end
   end
 
-  def toggle_button_option(step, value, radio, single_value)
+  def toggle_button_option(step, value, radio, single_value) # rubocop:disable Metrics/MethodLength
     active = value.active? || single_value
 
     content_tag(:div, class: 'o-form-control') do
@@ -116,7 +116,7 @@ def selected_area_summary(workflow)
   end
 
   def layout_custom_dates(delta, step, workflow)
-    year = Time.now.year - delta
+    year = Time.current.year - delta
     content_tag(:div, class: 'row') do
       concat(content_tag(:div, class: 'col-sm-12 col-md-1') do
         content_tag(:h3, year.to_s, class: 'u-font-bold u-align-top')
@@ -130,7 +130,7 @@ def layout_custom_dates(delta, step, workflow)
   end
 
   def layout_all_year(step, year, workflow)
-    all_year = year == Time.now.year ? 'to date' : 'all year'
+    all_year = year == Time.current.year ? 'to date' : 'all year'
     checked = workflow.has_state?(step.param_name, year.to_s)
     capture do
       concat prompted_row(
@@ -218,7 +218,7 @@ def show_change_link(workflow, step)
 
   def layout_map_control(_step)
     content_tag(:div, class: 'col-sm-12 col-md-6') do
-      tag(:div, id: 'map', class: 'o-map')
+      tag.div(id: 'map', class: 'o-map')
     end
   end
 end

app/lib/version.rb

@@ -3,7 +3,7 @@
 module Version
   MAJOR = 2
   MINOR = 0
-  PATCH = 0
+  PATCH = 2
   SUFFIX = nil
-  VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}"
+  VERSION = "#{MAJOR}.#{MINOR}.#{PATCH}#{SUFFIX && ".#{SUFFIX}"}".freeze
 end

app/models/report_specification.rb

@@ -28,7 +28,7 @@ def normalize_period(report_manager)
     end
   end
 
-  def latest_quarter(report_manager)
+  def latest_quarter(report_manager) # rubocop:disable Metrics/MethodLength
     case report_manager.latest_month
     when 1..2
       "#{report_manager.latest_year - 1}-Q4"

app/models/step_select_aggregation_type.rb

@@ -14,7 +14,7 @@ def initialize
     super(:select_aggregation_type, :aggregate, :radio)
   end
 
-  def values_options(workflow) # rubocop:disable Metrics/CyclomaticComplexity
+  def values_options(workflow) # rubocop:disable Metrics/CyclomaticComplexity, Metrics/MethodLength
     case workflow.state(:areaType)
     when 'country'
       [AGGREGATE_BY_REGION, AGGREGATE_BY_COUNTY, AGGREGATE_BY_DISTRICT,

app/models/step_select_dates.rb

@@ -51,9 +51,9 @@ def generic_name
     'select dates'
   end
 
-  def each_year(hidden_only = true, &block)
+  def each_year(hidden_only = true, &block) # rubocop:disable Style/OptionalBooleanParameter
     start_delta = hidden_only ? YEARS_SHOWN_BY_DEFAULT : 0
-    delta = Time.now.year - EARLIEST_YEAR
+    delta = Time.current.year - EARLIEST_YEAR
     (start_delta..delta).each(&block)
   end
 
@@ -95,7 +95,7 @@ def latest_values(workflow)
     ]
   end
 
-  def summarise_value
+  def summarise_value # rubocop:disable Metrics/MethodLength
     proc { |state_value|
       s = case state_value.to_sym
           when :ytd

app/models/step_select_district.rb

@@ -23,7 +23,8 @@ def successor_step
     :select_aggregation_type
   end
 
-  NAMES = [
+  # The names of all the districts
+  NAMES = [ # rubocop:disable Metrics/CollectionLiteralLength
     'ADUR',
     'ALLERDALE',
     'AMBER VALLEY',

app/models/step_select_postcode_area.rb

@@ -2,7 +2,7 @@
 
 # Workflow step of selecting a postcode area
 class StepSelectPostcodeArea < StepSelectPostcode
-  VALIDATION = /\A[A-Z][A-Z]?\Z/.freeze
+  VALIDATION = /\A[A-Z][A-Z]?\Z/
 
   def initialize
     super(:select_pc_area)

app/models/step_select_postcode_district.rb

@@ -2,7 +2,7 @@
 
 # Workflow step of selecting a postcode district
 class StepSelectPostcodeDistrict < StepSelectPostcode
-  VALIDATION = /\A[A-Z][A-Z]?[0-9][0-9]?[A-Z]?\Z/.freeze
+  VALIDATION = /\A[A-Z][A-Z]?[0-9][0-9]?[A-Z]?\Z/
 
   def initialize
     super(:select_pc_district)

app/models/step_select_postcode_sector.rb

@@ -2,7 +2,7 @@
 
 # Workflow step of selecting a postcode sector
 class StepSelectPostcodeSector < StepSelectPostcode
-  VALIDATION = /\A[A-Z][A-Z]?[0-9][0-9]?[A-Z]? [0-9]\Z/.freeze
+  VALIDATION = /\A[A-Z][A-Z]?[0-9][0-9]?[A-Z]? [0-9]\Z/
 
   def initialize
     super(:select_pc_sector)

app/models/workflow.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 # Model encapsulating the report-generation workflow
-class Workflow
+class Workflow # rubocop:disable Metrics/ClassLength
   extend Forwardable
 
   attr_reader :step_history

app/services/report_manager.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 # Service object for interacting with remote service-manager API
-class ReportManager
+class ReportManager # rubocop:disable Metrics/ClassLength
   def initialize(config = nil)
     return unless config
 
@@ -17,15 +17,24 @@ def initialize(config = nil)
   end
 
   def latest_month
-    latest_month_spec.split('-').second.to_i
+    latest_month_spec && latest_month_spec.split('-').second.to_i
+  rescue StandardError => e
+    msg = "Failed to retreive latest month from #{url}latest-month-available"
+    Rails.logger.error { "#{msg}: #{e}" }
   end
 
   def latest_year
-    latest_month_spec.split('-').first.to_i
+    latest_month_spec && latest_month_spec.split('-').first.to_i
+  rescue StandardError => e
+    msg = "Failed to retreive latest year from #{url}latest-month-available"
+    Rails.logger.error { "#{msg}: #{e}" }
   end
 
   def latest_quarter
-    (latest_month / 3).to_i
+    latest_month && (latest_month / 3).to_i
+  rescue StandardError => e
+    msg = "Failed to retreive latest quarter from #{url}latest-month-available"
+    Rails.logger.error { "#{msg}: #{e}" }
   end
 
   def latest_month_spec
@@ -74,7 +83,7 @@ def start_requests(params)
   #     a=1&b[]=2&b[]=3
   # becomes
   #     [{a: 1, b: 2}, {a: 1, b: 3}]
-  def create_params_sets(params)
+  def create_params_sets(params) # rubocop:disable Metrics/MethodLength
     product = [{}]
 
     params.each do |k, v|
@@ -95,7 +104,7 @@ def create_params_sets(params)
 
   def start_request(req_spec)
     json = api.post_json("#{url}report-request", req_spec.to_hash)
-    Rails.logger.debug { "ReportManager: #{json}" } if Rails.env.development?
+    Rails.logger.debug { "ReportManager response: #{json}" } if Rails.env.development?
     ReportStatus.new(json)
   end
 
@@ -113,15 +122,19 @@ def validate?(params)
   end
 
   def key_present?(params, key)
-    return unless !params.key?(key) || params[key].nil? || params[key].to_s.empty?
+    return false unless !params.key?(key) ||
+                        params[key].nil? ||
+                        params[key].to_s.empty?
 
     @errors ||= []
     @errors << "missing parameter #{key}"
   end
 
   def array_key_present?(params, key)
-    return unless !params.key?(key) || params[key].nil? ||
-                  !params[key].is_a?(Array) || params[key].empty?
+    return false unless !params.key?(key) ||
+                        params[key].nil? ||
+                        !params[key].is_a?(Array) ||
+                        params[key].empty?
 
     @errors ||= []
     @errors << "missing parameter #{key}"
@@ -131,7 +144,7 @@ def valid_postcodes?(params)
     area = params[:area]
     pattern = validation_pattern(params)
 
-    return unless pattern && !pattern.match?(area)
+    return false unless pattern && !pattern.match?(area)
 
     @errors ||= []
     @errors << 'invalid postal code'