From f28a8c87abe2def002987e5b170f83d8603e0708 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Nov 2023 09:51:18 +1100 Subject: [PATCH 1/7] Activate background report processing by default Many report specs are still testing the old behaviour. We need to migrate them to background reports. Some tests may be better as unit tests instead of system tests. --- ...222807_enable_feature_backround_reports.rb | 7 +++++ lib/open_food_network/feature_toggle.rb | 26 +++++++++++++++++- spec/base_spec_helper.rb | 27 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20231129222807_enable_feature_backround_reports.rb diff --git a/db/migrate/20231129222807_enable_feature_backround_reports.rb b/db/migrate/20231129222807_enable_feature_backround_reports.rb new file mode 100644 index 0000000000..a0688cb66c --- /dev/null +++ b/db/migrate/20231129222807_enable_feature_backround_reports.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class EnableFeatureBackroundReports < ActiveRecord::Migration[7.0] + def up + Flipper.enable("background_reports") + end +end diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index ccced3415c..f1a075ef8f 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -10,6 +10,17 @@ module OpenFoodNetwork # Please add your new feature here to appear in the Flipper UI. # We way move this to a YAML file when it becomes too awkward. # **WARNING:** Features not in this list will be removed. + # + # Once the feature is ready for general production use, + # copy the feature declaration to ACTIVE_BY_DEFAULT below and + # activate it for all instances with a migration: + # + # ./bin/rails generate migration EnableFeatureDragonMode + # + # Replace the `change` method with an `up` method and add this line: + # + # Flipper.enable("dragon_mode") + # CURRENT_FEATURES = { "admin_style_v3" => <<~DESC, Test the work-in-progress design updates. @@ -38,10 +49,23 @@ module OpenFoodNetwork DESC }.freeze + # Features you would like to be enabled to start with. + # + # Copy features here that were activated in a migration so that new + # instances, development and test environments have the feature active. + ACTIVE_BY_DEFAULT = { + "background_reports" => <<~DESC, + Generate reports in a background process to limit memory consumption. + DESC + }.freeze + def self.setup! CURRENT_FEATURES.each_key do |name| feature = Flipper.feature(name) - feature.add unless feature.exist? + unless feature.exist? + feature.add + feature.enable if ACTIVE_BY_DEFAULT[name] + end end Flipper.features.each do |feature| diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index ad026026ed..36895456dd 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -88,6 +88,33 @@ RSpec.configure do |config| expectations.syntax = :expect end + # Reset all feature toggles to prevent leaking. + config.before(:suite) do + Flipper.features.each(&:remove) + OpenFoodNetwork::FeatureToggle.setup! + end + + config.before(:each) do |example| + Flipper.disable(:background_reports) if example.file_path.in?( + [ + # rubocop:disable Layout/LineLength + "./spec/controllers/admin/reports_controller_spec.rb", + "./spec/system/admin/reports/enterprise_fee_summaries_spec.rb", + "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb", + "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb", + "./spec/system/admin/reports/orders_and_fulfillment_spec.rb", + "./spec/system/admin/reports/packing_report_spec.rb", + "./spec/system/admin/reports/payments_report_spec.rb", + "./spec/system/admin/reports/revenues_by_hub_spec.rb", + "./spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb", + "./spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb", + "./spec/system/admin/reports/users_and_enterprises_spec.rb", + "./spec/system/admin/reports_spec.rb", + # rubocop:enable Layout/LineLength + ] + ) + end + config.before(:each, :feature) do |example| feature = example.metadata[:feature].to_s From bbe37e1392d16a7eead6673045cdaccc7389cc2e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Nov 2023 13:51:32 +1100 Subject: [PATCH 2/7] Update ReportsController for background reports As it turns out, the background reports were missing a small feature to store user preferences. Yay specs! --- app/controllers/admin/reports_controller.rb | 2 ++ spec/base_spec_helper.rb | 1 - spec/controllers/admin/reports_controller_spec.rb | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb index ac6f1b73b4..8a972a859a 100644 --- a/app/controllers/admin/reports_controller.rb +++ b/app/controllers/admin/reports_controller.rb @@ -26,6 +26,8 @@ module Admin .enabled?(:background_reports, spree_current_user) if @background_reports && request.post? + rendering_options # stores user preferences + return background(report_format) end diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 36895456dd..31c8504b88 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -98,7 +98,6 @@ RSpec.configure do |config| Flipper.disable(:background_reports) if example.file_path.in?( [ # rubocop:disable Layout/LineLength - "./spec/controllers/admin/reports_controller_spec.rb", "./spec/system/admin/reports/enterprise_fee_summaries_spec.rb", "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb", "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb", diff --git a/spec/controllers/admin/reports_controller_spec.rb b/spec/controllers/admin/reports_controller_spec.rb index 4f99de87c6..32a44c1b88 100644 --- a/spec/controllers/admin/reports_controller_spec.rb +++ b/spec/controllers/admin/reports_controller_spec.rb @@ -309,7 +309,7 @@ describe Admin::ReportsController, type: :controller do controller_login_as_enterprise_user [coordinator1] end - it 'renders the delivery report' do + it "triggers the delivery report" do spree_post :show, { q: { completed_at_lt: 1.day.ago }, shipping_method_in: ["123"], # We just need to search for shipping methods @@ -317,7 +317,7 @@ describe Admin::ReportsController, type: :controller do report_subtype: "delivery", } - expect(response).to have_http_status(:ok) + expect(response).to have_http_status(:no_content) end end From 0b7cf86b5b55d0c4bc020043fc27508afebc8ef3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Nov 2023 14:38:12 +1100 Subject: [PATCH 3/7] Update report spec for background reports --- spec/base_spec_helper.rb | 1 - spec/system/admin/reports/enterprise_fee_summaries_spec.rb | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 31c8504b88..aa5c315524 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -98,7 +98,6 @@ RSpec.configure do |config| Flipper.disable(:background_reports) if example.file_path.in?( [ # rubocop:disable Layout/LineLength - "./spec/system/admin/reports/enterprise_fee_summaries_spec.rb", "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb", "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb", "./spec/system/admin/reports/orders_and_fulfillment_spec.rb", diff --git a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb index 39e8a387e7..1b8339ba26 100644 --- a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb @@ -94,6 +94,8 @@ describe "enterprise fee summaries" do it "generates file with data for all enterprises" do select "CSV" click_on "Go" + perform_enqueued_jobs(only: ReportJob) + click_on "Download Report" expect(downloaded_filename).to include ".csv" expect(downloaded_content).to have_content(distributor.name) end @@ -117,6 +119,8 @@ describe "enterprise fee summaries" do it "generates file with data for the enterprise" do select "CSV" click_on "Go" + perform_enqueued_jobs(only: ReportJob) + click_on "Download Report" expect(downloaded_filename).to include ".csv" expect(downloaded_content).to have_content(distributor.name) @@ -151,6 +155,8 @@ describe "enterprise fee summaries" do find("#report_format").click select "CSV" click_on "Go" + perform_enqueued_jobs(only: ReportJob) + click_on "Download Report" expect(downloaded_filename).to include ".csv" expect(downloaded_content).to have_content(distributor.name) From 049df77558298b557ae3bb89ca994f83d7ce337a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Nov 2023 16:20:10 +1100 Subject: [PATCH 4/7] Update specs for background reports --- spec/base_spec_helper.rb | 2 +- spec/support/reports_helper.rb | 8 ++++++++ ...nterprise_summary_fee_with_tax_report_by_order_spec.rb | 6 ++---- 3 files changed, 11 insertions(+), 5 deletions(-) create mode 100644 spec/support/reports_helper.rb diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index aa5c315524..a2c6776ec3 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -98,7 +98,6 @@ RSpec.configure do |config| Flipper.disable(:background_reports) if example.file_path.in?( [ # rubocop:disable Layout/LineLength - "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb", "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb", "./spec/system/admin/reports/orders_and_fulfillment_spec.rb", "./spec/system/admin/reports/packing_report_spec.rb", @@ -255,4 +254,5 @@ RSpec.configure do |config| config.include Features::DatepickerHelper, type: :system config.include Features::TrixEditorHelper, type: :system config.include DownloadsHelper, type: :system + config.include ReportsHelper, type: :system end diff --git a/spec/support/reports_helper.rb b/spec/support/reports_helper.rb new file mode 100644 index 0000000000..0145eb245e --- /dev/null +++ b/spec/support/reports_helper.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +module ReportsHelper + def run_report + click_on "Go" + perform_enqueued_jobs(only: ReportJob) + end +end diff --git a/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb b/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb index 13eea4aa19..5831946361 100644 --- a/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb +++ b/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_order_spec.rb @@ -163,8 +163,7 @@ describe "Enterprise Summary Fee with Tax Report By Order" do visit admin_reports_path click_on I18n.t("admin.reports.enterprise_fees_with_tax_report_by_order") - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) @@ -242,8 +241,7 @@ describe "Enterprise Summary Fee with Tax Report By Order" do visit admin_reports_path click_on I18n.t("admin.reports.enterprise_fees_with_tax_report_by_order") - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) From d1c73f86c0282df5127024ea83b60b3a6dc28f77 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 Dec 2023 09:27:58 +1100 Subject: [PATCH 5/7] Update specs for background reports --- spec/base_spec_helper.rb | 1 - ...ry_fee_with_tax_report_by_producer_spec.rb | 24 +++++++------------ 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index a2c6776ec3..d0da65f9a8 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -98,7 +98,6 @@ RSpec.configure do |config| Flipper.disable(:background_reports) if example.file_path.in?( [ # rubocop:disable Layout/LineLength - "./spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb", "./spec/system/admin/reports/orders_and_fulfillment_spec.rb", "./spec/system/admin/reports/packing_report_spec.rb", "./spec/system/admin/reports/payments_report_spec.rb", diff --git a/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb b/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb index 8fb6731421..b85a67b156 100644 --- a/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb +++ b/spec/system/admin/reports/enterprise_summary_fees/enterprise_summary_fee_with_tax_report_by_producer_spec.rb @@ -277,7 +277,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do context "with line items from a single supplier" do it 'generates the report and displays fees for the respective suppliers' do visit_report - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) @@ -313,8 +313,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do page.find("#s2id_q_order_cycle_id_in").click find('li', text: order_cycle.name).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) table = page.find("table.report__table tbody") @@ -489,8 +488,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do page.find("#s2id_q_order_cycle_id_in").click find('li', text: order_cycle3.name).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) table = page.find("table.report__table tbody") @@ -520,8 +518,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do page.find("#s2id_supplier_id_in").click find('li', text: supplier2.name).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) table = page.find("table.report__table tbody") @@ -545,8 +542,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do page.find(fee_name_selector).click find('li', text: supplier_fees.name).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) @@ -575,8 +571,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do page.find(fee_owner_selector).click find('li', text: supplier.name).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) table = page.find("table.report__table tbody") @@ -690,7 +685,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do context "with line items from a single supplier" do it 'generates the report and displays fees for the respective suppliers' do visit_report - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) @@ -828,7 +823,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do it 'should list all the tax rates' do visit_report - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) @@ -923,7 +918,7 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do it 'should list all the tax rates' do visit_report - click_on "Go" + run_report expect(page.find("table.report__table thead tr")).to have_content(table_header) @@ -951,6 +946,5 @@ describe "Enterprise Summary Fee with Tax Report By Producer" do report_type: :enterprise_fee_summary, report_subtype: :enterprise_fees_with_tax_report_by_producer ) - expect(page).to have_button("Go") end end From 75467a0c6510344de4a9286bc487402c4c87846d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 Dec 2023 10:06:55 +1100 Subject: [PATCH 6/7] Update specs for background reports --- spec/base_spec_helper.rb | 1 - .../reports/orders_and_fulfillment_spec.rb | 38 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index d0da65f9a8..306b31e092 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -98,7 +98,6 @@ RSpec.configure do |config| Flipper.disable(:background_reports) if example.file_path.in?( [ # rubocop:disable Layout/LineLength - "./spec/system/admin/reports/orders_and_fulfillment_spec.rb", "./spec/system/admin/reports/packing_report_spec.rb", "./spec/system/admin/reports/payments_report_spec.rb", "./spec/system/admin/reports/revenues_by_hub_spec.rb", diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index bcd3e9e5fc..23870f35fc 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -53,7 +53,7 @@ describe "Orders And Fulfillment" do end it "displays the report" do - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -104,7 +104,7 @@ describe "Orders And Fulfillment" do end it "correclty renders the report" do - click_button 'Go' + run_report expect(page).to have_content "My Order Cycle" end end @@ -127,20 +127,20 @@ describe "Orders And Fulfillment" do pick_datetime "#q_completed_at_lt", datetime_end find("#display_summary_row").set(false) # hides the summary rows - click_button 'Go' + run_report # Then I should see the rows for the first order but not the second # One row per line item - order1 only expect(all('table.report__table tbody tr').count).to eq(2) find("#display_summary_row").set(true) # displays the summary rows - click_button 'Go' + run_report # Then I should see the rows for the first order but not the second expect(all('table.report__table tbody tr').count).to eq(3) # 2 rows for order1 + 1 summary row # setting a time interval to include both orders pick_datetime "#q_completed_at_gt", datetime_start2 - click_button 'Go' + run_report # Then I should see the rows for both orders expect(all('table.report__table tbody tr').count).to eq(5) # 2 rows for order1 + 1 summary row @@ -168,7 +168,7 @@ describe "Orders And Fulfillment" do end it "orders the report by customer name, case insensitive" do - click_button 'Go' + run_report rows = find("table.report__table tbody").all("tr.summary-row") expect(rows[0]).to have_content "Dont Worry" expect(rows[1]).to have_content "Chamois xaxa" @@ -190,7 +190,7 @@ describe "Orders And Fulfillment" do it "includes only selected product" do tomselect_search_and_select(variant3.sku, from: "variant_id_in[]") - click_button 'Go' + run_report rows = find("table.report__table").all("tbody tr") table = rows.map { |r| r.all("td").map { |c| c.text.strip } } @@ -225,7 +225,7 @@ describe "Orders And Fulfillment" do describe "Totals" do before do click_link "Order Cycle Supplier Totals" - click_button 'Go' + run_report end context "with the header row option not selected" do @@ -285,7 +285,7 @@ describe "Orders And Fulfillment" do context "with the header row option selected" do before do find("#display_header_row").set(true) # displays the header row - click_button 'Go' + run_report end it "displays the report" do @@ -319,7 +319,7 @@ describe "Orders And Fulfillment" do context "with the header row option not selected" do before do find("#display_header_row").set(false) # hides the header row - click_button 'Go' + run_report end it "displays the report" do @@ -369,7 +369,7 @@ describe "Orders And Fulfillment" do context "with the header row option selected" do before do find("#display_header_row").set(true) # displays the header row - click_button 'Go' + run_report end it "displays the report" do @@ -418,7 +418,7 @@ describe "Orders And Fulfillment" do context "with the header row option not selected" do before do find("#display_header_row").set(false) # hides the header row - click_button 'Go' + run_report end it "displays the report" do @@ -473,7 +473,7 @@ describe "Orders And Fulfillment" do end it "displays the report" do - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -517,14 +517,14 @@ describe "Orders And Fulfillment" do check 'Header Row' uncheck 'Summary Row' - click_button 'Go' + run_report click_link "Report" click_link second_report_title expect(page).to have_unchecked_field('Header Row') expect(page).to have_checked_field('Summary Row') check 'Header Row' - click_button 'Go' + run_report # Step 2: check if report rendering options are saved properly click_link "Report" @@ -548,7 +548,7 @@ describe "Orders And Fulfillment" do expect(page).to have_checked_field('Product') uncheck('Producer') uncheck('Product') - click_button 'Go' + run_report click_link "Report" click_link second_report_title @@ -556,7 +556,7 @@ describe "Orders And Fulfillment" do expect(page).to have_checked_field('Producer') expect(page).to have_checked_field('Product') uncheck('Product') - click_button 'Go' + run_report # Step 2: check if report rendering options are saved properly click_link "Report" @@ -582,7 +582,7 @@ describe "Orders And Fulfillment" do expect(page).to have_checked_field('Summary Row') check 'Header Row' uncheck 'Summary Row' - click_button 'Go' + run_report logout login_as(current_user) @@ -602,7 +602,7 @@ describe "Orders And Fulfillment" do expect(page).to have_checked_field('Product') uncheck('Producer') uncheck('Product') - click_button 'Go' + run_report logout login_as(current_user) From 72755840c05984a7f20d3093e27c9e325e5a6244 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 Dec 2023 10:19:33 +1100 Subject: [PATCH 7/7] Update specs for background reports --- spec/base_spec_helper.rb | 16 ----- spec/support/reports_helper.rb | 2 + .../admin/reports/packing_report_spec.rb | 8 +-- .../admin/reports/payments_report_spec.rb | 4 +- .../admin/reports/revenues_by_hub_spec.rb | 2 +- .../sales_tax_totals_by_order_spec.rb | 20 +++--- .../sales_tax_totals_by_producer_spec.rb | 17 ++--- .../reports/users_and_enterprises_spec.rb | 2 +- spec/system/admin/reports_spec.rb | 65 +++++++------------ 9 files changed, 48 insertions(+), 88 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index 306b31e092..49c2631095 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -94,22 +94,6 @@ RSpec.configure do |config| OpenFoodNetwork::FeatureToggle.setup! end - config.before(:each) do |example| - Flipper.disable(:background_reports) if example.file_path.in?( - [ - # rubocop:disable Layout/LineLength - "./spec/system/admin/reports/packing_report_spec.rb", - "./spec/system/admin/reports/payments_report_spec.rb", - "./spec/system/admin/reports/revenues_by_hub_spec.rb", - "./spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb", - "./spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb", - "./spec/system/admin/reports/users_and_enterprises_spec.rb", - "./spec/system/admin/reports_spec.rb", - # rubocop:enable Layout/LineLength - ] - ) - end - config.before(:each, :feature) do |example| feature = example.metadata[:feature].to_s diff --git a/spec/support/reports_helper.rb b/spec/support/reports_helper.rb index 0145eb245e..7e5b2db003 100644 --- a/spec/support/reports_helper.rb +++ b/spec/support/reports_helper.rb @@ -3,6 +3,8 @@ module ReportsHelper def run_report click_on "Go" + expect(page).to have_selector ".loading" perform_enqueued_jobs(only: ReportJob) + expect(page).to have_no_selector ".loading" end end diff --git a/spec/system/admin/reports/packing_report_spec.rb b/spec/system/admin/reports/packing_report_spec.rb index 214212336d..f1117e5dee 100644 --- a/spec/system/admin/reports/packing_report_spec.rb +++ b/spec/system/admin/reports/packing_report_spec.rb @@ -55,7 +55,7 @@ describe "Packing Reports" do # pre-fills with dates check_prefilled_dates - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -76,7 +76,7 @@ describe "Packing Reports" do # pre-fills with dates check_prefilled_dates - click_button 'Go' + run_report rows = find("table.report__table").all("tr") table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[3] } expect(table).to eq([ @@ -102,7 +102,7 @@ describe "Packing Reports" do find(:css, "#display_summary_row").set(false) # does not include summary rows - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -150,7 +150,7 @@ describe "Packing Reports" do # pre-fills with dates check_prefilled_dates - find("button[type='submit']").click + run_report expect(page).to have_content li1.product.name expect(page).to have_content li2.product.name diff --git a/spec/system/admin/reports/payments_report_spec.rb b/spec/system/admin/reports/payments_report_spec.rb index b4e8cb0bff..762d3a5215 100644 --- a/spec/system/admin/reports/payments_report_spec.rb +++ b/spec/system/admin/reports/payments_report_spec.rb @@ -37,7 +37,7 @@ describe "Payments Reports" do context "when choosing itemised payments report type" do it "shows orders with payment state, their balance and totals" do click_link "Itemised Payment Totals" - find("[type='submit']").click + run_report expect(page.find("table.report__table thead tr").text).to have_content([ "Payment State", @@ -72,7 +72,7 @@ describe "Payments Reports" do it 'shows orders with payment state, their balance and and payment totals' do click_link "Payment Totals" - find("[type='submit']").click + run_report expect(page.find("table.report__table thead tr").text).to have_content([ "Payment State", diff --git a/spec/system/admin/reports/revenues_by_hub_spec.rb b/spec/system/admin/reports/revenues_by_hub_spec.rb index 9d622ec355..1daabb5359 100644 --- a/spec/system/admin/reports/revenues_by_hub_spec.rb +++ b/spec/system/admin/reports/revenues_by_hub_spec.rb @@ -27,7 +27,7 @@ describe "Revenues By Hub Reports" do context "testing report" do it "show the right values" do - find("[type='submit']").click + run_report expect(page.find("table.report__table thead tr").text).to have_content([ "HUB", diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb index 98bd05412f..7678f59c02 100644 --- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb +++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_order_spec.rb @@ -110,8 +110,7 @@ describe "Sales Tax Totals By order" do visit admin_reports_path click_on "Sales Tax Totals By Order" - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content([ @@ -171,8 +170,7 @@ describe "Sales Tax Totals By order" do visit_sales_tax_totals_by_order - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content([ @@ -339,8 +337,7 @@ describe "Sales Tax Totals By order" do end it "should load all the orders" do - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -365,8 +362,7 @@ describe "Sales Tax Totals By order" do page.find(customer_email_dropdown_selector).click find('li', text: customer1.email).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -384,8 +380,7 @@ describe "Sales Tax Totals By order" do page.find(customer_email_dropdown_selector).click find('li', text: customer2.email).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -404,7 +399,7 @@ describe "Sales Tax Totals By order" do find('li', text: customer1.email).click page.find(customer_email_dropdown_selector).click find('li', text: customer2.email).click - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -475,7 +470,8 @@ describe "Sales Tax Totals By order" do end def generate_report - click_on "Go" + run_report + click_on "Download Report" wait_for_download end diff --git a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb index d72ae51789..964537a037 100644 --- a/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb +++ b/spec/system/admin/reports/sales_tax/sales_tax_totals_by_producer_spec.rb @@ -79,8 +79,7 @@ describe "Sales Tax Totals By Producer" do visit admin_reports_path click_on 'Sales Tax Totals By Producer' - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content([ @@ -140,8 +139,7 @@ describe "Sales Tax Totals By Producer" do visit admin_reports_path click_on 'Sales Tax Totals By Producer' - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content([ @@ -329,8 +327,7 @@ describe "Sales Tax Totals By Producer" do end it "should load all the orders" do - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content(state_tax_rate_row) @@ -342,8 +339,7 @@ describe "Sales Tax Totals By Producer" do page.find(customer_email_dropdown_selector).click find('li', text: customer1.email).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -359,8 +355,7 @@ describe "Sales Tax Totals By Producer" do page.find(customer_email_dropdown_selector).click find('li', text: customer2.email).click - expect(page).to have_button("Go") - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect( @@ -377,7 +372,7 @@ describe "Sales Tax Totals By Producer" do find('li', text: customer1.email).click page.find(customer_email_dropdown_selector).click find('li', text: customer2.email).click - click_on "Go" + run_report expect(page.find("table.report__table thead tr").text).to have_content(table_header) expect(page.find("table.report__table tbody").text).to have_content(state_tax_rate_row) diff --git a/spec/system/admin/reports/users_and_enterprises_spec.rb b/spec/system/admin/reports/users_and_enterprises_spec.rb index ff3f49aaf4..e54c1c8e32 100644 --- a/spec/system/admin/reports/users_and_enterprises_spec.rb +++ b/spec/system/admin/reports/users_and_enterprises_spec.rb @@ -13,7 +13,7 @@ describe "Users & Enterprises reports" do it "displays the report" do enterprise = create(:supplier_enterprise) - click_button 'Go' + run_report expect(page.find("table.report__table thead tr").text).to have_content([ "USER", diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 5616575110..33b74e85b2 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -31,16 +31,7 @@ describe ' end end - describe "Background processing", feature: :background_reports do - it "can run the customers report" do - login_as_admin - visit admin_report_path(report_type: :customers) - generate_report - - expect(page).to have_selector "#report-table" - expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" - end - + describe "Background processing" do it "renders UTF-8 characters" do # We had a problem when UTF-8 was in the page and the report because # ActiveStorage read ASCII. @@ -58,7 +49,7 @@ describe ' # Run the report: login_as_admin visit admin_report_path(report_type: :customers) - generate_report + run_report expect(page).to have_content "Späti" expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" expect(page).to have_content "Müller" @@ -69,7 +60,7 @@ describe ' visit admin_report_path(report_type: :customers) stub_const("ReportJob::NOTIFICATION_TIME", 0) - generate_report + run_report # We also get an email. perform_enqueued_jobs(only: ActionMailer::MailDeliveryJob) @@ -114,9 +105,9 @@ describe ' ReportJob.perform_now(**args) breakpoint.synchronize { "continue after unlocked" } end - click_button "Go" - expect(page).to have_selector "#report-table table" + click_on "Go" + expect(page).to have_content "FIRST NAME LAST NAME BILLING ADDRESS EMAIL" # Now that we see the report, we need to make sure that it's not replaced @@ -145,7 +136,7 @@ describe ' within "table.index" do click_link "Customers" end - click_button "Go" + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -165,7 +156,7 @@ describe ' it "payment method report" do click_link "Payment Methods Report" - click_button "Go" + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([ @@ -176,7 +167,7 @@ describe ' it "delivery report" do click_link "Delivery Report" - click_button "Go" + run_report rows = find("table.report__table").all("thead tr") table = rows.map { |r| r.all("th").map { |c| c.text.strip } } expect(table.sort).to eq([ @@ -197,7 +188,7 @@ describe ' it "generates the orders and distributors report" do click_link 'Orders And Distributors' - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -235,7 +226,7 @@ describe ' it "generates the payments reports" do click_link 'Payments By Type' - click_button 'Go' + run_report rows = find("table.report__table").all("thead tr") table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -322,7 +313,7 @@ describe ' it "generate Tax Types reports" do click_link "Tax Types" - click_button "Go" + run_report # Then it should give me access only to managed enterprises expect(page).to have_select 'q_distributor_id_eq', @@ -332,7 +323,7 @@ describe ' # When I filter to just one distributor select user1.enterprises.first.name, from: 'q_distributor_id_eq' - click_button 'Go' + run_report # Then I should see the relevant order expect(page).to have_content order1.number.to_s @@ -353,7 +344,7 @@ describe ' it "generate Tax Rates report" do click_link "Tax Rates" - click_button "Go" + run_report expect(page).to have_css(".report__table thead th", text: "20.0% ($)") expect(page).to have_css(".report__table thead th", text: "0.0% ($)") @@ -396,7 +387,7 @@ describe ' expect(page).to have_content "All products" expect(page).to have_content "Inventory (on hand)" click_link 'All products' - click_button "Go" + run_report expect(page).to have_content "Supplier" expect(page).to have_table_row ["Supplier", "Producer Suburb", "Product", "Product Properties", "Taxons", "Variant Value", "Price", @@ -423,7 +414,7 @@ describe ' login_as_admin visit admin_reports_path click_link 'LettuceShare' - click_button "Go" + run_report expect(page).to have_table_row ['PRODUCT', 'Description', 'Qty', 'Pack Size', 'Unit', 'Unit Price', 'Total', 'GST incl.', @@ -448,7 +439,7 @@ describe ' end it "shows users and enterprises report" do - click_button "Go" + run_report rows = find("table.report__table").all("tr") table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[0..2] } @@ -469,7 +460,7 @@ describe ' select enterprise3.name, from: "enterprise_id_in" select enterprise1.owner.email, from: "user_id_in" - click_button "Go" + run_report rows = find("table.report__table").all("tr") table = rows.map { |r| r.all("th,td").map { |c| c.text.strip }[0..2] } @@ -489,7 +480,7 @@ describe ' it "generating Bulk Co-op Supplier Report" do click_link "Bulk Co-op Supplier Report" - click_button "Go" + run_report expect(page).to have_table_row [ "Supplier", @@ -508,7 +499,7 @@ describe ' it "generating Bulk Co-op Allocation report" do click_link "Bulk Co-op Allocation" - click_button "Go" + run_report expect(page).to have_table_row [ "Customer", @@ -527,7 +518,7 @@ describe ' it "generating Bulk Co-op Packing Sheets report" do click_link "Bulk Co-op Packing Sheets" - click_button "Go" + run_report expect(page).to have_table_row [ "Customer", @@ -539,7 +530,7 @@ describe ' it "generating Bulk Co-op Customer Payments report" do click_link "Bulk Co-op Customer Payments" - click_button "Go" + run_report expect(page).to have_table_row [ "Customer", @@ -652,7 +643,7 @@ describe ' login_as_admin visit admin_reports_path click_link "Summary" - click_button 'Go' + run_report end it "shows Xero invoices report" do @@ -682,7 +673,7 @@ describe ' pick_datetime '#due_date', Date.new(2021, 3, 12) fill_in 'account_code', with: 'abc123' - click_button 'Go' + run_report opts = { invoice_number: '5', invoice_date: '2021-02-12', due_date: '2021-03-12', account_code: 'abc123' } @@ -712,7 +703,7 @@ describe ' login_as_admin visit admin_reports_path click_link "Detailed" - click_button 'Go' + run_report opts = {} @@ -780,12 +771,4 @@ describe ' '', 'N'] end end - - def generate_report - click_button "Go" - expect(page).to have_selector ".loading" - perform_enqueued_jobs(only: ReportJob) - expect(page).to have_no_selector ".loading" - expect(page).to have_selector "#report-table table" - end end