-
Notifications
You must be signed in to change notification settings - Fork 0
Add header record using Fixy and DryRB Validation gems #5
Changes from all commits
d42619e
2cd34cc
1bcde41
860fea8
013c5d5
6d3a77d
3134987
0301fc8
6ad1f19
f45fa5f
02f2207
a2dfdf5
f34f6e1
c3e20e1
16ca73c
1d1271a
629c35f
8767d78
f79a8b8
07a98a1
d983493
7fd5cd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,15 +3,26 @@ require: | |
|
|
||
| AllCops: | ||
| NewCops: enable | ||
| TargetRubyVersion: 2.6 | ||
| TargetRubyVersion: 2.7 | ||
|
|
||
| Layout/LineLength: | ||
| Max: 120 | ||
|
|
||
| Lint/MissingSuper: | ||
| Enabled: false | ||
|
|
||
| Metrics/BlockLength: | ||
| Enabled: false | ||
|
|
||
| Style/Documentation: | ||
| Enabled: false | ||
|
|
||
| Style/StringLiterals: | ||
| Enabled: true | ||
| EnforcedStyle: double_quotes | ||
|
|
||
| Style/StringLiteralsInInterpolation: | ||
| Enabled: true | ||
| EnforcedStyle: double_quotes | ||
| RSpec/ExampleLength: | ||
| Enabled: false | ||
|
|
||
| Layout/LineLength: | ||
| Max: 120 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probs copy the current rubocop yml we have in zfp and split(main zepto) repos |
||
| RSpec/MultipleExpectations: | ||
| Enabled: false | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,23 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| require_relative "mt9/version" | ||
| require "dry-validation" | ||
| require "fixy" | ||
|
|
||
| require "mt9/values" | ||
| require "mt9/validators/base_contract" | ||
| require "mt9/validators/header_record_contract" | ||
| require "mt9/base_record" | ||
| require "mt9/header_record" | ||
| require "mt9/version" | ||
|
|
||
| module MT9 | ||
| class Error < StandardError; end | ||
| # Your code goes here... | ||
| class ValidationError < StandardError | ||
| attr_reader :result | ||
|
|
||
| def initialize(result) | ||
| @result = result | ||
| errors = result.errors(full: true).messages | ||
| super("Validation failed: #{errors.join(', ')}") | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module MT9 | ||
| class BaseRecord < Fixy::Record | ||
| include Fixy::Formatter::Alphanumeric | ||
|
|
||
| SPACE = " " | ||
|
|
||
| set_record_length 160 | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||
| # frozen_string_literal: true | ||||||
|
|
||||||
| module MT9 | ||||||
| class HeaderRecord < BaseRecord | ||||||
| set_line_ending Fixy::Record::LINE_ENDING_CRLF | ||||||
|
|
||||||
| attr_reader :file_type, :account_number, :due_date, :client_short_name | ||||||
|
|
||||||
| field :file_type, 2, "1-2", :alphanumeric | ||||||
| field :account_number, 16, "3-18", :alphanumeric | ||||||
| field :due_date, 8, "19-26", :alphanumeric | ||||||
| field :filler5, 5, "27-31", :alphanumeric | ||||||
| field :client_short_name, 20, "32-51", :alphanumeric | ||||||
| field :filler109, 109, "52-160", :alphanumeric | ||||||
|
|
||||||
| field_value :filler5, SPACE * 5 | ||||||
| field_value :filler109, SPACE * 109 | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lots of "magic numbers" here. I would try to name them here and make them constants. i.e. 109 could be
Suggested change
|
||||||
|
|
||||||
| def initialize(...) | ||||||
| validator = Validators::HeaderRecordContract.new | ||||||
| result = validator.call(...) | ||||||
| raise MT9::ValidationError, result unless result.success? | ||||||
|
|
||||||
| result.to_h.each do |key, value| | ||||||
| instance_variable_set("@#{key}", value) | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,12 @@ | ||||||
| # frozen_string_literal: true | ||||||
|
|
||||||
| module MT9 | ||||||
| module Validators | ||||||
| class BaseContract < Dry::Validation::Contract | ||||||
| register_macro(:is_account_number?) do | ||||||
| key.failure("must be 15 or 16 numeric characters") unless /^(\d{15}|\d{16})$/.match(value) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
nitpick, but shorter :) See: https://rubular.com/r/PAUYXnL7M2LDDv |
||||||
| key.failure("must not start with a reserved bank code") if value.start_with?(*MT9::Values::RESERVED_BANK_CODES) | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,20 @@ | ||||||
| # frozen_string_literal: true | ||||||
|
|
||||||
| module MT9 | ||||||
| module Validators | ||||||
| class HeaderRecordContract < BaseContract | ||||||
| schema do | ||||||
| required(:file_type).filled(:string, included_in?: MT9::Values::FILE_TYPES) | ||||||
| required(:account_number).filled(:string) | ||||||
| required(:due_date).filled(:string) | ||||||
| optional(:client_short_name).value(:string, size?: 0..20) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering whether we want to make this required, even though this isn't strictly the case when looking at the docs? It'd be good to know whether it's a big no-no to send a file without a short name & add a little protection if so
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be a @henripryde400 question to ask to the ASB guys |
||||||
| end | ||||||
|
|
||||||
| rule(:account_number).validate(:is_account_number?) | ||||||
|
|
||||||
| rule(:due_date) do | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking we should also have a check to see whether a valid due date has been supplied.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, to simplify validation we could change |
||||||
| key.failure("must be 6 or 8 numeric characters") unless /^(\d{6}|\d{8})$/.match(value) | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This notation is used for a range (i.e 6 to 8 digits). Here we want either 6 or 8, but not 7 digits |
||||||
| end | ||||||
| end | ||||||
| end | ||||||
| end | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module MT9 | ||
| module Values | ||
| RESERVED_BANK_CODES = %w[99].freeze | ||
|
|
||
| DIRECT_CREDIT = "12" | ||
| DIRECT_DEBIT = "20" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious why they are strings and not integers?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought process here was that we don't perform any integer-like operations on these numbers and these are identifiers, just like the account number, and so are better represented as strings |
||
| FILE_TYPES = [DIRECT_CREDIT, DIRECT_DEBIT].freeze | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| 12123113000631400 071020 asbd 2 | ||
| 13123113000214500 0510000000102PayeeName 0 000000000000Payeecde0 PayeeRef0 PayeePrt0 PayerName Payercde PayerRef PayerPrt | ||
| 13123113000214500 0510000000201PayeeName 1 000000000000Payeecde1 PayeeRef1 PayeePrt1 PayerName Payercde PayerRef PayerPrt | ||
| 139962260004290 303 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| 20123113000214500 031121 test co | ||
| 13123009000012300 0000000000100PayerName 0 000000000000Payercde0 PayerRef0 PayerPrt0 PayeeName Payeecde PayeeRef PayeePrt | ||
| 13123009000012300 0000000000200PayerName 1 000000000000Payercde1 PayerRef1 PayerPrt1 PayeeName Payeecde PayeeRef PayeePrt | ||
| 13123009000012300 0000000000300PayerName 2 000000000000Payercde2 PayerRef2 PayerPrt2 PayeeName Payeecde PayeeRef PayeePrt | ||
| 139990270000369 600 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| RSpec.describe MT9::HeaderRecord do | ||
| let(:header_record) do | ||
| { | ||
| file_type: "12", | ||
| account_number: "123456789012345", | ||
| due_date: "251220", | ||
| client_short_name: "ACME Pty Ltd" | ||
| } | ||
| end | ||
|
|
||
| describe "when generating header record" do | ||
| subject(:result) { described_class.new(**header_record).generate } | ||
|
|
||
| it "creates correctly formatted string" do | ||
| expect(result).to eq("12123456789012345 251220 ACME Pty Ltd \r\n") # rubocop:disable Layout/LineLength | ||
| end | ||
|
|
||
| it "creates correctly formatted string with a long account number and due date" do | ||
| header_record[:account_number] = "1234567890123456" | ||
| header_record[:due_date] = "25122020" | ||
|
|
||
| expect(result).to eq("12123456789012345625122020 ACME Pty Ltd \r\n") # rubocop:disable Layout/LineLength | ||
| end | ||
| end | ||
|
|
||
| it "raises ValidationError with an incorrect header record" do | ||
| header_record[:file_type] = "999" | ||
|
|
||
| expect { described_class.new(**header_record) }.to \ | ||
| raise_error(MT9::ValidationError, "Validation failed: file_type must be one of: 12, 20") | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| RSpec.describe MT9::Validators::HeaderRecordContract do | ||
| subject(:result) { described_class.new.call(header_record) } | ||
|
|
||
| let(:header_record) do | ||
| { | ||
| file_type: "12", | ||
| account_number: "123456789012345", | ||
| due_date: "120321", | ||
| client_short_name: "ACME Pty Ltd" | ||
| } | ||
| end | ||
|
|
||
| it "validates a correct header record" do | ||
| expect(result).to be_success | ||
| end | ||
|
|
||
| it "validates with a 16 digit account number" do | ||
| header_record[:account_number] = "1234567890123456" | ||
| expect(result).to be_success | ||
| end | ||
|
|
||
| it "validates with a 8 digit due date" do | ||
| header_record[:due_date] = "12032021" | ||
| expect(result).to be_success | ||
| end | ||
|
|
||
| it "validates a correct header record with no client short name" do | ||
| header_record.delete(:client_short_name) | ||
| expect(result).to be_success | ||
| end | ||
|
|
||
| describe "with an incorrect header record" do | ||
| it "validates an invalid file type" do | ||
| header_record[:file_type] = "999" | ||
| expect(result.errors[:file_type]).to eq(["must be one of: 12, 20"]) | ||
| end | ||
|
|
||
| it "validates a reserved bank code" do | ||
| header_record[:account_number] = "991234567890123" | ||
| expect(result.errors[:account_number]).to eq(["must not start with a reserved bank code"]) | ||
| end | ||
|
|
||
| it "validates a short account number" do | ||
| header_record[:account_number] = "123456790" | ||
| expect(result.errors[:account_number]).to eq(["must be 15 or 16 numeric characters"]) | ||
| end | ||
|
|
||
| it "validates a long account number" do | ||
| header_record[:account_number] = "12345678901234567" | ||
| expect(result.errors[:account_number]).to eq(["must be 15 or 16 numeric characters"]) | ||
| end | ||
|
|
||
| it "validates a short due date" do | ||
| header_record[:due_date] = "213" | ||
| expect(result.errors[:due_date]).to eq(["must be 6 or 8 numeric characters"]) | ||
| end | ||
|
|
||
| it "validates a 7 digit due date" do | ||
| header_record[:due_date] = "1234567" | ||
| expect(result.errors[:due_date]).to eq(["must be 6 or 8 numeric characters"]) | ||
| end | ||
|
|
||
| it "validates a long due date" do | ||
| header_record[:due_date] = "202105114" | ||
| expect(result.errors[:due_date]).to eq(["must be 6 or 8 numeric characters"]) | ||
| end | ||
|
|
||
| it "validates a long client short name" do | ||
| header_record[:client_short_name] = "ACME Transactions and Payments Proprietary Limited" | ||
| expect(result.errors[:client_short_name]).to eq(["length must be within 0 - 20"]) | ||
| end | ||
|
|
||
| it "validates a nil client short name" do | ||
| header_record[:client_short_name] = nil | ||
| expect(result.errors[:client_short_name]).to eq(["must be a string"]) | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason we're not in 3.0.3 land?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was the version specified in the gemspec, and I think it's best if the gem can support as wide a range of Ruby versions as possible (albeit support for 2.7 will be dropped in 10 months)