From 02c479564028bcadd3ad645efdcb5a59a84e6b4d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 29 May 2018 16:47:23 +1000 Subject: [PATCH 1/4] Remove default dates from reports Closes https://github.com/openfoodfoundation/openfoodnetwork/issues/2212 --- .../admin/reports_controller_decorator.rb | 37 +++---------------- 1 file changed, 5 insertions(+), 32 deletions(-) diff --git a/app/controllers/spree/admin/reports_controller_decorator.rb b/app/controllers/spree/admin/reports_controller_decorator.rb index 5556488424..52abed6fde 100644 --- a/app/controllers/spree/admin/reports_controller_decorator.rb +++ b/app/controllers/spree/admin/reports_controller_decorator.rb @@ -68,7 +68,7 @@ Spree::Admin::ReportsController.class_eval do end def order_cycle_management - prepare_date_params params + params[:q] ||= {} # -- Prepare form options my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) @@ -91,8 +91,7 @@ Spree::Admin::ReportsController.class_eval do end def packing - # -- Prepare date parameters - prepare_date_params params + params[:q] ||= {} # -- Prepare form options my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) @@ -115,7 +114,6 @@ Spree::Admin::ReportsController.class_eval do end def orders_and_distributors - prepare_date_params params @report = OpenFoodNetwork::OrderAndDistributorReport.new spree_current_user, params, render_content? @search = @report.search csv_file_name = "orders_and_distributors_#{timestamp}.csv" @@ -123,7 +121,6 @@ Spree::Admin::ReportsController.class_eval do end def sales_tax - prepare_date_params params @distributors = Enterprise.is_distributor.managed_by(spree_current_user) @report_type = params[:report_type] @report = OpenFoodNetwork::SalesTaxReport.new spree_current_user, params, render_content? @@ -131,8 +128,6 @@ Spree::Admin::ReportsController.class_eval do end def bulk_coop - prepare_date_params params - # -- Prepare form options @distributors = Enterprise.is_distributor.managed_by(spree_current_user) @report_type = params[:report_type] @@ -147,9 +142,6 @@ Spree::Admin::ReportsController.class_eval do end def payments - # -- Prepare Date Params - prepare_date_params params - # -- Prepare Form Options @distributors = Enterprise.is_distributor.managed_by(spree_current_user) @report_type = params[:report_type] @@ -164,8 +156,7 @@ Spree::Admin::ReportsController.class_eval do end def orders_and_fulfillment - # -- Prepare Date Params - prepare_date_params params + params[:q] ||= {} # -- Prepare Form Options permissions = OpenFoodNetwork::Permissions.new(spree_current_user) @@ -206,12 +197,8 @@ Spree::Admin::ReportsController.class_eval do end def xero_invoices - if request.get? - params[:q] ||= {} - params[:q][:completed_at_gt] = Time.zone.today.beginning_of_month - params[:invoice_date] = Time.zone.today - params[:due_date] = Time.zone.today + 1.month - end + params[:q] ||= {} + @distributors = Enterprise.is_distributor.managed_by(spree_current_user) @order_cycles = OrderCycle.active_or_complete.accessible_by(spree_current_user).order('orders_close_at DESC') @@ -263,20 +250,6 @@ Spree::Admin::ReportsController.class_eval do end end - def prepare_date_params(params) - # -- Prepare parameters - params[:q] ||= {} - if params[:q][:completed_at_gt].blank? - params[:q][:completed_at_gt] = Time.zone.now.beginning_of_month - else - params[:q][:completed_at_gt] = Time.zone.parse(params[:q][:completed_at_gt]) rescue Time.zone.now.beginning_of_month - end - if params[:q] && !params[:q][:completed_at_lt].blank? - params[:q][:completed_at_lt] = Time.zone.parse(params[:q][:completed_at_lt]) rescue "" - end - params[:q][:meta_sort] ||= "completed_at.desc" - end - def load_data # Load distributors either owned by the user or selling their enterprises products. my_distributors = Enterprise.is_distributor.managed_by(spree_current_user) From 72ae6f2af6a239ada8b97c5f09dd3af85f9c3b8a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 29 May 2018 16:49:00 +1000 Subject: [PATCH 2/4] Add spec helper to make spec run on its own --- spec/lib/open_food_network/xero_invoices_report_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/lib/open_food_network/xero_invoices_report_spec.rb b/spec/lib/open_food_network/xero_invoices_report_spec.rb index f2dce1fb5a..a8c45bd7d7 100644 --- a/spec/lib/open_food_network/xero_invoices_report_spec.rb +++ b/spec/lib/open_food_network/xero_invoices_report_spec.rb @@ -1,3 +1,4 @@ +require 'spec_helper' require 'open_food_network/xero_invoices_report' module OpenFoodNetwork From 54bdcf7679e0a26db10896bb7748ec80148ddd58 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 29 May 2018 16:53:18 +1000 Subject: [PATCH 3/4] Convert specs to RSpec 3.7.0 syntax with Transpec This conversion is done by Transpec 3.3.0 with the following command: transpec spec/lib/open_food_network/xero_invoices_report_spec.rb * 15 conversions from: obj.stub(:message) to: allow(obj).to receive(:message) * 10 conversions from: obj.should to: expect(obj).to * 4 conversions from: == expected to: eq(expected) * 3 conversions from: obj.should_not to: expect(obj).not_to For more details: https://github.com/yujinakayama/transpec#supported-conversions --- .../xero_invoices_report_spec.rb | 58 +++++++++---------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/spec/lib/open_food_network/xero_invoices_report_spec.rb b/spec/lib/open_food_network/xero_invoices_report_spec.rb index a8c45bd7d7..d119d26964 100644 --- a/spec/lib/open_food_network/xero_invoices_report_spec.rb +++ b/spec/lib/open_food_network/xero_invoices_report_spec.rb @@ -13,10 +13,10 @@ module OpenFoodNetwork around { |example| Timecop.travel(Time.zone.local(2015, 5, 5, 14, 0, 0)) { example.run } } it "uses defaults when blank params are passed" do - report.instance_variable_get(:@opts).should == {invoice_date: Date.civil(2015, 5, 5), + expect(report.instance_variable_get(:@opts)).to eq({invoice_date: Date.civil(2015, 5, 5), due_date: Date.civil(2015, 6, 5), account_code: 'food sales', - report_type: 'summary'} + report_type: 'summary'}) end end @@ -26,53 +26,53 @@ module OpenFoodNetwork let(:summary_rows) { report.send(:summary_rows_for_order, order, 1, {}) } before do - report.stub(:produce_summary_rows) { ['produce'] } - report.stub(:fee_summary_rows) { ['fee'] } - report.stub(:shipping_summary_rows) { ['shipping'] } - report.stub(:payment_summary_rows) { ['payment'] } - report.stub(:admin_adjustment_summary_rows) { ['admin'] } - order.stub(:account_invoice?) { false } + allow(report).to receive(:produce_summary_rows) { ['produce'] } + allow(report).to receive(:fee_summary_rows) { ['fee'] } + allow(report).to receive(:shipping_summary_rows) { ['shipping'] } + allow(report).to receive(:payment_summary_rows) { ['payment'] } + allow(report).to receive(:admin_adjustment_summary_rows) { ['admin'] } + allow(order).to receive(:account_invoice?) { false } end it "displays produce summary rows when summary report" do - report.stub(:detail?) { false } - summary_rows.should include 'produce' + allow(report).to receive(:detail?) { false } + expect(summary_rows).to include 'produce' end it "does not display produce summary rows when detail report" do - report.stub(:detail?) { true } - summary_rows.should_not include 'produce' + allow(report).to receive(:detail?) { true } + expect(summary_rows).not_to include 'produce' end it "displays fee summary rows when summary report" do - report.stub(:detail?) { false } - order.stub(:account_invoice?) { true } - summary_rows.should include 'fee' + allow(report).to receive(:detail?) { false } + allow(order).to receive(:account_invoice?) { true } + expect(summary_rows).to include 'fee' end it "displays fee summary rows when this is not an account invoice" do - report.stub(:detail?) { true } - order.stub(:account_invoice?) { false } - summary_rows.should include 'fee' + allow(report).to receive(:detail?) { true } + allow(order).to receive(:account_invoice?) { false } + expect(summary_rows).to include 'fee' end it "does not display fee summary rows when this is a detail report for an account invoice" do - report.stub(:detail?) { true } - order.stub(:account_invoice?) { true } - summary_rows.should_not include 'fee' + allow(report).to receive(:detail?) { true } + allow(order).to receive(:account_invoice?) { true } + expect(summary_rows).not_to include 'fee' end it "always displays shipping summary rows" do - summary_rows.should include 'shipping' + expect(summary_rows).to include 'shipping' end it "displays admin adjustment summary rows when summary report" do - summary_rows.should include 'admin' + expect(summary_rows).to include 'admin' end it "does not display admin adjustment summary rows when detail report" do - report.stub(:detail?) { true } - summary_rows.should_not include 'admin' + allow(report).to receive(:detail?) { true } + expect(summary_rows).not_to include 'admin' end end @@ -85,12 +85,12 @@ module OpenFoodNetwork let!(:adj_shipping) { create(:adjustment, adjustable: order, label: "Shipping", originator: shipping_method) } it "returns BillablePeriod adjustments only" do - report.send(:account_invoice_adjustments, order).should == [adj_invoice] + expect(report.send(:account_invoice_adjustments, order)).to eq([adj_invoice]) end it "excludes adjustments where the source is missing" do billable_period.destroy - report.send(:account_invoice_adjustments, order).should be_empty + expect(report.send(:account_invoice_adjustments, order)).to be_empty end end @@ -99,7 +99,7 @@ module OpenFoodNetwork describe "when no initial invoice number is given" do it "returns the order number" do - subject.send(:invoice_number_for, order, 123).should == 'R731032860' + expect(subject.send(:invoice_number_for, order, 123)).to eq('R731032860') end end @@ -107,7 +107,7 @@ module OpenFoodNetwork subject { XeroInvoicesReport.new user, {initial_invoice_number: '123'} } it "increments the number by the index" do - subject.send(:invoice_number_for, order, 456).should == 579 + expect(subject.send(:invoice_number_for, order, 456)).to eq(579) end end end From e6ef43c91d00d91c63a0b15f51fe862b7dd82e14 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 29 May 2018 16:55:30 +1000 Subject: [PATCH 4/4] Give a spec some style --- .rubocop_todo.yml | 11 ----------- .../xero_invoices_report_spec.rb | 18 +++++++++--------- 2 files changed, 9 insertions(+), 20 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 3e49ab57d3..bc47b9054c 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -193,7 +193,6 @@ Layout/EmptyLines: - 'lib/open_food_network/sales_tax_report.rb' - 'lib/open_food_network/scope_product_to_hub.rb' - 'lib/open_food_network/scope_variant_to_hub.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/spree/core/controller_helpers/order_decorator.rb' - 'lib/tasks/cache.rake' - 'lib/tasks/dev.rake' @@ -395,7 +394,6 @@ Layout/ExtraSpacing: - 'spec/features/consumer/shopping/shopping_spec.rb' - 'spec/lib/open_food_network/enterprise_fee_calculator_spec.rb' - 'spec/lib/open_food_network/reports/rule_spec.rb' - - 'spec/lib/open_food_network/xero_invoices_report_spec.rb' - 'spec/models/enterprise_fee_spec.rb' - 'spec/models/enterprise_spec.rb' - 'spec/models/order_cycle_spec.rb' @@ -669,7 +667,6 @@ Layout/SpaceAroundEqualsInParameterDefault: - 'lib/open_food_network/permissions.rb' - 'lib/open_food_network/scope_variant_to_hub.rb' - 'lib/open_food_network/tag_rule_applicator.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/spree/money_decorator.rb' - 'spec/features/admin/enterprise_relationships_spec.rb' - 'spec/features/admin/reports_spec.rb' @@ -693,7 +690,6 @@ Layout/SpaceAroundOperators: - 'app/overrides/remove_side_bar.rb' - 'app/overrides/replace_shipping_address_form_with_distributor_details.rb' - 'app/serializers/api/enterprise_serializer.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/spree/product_filters.rb' - 'spec/controllers/admin/enterprises_controller_spec.rb' - 'spec/controllers/cart_controller_spec.rb' @@ -862,7 +858,6 @@ Layout/SpaceInsideHashLiteralBraces: - 'lib/open_food_network/reports/rule.rb' - 'lib/open_food_network/sales_tax_report.rb' - 'lib/open_food_network/variant_and_line_item_naming.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/tasks/users.rake' - 'spec/controllers/admin/accounts_and_billing_settings_controller_spec.rb' - 'spec/controllers/admin/business_model_configuration_controller_spec.rb' @@ -901,7 +896,6 @@ Layout/SpaceInsideHashLiteralBraces: - 'spec/lib/open_food_network/reports/report_spec.rb' - 'spec/lib/open_food_network/reports/rule_spec.rb' - 'spec/lib/open_food_network/tag_rule_applicator_spec.rb' - - 'spec/lib/open_food_network/xero_invoices_report_spec.rb' - 'spec/lib/stripe/account_connector_spec.rb' - 'spec/models/customer_spec.rb' - 'spec/models/enterprise_fee_spec.rb' @@ -1089,7 +1083,6 @@ Lint/UnusedBlockArgument: - 'lib/open_food_network/reports/bulk_coop_allocation_report.rb' - 'lib/open_food_network/reports/bulk_coop_supplier_report.rb' - 'lib/open_food_network/sales_tax_report.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'spec/lib/open_food_network/order_grouper_spec.rb' - 'spec/support/cancan_helper.rb' - 'spec/support/delayed_job_helper.rb' @@ -1273,7 +1266,6 @@ Naming/UncommunicativeMethodParamName: - 'app/services/subscription_validator.rb' - 'lib/open_food_network/property_merge.rb' - 'lib/open_food_network/reports/bulk_coop_report.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'spec/lib/open_food_network/reports/report_spec.rb' - 'spec/mailers/producer_mailer_spec.rb' @@ -1695,7 +1687,6 @@ Style/BracesAroundHashParameters: - 'lib/open_food_network/order_cycle_form_applicator.rb' - 'lib/open_food_network/reports/rule.rb' - 'lib/open_food_network/variant_and_line_item_naming.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'spec/controllers/admin/accounts_and_billing_settings_controller_spec.rb' - 'spec/controllers/admin/business_model_configuration_controller_spec.rb' - 'spec/controllers/admin/enterprises_controller_spec.rb' @@ -1727,7 +1718,6 @@ Style/BracesAroundHashParameters: - 'spec/lib/open_food_network/feature_toggle_spec.rb' - 'spec/lib/open_food_network/order_cycle_form_applicator_spec.rb' - 'spec/lib/open_food_network/subscription_summarizer_spec.rb' - - 'spec/lib/open_food_network/xero_invoices_report_spec.rb' - 'spec/models/billable_period_spec.rb' - 'spec/models/product_distribution_spec.rb' - 'spec/models/spree/ability_spec.rb' @@ -2308,7 +2298,6 @@ Style/NumericPredicate: - 'app/models/spree/order_decorator.rb' - 'lib/open_food_network/integrity_checker.rb' - 'lib/open_food_network/rack_request_blocker.rb' - - 'lib/open_food_network/xero_invoices_report.rb' - 'lib/spree/money_decorator.rb' # Offense count: 2 diff --git a/spec/lib/open_food_network/xero_invoices_report_spec.rb b/spec/lib/open_food_network/xero_invoices_report_spec.rb index d119d26964..e5ae74d8d0 100644 --- a/spec/lib/open_food_network/xero_invoices_report_spec.rb +++ b/spec/lib/open_food_network/xero_invoices_report_spec.rb @@ -8,20 +8,20 @@ module OpenFoodNetwork let(:user) { create(:user) } describe "option defaults" do - let(:report) { XeroInvoicesReport.new user, {initial_invoice_number: '', invoice_date: '', due_date: '', account_code: ''} } + let(:report) { XeroInvoicesReport.new user, initial_invoice_number: '', invoice_date: '', due_date: '', account_code: '' } around { |example| Timecop.travel(Time.zone.local(2015, 5, 5, 14, 0, 0)) { example.run } } it "uses defaults when blank params are passed" do - expect(report.instance_variable_get(:@opts)).to eq({invoice_date: Date.civil(2015, 5, 5), - due_date: Date.civil(2015, 6, 5), - account_code: 'food sales', - report_type: 'summary'}) + expect(report.instance_variable_get(:@opts)).to eq( invoice_date: Date.civil(2015, 5, 5), + due_date: Date.civil(2015, 6, 5), + account_code: 'food sales', + report_type: 'summary' ) end end describe "summary rows" do - let(:report) { XeroInvoicesReport.new user, {initial_invoice_number: '', invoice_date: '', due_date: '', account_code: ''} } + let(:report) { XeroInvoicesReport.new user, initial_invoice_number: '', invoice_date: '', due_date: '', account_code: '' } let(:order) { double(:order) } let(:summary_rows) { report.send(:summary_rows_for_order, order, 1, {}) } @@ -31,7 +31,7 @@ module OpenFoodNetwork allow(report).to receive(:shipping_summary_rows) { ['shipping'] } allow(report).to receive(:payment_summary_rows) { ['payment'] } allow(report).to receive(:admin_adjustment_summary_rows) { ['admin'] } - allow(order).to receive(:account_invoice?) { false } + allow(order).to receive(:account_invoice?) { false } end it "displays produce summary rows when summary report" do @@ -77,7 +77,7 @@ module OpenFoodNetwork end describe "finding account invoice adjustments" do - let(:report) { XeroInvoicesReport.new user, {initial_invoice_number: '', invoice_date: '', due_date: '', account_code: ''} } + let(:report) { XeroInvoicesReport.new user, initial_invoice_number: '', invoice_date: '', due_date: '', account_code: '' } let!(:order) { create(:order) } let(:billable_period) { create(:billable_period) } let(:shipping_method) { create(:shipping_method) } @@ -104,7 +104,7 @@ module OpenFoodNetwork end describe "when an initial invoice number is given" do - subject { XeroInvoicesReport.new user, {initial_invoice_number: '123'} } + subject { XeroInvoicesReport.new user, initial_invoice_number: '123' } it "increments the number by the index" do expect(subject.send(:invoice_number_for, order, 456)).to eq(579)