Enable new cops and make automatic and manual corrections to satisfy …

…the new cops

.env.test

@@ -13,5 +13,6 @@ DFE_SIGNIN_CLIENT_SECRET=client-secret
 DFE_SIGNIN_REDIRECT_URI=http://redirect.example.com
 
 CCS_APP_API_DATA_BUCKET=test
+GOV_NOTIFY_API_KEY=d7101cf9-2a94-4728-a1dd-428e3309886f-1b0cecdd-88af-48ed-972f-f36aadadb6bd
 
 RAILS_ENV_URL=http://localhost:3000

.rubocop.yml

@@ -5,7 +5,7 @@ require:
  - './lib/cop/all'
 
 AllCops:
- NewCops: disable
+ NewCops: enable
  TargetRubyVersion: 2.7
  Exclude:
  - 'db/schema.rb'
@@ -161,4 +161,4 @@ Rails/HelperInstanceVariable:
  Enabled: false
 
 Style/OptionalBooleanParameter:
- Enabled: false
+ Enabled: false

app/calculators/facilities_management/rm3830/calculator.rb

@@ -187,13 +187,13 @@ def create_rates_and_variances_object
  end
 
  VARIANCE_KEY_TRANSFORMATION = {
- "Profit %": 'M142',
- "Corporate Overhead %": 'M141',
- "Management Overhead %": 'M140',
- "Mobilisation Cost (DA %)": 'B1',
- "TUPE Risk Premium (DA %)": 'M148',
- "London Location Variance Rate (%)": 'M144',
- "Cleaning Consumables per Building User (£)": 'M146'
+ 'Profit %': 'M142',
+ 'Corporate Overhead %': 'M141',
+ 'Management Overhead %': 'M140',
+ 'Mobilisation Cost (DA %)': 'B1',
+ 'TUPE Risk Premium (DA %)': 'M148',
+ 'London Location Variance Rate (%)': 'M144',
+ 'Cleaning Consumables per Building User (£)': 'M146'
  }.freeze
  end
  end

app/controllers/base/passwords_controller.rb

@@ -11,6 +11,10 @@ def new
  @response = Cognito::ForgotPassword.new(params[:email])
  end
 
+ def edit
+ @response = Cognito::ConfirmPasswordReset.new(cookies[:crown_marketplace_reset_email], params[:password], params[:password_confirmation], params[:confirmation_code])
+ end
+
  def create
  @response = Cognito::ForgotPassword.call(params[:email])
  if @response.success?
@@ -22,10 +26,6 @@ def create
  end
  end
 
- def edit
- @response = Cognito::ConfirmPasswordReset.new(cookies[:crown_marketplace_reset_email], params[:password], params[:password_confirmation], params[:confirmation_code])
- end
-
  def update
  @response = Cognito::ConfirmPasswordReset.call(params[:email], params[:password], params[:password_confirmation], params[:confirmation_code])
 

app/controllers/base/users_controller.rb

@@ -67,7 +67,7 @@ def sign_in_user(user)
 
  def set_user_phone
  user_phone_full = User.find_by(cognito_uuid: cookies[:crown_marketplace_challenge_username]).try(:phone_number) || Cognito::CreateUserFromCognito.call(cookies[:crown_marketplace_challenge_username]).try(:user).try(:phone_number)
- @user_phone = '*' * 9 + user_phone_full.last(3) if user_phone_full
+ @user_phone = ('*' * 9) + user_phone_full.last(3) if user_phone_full
  end
  end
 end

app/controllers/crown_marketplace/framework_controller.rb

@@ -1,4 +1,4 @@
-class CrownMarketplace::FrameworkController < ::ApplicationController
+class CrownMarketplace::FrameworkController < ApplicationController
  before_action :authenticate_user!
  before_action :authorize_user
 

app/controllers/crown_marketplace/manage_users_controller.rb

@@ -40,8 +40,14 @@ def create_add_user
  end
  end
 
+ def show
+ @minimum_editor = @user.minimum_editor_role
+ end
+
  def new; end
 
+ def edit; end
+
  def create
  @user = Cognito::Admin::User.new(@current_user_access, add_user_params)
 
@@ -54,12 +60,6 @@ def create
  end
  end
 
- def show
- @minimum_editor = @user.minimum_editor_role
- end
-
- def edit; end
-
  def update
  @user.assign_attributes(**user_params)
 

app/controllers/facilities_management/buildings_controller.rb

@@ -15,10 +15,12 @@ def index
  @buildings = current_user.buildings.order_by_building_name.page(params[:page])
  end
 
+ # rubocop:disable Rails/I18nLazyLookup
  def show
  @back_text = t('facilities_management.buildings.show.return_to_buildings')
  @back_path = index_path
  end
+ # rubocop:enable Rails/I18nLazyLookup
 
  def new
  @building = current_user.buildings.build

app/controllers/facilities_management/procurement_buildings_controller.rb

@@ -18,12 +18,14 @@ def update
  end
  end
 
+ # rubocop:disable Rails/I18nLazyLookup
  def missing_regions
  redirect_to procurement_show_path unless @procurement.procurement_buildings_missing_regions?
 
  @back_path = procurement_index_path
  @back_text = t('facilities_management.procurement_buildings.missing_regions.return_to_dashboard')
  end
+ # rubocop:enable Rails/I18nLazyLookup
 
  private
 

app/controllers/facilities_management/procurement_details_controller.rb

@@ -92,12 +92,12 @@ def set_paginated_buildings_data
  @buildings = current_user.buildings.order_by_building_name.page(params[:page])
  visible_buildings_ids = @buildings.map(&:id)
 
- hidden_building_ids = @building_params.reject { |building_id, _| visible_buildings_ids.include? building_id }.keys
+ hidden_building_ids = @building_params.except(*visible_buildings_ids).keys
  @hidden_buildings = current_user.buildings.order_by_building_name.where(id: hidden_building_ids)
  end
 
  def paramatise_building_selection
- @building_params = procurement_params['procurement_buildings_attributes'].to_h.values.select { |building| building['active'] }.map { |item| [item['building_id'], item['active']] }.to_h
+ @building_params = procurement_params['procurement_buildings_attributes'].to_h.values.select { |building| building['active'] }.to_h { |item| [item['building_id'], item['active']] }
  end
 
  def paginate_procurement_buildings
@@ -116,7 +116,7 @@ def assign_procurement_attributes
 
  return unless params[:commit]
 
- current_procurement_buildings = @procurement.procurement_buildings.select(:id, :building_id).map { |pb| [pb.building_id, pb.id] }.to_h
+ current_procurement_buildings = @procurement.procurement_buildings.select(:id, :building_id).to_h { |pb| [pb.building_id, pb.id] }
  @procurement.assign_attributes(procurement_buildings_attributes: @building_params.map { |building_id, active| { id: current_procurement_buildings[building_id], building_id: building_id, active: active } })
  else
  @procurement.assign_attributes(procurement_params)

app/controllers/facilities_management/rm3830/admin/sublot_regions_controller.rb

@@ -16,11 +16,11 @@ def update
  private
 
  def set_region_data
- @regions = Nuts1Region.all.map { |region| [region.code, region.name] }.to_h
+ @regions = Nuts1Region.all.to_h { |region| [region.code, region.name] }
  supplier_lot_data = @supplier.lot_data[@lot]['regions']
  @sublot_region_name = "Sub-lot #{@lot} regions"
  @selected_supplier_regions = Supplier::SupplierRegionsHelper.supllier_selected_regions(supplier_lot_data)
- @subregions = FacilitiesManagement::Region.all.map { |region| [region.code, region.name] }.to_h
+ @subregions = FacilitiesManagement::Region.all.to_h { |region| [region.code, region.name] }
  end
  end
  end

app/controllers/facilities_management/rm3830/admin/sublot_services_controller.rb

@@ -28,10 +28,10 @@ def set_service_data
 
  def setup_checkboxes(supplier_services)
  @supplier_rate_data_checkboxes = @full_services.map do |service|
- service['work_package'].map do |work_pckg|
+ service['work_package'].to_h do |work_pckg|
  code = work_pckg['code']
  [code, supplier_services.include?(code)]
- end.to_h
+ end
  end.inject(:merge)
  end
 

app/controllers/facilities_management/rm3830/procurements/spreadsheet_imports_controller.rb

@@ -7,6 +7,10 @@ class SpreadsheetImportsController < FacilitiesManagement::FrameworkController
  before_action :set_spreadsheet_import, only: %i[show destroy progress]
  before_action :redirect_to_requirements
 
+ def show
+ initialize_errors if @spreadsheet_import.failed?
+ end
+
  def new
  @spreadsheet_import = SpreadsheetImport.new(facilities_management_rm3830_procurement_id: params[:procurement_id])
  end
@@ -26,10 +30,6 @@ def create
  end
  end
 
- def show
- initialize_errors if @spreadsheet_import.failed?
- end
-
  def destroy
  @spreadsheet_import.delete
  redirect_to new_facilities_management_rm3830_procurement_spreadsheet_import_path(procurement_id: @procurement.id)
@@ -87,7 +87,7 @@ def import_error_type
  def cancel_and_return
  @procurement.spreadsheet_import.destroy if @procurement.spreadsheet_import.present?
 
- redirect_to facilities_management_rm3830_procurement_path(id: @procurement.id, 'spreadsheet': true)
+ redirect_to facilities_management_rm3830_procurement_path(id: @procurement.id, spreadsheet: true)
  end
 
  protected

app/controllers/facilities_management/rm3830/procurements_controller.rb

@@ -34,6 +34,10 @@ def new
  @back_text = 'Return to regions'
  end
 
+ def edit
+ redirect_to facilities_management_rm3830_procurement_path(@procurement) if params[:step].nil?
+ end
+
  def create
  @procurement = current_user.rm3830_procurements.build(procurement_params)
 
@@ -44,7 +48,7 @@ def create
  elsif params[:save_for_later].present?
  redirect_to facilities_management_rm3830_procurements_path
  else
- redirect_to facilities_management_rm3830_procurement_path(@procurement, 'what_happens_next': true)
+ redirect_to facilities_management_rm3830_procurement_path(@procurement, what_happens_next: true)
  end
  else
  @errors = @procurement.errors
@@ -53,10 +57,6 @@ def create
  end
  end
 
- def edit
- redirect_to facilities_management_rm3830_procurement_path(@procurement) if params[:step].nil?
- end
-
  def update
  return if updates_for_show_pages
  return if updates_for_edit_pages
@@ -155,13 +155,13 @@ def assign_procurement_parameters
  end
 
  PARAMS_METHODS_SHOW = {
- 'bulk_upload_spreadsheet': :start_bulk_upload,
- 'change_the_contract_value': :change_the_contract_value,
- 'change_requirements': :continue_to_summary,
- 'continue_to_results': :continue_to_results,
- 'continue_from_change_contract_value': :continue_from_change_contract_value,
- 'continue_from_results': :continue_from_results,
- 'exit_bulk_upload': :exit_detailed_search_bulk_upload
+ bulk_upload_spreadsheet: :start_bulk_upload,
+ change_the_contract_value: :change_the_contract_value,
+ change_requirements: :continue_to_summary,
+ continue_to_results: :continue_to_results,
+ continue_from_change_contract_value: :continue_from_change_contract_value,
+ continue_from_results: :continue_from_results,
+ exit_bulk_upload: :exit_detailed_search_bulk_upload
  }.freeze
 
  def updates_for_show_pages

app/controllers/facilities_management/rm3830/supplier/dashboard_controller.rb

@@ -6,16 +6,16 @@ class Supplier::DashboardController < FacilitiesManagement::Supplier::FrameworkC
  before_action :initialize_page_description
 
  def index
- if !@supplier.nil?
- @received_offers = contracts.sent.where(supplier_id: @supplier.id).order(:offer_sent_date)
- @accepted_offers = contracts.accepted.where(supplier_id: @supplier.id).order(supplier_response_date: :desc)
- @live_contracts = contracts.signed.where(supplier_id: @supplier.id).order(contract_start_date: :desc, contract_end_date: :desc)
- @closed_contracts = contracts.where(supplier_id: @supplier.id, aasm_state: ProcurementSupplier::CLOSED_TO_SUPPLIER).sort_by(&:closed_date).reverse
- else
+ if @supplier.nil?
  @received_offers = []
  @accepted_offers = []
  @live_contracts = []
  @closed_contracts = []
+ else
+ @received_offers = contracts.sent.where(supplier_id: @supplier.id).order(:offer_sent_date)
+ @accepted_offers = contracts.accepted.where(supplier_id: @supplier.id).order(supplier_response_date: :desc)
+ @live_contracts = contracts.signed.where(supplier_id: @supplier.id).order(contract_start_date: :desc, contract_end_date: :desc)
+ @closed_contracts = contracts.where(supplier_id: @supplier.id, aasm_state: ProcurementSupplier::CLOSED_TO_SUPPLIER).sort_by(&:closed_date).reverse
  end
  end
 

app/controllers/facilities_management/rm6232/admin/supplier_lot_data_controller.rb

@@ -67,7 +67,7 @@ def set_work_packages
  end
 
  def set_regions
- @regions = Nuts1Region.all_with_overseas.map { |nuts_1_region| [nuts_1_region.name, Region.all.select { |region| region.code.starts_with? nuts_1_region.code }] }.to_h
+ @regions = Nuts1Region.all_with_overseas.to_h { |nuts_1_region| [nuts_1_region.name, Region.all.select { |region| region.code.starts_with? nuts_1_region.code }] }
  end
 
  def lot_data_params

app/controllers/facilities_management/supplier/framework_controller.rb

@@ -1,4 +1,4 @@
-class FacilitiesManagement::Supplier::FrameworkController < ::ApplicationController
+class FacilitiesManagement::Supplier::FrameworkController < ApplicationController
  before_action :authenticate_user!
  before_action :authorize_user
  before_action :raise_if_unrecognised_live_framework

app/helpers/application_helper.rb

@@ -178,7 +178,7 @@ def page_title
  content_for(title_bit)
  end
  title += [t('layouts.application.title')]
- title.reject(&:blank?).map(&:strip).join(': ')
+ title.compact_blank.map(&:strip).join(': ')
  end
 
  def add_optional_error_prefix_to_page_title(errors)
@@ -312,7 +312,7 @@ def contact_link(link_text)
  end
 
  def accordion_region_items(region_codes, with_overseas: false)
- nuts1_regions = Nuts1Region.send(with_overseas ? :all_with_overseas : :all).map { |region| [region.code, { name: region.name, items: [] }] }.to_h
+ nuts1_regions = Nuts1Region.send(with_overseas ? :all_with_overseas : :all).to_h { |region| [region.code, { name: region.name, items: [] }] }
 
  FacilitiesManagement::Region.all.each do |region|
  region_group_code = region.code[..2]

app/helpers/facilities_management/buildings_helper.rb

@@ -28,7 +28,7 @@ def building_row_text(attribute, text)
  end
 
  def address_in_a_line
- [@building.address_line_1, @building.address_line_2, @building.address_town].reject(&:blank?).join(', ') + " #{@building.address_postcode}"
+ [@building.address_line_1, @building.address_line_2, @building.address_town].compact_blank.join(', ') + " #{@building.address_postcode}"
  end
 
  def type_description(text, attribute)
@@ -55,11 +55,11 @@ def section_number
  SECTIONS = %i[building_details building_area building_type security_type].freeze
 
  def should_building_details_be_open?
- @should_building_details_be_open ||= begin
- return false if @building.building_type.blank?
-
- @building.building_type == 'other' || @building.errors.key?(:other_building_type) || FacilitiesManagement::Building::BUILDING_TYPES[0..1].map { |bt| bt[:id] }.exclude?(@building.building_type)
- end
+ @should_building_details_be_open ||= if @building.building_type.blank?
+  false
+ else
+  @building.building_type == 'other' || @building.errors.key?(:other_building_type) || FacilitiesManagement::Building::BUILDING_TYPES[0..1].pluck(:id).exclude?(@building.building_type)
+  end
  end
 
  def building_type_radio_button(form, building_type, disabled)

app/helpers/facilities_management/find_address_helper.rb

@@ -46,7 +46,7 @@ def address_count
  end
 
  def address_in_a_line
- [address_line1, address_line2, address_town, address_county].reject(&:blank?).join(', ') + " #{address_postcode}"
+ [address_line1, address_line2, address_town, address_county].compact_blank.join(', ') + " #{address_postcode}"
  end
 
  ATTRIBUTES_TO_KEY = {

app/helpers/facilities_management/procurement_buildings_helper.rb

@@ -19,7 +19,7 @@ def building_summary(title, vlaue)
  end
 
  def regions
- Postcode::PostcodeCheckerV2.find_region(@building.address_postcode.delete(' ')).map { |region| region[:region] }
+ Postcode::PostcodeCheckerV2.find_region(@building.address_postcode.delete(' ')).pluck(:region)
  end
 
  def form_object

app/helpers/facilities_management/procurement_details_helper.rb

@@ -75,7 +75,7 @@ def procurement_building_checkbox_text(building)
  end
 
  def address_in_a_line(building)
- [building.address_line_1, building.address_line_2, building.address_town].reject(&:blank?).join(', ') + " #{building.address_postcode}"
+ [building.address_line_1, building.address_line_2, building.address_town].compact_blank.join(', ') + " #{building.address_postcode}"
  end
 
  def edit_page_title

app/helpers/facilities_management/rm3830/procurements_helper.rb

@@ -45,15 +45,15 @@ def link_url(section)
  end
 
  def work_packages_names
- @work_packages_names ||= StaticData.work_packages.map { |wp| [wp['code'], wp['name']] }.to_h
+ @work_packages_names ||= StaticData.work_packages.to_h { |wp| [wp['code'], wp['name']] }
  end
 
  def number_of_suppliers
  @procurement.procurement_suppliers.count
  end
 
  def procurement_services
- @procurement_services ||= @procurement.procurement_building_services.map { |s| s[:name] }.uniq
+ @procurement_services ||= @procurement.procurement_building_services.pluck(:name).uniq
  end
 
  def lowest_supplier_price