HMRC: Changed :uk options handling

Reverted :uk option back to default to false
Changed option :live to :sandbox
Added warning if :uk is set to true

README.md

@@ -80,7 +80,6 @@ To keep backwards compatibility lookups of UK VAT numbers against the HMRC API a
 Valvat::Lookup.validate(
   "GB553557881",
   uk: {
-    live: true,
     client_id: '<client_id>',
     client_secret: '<client_secret>'
   }
@@ -201,7 +200,7 @@ Valvat.configure(
   raise_error: true,
   http: { read_timeout: 5 },
   uk: {
-    live: true, # Live mode for switching between Sandbox and Production API (TRUE by default)
+    sandbox: false, # Use production mode (the default)
     client_id: <client_id> # Required for OAuth 2.0 Authentication
     client_secret: <client_secret> # Required for OAuth 2.0 Authentication
   }

lib/valvat/configuration.rb

@@ -43,16 +43,11 @@ class Configuration
       # Use lookup via HMRC for VAT numbers from the UK
       # if set to false lookup will always return false for UK VAT numbers
       # HMRC options:
-      # - API credentials for OAuth 2.0 Authentication
-      # - Live mode for switching between Sandbox and Production API:
-      # * TRUE - Production API
-      # * FALSE - Sandbox API
+      # :client_id and :client_secret     API credentials for OAuth 2.0 Authentication
+      # :sandbox                          Use sandboxed instead of production API (defaults to false)
       # See more details https://developer.service.hmrc.gov.uk/api-documentation/docs/development-practices
-      uk: {
-        live: true,
-        client_id: nil,
-        client_secret: nil
-      }.freeze,
+      #
+      uk: false,
 
       # Rate limit for Lookup and HMRC authentication requests
       rate_limit: 5

lib/valvat/hmrc/access_token.rb

@@ -14,10 +14,12 @@ class AccessToken
       GRANT_TYPE = 'client_credentials'
 
       def initialize(options = {})
-        @client_id = options.dig(:uk, :client_id).to_s
-        @client_secret = options.dig(:uk, :client_secret).to_s
+        uk_options = options[:uk].is_a?(Hash) ? options[:uk] : {}
+
+        @client_id = uk_options[:client_id].to_s
+        @client_secret = uk_options[:client_secret].to_s
         @rate_limit = options[:rate_limit]
-        @endpoint_uri = URI(options.dig(:uk, :live) ? PRODUCTION_ENDPOINT_URL : SANDBOX_ENDPOINT_URL)
+        @endpoint_uri = URI(uk_options[:sandbox] ? SANDBOX_ENDPOINT_URL : PRODUCTION_ENDPOINT_URL)
       end
 
       def self.fetch(options = {})

lib/valvat/lookup/hmrc.rb

@@ -17,9 +17,7 @@ class HMRC < Base
       }.freeze
 
       def perform
-        return { valid: false } unless @options[:uk].is_a?(Hash) &&
-                                       @options.dig(:uk, :client_id) &&
-                                       @options.dig(:uk, :client_secret)
+        return { valid: false } unless @options[:uk].is_a?(Hash)
 
         parse(fetch(endpoint_uri).body)
       rescue Valvat::HMRC::AccessToken::Error => e
@@ -31,7 +29,7 @@ def perform
       def endpoint_uri
         endpoint = "/#{@vat.to_s_wo_country}"
         endpoint += "/#{@requester.to_s_wo_country}" if @requester
-        endpoint_url = @options.dig(:uk, :live) ? PRODUCTION_ENDPOINT_URL : SANDBOX_ENDPOINT_URL
+        endpoint_url = @options.dig(:uk, :sandbox) ? SANDBOX_ENDPOINT_URL : PRODUCTION_ENDPOINT_URL
         URI.parse(endpoint_url + endpoint)
       end
 

lib/valvat/options.rb

@@ -18,6 +18,8 @@ def initialize(options, silence: false)
           @options[key] ||= @options[deprecated]
         end
       end
+
+      check_uk_key
     end
 
     def [](key)
@@ -27,6 +29,15 @@ def [](key)
     def dig(*keys)
       @options.dig(*keys).nil? ? Valvat.config.dig(*keys) : @options.dig(*keys)
     end
+
+    private
+
+    def check_uk_key
+      return if @options[:uk] != true || silence
+
+      puts 'DEPRECATED: The option :uk is not allowed to be set to `true` anymore. ' \
+           'Using the HMRC API requires authentication credentials.'
+    end
   end
 
   def self.Options(options)

spec/valvat/hmrc/access_token_spec.rb

@@ -6,7 +6,7 @@
   subject(:fetch_access_token) { described_class.fetch(Valvat::Options(options)) }
 
   let!(:options) { { uk: uk } }
-  let(:uk) { { live: false } }
+  let(:uk) { { sandbox: true } }
 
   shared_examples 'return access token' do
     it 'return access token' do

spec/valvat/lookup/hmrc_spec.rb

@@ -6,7 +6,7 @@
   subject(:lookup) { described_class.new("GB#{vat_number}", options).perform }
 
   let(:options) { { uk: uk } }
-  let(:uk) { { live: false, client_id: '<client_id>', client_secret: '<client_secret>' } }
+  let(:uk) { { sandbox: true, client_id: '<client_id>', client_secret: '<client_secret>' } }
   let(:vat_number) { '553557881' }
   let(:authentication_response) do
     {
@@ -81,7 +81,7 @@
   end
 
   after do
-    Valvat.configure(uk: uk.merge(live: true))
+    Valvat.configure(uk: false)
   end
 
   context 'with valid data' do
@@ -127,6 +127,7 @@
       it 'returns error' do
         expect { lookup }.not_to raise_error
         expect(lookup[:error]).to be_a(Valvat::HMRC::AccessToken::Error)
+        expect(lookup[:valid]).to be_nil
       end
     end
 
@@ -148,9 +149,7 @@
     context 'when missing Authentication credentials (the default)' do
       let(:uk) { super().merge(client_id: nil, client_secret: nil) }
 
-      it 'returns false' do
-        expect(lookup[:valid]).to be(false)
-      end
+      include_examples 'returns error'
     end
 
     context 'when Authentication failed' do

spec/valvat/lookup_spec.rb

@@ -4,7 +4,7 @@
 
 describe Valvat::Lookup do
   shared_context 'with hmrc configuration' do
-    let(:uk) { { live: false, client_id: '<client_id>', client_secret: '<client_secret>' } }
+    let(:uk) { { sandbox: true, client_id: '<client_id>', client_secret: '<client_secret>' } }
     let(:vat_number) { nil }
     let(:authentication_response) do
       {
@@ -63,7 +63,7 @@
     end
 
     after do
-      Valvat.configure(uk: uk.merge(live: true))
+      Valvat.configure(uk: false)
     end
   end
 

spec/valvat/options_spec.rb

@@ -5,17 +5,21 @@
 describe Valvat::Options do
   describe '#[]' do
     it 'returns global config by default' do
-      expect(described_class.new({})[:uk]).to include(
-        {
-          live: true,
-          client_id: '<client_id>',
-          client_secret: '<client_secret>'
-        }
-      )
+      expect(described_class.new({})[:uk]).to be(false)
     end
 
     it 'returns option if set' do
-      expect(described_class.new({ uk: false })[:uk]).to be(false)
+      expect(described_class.new({ uk: {
+                                   sandbox: true,
+                                   client_id: '<client_id>',
+                                   client_secret: '<client_secret>'
+                                 } })[:uk]).to include(
+                                   {
+                                     sandbox: true,
+                                     client_id: '<client_id>',
+                                     client_secret: '<client_secret>'
+                                   }
+                                 )
     end
 
     context 'when options contains deprecated key' do