From bcc02f35e7e2d81fbc21085aba09f566f13f1914 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 19 Jun 2019 13:29:30 +0100 Subject: [PATCH 1/9] Add new preference to invoice settings: enable invoices --- app/models/spree/app_configuration_decorator.rb | 1 + app/views/admin/invoice_settings/edit.html.haml | 5 +++++ config/locales/en.yml | 7 ++++--- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index 2df7c02ea7..6d0b48f666 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -29,6 +29,7 @@ Spree::AppConfiguration.class_eval do preference :matomo_site_id, :string, default: nil # Invoices & Receipts + preference :enable_invoices?, :boolean, default: true preference :invoice_style2?, :boolean, default: false preference :enable_receipt_printing?, :boolean, default: false diff --git a/app/views/admin/invoice_settings/edit.html.haml b/app/views/admin/invoice_settings/edit.html.haml index e134356927..85b53999df 100644 --- a/app/views/admin/invoice_settings/edit.html.haml +++ b/app/views/admin/invoice_settings/edit.html.haml @@ -5,6 +5,11 @@ = form_tag main_app.admin_invoice_settings_path, :method => :put do + .field.align-center + = hidden_field_tag 'preferences[enable_invoices?]', '0' + = check_box_tag 'preferences[enable_invoices?]', '1', Spree::Config[:enable_invoices?] + = label_tag nil, t('.enable_invoices?') + .field.align-center = hidden_field_tag 'preferences[invoice_style2?]', '0' = check_box_tag 'preferences[invoice_style2?]', '1', Spree::Config[:invoice_style2?] diff --git a/config/locales/en.yml b/config/locales/en.yml index 6ec88c03fd..7c2ec27cb8 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -359,9 +359,10 @@ en: invoice_settings: edit: - title: Invoice Settings - invoice_style2?: Use the alternative invoice model that includes total tax breakdown per rate and tax rate info per item (not yet suitable for countries displaying prices excluding tax) - enable_receipt_printing?: Show options for printing receipts using thermal printers in order dropdown? + title: "Invoice Settings" + enable_invoices?: "Enable Invoices?" + invoice_style2?: "Use the alternative invoice model that includes total tax breakdown per rate and tax rate info per item (not yet suitable for countries displaying prices excluding tax)" + enable_receipt_printing?: "Show options for printing receipts using thermal printers in order dropdown?" stripe_connect_settings: edit: From 95170bacd5a8f1eb380b1b89dbcf3d542a21a090 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 19 Jun 2019 15:02:15 +0100 Subject: [PATCH 2/9] Show print invoices button in orders list page only if invoices are enabled in the backoffice --- app/views/spree/admin/orders/index.html.haml | 5 +++-- spec/features/admin/orders_spec.rb | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/views/spree/admin/orders/index.html.haml b/app/views/spree/admin/orders/index.html.haml index fcf1b7cbc5..3f1e087b6e 100644 --- a/app/views/spree/admin/orders/index.html.haml +++ b/app/views/spree/admin/orders/index.html.haml @@ -19,8 +19,9 @@ .row.index-controls{'ng-show' => '!RequestMonitor.loading && orders.length > 0'} = render partial: 'per_page_controls' - %button.invoices-modal{'ng-controller' => 'bulkInvoiceCtrl', 'ng-click' => 'createBulkInvoice()', 'ng-disabled' => 'selected_orders.length == 0'} - = t('.print_invoices') + - if Spree::Config[:enable_invoices?] + %button.invoices-modal{'ng-controller' => 'bulkInvoiceCtrl', 'ng-click' => 'createBulkInvoice()', 'ng-disabled' => 'selected_orders.length == 0'} + = t('.print_invoices') %table#listing_orders.index.responsive{width: "100%", 'ng-init' => 'initialise()', 'ng-show' => "!RequestMonitor.loading && orders.length > 0" } %colgroup diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index 3b54d31cdb..ce12f02870 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -145,6 +145,17 @@ feature ' expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" end + scenario "can't print invoices when invocies are disabled" do + original_invoices_setting = Spree::Config[:enable_invoices?] + + Spree::Config[:enable_invoices?] = false + quick_login_as_admin + visit '/admin/orders' + expect(page).to have_no_selector 'button.invoices-modal' + + Spree::Config[:enable_invoices?] = original_invoices_setting + end + scenario "filling customer details" do # Given a customer with an order, which includes their shipping and billing address From 67b5f08b0753424c2708c0173f45150dba1655d0 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 19 Jun 2019 15:33:32 +0100 Subject: [PATCH 3/9] Refactor OrderHelper.order_links: extract links object to separate methods --- .../spree/admin/orders_helper_decorator.rb | 85 +++++++++++++++++-- 1 file changed, 76 insertions(+), 9 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index 74cc53dcfb..b2f96b16da 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -2,27 +2,94 @@ module Spree module Admin module OrdersHelper def order_links(order) + @order ||= order links = [] - links << { name: t(:edit_order), url: edit_admin_order_path(order), icon: 'icon-edit' } unless action_name == "edit" + links << edit_order_link unless action_name == "edit" if @order.complete? - links << { name: t(:resend_confirmation), url: resend_admin_order_path(order), icon: 'icon-email', method: 'post', confirm: t(:confirm_resend_order_confirmation) } + links << resend_confirmation_link if @order.distributor.can_invoice? - links << { name: t(:send_invoice), url: invoice_admin_order_path(order), icon: 'icon-email', confirm: t(:confirm_send_invoice) } + links << send_invoice_link_with_url else - links << { name: t(:send_invoice), url: "#", icon: 'icon-email', confirm: t(:must_have_valid_business_number, enterprise_name: order.distributor.name) } + links << send_invoice_link_without_url end - links << { name: t(:print_invoice), url: print_admin_order_path(order), icon: 'icon-print', target: "_blank" } + links << print_invoice_link if Spree::Config.enable_receipt_printing? - links << { name: t(:print_ticket), url: print_ticket_admin_order_path(order), icon: 'icon-print', target: "_blank" } - links << { name: t(:select_ticket_printer), url: "#{print_ticket_admin_order_path(order)}#select-printer", icon: 'icon-print', target: "_blank" } + links << print_ticket_link + links << select_ticket_printer_link end end if @order.ready_to_ship? - links << { name: t(:ship_order), url: fire_admin_order_path(@order, e: 'ship'), method: 'put', icon: 'icon-truck', confirm: t(:are_you_sure) } + links << ship_order_link end - links << { name: t(:cancel_order), url: fire_admin_order_path(@order.number, e: 'cancel'), icon: 'icon-trash', confirm: t(:are_you_sure) } if order.can_cancel? + links << cancel_order_link if @order.can_cancel? links end + + private + + def edit_order_link + { name: t(:edit_order), + url: edit_admin_order_path(@order), + icon: 'icon-edit' } + end + + def resend_confirmation_link + { name: t(:resend_confirmation), + url: resend_admin_order_path(@order), + icon: 'icon-email', + method: 'post', + confirm: t(:confirm_resend_order_confirmation) } + end + + def send_invoice_link_with_url + { name: t(:send_invoice), + url: invoice_admin_order_path(@order), + icon: 'icon-email', + confirm: t(:confirm_send_invoice) } + end + + def send_invoice_link_without_url + { name: t(:send_invoice), + url: "#", + icon: 'icon-email', + confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) } + end + + def print_invoice_link + { name: t(:print_invoice), + url: print_admin_order_path(@order), + icon: 'icon-print', + target: "_blank" } + end + + def print_ticket_link + { name: t(:print_ticket), + url: print_ticket_admin_order_path(@order), + icon: 'icon-print', + target: "_blank" } + end + + def select_ticket_printer_link + { name: t(:select_ticket_printer), + url: "#{print_ticket_admin_order_path(@order)}#select-printer", + icon: 'icon-print', + target: "_blank" } + end + + def ship_order_link + { name: t(:ship_order), + url: fire_admin_order_path(@order, e: 'ship'), + method: 'put', + icon: 'icon-truck', + confirm: t(:are_you_sure) } + end + + def cancel_order_link + { name: t(:cancel_order), + url: fire_admin_order_path(@order.number, e: 'cancel'), + icon: 'icon-trash', + confirm: t(:are_you_sure) } + end end end end From ec2f99a46714fd50061e003542e932a402cb7847 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 19 Jun 2019 15:50:37 +0100 Subject: [PATCH 4/9] Refactor OrderHelper.order_links: extract some logic from order_links to make it more simple and pass rubocop tests --- .rubocop_manual_todo.yml | 5 --- .../spree/admin/orders_helper_decorator.rb | 42 ++++++++++++------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index b8ad5662cc..062a6ce8eb 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -68,7 +68,6 @@ Metrics/LineLength: - app/helpers/shop_helper.rb - app/helpers/spree/admin/base_helper_decorator.rb - app/helpers/spree/admin/navigation_helper_decorator.rb - - app/helpers/spree/admin/orders_helper_decorator.rb - app/helpers/spree/orders_helper.rb - app/jobs/subscription_confirm_job.rb - app/mailers/subscription_mailer.rb @@ -429,7 +428,6 @@ Metrics/AbcSize: - app/helpers/checkout_helper.rb - app/helpers/i18n_helper.rb - app/helpers/order_cycles_helper.rb - - app/helpers/spree/admin/orders_helper_decorator.rb - app/helpers/spree/orders_helper.rb - app/jobs/subscription_placement_job.rb - app/mailers/producer_mailer.rb @@ -570,7 +568,6 @@ Metrics/CyclomaticComplexity: - app/helpers/checkout_helper.rb - app/helpers/i18n_helper.rb - app/helpers/order_cycles_helper.rb - - app/helpers/spree/admin/orders_helper_decorator.rb - app/models/enterprise.rb - app/models/enterprise_relationship.rb - app/models/product_import/entry_processor.rb @@ -600,7 +597,6 @@ Metrics/PerceivedComplexity: - app/helpers/checkout_helper.rb - app/helpers/i18n_helper.rb - app/helpers/order_cycles_helper.rb - - app/helpers/spree/admin/orders_helper_decorator.rb - app/models/enterprise_relationship.rb - app/models/product_import/entry_processor.rb - app/models/product_import/entry_validator.rb @@ -646,7 +642,6 @@ Metrics/MethodLength: - app/controllers/user_registrations_controller.rb - app/helpers/checkout_helper.rb - app/helpers/order_cycles_helper.rb - - app/helpers/spree/admin/orders_helper_decorator.rb - app/jobs/subscription_placement_job.rb - app/mailers/producer_mailer.rb - app/models/column_preference.rb diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index b2f96b16da..6035445c4d 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -5,28 +5,38 @@ module Spree @order ||= order links = [] links << edit_order_link unless action_name == "edit" - if @order.complete? - links << resend_confirmation_link - if @order.distributor.can_invoice? - links << send_invoice_link_with_url - else - links << send_invoice_link_without_url - end - links << print_invoice_link - if Spree::Config.enable_receipt_printing? - links << print_ticket_link - links << select_ticket_printer_link - end - end - if @order.ready_to_ship? - links << ship_order_link - end + links << complete_order_links if @order.complete? + links << ship_order_link if @order.ready_to_ship? links << cancel_order_link if @order.can_cancel? links end private + def complete_order_links + complete_order_links = [] + complete_order_links << resend_confirmation_link + complete_order_links << invoice_links + complete_order_links << ticket_links + complete_order_links + end + + def invoice_links + invoice_links = [] + invoice_links << if @order.distributor.can_invoice? + send_invoice_link_with_url + else + send_invoice_link_without_url + end + invoice_links << print_invoice_link + invoice_links + end + + def ticket_links + return [] unless Spree::Config.enable_receipt_printing? + [print_ticket_link, select_ticket_printer_link] + end + def edit_order_link { name: t(:edit_order), url: edit_admin_order_path(@order), From e33ce235c43b60cd203e46f4297d881e6daa6260 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 19 Jun 2019 15:53:17 +0100 Subject: [PATCH 5/9] Hide invoice links in order edit page if invoices are disabled --- app/helpers/spree/admin/orders_helper_decorator.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index 6035445c4d..b3623c5721 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -22,6 +22,7 @@ module Spree end def invoice_links + return [] unless Spree::Config[:enable_invoices?] invoice_links = [] invoice_links << if @order.distributor.can_invoice? send_invoice_link_with_url From 61c8859da8f05cb92a9f17815ff7690d4bdd427b Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Wed, 26 Jun 2019 13:40:59 +0100 Subject: [PATCH 6/9] Fix problem in array concatenation in orders helper and cover it with specs --- .../spree/admin/orders_helper_decorator.rb | 8 +- .../helpers/spree/admin/orders_helper_spec.rb | 102 ++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 spec/helpers/spree/admin/orders_helper_spec.rb diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index b3623c5721..63a8b925a5 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -5,7 +5,7 @@ module Spree @order ||= order links = [] links << edit_order_link unless action_name == "edit" - links << complete_order_links if @order.complete? + links.concat(complete_order_links) if @order.complete? links << ship_order_link if @order.ready_to_ship? links << cancel_order_link if @order.can_cancel? links @@ -16,8 +16,8 @@ module Spree def complete_order_links complete_order_links = [] complete_order_links << resend_confirmation_link - complete_order_links << invoice_links - complete_order_links << ticket_links + complete_order_links.concat(invoice_links) + complete_order_links.concat(ticket_links) complete_order_links end @@ -34,7 +34,7 @@ module Spree end def ticket_links - return [] unless Spree::Config.enable_receipt_printing? + return [] unless Spree::Config[:enable_receipt_printing?] [print_ticket_link, select_ticket_printer_link] end diff --git a/spec/helpers/spree/admin/orders_helper_spec.rb b/spec/helpers/spree/admin/orders_helper_spec.rb new file mode 100644 index 0000000000..deaf4c6daa --- /dev/null +++ b/spec/helpers/spree/admin/orders_helper_spec.rb @@ -0,0 +1,102 @@ +require 'spec_helper' + +describe Spree::Admin::OrdersHelper, type: :helper do + describe "#orders_links" do + let(:order) { double(:order) } + let(:distributor) { double(:enterprise) } + + around do |example| + original_invoices_setting = Spree::Config[:enable_invoices?] + original_tickets_setting = Spree::Config[:enable_receipt_printing?] + example.run + Spree::Config[:enable_invoices?] = original_invoices_setting + Spree::Config[:enable_receipt_printing?] = original_tickets_setting + end + + before do + allow(order).to receive(:complete?) { false } + allow(order).to receive(:ready_to_ship?) { false } + allow(order).to receive(:can_cancel?) { false } + Spree::Config[:enable_invoices?] = false + Spree::Config[:enable_receipt_printing?] = false + end + + it "returns only edit order link when all conditions are set to false" do + links = helper.order_links(order) + + expect(links.size).to eq 1 + expect(links.first[:name]).to eq "Edit Order" + end + + context "complete order" do + before do + allow(order).to receive(:complete?) { true } + end + + it "returns edit order and resend confirmation links" do + links = helper.order_links(order) + + expect(links.size).to eq 2 + expect(links[0][:name]).to eq "Edit Order" + expect(links[1][:name]).to eq "Resend Confirmation" + end + + context "that can be canceled" do + before do + allow(order).to receive(:can_cancel?) { true } + allow(order).to receive(:number) { 111 } + end + + it "adds cancel order link" do + links = helper.order_links(order) + + expect(links.size).to eq 3 + expect(links[2][:name]).to eq "Cancel Order" + end + end + + context "that can be shipped" do + before do + allow(order).to receive(:ready_to_ship?) { true } + end + + it "adds ship order link" do + links = helper.order_links(order) + + expect(links.size).to eq 3 + expect(links[2][:name]).to eq "Ship Order" + end + end + + context "with invoices enabled" do + before do + Spree::Config[:enable_invoices?] = true + allow(order).to receive(:distributor) { distributor } + allow(distributor).to receive(:can_invoice?) { true } + end + + it "adds send and print invoice links" do + links = helper.order_links(order) + + expect(links.size).to eq 4 + expect(links[2][:name]).to eq "Send Invoice" + expect(links[3][:name]).to eq "Print Invoice" + end + end + + context "with tickets enabled" do + before do + Spree::Config[:enable_receipt_printing?] = true + end + + it "adds print and select ticket links" do + links = helper.order_links(order) + + expect(links.size).to eq 4 + expect(links[2][:name]).to eq "Print Ticket" + expect(links[3][:name]).to eq "Select printer for tickets" + end + end + end + end +end From 9be3ff90f7309070b510039c0fd069d3f0e43358 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 5 Jul 2019 11:06:49 +0100 Subject: [PATCH 7/9] Make OrdersHelper.complete_order_links simpler --- app/helpers/spree/admin/orders_helper_decorator.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index 63a8b925a5..947a847fb3 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -14,11 +14,7 @@ module Spree private def complete_order_links - complete_order_links = [] - complete_order_links << resend_confirmation_link - complete_order_links.concat(invoice_links) - complete_order_links.concat(ticket_links) - complete_order_links + [resend_confirmation_link] + invoice_links + ticket_links end def invoice_links From 88312b8e4a619f5376777c1d0b1294cc028c6e2c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Fri, 5 Jul 2019 11:08:51 +0100 Subject: [PATCH 8/9] Make OrdersHelper.invoice_links simpler --- .../spree/admin/orders_helper_decorator.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/app/helpers/spree/admin/orders_helper_decorator.rb b/app/helpers/spree/admin/orders_helper_decorator.rb index 947a847fb3..074020e1d4 100644 --- a/app/helpers/spree/admin/orders_helper_decorator.rb +++ b/app/helpers/spree/admin/orders_helper_decorator.rb @@ -19,14 +19,15 @@ module Spree def invoice_links return [] unless Spree::Config[:enable_invoices?] - invoice_links = [] - invoice_links << if @order.distributor.can_invoice? - send_invoice_link_with_url - else - send_invoice_link_without_url - end - invoice_links << print_invoice_link - invoice_links + [send_invoice_link, print_invoice_link] + end + + def send_invoice_link + if @order.distributor.can_invoice? + send_invoice_link_with_url + else + send_invoice_link_without_url + end end def ticket_links From 95832c96acc84942036ba8cbbb5e1237af75fdea Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Tue, 9 Jul 2019 09:15:02 +0100 Subject: [PATCH 9/9] Delete redundant spec, this case is now covered in a view spec --- spec/features/admin/orders_spec.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/spec/features/admin/orders_spec.rb b/spec/features/admin/orders_spec.rb index ce12f02870..3b54d31cdb 100644 --- a/spec/features/admin/orders_spec.rb +++ b/spec/features/admin/orders_spec.rb @@ -145,17 +145,6 @@ feature ' expect(page).to have_selector 'p', text: "Order cycle: #{@order.order_cycle.name}" end - scenario "can't print invoices when invocies are disabled" do - original_invoices_setting = Spree::Config[:enable_invoices?] - - Spree::Config[:enable_invoices?] = false - quick_login_as_admin - visit '/admin/orders' - expect(page).to have_no_selector 'button.invoices-modal' - - Spree::Config[:enable_invoices?] = original_invoices_setting - end - scenario "filling customer details" do # Given a customer with an order, which includes their shipping and billing address