Skip to content
This repository has been archived by the owner on Jun 24, 2021. It is now read-only.

Commit

Permalink
Replace Ibanomat with Ibanizator
Browse files Browse the repository at this point in the history
We're making the assumption that the BIC that we get back from the Ibanizator is "good enough".

There is more to read about this at:
softwareinmotion/ibanizator#10

This reverts commit 558d56e.
  • Loading branch information
tskogberg committed Nov 2, 2015
1 parent 558d56e commit e67a8cd
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 45 deletions.
3 changes: 2 additions & 1 deletion banktools-de.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ Gem::Specification.new do |spec|
spec.require_paths = ["lib"]

spec.add_dependency "attr_extras", ">= 4"
spec.add_dependency "ibanomat" # Convert BLZ to IBAN/BIC
spec.add_dependency "memoit"
spec.add_dependency "ibanizator" # Convert BLZ to IBAN and lookup BIC from IBAN
spec.add_development_dependency "bundler", "~> 1.5"
spec.add_development_dependency "rake"
spec.add_development_dependency "rspec"
Expand Down
41 changes: 22 additions & 19 deletions lib/banktools-de/iban_bic.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
require "ibanomat"
require "memoit"
require "ibanizator"
require "attr_extras/explicit"

class BankTools::DE::IbanBicConverter
extend AttrExtras.mixin

class CouldNotConvertError < StandardError; end
class ServiceUnavailable < StandardError; end
class UnknownError < StandardError; end
class CouldNotConvertIbanError < StandardError; end
class CouldNotFindBicError < StandardError; end

class IbanBic
extend AttrExtras.mixin
Expand All @@ -16,27 +16,30 @@ class IbanBic
method_object [ :blz!, :account! ]

def call
account_data = get_account_data
build_result(account_data)
IbanBic.new(iban, bic)
end

private

def get_account_data
Ibanomat.find(bank_code: blz, bank_account_number: account)
rescue RestClient::RequestTimeout, Errno::ECONNREFUSED, RestClient::SSLCertificateNotVerified
raise ServiceUnavailable
rescue
raise UnknownError
def iban
iban_object.iban_string
end

def bic
iban_object.extended_data.bic
rescue Ibanizator::BankDb::BankNotFoundError => e
raise CouldNotFindBicError.new(e.message)
end

def build_result(account_data)
# Non-"00" values represent an Ibanomat warning or error.
return_code = account_data.fetch(:return_code)
raise CouldNotConvertError unless return_code == "00"
memoize \
def iban_object
iban_string = Ibanizator.new.calculate_iban(country_code: :de, bank_code: blz, account_number: account)
iban = Ibanizator.iban_from_string(iban_string)

iban = account_data.fetch(:iban)
bic = account_data.fetch(:bic)
IbanBic.new(iban, bic)
if Ibanizator.iban_from_string(iban).valid?

This comment has been minimized.

Copy link
@soma

soma Nov 3, 2015

Member

Hm, Isn’t this line kinda duplicated with the above line #37? Or its sent into that method twice? Looks weird!

This comment has been minimized.

Copy link
@tskogberg

tskogberg Nov 3, 2015

Author Member

Yes, missed that. iban.valid? it is.

iban
else
raise CouldNotConvertIbanError.new("Invalid IBAN: #{iban.iban_string}")
end
end
end
2 changes: 1 addition & 1 deletion lib/banktools-de/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module BankTools
module DE
VERSION = "2.0.1"
VERSION = "2.1.0"
end
end
4 changes: 2 additions & 2 deletions spec/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ def expect_normalization(from, unto)

describe BankTools::DE::Account, "#valid?" do
it "is true with no errors" do
expect(BankTools::DE::Account.new("12").valid?).to be_true
expect(BankTools::DE::Account.new("12").valid?).to be_truthy

This comment has been minimized.

Copy link
@soma

soma Nov 3, 2015

Member

Why does this not return “true”, I think it should, since its a ..? - method.

This comment has been minimized.

Copy link
@tskogberg

tskogberg Nov 3, 2015

Author Member

Yes, we should do a be true check here. Will change. 👍

end

it "is false with errors" do
expect(BankTools::DE::Account.new("1").valid?).to be_false
expect(BankTools::DE::Account.new("1").valid?).to be_falsy
end
end

Expand Down
4 changes: 2 additions & 2 deletions spec/blz_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ def expect_normalization(from, unto)

describe BankTools::DE::BLZ, "#valid?" do
it "is true with no errors" do
expect(BankTools::DE::BLZ.new("123 456 78").valid?).to be_true
expect(BankTools::DE::BLZ.new("123 456 78").valid?).to be_truthy

This comment has been minimized.

Copy link
@soma

soma Nov 3, 2015

Member

Same as above, I think this method should made sure its either true or false.

This comment has been minimized.

Copy link
@tskogberg

tskogberg Nov 3, 2015

Author Member

Same as above fix wise.

end

it "is false with errors" do
expect(BankTools::DE::BLZ.new("1").valid?).to be_false
expect(BankTools::DE::BLZ.new("1").valid?).to be_falsy
end
end

Expand Down
32 changes: 12 additions & 20 deletions spec/iban_bic_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,31 @@
require "banktools-de/iban_bic"

describe BankTools::DE::IbanBicConverter, ".call" do
let(:blz) { "abc" }
let(:account) { "def" }

it "returns the BIC and IBAN" do
allow(Ibanomat).to receive(:find).and_return(iban: "some iban", bic: "some bic", return_code: "00")
blz = "25050180"
account = "39370089"

actual = BankTools::DE::IbanBicConverter.call(blz: blz, account: account)

expect(actual.iban).to eq("some iban")
expect(actual.bic).to eq("some bic")
end

it "raises CouldNotConvertError on conversion errors" do
allow(Ibanomat).to receive(:find).and_return(return_code: "1")

expect {
BankTools::DE::IbanBicConverter.call(blz: blz, account: account)
}.to raise_error(BankTools::DE::IbanBicConverter::CouldNotConvertError)
expect(actual.iban).to eq("DE53250501800039370089")
expect(actual.bic).to eq("SPKHDE2HXXX")
end

it "raises ServiceUnavailable when the service is down" do
allow(Ibanomat).to receive(:find).and_raise(RestClient::RequestTimeout)
it "raises CouldNotConvertIbanError on IBAN conversion errors" do
blz = "abc"
account = "def"

expect {
BankTools::DE::IbanBicConverter.call(blz: blz, account: account)
}.to raise_error(BankTools::DE::IbanBicConverter::ServiceUnavailable)
}.to raise_error(BankTools::DE::IbanBicConverter::CouldNotConvertIbanError)
end

it "raises UnknownError on other errors" do
allow(Ibanomat).to receive(:find).and_raise("unknown error")
it "raises CouldNotFindBicError on find BIC errors" do
blz = "21050171"
account = "12345678"

expect {
BankTools::DE::IbanBicConverter.call(blz: blz, account: account)
}.to raise_error(BankTools::DE::IbanBicConverter::UnknownError)
}.to raise_error(BankTools::DE::IbanBicConverter::CouldNotFindBicError)
end
end

2 comments on commit e67a8cd

@soma
Copy link
Member

@soma soma commented on e67a8cd Nov 3, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice how small a change this was!

@tskogberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

Please sign in to comment.