diff --git a/Gemfile.lock b/Gemfile.lock index ea758e178..62912070b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -6,9 +6,10 @@ PATH countries (~> 5.5.0) defra_ruby_address (~> 0.1.0) defra_ruby_alert (~> 2.2.1) + defra_ruby_companies_house defra_ruby_email (~> 1.3.0) defra_ruby_govpay - defra_ruby_validators (~> 2.6) + defra_ruby_validators (~> 3.0) high_voltage (~> 3.1.2) jbuilder (~> 2.11.5) mongo_session_store (~> 3.2.1) @@ -143,6 +144,9 @@ GEM rest-client (~> 2.0) defra_ruby_alert (2.2.1) airbrake + defra_ruby_companies_house (1.0.0) + i18n + rest-client (~> 2.0) defra_ruby_email (1.3.0) notifications-ruby-client rails @@ -152,8 +156,9 @@ GEM defra_ruby_style (0.3.0) rubocop (>= 1.0, < 2.0) defra_ruby_template (5.4.1) - defra_ruby_validators (2.6.0) + defra_ruby_validators (3.0.0) activemodel + defra_ruby_companies_house i18n matrix os_map_ref @@ -271,7 +276,7 @@ GEM parser (3.2.2.4) ast (~> 2.4.1) racc - phonelib (0.8.6) + phonelib (0.10.1) protocol-hpack (1.4.2) protocol-http (0.25.0) protocol-http1 (0.16.0) @@ -400,6 +405,7 @@ GEM simplecov_json_formatter (~> 0.1) simplecov-html (0.12.3) simplecov_json_formatter (0.1.4) + simpleidn (0.2.3) spring (4.1.3) spring-commands-rspec (1.0.4) spring (>= 0.9.1) @@ -428,8 +434,9 @@ GEM uk_postcode (2.1.8) unaccent (0.4.0) unicode-display_width (2.5.0) - validates_email_format_of (1.7.2) - i18n + validates_email_format_of (1.8.2) + i18n (>= 0.8.0) + simpleidn vcr (6.2.0) web-console (4.2.1) actionview (>= 6.0.0) diff --git a/app/controllers/waste_carriers_engine/check_registered_company_name_forms_controller.rb b/app/controllers/waste_carriers_engine/check_registered_company_name_forms_controller.rb index 953fec862..b5446b9c7 100644 --- a/app/controllers/waste_carriers_engine/check_registered_company_name_forms_controller.rb +++ b/app/controllers/waste_carriers_engine/check_registered_company_name_forms_controller.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine class CheckRegisteredCompanyNameFormsController < ::WasteCarriersEngine::FormsController @@ -26,7 +26,8 @@ def transient_registration_attributes end def company_name - DefraRubyCompaniesHouse.new(@transient_registration.company_no).company_name + company_details = DefraRuby::CompaniesHouse::API.run(company_number: @transient_registration.company_no) + company_details[:company_name] end end end diff --git a/app/forms/waste_carriers_engine/check_registered_company_name_form.rb b/app/forms/waste_carriers_engine/check_registered_company_name_form.rb index 84fa300dc..3580c4b9a 100644 --- a/app/forms/waste_carriers_engine/check_registered_company_name_form.rb +++ b/app/forms/waste_carriers_engine/check_registered_company_name_form.rb @@ -8,11 +8,11 @@ class CheckRegisteredCompanyNameForm < ::WasteCarriersEngine::BaseForm validates :temp_use_registered_company_details, "waste_carriers_engine/yes_no": true def registered_company_name - companies_house_service.company_name + @registered_company_name ||= companies_house_details[:company_name] end def registered_office_address_lines - companies_house_service.registered_office_address_lines + @registered_office_address_lines ||= companies_house_details[:registered_office_address] end def submit(params) @@ -26,8 +26,8 @@ def submit(params) private - def companies_house_service - @_companies_house_service ||= DefraRubyCompaniesHouse.new(company_no) + def companies_house_details + @_companies_house_details ||= DefraRuby::CompaniesHouse::API.run(company_number: company_no) end end end diff --git a/app/models/concerns/waste_carriers_engine/can_use_renewing_registration_workflow.rb b/app/models/concerns/waste_carriers_engine/can_use_renewing_registration_workflow.rb index 3d0fe504f..ee349ece3 100644 --- a/app/models/concerns/waste_carriers_engine/can_use_renewing_registration_workflow.rb +++ b/app/models/concerns/waste_carriers_engine/can_use_renewing_registration_workflow.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine # rubocop:disable Metrics/ModuleLength @@ -258,9 +258,11 @@ def company_status_invalid? return false if company_no.blank? || overseas? begin - company_status = DefraRubyCompaniesHouse.new(company_no).company_status + company_status = DefraRuby::CompaniesHouse::API.run(company_number: company_no)[:company_status] !%w[active voluntary-arrangement].include?(company_status) - rescue StandardError + rescue StandardError => e + Rails.logger.error "Error checking company status: #{e}" + Airbrake.notify(e) true end end diff --git a/app/services/waste_carriers_engine/refresh_companies_house_name_service.rb b/app/services/waste_carriers_engine/refresh_companies_house_name_service.rb index 673bf6aec..56e84a2a7 100644 --- a/app/services/waste_carriers_engine/refresh_companies_house_name_service.rb +++ b/app/services/waste_carriers_engine/refresh_companies_house_name_service.rb @@ -1,14 +1,15 @@ # frozen_string_literal: true -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine class RefreshCompaniesHouseNameService < WasteCarriersEngine::BaseService def run(reg_identifier:) registration = Registration.find_by(reg_identifier: reg_identifier) - company_name = DefraRubyCompaniesHouse.new(registration.company_no).company_name - registration.registered_company_name = company_name + company_details = DefraRuby::CompaniesHouse::API.run(company_number: registration.company_no) + + registration.registered_company_name = company_details[:company_name] registration.companies_house_updated_at = Time.current registration.save! diff --git a/lib/defra_ruby_companies_house.rb b/lib/defra_ruby_companies_house.rb deleted file mode 100644 index f863ba2c9..000000000 --- a/lib/defra_ruby_companies_house.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require "rest-client" - -class DefraRubyCompaniesHouse - def initialize(company_no) - @company_url = "#{Rails.configuration.companies_house_host}#{format_company_number(company_no)}" - @api_key = Rails.configuration.companies_house_api_key - end - - def company_name - load_company unless @company - - @company[:company_name] - end - - def registered_office_address_lines - load_company unless @company - - address = @company[:registered_office_address] - - [ - address[:address_line_1], - address[:address_line_2], - address[:locality], - address[:postal_code] - ].compact - end - - def company_status - load_company unless @company - - @company[:company_status] - end - - private - - def format_company_number(company_number) - company_number&.to_s&.upcase&.rjust(8, "0") - end - - def load_company - @company = - JSON.parse( - RestClient::Request.execute( - method: :get, - url: @company_url, - user: @api_key, - password: "" - ) - ).deep_symbolize_keys - rescue RestClient::ResourceNotFound, RestClient::NotFound - raise StandardError, "Failed to load company" - end -end diff --git a/lib/waste_carriers_engine.rb b/lib/waste_carriers_engine.rb index aa19677bd..fc05b6143 100644 --- a/lib/waste_carriers_engine.rb +++ b/lib/waste_carriers_engine.rb @@ -37,13 +37,13 @@ def initialize end def companies_house_host=(value) - DefraRuby::Validators.configure do |configuration| + DefraRuby::CompaniesHouse.configure do |configuration| configuration.companies_house_host = value end end def companies_house_api_key=(value) - DefraRuby::Validators.configure do |configuration| + DefraRuby::CompaniesHouse.configure do |configuration| configuration.companies_house_api_key = value end end diff --git a/spec/dummy/config/application.rb b/spec/dummy/config/application.rb index abf5f718e..463448a65 100644 --- a/spec/dummy/config/application.rb +++ b/spec/dummy/config/application.rb @@ -53,7 +53,7 @@ class Application < Rails::Application config.airbrake_key = ENV["WCRS_RENEWALS_AIRBRAKE_PROJECT_KEY"] || "dummy" # Companies House config - config.companies_house_host = ENV["WCRS_COMPANIES_HOUSE_URL"] || "https://api.companieshouse.gov.uk/company/" + config.companies_house_host = ENV["WCRS_COMPANIES_HOUSE_URL"] || "https://api.companieshouse.gov.uk/" config.companies_house_api_key = ENV["WCRS_COMPANIES_HOUSE_API_KEY"] # Paths diff --git a/spec/dummy/config/initializers/waste_carriers_engine.rb b/spec/dummy/config/initializers/waste_carriers_engine.rb index 93f0205a6..bcf1af4f4 100644 --- a/spec/dummy/config/initializers/waste_carriers_engine.rb +++ b/spec/dummy/config/initializers/waste_carriers_engine.rb @@ -2,7 +2,7 @@ WasteCarriersEngine.configure do |config| # Companies House API config - config.companies_house_host = ENV["WCRS_COMPANIES_HOUSE_URL"] || "https://api.companieshouse.gov.uk/company/" + config.companies_house_host = ENV["WCRS_COMPANIES_HOUSE_URL"] || "https://api.companieshouse.gov.uk/" config.companies_house_api_key = ENV["WCRS_COMPANIES_HOUSE_API_KEY"] # Airbrake config diff --git a/spec/forms/waste_carriers_engine/check_registered_company_name_forms_spec.rb b/spec/forms/waste_carriers_engine/check_registered_company_name_forms_spec.rb index c48bab2db..c3df53b9d 100644 --- a/spec/forms/waste_carriers_engine/check_registered_company_name_forms_spec.rb +++ b/spec/forms/waste_carriers_engine/check_registered_company_name_forms_spec.rb @@ -1,18 +1,23 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine RSpec.describe CheckRegisteredCompanyNameForm do let(:registered_company_name) { Faker::Company.name } let(:company_address) { ["10 Downing St", "Horizon House", "Bristol", "BS1 5AH"] } - let(:companies_house_service) { instance_double(DefraRubyCompaniesHouse) } + let(:companies_house_api) { instance_double(DefraRuby::CompaniesHouse::API) } + let(:companies_house_api_response) do + { + company_name: registered_company_name, + registered_office_address: company_address + } + end before do - allow(DefraRubyCompaniesHouse).to receive(:new).and_return(companies_house_service) - allow(companies_house_service).to receive(:company_name).and_return(registered_company_name) - allow(companies_house_service).to receive(:registered_office_address_lines).and_return(company_address) + allow(DefraRuby::CompaniesHouse::API).to receive(:new).and_return(companies_house_api) + allow(companies_house_api).to receive(:run).and_return(companies_house_api_response) end describe "#submit" do diff --git a/spec/forms/waste_carriers_engine/use_trading_name_forms_spec.rb b/spec/forms/waste_carriers_engine/use_trading_name_forms_spec.rb index 4e4a7750f..665c48b06 100644 --- a/spec/forms/waste_carriers_engine/use_trading_name_forms_spec.rb +++ b/spec/forms/waste_carriers_engine/use_trading_name_forms_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" module WasteCarriersEngine RSpec.describe UseTradingNameForm do diff --git a/spec/lib/defra_ruby_companies_house_spec.rb b/spec/lib/defra_ruby_companies_house_spec.rb deleted file mode 100644 index 21ede140a..000000000 --- a/spec/lib/defra_ruby_companies_house_spec.rb +++ /dev/null @@ -1,135 +0,0 @@ -# frozen_string_literal: true - -require "rails_helper" -require "defra_ruby_companies_house" - -RSpec.describe DefraRubyCompaniesHouse do - - let(:valid_company_no) { "00987654" } - let(:short_company_no) { "1987654" } - let(:invalid_company_no) { "-" } - - before do - # stub all calls to fail first... - stub_request(:get, /#{Rails.configuration.companies_house_host}*/).to_return( - status: 404 - ) - # ... then add a stub to cover valid company_no values - stub_request(:get, /#{Rails.configuration.companies_house_host}[A-Z\d]{8}/).to_return( - status: 200, - body: File.read("./spec/fixtures/files/companies_house_response.json") - ) - end - - describe "#company_name" do - subject(:company_name_for_number) { described_class.new(company_no).company_name } - - context "with an invalid company number" do - let(:company_no) { invalid_company_no } - - it "raises a standard error" do - expect { company_name_for_number }.to raise_error(StandardError) - end - end - - context "with a valid company number" do - let(:company_no) { valid_company_no } - - it "returns the company name" do - expect(company_name_for_number).to eq "BOGUS LIMITED" - end - end - - context "with a short company number" do - let(:company_no) { short_company_no } - - it "returns the company name" do - expect(company_name_for_number).to eq "BOGUS LIMITED" - end - end - - context "with a lower case company number" do - let(:company_no) { "xy12345z" } - - it "returns the company name" do - expect(company_name_for_number).to eq "BOGUS LIMITED" - end - end - - context "when the request times out" do - it "raises a standard error" do - expect { company_name_for_number }.to raise_error(StandardError) - end - end - - context "when the request returns an error" do - before { stub_request(:get, /#{Rails.configuration.companies_house_host}*/).to_raise(SocketError) } - - it "raises an exception" do - expect { company_name_for_number }.to raise_error(StandardError) - end - end - end - - describe "#registered_office_address_lines" do - subject(:address_for_company_number) { described_class.new(company_no).registered_office_address_lines } - - context "with an invalid company number" do - let(:company_no) { invalid_company_no } - - it "raises a standard error" do - expect { address_for_company_number }.to raise_error(StandardError) - end - end - - context "with a valid company number" do - let(:company_no) { valid_company_no } - - it "returns the address lines" do - expect(address_for_company_number).to eq ["R House", "Middle Street", "Thereabouts", "HD1 2BN"] - end - end - - context "with a short company number" do - let(:company_no) { short_company_no } - - it "returns the address lines" do - expect(address_for_company_number).to eq ["R House", "Middle Street", "Thereabouts", "HD1 2BN"] - end - end - end - - describe "#company_status" do - subject(:company_status) { described_class.new(company_no).company_status } - - context "with an invalid company number" do - let(:company_no) { invalid_company_no } - - it "raises a standard error" do - expect { company_status }.to raise_error(StandardError) - end - end - - context "when the request times out" do - it "raises a standard error" do - expect { company_status }.to raise_error(StandardError) - end - end - - context "when the request returns an error" do - before { stub_request(:get, /#{Rails.configuration.companies_house_host}*/).to_raise(SocketError) } - - it "raises a standard error" do - expect { company_status }.to raise_error(StandardError) - end - end - - context "with a valid company number" do - let(:company_no) { valid_company_no } - - it "returns the expected status" do - expect(company_status).to eq "active" - end - end - end -end diff --git a/spec/models/waste_carriers_engine/renewing_registration_workflow/renewal_information_form_spec.rb b/spec/models/waste_carriers_engine/renewing_registration_workflow/renewal_information_form_spec.rb index 5ef4b7c01..18f093103 100644 --- a/spec/models/waste_carriers_engine/renewing_registration_workflow/renewal_information_form_spec.rb +++ b/spec/models/waste_carriers_engine/renewing_registration_workflow/renewal_information_form_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine RSpec.describe RenewingRegistration do @@ -17,13 +17,19 @@ module WasteCarriersEngine let(:tier) { WasteCarriersEngine::Registration::UPPER_TIER } let(:business_type) { nil } let(:location) { "england" } - let(:defra_ruby_companies_house) { instance_double(DefraRubyCompaniesHouse) } + let(:companies_house_api) { instance_double(DefraRuby::CompaniesHouse::API) } + let(:companies_house_api_reponse) do + { + company_status: + } + end + let(:company_status) { "active" } let(:company_number) { "12345678" } before do - allow(DefraRubyCompaniesHouse).to receive(:new).and_return(defra_ruby_companies_house) - allow(defra_ruby_companies_house).to receive(:company_status).and_return(company_status) + allow(DefraRuby::CompaniesHouse::API).to receive(:new).and_return(companies_house_api) + allow(companies_house_api).to receive(:run).and_return(companies_house_api_reponse) end describe "#workflow_state" do @@ -119,7 +125,7 @@ module WasteCarriersEngine context "when Companies House Api returns an error" do before do - allow(defra_ruby_companies_house).to receive(:company_status).and_raise(StandardError) + allow(companies_house_api).to receive(:run).and_raise(StandardError) end context "when the business type is limitedCompany with non-UK company number" do diff --git a/spec/requests/waste_carriers_engine/check_registered_company_name_forms_spec.rb b/spec/requests/waste_carriers_engine/check_registered_company_name_forms_spec.rb index 08ac34544..f6ca792c4 100644 --- a/spec/requests/waste_carriers_engine/check_registered_company_name_forms_spec.rb +++ b/spec/requests/waste_carriers_engine/check_registered_company_name_forms_spec.rb @@ -1,19 +1,24 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine RSpec.describe "CheckRegisteredCompanyNameForms" do let(:company_name) { Faker::Company.name } let(:company_address) { ["10 Downing St", "Horizon House", "Bristol", "BS1 5AH"] } - let(:companies_house_service) { instance_double(DefraRubyCompaniesHouse) } + let(:companies_house_api) { instance_double(DefraRuby::CompaniesHouse::API) } + let(:companies_house_api_reponse) do + { + company_name:, + registered_office_address: company_address + } + end before do - allow(DefraRubyCompaniesHouse).to receive(:new).and_return(companies_house_service) - allow(companies_house_service).to receive(:company_name).and_return(company_name) - allow(companies_house_service).to receive(:registered_office_address_lines).and_return(company_address) + allow(DefraRuby::CompaniesHouse::API).to receive(:new).and_return(companies_house_api) + allow(companies_house_api).to receive(:run).and_return(companies_house_api_reponse) end include_examples "GET flexible form", "check_registered_company_name_form" @@ -42,7 +47,7 @@ module WasteCarriersEngine context "when the company house API is down" do before do - allow(companies_house_service).to receive(:company_name).and_raise(StandardError) + allow(companies_house_api).to receive(:run).and_raise(StandardError) end it "raises an error" do diff --git a/spec/requests/waste_carriers_engine/renewal_information_forms_spec.rb b/spec/requests/waste_carriers_engine/renewal_information_forms_spec.rb index ad851cfcb..f000fce21 100644 --- a/spec/requests/waste_carriers_engine/renewal_information_forms_spec.rb +++ b/spec/requests/waste_carriers_engine/renewal_information_forms_spec.rb @@ -1,15 +1,20 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" module WasteCarriersEngine RSpec.describe "RenewalInformationForms" do - let(:defra_ruby_companies_house) { instance_double(DefraRubyCompaniesHouse) } + let(:defra_ruby_companies_api) { instance_double(DefraRuby::CompaniesHouse::API) } + let(:companies_house_api_response) do + { + company_status: "active" + } + end before do - allow(DefraRubyCompaniesHouse).to receive(:new).and_return(defra_ruby_companies_house) - allow(defra_ruby_companies_house).to receive(:company_status).and_return("active") + allow(DefraRuby::CompaniesHouse::API).to receive(:new).and_return(defra_ruby_companies_api) + allow(defra_ruby_companies_api).to receive(:run).and_return(companies_house_api_response) end include_examples "GET flexible form", "renewal_information_form" diff --git a/spec/services/waste_carriers_engine/refresh_companies_house_name_service_spec.rb b/spec/services/waste_carriers_engine/refresh_companies_house_name_service_spec.rb index 423cf20b5..f6986ffed 100644 --- a/spec/services/waste_carriers_engine/refresh_companies_house_name_service_spec.rb +++ b/spec/services/waste_carriers_engine/refresh_companies_house_name_service_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "rails_helper" -require "defra_ruby_companies_house" +require "defra_ruby/companies_house" RSpec.describe WasteCarriersEngine::RefreshCompaniesHouseNameService do @@ -11,12 +11,16 @@ let(:registration) { create(:registration, :has_required_data, registered_company_name: old_registered_name) } let(:reg_identifier) { registration.reg_identifier } let(:companies_house_name) { new_registered_name } - let(:drch_instance) { instance_double(DefraRubyCompaniesHouse) } + let(:companies_house_api) { instance_double(DefraRuby::CompaniesHouse::API) } + let(:companies_house_api_reponse) do + { + company_name: companies_house_name + } + end before do - allow(DefraRubyCompaniesHouse).to receive(:new).and_return(drch_instance) - allow(drch_instance).to receive(:load_company).and_return(true) - allow(drch_instance).to receive(:company_name).and_return(companies_house_name) + allow(DefraRuby::CompaniesHouse::API).to receive(:new).and_return(companies_house_api) + allow(companies_house_api).to receive(:run).and_return(companies_house_api_reponse) end context "with no previous companies house name" do diff --git a/waste_carriers_engine.gemspec b/waste_carriers_engine.gemspec index bdd3d2a59..e4b999966 100644 --- a/waste_carriers_engine.gemspec +++ b/waste_carriers_engine.gemspec @@ -48,7 +48,7 @@ Gem::Specification.new do |s| s.add_dependency "high_voltage", "~> 3.1.2" # Validations - s.add_dependency "defra_ruby_validators", "~> 2.6" + s.add_dependency "defra_ruby_validators", "~> 3.0" s.add_dependency "uk_postcode", "~> 2.1.8" s.add_dependency "defra_ruby_govpay" @@ -60,6 +60,9 @@ Gem::Specification.new do |s| # EA Address Facade v1) s.add_dependency "defra_ruby_address", "~> 0.1.0" + # # Used to access the Companies House API for company details validation + s.add_dependency "defra_ruby_companies_house" + # Used as part of testing. When enabled adds a /last-email route from which # details of the last email sent by the app can be accessed s.add_dependency "defra_ruby_email", "~> 1.3.0"