diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 25df407c86..f0bc6f67b5 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -576,16 +576,6 @@ Rails/HasManyOrHasOneDependent: - 'app/models/spree/tax_rate.rb' - 'app/models/spree/variant.rb' -# Offense count: 26 -# Configuration parameters: Include. -# Include: app/helpers/**/*.rb -Rails/HelperInstanceVariable: - Exclude: - - 'app/helpers/injection_helper.rb' - - 'app/helpers/shared_helper.rb' - - 'app/helpers/spree/admin/orders_helper.rb' - - 'app/helpers/spree/orders_helper.rb' - # Offense count: 8 # Configuration parameters: Include. # Include: spec/**/*.rb, test/**/*.rb diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index cb191f1367..63a0797992 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -154,7 +154,6 @@ module InjectionHelper end def enterprise_injection_data - @enterprise_injection_data ||= OpenFoodNetwork::EnterpriseInjectionData.new - { data: @enterprise_injection_data } + @enterprise_injection_data ||= { data: OpenFoodNetwork::EnterpriseInjectionData.new } end end diff --git a/app/helpers/shared_helper.rb b/app/helpers/shared_helper.rb index 17bd12adbe..63e5498409 100644 --- a/app/helpers/shared_helper.rb +++ b/app/helpers/shared_helper.rb @@ -1,16 +1,6 @@ # frozen_string_literal: true module SharedHelper - def distributor_link_class(distributor) - cart = current_order(true) - @active_distributors ||= Enterprise.distributors_with_active_order_cycles - - klass = "shop-distributor" - klass += " empties-cart" unless cart.line_items.empty? || cart.distributor == distributor - klass += @active_distributors.include?(distributor) ? ' active' : ' inactive' - klass - end - def enterprise_user? spree_current_user&.enterprises&.count.to_i > 0 end diff --git a/app/helpers/spree/admin/orders_helper.rb b/app/helpers/spree/admin/orders_helper.rb index 893a417297..71970c53d3 100644 --- a/app/helpers/spree/admin/orders_helper.rb +++ b/app/helpers/spree/admin/orders_helper.rb @@ -3,20 +3,20 @@ module Spree module Admin module OrdersHelper - def event_links + def event_links(order) links = [] - links << cancel_event_link if @order.can_cancel? - links << resume_event_link if @order.can_resume? + links << cancel_event_link(order) if order.can_cancel? + links << resume_event_link(order) if order.can_resume? links.join(' ').html_safe # rubocop:disable Rails/OutputSafety end def generate_invoice_button(order) if order.distributor.can_invoice? - button_link_to t(:create_or_update_invoice), generate_admin_order_invoices_path(@order), + button_link_to t(:create_or_update_invoice), generate_admin_order_invoices_path(order), data: { method: 'post' }, icon: 'icon-plus' else button_link_to t(:create_or_update_invoice), "#", data: { - confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) + confirm: t(:must_have_valid_business_number, enterprise_name: order.distributor.name) }, icon: 'icon-plus' end end @@ -26,82 +26,81 @@ module Spree end def order_links(order) - @order ||= order links = [] - links << edit_order_link unless action_name == "edit" - links.concat(complete_order_links) if @order.complete? || @order.resumed? - links << ship_order_link if @order.ready_to_ship? - links << cancel_order_link if @order.can_cancel? + links << edit_order_link(order) unless action_name == "edit" + links.concat(complete_order_links(order)) if order.complete? || order.resumed? + links << ship_order_link if order.ready_to_ship? + links << cancel_order_link(order) if order.can_cancel? links end private - def complete_order_links - [resend_confirmation_link] + invoice_links + def complete_order_links(order) + [resend_confirmation_link(order)] + invoice_links(order) end - def invoice_links + def invoice_links(order) return [] unless Spree::Config[:enable_invoices?] - [send_invoice_link, print_invoice_link] + [send_invoice_link(order), print_invoice_link(order)] end - def send_invoice_link - if @order.distributor.can_invoice? - send_invoice_link_with_url + def send_invoice_link(order) + if order.distributor.can_invoice? + send_invoice_link_with_url(order) else - send_invoice_link_without_url + send_invoice_link_without_url(order) end end - def print_invoice_link - if @order.distributor.can_invoice? - print_invoice_link_with_url + def print_invoice_link(order) + if order.distributor.can_invoice? + print_invoice_link_with_url(order) else - notify_about_required_enterprise_number + notify_about_required_enterprise_number(order) end end - def edit_order_link + def edit_order_link(order) { name: t(:edit_order), - url: spree.edit_admin_order_path(@order), + url: spree.edit_admin_order_path(order), icon: 'icon-edit' } end - def resend_confirmation_link + def resend_confirmation_link(order) { name: t(:resend_confirmation), - url: spree.resend_admin_order_path(@order), + url: spree.resend_admin_order_path(order), icon: 'icon-email', confirm: t(:confirm_resend_order_confirmation) } end - def send_invoice_link_with_url + def send_invoice_link_with_url(order) { name: t(:send_invoice), - url: invoice_admin_order_path(@order), + url: invoice_admin_order_path(order), icon: 'icon-email', confirm: t(:confirm_send_invoice) } end - def send_invoice_link_without_url + def send_invoice_link_without_url(order) { name: t(:send_invoice), url: "#", icon: 'icon-email', - confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) } + confirm: t(:must_have_valid_business_number, enterprise_name: order.distributor.name) } end - def print_invoice_link_with_url + def print_invoice_link_with_url(order) { name: t(:print_invoice), - url: spree.print_admin_order_path(@order), + url: spree.print_admin_order_path(order), icon: 'icon-print', target: "_blank" } end - def notify_about_required_enterprise_number + def notify_about_required_enterprise_number(order) { name: t(:print_invoice), url: "#", icon: 'icon-print', - confirm: t(:must_have_valid_business_number, enterprise_name: @order.distributor.name) } + confirm: t(:must_have_valid_business_number, enterprise_name: order.distributor.name) } end def ship_order_link @@ -110,24 +109,24 @@ module Spree icon: 'icon-truck' } end - def cancel_order_link + def cancel_order_link(order) { name: t(:cancel_order), - url: spree.fire_admin_order_path(@order.number, e: 'cancel'), + url: spree.fire_admin_order_path(order.number, e: 'cancel'), icon: 'icon-trash' } end - def cancel_event_link + def cancel_event_link(order) event_label = I18n.t("cancel", scope: "actions") button_link_to(event_label, - fire_admin_order_url(@order, e: "cancel"), + fire_admin_order_url(order, e: "cancel"), method: :put, icon: "icon-cancel", form_id: "cancel_order_form") end - def resume_event_link + def resume_event_link(order) event_label = I18n.t("resume", scope: "actions") confirm_message = I18n.t("admin.orders.edit.order_sure_want_to", event: event_label) button_link_to(event_label, - fire_admin_order_url(@order, e: "resume"), + fire_admin_order_url(order, e: "resume"), method: :put, icon: "icon-resume", data: { confirm: confirm_message }) end diff --git a/app/helpers/spree/orders_helper.rb b/app/helpers/spree/orders_helper.rb index 63a4682bc2..d3d1a155c1 100644 --- a/app/helpers/spree/orders_helper.rb +++ b/app/helpers/spree/orders_helper.rb @@ -17,17 +17,18 @@ module Spree def changeable_orders # Only returns open order for the current user + shop + oc combo - return @changeable_orders unless @changeable_orders.nil? - return @changeable_orders = [] unless spree_current_user && - current_distributor && current_order_cycle - return @changeable_orders = [] unless current_distributor.allow_order_changes? + @changeable_orders ||= if spree_current_user && + current_order_cycle && current_distributor&.allow_order_changes? - @changeable_orders = Spree::Order.complete.where( - state: 'complete', - user_id: spree_current_user.id, - distributor_id: current_distributor.id, - order_cycle_id: current_order_cycle.id - ) + Spree::Order.complete.where( + state: 'complete', + user_id: spree_current_user.id, + distributor_id: current_distributor.id, + order_cycle_id: current_order_cycle.id + ) + else + [] + end end def changeable_orders_link_path diff --git a/app/views/shared/_distributor.html.haml b/app/views/shared/_distributor.html.haml deleted file mode 100644 index ea1eeae0a4..0000000000 --- a/app/views/shared/_distributor.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -= succeed ',' do - = link_to "#{distributor.name}".html_safe, enterprise_shop_path(distributor), {class: distributor_link_class(distributor)} -%span.secondary= distributor.city diff --git a/app/views/spree/admin/orders/edit.html.haml b/app/views/spree/admin/orders/edit.html.haml index c74f2e6bcb..eb10841bbc 100644 --- a/app/views/spree/admin/orders/edit.html.haml +++ b/app/views/spree/admin/orders/edit.html.haml @@ -5,7 +5,7 @@ - content_for :page_actions do - if can?(:fire, @order) - %li= event_links + %li= event_links(@order) = render partial: 'spree/admin/shared/order_links' - if can?(:admin, Spree::Order) %li diff --git a/spec/helpers/shared_helper_spec.rb b/spec/helpers/shared_helper_spec.rb deleted file mode 100644 index 7d61898f93..0000000000 --- a/spec/helpers/shared_helper_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe SharedHelper, type: :helper do - it "does not require emptying the cart when it is empty" do - d = double(:distributor) - order = double(:order, line_items: []) - allow(helper).to receive(:current_order) { order } - expect(helper.distributor_link_class(d)).not_to match(/empties-cart/) - end - - it "does not require emptying the cart when we are on the same distributor" do - d = double(:distributor) - order = double(:order, line_items: [double(:line_item)], distributor: d) - allow(helper).to receive(:current_order) { order } - expect(helper.distributor_link_class(d)).not_to match(/empties-cart/) - end - - it "requires emptying the cart otherwise" do - d1 = double(:distributor) - d2 = double(:distributor) - order = double(:order, line_items: [double(:line_item)], distributor: d2) - allow(helper).to receive(:current_order) { order } - expect(helper.distributor_link_class(d1)).to match(/empties-cart/) - end -end diff --git a/spec/helpers/spree/orders_helper_spec.rb b/spec/helpers/spree/orders_helper_spec.rb index 1b47d33a0e..a44bd9afab 100644 --- a/spec/helpers/spree/orders_helper_spec.rb +++ b/spec/helpers/spree/orders_helper_spec.rb @@ -31,6 +31,11 @@ describe Spree::OrdersHelper, type: :helper do before { allow(current_distributor).to receive(:allow_order_changes?) { false } } it { expect(helper.changeable_orders).to eq [] } end + + context "when a current_distributor is not defined" do + let(:current_distributor) { nil } + it { expect(helper.changeable_orders).to eq [] } + end end context "when a current_order_cycle is not defined" do @@ -38,11 +43,6 @@ describe Spree::OrdersHelper, type: :helper do it { expect(helper.changeable_orders).to eq [] } end end - - context "when a current_distributor is not defined" do - let(:current_distributor) { nil } - it { expect(helper.changeable_orders).to eq [] } - end end context "when spree_current_user is not defined" do