From fd45dea9f711015ce6394a9eb3108518d0670493 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Mon, 19 Aug 2024 16:05:14 -0600 Subject: [PATCH 01/10] Moves report test case into dedicated file Sets up an enterprise user instead of an admin user --- .../reports/orders_and_distributors_spec.rb | 65 +++++++++++++++++++ spec/system/admin/reports_spec.rb | 37 ----------- 2 files changed, 65 insertions(+), 37 deletions(-) create mode 100644 spec/system/admin/reports/orders_and_distributors_spec.rb diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb new file mode 100644 index 00000000000..457632cc834 --- /dev/null +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +require "system_helper" + +RSpec.describe "Orders And Distributors" do + include AuthenticationHelper + include WebHelper + + describe "Orders And Distributors" do + let!(:distributor_enterprise1) { create(:distributor_enterprise) } + let!(:distributor_enterprise2) { create(:distributor_enterprise) } + let!(:ready_to_ship_order1) { + create(:order_ready_to_ship, distributor_id: distributor_enterprise1.id) + } + let!(:ready_to_ship_order2) { + create(:order_ready_to_ship, distributor_id: distributor_enterprise2.id) + } + + before do + login_as(distributor_enterprise1.owner) + visit admin_reports_path + click_link "Orders And Distributors" + end + + it "generates the report" do + run_report + + rows = find("table.report__table").all("thead tr") + table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } + + expect(table_headers).to eq([ + ['Order date', + 'Order Id', + 'Customer Name', + 'Customer Email', + 'Customer Phone', + 'Customer City', + 'SKU', + 'Item name', + 'Variant', + 'Quantity', + 'Max Quantity', + 'Cost', + 'Shipping Cost', + 'Payment Method', + 'Distributor', + 'Distributor address', + 'Distributor city', + 'Distributor postcode', + 'Shipping Method', + 'Shipping instructions'] + ]) + + expect(all('table.report__table tbody tr').count).to eq( + Spree::LineItem.where( + order_id: ready_to_ship_order1.id # Total rows should equal nr. of line items, per order + ).count + ) + + # displays only orders from the hub it is managing + expect(page).to have_content(distributor_enterprise1.name), count: 5 + expect(page).to_not have_content(distributor_enterprise2.name) + end + end +end diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb index 4d1b8bf5335..57d7dfff22a 100644 --- a/spec/system/admin/reports_spec.rb +++ b/spec/system/admin/reports_spec.rb @@ -211,43 +211,6 @@ visit admin_reports_path end - it "generates the orders and distributors report" do - click_link 'Orders And Distributors' - run_report - - rows = find("table.report__table").all("thead tr") - table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } - - expect(table_headers).to eq([ - ['Order date', - 'Order Id', - 'Customer Name', - 'Customer Email', - 'Customer Phone', - 'Customer City', - 'SKU', - 'Item name', - 'Variant', - 'Quantity', - 'Max Quantity', - 'Cost', - 'Shipping Cost', - 'Payment Method', - 'Distributor', - 'Distributor address', - 'Distributor city', - 'Distributor postcode', - 'Shipping Method', - 'Shipping instructions'] - ]) - - expect(all('table.report__table tbody tr').count).to eq( - Spree::LineItem.where( - order_id: ready_to_ship_order.id # Total rows should equal number of line items, per order - ).count - ) - end - it "generates the payments reports" do click_link 'Payments By Type' run_report From 2a1d494301f15be447eab8eb665709250f919959 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Tue, 20 Aug 2024 11:40:20 -0600 Subject: [PATCH 02/10] Adds coverage for table contents --- .../reports/orders_and_distributors_spec.rb | 59 +++++++++++++++---- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index 457632cc834..8c48350abb2 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -7,24 +7,51 @@ include WebHelper describe "Orders And Distributors" do - let!(:distributor_enterprise1) { create(:distributor_enterprise) } - let!(:distributor_enterprise2) { create(:distributor_enterprise) } - let!(:ready_to_ship_order1) { - create(:order_ready_to_ship, distributor_id: distributor_enterprise1.id) + let!(:distributor) { create(:distributor_enterprise, name: "By Bike") } + let!(:distributor2) { create(:distributor_enterprise, name: "By Moto") } + let!(:completed_at) { Time.zone.now.to_fs(:db) } + + let!(:order) { + create(:order_ready_to_ship, distributor_id: distributor.id, completed_at:) } - let!(:ready_to_ship_order2) { - create(:order_ready_to_ship, distributor_id: distributor_enterprise2.id) + let!(:order2) { + create(:order_ready_to_ship, distributor_id: distributor2.id, completed_at:) + } + + let(:line_item1) { + [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", + "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item2) { + [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", + "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item3) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", + "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item4) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", + "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item5) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", + "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") } before do - login_as(distributor_enterprise1.owner) + login_as(distributor.owner) visit admin_reports_path click_link "Orders And Distributors" + run_report end it "generates the report" do - run_report - rows = find("table.report__table").all("thead tr") table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } @@ -53,13 +80,21 @@ expect(all('table.report__table tbody tr').count).to eq( Spree::LineItem.where( - order_id: ready_to_ship_order1.id # Total rows should equal nr. of line items, per order + order_id: order.id # Total rows should equal nr. of line items, per order ).count ) # displays only orders from the hub it is managing - expect(page).to have_content(distributor_enterprise1.name), count: 5 - expect(page).to_not have_content(distributor_enterprise2.name) + expect(page).to have_content(distributor.name), count: 5 + expect(page).not_to have_content(distributor2.name) + + # displayes table contents correctly, per line item + table = page.find("table.report__table tbody") + expect(table).to have_content(line_item1) + expect(table).to have_content(line_item2) + expect(table).to have_content(line_item3) + expect(table).to have_content(line_item4) + expect(table).to have_content(line_item5) end end end From 32e32117e3e2c96a3798d370c22f4e0889f25630 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Wed, 21 Aug 2024 16:50:26 -0600 Subject: [PATCH 03/10] Adds test case around hub filters --- spec/support/request/web_helper.rb | 10 + .../reports/orders_and_distributors_spec.rb | 190 +++++++++++------- 2 files changed, 126 insertions(+), 74 deletions(-) diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index a5c4924a835..21f5159c075 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -88,6 +88,16 @@ def click_on_select2(value, options) find(:css, ".select2-result-label", text: options[:select_text] || value).click end + def clear_select2(selector) + page.find(selector).scroll_to(page.find(selector)) + .find(:css, '.select2-choice, .select2-search-field').click + page.find(selector).scroll_to(page.find(selector)) + .find(:css, '.select2-choice, .select2-search-field').send_keys :backspace + page.find(selector).scroll_to(page.find(selector)) + .find(:css, '.select2-choice, .select2-search-field').send_keys :backspace + find("body").send_keys(:escape) + end + def request_monitor_finished(controller = nil) page.evaluate_script("#{angular_scope(controller)}.scope().RequestMonitor.loading == false") end diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index 8c48350abb2..ecacbeec15e 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -18,83 +18,125 @@ create(:order_ready_to_ship, distributor_id: distributor2.id, completed_at:) } - let(:line_item1) { - [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", - "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", - "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") - } - let(:line_item2) { - [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", - "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", - "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") - } - let(:line_item3) { - [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", - "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", - "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") - } - let(:line_item4) { - [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", - "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", - "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") - } - let(:line_item5) { - [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", - "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", "By Bike", - "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") - } + context "as an enterprise user" do + let(:line_item1) { + [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", + "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item2) { + [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", + "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item3) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", + "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item4) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", + "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + let(:line_item5) { + [completed_at.to_s, order.id, "John Doe", order.email, "123-456-7890", "Herndon", + "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", + "By Bike", "10 Lovely Street", "Herndon", "20170", "UPS Ground", "none"].join(" ") + } + + before do + login_as(distributor.owner) + visit admin_reports_path + click_link "Orders And Distributors" + run_report + end + + it "generates the report" do + rows = find("table.report__table").all("thead tr") + table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } + + expect(table_headers).to eq([ + ['Order date', + 'Order Id', + 'Customer Name', + 'Customer Email', + 'Customer Phone', + 'Customer City', + 'SKU', + 'Item name', + 'Variant', + 'Quantity', + 'Max Quantity', + 'Cost', + 'Shipping Cost', + 'Payment Method', + 'Distributor', + 'Distributor address', + 'Distributor city', + 'Distributor postcode', + 'Shipping Method', + 'Shipping instructions'] + ]) - before do - login_as(distributor.owner) - visit admin_reports_path - click_link "Orders And Distributors" - run_report + expect(all('table.report__table tbody tr').count).to eq( + Spree::LineItem.where( + order_id: order.id # Total rows should equal nr. of line items, per order + ).count + ) + + # displays only orders from the hub it is managing + expect(page).to have_content(distributor.name), count: 5 + + # only sees line items from orders it manages + expect(page).not_to have_content(distributor2.name) + + # displayes table contents correctly, per line item + table = page.find("table.report__table tbody") + expect(table).to have_content(line_item1) + expect(table).to have_content(line_item2) + expect(table).to have_content(line_item3) + expect(table).to have_content(line_item4) + expect(table).to have_content(line_item5) + end end - it "generates the report" do - rows = find("table.report__table").all("thead tr") - table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } - - expect(table_headers).to eq([ - ['Order date', - 'Order Id', - 'Customer Name', - 'Customer Email', - 'Customer Phone', - 'Customer City', - 'SKU', - 'Item name', - 'Variant', - 'Quantity', - 'Max Quantity', - 'Cost', - 'Shipping Cost', - 'Payment Method', - 'Distributor', - 'Distributor address', - 'Distributor city', - 'Distributor postcode', - 'Shipping Method', - 'Shipping instructions'] - ]) - - expect(all('table.report__table tbody tr').count).to eq( - Spree::LineItem.where( - order_id: order.id # Total rows should equal nr. of line items, per order - ).count - ) - - # displays only orders from the hub it is managing - expect(page).to have_content(distributor.name), count: 5 - expect(page).not_to have_content(distributor2.name) - - # displayes table contents correctly, per line item - table = page.find("table.report__table tbody") - expect(table).to have_content(line_item1) - expect(table).to have_content(line_item2) - expect(table).to have_content(line_item3) - expect(table).to have_content(line_item4) - expect(table).to have_content(line_item5) + context "as admin" do + before do + login_as_admin + visit admin_reports_path + click_link "Orders And Distributors" + end + + context "with two orders on the same day at different times" do + let(:completed_at1) { 1500.hours.ago } # 1500 hours in the past + let(:completed_at2) { 1700.hours.ago } # 1700 hours in the past + let(:datetime_start1) { 1600.hours.ago } # 1600 hours in the past + let(:datetime_start2) { 1800.hours.ago } # 1600 hours in the past + let(:datetime_end) { 1400.hours.ago } # 1400 hours in the past + let!(:order3) { + create(:order_ready_to_ship, distributor_id: distributor.id, completed_at: completed_at1) + } + let!(:order4) { + create(:order_ready_to_ship, distributor_id: distributor.id, completed_at: completed_at2) + } + + context "applying filters" do + it "displays line items from the correct distributors" do + # for one distributor + select2_select distributor.name, from: "q_distributor_id_in" + run_report + expect(page).to have_content(distributor.name), count: 15 + + clear_select2("#s2id_q_distributor_id_in") + + # for another distributor + select2_select distributor2.name, from: "q_distributor_id_in" + run_report + expect(page).to have_content(distributor2.name), count: 5 + end + end + end end end end From 2e5c5261701f00bbac44eaa3a68a835a5dd32696 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Thu, 22 Aug 2024 17:12:47 -0600 Subject: [PATCH 04/10] Adds basic coverage on report file download Moves file download into report helper Removes pdf file assertation Removes test on PDF file on sales tax report Removes PDF testing from helper --- spec/support/reports_helper.rb | 16 ++++++ .../reports/orders_and_distributors_spec.rb | 55 +++++++++++-------- .../sales_tax_totals_by_order_spec.rb | 18 +----- 3 files changed, 51 insertions(+), 38 deletions(-) diff --git a/spec/support/reports_helper.rb b/spec/support/reports_helper.rb index d8f6df5f464..2927fee4058 100644 --- a/spec/support/reports_helper.rb +++ b/spec/support/reports_helper.rb @@ -12,4 +12,20 @@ def run_report expect(page).not_to have_selector ".loading" expect(page).to have_button "Go", disabled: false end + + def generate_report + run_report + click_on "Download Report" + wait_for_download + end + + def load_file_txt(extension, downloaded_filename) + case extension + when "csv" + CSV.read(downloaded_filename).join(" ") + when "xlsx" + xlsx = Roo::Excelx.new(downloaded_filename) + xlsx.map(&:to_a).join(" ") + end + end end diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index ecacbeec15e..37c3267c31b 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -5,6 +5,7 @@ RSpec.describe "Orders And Distributors" do include AuthenticationHelper include WebHelper + include ReportsHelper describe "Orders And Distributors" do let!(:distributor) { create(:distributor_enterprise, name: "By Bike") } @@ -19,6 +20,13 @@ } context "as an enterprise user" do + let(:header) { + ["Order date", "Order Id", "Customer Name", "Customer Email", "Customer Phone", + "Customer City", "SKU", "Item name", "Variant", "Quantity", "Max Quantity", + "Cost", "Shipping Cost", "Payment Method", "Distributor", "Distributor address", + "Distributor city", "Distributor postcode", "Shipping Method", + "Shipping instructions"] + } let(:line_item1) { [completed_at, order.id, "John Doe", order.email, "123-456-7890", "Herndon", "ABC", Spree::Product.first.name.to_s, "1g", "1", "none", "10.0", "none", "Check", @@ -56,28 +64,7 @@ rows = find("table.report__table").all("thead tr") table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } - expect(table_headers).to eq([ - ['Order date', - 'Order Id', - 'Customer Name', - 'Customer Email', - 'Customer Phone', - 'Customer City', - 'SKU', - 'Item name', - 'Variant', - 'Quantity', - 'Max Quantity', - 'Cost', - 'Shipping Cost', - 'Payment Method', - 'Distributor', - 'Distributor address', - 'Distributor city', - 'Distributor postcode', - 'Shipping Method', - 'Shipping instructions'] - ]) + expect(table_headers).to eq([header]) expect(all('table.report__table tbody tr').count).to eq( Spree::LineItem.where( @@ -99,6 +86,30 @@ expect(table).to have_content(line_item4) expect(table).to have_content(line_item5) end + + describe "downloading the report" do + shared_examples "reports generated as" do |output_type, extension| + context output_type.to_s do + it "downloads the #{output_type} file" do + select output_type, from: "report_format" + + expect { generate_report }.to change { downloaded_filenames.length }.from(0).to(1) + + expect(downloaded_filename).to match(/.*\.#{extension}/) + + downloaded_file_txt = load_file_txt(extension, downloaded_filename) + + expect(downloaded_file_txt).to have_text header.join(" ") + expect(downloaded_file_txt).to have_text( + "By Bike 10 Lovely Street Herndon 20170 UPS Ground", count: 5 + ) + end + end + end + + it_behaves_like "reports generated as", "CSV", "csv" + it_behaves_like "reports generated as", "Spreadsheet", "xlsx" + end end context "as admin" do 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 7377869a9f7..690d2df0e73 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 @@ -3,6 +3,8 @@ require 'system_helper' RSpec.describe "Sales Tax Totals By order" do + include ReportsHelper + # Scenarion 1: added tax # 1 producer # 1 distributor @@ -467,20 +469,4 @@ def visit_sales_tax_totals_by_order report_subtype: :sales_tax_totals_by_order ) end - - def generate_report - run_report - click_on "Download Report" - wait_for_download - end - - def load_file_txt(extension, downloaded_filename) - case extension - when "csv" - CSV.read(downloaded_filename).join(" ") - when "xlsx" - xlsx = Roo::Excelx.new(downloaded_filename) - xlsx.map(&:to_a).join(" ") - end - end end From 6117d70fae95ae68e855a2bfba4633c76b9415d0 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Fri, 30 Aug 2024 16:35:15 -0600 Subject: [PATCH 05/10] Replaces code with shared examples This spec was appearently flaky, let's see if this change stabilizes it. It came up here: https://github.com/openfoodfoundation/openfoodnetwork/actions/runs/10639846576/job/29498582671?pr=12790 Removes CSV tests based on permissions Not sure we need these tests, proposing to remove them and use shared examples to test file download --- .../reports/enterprise_fee_summaries_spec.rb | 72 +++++++++---------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb index 13bffa16e1a..3bbed9a3547 100644 --- a/spec/system/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/system/admin/reports/enterprise_fee_summaries_spec.rb @@ -5,6 +5,7 @@ RSpec.describe "enterprise fee summaries" do include AuthenticationHelper include WebHelper + include ReportsHelper let!(:distributor) { create(:distributor_enterprise) } let!(:other_distributor) { create(:distributor_enterprise) } @@ -78,59 +79,48 @@ end end - describe "csv downloads" do + describe "permissions" do describe "smoke test for generation of report based on permissions" do + let!(:order) do + create(:completed_order_with_fees, order_cycle:, + distributor:) + end + let!(:other_order) do + create(:completed_order_with_fees, order_cycle: other_order_cycle, + distributor: other_distributor) + end context "when logged in as admin" do - let!(:order) do - create(:completed_order_with_fees, order_cycle:, - distributor:) - end - let(:current_user) { create(:admin_user) } + let!(:current_user) { create(:admin_user) } before do visit main_app.admin_report_path(report_type: 'enterprise_fee_summary') end it "generates file with data for all enterprises" do - select "CSV" run_report - click_on "Download Report" - expect(downloaded_filename).to include ".csv" - expect(downloaded_content).to have_content(distributor.name) + expect(page).to have_content(distributor.name) + expect(page).to have_content(other_distributor.name) end end context "when logged in as enterprise user" do - let!(:order) do - create(:completed_order_with_fees, order_cycle:, - distributor:) - end - let!(:other_order) do - create(:completed_order_with_fees, order_cycle: other_order_cycle, - distributor: other_distributor) - end - let(:current_user) { distributor.owner } + let!(:current_user) { distributor.owner } before do visit main_app.admin_report_path(report_type: 'enterprise_fee_summary') end it "generates file with data for the enterprise" do - select "CSV" run_report - click_on "Download Report" - - expect(downloaded_filename).to include ".csv" - expect(downloaded_content).to have_content(distributor.name) - expect(downloaded_content).not_to have_content(other_distributor.name) + expect(page).to have_content(distributor.name) + expect(page).not_to have_content(other_distributor.name) end end end - describe "smoke test for filtering report based on filters" do + describe "downloading the report" do let!(:second_distributor) { create(:distributor_enterprise) } let!(:second_order_cycle) { create(:simple_order_cycle, coordinator: second_distributor) } - let!(:order) do create(:completed_order_with_fees, order_cycle:, distributor:) @@ -144,21 +134,29 @@ before do visit main_app.admin_report_path(report_type: 'enterprise_fee_summary') - end - - it "generates file with data for selected order cycle" do find("#s2id_q_order_cycle_ids").click select order_cycle.name + end - find("#report_format").click - select "CSV" - run_report - click_on "Download Report" + shared_examples "reports generated as" do |output_type, extension| + context output_type.to_s do + it "downloads the #{output_type} file" do + select output_type, from: "report_format" - expect(downloaded_filename).to include ".csv" - expect(downloaded_content).to have_content(distributor.name) - expect(downloaded_content).not_to have_content(second_distributor.name) + expect { generate_report }.to change { downloaded_filenames.length }.from(0).to(1) + + expect(downloaded_filename).to match(/.*\.#{extension}/) + + downloaded_file_txt = load_file_txt(extension, downloaded_filename) + + expect(downloaded_file_txt).to have_content(distributor.name) + expect(downloaded_file_txt).not_to have_content(second_distributor.name) + end + end end + + it_behaves_like "reports generated as", "CSV", "csv" + it_behaves_like "reports generated as", "Spreadsheet", "xlsx" end end From ec4dba71c2a6f7640a30bc46a71b755e07aa8dd5 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Mon, 2 Sep 2024 18:39:37 -0600 Subject: [PATCH 06/10] Adds flatpickr test --- .../reports/orders_and_distributors_spec.rb | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index 37c3267c31b..e6011a8ee5e 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -73,7 +73,7 @@ ) # displays only orders from the hub it is managing - expect(page).to have_content(distributor.name), count: 5 + expect(page).to have_content(distributor.name, count: 6) # only sees line items from orders it manages expect(page).not_to have_content(distributor2.name) @@ -132,7 +132,33 @@ create(:order_ready_to_ship, distributor_id: distributor.id, completed_at: completed_at2) } - context "applying filters" do + context "applying time/date filters" do + it "is precise to time of day, not just date" do + # When I generate a customer report + # with a timeframe that includes one order but not the other + find("input.datepicker").click + select_dates_from_daterangepicker datetime_start1, datetime_end + find(".shortcut-buttons-flatpickr-button").click # closes flatpickr + + run_report + # Then I should see the rows for the first order but not the second + # One row per line item - order3 only + expect(page).to have_text(distributor.name, count: 6) + expect(page).to have_text(order3.email, count: 5) + + # setting a time interval to include both orders + find("input.datepicker").click + select_dates_from_daterangepicker datetime_start2, Time.zone.now + + run_report + # Then I should see the rows for both orders + expect(page).to have_text(distributor.name, count: 11) + expect(page).to have_text(order3.email, count: 5) + expect(page).to have_text(order4.email, count: 5) + end + end + + context "applying distributor filters" do it "displays line items from the correct distributors" do # for one distributor select2_select distributor.name, from: "q_distributor_id_in" From 8500f6c198adb4c8f22ffcdd33c90f66b204bfaa Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Tue, 3 Sep 2024 16:54:14 -0600 Subject: [PATCH 07/10] Addresses reviews. The biggest change is moving the table CSS into its helper, which touches other system specs (namely orders_and_fulfillment_spec.rb). Rubocop fixup --- spec/support/reports_helper.rb | 5 + .../reports/orders_and_distributors_spec.rb | 18 +-- .../reports/orders_and_fulfillment_spec.rb | 141 +++++++++++------- 3 files changed, 102 insertions(+), 62 deletions(-) diff --git a/spec/support/reports_helper.rb b/spec/support/reports_helper.rb index 2927fee4058..fda57daca00 100644 --- a/spec/support/reports_helper.rb +++ b/spec/support/reports_helper.rb @@ -28,4 +28,9 @@ def load_file_txt(extension, downloaded_filename) xlsx.map(&:to_a).join(" ") end end + + def table_headers + rows = find("table.report__table").all("thead tr") + rows.map { |r| r.all("th").map { |c| c.text.strip } } + end end diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index e6011a8ee5e..7a79f6e127d 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -8,6 +8,7 @@ include ReportsHelper describe "Orders And Distributors" do + let!(:report_url) { admin_report_path(report_type: :orders_and_distributors) } let!(:distributor) { create(:distributor_enterprise, name: "By Bike") } let!(:distributor2) { create(:distributor_enterprise, name: "By Moto") } let!(:completed_at) { Time.zone.now.to_fs(:db) } @@ -55,22 +56,15 @@ before do login_as(distributor.owner) - visit admin_reports_path - click_link "Orders And Distributors" + visit report_url run_report end it "generates the report" do - rows = find("table.report__table").all("thead tr") - table_headers = rows.map { |r| r.all("th").map { |c| c.text.strip } } - expect(table_headers).to eq([header]) - expect(all('table.report__table tbody tr').count).to eq( - Spree::LineItem.where( - order_id: order.id # Total rows should equal nr. of line items, per order - ).count - ) + # Total rows should equal nr. of line items, per order + expect(all('table.report__table tbody tr').count).to eq(5) # displays only orders from the hub it is managing expect(page).to have_content(distributor.name, count: 6) @@ -115,8 +109,8 @@ context "as admin" do before do login_as_admin - visit admin_reports_path - click_link "Orders And Distributors" + visit report_url + run_report end context "with two orders on the same day at different times" do diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index 95139fab80c..4f952b28623 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -439,20 +439,17 @@ end it "displays the report" do - rows = find("table.report__table").all("thead tr") - table = rows.map { |r| r.all("th").map { |c| c.text.strip } } - # displays the producer column - expect(table).to eq([ - ["Producer", - "Product", - "Variant", - "Hub", - "Quantity", - "Curr. Cost per Unit", - "Total Cost", - "Shipping Method"] - ]) + expect(table_headers).to eq([ + ["Producer", + "Product", + "Variant", + "Hub", + "Quantity", + "Curr. Cost per Unit", + "Total Cost", + "Shipping Method"] + ]) # displays the producer name in the respective column # does not display the header row @@ -461,6 +458,53 @@ expect(page).not_to have_css("td.header-row") end end + + xit "aggregates results per variant" do + pending '#9678' + expect(all('table.report__table tbody tr').count).to eq(4) + # 1 row per variant = 2 rows + # 2 TOTAL rows + # 4 rows total + + expect(table_headers[0]).to eq( + ["Supplier Name", "Baked Beans", "1g Small, S", + "Distributor Name", "7", "10.0", "70.0", "UPS Ground"] + ) + expect(table_headers[1]).to eq( + ["", "", "", "TOTAL", "7", "", "70.0", ""] + ) + expect(table_headers[2]).to eq( + ["Supplier Name", "Baked Beans", "1g Big, S", + "Distributor Name", + "3", "10.0", "30.0", "UPS Ground"] + ) + expect(table_headers[3]).to eq(["", "", "", "TOTAL", "3", "", "30.0", ""]) + end + end + + context "with the header row option selected" do + before do + find("#display_header_row").set(true) # displays the header row + run_report + end + + it "displays the report" do + # hides the producer column + expect(table_headers).to eq([ + ["Product", + "Variant", + "Quantity", + "Curr. Cost per Unit", + "Total Cost", + "Shipping Method"] + ]) + + # displays the producer name in own row + within "td.header-row" do + expect(page).to have_content("Supplier Name") + end + end +>>>>>>> 50da07bb61 (Addresses reviews. The biggest change is moving the table CSS) end end end @@ -495,21 +539,18 @@ end it "displays the report" do - rows = find("table.report__table").all("thead tr") - table = rows.map { |r| r.all("th").map { |c| c.text.strip } } - # displays the producer column - expect(table).to eq([ - ["Hub", - "Producer", - "Product", - "Variant", - "Quantity", - "Curr. Cost per Unit", - "Total Cost", - "Total Shipping Cost", - "Shipping Method"] - ]) + expect(table_headers).to eq([ + ["Hub", + "Producer", + "Product", + "Variant", + "Quantity", + "Curr. Cost per Unit", + "Total Cost", + "Total Shipping Cost", + "Shipping Method"] + ]) # displays the Distributor name in the respective column # does not display the header row @@ -526,16 +567,19 @@ # 1 TOTAL rows # 4 rows total - rows = find("table.report__table").all("tbody tr") - table = rows.map { |r| r.all("td").map { |c| c.text.strip } } - - expect(table[0]).to eq(["Distributor Name", "Another Supplier Name", "Salted Peanuts", - "1g Bag, S", "2", "10.0", "20.0", "", "UPS Ground"]) - expect(table[1]).to eq(["Distributor Name", "Supplier Name", "Baked Beans", - "1g Small, S", "3", "10.0", "30.0", "", "UPS Ground"]) - expect(table[2]).to eq(["Distributor Name", "Supplier Name", "Baked Beans", - "1g Big, S", "3", "10.0", "30.0", "", "UPS Ground"]) - expect(table[3]).to eq(["", "", "", "", "", "TOTAL", "80.0", "0.0", ""]) + expect(table_headers[0]).to eq( + ["Distributor Name", "Another Supplier Name", "Salted Peanuts", + "1g Bag, S", "2", "10.0", "20.0", "", "UPS Ground"] + ) + expect(table_headers[1]).to eq( + ["Distributor Name", "Supplier Name", "Baked Beans", + "1g Small, S", "3", "10.0", "30.0", "", "UPS Ground"] + ) + expect(table_headers[2]).to eq( + ["Distributor Name", "Supplier Name", "Baked Beans", + "1g Big, S", "3", "10.0", "30.0", "", "UPS Ground"] + ) + expect(table_headers[3]).to eq(["", "", "", "", "", "TOTAL", "80.0", "0.0", ""]) end end @@ -547,20 +591,17 @@ it "displays the report" do run_report - rows = find("table.report__table").all("thead tr") - table = rows.map { |r| r.all("th").map { |c| c.text.strip } } - # hides the Hub column - expect(table).to eq([ - ["Producer", - "Product", - "Variant", - "Quantity", - "Curr. Cost per Unit", - "Total Cost", - "Total Shipping Cost", - "Shipping Method"] - ]) + expect(table_headers).to eq([ + ["Producer", + "Product", + "Variant", + "Quantity", + "Curr. Cost per Unit", + "Total Cost", + "Total Shipping Cost", + "Shipping Method"] + ]) # displays the Distributor name in own row within "td.header-row" do From 632184b0a8f180771a68205ffb291927891df8c2 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Wed, 4 Sep 2024 17:52:26 -0600 Subject: [PATCH 08/10] Addresses Davids review --- .../reports/orders_and_distributors_spec.rb | 23 ++++++++++++----- .../reports/orders_and_fulfillment_spec.rb | 25 ------------------- 2 files changed, 17 insertions(+), 31 deletions(-) diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index 7a79f6e127d..7da1057c4e0 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -67,7 +67,9 @@ expect(all('table.report__table tbody tr').count).to eq(5) # displays only orders from the hub it is managing - expect(page).to have_content(distributor.name, count: 6) + within ".report__table" do + expect(page).to have_content(distributor.name, count: 5) + end # only sees line items from orders it manages expect(page).not_to have_content(distributor2.name) @@ -137,7 +139,9 @@ run_report # Then I should see the rows for the first order but not the second # One row per line item - order3 only - expect(page).to have_text(distributor.name, count: 6) + within ".report__table" do + expect(page).to have_content(distributor.name, count: 5) + end expect(page).to have_text(order3.email, count: 5) # setting a time interval to include both orders @@ -145,8 +149,10 @@ select_dates_from_daterangepicker datetime_start2, Time.zone.now run_report - # Then I should see the rows for both orders - expect(page).to have_text(distributor.name, count: 11) + # Then I should see the both orders + within ".report__table" do + expect(page).to have_content(distributor.name, count: 10) + end expect(page).to have_text(order3.email, count: 5) expect(page).to have_text(order4.email, count: 5) end @@ -157,14 +163,19 @@ # for one distributor select2_select distributor.name, from: "q_distributor_id_in" run_report - expect(page).to have_content(distributor.name), count: 15 + within ".report__table" do + expect(page).to have_content(distributor.name, count: 15) + end clear_select2("#s2id_q_distributor_id_in") # for another distributor select2_select distributor2.name, from: "q_distributor_id_in" run_report - expect(page).to have_content(distributor2.name), count: 5 + + within ".report__table" do + expect(page).to have_content(distributor2.name, count: 5) + end end end end diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index 4f952b28623..ca9355a0dff 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -481,31 +481,6 @@ expect(table_headers[3]).to eq(["", "", "", "TOTAL", "3", "", "30.0", ""]) end end - - context "with the header row option selected" do - before do - find("#display_header_row").set(true) # displays the header row - run_report - end - - it "displays the report" do - # hides the producer column - expect(table_headers).to eq([ - ["Product", - "Variant", - "Quantity", - "Curr. Cost per Unit", - "Total Cost", - "Shipping Method"] - ]) - - # displays the producer name in own row - within "td.header-row" do - expect(page).to have_content("Supplier Name") - end - end ->>>>>>> 50da07bb61 (Addresses reviews. The biggest change is moving the table CSS) - end end end end From b7aaab204cd3c5685d5c16d653bfd884565eea7c Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Thu, 5 Sep 2024 16:50:33 -0600 Subject: [PATCH 09/10] Adds timer restriction with Timecop The datet-time-picker test case was failing for me locally, but passing on GH-Actions. Controlling the time should prevent this type of flakyness --- spec/system/admin/reports/orders_and_distributors_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/system/admin/reports/orders_and_distributors_spec.rb b/spec/system/admin/reports/orders_and_distributors_spec.rb index 7da1057c4e0..3ee051923b5 100644 --- a/spec/system/admin/reports/orders_and_distributors_spec.rb +++ b/spec/system/admin/reports/orders_and_distributors_spec.rb @@ -13,6 +13,10 @@ let!(:distributor2) { create(:distributor_enterprise, name: "By Moto") } let!(:completed_at) { Time.zone.now.to_fs(:db) } + around do |example| + Timecop.travel(completed_at) { example.run } + end + let!(:order) { create(:order_ready_to_ship, distributor_id: distributor.id, completed_at:) } From 556539d1b175fb43f9a09a10834684256fff607f Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Mon, 9 Sep 2024 17:32:25 -0600 Subject: [PATCH 10/10] Removes pending from fixed issue The pending was not signalling the bug fix as ordering needed to be corrected --- .../reports/orders_and_fulfillment_spec.rb | 43 ++++++++----------- 1 file changed, 19 insertions(+), 24 deletions(-) diff --git a/spec/system/admin/reports/orders_and_fulfillment_spec.rb b/spec/system/admin/reports/orders_and_fulfillment_spec.rb index ca9355a0dff..4495b827001 100644 --- a/spec/system/admin/reports/orders_and_fulfillment_spec.rb +++ b/spec/system/admin/reports/orders_and_fulfillment_spec.rb @@ -357,20 +357,17 @@ end it "displays the report" do - rows = find("table.report__table").all("thead tr") - table = rows.map { |r| r.all("th").map { |c| c.text.strip } } - # displays the producer column - expect(table).to eq([ - ["Producer", - "Product", - "Variant", - "Hub", - "Quantity", - "Curr. Cost per Unit", - "Total Cost", - "Shipping Method"] - ]) + expect(table_headers).to eq([ + ["Producer", + "Product", + "Variant", + "Hub", + "Quantity", + "Curr. Cost per Unit", + "Total Cost", + "Shipping Method"] + ]) # displays the producer name in the respective column # does not display the header row @@ -380,8 +377,7 @@ end end - xit "aggregates results per variant" do - pending '#9678' + it "aggregates results per variant" do expect(all('table.report__table tbody tr').count).to eq(4) # 1 row per variant = 2 rows # 2 TOTAL rows @@ -390,12 +386,12 @@ rows = find("table.report__table").all("tbody tr") table = rows.map { |r| r.all("td").map { |c| c.text.strip } } - expect(table[0]).to eq(["Supplier Name", "Baked Beans", "1g Small, S", - "Distributor Name", "7", "10.0", "70.0", "UPS Ground"]) - expect(table[1]).to eq(["", "", "", "TOTAL", "7", "", "70.0", ""]) - expect(table[2]).to eq(["Supplier Name", "Baked Beans", "1g Big, S", + expect(table[0]).to eq(["Supplier Name", "Baked Beans", "1g Big", "Distributor Name", "3", "10.0", "30.0", "UPS Ground"]) - expect(table[3]).to eq(["", "", "", "TOTAL", "3", "", "30.0", ""]) + expect(table[1]).to eq(["", "", "", "TOTAL", "3", "", "30.0", ""]) + expect(table[2]).to eq(["Supplier Name", "Baked Beans", "1g Small", + "Distributor Name", "7", "10.0", "70.0", "UPS Ground"]) + expect(table[3]).to eq(["", "", "", "TOTAL", "7", "", "70.0", ""]) end end @@ -459,22 +455,21 @@ end end - xit "aggregates results per variant" do - pending '#9678' + it "aggregates results per variant" do expect(all('table.report__table tbody tr').count).to eq(4) # 1 row per variant = 2 rows # 2 TOTAL rows # 4 rows total expect(table_headers[0]).to eq( - ["Supplier Name", "Baked Beans", "1g Small, S", + ["Supplier Name", "Baked Beans", "1g Small", "Distributor Name", "7", "10.0", "70.0", "UPS Ground"] ) expect(table_headers[1]).to eq( ["", "", "", "TOTAL", "7", "", "70.0", ""] ) expect(table_headers[2]).to eq( - ["Supplier Name", "Baked Beans", "1g Big, S", + ["Supplier Name", "Baked Beans", "1g Big", "Distributor Name", "3", "10.0", "30.0", "UPS Ground"] )