diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 72e4621f40..c8e4d81993 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -732,7 +732,7 @@ Rails/FilePath: - 'spec/models/content_configuration_spec.rb' - 'spec/support/downloads_helper.rb' -# Offense count: 11 +# Offense count: 12 # Configuration parameters: Include. # Include: app/models/**/*.rb Rails/HasAndBelongsToMany: @@ -740,6 +740,7 @@ Rails/HasAndBelongsToMany: - 'app/models/concerns/payment_method_distributors.rb' - 'app/models/enterprise.rb' - 'app/models/enterprise_group.rb' + - 'app/models/order_cycle.rb' - 'app/models/spree/line_item.rb' - 'app/models/spree/option_value.rb' - 'app/models/spree/role.rb' diff --git a/app/controllers/admin/order_cycles_controller.rb b/app/controllers/admin/order_cycles_controller.rb index 866fd94334..5812d2395c 100644 --- a/app/controllers/admin/order_cycles_controller.rb +++ b/app/controllers/admin/order_cycles_controller.rb @@ -2,10 +2,10 @@ module Admin class OrderCyclesController < Admin::ResourceController - include OrderCyclesHelper + include ::OrderCyclesHelper include PaperTrailLogging - prepend_before_action :set_order_cycle_id, only: [:incoming, :outgoing] + prepend_before_action :set_order_cycle_id, only: [:incoming, :outgoing, :checkout_options] before_action :load_data_for_index, only: :index before_action :require_coordinator, only: :new before_action :remove_protected_attrs, only: [:update] @@ -67,10 +67,12 @@ module Admin update_nil_subscription_line_items_price_estimate(@order_cycle) respond_to do |format| flash[:notice] = I18n.t(:order_cycles_update_notice) if params[:reloading] == '1' - format.html { redirect_back(fallback_location: root_path) } + format.html { redirect_to_after_update_path } format.json { render json: { success: true } } end - else + elsif request.format.html? + render :checkout_options + elsif request.format.json? render json: { errors: @order_cycle.errors.full_messages }, status: :unprocessable_entity end end @@ -190,6 +192,16 @@ module Admin end end + def redirect_to_after_update_path + if params[:context] == "checkout_options" && params[:save] + redirect_to main_app.admin_order_cycle_checkout_options_path(@order_cycle) + elsif params[:context] == "checkout_options" && params[:save_and_back_to_list] + redirect_to main_app.admin_order_cycles_path + else + redirect_back(fallback_location: root_path) + end + end + def require_coordinator @order_cycle.coordinator = permitted_coordinating_enterprises_for(@order_cycle).find_by(id: params[:coordinator_id]) diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 7816066b8b..952c66afee 100644 --- a/app/controllers/base_controller.rb +++ b/app/controllers/base_controller.rb @@ -16,12 +16,7 @@ class BaseController < ApplicationController private def set_order_cycles - unless @distributor.ready_for_checkout? - @order_cycles = OrderCycle.where('false') - return - end - - @order_cycles = Shop::OrderCyclesList.new(@distributor, current_customer).call + @order_cycles = Shop::OrderCyclesList.ready_for_checkout_for(@distributor, current_customer) set_order_cycle end diff --git a/app/helpers/enterprises_helper.rb b/app/helpers/enterprises_helper.rb index a93eb56b24..701f736180 100644 --- a/app/helpers/enterprises_helper.rb +++ b/app/helpers/enterprises_helper.rb @@ -14,15 +14,7 @@ module EnterprisesHelper end def available_shipping_methods - return [] if current_distributor.blank? - - shipping_methods = current_distributor.shipping_methods.display_on_checkout.to_a - - applicator = OpenFoodNetwork::TagRuleApplicator.new(current_distributor, - "FilterShippingMethods", current_customer&.tag_list) - applicator.filter!(shipping_methods) - - shipping_methods.uniq + OrderAvailableShippingMethods.new(current_order, current_customer).to_a end def available_payment_methods diff --git a/app/helpers/order_cycles_helper.rb b/app/helpers/order_cycles_helper.rb index 741613e246..c97e0c1a96 100644 --- a/app/helpers/order_cycles_helper.rb +++ b/app/helpers/order_cycles_helper.rb @@ -56,10 +56,6 @@ module OrderCyclesHelper @simple_index ||= !OpenFoodNetwork::Permissions.new(spree_current_user).can_manage_complex_order_cycles? end - def order_cycles_simple_form - @order_cycles_simple_form ||= @order_cycle.coordinator.sells == 'own' - end - def pickup_time(order_cycle = current_order_cycle) order_cycle.exchanges.to_enterprises(current_distributor).outgoing.first.pickup_time end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index e08d508ad2..8d62a1cc94 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -120,6 +120,7 @@ class Enterprise < ApplicationRecord joins(:shipping_methods). joins(:payment_methods). merge(Spree::PaymentMethod.available). + merge(Spree::ShippingMethod.frontend). select('DISTINCT enterprises.*') } scope :not_ready_for_checkout, lambda { @@ -387,7 +388,7 @@ class Enterprise < ApplicationRecord end def ready_for_checkout? - shipping_methods.any? && payment_methods.available.any? + shipping_methods.frontend.any? && payment_methods.available.any? end def self.find_available_permalink(test_permalink) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 26fe34eec7..e31fe4bf82 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -22,9 +22,11 @@ class OrderCycle < ApplicationRecord has_many :suppliers, -> { distinct }, source: :sender, through: :cached_incoming_exchanges has_many :distributors, -> { distinct }, source: :receiver, through: :cached_outgoing_exchanges - has_many :order_cycle_schedules has_many :schedules, through: :order_cycle_schedules + has_and_belongs_to_many :selected_distributor_shipping_methods, + class_name: 'DistributorShippingMethod', + join_table: 'order_cycles_distributor_shipping_methods' has_paper_trail meta: { custom_data: proc { |order_cycle| order_cycle.schedule_ids.to_s } } attr_accessor :incoming_exchanges, :outgoing_exchanges @@ -150,6 +152,20 @@ class OrderCycle < ApplicationRecord ] end + def attachable_payment_methods + Spree::PaymentMethod.available(:both). + joins("INNER JOIN distributors_payment_methods + ON payment_method_id = spree_payment_methods.id"). + where("distributor_id IN (?)", distributor_ids). + distinct + end + + def attachable_distributor_shipping_methods + DistributorShippingMethod.joins(:shipping_method). + merge(Spree::ShippingMethod.frontend). + where("distributor_id IN (?)", distributor_ids) + end + def clone! oc = dup oc.name = I18n.t("models.order_cycle.cloned_order_cycle_name", order_cycle: oc.name) @@ -161,6 +177,9 @@ class OrderCycle < ApplicationRecord oc.schedule_ids = schedule_ids oc.save! exchanges.each { |e| e.clone!(oc) } + oc.selected_distributor_shipping_method_ids = ( + attachable_distributor_shipping_methods.map(&:id) & selected_distributor_shipping_method_ids + ) sync_subscriptions oc.reload end @@ -274,6 +293,22 @@ class OrderCycle < ApplicationRecord items.each { |li| scoper.scope(li.variant) } end + def distributor_shipping_methods + if simple? || selected_distributor_shipping_methods.none? + attachable_distributor_shipping_methods + else + attachable_distributor_shipping_methods.where( + "distributors_shipping_methods.id IN (?) OR distributor_id NOT IN (?)", + selected_distributor_shipping_methods.map(&:id), + selected_distributor_shipping_methods.map(&:distributor_id) + ) + end + end + + def simple? + coordinator.sells == 'own' + end + private def opening? diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 2e9aec849f..63857559cd 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -243,7 +243,9 @@ module Spree end def add_order_cycle_management_abilities(user) - can [:admin, :index, :read, :edit, :update, :incoming, :outgoing], OrderCycle do |order_cycle| + can [ + :admin, :index, :read, :edit, :update, :incoming, :outgoing, :checkout_options + ], OrderCycle do |order_cycle| OrderCycle.visible_by(user).include? order_cycle end can [:admin, :index, :create], Schedule diff --git a/app/models/spree/shipping_method.rb b/app/models/spree/shipping_method.rb index 53d77d60c2..589bc8ce8c 100644 --- a/app/models/spree/shipping_method.rb +++ b/app/models/spree/shipping_method.rb @@ -3,7 +3,10 @@ module Spree class ShippingMethod < ApplicationRecord include CalculatedAdjustments - DISPLAY = [:both, :front_end, :back_end].freeze + DISPLAY_ON_OPTIONS = { + both: "", + back_end: "back_end" + }.freeze acts_as_paranoid acts_as_taggable @@ -27,6 +30,7 @@ module Spree validates :name, presence: true validate :distributor_validation validate :at_least_one_shipping_category + validates :display_on, inclusion: { in: DISPLAY_ON_OPTIONS.values }, allow_nil: true after_save :touch_distributors @@ -51,9 +55,6 @@ module Spree } scope :by_name, -> { order('spree_shipping_methods.name ASC') } - scope :display_on_checkout, -> { - where("spree_shipping_methods.display_on is null OR spree_shipping_methods.display_on = ''") - } # Here we allow checkout with shipping methods without zones (see issue #3928 for details) # and also checkout with addresses outside of the zones of the selected shipping method @@ -101,16 +102,26 @@ module Spree ] end - def self.on_backend_query - "#{table_name}.display_on != 'front_end' OR #{table_name}.display_on IS NULL" + def self.backend + where(display_on: DISPLAY_ON_OPTIONS[:back_end]) end - def self.on_frontend_query - "#{table_name}.display_on != 'back_end' OR #{table_name}.display_on IS NULL" + def self.frontend + where(display_on: [nil, ""]) end private + def no_active_or_upcoming_order_cycle_distributors_with_only_one_shipping_method? + return true if new_record? + + distributors. + with_order_cycles_as_distributor_outer. + merge(OrderCycle.active.or(OrderCycle.upcoming)).none? do |distributor| + distributor.shipping_method_ids.one? + end + end + def at_least_one_shipping_category return unless shipping_categories.empty? diff --git a/app/models/spree/shipping_rate.rb b/app/models/spree/shipping_rate.rb index 710a8a25b4..5116a2de6b 100644 --- a/app/models/spree/shipping_rate.rb +++ b/app/models/spree/shipping_rate.rb @@ -8,14 +8,7 @@ module Spree scope :frontend, -> { includes(:shipping_method). - where(ShippingMethod.on_frontend_query). - references(:shipping_method). - order("cost ASC") - } - scope :backend, - -> { - includes(:shipping_method). - where(ShippingMethod.on_backend_query). + merge(ShippingMethod.frontend). references(:shipping_method). order("cost ASC") } diff --git a/app/services/order_available_shipping_methods.rb b/app/services/order_available_shipping_methods.rb new file mode 100644 index 0000000000..96d1b99559 --- /dev/null +++ b/app/services/order_available_shipping_methods.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class OrderAvailableShippingMethods + attr_reader :order, :customer + + delegate :distributor, + :order_cycle, + to: :order + + def initialize(order, customer = nil) + @order, @customer = order, customer + end + + def to_a + return [] if distributor.blank? + + shipping_methods = shipping_methods_before_tag_rules_applied + + applicator = OpenFoodNetwork::TagRuleApplicator.new(distributor, + "FilterShippingMethods", customer&.tag_list) + applicator.filter!(shipping_methods) + + shipping_methods.uniq + end + + private + + def shipping_methods_before_tag_rules_applied + if order_cycle.nil? || order_cycle.simple? + distributor.shipping_methods + else + distributor.shipping_methods.where( + id: order_cycle.distributor_shipping_methods.select(:shipping_method_id) + ) + end.frontend.to_a + end +end diff --git a/app/services/order_cart_reset.rb b/app/services/order_cart_reset.rb index 4faec18a30..9401d60c4c 100644 --- a/app/services/order_cart_reset.rb +++ b/app/services/order_cart_reset.rb @@ -34,7 +34,7 @@ class OrderCartReset end def reset_order_cycle(current_customer) - listed_order_cycles = Shop::OrderCyclesList.new(distributor, current_customer).call + listed_order_cycles = Shop::OrderCyclesList.active_for(distributor, current_customer) if order_cycle_not_listed?(order.order_cycle, listed_order_cycles) order.order_cycle = nil diff --git a/app/services/order_cycle_form.rb b/app/services/order_cycle_form.rb index 6e96b9af68..252c3bc783 100644 --- a/app/services/order_cycle_form.rb +++ b/app/services/order_cycle_form.rb @@ -8,9 +8,13 @@ class OrderCycleForm def initialize(order_cycle, order_cycle_params, user) @order_cycle = order_cycle @order_cycle_params = order_cycle_params + @specified_params = order_cycle_params.keys @user = user @permissions = OpenFoodNetwork::Permissions.new(user) @schedule_ids = order_cycle_params.delete(:schedule_ids) + @selected_distributor_shipping_method_ids = order_cycle_params.delete( + :selected_distributor_shipping_method_ids + ) end def save @@ -20,13 +24,15 @@ class OrderCycleForm order_cycle.transaction do order_cycle.save! - order_cycle.schedule_ids = schedule_ids + order_cycle.schedule_ids = schedule_ids if parameter_specified?(:schedule_ids) order_cycle.save! apply_exchange_changes + attach_selected_distributor_shipping_methods sync_subscriptions true end - rescue ActiveRecord::RecordInvalid + rescue ActiveRecord::RecordInvalid => e + add_exception_to_order_cycle_errors(e) false end @@ -34,24 +40,50 @@ class OrderCycleForm attr_accessor :order_cycle, :order_cycle_params, :user, :permissions + def add_exception_to_order_cycle_errors(exception) + error = exception.message.split(":").last.strip + order_cycle.errors.add(:base, error) if order_cycle.errors.to_a.exclude?(error) + end + def apply_exchange_changes return if exchanges_unchanged? OpenFoodNetwork::OrderCycleFormApplicator.new(order_cycle, user).go! end + def attach_selected_distributor_shipping_methods + return if @selected_distributor_shipping_method_ids.nil? + + order_cycle.reload # so outgoing exchanges are up-to-date for shipping method validations + order_cycle.selected_distributor_shipping_method_ids = selected_distributor_shipping_method_ids + order_cycle.save! + end + + def attachable_distributor_shipping_method_ids + @attachable_distributor_shipping_method_ids ||= order_cycle.attachable_distributor_shipping_methods.map(&:id) + end + def exchanges_unchanged? [:incoming_exchanges, :outgoing_exchanges].all? do |direction| order_cycle_params[direction].nil? end end - def schedule_ids? - @schedule_ids.present? + def selected_distributor_shipping_method_ids + @selected_distributor_shipping_method_ids = ( + attachable_distributor_shipping_method_ids & + @selected_distributor_shipping_method_ids.reject(&:blank?).map(&:to_i) + ) + + if attachable_distributor_shipping_method_ids.sort == @selected_distributor_shipping_method_ids.sort + @selected_distributor_shipping_method_ids = [] + end + + @selected_distributor_shipping_method_ids end def build_schedule_ids - return unless schedule_ids? + return unless parameter_specified?(:schedule_ids) result = existing_schedule_ids result |= (requested_schedule_ids & permitted_schedule_ids) # Add permitted and requested @@ -60,7 +92,7 @@ class OrderCycleForm end def sync_subscriptions - return unless schedule_ids? + return unless parameter_specified?(:schedule_ids) return unless schedule_sync_required? OrderManagement::Subscriptions::ProxyOrderSyncer.new(subscriptions_to_sync).sync! @@ -78,6 +110,10 @@ class OrderCycleForm @schedule_ids.map(&:to_i) end + def parameter_specified?(key) + @specified_params.map(&:to_s).include?(key.to_s) + end + def permitted_schedule_ids Schedule.where(id: requested_schedule_ids | existing_schedule_ids) .merge(permissions.editable_schedules).pluck(:id) diff --git a/app/services/permitted_attributes/order_cycle.rb b/app/services/permitted_attributes/order_cycle.rb index dd74607168..15d652268c 100644 --- a/app/services/permitted_attributes/order_cycle.rb +++ b/app/services/permitted_attributes/order_cycle.rb @@ -17,7 +17,7 @@ module PermittedAttributes :name, :orders_open_at, :orders_close_at, :coordinator_id, :preferred_product_selection_from_coordinator_inventory_only, :automatic_notifications, - { schedule_ids: [], coordinator_fee_ids: [] } + { schedule_ids: [], selected_distributor_shipping_method_ids: [], coordinator_fee_ids: [] } ] end diff --git a/app/services/shop/order_cycles_list.rb b/app/services/shop/order_cycles_list.rb index 1afed9acd8..af4b9cc63f 100644 --- a/app/services/shop/order_cycles_list.rb +++ b/app/services/shop/order_cycles_list.rb @@ -3,6 +3,16 @@ # Lists available order cycles for a given customer in a given distributor module Shop class OrderCyclesList + def self.active_for(distributor, customer) + new(distributor, customer).call + end + + def self.ready_for_checkout_for(distributor, customer) + return OrderCycle.none unless distributor.ready_for_checkout? + + new(distributor, customer).call + end + def initialize(distributor, customer) @distributor = distributor @customer = customer diff --git a/app/views/admin/order_cycles/_wizard_progress.html.haml b/app/views/admin/order_cycles/_wizard_progress.html.haml index 3224324ca6..cc13adcebf 100644 --- a/app/views/admin/order_cycles/_wizard_progress.html.haml +++ b/app/views/admin/order_cycles/_wizard_progress.html.haml @@ -6,6 +6,8 @@ = t("admin.order_cycles.wizard_progress.incoming") %li = t("admin.order_cycles.wizard_progress.outgoing") + %li + = t("admin.order_cycles.wizard_progress.checkout_options") - else %li{ class: "#{'current' if action_name == 'edit'}" } %a{ href: main_app.edit_admin_order_cycle_path(@order_cycle) } @@ -16,3 +18,6 @@ %li{ class: "#{'current' if action_name == 'outgoing'}" } %a{ href: main_app.admin_order_cycle_outgoing_path(@order_cycle) } = t("admin.order_cycles.wizard_progress.outgoing") + %li{ class: "#{'current' if action_name == 'checkout_options'}" } + %a{ href: main_app.admin_order_cycle_checkout_options_path(@order_cycle) } + = t("admin.order_cycles.wizard_progress.checkout_options") diff --git a/app/views/admin/order_cycles/checkout_options.html.haml b/app/views/admin/order_cycles/checkout_options.html.haml new file mode 100644 index 0000000000..32eb731895 --- /dev/null +++ b/app/views/admin/order_cycles/checkout_options.html.haml @@ -0,0 +1,74 @@ += render partial: "/admin/order_cycles/order_cycle_top_buttons" + +- content_for :page_title do + = t :edit_order_cycle + += form_for [main_app, :admin, @order_cycle], html: { class: "order_cycle" } do |f| + + = render 'wizard_progress' + + %fieldset.no-border-bottom + %legend{ align: 'center'}= t('.checkout_options') + + = hidden_field_tag "order_cycle[selected_distributor_shipping_method_ids][]", "" + + .row + .three.columns +   + .ten.columns + %table.checkout-options + %thead + %tr + %th{ colspan: 2 }= t('.shipping_methods') + - @order_cycle.distributors.each do |distributor| + - distributor_shipping_methods = @order_cycle.attachable_distributor_shipping_methods.where("distributor_id = ?", distributor.id).includes(:shipping_method) + %tr{ "data-controller": "select-all" } + %td.text-center + - if distributor_shipping_methods.many? + %label + = check_box_tag nil, nil, nil, { "data-action": "change->select-all#toggleAll", "data-select-all-target": "all" } + = t(".select_all") + %td + %em= distributor.name + - distributor_shipping_methods.each do |distributor_shipping_method| + %p + %label{ class: ("disabled" if distributor_shipping_methods.one? || !distributor_shipping_method.shipping_method.frontend?) } + = check_box_tag "order_cycle[selected_distributor_shipping_method_ids][]", + distributor_shipping_method.id, + @order_cycle.distributor_shipping_methods.include?(distributor_shipping_method), + id: "order_cycle_selected_distributor_shipping_method_ids_#{distributor_shipping_method.id}", + data: ({ "action" => "change->select-all#toggleCheckbox", "select-all-target" => "checkbox" } if distributor_shipping_method.shipping_method.frontend?) + = distributor_shipping_method.shipping_method.name + - distributor.shipping_methods.backend.each do |shipping_method| + %label.disabled + = check_box_tag nil, nil, false, disabled: true + = shipping_method.name + = "(#{t('.back_end')})" + - if distributor.shipping_methods.frontend.none? + %p + = t('.no_shipping_methods') + %tr + %th{ colspan: 2 }= t('.payment_methods') + %tr + %td + %td + - if @order_cycle.attachable_payment_methods.available(:both).any? + %ul + - @order_cycle.attachable_payment_methods.available(:both).each do |payment_method| + %li= payment_method.name + - else + %p + = t('.no_payment_methods') + + %div#save-bar + %div.container + %div.seven.columns.alpha + - if @order_cycle.errors.any? + %h5#status-message.error + = @order_cycle.errors.to_a.to_sentence + %div.nine.columns.omega.text-right + = hidden_field_tag :context, :checkout_options + = f.submit t('.save'), class: "red", name: :save + = f.submit t('.save_and_back_to_list'), class: "red", name: :save_and_back_to_list + %a.button.cancel{ href: main_app.admin_order_cycles_path } + = t('.cancel') diff --git a/app/views/admin/order_cycles/edit.html.haml b/app/views/admin/order_cycles/edit.html.haml index fdad0e3599..56151db299 100644 --- a/app/views/admin/order_cycles/edit.html.haml +++ b/app/views/admin/order_cycles/edit.html.haml @@ -13,20 +13,20 @@ - content_for :page_title do = t :edit_order_cycle -- ng_controller = order_cycles_simple_form ? 'AdminSimpleEditOrderCycleCtrl' : 'AdminEditOrderCycleCtrl' +- ng_controller = @order_cycle.simple? ? 'AdminSimpleEditOrderCycleCtrl' : 'AdminEditOrderCycleCtrl' = admin_inject_order_cycle_instance = form_for [main_app, :admin, @order_cycle], :url => '', :html => {:class => 'ng order_cycle', 'ng-app' => 'admin.orderCycles', 'ng-controller' => ng_controller, name: 'order_cycle_form'} do |f| %save-bar{ dirty: "order_cycle_form.$dirty", persist: "true" } %input.red{ type: "button", value: t('.save'), ng: { click: "submit($event, null)", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } - - if order_cycles_simple_form + - if @order_cycle.simple? %input.red{ type: "button", value: t('.save_and_back_to_list'), ng: { click: "submit($event, '#{main_app.admin_order_cycles_path}')", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } - else %input.red{ type: "button", value: t('.save_and_next'), ng: { click: "submit($event, '#{main_app.admin_order_cycle_incoming_path(@order_cycle)}')", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } %input{ type: "button", value: t('.next'), ng: { click: "cancel('#{main_app.admin_order_cycle_incoming_path(@order_cycle)}')", disabled: "order_cycle_form.$dirty" } } %input{ type: "button", ng: { value: "order_cycle_form.$dirty ? '#{t('.cancel')}' : '#{t('.back_to_list')}'", click: "cancel('#{main_app.admin_order_cycles_path}')" } } - - if order_cycles_simple_form + - if @order_cycle.simple? = render 'simple_form', f: f - else = render 'form', f: f diff --git a/app/views/admin/order_cycles/new.html.haml b/app/views/admin/order_cycles/new.html.haml index e01896ab8c..e40ee808ef 100644 --- a/app/views/admin/order_cycles/new.html.haml +++ b/app/views/admin/order_cycles/new.html.haml @@ -1,18 +1,18 @@ - content_for :page_title do =t('new_order_cycle') -- ng_controller = order_cycles_simple_form ? 'AdminSimpleCreateOrderCycleCtrl' : 'AdminCreateOrderCycleCtrl' +- ng_controller = @order_cycle.simple? ? 'AdminSimpleCreateOrderCycleCtrl' : 'AdminCreateOrderCycleCtrl' = admin_inject_order_cycle_instance = form_for [main_app, :admin, @order_cycle], :url => '', :html => {:class => 'ng order_cycle', 'ng-app' => 'admin.orderCycles', 'ng-controller' => ng_controller, name: 'order_cycle_form'} do |f| %save-bar{ dirty: "order_cycle_form.$dirty", persist: "true" } - - if order_cycles_simple_form + - if @order_cycle.simple? - custom_redirect_path = main_app.admin_order_cycles_path %input.red{ type: "button", value: t('.create'), ng: { click: "submit($event, '#{custom_redirect_path}')", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } %input{ type: "button", ng: { value: "order_cycle_form.$dirty ? '#{t('.cancel')}' : '#{t('.back_to_list')}'", click: "cancel('#{main_app.admin_order_cycles_path}')" } } - - if order_cycles_simple_form + - if @order_cycle.simple? = render 'simple_form', f: f - else = render 'form', f: f diff --git a/app/views/admin/order_cycles/outgoing.html.haml b/app/views/admin/order_cycles/outgoing.html.haml index 9924e3dae9..e7756a713d 100644 --- a/app/views/admin/order_cycles/outgoing.html.haml +++ b/app/views/admin/order_cycles/outgoing.html.haml @@ -10,7 +10,8 @@ %save-bar{ dirty: "order_cycle_form.$dirty", persist: "true" } %input.red{ type: "button", value: t('.save'), ng: { click: "submit($event, null)", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } - %input.red{ type: "button", value: t('.save_and_back_to_list'), ng: { click: "submit($event, '#{main_app.admin_order_cycles_path}')", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } + %input.red{ type: "button", value: t('.save_and_next'), ng: { click: "submit($event, '#{main_app.admin_order_cycle_checkout_options_path(@order_cycle)}')", disabled: "!order_cycle_form.$dirty || order_cycle_form.$invalid" } } + %input{ type: "button", value: t('.next'), ng: { click: "cancel('#{main_app.admin_order_cycle_checkout_options_path(@order_cycle)}')", disabled: "order_cycle_form.$dirty" } } %input{ type: "button", ng: { value: "order_cycle_form.$dirty ? '#{t('.cancel')}' : '#{t('.back_to_list')}'", click: "cancel('#{main_app.admin_order_cycles_path}')" } } %fieldset.no-border-bottom diff --git a/app/views/spree/admin/orders/_shipment.html.haml b/app/views/spree/admin/orders/_shipment.html.haml index e9fcb169de..653f93ea5c 100644 --- a/app/views/spree/admin/orders/_shipment.html.haml +++ b/app/views/spree/admin/orders/_shipment.html.haml @@ -38,7 +38,7 @@ %td{ :colspan => "5" } %div.field.alpha.five.columns = label_tag 'selected_shipping_rate_id', Spree.t(:shipping_method) - = select_tag :selected_shipping_rate_id, options_for_select(shipment.shipping_rates.backend.map { |sr| ["#{sr.name} #{sr.display_price}", sr.id] }, shipment.selected_shipping_rate_id), { :class => 'select2 fullwidth', :data => { 'shipment-number' => shipment.number } } + = select_tag :selected_shipping_rate_id, options_for_select(shipment.shipping_rates.map { |sr| ["#{sr.name} #{sr.display_price}", sr.id] }, shipment.selected_shipping_rate_id), { :class => 'select2 fullwidth', :data => { 'shipment-number' => shipment.number } } %td.actions - if can? :update, shipment diff --git a/app/views/spree/admin/shipping_methods/_form.html.haml b/app/views/spree/admin/shipping_methods/_form.html.haml index 2148576ca4..5ca959f927 100644 --- a/app/views/spree/admin/shipping_methods/_form.html.haml +++ b/app/views/spree/admin/shipping_methods/_form.html.haml @@ -19,7 +19,7 @@ .alpha.four.columns = f.label :display_on, t(:display) .omega.twelve.columns - = select(:shipping_method, :display_on, [[t(".both"), nil], [t(".back_end"), "back_end"]], {}, {class: 'select2 fullwidth'}) + = select(:shipping_method, :display_on, Spree::ShippingMethod::DISPLAY_ON_OPTIONS.map { |key, value| [t(".#{key}"), value] }, {}, {class: 'select2 fullwidth'}) = error_message_on :shipping_method, :display_on .row diff --git a/app/webpacker/controllers/select_all_controller.js b/app/webpacker/controllers/select_all_controller.js new file mode 100644 index 0000000000..30a00aee79 --- /dev/null +++ b/app/webpacker/controllers/select_all_controller.js @@ -0,0 +1,19 @@ +import { Controller } from "stimulus"; + +export default class extends Controller { + static targets = ["all", "checkbox"]; + + connect() { + this.toggleCheckbox() + } + + toggleAll() { + this.checkboxTargets.forEach(checkbox => { + checkbox.checked = this.allTarget.checked; + }); + } + + toggleCheckbox() { + this.allTarget.checked = this.checkboxTargets.every(checkbox => checkbox.checked); + } +} diff --git a/app/webpacker/css/admin/components/save_bar.scss b/app/webpacker/css/admin/components/save_bar.scss index 70f906119f..b60482de11 100644 --- a/app/webpacker/css/admin/components/save_bar.scss +++ b/app/webpacker/css/admin/components/save_bar.scss @@ -11,6 +11,10 @@ h5 { color: $spree-blue; + + &.error { + color: $red-500; + } } input { diff --git a/app/webpacker/css/admin/openfoodnetwork.scss b/app/webpacker/css/admin/openfoodnetwork.scss index 636b938d6d..aa22632fd0 100644 --- a/app/webpacker/css/admin/openfoodnetwork.scss +++ b/app/webpacker/css/admin/openfoodnetwork.scss @@ -96,6 +96,14 @@ form.order_cycle { .icon-question-sign { font-size: 18px; } + table.checkout-options { + ul { + margin-left: 1em; + } + p, li { + margin: 0.5em 0; + } + } table.exchanges { tr td.active { width: 20px; diff --git a/config/locales/en.yml b/config/locales/en.yml index be87cc9fab..63e4d3f830 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1156,15 +1156,29 @@ en: tags: "Tags" delivery_details: "Delivery Details" fees: "Fees" + next: "Next" previous: "Previous" save: "Save" - save_and_back_to_list: "Save and Back to List" + save_and_next: "Save and Next" cancel: "Cancel" back_to_list: "Back To List" + checkout_options: + back_end: "Back office only" + cancel: "Cancel" + checkout_options: "Checkout options" + distributor: "Distributor" + no_payment_methods: Each distributor on this order cycle requires at least one payment method. + no_shipping_methods: Each distributor on this order cycle requires at least one shipping method. + payment_methods: "Payment Methods" + save: "Save" + save_and_back_to_list: "Save and Back to List" + select_all: "Select all" + shipping_methods: "Shipping Methods" wizard_progress: edit: "1. General Settings" incoming: "2. Incoming Products" outgoing: "3. Outgoing Products" + checkout_options: "4. Checkout Options" exchange_form: pickup_time_tip: When orders from this OC will be ready for the customer pickup_instructions_placeholder: "Pick-up instructions" diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 7fc4afcd8f..99beb534d0 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -15,6 +15,7 @@ Openfoodnetwork::Application.routes.draw do post :bulk_update, on: :collection, as: :bulk_update get :incoming get :outgoing + get :checkout_options member do get :clone diff --git a/db/migrate/20220429092052_create_order_cycles_distributor_shipping_methods.rb b/db/migrate/20220429092052_create_order_cycles_distributor_shipping_methods.rb new file mode 100644 index 0000000000..e2d5224235 --- /dev/null +++ b/db/migrate/20220429092052_create_order_cycles_distributor_shipping_methods.rb @@ -0,0 +1,17 @@ +class CreateOrderCyclesDistributorShippingMethods < ActiveRecord::Migration[6.1] + def up + create_table :order_cycles_distributor_shipping_methods, id: false do |t| + t.belongs_to :order_cycle, + index: { name: "index_oc_id_on_order_cycles_distributor_shipping_methods" } + t.belongs_to :distributor_shipping_method, + index: { name: "index_dsm_id_on_order_cycles_distributor_shipping_methods" } + t.index [:order_cycle_id, :distributor_shipping_method_id], + name: "order_cycles_distributor_shipping_methods_join_index", + unique: true + end + end + + def down + drop_table :order_cycles_distributor_shipping_methods + end +end diff --git a/db/schema.rb b/db/schema.rb index 6c9b1b00fe..1925baa7fa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -330,6 +330,14 @@ ActiveRecord::Schema.define(version: 2022_09_07_055044) do t.boolean "mails_sent", default: false end + create_table "order_cycles_distributor_shipping_methods", id: false, force: :cascade do |t| + t.bigint "order_cycle_id" + t.bigint "distributor_shipping_method_id" + t.index ["distributor_shipping_method_id"], name: "index_dsm_id_on_order_cycles_distributor_shipping_methods" + t.index ["order_cycle_id", "distributor_shipping_method_id"], name: "order_cycles_distributor_shipping_methods_join_index", unique: true + t.index ["order_cycle_id"], name: "index_oc_id_on_order_cycles_distributor_shipping_methods" + end + create_table "producer_properties", id: :serial, force: :cascade do |t| t.string "value", limit: 255 t.integer "producer_id" diff --git a/spec/factories/order_cycle_factory.rb b/spec/factories/order_cycle_factory.rb index 8dbfcc6acd..469fb94019 100644 --- a/spec/factories/order_cycle_factory.rb +++ b/spec/factories/order_cycle_factory.rb @@ -59,6 +59,10 @@ FactoryBot.define do end end + # Note: Order cycles are sometimes referred to as 'simple if they are for a shop selling their + # own produce i.e. :sells = 'own'. However the 'simple_order_cycle' name does not mean this + # and may need to be renamed to avoid potential confusion because it actually can create + # 'non-simple' order cycles too for distributors selling produce from other enterprises. factory :simple_order_cycle, class: OrderCycle do sequence(:name) { |n| "Order Cycle #{n}" } @@ -116,4 +120,12 @@ FactoryBot.define do orders_open_at { 2.weeks.ago } orders_close_at { 1.week.ago } end + + factory :distributor_order_cycle, parent: :simple_order_cycle do + coordinator { create(:distributor_enterprise) } + end + + factory :sells_own_order_cycle, parent: :simple_order_cycle do + coordinator { create(:enterprise, sells: "own") } + end end diff --git a/spec/helpers/enterprises_helper_spec.rb b/spec/helpers/enterprises_helper_spec.rb index 7029ede7c3..9bea500126 100644 --- a/spec/helpers/enterprises_helper_spec.rb +++ b/spec/helpers/enterprises_helper_spec.rb @@ -9,140 +9,6 @@ describe EnterprisesHelper, type: :helper do before { allow(helper).to receive(:spree_current_user) { user } } - describe "loading available shipping methods" do - let!(:distributor_shipping_method) { - create(:shipping_method, require_ship_address: false, distributors: [distributor]) - } - let!(:other_distributor_shipping_method) { - create(:shipping_method, require_ship_address: false, distributors: [some_other_distributor]) - } - - context "when the order has no current_distributor" do - before do - allow(helper).to receive(:current_distributor) { nil } - end - - it "returns an empty array" do - expect(helper.available_shipping_methods).to eq [] - end - end - - context "when no tag rules are in effect" do - before { allow(helper).to receive(:current_distributor) { distributor } } - - it "finds the shipping methods for the current distributor" do - expect(helper.available_shipping_methods).to_not include other_distributor_shipping_method - expect(helper.available_shipping_methods).to include distributor_shipping_method - end - - it "does not return 'back office only' shipping method" do - backoffice_only_shipping_method = create(:shipping_method, require_ship_address: false, - distributors: [distributor], display_on: 'back_end') - - expect(helper.available_shipping_methods).to_not include backoffice_only_shipping_method - expect(helper.available_shipping_methods).to_not include other_distributor_shipping_method - expect(helper.available_shipping_methods).to include distributor_shipping_method - end - end - - context "when FilterShippingMethods tag rules are in effect" do - let(:customer) { create(:customer, user: user, enterprise: distributor) } - let!(:tag_rule) { - create(:filter_shipping_methods_tag_rule, - enterprise: distributor, - preferred_customer_tags: "local", - preferred_shipping_method_tags: "local-delivery") - } - let!(:default_tag_rule) { - create(:filter_shipping_methods_tag_rule, - enterprise: distributor, - is_default: true, - preferred_shipping_method_tags: "local-delivery") - } - let!(:tagged_sm) { distributor_shipping_method } - let!(:untagged_sm) { other_distributor_shipping_method } - - before do - tagged_sm.update_attribute(:tag_list, 'local-delivery') - distributor.shipping_methods = [tagged_sm, untagged_sm] - allow(helper).to receive(:current_distributor) { distributor } - end - - context "with a preferred visiblity of 'visible', default visibility of 'hidden'" do - before { - tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, 'visible') - } - before { - default_tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, - 'hidden') - } - - context "when the customer is nil" do - it "applies default action (hide)" do - expect(helper.current_customer).to be nil - expect(helper.available_shipping_methods).to include untagged_sm - expect(helper.available_shipping_methods).to_not include tagged_sm - end - end - - context "when the customer's tags match" do - before { customer.update_attribute(:tag_list, 'local') } - - it "applies the action (show)" do - expect(helper.current_customer).to eq customer - expect(helper.available_shipping_methods).to include tagged_sm, untagged_sm - end - end - - context "when the customer's tags don't match" do - before { customer.update_attribute(:tag_list, 'something') } - - it "applies the default action (hide)" do - expect(helper.current_customer).to eq customer - expect(helper.available_shipping_methods).to include untagged_sm - expect(helper.available_shipping_methods).to_not include tagged_sm - end - end - end - - context "with a preferred visiblity of 'hidden', default visibility of 'visible'" do - before { - tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, 'hidden') - } - before { - default_tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, - 'visible') - } - - context "when the customer is nil" do - it "applies default action (show)" do - expect(helper.current_customer).to be nil - expect(helper.available_shipping_methods).to include tagged_sm, untagged_sm - end - end - - context "when the customer's tags match" do - before { customer.update_attribute(:tag_list, 'local') } - - it "applies the action (hide)" do - expect(helper.current_customer).to eq customer - expect(helper.available_shipping_methods).to include untagged_sm - expect(helper.available_shipping_methods).to_not include tagged_sm - end - end - - context "when the customer's tags don't match" do - before { customer.update_attribute(:tag_list, 'something') } - - it "applies the default action (show)" do - expect(helper.current_customer).to eq customer - expect(helper.available_shipping_methods).to include tagged_sm, untagged_sm - end - end - end - end - end - describe "loading available payment methods" do let!(:pm1) { create(:payment_method, distributors: [distributor]) } let!(:pm2) { create(:payment_method, distributors: [some_other_distributor]) } diff --git a/spec/javascripts/stimulus/select_all_controller_test.js b/spec/javascripts/stimulus/select_all_controller_test.js new file mode 100644 index 0000000000..f7a17b884f --- /dev/null +++ b/spec/javascripts/stimulus/select_all_controller_test.js @@ -0,0 +1,113 @@ +/** + * @jest-environment jsdom + */ + +import { Application } from "stimulus"; +import select_all_controller from "../../../app/webpacker/controllers/select_all_controller"; + +describe("SelectAllController", () => { + beforeAll(() => { + const application = Application.start(); + application.register("select-all", select_all_controller); + }); + + beforeEach(() => { + document.body.innerHTML = ` +
+ + + +
+ `; + }); + + describe("#toggleAll", () => { + it("checks all checkboxes when it's checked and unchecks them all when unchecked", () => { + const selectAllCheckbox = document.getElementById("selectAllCheckbox"); + const checkboxA = document.getElementById("checkboxA"); + const checkboxB = document.getElementById("checkboxB"); + expect(selectAllCheckbox.checked).toBe(false); + expect(checkboxA.checked).toBe(false); + expect(checkboxB.checked).toBe(false); + + selectAllCheckbox.click() + + expect(selectAllCheckbox.checked).toBe(true); + expect(checkboxA.checked).toBe(true); + expect(checkboxB.checked).toBe(true); + + selectAllCheckbox.click() + + expect(selectAllCheckbox.checked).toBe(false); + expect(checkboxA.checked).toBe(false); + expect(checkboxB.checked).toBe(false); + }); + }); + + describe("#toggleCheckbox", () => { + it("checks the individual checkbox and checks the select all checkbox if all checkboxes are checked and vice versa", () => { + const selectAllCheckbox = document.getElementById("selectAllCheckbox"); + const checkboxA = document.getElementById("checkboxA"); + const checkboxB = document.getElementById("checkboxB"); + checkboxA.click() + expect(selectAllCheckbox.checked).toBe(false); + expect(checkboxA.checked).toBe(true); + expect(checkboxB.checked).toBe(false); + + checkboxB.click() + + expect(selectAllCheckbox.checked).toBe(true); + expect(checkboxA.checked).toBe(true); + expect(checkboxB.checked).toBe(true); + + checkboxB.click() + + expect(selectAllCheckbox.checked).toBe(false); + expect(checkboxA.checked).toBe(true); + expect(checkboxB.checked).toBe(false); + }); + }); + + describe("#connect", () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ + + +
+ `; + }); + + it("checks the select all checkbox on page load if all checkboxes are checked", () => { + const selectAllCheckbox = document.getElementById("selectAllCheckbox"); + expect(selectAllCheckbox.checked).toBe(true); + }); + }); +}); diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index bc35b51f4a..47b7d41c0f 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -276,6 +276,13 @@ describe Enterprise do expect(Enterprise.ready_for_checkout).not_to include e end + it "does not show enterprises wchich only have backend shipping methods" do + create(:shipping_method, distributors: [e], + display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end]) + create(:payment_method, distributors: [e]) + expect(Enterprise.ready_for_checkout).not_to include e + end + it "shows enterprises with available payment and shipping methods" do create(:shipping_method, distributors: [e]) create(:payment_method, distributors: [e]) @@ -302,6 +309,13 @@ describe Enterprise do expect(Enterprise.not_ready_for_checkout).to include e end + it "shows enterprises which only have backend shipping methods" do + create(:shipping_method, distributors: [e], + display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end]) + create(:payment_method, distributors: [e]) + expect(Enterprise.not_ready_for_checkout).to include e + end + it "does not show enterprises with available payment and shipping methods" do create(:shipping_method, distributors: [e]) create(:payment_method, distributors: [e]) @@ -328,6 +342,13 @@ describe Enterprise do expect(e.reload).not_to be_ready_for_checkout end + it "returns false for enterprises which only have backend shipping methods" do + create(:shipping_method, distributors: [e], + display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end]) + create(:payment_method, distributors: [e]) + expect(e.reload).not_to be_ready_for_checkout + end + it "returns true for enterprises with available payment and shipping methods" do create(:shipping_method, distributors: [e]) create(:payment_method, distributors: [e]) diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 4ebb79a9a1..3789a4aa1a 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -368,39 +368,66 @@ describe OrderCycle do end end - it "clones itself" do - coordinator = create(:enterprise); - oc = create(:simple_order_cycle, - coordinator_fees: [create(:enterprise_fee, enterprise: coordinator)], - preferred_product_selection_from_coordinator_inventory_only: true, - automatic_notifications: true, processed_at: Time.zone.now, mails_sent: true) - schedule = create(:schedule, order_cycles: [oc]) - ex1 = create(:exchange, order_cycle: oc) - ex2 = create(:exchange, order_cycle: oc) - oc.clone! + describe "clone!" do + it "clones itself" do + coordinator = create(:enterprise); + oc = create(:simple_order_cycle, + coordinator_fees: [create(:enterprise_fee, enterprise: coordinator)], + preferred_product_selection_from_coordinator_inventory_only: true, + automatic_notifications: true, processed_at: Time.zone.now, mails_sent: true) + schedule = create(:schedule, order_cycles: [oc]) + ex1 = create(:exchange, order_cycle: oc) + ex2 = create(:exchange, order_cycle: oc) + oc.clone! - occ = OrderCycle.last - expect(occ.name).to eq("COPY OF #{oc.name}") - expect(occ.orders_open_at).to be_nil - expect(occ.orders_close_at).to be_nil - expect(occ.coordinator).not_to be_nil - expect(occ.preferred_product_selection_from_coordinator_inventory_only).to be true - expect(occ.automatic_notifications).to eq(oc.automatic_notifications) - expect(occ.processed_at).to eq(nil) - expect(occ.mails_sent).to eq(nil) - expect(occ.coordinator).to eq(oc.coordinator) + occ = OrderCycle.last + expect(occ.name).to eq("COPY OF #{oc.name}") + expect(occ.orders_open_at).to be_nil + expect(occ.orders_close_at).to be_nil + expect(occ.coordinator).not_to be_nil + expect(occ.preferred_product_selection_from_coordinator_inventory_only).to be true + expect(occ.automatic_notifications).to eq(oc.automatic_notifications) + expect(occ.processed_at).to eq(nil) + expect(occ.mails_sent).to eq(nil) + expect(occ.coordinator).to eq(oc.coordinator) - expect(occ.coordinator_fee_ids).not_to be_empty - expect(occ.coordinator_fee_ids).to eq(oc.coordinator_fee_ids) - expect(occ.preferred_product_selection_from_coordinator_inventory_only).to eq(oc.preferred_product_selection_from_coordinator_inventory_only) - expect(occ.schedule_ids).not_to be_empty - expect(occ.schedule_ids).to eq(oc.schedule_ids) + expect(occ.coordinator_fee_ids).not_to be_empty + expect(occ.coordinator_fee_ids).to eq(oc.coordinator_fee_ids) + expect(occ.preferred_product_selection_from_coordinator_inventory_only).to eq(oc.preferred_product_selection_from_coordinator_inventory_only) + expect(occ.schedule_ids).not_to be_empty + expect(occ.schedule_ids).to eq(oc.schedule_ids) - # Check that the exchanges have been cloned. - original_exchange_attributes = oc.exchanges.map { |ex| core_exchange_attributes(ex) } - cloned_exchange_attributes = occ.exchanges.map { |ex| core_exchange_attributes(ex) } + # Check that the exchanges have been cloned. + original_exchange_attributes = oc.exchanges.map { |ex| core_exchange_attributes(ex) } + cloned_exchange_attributes = occ.exchanges.map { |ex| core_exchange_attributes(ex) } - expect(cloned_exchange_attributes).to match_array original_exchange_attributes + expect(cloned_exchange_attributes).to match_array original_exchange_attributes + end + + context "when it has preferred shipping methods which can longer be applied validly + e.g. shipping method is backoffice only" do + it "only attaches the valid ones to the clone" do + distributor = create(:distributor_enterprise) + distributor_shipping_method_i = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + distributor_shipping_method_ii = create( + :shipping_method, + distributors: [distributor], + display_on: Spree::ShippingMethod::DISPLAY_ON_OPTIONS[:back_end] + ).distributor_shipping_methods.first + order_cycle = create(:distributor_order_cycle, distributors: [distributor]) + order_cycle.selected_distributor_shipping_methods = [ + distributor_shipping_method_i, + distributor_shipping_method_ii + ] + + cloned_order_cycle = order_cycle.clone! + + expect(cloned_order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method_i] + end + end end describe "finding recently closed order cycles" do @@ -610,6 +637,112 @@ describe OrderCycle do end end + describe "#attachable_distributor_shipping_methods" do + it "includes distributor shipping methods from the distributors on the order cycle" do + shipping_method = create(:shipping_method) + oc = create(:simple_order_cycle, distributors: [shipping_method.distributors.first]) + distributor_shipping_method = shipping_method.distributor_shipping_methods.first + + expect(oc.attachable_distributor_shipping_methods).to eq([distributor_shipping_method]) + end + + it "does not include backoffice only distributor shipping methods" do + shipping_method = create(:shipping_method, display_on: "back_end") + enterprise = create(:enterprise, shipping_methods: [shipping_method]) + oc = create(:simple_order_cycle, distributors: [enterprise]) + + expect(oc.attachable_distributor_shipping_methods).to be_empty + end + end + + describe "#distributor_shipping_methods" do + let(:distributor) { create(:distributor_enterprise) } + + it "returns all attachable distributor shipping methods if the order cycle is simple" do + oc = create(:sells_own_order_cycle, distributors: [distributor]) + + distributor_shipping_method = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + + expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method] + end + + context "distributor order cycle i.e. non-simple" do + let(:oc) { create(:distributor_order_cycle, distributors: [distributor]) } + + it "returns all attachable distributor shipping methods if no distributor shipping methods + have been selected specifically" do + distributor_shipping_method = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + + expect(oc.selected_distributor_shipping_methods).to be_empty + expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method] + end + + it "returns selected distributor shipping methods if they have been specified" do + distributor_shipping_method_i = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + distributor_shipping_method_ii = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + + oc.selected_distributor_shipping_methods << distributor_shipping_method_ii + + expect(oc.distributor_shipping_methods).to eq [distributor_shipping_method_ii] + end + + context "with multiple distributors" do + let(:other_distributor) { create(:distributor_enterprise) } + let(:oc) { create(:distributor_order_cycle, distributors: [distributor, other_distributor]) } + + it "returns all attachable distributor shipping methods for a distributor if no distributor + shipping methods have been selected specifically for that distributor, even if + distributor shipping methods have been selected specifically for a different distributor + on the order cycle" do + distributor_shipping_method = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + other_distributor_shipping_method_i = create( + :shipping_method, + distributors: [other_distributor] + ).distributor_shipping_methods.first + other_distributor_shipping_method_ii = create( + :shipping_method, + distributors: [other_distributor] + ).distributor_shipping_methods.first + oc.selected_distributor_shipping_methods << other_distributor_shipping_method_i + + expect(oc.distributor_shipping_methods).to eq [ + distributor_shipping_method, + other_distributor_shipping_method_i + ] + end + end + end + end + + describe "#simple?" do + it "returns true if the coordinator sells their own products i.e. shops" do + order_cycle = build(:simple_order_cycle, coordinator: build(:enterprise, sells: "own")) + + expect(order_cycle).to be_simple + end + + it "returns false if the coordinator can sell other people's products i.e. hubs" do + order_cycle = build(:simple_order_cycle, coordinator: build(:enterprise, sells: "any")) + + expect(order_cycle).not_to be_simple + end + end + def core_exchange_attributes(exchange) exterior_attribute_keys = %w(id order_cycle_id created_at updated_at) exchange.attributes. diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index b505a3a808..d6ba024989 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -143,6 +143,25 @@ module Spree expect(shipping_method.errors[:name].first).to eq "can't be blank" end + describe "#display_on" do + it "is valid when it's set to nil, an empty string or 'back_end'" do + shipping_method = build_stubbed( + :shipping_method, + shipping_categories: [Spree::ShippingCategory.new(name: 'Test')] + ) + [nil, "", "back_end"].each do |display_on_option| + shipping_method.display_on = display_on_option + expect(shipping_method).to be_valid + end + end + + it "is not valid when it's set to an unknown value" do + shipping_method = build_stubbed(:shipping_method, display_on: "front_end") + expect(shipping_method).not_to be_valid + expect(shipping_method.errors[:display_on]).to eq ["is not included in the list"] + end + end + context "shipping category" do it "validates presence of at least one" do shipping_method = build_stubbed( diff --git a/spec/services/order_available_shipping_methods_spec.rb b/spec/services/order_available_shipping_methods_spec.rb new file mode 100644 index 0000000000..59acb35dc6 --- /dev/null +++ b/spec/services/order_available_shipping_methods_spec.rb @@ -0,0 +1,193 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe OrderAvailableShippingMethods do + context "when the order has no current_distributor" do + it "returns an empty array" do + order_cycle = create(:sells_own_order_cycle) + order = build(:order, distributor: nil, order_cycle: order_cycle) + + expect(OrderAvailableShippingMethods.new(order).to_a).to eq [] + end + end + + it "does not return 'back office only' shipping method" do + distributor = create(:distributor_enterprise) + frontend_shipping_method = create(:shipping_method, distributors: [distributor]) + backoffice_only_shipping_method = create(:shipping_method, + distributors: [distributor], display_on: 'back_end') + order_cycle = create(:sells_own_order_cycle) + order = build(:order, distributor: distributor, order_cycle: order_cycle) + + available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a + + expect(available_shipping_methods).to eq [frontend_shipping_method] + end + + context "when no tag rules are in effect" do + context "sells own order cycle i.e. simple" do + it "only returns the shipping methods which are available on the order cycle + and belong to the order distributor" do + distributor_i = create(:distributor_enterprise) + distributor_ii = create(:distributor_enterprise) + distributor_iii = create(:distributor_enterprise) + shipping_method_i = create(:shipping_method, distributors: [distributor_i]) + shipping_method_ii = create(:shipping_method, distributors: [distributor_ii]) + shipping_method_iii = create(:shipping_method, distributors: [distributor_iii]) + order_cycle = create(:sells_own_order_cycle, distributors: [distributor_i, distributor_ii]) + order = build(:order, distributor: distributor_i, order_cycle: order_cycle) + + available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a + + expect(available_shipping_methods).to eq [shipping_method_i] + end + end + + context "distributor order cycle i.e. not simple" do + it "only returns the shipping methods which are available on the order cycle + and belong to the order distributor" do + distributor_i = create(:distributor_enterprise, shipping_methods: []) + distributor_ii = create(:distributor_enterprise, shipping_methods: []) + shipping_method_i = create(:shipping_method, distributors: [distributor_i]) + shipping_method_ii = create(:shipping_method, distributors: [distributor_i]) + shipping_method_iii = create(:shipping_method, distributors: [distributor_ii]) + shipping_method_iv = create(:shipping_method, distributors: [distributor_ii]) + order_cycle = create(:distributor_order_cycle, + distributors: [distributor_i, distributor_ii]) + order_cycle.selected_distributor_shipping_methods << [ + distributor_i.distributor_shipping_methods.first, + distributor_ii.distributor_shipping_methods.first, + ] + order = build(:order, distributor: distributor_i, order_cycle: order_cycle) + + available_shipping_methods = OrderAvailableShippingMethods.new(order).to_a + + expect(available_shipping_methods).to eq [shipping_method_i] + end + end + end + + context "when FilterShippingMethods tag rules are in effect" do + let(:user) { create(:user) } + let(:distributor) { create(:distributor_enterprise) } + let(:other_distributor) { create(:distributor_enterprise) } + let!(:distributor_shipping_method) { create(:shipping_method, distributors: [distributor]) } + let!(:other_distributor_shipping_method) do + create(:shipping_method, distributors: [other_distributor]) + end + let(:customer) { create(:customer, user: user, enterprise: distributor) } + let!(:tag_rule) { + create(:filter_shipping_methods_tag_rule, + enterprise: distributor, + preferred_customer_tags: "local", + preferred_shipping_method_tags: "local-delivery") + } + let!(:default_tag_rule) { + create(:filter_shipping_methods_tag_rule, + enterprise: distributor, + is_default: true, + preferred_shipping_method_tags: "local-delivery") + } + let!(:tagged_sm) { distributor_shipping_method } + let!(:untagged_sm) { other_distributor_shipping_method } + + before do + tagged_sm.update_attribute(:tag_list, 'local-delivery') + distributor.shipping_methods = [tagged_sm, untagged_sm] + end + + context "with a preferred visiblity of 'visible', default visibility of 'hidden'" do + before { + tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, 'visible') + } + before { + default_tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, + 'hidden') + } + + let(:order_cycle) { create(:sells_own_order_cycle) } + let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) } + + context "when the customer is nil" do + let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a } + + it "applies default action (hide)" do + expect(available_shipping_methods).to include untagged_sm + expect(available_shipping_methods).to_not include tagged_sm + end + end + + context "when a customer is present" do + let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a } + + context "and the customer's tags match" do + before do + customer.update_attribute(:tag_list, 'local') + end + + it "applies the action (show)" do + expect(available_shipping_methods).to include tagged_sm, untagged_sm + end + end + + context "and the customer's tags don't match" do + before do + customer.update_attribute(:tag_list, 'something') + end + + it "applies the default action (hide)" do + expect(available_shipping_methods).to include untagged_sm + expect(available_shipping_methods).to_not include tagged_sm + end + end + end + end + + context "with a preferred visiblity of 'hidden', default visibility of 'visible'" do + before { + tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, 'hidden') + } + before { + default_tag_rule.update_attribute(:preferred_matched_shipping_methods_visibility, + 'visible') + } + + let(:order_cycle) { create(:sells_own_order_cycle) } + let(:order) { build(:order, distributor: distributor, order_cycle: order_cycle) } + + context "when the customer is nil" do + let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order).to_a } + + it "applies default action (show)" do + expect(available_shipping_methods).to include tagged_sm, untagged_sm + end + end + + context "when a customer is present" do + let(:available_shipping_methods) { OrderAvailableShippingMethods.new(order, customer).to_a } + + context "and the customer's tags match" do + before do + customer.update_attribute(:tag_list, 'local') + end + + it "applies the action (hide)" do + expect(available_shipping_methods).to include untagged_sm + expect(available_shipping_methods).to_not include tagged_sm + end + end + + context "and the customer's tags don't match" do + before do + customer.update_attribute(:tag_list, 'something') + end + + it "applies the default action (show)" do + expect(available_shipping_methods).to include tagged_sm, untagged_sm + end + end + end + end + end +end diff --git a/spec/services/order_cycle_form_spec.rb b/spec/services/order_cycle_form_spec.rb index 4d5bce792c..0117a1537a 100644 --- a/spec/services/order_cycle_form_spec.rb +++ b/spec/services/order_cycle_form_spec.rb @@ -55,6 +55,17 @@ describe OrderCycleForm do end.to_not change{ order_cycle.reload.name } end end + + context "when schedules are present but updating something other than the :schedule_ids" do + let(:params) { { name: "New Order Cycle Name" } } + before { create(:schedule, order_cycles: [order_cycle]) } + + it "doesn't delete the schedules" do + expect(order_cycle.schedules).to be_present + form.save + expect(order_cycle.schedules).to be_present + end + end end end @@ -119,34 +130,128 @@ describe OrderCycleForm do end end - describe "updating exchanges" do - let(:user) { instance_double(Spree::User) } - let(:order_cycle) { create(:simple_order_cycle) } - let(:form_applicator_mock) { instance_double(OpenFoodNetwork::OrderCycleFormApplicator) } - let(:form) { OrderCycleForm.new(order_cycle, params, user) } + context "distributor order cycle" do + let(:order_cycle) { create(:distributor_order_cycle) } + let(:distributor) { order_cycle.coordinator } + let(:supplier) { create(:supplier_enterprise) } + let(:user) { distributor.owner } + let(:shipping_method) { create(:shipping_method, distributors: [distributor]) } + let(:distributor_shipping_method) { shipping_method.distributor_shipping_methods.first } + let(:variant) { create(:variant, product: create(:product, supplier: supplier)) } let(:params) { { name: 'Some new name' } } - - before do - allow(OpenFoodNetwork::OrderCycleFormApplicator).to receive(:new) { form_applicator_mock } - allow(form_applicator_mock).to receive(:go!) + let(:form) { OrderCycleForm.new(order_cycle, params, user) } + let(:outgoing_exchange_params) do + { + enterprise_id: distributor.id, + incoming: false, + active: true, + variants: { variant.id => true }, + pickup_time: "Saturday morning", + enterprise_fee_ids: [] + } end - context "when exchange params are provided" do - let(:exchange_params) { { incoming_exchanges: [], outgoing_exchanges: [] } } - before { params.merge!(exchange_params) } - - it "runs the OrderCycleFormApplicator, and saves other changes" do + context "basic update i.e. without exchanges or shipping methods" do + it do expect(form.save).to be true - expect(form_applicator_mock).to have_received(:go!) expect(order_cycle.name).to eq 'Some new name' end end - context "when no exchange params are provided" do - it "does not run the OrderCycleFormApplicator, but saves other changes" do + context "updating basics, incoming exchanges, outcoming exchanges + and shipping methods simultaneously" do + before do + params.merge!( + incoming_exchanges: [{ + enterprise_id: supplier.id, + incoming: true, + active: true, + variants: { variant.id => true }, + receival_instructions: "Friday evening", + enterprise_fee_ids: [] + }], + outgoing_exchanges: [outgoing_exchange_params], + selected_distributor_shipping_method_ids: [distributor_shipping_method.id] + ) + end + + it "saves everything i.e. the basics, incoming and outgoing exchanges and shipping methods" do expect(form.save).to be true - expect(form_applicator_mock).to_not have_received(:go!) expect(order_cycle.name).to eq 'Some new name' + expect(order_cycle.cached_incoming_exchanges.count).to eq 1 + expect(order_cycle.cached_outgoing_exchanges.count).to eq 1 + expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method] + end + end + + context "updating outgoing exchanges and shipping methods simultaneously but the shipping + method doesn't belong to the new or any existing order cycle distributor" do + let(:other_distributor_shipping_method) do + create( + :shipping_method, + distributors: [create(:distributor_enterprise)] + ).distributor_shipping_methods.first + end + + before do + params.merge!( + outgoing_exchanges: [outgoing_exchange_params], + selected_distributor_shipping_method_ids: [other_distributor_shipping_method.id] + ) + end + + it "saves the outgoing exchange but ignores the shipping method" do + expect(form.save).to be true + expect(order_cycle.distributors).to eq [distributor] + expect(order_cycle.distributor_shipping_methods).to be_empty + end + end + + context "updating shipping methods" do + context "and it's valid" do + it "saves the changes" do + distributor = create(:distributor_enterprise) + distributor_shipping_method = create( + :shipping_method, + distributors: [distributor] + ).distributor_shipping_methods.first + order_cycle = create(:distributor_order_cycle, distributors: [distributor]) + + form = OrderCycleForm.new( + order_cycle, + { selected_distributor_shipping_method_ids: [distributor_shipping_method.id] }, + order_cycle.coordinator + ) + + expect(form.save).to be true + expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method] + end + end + + context "with a shipping method which doesn't belong to any distributor on the order cycle" do + it "ignores it" do + distributor_i = create(:distributor_enterprise) + distributor_ii = create(:distributor_enterprise) + distributor_shipping_method_i = create( + :shipping_method, + distributors: [distributor_i] + ).distributor_shipping_methods.first + distributor_shipping_method_ii = create( + :shipping_method, + distributors: [distributor_ii] + ).distributor_shipping_methods.first + order_cycle = create(:distributor_order_cycle, + distributors: [distributor_i]) + + form = OrderCycleForm.new( + order_cycle, + { selected_distributor_shipping_method_ids: [distributor_shipping_method_ii.id] }, + order_cycle.coordinator + ) + + expect(form.save).to be true + expect(order_cycle.distributor_shipping_methods).to eq [distributor_shipping_method_i] + end end end end diff --git a/spec/services/shop/order_cycles_list_spec.rb b/spec/services/shop/order_cycles_list_spec.rb new file mode 100644 index 0000000000..3b0e5fe045 --- /dev/null +++ b/spec/services/shop/order_cycles_list_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Shop::OrderCyclesList do + describe ".active_for" do + let(:customer) { nil } + + context "when the order cycle is open and the distributor belongs to the order cycle" do + context "and the distributor is ready for checkout" do + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) } + + it "returns the order cycle" do + open_order_cycle = create(:open_order_cycle, distributors: [distributor]) + + expect(Shop::OrderCyclesList.active_for(distributor, customer)).to eq [open_order_cycle] + end + end + + context "and the distributor is not ready for checkout" do + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: false) } + + it "returns the order cycle" do + open_order_cycle = create(:open_order_cycle, distributors: [distributor]) + + expect(Shop::OrderCyclesList.active_for(distributor, customer)).to eq [open_order_cycle] + end + end + end + + it "doesn't returns closed order cycles or ones belonging to other distributors" do + distributor = create(:distributor_enterprise) + closed_order_cycle = create(:closed_order_cycle, distributors: [distributor]) + other_distributor_order_cycle = create(:open_order_cycle) + + expect(Shop::OrderCyclesList.active_for(distributor, customer)).to be_empty + end + end + + describe ".ready_for_checkout_for" do + let(:customer) { nil } + + context "when the order cycle is open and belongs to the distributor" do + context "and the distributor is ready for checkout" do + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) } + + it "returns the order cycle" do + open_order_cycle = create(:open_order_cycle, distributors: [distributor]) + + expect(Shop::OrderCyclesList.ready_for_checkout_for(distributor, customer)).to eq [ + open_order_cycle + ] + end + end + + context "but the distributor not is ready for checkout" do + let(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: false) } + + it "doesn't return the order cycle" do + open_order_cycle = create(:open_order_cycle, distributors: [distributor]) + + expect(Shop::OrderCyclesList.ready_for_checkout_for(distributor, customer)).to be_empty + end + end + end + end +end diff --git a/spec/system/admin/order_cycles/complex_creating_specific_time_spec.rb b/spec/system/admin/order_cycles/complex_creating_specific_time_spec.rb index e16d86796f..4654b12109 100644 --- a/spec/system/admin/order_cycles/complex_creating_specific_time_spec.rb +++ b/spec/system/admin/order_cycles/complex_creating_specific_time_spec.rb @@ -13,16 +13,27 @@ describe ' let(:order_cycle_opening_time) { 1.day.from_now(Time.zone.now) } let(:order_cycle_closing_time) { 2.days.from_now(Time.zone.now) } - it "creating an order cycle with full interface", js: true do - # Given coordinating, supplying and distributing enterprises with some products with variants - coordinator = create(:distributor_enterprise, name: 'My coordinator') - supplier = create(:supplier_enterprise, name: 'My supplier') - product = create(:product, supplier: supplier) - v1 = create(:variant, product: product) - v2 = create(:variant, product: product) - distributor = create(:distributor_enterprise, name: 'My distributor', - with_payment_and_shipping: true) + # Given coordinating, supplying and distributing enterprises with some products with variants + let!(:coordinator) { create(:distributor_enterprise, name: 'My coordinator') } + let!(:supplier) { create(:supplier_enterprise, name: 'My supplier') } + let!(:product) { create(:product, supplier: supplier) } + let!(:v1) { create(:variant, product: product) } + let!(:v2) { create(:variant, product: product) } + let!(:distributor) { + create(:distributor_enterprise, name: 'My distributor', with_payment_and_shipping: true) + } + let!(:shipping_method_i) { distributor.shipping_methods.first } + let!(:shipping_method_ii) { create(:shipping_method, distributors: [distributor]) } + let(:oc) { OrderCycle.last } + # And some enterprise fees + let!(:supplier_fee) { create(:enterprise_fee, enterprise: supplier, name: 'Supplier fee') } + let!(:coordinator_fee) { create(:enterprise_fee, enterprise: coordinator, name: 'Coord fee') } + let!(:distributor_fee) do + create(:enterprise_fee, enterprise: distributor, name: 'Distributor fee') + end + + before do # Relationships required for interface to work create(:enterprise_relationship, parent: supplier, child: coordinator, permissions_list: [:add_to_order_cycle]) @@ -31,12 +42,12 @@ describe ' create(:enterprise_relationship, parent: supplier, child: distributor, permissions_list: [:add_to_order_cycle]) - # And some enterprise fees - supplier_fee = create(:enterprise_fee, enterprise: supplier, name: 'Supplier fee') - coordinator_fee = create(:enterprise_fee, enterprise: coordinator, name: 'Coord fee') - distributor_fee = create(:enterprise_fee, enterprise: distributor, name: 'Distributor fee') + shipping_method_i.update!(name: "Pickup - always available") + shipping_method_ii.update!(name: "Delivery - sometimes available") + end - # When I go to the new order cycle page + it "creating an order cycle with full interface", js: true do + ## CREATE login_as_admin_and_visit admin_order_cycles_path click_link 'New Order Cycle' @@ -44,46 +55,53 @@ describe ' select2_select 'My coordinator', from: 'coordinator_id' click_button "Continue >" - # I cannot save before filling in the required fields - expect(page).to have_button("Create", disabled: true) + fill_in_order_cycle_name + select_opening_and_closing_times - # The Create button is enabled once Name is entered - fill_in 'order_cycle_name', with: 'Plums & Avos' - expect(page).to have_button("Create", disabled: false) - - # If I fill in the basic fields - find('#order_cycle_orders_open_at').click - # select date - select_date_from_datepicker Time.zone.at(order_cycle_opening_time) - # select time - within(".flatpickr-calendar.open .flatpickr-time") do - find('.flatpickr-hour').set('%02d' % order_cycle_opening_time.hour) - find('.flatpickr-minute').set('%02d' % order_cycle_opening_time.min) - end - # hide the datetimepicker - find("body").send_keys(:escape) - - find('#order_cycle_orders_close_at').click - # select date - select_date_from_datepicker Time.zone.at(order_cycle_closing_time) - # select time - within(".flatpickr-calendar.open .flatpickr-time") do - find('.flatpickr-hour').set('%02d' % order_cycle_closing_time.hour) - find('.flatpickr-minute').set('%02d' % order_cycle_closing_time.min) - end - # hide the datetimepicker - find("body").send_keys(:escape) - - # And I add a coordinator fee click_button 'Add coordinator fee' select 'Coord fee', from: 'order_cycle_coordinator_fee_0_id' click_button 'Create' expect(page).to have_content 'Your order cycle has been created.' - # I should not be able to add a blank supplier - expect(page).to have_select 'new_supplier_id', selected: '' - expect(page).to have_button 'Add supplier', disabled: true + ## UPDATE + add_supplier_with_fees + add_distributor_with_fees + select_distributor_shipping_methods + + expect_all_data_saved + end + + def fill_in_order_cycle_name + # I cannot save before filling in the required fields + expect(page).to have_button("Create", disabled: true) + + # The Create button is enabled once Name is entered + fill_in 'order_cycle_name', with: "Plums & Avos" + expect(page).to have_button("Create", disabled: false) + end + + def select_opening_and_closing_times + select_time("#order_cycle_orders_open_at", order_cycle_opening_time) + select_time("#order_cycle_orders_close_at", order_cycle_closing_time) + end + + def select_time(selector, time) + # If I fill in the basic fields + find(selector).click + # select date + select_date_from_datepicker Time.zone.at(time) + # select time + within(".flatpickr-calendar.open .flatpickr-time") do + find('.flatpickr-hour').set('%02d' % time.hour) + find('.flatpickr-minute').set('%02d' % time.min) + end + # hide the datetimepicker + find("body").send_keys(:escape) + end + + def add_supplier_with_fees + expect_not_able_to_add_blank_supplier # And I add a supplier and some products select 'My supplier', from: 'new_supplier_id' @@ -93,10 +111,7 @@ describe ' check "order_cycle_incoming_exchange_0_variants_#{v1.id}" check "order_cycle_incoming_exchange_0_variants_#{v2.id}" - # I should not be able to re-add the supplier - expect(page).not_to have_select 'new_supplier_id', with_options: ['My supplier'] - expect(page).to have_button 'Add supplier', disabled: true - expect(page.all("td.supplier_name").map(&:text)).to eq(['My supplier']) + expect_not_able_to_readd_supplier('My supplier') # And I add a supplier fee within("tr.supplier-#{supplier.id}") { click_button 'Add fee' } @@ -105,11 +120,25 @@ describe ' from: 'order_cycle_incoming_exchange_0_enterprise_fees_0_enterprise_fee_id' click_button 'Save and Next' + end + def expect_not_able_to_add_blank_supplier + expect(page).to have_select 'new_supplier_id', selected: '' + expect(page).to have_button 'Add supplier', disabled: true + end + + def expect_not_able_to_readd_supplier(supplier_name) + expect(page).not_to have_select 'new_supplier_id', with_options: [supplier_name] + expect(page).to have_button 'Add supplier', disabled: true + expect(page.all("td.supplier_name").map(&:text)).to eq([supplier_name]) + end + + def add_distributor_with_fees # And I add a distributor with the same products select 'My distributor', from: 'new_distributor_id' click_button 'Add distributor' + expect(page).to have_field "order_cycle_outgoing_exchange_0_pickup_time" fill_in 'order_cycle_outgoing_exchange_0_pickup_time', with: 'pickup time' fill_in 'order_cycle_outgoing_exchange_0_pickup_instructions', with: 'pickup instructions' @@ -129,37 +158,89 @@ describe ' select 'Distributor fee', from: 'order_cycle_outgoing_exchange_0_enterprise_fees_0_enterprise_fee_id' - click_button 'Save and Back to List' + click_button 'Save and Next' + end - oc = OrderCycle.last + def select_distributor_shipping_methods + expect(page).to have_checked_field "Select all" + + expect(page).to have_checked_field "Pickup - always available" + expect(page).to have_checked_field "Delivery - sometimes available" + + uncheck "Delivery - sometimes available" + + expect(page).to have_unchecked_field "Select all" + + expect_checking_select_all_shipping_methods_works + expect_unchecking_select_all_shipping_methods_works + + # Our final selection: + check "Pickup - always available" + click_button 'Save and Back to List' + end + + def expect_checking_select_all_shipping_methods_works + # Now test that the "Select all" input is doing what it's supposed to: + check "Select all" + + expect(page).to have_checked_field "Pickup - always available" + expect(page).to have_checked_field "Delivery - sometimes available" + end + + def expect_unchecking_select_all_shipping_methods_works + uncheck "Select all" + + expect(page).to have_unchecked_field "Pickup - always available" + expect(page).to have_unchecked_field "Delivery - sometimes available" + end + + def expect_all_data_saved toggle_columns "Producers", "Shops" expect(page).to have_input "oc#{oc.id}[name]", value: "Plums & Avos" + expect(page).to have_content "My coordinator" + expect_opening_and_closing_times_saved + expect(page).to have_selector 'td.producers', text: 'My supplier' + expect(page).to have_selector 'td.shops', text: 'My distributor' + + expect_fees_saved + expect_variants_saved + expect_receival_instructions_saved + expect_pickup_time_and_instructions_saved + expect_distributor_shipping_methods_saved + end + + def expect_opening_and_closing_times_saved expect(page).to have_input "oc#{oc.id}[orders_open_at]", value: Time.zone.at(order_cycle_opening_time), visible: false expect(page).to have_input "oc#{oc.id}[orders_close_at]", value: Time.zone.at(order_cycle_closing_time), visible: false - expect(page).to have_content "My coordinator" + end - expect(page).to have_selector 'td.producers', text: 'My supplier' - expect(page).to have_selector 'td.shops', text: 'My distributor' - - # And it should have some fees + def expect_fees_saved expect(oc.exchanges.incoming.first.enterprise_fees).to eq([supplier_fee]) expect(oc.coordinator_fees).to eq([coordinator_fee]) expect(oc.exchanges.outgoing.first.enterprise_fees).to eq([distributor_fee]) + end - # And it should have some variants selected + def expect_variants_saved expect(oc.exchanges.first.variants.count).to eq(2) expect(oc.exchanges.last.variants.count).to eq(2) + end - # And my receival and pickup time and instructions should have been saved + def expect_receival_instructions_saved exchange = oc.exchanges.incoming.first expect(exchange.receival_instructions).to eq('receival instructions') + end + def expect_pickup_time_and_instructions_saved exchange = oc.exchanges.outgoing.first expect(exchange.pickup_time).to eq('pickup time') expect(exchange.pickup_instructions).to eq('pickup instructions') expect(exchange.tag_list).to eq(['wholesale']) end + + def expect_distributor_shipping_methods_saved + expect(oc.distributor_shipping_methods).to eq(shipping_method_i.distributor_shipping_methods) + end end diff --git a/spec/system/admin/order_cycles/simple_spec.rb b/spec/system/admin/order_cycles/simple_spec.rb index b696134b6e..0d2aac2561 100644 --- a/spec/system/admin/order_cycles/simple_spec.rb +++ b/spec/system/admin/order_cycles/simple_spec.rb @@ -260,6 +260,11 @@ describe ' find(:css, "tags-input .tags input").set "wholesale\n" end + click_button 'Save and Next' + + expect_shipping_methods_to_be_checked_for(distributor_managed) + expect_shipping_methods_to_be_checked_for(distributor_permitted) + click_button 'Save and Back to List' order_cycle = OrderCycle.find_by(name: 'My order cycle') expect(page).to have_input "oc#{order_cycle.id}[name]", value: order_cycle.name @@ -270,6 +275,9 @@ describe ' expect(order_cycle.schedules).to eq([schedule]) exchange = order_cycle.exchanges.outgoing.to_enterprise(distributor_managed).first expect(exchange.tag_list).to eq(["wholesale"]) + expect(order_cycle.distributor_shipping_methods).to match_array( + order_cycle.attachable_distributor_shipping_methods + ) end context "editing an order cycle" do @@ -308,6 +316,7 @@ describe ' # And I remove all outgoing exchanges page.find("tr.distributor-#{distributor_managed.id} a.remove-exchange").click page.find("tr.distributor-#{distributor_permitted.id} a.remove-exchange").click + click_button 'Save and Next' click_button 'Save and Back to List' expect(page).to have_input "oc#{oc.id}[name]", value: oc.name @@ -706,6 +715,14 @@ describe ' private + def expect_shipping_methods_to_be_checked_for(distributor) + distributor.distributor_shipping_method_ids.each do |distributor_shipping_method_id| + expect(page).to have_checked_field( + "order_cycle_selected_distributor_shipping_method_ids_#{distributor_shipping_method_id}" + ) + end + end + def wait_for_edit_form_to_load_order_cycle(order_cycle) expect(page).to have_field "order_cycle_name", with: order_cycle.name end