Merge pull request #140 from xero-gateway/report-row-refactor

Refactors Report row handling, fixes a couple of bugs

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)

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

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
+        "<XeroGateway::Report::Row:#{object_id} #{pairs}>"
+      end
+
+      private
+
+        def pairs
+          @column_titles.zip(@columns).map do |title, value|
+            "#{title.to_s.underscore}: #{value.inspect}"
+          end.join(" ")
+        end
+
+    end
+  end
+end

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

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
 

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