From d96d6b23378124b0cab264dd145a9ba4fec88b84 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 15:21:03 +0100 Subject: [PATCH 1/9] Split orders_spec in two: tests for orders list page and tests for orders edit page --- spec/features/admin/order_spec.rb | 373 +++++++++++++++++++++++++++++ spec/features/admin/orders_spec.rb | 299 ----------------------- 2 files changed, 373 insertions(+), 299 deletions(-) create mode 100644 spec/features/admin/order_spec.rb diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb new file mode 100644 index 0000000000..a957e47617 --- /dev/null +++ b/spec/features/admin/order_spec.rb @@ -0,0 +1,373 @@ +require "spec_helper" +include ActionView::Helpers::NumberHelper + +feature ' + As an administrator + I want to create and edit orders +', js: true do + include AuthenticationWorkflow + include WebHelper + include CheckoutHelper + + background do + @user = create(:user) + @product = create(:simple_product) + @distributor = create(:distributor_enterprise, owner: @user, charges_sales_tax: true) + @order_cycle = create(:simple_order_cycle, name: 'One', distributors: [@distributor], variants: [@product.variants.first]) + + @order = create(:order_with_totals_and_distribution, user: @user, distributor: @distributor, order_cycle: @order_cycle, state: 'complete', payment_state: 'balance_due') + @customer = create(:customer, enterprise: @distributor, email: @user.email, user: @user, ship_address: create(:address)) + + # ensure order has a payment to capture + @order.finalize! + + create :check_payment, order: @order, amount: @order.total + end + + def new_order_with_distribution(distributor, order_cycle) + visit 'admin/orders/new' + expect(page).to have_selector('#s2id_order_distributor_id') + select2_select distributor.name, from: 'order_distributor_id' + select2_select order_cycle.name, from: 'order_order_cycle_id' + click_button 'Next' + end + + scenario "creating an order with distributor and order cycle" do + distributor_disabled = create(:distributor_enterprise) + create(:simple_order_cycle, name: 'Two') + + quick_login_as_admin + + visit '/admin/orders' + click_link 'New Order' + + # Distributors without an order cycle should be shown as disabled + open_select2('#s2id_order_distributor_id') + expect(page).to have_selector "ul.select2-results li.select2-result.select2-disabled", text: distributor_disabled.name + close_select2('#s2id_order_distributor_id') + + # Order cycle selector should be disabled + expect(page).to have_selector "#s2id_order_order_cycle_id.select2-container-disabled" + + # When we select a distributor, it should limit order cycle selection to those for that distributor + select2_select @distributor.name, from: 'order_distributor_id' + expect(page).to have_select2 'order_order_cycle_id', options: ['One (open)'] + select2_select @order_cycle.name, from: 'order_order_cycle_id' + click_button 'Next' + + # it suppresses validation errors when setting distribution + expect(page).not_to have_selector '#errorExplanation' + expect(page).to have_content 'ADD PRODUCT' + targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + find('button.add_variant').click + page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS + expect(page).to have_selector 'td', text: @product.name + + click_button 'Update' + + expect(page).to have_selector 'h1', text: 'Customer Details' + o = Spree::Order.last + expect(o.distributor).to eq(@distributor) + expect(o.order_cycle).to eq(@order_cycle) + end + + scenario "can add a product to an existing order" do + quick_login_as_admin + visit '/admin/orders' + + click_icon :edit + + targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + + find('button.add_variant').click + + expect(page).to have_selector 'td', text: @product.name + expect(@order.line_items(true).map(&:product)).to include @product + end + + scenario "displays error when incorrect distribution for products is chosen" do + d = create(:distributor_enterprise) + oc = create(:simple_order_cycle, distributors: [d]) + + # Move the order back to the cart state + @order.state = 'cart' + @order.completed_at = nil + # A nil user keeps the order in the cart state + # Even if the edit page tries to automatically progress the order workflow + @order.user = nil + @order.save + + quick_login_as_admin + visit '/admin/orders' + uncheck 'Only show complete orders' + page.find('a.icon-search').click + + click_icon :edit + expect(page).to have_select2 "order_distributor_id", with_options: [d.name] + select2_select d.name, from: 'order_distributor_id' + select2_select oc.name, from: 'order_order_cycle_id' + + click_button 'Update And Recalculate Fees' + expect(page).to have_content "Distributor or order cycle cannot supply the products in your cart" + end + + scenario "can't add products to an order outside the order's hub and order cycle" do + product = create(:simple_product) + + quick_login_as_admin + visit '/admin/orders' + page.find('td.actions a.icon-edit').click + + expect(page).not_to have_select2 "add_variant_id", with_options: [product.name] + end + + scenario "can't change distributor or order cycle once order has been finalized" do + quick_login_as_admin + visit '/admin/orders' + page.find('td.actions a.icon-edit').click + + expect(page).not_to have_select2 'order_distributor_id' + expect(page).not_to have_select2 'order_order_cycle_id' + + expect(page).to have_selector 'p', text: "Distributor: #{@order.distributor.name}" + expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" + end + + scenario "filling customer details" do + # Given a customer with an order, which includes their shipping and billing address + + # We change the 1st order's address details + # This way we validate that the original details (stored in customer) are picked up in the 2nd order + @order.ship_address = create(:address, lastname: 'Ship') + @order.bill_address = create(:address, lastname: 'Bill') + @order.save! + + # We set the existing shipping method to delivery, this shipping method will be used in the 2nd order + # Otherwise order_updater.shipping_address_from_distributor will set the 2nd order address to the distributor address + @order.shipping_method.update_attribute :require_ship_address, true + + # When I create a new order + quick_login_as @user + new_order_with_distribution(@distributor, @order_cycle) + targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + find('button.add_variant').click + page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS + click_button 'Update' + + expect(page).to have_selector 'h1.page-title', text: "Customer Details" + + # And I select that customer's email address and save the order + targetted_select2_search @customer.email, from: '#customer_search_override', dropdown_css: '.select2-drop' + click_button 'Update' + expect(page).to have_selector "h1.page-title", text: "Customer Details" + + # Then their addresses should be associated with the order + order = Spree::Order.last + expect(order.ship_address.lastname).to eq @customer.ship_address.lastname + expect(order.bill_address.lastname).to eq @customer.bill_address.lastname + end + + context "as an enterprise manager" do + let(:coordinator1) { create(:distributor_enterprise) } + let(:coordinator2) { create(:distributor_enterprise) } + let!(:order_cycle1) { create(:order_cycle, coordinator: coordinator1) } + let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) } + let!(:supplier1) { order_cycle1.suppliers.first } + let!(:supplier2) { order_cycle1.suppliers.last } + let!(:distributor1) { order_cycle1.distributors.first } + let!(:distributor2) { order_cycle1.distributors.reject{ |d| d == distributor1 }.last } # ensure d1 != d2 + let(:product) { order_cycle1.products.first } + + before(:each) do + @enterprise_user = create_enterprise_user + @enterprise_user.enterprise_roles.build(enterprise: supplier1).save + @enterprise_user.enterprise_roles.build(enterprise: coordinator1).save + @enterprise_user.enterprise_roles.build(enterprise: distributor1).save + + quick_login_as @enterprise_user + end + + feature "viewing the edit page" do + let!(:shipping_method_for_distributor1) { create(:shipping_method, name: "Normal", distributors: [distributor1]) } + let!(:different_shipping_method_for_distributor1) { create(:shipping_method, name: "Different", distributors: [distributor1]) } + let!(:shipping_method_for_distributor2) { create(:shipping_method, name: "Other", distributors: [distributor2]) } + + let!(:order) do + create(:order_with_taxes, distributor: distributor1, ship_address: create(:address), + product_price: 110, tax_rate_amount: 0.1, + tax_rate_name: "Tax 1").tap do |record| + Spree::TaxRate.adjust(record) + record.update_shipping_fees! + end + end + + background do + Spree::Config[:enable_receipt_printing?] = true + distributor1.update_attribute(:abn, '12345678') + + visit spree.edit_admin_order_path(order) + end + + scenario "shows a list of line_items" do + within('table.index tbody', match: :first) do + order.line_items.each do |item| + expect(page).to have_selector "td", match: :first, text: item.full_name + expect(page).to have_selector "td.item-price", text: item.single_display_amount + expect(page).to have_selector "input#quantity[value='#{item.quantity}']", visible: false + expect(page).to have_selector "td.item-total", text: item.display_amount + end + end + end + + scenario "shows the order items total" do + within('fieldset#order-total') do + expect(page).to have_selector "span.order-total", text: order.display_item_total + end + end + + scenario "shows the order non-tax adjustments" do + within('table.index tbody') do + order.adjustments.eligible.each do |adjustment| + expect(page).to have_selector "td", match: :first, text: adjustment.label + expect(page).to have_selector "td.total", text: adjustment.display_amount + end + end + end + + scenario "shows the order total" do + expect(page).to have_selector "fieldset#order-total", text: order.display_total + end + + scenario "shows the order tax adjustments" do + within('fieldset', text: I18n.t('spree.admin.orders.form.line_item_adjustments').upcase) do + expect(page).to have_selector "td", match: :first, text: "Tax 1" + expect(page).to have_selector "td.total", text: Spree::Money.new(10) + end + end + + scenario "shows the dropdown menu" do + find("#links-dropdown .ofn-drop-down").click + within "#links-dropdown" do + expect(page).to have_link "Resend Confirmation", href: spree.resend_admin_order_path(order) + expect(page).to have_link "Send Invoice", href: spree.invoice_admin_order_path(order) + expect(page).to have_link "Print Invoice", href: spree.print_admin_order_path(order) + expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, e: 'cancel') + end + end + + scenario "cannot split the order in different stock locations" do + # There's only 1 stock location in OFN, so the split functionality that comes with spree should be hidden + expect(page).to_not have_selector '.split-item' + end + + scenario "can edit shipping method" do + expect(page).to_not have_content different_shipping_method_for_distributor1.name + + find('.edit-method').click + expect(page).to have_select2 'selected_shipping_rate_id', with_options: [shipping_method_for_distributor1.name, different_shipping_method_for_distributor1.name], without_options: [shipping_method_for_distributor2.name] + select2_select different_shipping_method_for_distributor1.name, from: 'selected_shipping_rate_id' + find('.save-method').click + + expect(page).to have_content different_shipping_method_for_distributor1.name + end + + scenario "can edit tracking number" do + test_tracking_number = "ABCCBA" + expect(page).to_not have_content test_tracking_number + + find('.edit-tracking').click + fill_in "tracking", with: test_tracking_number + find('.save-tracking').click + + expect(page).to have_content test_tracking_number + end + + scenario "can print an order's ticket" do + find("#links-dropdown .ofn-drop-down").click + + ticket_window = window_opened_by do + within('#links-dropdown') do + click_link('Print Ticket') + end + end + + within_window ticket_window do + accept_alert do + print_data = page.evaluate_script('printData'); + elements_in_print_data = + [ + order.distributor.name, + order.distributor.address.address_part1, + order.distributor.address.address_part2, + order.distributor.contact.email, + order.number, + order.line_items.map { |line_item| + [line_item.quantity.to_s, + line_item.product.name, + line_item.single_display_amount_with_adjustments.format(symbol: false, with_currency: false), + line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] + }, + checkout_adjustments_for(order, exclude: [:line_item]).reject { |a| a.amount == 0 }.map { |adjustment| + [raw(adjustment.label), + display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] + }, + order.display_total.format(with_currency: false), + display_checkout_taxes_hash(order).map { |tax_rate, tax_value| + [tax_rate, + tax_value.format(with_currency: false)] + }, + display_checkout_total_less_tax(order).format(with_currency: false) + ] + expect(print_data.join).to include(*elements_in_print_data.flatten) + end + end + end + + scenario "editing shipping fees" do + click_link "Adjustments" + page.find('td.actions a.icon-edit').click + + fill_in "Amount", with: "5" + click_button "Continue" + + expect(page.find("td.amount")).to have_content "$5.00" + end + + context "when an included variant has been deleted" do + let!(:deleted_variant) do + order.line_items.first.variant.tap(&:delete) + end + + it "still lists the variant in the order page" do + within ".stock-contents" do + expect(page).to have_content deleted_variant.product_and_full_name + end + end + end + end + + scenario "creating an order with distributor and order cycle" do + new_order_with_distribution(distributor1, order_cycle1) + + expect(page).to have_content 'ADD PRODUCT' + targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + + find('button.add_variant').click + page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS + expect(page).to have_selector 'td', text: product.name + + expect(page).to have_select2 'order_distributor_id', with_options: [distributor1.name] + expect(page).to_not have_select2 'order_distributor_id', with_options: [distributor2.name] + + expect(page).to have_select2 'order_order_cycle_id', with_options: ["#{order_cycle1.name} (open)"] + expect(page).to_not have_select2 'order_order_cycle_id', with_options: ["#{order_cycle2.name} (open)"] + + click_button 'Update' + + expect(page).to have_selector 'h1', text: 'Customer Details' + o = Spree::Order.last + expect(o.distributor).to eq distributor1 + expect(o.order_cycle).to eq order_cycle1 + end + end +end diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 09bec96ccf..79e6d444b2 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -24,14 +24,6 @@ feature ' create :check_payment, order: @order, amount: @order.total end - def new_order_with_distribution(distributor, order_cycle) - visit 'admin/orders/new' - expect(page).to have_selector('#s2id_order_distributor_id') - select2_select distributor.name, from: 'order_distributor_id' - select2_select order_cycle.name, from: 'order_order_cycle_id' - click_button 'Next' - end - scenario "order cycles appear in descending order by close date on orders page" do create(:simple_order_cycle, name: 'Two', orders_close_at: 2.weeks.from_now) create(:simple_order_cycle, name: 'Four', orders_close_at: 4.weeks.from_now) @@ -45,59 +37,6 @@ feature ' expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two.*One/m) end - scenario "creating an order with distributor and order cycle" do - distributor_disabled = create(:distributor_enterprise) - create(:simple_order_cycle, name: 'Two') - - quick_login_as_admin - - visit '/admin/orders' - click_link 'New Order' - - # Distributors without an order cycle should be shown as disabled - open_select2('#s2id_order_distributor_id') - expect(page).to have_selector "ul.select2-results li.select2-result.select2-disabled", text: distributor_disabled.name - close_select2('#s2id_order_distributor_id') - - # Order cycle selector should be disabled - expect(page).to have_selector "#s2id_order_order_cycle_id.select2-container-disabled" - - # When we select a distributor, it should limit order cycle selection to those for that distributor - select2_select @distributor.name, from: 'order_distributor_id' - expect(page).to have_select2 'order_order_cycle_id', options: ['One (open)'] - select2_select @order_cycle.name, from: 'order_order_cycle_id' - click_button 'Next' - - # it suppresses validation errors when setting distribution - expect(page).not_to have_selector '#errorExplanation' - expect(page).to have_content 'ADD PRODUCT' - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' - find('button.add_variant').click - page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS - expect(page).to have_selector 'td', text: @product.name - - click_button 'Update' - - expect(page).to have_selector 'h1', text: 'Customer Details' - o = Spree::Order.last - expect(o.distributor).to eq(@distributor) - expect(o.order_cycle).to eq(@order_cycle) - end - - scenario "can add a product to an existing order" do - quick_login_as_admin - visit '/admin/orders' - - click_icon :edit - - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' - - find('button.add_variant').click - - expect(page).to have_selector 'td', text: @product.name - expect(@order.line_items(true).map(&:product)).to include @product - end - scenario "displays error when incorrect distribution for products is chosen" do d = create(:distributor_enterprise) oc = create(:simple_order_cycle, distributors: [d]) @@ -146,40 +85,6 @@ feature ' expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" end - scenario "filling customer details" do - # Given a customer with an order, which includes their shipping and billing address - - # We change the 1st order's address details - # This way we validate that the original details (stored in customer) are picked up in the 2nd order - @order.ship_address = create(:address, lastname: 'Ship') - @order.bill_address = create(:address, lastname: 'Bill') - @order.save! - - # We set the existing shipping method to delivery, this shipping method will be used in the 2nd order - # Otherwise order_updater.shipping_address_from_distributor will set the 2nd order address to the distributor address - @order.shipping_method.update_attribute :require_ship_address, true - - # When I create a new order - quick_login_as @user - new_order_with_distribution(@distributor, @order_cycle) - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' - find('button.add_variant').click - page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS - click_button 'Update' - - expect(page).to have_selector 'h1.page-title', text: "Customer Details" - - # And I select that customer's email address and save the order - targetted_select2_search @customer.email, from: '#customer_search_override', dropdown_css: '.select2-drop' - click_button 'Update' - expect(page).to have_selector "h1.page-title", text: "Customer Details" - - # Then their addresses should be associated with the order - order = Spree::Order.last - expect(order.ship_address.lastname).to eq @customer.ship_address.lastname - expect(order.bill_address.lastname).to eq @customer.bill_address.lastname - end - scenario "capture payment from the orders index page" do quick_login_as_admin @@ -209,208 +114,4 @@ feature ' expect(page).to have_css "i.success" expect(@order.reload.shipments.any?(&:shipped?)).to be true end - - context "as an enterprise manager" do - let(:coordinator1) { create(:distributor_enterprise) } - let(:coordinator2) { create(:distributor_enterprise) } - let!(:order_cycle1) { create(:order_cycle, coordinator: coordinator1) } - let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) } - let!(:supplier1) { order_cycle1.suppliers.first } - let!(:supplier2) { order_cycle1.suppliers.last } - let!(:distributor1) { order_cycle1.distributors.first } - let!(:distributor2) { order_cycle1.distributors.reject{ |d| d == distributor1 }.last } # ensure d1 != d2 - let(:product) { order_cycle1.products.first } - - before(:each) do - @enterprise_user = create_enterprise_user - @enterprise_user.enterprise_roles.build(enterprise: supplier1).save - @enterprise_user.enterprise_roles.build(enterprise: coordinator1).save - @enterprise_user.enterprise_roles.build(enterprise: distributor1).save - - quick_login_as @enterprise_user - end - - feature "viewing the edit page" do - let!(:shipping_method_for_distributor1) { create(:shipping_method, name: "Normal", distributors: [distributor1]) } - let!(:different_shipping_method_for_distributor1) { create(:shipping_method, name: "Different", distributors: [distributor1]) } - let!(:shipping_method_for_distributor2) { create(:shipping_method, name: "Other", distributors: [distributor2]) } - - let!(:order) do - create(:order_with_taxes, distributor: distributor1, ship_address: create(:address), - product_price: 110, tax_rate_amount: 0.1, - tax_rate_name: "Tax 1").tap do |record| - Spree::TaxRate.adjust(record) - record.update_shipping_fees! - end - end - - background do - Spree::Config[:enable_receipt_printing?] = true - distributor1.update_attribute(:abn, '12345678') - - visit spree.edit_admin_order_path(order) - end - - scenario "shows a list of line_items" do - within('table.index tbody', match: :first) do - order.line_items.each do |item| - expect(page).to have_selector "td", match: :first, text: item.full_name - expect(page).to have_selector "td.item-price", text: item.single_display_amount - expect(page).to have_selector "input#quantity[value='#{item.quantity}']", visible: false - expect(page).to have_selector "td.item-total", text: item.display_amount - end - end - end - - scenario "shows the order items total" do - within('fieldset#order-total') do - expect(page).to have_selector "span.order-total", text: order.display_item_total - end - end - - scenario "shows the order non-tax adjustments" do - within('table.index tbody') do - order.adjustments.eligible.each do |adjustment| - expect(page).to have_selector "td", match: :first, text: adjustment.label - expect(page).to have_selector "td.total", text: adjustment.display_amount - end - end - end - - scenario "shows the order total" do - expect(page).to have_selector "fieldset#order-total", text: order.display_total - end - - scenario "shows the order tax adjustments" do - within('fieldset', text: I18n.t('spree.admin.orders.form.line_item_adjustments').upcase) do - expect(page).to have_selector "td", match: :first, text: "Tax 1" - expect(page).to have_selector "td.total", text: Spree::Money.new(10) - end - end - - scenario "shows the dropdown menu" do - find("#links-dropdown .ofn-drop-down").click - within "#links-dropdown" do - expect(page).to have_link "Resend Confirmation", href: spree.resend_admin_order_path(order) - expect(page).to have_link "Send Invoice", href: spree.invoice_admin_order_path(order) - expect(page).to have_link "Print Invoice", href: spree.print_admin_order_path(order) - expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, e: 'cancel') - end - end - - scenario "cannot split the order in different stock locations" do - # There's only 1 stock location in OFN, so the split functionality that comes with spree should be hidden - expect(page).to_not have_selector '.split-item' - end - - scenario "can edit shipping method" do - expect(page).to_not have_content different_shipping_method_for_distributor1.name - - find('.edit-method').click - expect(page).to have_select2 'selected_shipping_rate_id', with_options: [shipping_method_for_distributor1.name, different_shipping_method_for_distributor1.name], without_options: [shipping_method_for_distributor2.name] - select2_select different_shipping_method_for_distributor1.name, from: 'selected_shipping_rate_id' - find('.save-method').click - - expect(page).to have_content different_shipping_method_for_distributor1.name - end - - scenario "can edit tracking number" do - test_tracking_number = "ABCCBA" - expect(page).to_not have_content test_tracking_number - - find('.edit-tracking').click - fill_in "tracking", with: test_tracking_number - find('.save-tracking').click - - expect(page).to have_content test_tracking_number - end - - scenario "can print an order's ticket" do - find("#links-dropdown .ofn-drop-down").click - - ticket_window = window_opened_by do - within('#links-dropdown') do - click_link('Print Ticket') - end - end - - within_window ticket_window do - accept_alert do - print_data = page.evaluate_script('printData'); - elements_in_print_data = - [ - order.distributor.name, - order.distributor.address.address_part1, - order.distributor.address.address_part2, - order.distributor.contact.email, - order.number, - order.line_items.map { |line_item| - [line_item.quantity.to_s, - line_item.product.name, - line_item.single_display_amount_with_adjustments.format(symbol: false, with_currency: false), - line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] - }, - checkout_adjustments_for(order, exclude: [:line_item]).reject { |a| a.amount == 0 }.map { |adjustment| - [raw(adjustment.label), - display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] - }, - order.display_total.format(with_currency: false), - display_checkout_taxes_hash(order).map { |tax_rate, tax_value| - [tax_rate, - tax_value.format(with_currency: false)] - }, - display_checkout_total_less_tax(order).format(with_currency: false) - ] - expect(print_data.join).to include(*elements_in_print_data.flatten) - end - end - end - - scenario "editing shipping fees" do - click_link "Adjustments" - page.find('td.actions a.icon-edit').click - - fill_in "Amount", with: "5" - click_button "Continue" - - expect(page.find("td.amount")).to have_content "$5.00" - end - - context "when an included variant has been deleted" do - let!(:deleted_variant) do - order.line_items.first.variant.tap(&:delete) - end - - it "still lists the variant in the order page" do - within ".stock-contents" do - expect(page).to have_content deleted_variant.product_and_full_name - end - end - end - end - - scenario "creating an order with distributor and order cycle" do - new_order_with_distribution(distributor1, order_cycle1) - - expect(page).to have_content 'ADD PRODUCT' - targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' - - find('button.add_variant').click - page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS - expect(page).to have_selector 'td', text: product.name - - expect(page).to have_select2 'order_distributor_id', with_options: [distributor1.name] - expect(page).to_not have_select2 'order_distributor_id', with_options: [distributor2.name] - - expect(page).to have_select2 'order_order_cycle_id', with_options: ["#{order_cycle1.name} (open)"] - expect(page).to_not have_select2 'order_order_cycle_id', with_options: ["#{order_cycle2.name} (open)"] - - click_button 'Update' - - expect(page).to have_selector 'h1', text: 'Customer Details' - o = Spree::Order.last - expect(o.distributor).to eq distributor1 - expect(o.order_cycle).to eq order_cycle1 - end - end end From c455dfb609d684925d461b8f556ff777a367e7c4 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 15:58:43 +0100 Subject: [PATCH 2/9] Make some specs faster by going directly to the order edit page and move incomplete order spec to a specific context --- spec/features/admin/order_spec.rb | 22 ++-- spec/features/admin/orders_spec.rb | 160 ++++++++++++----------------- 2 files changed, 72 insertions(+), 110 deletions(-) diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index a957e47617..831e4d8824 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -25,7 +25,7 @@ feature ' end def new_order_with_distribution(distributor, order_cycle) - visit 'admin/orders/new' + visit spree.new_admin_order_path expect(page).to have_selector('#s2id_order_distributor_id') select2_select distributor.name, from: 'order_distributor_id' select2_select order_cycle.name, from: 'order_order_cycle_id' @@ -37,8 +37,7 @@ feature ' create(:simple_order_cycle, name: 'Two') quick_login_as_admin - - visit '/admin/orders' + visit spree.admin_orders_path click_link 'New Order' # Distributors without an order cycle should be shown as disabled @@ -73,9 +72,7 @@ feature ' scenario "can add a product to an existing order" do quick_login_as_admin - visit '/admin/orders' - - click_icon :edit + visit spree.edit_admin_order_path(@order) targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' @@ -98,11 +95,8 @@ feature ' @order.save quick_login_as_admin - visit '/admin/orders' - uncheck 'Only show complete orders' - page.find('a.icon-search').click + visit spree.edit_admin_order_path(@order) - click_icon :edit expect(page).to have_select2 "order_distributor_id", with_options: [d.name] select2_select d.name, from: 'order_distributor_id' select2_select oc.name, from: 'order_order_cycle_id' @@ -115,16 +109,14 @@ feature ' product = create(:simple_product) quick_login_as_admin - visit '/admin/orders' - page.find('td.actions a.icon-edit').click + visit spree.edit_admin_order_path(@order) expect(page).not_to have_select2 "add_variant_id", with_options: [product.name] end scenario "can't change distributor or order cycle once order has been finalized" do quick_login_as_admin - visit '/admin/orders' - page.find('td.actions a.icon-edit').click + visit spree.edit_admin_order_path(@order) expect(page).not_to have_select2 'order_distributor_id' expect(page).not_to have_select2 'order_order_cycle_id' @@ -330,7 +322,7 @@ feature ' fill_in "Amount", with: "5" click_button "Continue" - expect(page.find("td.amount")).to have_content "$5.00" + expect(page.find("td.amount")).to have_content "5.00" end context "when an included variant has been deleted" do diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 79e6d444b2..307fc71b9c 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -9,109 +9,79 @@ feature ' include WebHelper include CheckoutHelper - background do - @user = create(:user) - @product = create(:simple_product) - @distributor = create(:distributor_enterprise, owner: @user, charges_sales_tax: true) - @order_cycle = create(:simple_order_cycle, name: 'One', distributors: [@distributor], variants: [@product.variants.first]) + let(:user) { create(:user) } + let(:product) { create(:simple_product) } + let(:distributor) { create(:distributor_enterprise, owner: user, charges_sales_tax: true) } + let(:order_cycle) { create(:simple_order_cycle, name: 'One', distributors: [distributor], variants: [product.variants.first]) } - @order = create(:order_with_totals_and_distribution, user: @user, distributor: @distributor, order_cycle: @order_cycle, state: 'complete', payment_state: 'balance_due') - @customer = create(:customer, enterprise: @distributor, email: @user.email, user: @user, ship_address: create(:address)) + context "with complete order" do + before do + @order = create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') + @customer = create(:customer, enterprise: distributor, email: user.email, user: user, ship_address: create(:address)) - # ensure order has a payment to capture - @order.finalize! + # ensure order has a payment to capture + @order.finalize! - create :check_payment, order: @order, amount: @order.total + create :check_payment, order: @order, amount: @order.total + end + + scenario "order cycles appear in descending order by close date on orders page" do + create(:simple_order_cycle, name: 'Two', orders_close_at: 2.weeks.from_now) + create(:simple_order_cycle, name: 'Four', orders_close_at: 4.weeks.from_now) + create(:simple_order_cycle, name: 'Three', orders_close_at: 3.weeks.from_now) + + quick_login_as_admin + visit 'admin/orders' + + open_select2('#s2id_q_order_cycle_id_in') + + expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two.*One/m) + end + + scenario "capture payment from the orders index page" do + quick_login_as_admin + + visit spree.admin_orders_path + expect(page).to have_current_path spree.admin_orders_path + + # click the 'capture' link for the order + page.find("[data-powertip=Capture]").click + + expect(page).to have_css "i.success" + expect(page).to have_css "button.icon-road" + + # check the order was captured + expect(@order.reload.payment_state).to eq "paid" + + # we should still be on the same page + expect(page).to have_current_path spree.admin_orders_path + end + + scenario "ship order from the orders index page" do + @order.payments.first.capture! + quick_login_as_admin + visit spree.admin_orders_path + + page.find("[data-powertip=Ship]").click + + expect(page).to have_css "i.success" + expect(@order.reload.shipments.any?(&:shipped?)).to be true + end end - scenario "order cycles appear in descending order by close date on orders page" do - create(:simple_order_cycle, name: 'Two', orders_close_at: 2.weeks.from_now) - create(:simple_order_cycle, name: 'Four', orders_close_at: 4.weeks.from_now) - create(:simple_order_cycle, name: 'Three', orders_close_at: 3.weeks.from_now) + context "with incomplete order" do + scenario "can edit order" do + incomplete_order = create(:order, distributor: distributor, order_cycle: order_cycle) - quick_login_as_admin - visit 'admin/orders' + quick_login_as_admin - open_select2('#s2id_q_order_cycle_id_in') + visit spree.admin_orders_path + uncheck 'Only show complete orders' + page.find('a.icon-search').click - expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two.*One/m) - end + click_icon :edit - scenario "displays error when incorrect distribution for products is chosen" do - d = create(:distributor_enterprise) - oc = create(:simple_order_cycle, distributors: [d]) - - # Move the order back to the cart state - @order.state = 'cart' - @order.completed_at = nil - # A nil user keeps the order in the cart state - # Even if the edit page tries to automatically progress the order workflow - @order.user = nil - @order.save - - quick_login_as_admin - visit '/admin/orders' - uncheck 'Only show complete orders' - page.find('a.icon-search').click - - click_icon :edit - expect(page).to have_select2 "order_distributor_id", with_options: [d.name] - select2_select d.name, from: 'order_distributor_id' - select2_select oc.name, from: 'order_order_cycle_id' - - click_button 'Update And Recalculate Fees' - expect(page).to have_content "Distributor or order cycle cannot supply the products in your cart" - end - - scenario "can't add products to an order outside the order's hub and order cycle" do - product = create(:simple_product) - - quick_login_as_admin - visit '/admin/orders' - page.find('td.actions a.icon-edit').click - - expect(page).not_to have_select2 "add_variant_id", with_options: [product.name] - end - - scenario "can't change distributor or order cycle once order has been finalized" do - quick_login_as_admin - visit '/admin/orders' - page.find('td.actions a.icon-edit').click - - expect(page).not_to have_select2 'order_distributor_id' - expect(page).not_to have_select2 'order_order_cycle_id' - - expect(page).to have_selector 'p', text: "Distributor: #{@order.distributor.name}" - expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" - end - - scenario "capture payment from the orders index page" do - quick_login_as_admin - - visit spree.admin_orders_path - expect(page).to have_current_path spree.admin_orders_path - - # click the 'capture' link for the order - page.find("[data-powertip=Capture]").click - - expect(page).to have_css "i.success" - expect(page).to have_css "button.icon-road" - - # check the order was captured - expect(@order.reload.payment_state).to eq "paid" - - # we should still be on the same page - expect(page).to have_current_path spree.admin_orders_path - end - - scenario "ship order from the orders index page" do - @order.payments.first.capture! - quick_login_as_admin - visit spree.admin_orders_path - - page.find("[data-powertip=Ship]").click - - expect(page).to have_css "i.success" - expect(@order.reload.shipments.any?(&:shipped?)).to be true + expect(page).to have_current_path spree.edit_admin_order_path(incomplete_order) + end end end From e901615b61e3ada7e5cc0a0504dfe31260b9f31a Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 16:56:56 +0100 Subject: [PATCH 3/9] Make spec simpler --- spec/features/admin/orders_spec.rb | 61 +++++++++++++++--------------- 1 file changed, 30 insertions(+), 31 deletions(-) diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 307fc71b9c..b50f64a955 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -14,16 +14,8 @@ feature ' let(:distributor) { create(:distributor_enterprise, owner: user, charges_sales_tax: true) } let(:order_cycle) { create(:simple_order_cycle, name: 'One', distributors: [distributor], variants: [product.variants.first]) } - context "with complete order" do - before do - @order = create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') - @customer = create(:customer, enterprise: distributor, email: user.email, user: user, ship_address: create(:address)) - - # ensure order has a payment to capture - @order.finalize! - - create :check_payment, order: @order, amount: @order.total - end + context "with a complete order" do + let(:order) { create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') } scenario "order cycles appear in descending order by close date on orders page" do create(:simple_order_cycle, name: 'Two', orders_close_at: 2.weeks.from_now) @@ -35,37 +27,44 @@ feature ' open_select2('#s2id_q_order_cycle_id_in') - expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two.*One/m) + expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two/m) end - scenario "capture payment from the orders index page" do - quick_login_as_admin + context "with a capturable order" do + before do + order.finalize! # ensure order has a payment to capture + create :check_payment, order: order, amount: order.total + end - visit spree.admin_orders_path - expect(page).to have_current_path spree.admin_orders_path + scenario "capture payment" do + quick_login_as_admin - # click the 'capture' link for the order - page.find("[data-powertip=Capture]").click + visit spree.admin_orders_path + expect(page).to have_current_path spree.admin_orders_path - expect(page).to have_css "i.success" - expect(page).to have_css "button.icon-road" + # click the 'capture' link for the order + page.find("[data-powertip=Capture]").click - # check the order was captured - expect(@order.reload.payment_state).to eq "paid" + expect(page).to have_css "i.success" + expect(page).to have_css "button.icon-road" - # we should still be on the same page - expect(page).to have_current_path spree.admin_orders_path - end + # check the order was captured + expect(order.reload.payment_state).to eq "paid" - scenario "ship order from the orders index page" do - @order.payments.first.capture! - quick_login_as_admin - visit spree.admin_orders_path + # we should still be on the same page + expect(page).to have_current_path spree.admin_orders_path + end - page.find("[data-powertip=Ship]").click + scenario "ship order from the orders index page" do + order.payments.first.capture! + quick_login_as_admin + visit spree.admin_orders_path - expect(page).to have_css "i.success" - expect(@order.reload.shipments.any?(&:shipped?)).to be true + page.find("[data-powertip=Ship]").click + + expect(page).to have_css "i.success" + expect(order.reload.shipments.any?(&:shipped?)).to be true + end end end From c7fb85a71577d80e23806b6cbc79f27dc248da13 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 17:34:36 +0100 Subject: [PATCH 4/9] Replace background with members with before with let statements --- spec/features/admin/order_spec.rb | 76 +++++++++++++++--------------- spec/features/admin/orders_spec.rb | 1 - 2 files changed, 38 insertions(+), 39 deletions(-) diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 831e4d8824..4cc32bcdc0 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -9,19 +9,19 @@ feature ' include WebHelper include CheckoutHelper - background do - @user = create(:user) - @product = create(:simple_product) - @distributor = create(:distributor_enterprise, owner: @user, charges_sales_tax: true) - @order_cycle = create(:simple_order_cycle, name: 'One', distributors: [@distributor], variants: [@product.variants.first]) + let(:user) { create(:user) } + let(:product) { create(:simple_product) } + let(:distributor) { create(:distributor_enterprise, owner: user, charges_sales_tax: true) } + let(:order_cycle) { create(:simple_order_cycle, name: 'One', distributors: [distributor], variants: [product.variants.first]) } - @order = create(:order_with_totals_and_distribution, user: @user, distributor: @distributor, order_cycle: @order_cycle, state: 'complete', payment_state: 'balance_due') - @customer = create(:customer, enterprise: @distributor, email: @user.email, user: @user, ship_address: create(:address)) + let(:order) { create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') } + let(:customer) { create(:customer, enterprise: distributor, email: user.email, user: user, ship_address: create(:address)) } + before do # ensure order has a payment to capture - @order.finalize! + order.finalize! - create :check_payment, order: @order, amount: @order.total + create :check_payment, order: order, amount: order.total end def new_order_with_distribution(distributor, order_cycle) @@ -49,37 +49,37 @@ feature ' expect(page).to have_selector "#s2id_order_order_cycle_id.select2-container-disabled" # When we select a distributor, it should limit order cycle selection to those for that distributor - select2_select @distributor.name, from: 'order_distributor_id' + select2_select distributor.name, from: 'order_distributor_id' expect(page).to have_select2 'order_order_cycle_id', options: ['One (open)'] - select2_select @order_cycle.name, from: 'order_order_cycle_id' + select2_select order_cycle.name, from: 'order_order_cycle_id' click_button 'Next' # it suppresses validation errors when setting distribution expect(page).not_to have_selector '#errorExplanation' expect(page).to have_content 'ADD PRODUCT' - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' find('button.add_variant').click page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS - expect(page).to have_selector 'td', text: @product.name + expect(page).to have_selector 'td', text: product.name click_button 'Update' expect(page).to have_selector 'h1', text: 'Customer Details' o = Spree::Order.last - expect(o.distributor).to eq(@distributor) - expect(o.order_cycle).to eq(@order_cycle) + expect(o.distributor).to eq(distributor) + expect(o.order_cycle).to eq(order_cycle) end scenario "can add a product to an existing order" do quick_login_as_admin - visit spree.edit_admin_order_path(@order) + visit spree.edit_admin_order_path(order) - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' find('button.add_variant').click - expect(page).to have_selector 'td', text: @product.name - expect(@order.line_items(true).map(&:product)).to include @product + expect(page).to have_selector 'td', text: product.name + expect(order.line_items(true).map(&:product)).to include product end scenario "displays error when incorrect distribution for products is chosen" do @@ -87,15 +87,15 @@ feature ' oc = create(:simple_order_cycle, distributors: [d]) # Move the order back to the cart state - @order.state = 'cart' - @order.completed_at = nil + order.state = 'cart' + order.completed_at = nil # A nil user keeps the order in the cart state # Even if the edit page tries to automatically progress the order workflow - @order.user = nil - @order.save + order.user = nil + order.save quick_login_as_admin - visit spree.edit_admin_order_path(@order) + visit spree.edit_admin_order_path(order) expect(page).to have_select2 "order_distributor_id", with_options: [d.name] select2_select d.name, from: 'order_distributor_id' @@ -109,20 +109,20 @@ feature ' product = create(:simple_product) quick_login_as_admin - visit spree.edit_admin_order_path(@order) + visit spree.edit_admin_order_path(order) expect(page).not_to have_select2 "add_variant_id", with_options: [product.name] end scenario "can't change distributor or order cycle once order has been finalized" do quick_login_as_admin - visit spree.edit_admin_order_path(@order) + visit spree.edit_admin_order_path(order) expect(page).not_to have_select2 'order_distributor_id' expect(page).not_to have_select2 'order_order_cycle_id' - expect(page).to have_selector 'p', text: "Distributor: #{@order.distributor.name}" - expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" + expect(page).to have_selector 'p', text: "Distributor: #{order.distributor.name}" + expect(page).to have_selector 'p', text: "Order cycle: #{order.order_cycle.name}" end scenario "filling customer details" do @@ -130,18 +130,18 @@ feature ' # We change the 1st order's address details # This way we validate that the original details (stored in customer) are picked up in the 2nd order - @order.ship_address = create(:address, lastname: 'Ship') - @order.bill_address = create(:address, lastname: 'Bill') - @order.save! + order.ship_address = create(:address, lastname: 'Ship') + order.bill_address = create(:address, lastname: 'Bill') + order.save! # We set the existing shipping method to delivery, this shipping method will be used in the 2nd order # Otherwise order_updater.shipping_address_from_distributor will set the 2nd order address to the distributor address - @order.shipping_method.update_attribute :require_ship_address, true + order.shipping_method.update_attribute :require_ship_address, true # When I create a new order - quick_login_as @user - new_order_with_distribution(@distributor, @order_cycle) - targetted_select2_search @product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' + quick_login_as user + new_order_with_distribution(distributor, order_cycle) + targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' find('button.add_variant').click page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS click_button 'Update' @@ -149,14 +149,14 @@ feature ' expect(page).to have_selector 'h1.page-title', text: "Customer Details" # And I select that customer's email address and save the order - targetted_select2_search @customer.email, from: '#customer_search_override', dropdown_css: '.select2-drop' + targetted_select2_search customer.email, from: '#customer_search_override', dropdown_css: '.select2-drop' click_button 'Update' expect(page).to have_selector "h1.page-title", text: "Customer Details" # Then their addresses should be associated with the order order = Spree::Order.last - expect(order.ship_address.lastname).to eq @customer.ship_address.lastname - expect(order.bill_address.lastname).to eq @customer.bill_address.lastname + expect(order.ship_address.lastname).to eq customer.ship_address.lastname + expect(order.bill_address.lastname).to eq customer.bill_address.lastname end context "as an enterprise manager" do diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index b50f64a955..c005287f12 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -7,7 +7,6 @@ feature ' ', js: true do include AuthenticationWorkflow include WebHelper - include CheckoutHelper let(:user) { create(:user) } let(:product) { create(:simple_product) } From 8bd3062b16bc924e417c304c11b49d70a129c0d6 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 18:17:54 +0100 Subject: [PATCH 5/9] Fix rubocop issues --- .rubocop_manual_todo.yml | 2 - spec/features/admin/order_spec.rb | 143 +++++++++++++++++++---------- spec/features/admin/orders_spec.rb | 14 ++- 3 files changed, 106 insertions(+), 53 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index de9861fa40..b6926715ef 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -181,7 +181,6 @@ Layout/LineLength: - spec/features/admin/image_settings_spec.rb - spec/features/admin/multilingual_spec.rb - spec/features/admin/order_cycles_spec.rb - - spec/features/admin/orders_spec.rb - spec/features/admin/overview_spec.rb - spec/features/admin/payment_method_spec.rb - spec/features/admin/product_import_spec.rb @@ -468,7 +467,6 @@ Metrics/BlockLength: - spec/factories/shipping_method_factory.rb - spec/factories/subscription_factory.rb - spec/factories/variant_factory.rb - - spec/features/admin/orders_spec.rb - spec/features/consumer/shopping/embedded_shopfronts_spec.rb - spec/lib/open_food_network/group_buy_report_spec.rb - spec/models/tag_rule/discount_order_spec.rb diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 4cc32bcdc0..69cbcae456 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -1,10 +1,12 @@ +# frozen_string_literal: true + require "spec_helper" -include ActionView::Helpers::NumberHelper feature ' As an administrator I want to create and edit orders ', js: true do + include ActionView::Helpers::NumberHelper include AuthenticationWorkflow include WebHelper include CheckoutHelper @@ -12,10 +14,20 @@ feature ' let(:user) { create(:user) } let(:product) { create(:simple_product) } let(:distributor) { create(:distributor_enterprise, owner: user, charges_sales_tax: true) } - let(:order_cycle) { create(:simple_order_cycle, name: 'One', distributors: [distributor], variants: [product.variants.first]) } + let(:order_cycle) do + create(:simple_order_cycle, name: 'One', distributors: [distributor], + variants: [product.variants.first]) + end - let(:order) { create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') } - let(:customer) { create(:customer, enterprise: distributor, email: user.email, user: user, ship_address: create(:address)) } + let(:order) do + create(:order_with_totals_and_distribution, user: user, distributor: distributor, + order_cycle: order_cycle, state: 'complete', + payment_state: 'balance_due') + end + let(:customer) do + create(:customer, enterprise: distributor, email: user.email, + user: user, ship_address: create(:address)) + end before do # ensure order has a payment to capture @@ -42,13 +54,14 @@ feature ' # Distributors without an order cycle should be shown as disabled open_select2('#s2id_order_distributor_id') - expect(page).to have_selector "ul.select2-results li.select2-result.select2-disabled", text: distributor_disabled.name + expect(page).to have_selector "ul.select2-results li.select2-result.select2-disabled", + text: distributor_disabled.name close_select2('#s2id_order_distributor_id') # Order cycle selector should be disabled expect(page).to have_selector "#s2id_order_order_cycle_id.select2-container-disabled" - # When we select a distributor, it should limit order cycle selection to those for that distributor + # The distributor selector should limit the order cycle selection to those for that distributor select2_select distributor.name, from: 'order_distributor_id' expect(page).to have_select2 'order_order_cycle_id', options: ['One (open)'] select2_select order_cycle.name, from: 'order_order_cycle_id' @@ -102,7 +115,8 @@ feature ' select2_select oc.name, from: 'order_order_cycle_id' click_button 'Update And Recalculate Fees' - expect(page).to have_content "Distributor or order cycle cannot supply the products in your cart" + expect(page).to have_content "Distributor or order cycle " \ + "cannot supply the products in your cart" end scenario "can't add products to an order outside the order's hub and order cycle" do @@ -128,14 +142,15 @@ feature ' scenario "filling customer details" do # Given a customer with an order, which includes their shipping and billing address - # We change the 1st order's address details - # This way we validate that the original details (stored in customer) are picked up in the 2nd order + # We change the 1st order's address details, this way + # we validate that the original details (stored in customer) are picked up in the 2nd order order.ship_address = create(:address, lastname: 'Ship') order.bill_address = create(:address, lastname: 'Bill') order.save! - # We set the existing shipping method to delivery, this shipping method will be used in the 2nd order - # Otherwise order_updater.shipping_address_from_distributor will set the 2nd order address to the distributor address + # We set the existing ship method to delivery, this ship method will be used in the 2nd order + # Otherwise order_updater.shipping_address_from_distributor will set + # the 2nd order address to the distributor address order.shipping_method.update_attribute :require_ship_address, true # When I create a new order @@ -149,7 +164,8 @@ feature ' expect(page).to have_selector 'h1.page-title', text: "Customer Details" # And I select that customer's email address and save the order - targetted_select2_search customer.email, from: '#customer_search_override', dropdown_css: '.select2-drop' + targetted_select2_search customer.email, from: '#customer_search_override', + dropdown_css: '.select2-drop' click_button 'Update' expect(page).to have_selector "h1.page-title", text: "Customer Details" @@ -167,7 +183,9 @@ feature ' let!(:supplier1) { order_cycle1.suppliers.first } let!(:supplier2) { order_cycle1.suppliers.last } let!(:distributor1) { order_cycle1.distributors.first } - let!(:distributor2) { order_cycle1.distributors.reject{ |d| d == distributor1 }.last } # ensure d1 != d2 + let!(:distributor2) do + order_cycle1.distributors.reject{ |d| d == distributor1 }.last # ensure d1 != d2 + end let(:product) { order_cycle1.products.first } before(:each) do @@ -180,9 +198,15 @@ feature ' end feature "viewing the edit page" do - let!(:shipping_method_for_distributor1) { create(:shipping_method, name: "Normal", distributors: [distributor1]) } - let!(:different_shipping_method_for_distributor1) { create(:shipping_method, name: "Different", distributors: [distributor1]) } - let!(:shipping_method_for_distributor2) { create(:shipping_method, name: "Other", distributors: [distributor2]) } + let!(:shipping_method_for_distributor1) do + create(:shipping_method, name: "Normal", distributors: [distributor1]) + end + let!(:different_shipping_method_for_distributor1) do + create(:shipping_method, name: "Different", distributors: [distributor1]) + end + let!(:shipping_method_for_distributor2) do + create(:shipping_method, name: "Other", distributors: [distributor2]) + end let!(:order) do create(:order_with_taxes, distributor: distributor1, ship_address: create(:address), @@ -240,15 +264,18 @@ feature ' scenario "shows the dropdown menu" do find("#links-dropdown .ofn-drop-down").click within "#links-dropdown" do - expect(page).to have_link "Resend Confirmation", href: spree.resend_admin_order_path(order) + expect(page).to have_link "Resend Confirmation", + href: spree.resend_admin_order_path(order) expect(page).to have_link "Send Invoice", href: spree.invoice_admin_order_path(order) expect(page).to have_link "Print Invoice", href: spree.print_admin_order_path(order) - expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, e: 'cancel') + expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, + e: 'cancel') end end scenario "cannot split the order in different stock locations" do - # There's only 1 stock location in OFN, so the split functionality that comes with spree should be hidden + # There's only 1 stock location in OFN, + # so the split functionality that comes with spree should be hidden expect(page).to_not have_selector '.split-item' end @@ -256,8 +283,13 @@ feature ' expect(page).to_not have_content different_shipping_method_for_distributor1.name find('.edit-method').click - expect(page).to have_select2 'selected_shipping_rate_id', with_options: [shipping_method_for_distributor1.name, different_shipping_method_for_distributor1.name], without_options: [shipping_method_for_distributor2.name] - select2_select different_shipping_method_for_distributor1.name, from: 'selected_shipping_rate_id' + expect(page).to have_select2 'selected_shipping_rate_id', + with_options: [ + shipping_method_for_distributor1.name, + different_shipping_method_for_distributor1.name + ], without_options: [shipping_method_for_distributor2.name] + select2_select different_shipping_method_for_distributor1.name, + from: 'selected_shipping_rate_id' find('.save-method').click expect(page).to have_content different_shipping_method_for_distributor1.name @@ -286,35 +318,48 @@ feature ' within_window ticket_window do accept_alert do print_data = page.evaluate_script('printData'); - elements_in_print_data = - [ - order.distributor.name, - order.distributor.address.address_part1, - order.distributor.address.address_part2, - order.distributor.contact.email, - order.number, - order.line_items.map { |line_item| - [line_item.quantity.to_s, - line_item.product.name, - line_item.single_display_amount_with_adjustments.format(symbol: false, with_currency: false), - line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] - }, - checkout_adjustments_for(order, exclude: [:line_item]).reject { |a| a.amount == 0 }.map { |adjustment| - [raw(adjustment.label), - display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] - }, - order.display_total.format(with_currency: false), - display_checkout_taxes_hash(order).map { |tax_rate, tax_value| - [tax_rate, - tax_value.format(with_currency: false)] - }, - display_checkout_total_less_tax(order).format(with_currency: false) - ] + elements_in_print_data = [ + order.distributor.name, + order.distributor.address.address_part1, + order.distributor.address.address_part2, + order.distributor.contact.email, order.number, + line_items_in_print_data, + adjustments_in_print_data, + order.display_total.format(with_currency: false), + taxes_in_print_data, + display_checkout_total_less_tax(order).format(with_currency: false) + ] expect(print_data.join).to include(*elements_in_print_data.flatten) end end end + def line_items_in_print_data + order.line_items.map { |line_item| + [line_item.quantity.to_s, + line_item.product.name, + line_item.single_display_amount_with_adjustments.format(symbol: false, + with_currency: false), + line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] + } + end + + def adjustments_in_print_data + checkout_adjustments_for(order, exclude: [:line_item]). + reject { |a| a.amount == 0 }. + map do |adjustment| + [raw(adjustment.label), + display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] + end + end + + def taxes_in_print_data + display_checkout_taxes_hash(order).map { |tax_rate, tax_value| + [tax_rate, + tax_value.format(with_currency: false)] + } + end + scenario "editing shipping fees" do click_link "Adjustments" page.find('td.actions a.icon-edit').click @@ -345,14 +390,16 @@ feature ' targetted_select2_search product.name, from: '#add_variant_id', dropdown_css: '.select2-drop' find('button.add_variant').click - page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" # Wait for JS + page.has_selector? "table.index tbody[data-hook='admin_order_form_line_items'] tr" expect(page).to have_selector 'td', text: product.name expect(page).to have_select2 'order_distributor_id', with_options: [distributor1.name] expect(page).to_not have_select2 'order_distributor_id', with_options: [distributor2.name] - expect(page).to have_select2 'order_order_cycle_id', with_options: ["#{order_cycle1.name} (open)"] - expect(page).to_not have_select2 'order_order_cycle_id', with_options: ["#{order_cycle2.name} (open)"] + expect(page).to have_select2 'order_order_cycle_id', + with_options: ["#{order_cycle1.name} (open)"] + expect(page).to_not have_select2 'order_order_cycle_id', + with_options: ["#{order_cycle2.name} (open)"] click_button 'Update' diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index c005287f12..f17068799e 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -11,10 +11,17 @@ feature ' let(:user) { create(:user) } let(:product) { create(:simple_product) } let(:distributor) { create(:distributor_enterprise, owner: user, charges_sales_tax: true) } - let(:order_cycle) { create(:simple_order_cycle, name: 'One', distributors: [distributor], variants: [product.variants.first]) } + let(:order_cycle) do + create(:simple_order_cycle, name: 'One', distributors: [distributor], + variants: [product.variants.first]) + end context "with a complete order" do - let(:order) { create(:order_with_totals_and_distribution, user: user, distributor: distributor, order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') } + let(:order) do + create(:order_with_totals_and_distribution, user: user, distributor: distributor, + order_cycle: order_cycle, + state: 'complete', payment_state: 'balance_due') + end scenario "order cycles appear in descending order by close date on orders page" do create(:simple_order_cycle, name: 'Two', orders_close_at: 2.weeks.from_now) @@ -26,7 +33,8 @@ feature ' open_select2('#s2id_q_order_cycle_id_in') - expect(find('#q_order_cycle_id_in', visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two/m) + expect(find('#q_order_cycle_id_in', + visible: :all)[:innerHTML]).to have_content(/.*Four.*Three.*Two/m) end context "with a capturable order" do From c33352904aa60fd09a223aea36d866f80825eaa8 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 18:48:20 +0100 Subject: [PATCH 6/9] Make spec a bit more resilient --- spec/features/admin/order_spec.rb | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 69cbcae456..4d1b4ed2eb 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -24,10 +24,7 @@ feature ' order_cycle: order_cycle, state: 'complete', payment_state: 'balance_due') end - let(:customer) do - create(:customer, enterprise: distributor, email: user.email, - user: user, ship_address: create(:address)) - end + let(:customer) { order.customer } before do # ensure order has a payment to capture @@ -242,11 +239,9 @@ feature ' end scenario "shows the order non-tax adjustments" do - within('table.index tbody') do - order.adjustments.eligible.each do |adjustment| - expect(page).to have_selector "td", match: :first, text: adjustment.label - expect(page).to have_selector "td.total", text: adjustment.display_amount - end + order.adjustments.eligible.each do |adjustment| + expect(page).to have_selector "td", match: :first, text: adjustment.label + expect(page).to have_selector "td.total", text: adjustment.display_amount end end @@ -362,12 +357,13 @@ feature ' scenario "editing shipping fees" do click_link "Adjustments" - page.find('td.actions a.icon-edit').click + shipping_adjustment_tr_selector = "tr#spree_adjustment_#{order.adjustments.shipping.first.id}" + page.find("#{shipping_adjustment_tr_selector} td.actions a.icon-edit").click fill_in "Amount", with: "5" click_button "Continue" - expect(page.find("td.amount")).to have_content "5.00" + expect(page.find("#{shipping_adjustment_tr_selector} td.amount")).to have_content "5.00" end context "when an included variant has been deleted" do From c1b28543c60f78bcebf512cf2f6707df38de7a9e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 19:57:42 +0100 Subject: [PATCH 7/9] Extract print ticket spec to a separate file --- .../features/admin/order_print_ticket_spec.rb | 93 +++++++++++++++++ spec/features/admin/order_spec.rb | 99 +++++-------------- spec/features/admin/orders_spec.rb | 3 +- 3 files changed, 118 insertions(+), 77 deletions(-) create mode 100644 spec/features/admin/order_print_ticket_spec.rb diff --git a/spec/features/admin/order_print_ticket_spec.rb b/spec/features/admin/order_print_ticket_spec.rb new file mode 100644 index 0000000000..add2ef9e7d --- /dev/null +++ b/spec/features/admin/order_print_ticket_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require "spec_helper" + +feature ' + As an administrator + I want to print a ticket for an order +', js: true do + include AuthenticationWorkflow + include CheckoutHelper + include ActionView::Helpers::NumberHelper + + context "as an enterprise manager" do + let!(:shipping_method) { create(:shipping_method, distributors: [distributor]) } + let!(:distributor) { create(:distributor_enterprise) } + + let!(:order) do + create(:order_with_taxes, distributor: distributor, ship_address: create(:address), + product_price: 110, tax_rate_amount: 0.1, + tax_rate_name: "Tax 1").tap do |record| + Spree::TaxRate.adjust(record) + record.update_shipping_fees! + end + end + + before do + @enterprise_user = create_enterprise_user + @enterprise_user.enterprise_roles.build(enterprise: distributor).save + + quick_login_as @enterprise_user + + Spree::Config[:enable_receipt_printing?] = true + end + + feature "viewing the edit page" do + scenario "can print an order's ticket" do + visit spree.edit_admin_order_path(order) + + find("#links-dropdown .ofn-drop-down").click + + ticket_window = window_opened_by do + within('#links-dropdown') do + click_link('Print Ticket') + end + end + + within_window ticket_window do + accept_alert do + print_data = page.evaluate_script('printData'); + elements_in_print_data = [ + order.distributor.name, + order.distributor.address.address_part1, + order.distributor.address.address_part2, + order.distributor.contact.email, order.number, + line_items_in_print_data, + adjustments_in_print_data, + order.display_total.format(with_currency: false), + taxes_in_print_data, + display_checkout_total_less_tax(order).format(with_currency: false) + ] + expect(print_data.join).to include(*elements_in_print_data.flatten) + end + end + end + + def line_items_in_print_data + order.line_items.map { |line_item| + [line_item.quantity.to_s, + line_item.product.name, + line_item.single_display_amount_with_adjustments.format(symbol: false, + with_currency: false), + line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] + } + end + + def adjustments_in_print_data + checkout_adjustments_for(order, exclude: [:line_item]). + reject { |a| a.amount == 0 }. + map do |adjustment| + [raw(adjustment.label), + display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] + end + end + + def taxes_in_print_data + display_checkout_taxes_hash(order).map { |tax_rate, tax_value| + [tax_rate, + tax_value.format(with_currency: false)] + } + end + end + end +end diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 4d1b4ed2eb..28bf21999f 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -6,10 +6,8 @@ feature ' As an administrator I want to create and edit orders ', js: true do - include ActionView::Helpers::NumberHelper include AuthenticationWorkflow include WebHelper - include CheckoutHelper let(:user) { create(:user) } let(:product) { create(:simple_product) } @@ -198,13 +196,6 @@ feature ' let!(:shipping_method_for_distributor1) do create(:shipping_method, name: "Normal", distributors: [distributor1]) end - let!(:different_shipping_method_for_distributor1) do - create(:shipping_method, name: "Different", distributors: [distributor1]) - end - let!(:shipping_method_for_distributor2) do - create(:shipping_method, name: "Other", distributors: [distributor2]) - end - let!(:order) do create(:order_with_taxes, distributor: distributor1, ship_address: create(:address), product_price: 110, tax_rate_amount: 0.1, @@ -215,7 +206,6 @@ feature ' end background do - Spree::Config[:enable_receipt_printing?] = true distributor1.update_attribute(:abn, '12345678') visit spree.edit_admin_order_path(order) @@ -274,20 +264,31 @@ feature ' expect(page).to_not have_selector '.split-item' end - scenario "can edit shipping method" do - expect(page).to_not have_content different_shipping_method_for_distributor1.name + context "with different shipping methods" do + let!(:different_shipping_method_for_distributor1) do + create(:shipping_method, name: "Different", distributors: [distributor1]) + end + let!(:shipping_method_for_distributor2) do + create(:shipping_method, name: "Other", distributors: [distributor2]) + end - find('.edit-method').click - expect(page).to have_select2 'selected_shipping_rate_id', - with_options: [ - shipping_method_for_distributor1.name, - different_shipping_method_for_distributor1.name - ], without_options: [shipping_method_for_distributor2.name] - select2_select different_shipping_method_for_distributor1.name, - from: 'selected_shipping_rate_id' - find('.save-method').click + scenario "can edit shipping method" do + visit spree.edit_admin_order_path(order) - expect(page).to have_content different_shipping_method_for_distributor1.name + expect(page).to_not have_content different_shipping_method_for_distributor1.name + + find('.edit-method').click + expect(page).to have_select2 'selected_shipping_rate_id', + with_options: [ + shipping_method_for_distributor1.name, + different_shipping_method_for_distributor1.name + ], without_options: [shipping_method_for_distributor2.name] + select2_select different_shipping_method_for_distributor1.name, + from: 'selected_shipping_rate_id' + find('.save-method').click + + expect(page).to have_content different_shipping_method_for_distributor1.name + end end scenario "can edit tracking number" do @@ -301,60 +302,6 @@ feature ' expect(page).to have_content test_tracking_number end - scenario "can print an order's ticket" do - find("#links-dropdown .ofn-drop-down").click - - ticket_window = window_opened_by do - within('#links-dropdown') do - click_link('Print Ticket') - end - end - - within_window ticket_window do - accept_alert do - print_data = page.evaluate_script('printData'); - elements_in_print_data = [ - order.distributor.name, - order.distributor.address.address_part1, - order.distributor.address.address_part2, - order.distributor.contact.email, order.number, - line_items_in_print_data, - adjustments_in_print_data, - order.display_total.format(with_currency: false), - taxes_in_print_data, - display_checkout_total_less_tax(order).format(with_currency: false) - ] - expect(print_data.join).to include(*elements_in_print_data.flatten) - end - end - end - - def line_items_in_print_data - order.line_items.map { |line_item| - [line_item.quantity.to_s, - line_item.product.name, - line_item.single_display_amount_with_adjustments.format(symbol: false, - with_currency: false), - line_item.display_amount_with_adjustments.format(symbol: false, with_currency: false)] - } - end - - def adjustments_in_print_data - checkout_adjustments_for(order, exclude: [:line_item]). - reject { |a| a.amount == 0 }. - map do |adjustment| - [raw(adjustment.label), - display_adjustment_amount(adjustment).format(symbol: false, with_currency: false)] - end - end - - def taxes_in_print_data - display_checkout_taxes_hash(order).map { |tax_rate, tax_value| - [tax_rate, - tax_value.format(with_currency: false)] - } - end - scenario "editing shipping fees" do click_link "Adjustments" shipping_adjustment_tr_selector = "tr#spree_adjustment_#{order.adjustments.shipping.first.id}" diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index f17068799e..6713a7521f 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -1,5 +1,6 @@ +# frozen_string_literal: true + require "spec_helper" -include ActionView::Helpers::NumberHelper feature ' As an administrator From 8973a1b76cbb157bbc395b38641a3e5d9179b1c1 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 20:18:13 +0100 Subject: [PATCH 8/9] Merging 6 specs in one takes around 1 minute of execution time --- spec/features/admin/order_spec.rb | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/spec/features/admin/order_spec.rb b/spec/features/admin/order_spec.rb index 28bf21999f..d9071aff94 100644 --- a/spec/features/admin/order_spec.rb +++ b/spec/features/admin/order_spec.rb @@ -211,7 +211,8 @@ feature ' visit spree.edit_admin_order_path(order) end - scenario "shows a list of line_items" do + scenario "verifying page contents" do + # shows a list of line_items within('table.index tbody', match: :first) do order.line_items.each do |item| expect(page).to have_selector "td", match: :first, text: item.full_name @@ -220,41 +221,35 @@ feature ' expect(page).to have_selector "td.item-total", text: item.display_amount end end - end - scenario "shows the order items total" do + # shows the order items total within('fieldset#order-total') do expect(page).to have_selector "span.order-total", text: order.display_item_total end - end - scenario "shows the order non-tax adjustments" do + # shows the order non-tax adjustments order.adjustments.eligible.each do |adjustment| expect(page).to have_selector "td", match: :first, text: adjustment.label expect(page).to have_selector "td.total", text: adjustment.display_amount end - end - scenario "shows the order total" do + # shows the order total expect(page).to have_selector "fieldset#order-total", text: order.display_total - end - scenario "shows the order tax adjustments" do + # shows the order tax adjustments within('fieldset', text: I18n.t('spree.admin.orders.form.line_item_adjustments').upcase) do expect(page).to have_selector "td", match: :first, text: "Tax 1" expect(page).to have_selector "td.total", text: Spree::Money.new(10) end - end - scenario "shows the dropdown menu" do + # shows the dropdown menu" do find("#links-dropdown .ofn-drop-down").click within "#links-dropdown" do expect(page).to have_link "Resend Confirmation", href: spree.resend_admin_order_path(order) expect(page).to have_link "Send Invoice", href: spree.invoice_admin_order_path(order) expect(page).to have_link "Print Invoice", href: spree.print_admin_order_path(order) - expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, - e: 'cancel') + expect(page).to have_link "Cancel Order", href: spree.fire_admin_order_path(order, e: 'cancel') end end From 8f8dce4baba19aa370387612f9ab2b6edd76558e Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Fri, 17 Apr 2020 20:23:29 +0100 Subject: [PATCH 9/9] Do not render inventory items in the shipment that dont have a line item in the order --- app/views/spree/admin/orders/_shipment_manifest.html.haml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/spree/admin/orders/_shipment_manifest.html.haml b/app/views/spree/admin/orders/_shipment_manifest.html.haml index 8651ee3188..611cf4a5d7 100644 --- a/app/views/spree/admin/orders/_shipment_manifest.html.haml +++ b/app/views/spree/admin/orders/_shipment_manifest.html.haml @@ -1,5 +1,6 @@ - shipment.manifest.each do |item| - line_item = order.find_line_item_by_variant(item.variant) + - break if line_item.blank? %tr.stock-item{ "data-item-quantity" => "#{item.quantity}" } %td.item-image