From 5f7bee5ea2df6c35b2bb0879b7831f68b2947da5 Mon Sep 17 00:00:00 2001 From: Mike Frawley Date: Wed, 22 Jul 2015 10:11:50 -0500 Subject: [PATCH 1/3] ReportsService now returns a Report model Instead of trying to parse the response as a collection which always ends up blank, it now just returns a single Report model. The report model has an "xml" attribute containing the full response document, similar to has last_xml_response was previously used in the api. Added some tests and did some cleanup --- lib/quickbooks-ruby.rb | 2 +- lib/quickbooks/model/report.rb | 59 ++++++++++++++ lib/quickbooks/model/reports.rb | 17 ---- lib/quickbooks/service/base_service.rb | 8 +- lib/quickbooks/service/reports.rb | 13 +-- spec/lib/quickbooks/model/report_spec.rb | 45 +++++++++++ spec/lib/quickbooks/service/reports_spec.rb | 89 +++++++++------------ 7 files changed, 147 insertions(+), 86 deletions(-) create mode 100644 lib/quickbooks/model/report.rb delete mode 100644 lib/quickbooks/model/reports.rb create mode 100644 spec/lib/quickbooks/model/report_spec.rb diff --git a/lib/quickbooks-ruby.rb b/lib/quickbooks-ruby.rb index 40443217..48047221 100644 --- a/lib/quickbooks-ruby.rb +++ b/lib/quickbooks-ruby.rb @@ -102,7 +102,7 @@ require 'quickbooks/model/customer_change' require 'quickbooks/model/vendor_change' require 'quickbooks/model/item_change' -require 'quickbooks/model/reports' +require 'quickbooks/model/report' require 'quickbooks/model/credit_memo_change' require 'quickbooks/model/payment_change' diff --git a/lib/quickbooks/model/report.rb b/lib/quickbooks/model/report.rb new file mode 100644 index 00000000..7dc16efe --- /dev/null +++ b/lib/quickbooks/model/report.rb @@ -0,0 +1,59 @@ +module Quickbooks + module Model + class Report < BaseModel + + attr_reader :xml + + def xml=(xml) + xml.remove_namespaces! + @xml = xml + end + + def find_row(label) + rows = xml.xpath("//ColData[1]") + row = rows.find {|r| r.attr('value') == label } + row.parent.children.map {|c| c.attr('value') } + end + + def all_rows + nodes = xml.xpath("//ColData[1]") + nodes.map {|node| parse_row(node.parent) } + end + + def columns + @columns ||= begin + nodes = xml.xpath('/Report/Columns/Column') + nodes.map do |node| + # There is also a ColType field, but it does not seem valuable to capture + node.at('ColTitle').content + end + end + end + + private + # Parses the given row + # + # + # + # + # + # + # To: + # ['Checking', BigDecimal(1201.00)] + def parse_row(row_node) + row_node.elements.map.with_index do |el, i| + value = el.attr('value') + + if i.zero? # Return the first column as a string, its the label. + value + elsif value.blank? + nil + else + BigDecimal(value) + end + end + end + + end + end +end diff --git a/lib/quickbooks/model/reports.rb b/lib/quickbooks/model/reports.rb deleted file mode 100644 index 60fc9533..00000000 --- a/lib/quickbooks/model/reports.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Quickbooks - module Model - class Reports < BaseModel - XML_COLLECTION_NODE = "Reports" - XML_NODE = "Reports" - REST_RESOURCE = 'reports' - - REPORT_TYPES = ['BalanceSheet', 'ProfitandLoss', 'ProfitAndLossDetail', 'TrialBalance', 'CashFlow', 'InventoryValuationSummary', 'CustomerSales', 'ItemSales', 'DepartmentSales', 'ClassSales', 'CustomerIncome', 'CustomerBalance', 'CustomerBalanceDetail', 'AgedReceivables', 'AgedReceivableDetail', 'VendorBalance', 'VendorBalanceDetail', 'AgedPayables', 'AgedPayableDetail', 'VendorExpenses', 'GeneralLedgerDetail'] - - xml_name XML_NODE - - xml_accessor :currency, :from => 'Header/Currency' - xml_accessor :body - - end - end -end diff --git a/lib/quickbooks/service/base_service.rb b/lib/quickbooks/service/base_service.rb index 1c6baff0..a6fb5278 100644 --- a/lib/quickbooks/service/base_service.rb +++ b/lib/quickbooks/service/base_service.rb @@ -121,13 +121,9 @@ def parse_collection(response, model) collection.count = xml.xpath(path_to_nodes).count if collection.count > 0 xml.xpath(path_to_nodes).each do |xa| - entry = model.from_xml(xa) - addition = xml.xpath(path_to_nodes)[0].xpath("//xmlns:Currency").children.to_s if "#{model::XML_NODE}" == "Reports" - entry.currency = addition if "#{model::XML_NODE}" == "Reports" - collection.body = response.body if "#{model::XML_NODE}" == "Reports" - results << entry + results << model.from_xml(xa) end - end + end collection.entries = results rescue => ex diff --git a/lib/quickbooks/service/reports.rb b/lib/quickbooks/service/reports.rb index 8d75ff8e..61b46675 100644 --- a/lib/quickbooks/service/reports.rb +++ b/lib/quickbooks/service/reports.rb @@ -1,3 +1,4 @@ +# Docs: https://developer.intuit.com/docs/0100_accounting/0400_references/reports module Quickbooks module Service class Reports < BaseService @@ -16,21 +17,15 @@ def url_for_query(which_report = 'BalanceSheet', date_macro = 'This Fiscal Year- end end - def fetch_collection(model, date_macro, object_query, options = {}) - response = do_http_get(url_for_query(object_query, date_macro, options)) - - parse_collection(response, model) - - end - def query(object_query = 'BalanceSheet', date_macro = 'This Fiscal Year-to-date', options = {}) - fetch_collection(model, date_macro , object_query, options) + do_http_get(url_for_query(object_query, date_macro, options)) + model.new(:xml => @last_response_xml) end private def model - Quickbooks::Model::Reports + Quickbooks::Model::Report end end diff --git a/spec/lib/quickbooks/model/report_spec.rb b/spec/lib/quickbooks/model/report_spec.rb new file mode 100644 index 00000000..5fbcdcf4 --- /dev/null +++ b/spec/lib/quickbooks/model/report_spec.rb @@ -0,0 +1,45 @@ +describe Quickbooks::Model::Report do + + def build_report(fixture_name) + xml = Nokogiri.parse(fixture(fixture_name)) + Quickbooks::Model::Report.new(xml: xml) + end + + before(:each) do + @report = build_report("balancesheet.xml") + end + + describe "#xml" do + it 'exposes the full xml response' do + @report.xml.should be_a(Nokogiri::XML::Document) + end + + it 'removes xml namespaces so xpath queries can be done made without specifying the namespace' do + @report.xml.xpath('//Column').length.should == 2 + end + end + + describe "#columns" do + it 'returns an array containing items for each ' do + @report.columns.should == ["", "TOTAL"] + end + end + + describe "#all_rows" do + it 'returns a flat array containing all Row and Summary elements from the response xml' do + @report.all_rows.length.should > 1 + @report.all_rows.length.should == (@report.xml.xpath('//Row').length + @report.xml.xpath('//Summary').length) + end + + it 'returns a flat array containing all and elements' do + @report.all_rows[0].should == ['ASSETS', nil] + @report.all_rows[1].should == ['Current Assets', nil] + @report.all_rows[2].should == ['Bank Accounts', nil] + @report.all_rows[3].should == ['Checking', BigDecimal.new("1201.00")] + @report.all_rows[4].should == ['Savings', BigDecimal.new("800.00")] + end + end + + describe "#find_row" do + end +end diff --git a/spec/lib/quickbooks/service/reports_spec.rb b/spec/lib/quickbooks/service/reports_spec.rb index 458f8652..3a56aeb0 100644 --- a/spec/lib/quickbooks/service/reports_spec.rb +++ b/spec/lib/quickbooks/service/reports_spec.rb @@ -1,68 +1,51 @@ -describe "Quickbooks::Service::Reports" do +describe "Quickbooks::Service::Report" do before(:all) do construct_service :reports end - it "can query for Balance Sheets", focus: true do - xml = fixture("balancesheet.xml") - model = Quickbooks::Model::Reports - - stub_request(:get, @service.url_for_query, ["200", "OK"], xml) - reports = @service.query('BalanceSheet') - - balance_sheet_xml = @service.last_response_xml - balance_sheet_xml.xpath('//xmlns:ReportName').children.to_s == 'BalanceSheet' + describe '.url_for_query()' do + it 'uses "BalanceSheet" and "This Fiscal Year to date" as the default parameters' do + @service.url_for_query().should == 'https://quickbooks.api.intuit.com/v3/company/9991111222/reports/BalanceSheet?date_macro=This+Fiscal+Year-to-date' + end + + it 'allows overriding the report type' do + @service.url_for_query('ProfitAndLoss').should == 'https://quickbooks.api.intuit.com/v3/company/9991111222/reports/ProfitAndLoss?date_macro=This+Fiscal+Year-to-date' + end + + it 'allows overriding the date_macro' do + @service.url_for_query('BalanceSheet', 'Last Year').should == 'https://quickbooks.api.intuit.com/v3/company/9991111222/reports/BalanceSheet?date_macro=Last+Year' + end + + it 'allows passing additional query parameters' do + url = @service.url_for_query('BalanceSheet', 'Last Year', + :start_date => '2015-01-01', + :end_date => '2015-01-31', + :accounting_method => 'Cash', + :columns => 'subt_nat_amount,tax_amount', + ) + url.should == 'https://quickbooks.api.intuit.com/v3/company/9991111222/reports/BalanceSheet?start_date=2015-01-01&end_date=2015-01-31&accounting_method=Cash&columns=subt_nat_amount,tax_amount' + end + + it 'currently ignores the date_macro argument when passed in additional options' do + @service.url_for_query('BalanceSheet', 'Today', :accounting_method => 'Cash').should == 'https://quickbooks.api.intuit.com/v3/company/9991111222/reports/BalanceSheet?accounting_method=Cash' + end end - it 'can query Balance Sheets between different dates' do + it 'forwards arguments to .url_for_query' do xml = fixture("balancesheet.xml") - model = Quickbooks::Model::Reports - - stub_request(:get, @service.url_for_query('BalanceSheet', 'This Fiscal Quarter This Fiscal Quarter-to-date' ), ["200", "OK"], xml) - reports = @service.query('BalanceSheet', 'This Fiscal Quarter This Fiscal Quarter-to-date' ) + url = @service.url_for_query('BalanceSheet', 'Today', :accounting_method => 'Cash') + stub_request(:get, url, ["200", "OK"], xml) - balance_sheet_xml = @service.last_response_xml - balance_sheet_xml.xpath('//xmlns:ReportName').children.to_s == 'BalanceSheet' + @service.query('BalanceSheet', 'Today', :accounting_method => 'Cash') end - it 'can query Cash Flow' do - xml = fixture("cashflow.xml") - model = Quickbooks::Model::Reports - - stub_request(:get, @service.url_for_query('CashFlow'), ["200", "OK"], xml) - - reports = @service.query('CashFlow') - - cash_flow_xml = @service.last_response_xml - cash_flow_xml.xpath('//xmlns:ReportName').children.to_s == 'CashFlow' - end - - it 'can query Profit and Loss' do - xml = fixture("profitloss.xml") - model = Quickbooks::Model::Reports - - stub_request(:get, @service.url_for_query('ProfitAndLoss'), ["200", "OK"], xml) + it "returns a Report model" do + xml = fixture("balancesheet.xml") - reports = @service.query('ProfitAndLoss') + stub_request(:get, @service.url_for_query, ["200", "OK"], xml) + report = @service.query('BalanceSheet') - profit_loss_xml = @service.last_response_xml - profit_loss_xml.xpath('//xmlns:ReportName').children.to_s == 'ProfitAndLoss' + report.should be_a Quickbooks::Model::Report end - it 'can accept additional options for a query' do - xml = fixture("profitandlossdetailwithoptions.xml") - model = Quickbooks::Model::Reports - options = {} - options[:start_date] = '2015-01-01' - options[:end_date] = '2015-01-31' - options[:accounting_method] = 'Cash' - options[:columns] = 'subt_nat_amount,tax_amount' - - stub_request(:get, @service.url_for_query('ProfitAndLossDetail', {}, options), ["200", "OK"], xml) - - reports = @service.query('ProfitAndLossDetail', {}, options) - - profit_loss_xml = @service.last_response_xml - profit_loss_xml.xpath('//xmlns:ReportName').children.to_s == 'ProfitAndLoss' - end end From fa35e5cd4bef8bf68d4ecf1b21ef00639954362d Mon Sep 17 00:00:00 2001 From: Mike Frawley Date: Wed, 22 Jul 2015 14:42:47 -0500 Subject: [PATCH 2/3] add fixture for balance sheet with multiple columns and specs --- .../balance_sheet_with_year_summary.xml | 399 ++++++++++++++++++ spec/fixtures/balancesheet.xml | 2 +- spec/lib/quickbooks/model/report_spec.rb | 31 +- 3 files changed, 418 insertions(+), 14 deletions(-) create mode 100644 spec/fixtures/balance_sheet_with_year_summary.xml diff --git a/spec/fixtures/balance_sheet_with_year_summary.xml b/spec/fixtures/balance_sheet_with_year_summary.xml new file mode 100644 index 00000000..25437139 --- /dev/null +++ b/spec/fixtures/balance_sheet_with_year_summary.xml @@ -0,0 +1,399 @@ + + +
+ + BalanceSheet + all + Accrual + Year + USD + + +
+ + + + Account + + + Jul 4 - Dec 31, 2013 + Money + + StartDate + 2013-07-04 + + + EndDate + 2013-12-31 + + + + Jan - Dec 2014 + Money + + StartDate + 2014-01-01 + + + EndDate + 2014-12-31 + + + + Jan 1 - Jun 21, 2015 + Money + + StartDate + 2015-01-01 + + + EndDate + 2015-06-21 + + + + + +
+ + + + +
+ + +
+ + + + +
+ + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + +
+ + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + +
+ + +
+ + + + +
+ + +
+ + + + +
+ + +
+ + + + +
+ + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + +
+ + + + +
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spec/fixtures/balancesheet.xml b/spec/fixtures/balancesheet.xml index e22f602d..6f828478 100644 --- a/spec/fixtures/balancesheet.xml +++ b/spec/fixtures/balancesheet.xml @@ -261,4 +261,4 @@ - \ No newline at end of file + diff --git a/spec/lib/quickbooks/model/report_spec.rb b/spec/lib/quickbooks/model/report_spec.rb index 5fbcdcf4..ac5d03b7 100644 --- a/spec/lib/quickbooks/model/report_spec.rb +++ b/spec/lib/quickbooks/model/report_spec.rb @@ -5,38 +5,43 @@ def build_report(fixture_name) Quickbooks::Model::Report.new(xml: xml) end - before(:each) do - @report = build_report("balancesheet.xml") - end + let(:report) { build_report("balancesheet.xml") } + let(:report_with_years) { build_report("balance_sheet_with_year_summary.xml") } describe "#xml" do it 'exposes the full xml response' do - @report.xml.should be_a(Nokogiri::XML::Document) + report.xml.should be_a(Nokogiri::XML::Document) end it 'removes xml namespaces so xpath queries can be done made without specifying the namespace' do - @report.xml.xpath('//Column').length.should == 2 + report.xml.xpath('//Column').length.should == 2 end end describe "#columns" do it 'returns an array containing items for each ' do - @report.columns.should == ["", "TOTAL"] + report.columns.should == ["", "TOTAL"] + report_with_years.columns.should == ["", "Jul 4 - Dec 31, 2013", "Jan - Dec 2014", "Jan 1 - Jun 21, 2015"] end end describe "#all_rows" do it 'returns a flat array containing all Row and Summary elements from the response xml' do - @report.all_rows.length.should > 1 - @report.all_rows.length.should == (@report.xml.xpath('//Row').length + @report.xml.xpath('//Summary').length) + report.all_rows.length.should > 1 + report.all_rows.length.should == (report.xml.xpath('//Row').length + report.xml.xpath('//Summary').length) end it 'returns a flat array containing all and elements' do - @report.all_rows[0].should == ['ASSETS', nil] - @report.all_rows[1].should == ['Current Assets', nil] - @report.all_rows[2].should == ['Bank Accounts', nil] - @report.all_rows[3].should == ['Checking', BigDecimal.new("1201.00")] - @report.all_rows[4].should == ['Savings', BigDecimal.new("800.00")] + report.all_rows[0].should == ['ASSETS', nil] + report.all_rows[1].should == ['Current Assets', nil] + report.all_rows[2].should == ['Bank Accounts', nil] + report.all_rows[3].should == ['Checking', BigDecimal("1201.00")] + report.all_rows[4].should == ['Savings', BigDecimal("800.00")] + end + + it 'works with multiple columns' do + report_with_years.all_rows[3].should == ['Checking', nil, nil, BigDecimal("1201.00")] + report_with_years.all_rows[4].should == ['Savings', BigDecimal("19.99"), BigDecimal("19.99"), BigDecimal("819.99")] end end From 15535897113686508791104ee7108e87a8d6426a Mon Sep 17 00:00:00 2001 From: Mike Frawley Date: Mon, 27 Jul 2015 15:46:07 -0500 Subject: [PATCH 3/3] Cleanup Report Model implementation - remove call to remove_xml_namespaces! Use css queries instead. - cache @all_rows - implement find_row using all_rows --- lib/quickbooks/model/report.rb | 37 ++++++++++-------------- spec/lib/quickbooks/model/report_spec.rb | 16 ++++++++-- 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/quickbooks/model/report.rb b/lib/quickbooks/model/report.rb index 7dc16efe..27f24e1d 100644 --- a/lib/quickbooks/model/report.rb +++ b/lib/quickbooks/model/report.rb @@ -2,27 +2,15 @@ module Quickbooks module Model class Report < BaseModel - attr_reader :xml - - def xml=(xml) - xml.remove_namespaces! - @xml = xml - end - - def find_row(label) - rows = xml.xpath("//ColData[1]") - row = rows.find {|r| r.attr('value') == label } - row.parent.children.map {|c| c.attr('value') } - end + attr_accessor :xml def all_rows - nodes = xml.xpath("//ColData[1]") - nodes.map {|node| parse_row(node.parent) } + @all_rows ||= xml.css("ColData:first-child").map {|node| parse_row(node.parent) } end def columns @columns ||= begin - nodes = xml.xpath('/Report/Columns/Column') + nodes = xml.css('Column') nodes.map do |node| # There is also a ColType field, but it does not seem valuable to capture node.at('ColTitle').content @@ -30,16 +18,21 @@ def columns end end + def find_row(label) + all_rows.find {|r| r[0] == label } + end + private - # Parses the given row - # - # - # - # - # + + # Parses the given row: + # + # + # + # + # # # To: - # ['Checking', BigDecimal(1201.00)] + # ['Checking', BigDecimal(1201.00), BigDecimal(200.50)] def parse_row(row_node) row_node.elements.map.with_index do |el, i| value = el.attr('value') diff --git a/spec/lib/quickbooks/model/report_spec.rb b/spec/lib/quickbooks/model/report_spec.rb index ac5d03b7..6abfd5c9 100644 --- a/spec/lib/quickbooks/model/report_spec.rb +++ b/spec/lib/quickbooks/model/report_spec.rb @@ -11,10 +11,12 @@ def build_report(fixture_name) describe "#xml" do it 'exposes the full xml response' do report.xml.should be_a(Nokogiri::XML::Document) + report.xml.css('Column').length.should == 2 end - it 'removes xml namespaces so xpath queries can be done made without specifying the namespace' do - report.xml.xpath('//Column').length.should == 2 + it 'preserves the xml namespace, which must be included in xpath queries' do + report.xml.xpath('/Report/Columns/Column').length.should == 0 + report.xml.xpath("/xmlns:Report/xmlns:Columns/xmlns:Column").length.should == 2 end end @@ -28,7 +30,7 @@ def build_report(fixture_name) describe "#all_rows" do it 'returns a flat array containing all Row and Summary elements from the response xml' do report.all_rows.length.should > 1 - report.all_rows.length.should == (report.xml.xpath('//Row').length + report.xml.xpath('//Summary').length) + report.all_rows.length.should == report.xml.css('Row,Summary').length end it 'returns a flat array containing all and elements' do @@ -46,5 +48,13 @@ def build_report(fixture_name) end describe "#find_row" do + it "returns the given row if the label matches" do + report.find_row('Checking').should == ['Checking', BigDecimal("1201.00")] + report.find_row('Opening Balance Equity').should == ['Opening Balance Equity', BigDecimal("-9337.50")] + end + + it "returns nil if the label was not found" do + report.find_row('Shoes for the dog!').should == nil + end end end