From b65582377683ce867d490af362af24b436e0644a Mon Sep 17 00:00:00 2001 From: nikz Date: Tue, 29 May 2018 22:01:05 +0100 Subject: [PATCH] Refactors Report row handling, fixes a couple of bugs * Accessing report rows via #column_1 etc will be deprecated, please use the column title (e.g `report.rows.first.account`) or an array/hash-style accessor (e.g `report.rows.first[0]` or `report.rows.first["Account"]` * Fixed a bug where we weren't including all Sections in the report object * Added the ability to get the section name for a Row by calling #section_name on the Row --- lib/xero_gateway/report.rb | 27 +++++++++------- lib/xero_gateway/report/cell.rb | 4 +-- lib/xero_gateway/report/row.rb | 57 +++++++++++++++++++++++++++++++++ test/unit/report/row_test.rb | 45 ++++++++++++++++++++++++++ test/unit/report_test.rb | 49 +++++++++++++++------------- xero_gateway.gemspec | 1 + 6 files changed, 147 insertions(+), 36 deletions(-) create mode 100644 lib/xero_gateway/report/row.rb create mode 100644 test/unit/report/row_test.rb diff --git a/lib/xero_gateway/report.rb b/lib/xero_gateway/report.rb index f197ab95..9c8d1784 100644 --- a/lib/xero_gateway/report.rb +++ b/lib/xero_gateway/report.rb @@ -1,4 +1,5 @@ require_relative './report/cell' +require_relative './report/row' module XeroGateway class Report @@ -10,6 +11,8 @@ class Report attr_accessor :report_id, :report_name, :report_type, :report_titles, :report_date, :updated_at, :body, :column_names + alias :rows :body + def initialize(params={}) @errors ||= [] @report_titles ||= [] @@ -36,9 +39,9 @@ def from_xml(report_element) when 'ReportDate' then report.report_date = Date.parse(element.text) when 'UpdatedDateUTC' then report.updated_at = parse_date_time_utc(element.text) when 'Rows' - report.column_names ||= find_body_column_names(element) - each_row_content(element) do |content_hash| - report.body << OpenStruct.new(content_hash) + report.column_names ||= find_body_column_names(element) + each_row_content(element) do |row| + report.body << row end end end @@ -48,16 +51,16 @@ def from_xml(report_element) private def each_row_content(xml_element, &block) - column_names = find_body_column_names(xml_element).keys - xpath_body = REXML::XPath.first(xml_element, "//RowType[text()='Section']").parent - rows_contents = [] - xpath_body.elements.each("Rows/Row") do |xpath_cells| - values = find_body_cell_values(xpath_cells) - content_hash = Hash[column_names.zip values] - rows_contents << content_hash - yield content_hash if block_given? + column_names = find_body_column_names(xml_element).values + report_sections = REXML::XPath.each(xml_element, "//RowType[text()='Section']/parent::Row") + + report_sections.each do |section_row| + section_name = section_row.get_elements("Title").first.try(:text) + section_row.elements.each("Rows/Row") do |xpath_cells| + values = find_body_cell_values(xpath_cells) + yield Row.new(column_names, values, section_name) + end end - rows_contents end def each_title(xml_element, &block) diff --git a/lib/xero_gateway/report/cell.rb b/lib/xero_gateway/report/cell.rb index b1a95fe0..e1c5410a 100644 --- a/lib/xero_gateway/report/cell.rb +++ b/lib/xero_gateway/report/cell.rb @@ -18,8 +18,8 @@ class Cell < SimpleDelegator attr_reader :attributes, :value def initialize(value, new_attributes = {}) - @value = value - @attributes = new_attributes + @value = value + @attributes = new_attributes super(value) end end diff --git a/lib/xero_gateway/report/row.rb b/lib/xero_gateway/report/row.rb new file mode 100644 index 00000000..23901118 --- /dev/null +++ b/lib/xero_gateway/report/row.rb @@ -0,0 +1,57 @@ +module XeroGateway + class Report + class Row + COLUMN_METHOD_NAME_RE = /^column\_([0-9])+$/ + + attr_accessor :section_name + + def initialize(column_titles, columns, section_name = nil) + @columns = columns + @column_titles = column_titles + @column_titles_underscored = column_titles.map(&:to_s).map(&:underscore) + @section_name = section_name + end + + def [](key) + return @columns[key] if key.is_a?(Integer) + + [ @column_titles, @column_titles_underscored ].each do |names| + if index = names.index(key.to_s) + return @columns[index] + end + end + + nil + end + + def method_missing(method_name, *args, &block) + if method_name =~ COLUMN_METHOD_NAME_RE + # support column_#{n} style deprecated API + ActiveSupport::Deprecation.warn("XeroGateway: The #column_n API for accessing report cells will be deprecated in a future version. Please use the underscored column title, a hash or array index accessor", caller_locations) + @columns[$1.to_i - 1] + elsif (column_index = @column_titles_underscored.index(method_name.to_s)) + @columns[column_index] + else + super + end + end + + def respond_to_missing?(method_name, *args) + (method_name =~ COLUMN_METHOD_NAME_RE) || @column_titles_underscored.include?(method_name.to_s) || super + end + + def inspect + "" + end + + private + + def pairs + @column_titles.zip(@columns).map do |title, value| + "#{title.to_s.underscore}: #{value.inspect}" + end.join(" ") + end + + end + end +end \ No newline at end of file diff --git a/test/unit/report/row_test.rb b/test/unit/report/row_test.rb new file mode 100644 index 00000000..1b7b630d --- /dev/null +++ b/test/unit/report/row_test.rb @@ -0,0 +1,45 @@ +require_relative '../../test_helper.rb' + +class ReportRowTest < Test::Unit::TestCase + + context "with a sample row" do + setup do + @row = XeroGateway::Report::Row.new( + ["Account", "Debit", "Credit"], + ["Sales (200)", 560_00, 0], + "Bank Accounts" + ) + end + + should "be able to access using the deprecated column_n API" do + ActiveSupport::Deprecation.silence do + assert_equal @row.column_1, "Sales (200)" + assert_equal @row.column_3, 0 + + assert @row.respond_to?(:column_1) + end + end + + should "be able to access using an underscored column name" do + assert_equal @row.account, "Sales (200)" + assert @row.respond_to?(:account) + end + + should "be able to access using an array index" do + assert_equal @row[0], "Sales (200)" + assert_equal @row[1], 560_00 + end + + should "be able to access using a string index" do + assert_equal @row["Account"], "Sales (200)" + assert_equal @row["Debit"], 560_00 + end + + should "be able to access using a symbol index" do + assert_equal @row[:account], "Sales (200)" + assert_equal @row[:debit], 560_00 + end + + end + +end \ No newline at end of file diff --git a/test/unit/report_test.rb b/test/unit/report_test.rb index e46eee16..a12e1d83 100644 --- a/test/unit/report_test.rb +++ b/test/unit/report_test.rb @@ -43,32 +43,32 @@ class ReportTest < Test::Unit::TestCase # First = Opening Balance first_statement = @report.body.first - assert_equal "2014-05-01T00:00:00", first_statement.column_1 - assert_equal "Opening Balance", first_statement.column_2 - assert_equal nil, first_statement.column_3 - assert_equal nil, first_statement.column_4 - assert_equal nil, first_statement.column_5 - assert_equal nil, first_statement.column_6 - assert_equal "15461.97", first_statement.column_7 + assert_equal "2014-05-01T00:00:00", first_statement.date + assert_equal "Opening Balance", first_statement.description + assert_equal nil, first_statement.reference + assert_equal nil, first_statement.reconciled + assert_equal nil, first_statement.source + assert_equal nil, first_statement.amount + assert_equal "15461.97", first_statement.balance # Second = Bank Transaction/Statement second_statement = @report.body.second - assert_equal "2014-05-01T00:00:00", second_statement.column_1 - assert_equal "Ridgeway Banking Corporation", second_statement.column_2 - assert_equal "Fee", second_statement.column_3 - assert_equal "No", second_statement.column_4 - assert_equal "Import", second_statement.column_5 - assert_equal "-15.00", second_statement.column_6 - assert_equal "15446.97", second_statement.column_7 + assert_equal "2014-05-01T00:00:00", second_statement.date + assert_equal "Ridgeway Banking Corporation", second_statement.description + assert_equal "Fee", second_statement.reference + assert_equal "No", second_statement.reconciled + assert_equal "Import", second_statement.source + assert_equal "-15.00", second_statement.amount + assert_equal "15446.97", second_statement.balance # Third third_statement = @report.body.third - assert_equal nil, third_statement.column_2.value # no description, but other attributes - assert_equal "Eft", third_statement.column_3 - assert_equal "No", third_statement.column_4 - assert_equal "Import", third_statement.column_5 - assert_equal "-15.75", third_statement.column_6 - assert_equal "15431.22", third_statement.column_7 + assert_equal nil, third_statement.description.value # no description, but other attributes + assert_equal "Eft", third_statement.reference + assert_equal "No", third_statement.reconciled + assert_equal "Import", third_statement.source + assert_equal "-15.75", third_statement.amount + assert_equal "15431.22", third_statement.balance end end @@ -79,8 +79,13 @@ class ReportTest < Test::Unit::TestCase should "set attributes on individual cells" do first_statement = @report.body.first - assert_equal "Sales (200)", first_statement.column_1.value - assert_equal({ account: "7d05a53d-613d-4eb2-a2fc-dcb6adb80b80" }, first_statement.column_1.attributes) + assert_equal "Sales (200)", first_statement.account.value + assert_equal({ account: "7d05a53d-613d-4eb2-a2fc-dcb6adb80b80" }, first_statement.account.attributes) + end + + should "have all rows and section titles" do + assert_equal 15, @report.rows.length + assert_equal %w(Revenue Expenses Assets Liabilities Equity), @report.rows.map(&:section_name).uniq.compact end end diff --git a/xero_gateway.gemspec b/xero_gateway.gemspec index 4c04d0e0..80680d90 100644 --- a/xero_gateway.gemspec +++ b/xero_gateway.gemspec @@ -32,5 +32,6 @@ Gem::Specification.new do |s| s.add_development_dependency "mocha" s.add_development_dependency "shoulda" s.add_development_dependency "libxml-ruby", "2.7.0" + s.add_development_dependency "byebug" end