From 55f9afef77c7c7d4e018e4885fd90a423dd3f95e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:07:05 +1000 Subject: [PATCH 1/8] Regenerate Rubocop's TODO file --- .rubocop_todo.yml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 421215463d..44190b2d5a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -224,17 +224,6 @@ Rails/TransactionExitStatement: Exclude: - 'app/services/place_proxy_order.rb' -# Offense count: 5 -# Configuration parameters: Include. -# Include: app/models/**/*.rb -Rails/UniqueValidationWithoutIndex: - Exclude: - - 'app/models/customer.rb' - - 'app/models/exchange.rb' - - 'app/models/spree/stock_item.rb' - - 'app/models/spree/tax_category.rb' - - 'app/models/spree/zone.rb' - # Offense count: 23 # This cop supports unsafe autocorrection (--autocorrect-all). # Configuration parameters: EnforcedStyle. From 441844dd79fefa72a4a5a34b0b5f99a214ccf30e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:11:08 +1000 Subject: [PATCH 2/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 1 - .../admin/customers_controller_spec.rb | 416 +++++++++--------- 2 files changed, 207 insertions(+), 210 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 44190b2d5a..bb7acfc501 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,7 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/admin/customers_controller_spec.rb' - 'spec/controllers/admin/order_cycles_controller_spec.rb' - 'spec/controllers/api/v0/order_cycles_controller_spec.rb' - 'spec/controllers/api/v0/orders_controller_spec.rb' diff --git a/spec/controllers/admin/customers_controller_spec.rb b/spec/controllers/admin/customers_controller_spec.rb index 9d3265af9a..baac706167 100644 --- a/spec/controllers/admin/customers_controller_spec.rb +++ b/spec/controllers/admin/customers_controller_spec.rb @@ -2,260 +2,258 @@ require 'spec_helper' -module Admin - RSpec.describe CustomersController do - include AuthenticationHelper +RSpec.describe Admin::CustomersController do + include AuthenticationHelper - describe "index" do - let(:enterprise) { create(:distributor_enterprise) } - let(:another_enterprise) { create(:distributor_enterprise) } + describe "index" do + let(:enterprise) { create(:distributor_enterprise) } + let(:another_enterprise) { create(:distributor_enterprise) } - context "html" do + context "html" do + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + end + + it "returns an empty @collection" do + get :index, as: :html + expect(assigns(:collection)).to eq [] + end + end + + context "json" do + let!(:customer) { create(:customer, enterprise:, created_manually: true) } + + context "where I manage the enterprise" do before do allow(controller).to receive(:spree_current_user) { enterprise.owner } end - it "returns an empty @collection" do - get :index, as: :html - expect(assigns(:collection)).to eq [] - end - end + context "and enterprise_id is given in params" do + let(:user){ enterprise.users.first } + let(:customers){ Customer.visible.managed_by(user).where(enterprise_id: enterprise.id) } + let(:params) { { format: :json, enterprise_id: enterprise.id } } - context "json" do - let!(:customer) { create(:customer, enterprise:, created_manually: true) } - - context "where I manage the enterprise" do - before do - allow(controller).to receive(:spree_current_user) { enterprise.owner } + it "scopes @collection to customers of that enterprise" do + get(:index, params:) + expect(assigns(:collection)).to eq [customer] end - context "and enterprise_id is given in params" do - let(:user){ enterprise.users.first } - let(:customers){ Customer.visible.managed_by(user).where(enterprise_id: enterprise.id) } - let(:params) { { format: :json, enterprise_id: enterprise.id } } + it "serializes the data" do + expect(ActiveModel::ArraySerializer).to receive(:new) + get :index, params: + end - it "scopes @collection to customers of that enterprise" do + it 'calls CustomersWithBalanceQuery' do + customers_with_balance = instance_double(CustomersWithBalanceQuery) + allow(CustomersWithBalanceQuery) + .to receive(:new).with(customers) { customers_with_balance } + + expect(customers_with_balance).to receive(:call) { Customer.none } + + get :index, params: + end + + it 'serializes using CustomerWithBalanceSerializer' do + expect(Api::Admin::CustomerWithBalanceSerializer).to receive(:new) + + get :index, params: + end + + context 'when the customer has no orders' do + it 'includes the customer balance in the response' do get(:index, params:) - expect(assigns(:collection)).to eq [customer] - end - - it "serializes the data" do - expect(ActiveModel::ArraySerializer).to receive(:new) - get :index, params: - end - - it 'calls CustomersWithBalanceQuery' do - customers_with_balance = instance_double(CustomersWithBalanceQuery) - allow(CustomersWithBalanceQuery) - .to receive(:new).with(customers) { customers_with_balance } - - expect(customers_with_balance).to receive(:call) { Customer.none } - - get :index, params: - end - - it 'serializes using CustomerWithBalanceSerializer' do - expect(Api::Admin::CustomerWithBalanceSerializer).to receive(:new) - - get :index, params: - end - - context 'when the customer has no orders' do - it 'includes the customer balance in the response' do - get(:index, params:) - expect(json_response.first["balance"]).to eq("$0.00") - end - end - - context 'when the customer has complete orders' do - let(:order) { create(:order, customer:, state: 'complete') } - let!(:line_item) { create(:line_item, order:, price: 10.0) } - - it 'includes the customer balance in the response' do - order.update_order! - get(:index, params:) - expect(json_response.first["balance"]).to eq("$-10.00") - end - end - - context 'when the customer has canceled orders' do - let(:order) { create(:order, customer:) } - let!(:variant) { create(:variant, price: 10.0) } - - before do - order.contents.add(variant) - order.payments << create(:payment, :completed, order:, amount: order.total) - - order.update_attribute(:state, 'canceled') - end - - it 'includes the customer balance in the response' do - get(:index, params:) - expect(json_response.first["balance"]).to eq("$10.00") - end - end - - context 'when the customer has cart orders' do - let(:order) { create(:order, customer:, state: 'cart') } - let!(:line_item) { create(:line_item, order:, price: 10.0) } - - it 'includes the customer balance in the response' do - get(:index, params:) - expect(json_response.first["balance"]).to eq("$0.00") - end - end - - context 'when the customer has an order with a void payment' do - let(:order) { create(:order_with_totals, customer:, state: 'complete') } - let!(:payment) { create(:payment, order:, amount: order.total) } - - before do - allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true) - order.process_payments! - - payment.void_transaction! - end - - it 'includes the customer balance in the response' do - expect(order.payment_total).to eq(0) - get(:index, params:) - expect(json_response.first["balance"]).to eq('$-10.00') - end + expect(json_response.first["balance"]).to eq("$0.00") end end - context "and enterprise_id is not given in params" do - it "returns an empty collection" do - get :index, as: :json - expect(assigns(:collection)).to eq [] + context 'when the customer has complete orders' do + let(:order) { create(:order, customer:, state: 'complete') } + let!(:line_item) { create(:line_item, order:, price: 10.0) } + + it 'includes the customer balance in the response' do + order.update_order! + get(:index, params:) + expect(json_response.first["balance"]).to eq("$-10.00") + end + end + + context 'when the customer has canceled orders' do + let(:order) { create(:order, customer:) } + let!(:variant) { create(:variant, price: 10.0) } + + before do + order.contents.add(variant) + order.payments << create(:payment, :completed, order:, amount: order.total) + + order.update_attribute(:state, 'canceled') + end + + it 'includes the customer balance in the response' do + get(:index, params:) + expect(json_response.first["balance"]).to eq("$10.00") + end + end + + context 'when the customer has cart orders' do + let(:order) { create(:order, customer:, state: 'cart') } + let!(:line_item) { create(:line_item, order:, price: 10.0) } + + it 'includes the customer balance in the response' do + get(:index, params:) + expect(json_response.first["balance"]).to eq("$0.00") + end + end + + context 'when the customer has an order with a void payment' do + let(:order) { create(:order_with_totals, customer:, state: 'complete') } + let!(:payment) { create(:payment, order:, amount: order.total) } + + before do + allow_any_instance_of(Spree::Payment).to receive(:completed?).and_return(true) + order.process_payments! + + payment.void_transaction! + end + + it 'includes the customer balance in the response' do + expect(order.payment_total).to eq(0) + get(:index, params:) + expect(json_response.first["balance"]).to eq('$-10.00') end end end - context "and I do not manage the enterprise" do - before do - allow(controller).to receive(:spree_current_user) { another_enterprise.owner } - end - + context "and enterprise_id is not given in params" do it "returns an empty collection" do get :index, as: :json expect(assigns(:collection)).to eq [] end end end - end - describe "update" do - let(:enterprise) { create(:distributor_enterprise) } - let(:another_enterprise) { create(:distributor_enterprise) } - - context "json" do - let!(:customer) { create(:customer, enterprise:) } - - context "where I manage the customer's enterprise" do - render_views - - before do - allow(controller).to receive(:spree_current_user) { enterprise.owner } - end - - it "allows me to update the customer" do - spree_put :update, format: :json, id: customer.id, - customer: { email: 'new.email@gmail.com' } - expect(response.parsed_body["id"]).to eq customer.id - expect(assigns(:customer)).to eq customer - expect(customer.reload.email).to eq 'new.email@gmail.com' - end + context "and I do not manage the enterprise" do + before do + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end - context "where I don't manage the customer's enterprise" do - before do - allow(controller).to receive(:spree_current_user) { another_enterprise.owner } - end - - it "prevents me from updating the customer" do - spree_put :update, format: :json, id: customer.id, - customer: { email: 'new.email@gmail.com' } - expect(response).to redirect_to unauthorized_path - expect(assigns(:customer)).to eq nil - expect(customer.email).not_to eq 'new.email@gmail.com' - end + it "returns an empty collection" do + get :index, as: :json + expect(assigns(:collection)).to eq [] end end end + end - describe "create" do - let(:enterprise) { create(:distributor_enterprise) } - let(:another_enterprise) { create(:distributor_enterprise) } + describe "update" do + let(:enterprise) { create(:distributor_enterprise) } + let(:another_enterprise) { create(:distributor_enterprise) } - def create_customer(enterprise) - spree_put :create, format: :json, - customer: { email: 'new@example.com', enterprise_id: enterprise.id } + context "json" do + let!(:customer) { create(:customer, enterprise:) } + + context "where I manage the customer's enterprise" do + render_views + + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + end + + it "allows me to update the customer" do + spree_put :update, format: :json, id: customer.id, + customer: { email: 'new.email@gmail.com' } + expect(response.parsed_body["id"]).to eq customer.id + expect(assigns(:customer)).to eq customer + expect(customer.reload.email).to eq 'new.email@gmail.com' + end end - context "json" do - context "where I manage the customer's enterprise" do - before do - allow(controller).to receive(:spree_current_user) { enterprise.owner } - end - - it "allows me to create the customer" do - expect { create_customer enterprise }.to change { Customer.count }.by(1) - expect(Customer.reorder(:id).last.created_manually).to eq true - end + context "where I don't manage the customer's enterprise" do + before do + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } end - context "where I don't manage the customer's enterprise" do - before do - allow(controller).to receive(:spree_current_user) { another_enterprise.owner } - end - - it "prevents me from creating the customer" do - expect { create_customer enterprise }.to change { Customer.count }.by(0) - end - end - - context "where I am the admin user" do - before do - allow(controller).to receive(:spree_current_user) { create(:admin_user) } - end - - it "allows admins to create the customer" do - expect { create_customer enterprise }.to change { Customer.count }.by(1) - end + it "prevents me from updating the customer" do + spree_put :update, format: :json, id: customer.id, + customer: { email: 'new.email@gmail.com' } + expect(response).to redirect_to unauthorized_path + expect(assigns(:customer)).to eq nil + expect(customer.email).not_to eq 'new.email@gmail.com' end end end + end - describe "show" do - let(:enterprise) { create(:distributor_enterprise) } - let(:another_enterprise) { create(:distributor_enterprise) } + describe "create" do + let(:enterprise) { create(:distributor_enterprise) } + let(:another_enterprise) { create(:distributor_enterprise) } - context "json" do - let!(:customer) { create(:customer, enterprise:) } + def create_customer(enterprise) + spree_put :create, format: :json, + customer: { email: 'new@example.com', enterprise_id: enterprise.id } + end - context "where I manage the customer's enterprise" do - render_views - - before do - allow(controller).to receive(:spree_current_user) { enterprise.owner } - end - - it "renders the customer as json" do - get :show, as: :json, params: { id: customer.id } - expect(response.parsed_body["id"]).to eq customer.id - end + context "json" do + context "where I manage the customer's enterprise" do + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } end - context "where I don't manage the customer's enterprise" do - before do - allow(controller).to receive(:spree_current_user) { another_enterprise.owner } - end + it "allows me to create the customer" do + expect { create_customer enterprise }.to change { Customer.count }.by(1) + expect(Customer.reorder(:id).last.created_manually).to eq true + end + end - it "prevents me from updating the customer" do - get :show, as: :json, params: { id: customer.id } - expect(response).to redirect_to unauthorized_path - end + context "where I don't manage the customer's enterprise" do + before do + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } + end + + it "prevents me from creating the customer" do + expect { create_customer enterprise }.to change { Customer.count }.by(0) + end + end + + context "where I am the admin user" do + before do + allow(controller).to receive(:spree_current_user) { create(:admin_user) } + end + + it "allows admins to create the customer" do + expect { create_customer enterprise }.to change { Customer.count }.by(1) + end + end + end + end + + describe "show" do + let(:enterprise) { create(:distributor_enterprise) } + let(:another_enterprise) { create(:distributor_enterprise) } + + context "json" do + let!(:customer) { create(:customer, enterprise:) } + + context "where I manage the customer's enterprise" do + render_views + + before do + allow(controller).to receive(:spree_current_user) { enterprise.owner } + end + + it "renders the customer as json" do + get :show, as: :json, params: { id: customer.id } + expect(response.parsed_body["id"]).to eq customer.id + end + end + + context "where I don't manage the customer's enterprise" do + before do + allow(controller).to receive(:spree_current_user) { another_enterprise.owner } + end + + it "prevents me from updating the customer" do + get :show, as: :json, params: { id: customer.id } + expect(response).to redirect_to unauthorized_path end end end From 8e1fb76327d8ba7889d7ed283df0ce75be5138ae Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:13:11 +1000 Subject: [PATCH 3/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 1 - .../admin/order_cycles_controller_spec.rb | 988 +++++++++--------- 2 files changed, 493 insertions(+), 496 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index bb7acfc501..32c3f25017 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,7 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/admin/order_cycles_controller_spec.rb' - 'spec/controllers/api/v0/order_cycles_controller_spec.rb' - 'spec/controllers/api/v0/orders_controller_spec.rb' - 'spec/controllers/spree/admin/adjustments_controller_spec.rb' diff --git a/spec/controllers/admin/order_cycles_controller_spec.rb b/spec/controllers/admin/order_cycles_controller_spec.rb index bd0c911011..fc69bfa60b 100644 --- a/spec/controllers/admin/order_cycles_controller_spec.rb +++ b/spec/controllers/admin/order_cycles_controller_spec.rb @@ -2,518 +2,516 @@ require 'spec_helper' -module Admin - RSpec.describe OrderCyclesController do - let!(:distributor_owner) { create(:user) } - let(:datetime_confirmation_attrs) { - { confirm_datetime_change: nil, - error_class: Admin::OrderCyclesController::DateTimeChangeError } - } +RSpec.describe Admin::OrderCyclesController do + let!(:distributor_owner) { create(:user) } + let(:datetime_confirmation_attrs) { + { confirm_datetime_change: nil, + error_class: Admin::OrderCyclesController::DateTimeChangeError } + } - before do - allow(controller).to receive_messages spree_current_user: distributor_owner - end + before do + allow(controller).to receive_messages spree_current_user: distributor_owner + end - describe "#index" do - describe "when the user manages a coordinator" do - let!(:coordinator) { create(:distributor_enterprise, owner: distributor_owner) } - let!(:oc1) { - create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 60.days.ago ) - } - let!(:oc2) { - create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 40.days.ago ) - } - let!(:oc3) { - create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 20.days.ago ) - } - let!(:oc4) { - create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: nil ) - } - - context "html" do - it "doesn't load any data" do - get :index, as: :html - expect(assigns(:collection)).to be_empty - end - end - - context "json" do - context "where ransack conditions are specified" do - it "loads order cycles closed within past month, and orders w/o a close_at date" do - get :index, as: :json - expect(assigns(:collection)).not_to include oc1, oc2 - expect(assigns(:collection)).to include oc3, oc4 - end - end - - context "where q[orders_close_at_gt] is set" do - let(:q) { { orders_close_at_gt: 45.days.ago } } - - it "loads order cycles closed after specified date, and orders w/o a close_at date" do - get :index, as: :json, params: { q: } - expect(assigns(:collection)).not_to include oc1 - expect(assigns(:collection)).to include oc2, oc3, oc4 - end - - context "and other conditions are specified" do - before { q.merge!(id_not_in: [oc2.id, oc4.id]) } - - it "loads order cycles that meet all conditions" do - get :index, format: :json, params: { q: } - expect(assigns(:collection)).not_to include oc1, oc2, oc4 - expect(assigns(:collection)).to include oc3 - end - end - end - end - end - end - - describe "new" do - describe "when the user manages a single distributor enterprise suitable for coordinator" do - let!(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } - - it "renders the new template" do - get :new - expect(response).to render_template :new - end - end - - describe "when a user manages multiple enterprises suitable for coordinator" do - let!(:distributor1) { create(:distributor_enterprise, owner: distributor_owner) } - let!(:distributor2) { create(:distributor_enterprise, owner: distributor_owner) } - let!(:distributor3) { create(:distributor_enterprise) } - - it "renders the set_coordinator template" do - get :new - expect(response).to render_template :set_coordinator - end - - describe "and a coordinator_id is submitted as part of the request" do - describe "when the user manages the enterprise" do - it "renders the new template" do - get :new, params: { coordinator_id: distributor1.id } - expect(response).to render_template :new - end - end - - describe "when the user does not manage the enterprise" do - it "renders the set_coordinator template and sets a flash error" do - get :new, params: { coordinator_id: distributor3.id } - expect(response).to render_template :set_coordinator - expect(flash[:error]) - .to eq "You don't have permission to create an order cycle " \ - "coordinated by that enterprise" - end - end - end - end - end - - describe "show" do - context 'a distributor manages an order cycle' do - let(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } - let(:oc) { create(:simple_order_cycle, coordinator: distributor) } - - context "distributor navigates to order cycle show page" do - it 'redirects to edit page' do - get :show, params: { id: oc.id } - expect(response).to redirect_to edit_admin_order_cycle_path(oc.id) - end - end - end - - describe "queries" do - context "as manager, when order cycle has multiple exchanges" do - let!(:distributor) { create(:distributor_enterprise) } - let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor) } - before do - order_cycle.exchanges.create! sender: distributor, receiver: distributor, - incoming: true, receival_instructions: 'A', tag_list: "A" - order_cycle.exchanges.create! sender: distributor, receiver: distributor, - incoming: false, pickup_instructions: 'B', tag_list: "B" - controller_login_as_enterprise_user([distributor]) - end - - it do - expect { - get :show, params: { id: order_cycle.id }, as: :json - }.to query_database( - { - select: { - enterprise_fees: 3, - enterprise_groups: 1, - enterprises: 22, - exchanges: 7, - order_cycles: 6, - proxy_orders: 1, - schedules: 1, - spree_variants: 8, - tags: 1 - }, - update: { spree_users: 1 } - } - ) - end - end - end - end - - describe "create" do - let(:shop) { create(:distributor_enterprise) } - - context "as a manager of a shop" do - let(:form_mock) { instance_double(OrderCycles::FormService) } - let(:params) { { as: :json, order_cycle: {} } } - - before do - controller_login_as_enterprise_user([shop]) - allow(OrderCycles::FormService).to receive(:new) { form_mock } - end - - context "when creation is successful" do - before { allow(form_mock).to receive(:save) { true } } - - # mock build_resource so that we can control the edit_path - OrderCyclesController.class_eval do - def build_resource - order_cycle = OrderCycle.new - order_cycle.id = 1 - order_cycle - end - end - - it "returns success: true and a valid edit path" do - spree_post :create, params - json_response = response.parsed_body - expect(json_response['success']).to be true - expect(json_response['edit_path']).to eq "/admin/order_cycles/1/incoming" - end - end - - context "when an error occurs" do - before { allow(form_mock).to receive(:save) { false } } - - it "returns an errors hash" do - spree_post :create, params - json_response = response.parsed_body - expect(json_response['errors']).to be - end - end - end - end - - describe "update" do - let(:order_cycle) { create(:simple_order_cycle) } - let(:coordinator) { order_cycle.coordinator } - let(:form_mock) { instance_double(OrderCycles::FormService) } - - before do - allow(OrderCycles::FormService).to receive(:new) { form_mock } - end - - context "as a manager of the coordinator" do - before { controller_login_as_enterprise_user([coordinator]) } - let(:params) { { format: :json, id: order_cycle.id, order_cycle: {} } } - - context "when order cycle has subscriptions" do - let(:coordinator) { order_cycle.coordinator } - let(:producer) { create(:supplier_enterprise) } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - let(:v) { create(:variant) } - let!(:incoming_exchange) { - create(:exchange, order_cycle:, sender: producer, receiver: coordinator, - incoming: true, variants: [v]) - } - let!(:outgoing_exchange) { - create(:exchange, order_cycle:, sender: coordinator, receiver: coordinator, - incoming: false, variants: [v]) - } - let!(:subscription) { create(:subscription, shop: coordinator, schedule:) } - let!(:subscription_line_item) { - create(:subscription_line_item, subscription:, variant: v) - } - - before do - allow(form_mock).to receive(:save) { true } - v.destroy - end - - it "can update order cycle even if the variant has been deleted" do - spree_put :update, { format: :json, id: order_cycle.id, order_cycle: {} } - expect(response).to have_http_status :ok - end - end - - context "when updating succeeds" do - before { allow(form_mock).to receive(:save) { true } } - - context "when the page is reloading" do - before { params[:reloading] = '1' } - - it "sets flash message" do - spree_put :update, params - expect(flash[:success]).to eq('Your order cycle has been updated.') - end - end - - context "when the page is not reloading" do - it "does not set flash message" do - spree_put :update, params - expect(flash[:success]).to be nil - end - end - end - - context "when a validation error occurs" do - before { allow(form_mock).to receive(:save) { false } } - - it "returns an error message" do - spree_put :update, params - - json_response = response.parsed_body - expect(json_response['errors']).to be - end - end - - it "can update preference product_selection_from_coordinator_inventory_only" do - expect(OrderCycles::FormService).to receive(:new). - with(order_cycle, - { "preferred_product_selection_from_coordinator_inventory_only" => true, - **datetime_confirmation_attrs }, - anything) { form_mock } - allow(form_mock).to receive(:save) { true } - - spree_put :update, params. - merge(order_cycle: { - preferred_product_selection_from_coordinator_inventory_only: true - }) - end - - it "can update preference automatic_notifications" do - expect(OrderCycles::FormService).to receive(:new). - with(order_cycle, - { "automatic_notifications" => true, **datetime_confirmation_attrs }, - anything) { form_mock } - allow(form_mock).to receive(:save) { true } - - spree_put :update, params. - merge(order_cycle: { automatic_notifications: true }) - end - end - end - - describe "limiting update scope" do - let(:order_cycle) { create(:simple_order_cycle) } - let(:producer) { create(:supplier_enterprise) } - let(:coordinator) { order_cycle.coordinator } - let(:hub) { create(:distributor_enterprise) } - let(:v) { create(:variant) } - let!(:incoming_exchange) { - create(:exchange, order_cycle:, sender: producer, receiver: coordinator, - incoming: true, variants: [v]) + describe "#index" do + describe "when the user manages a coordinator" do + let!(:coordinator) { create(:distributor_enterprise, owner: distributor_owner) } + let!(:oc1) { + create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 60.days.ago ) } - let!(:outgoing_exchange) { - create(:exchange, order_cycle:, sender: coordinator, receiver: hub, - incoming: false, variants: [v]) + let!(:oc2) { + create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 40.days.ago ) + } + let!(:oc3) { + create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: 20.days.ago ) + } + let!(:oc4) { + create(:simple_order_cycle, orders_open_at: 70.days.ago, orders_close_at: nil ) } - let(:allowed) { { incoming_exchanges: [], outgoing_exchanges: [] } } - let(:restricted) { - { name: 'some name', orders_open_at: 1.day.from_now.to_s, orders_close_at: 1.day.ago.to_s } - } - let(:params) { - { - format: :json, id: order_cycle.id, order_cycle: allowed.merge(restricted) - } - } - let(:form_mock) { - instance_double(OrderCycles::FormService, save: true) - } - - before { allow(controller).to receive(:spree_current_user) { user } } - - context "as a manager of the coordinator" do - let(:user) { coordinator.owner } - let(:expected) { - [order_cycle, allowed.merge(restricted).merge(datetime_confirmation_attrs), user] - } - - it "allows me to update exchange information for exchanges, name and dates" do - expect(OrderCycles::FormService).to receive(:new).with(*expected) { form_mock } - spree_put :update, params + context "html" do + it "doesn't load any data" do + get :index, as: :html + expect(assigns(:collection)).to be_empty end end - context "as a producer supplying to an order cycle" do - let(:user) { producer.owner } - let(:expected) { [order_cycle, allowed.merge(datetime_confirmation_attrs), user] } - - it "allows me to update exchange information for exchanges, but not name or dates" do - expect(OrderCycles::FormService).to receive(:new).with(*expected) { form_mock } - spree_put :update, params - end - end - end - - describe "bulk_update" do - let(:oc) { create(:simple_order_cycle) } - let!(:coordinator) { oc.coordinator } - - context "when I manage the coordinator of an order cycle" do - let(:params) do - { format: :json, order_cycle_set: { collection_attributes: { '0' => { - id: oc.id, - name: "Updated Order Cycle", - orders_open_at: Date.current - 21.days, - orders_close_at: Date.current + 21.days, - } } } } - end - - before { create(:enterprise_role, user: distributor_owner, enterprise: coordinator) } - - it "updates order cycle properties" do - spree_put :bulk_update, params - oc.reload - expect(oc.name).to eq "Updated Order Cycle" - expect(oc.orders_open_at.to_date).to eq Date.current - 21.days - expect(oc.orders_close_at.to_date).to eq Date.current + 21.days - end - - it "does nothing when no data is supplied" do - expect do - spree_put :bulk_update, format: :json - end.to change { oc.orders_open_at }.by(0) - json_response = response.parsed_body - expect(json_response['errors']) - .to eq 'Hm, something went wrong. No order cycle data found.' - end - - context "when a validation error occurs" do - let(:params) do - { format: :json, order_cycle_set: { collection_attributes: { '0' => { - id: oc.id, - name: "Updated Order Cycle", - orders_open_at: Date.current + 25.days, - orders_close_at: Date.current + 21.days, - } } } } - end - - it "returns an error message" do - spree_put :bulk_update, params - json_response = response.parsed_body - expect(json_response['errors']).to be_present + context "json" do + context "where ransack conditions are specified" do + it "loads order cycles closed within past month, and orders w/o a close_at date" do + get :index, as: :json + expect(assigns(:collection)).not_to include oc1, oc2 + expect(assigns(:collection)).to include oc3, oc4 end end - end - context "when I do not manage the coordinator of an order cycle" do - # I need to manage a hub in order to access the bulk_update action - let!(:another_distributor) { create(:distributor_enterprise, users: [distributor_owner]) } + context "where q[orders_close_at_gt] is set" do + let(:q) { { orders_close_at_gt: 45.days.ago } } - it "doesn't update order cycle properties" do - spree_put :bulk_update, - format: :json, - order_cycle_set: { collection_attributes: { '0' => { - id: oc.id, - name: "Updated Order Cycle", - orders_open_at: Date.current - 21.days, - orders_close_at: Date.current + 21.days, - } } } + it "loads order cycles closed after specified date, and orders w/o a close_at date" do + get :index, as: :json, params: { q: } + expect(assigns(:collection)).not_to include oc1 + expect(assigns(:collection)).to include oc2, oc3, oc4 + end - oc.reload - expect(oc.name).not_to eq "Updated Order Cycle" - expect(oc.orders_open_at.to_date).not_to eq Date.current - 21.days - expect(oc.orders_close_at.to_date).not_to eq Date.current + 21.days - end - end - end + context "and other conditions are specified" do + before { q.merge!(id_not_in: [oc2.id, oc4.id]) } - describe "notifying producers" do - let(:user) { create(:user) } - let(:admin_user) { create(:admin_user) } - let(:order_cycle) { create(:simple_order_cycle) } - - before do - allow(controller).to receive_messages spree_current_user: admin_user - end - - it "enqueues a job" do - expect do - spree_post :notify_producers, id: order_cycle.id - end.to enqueue_job OrderCycleNotificationJob - end - - it "redirects back to the order cycles path with a success message" do - spree_post :notify_producers, id: order_cycle.id - expect(response).to redirect_to admin_order_cycles_path - expect(flash[:success]).to eq( - 'Emails to be sent to producers have been queued for sending.' - ) - end - end - - describe "destroy" do - let(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } - let(:oc) { create(:simple_order_cycle, coordinator: distributor) } - - describe "when an order cycle is deleteable" do - it "allows the order_cycle to be destroyed" do - get :destroy, params: { id: oc.id } - expect(OrderCycle.find_by(id: oc.id)).to be nil - end - end - - describe "when an order cycle becomes non-deletable due to the presence of an order" do - let!(:order) { create(:order, order_cycle: oc) } - - it "displays an error message when we attempt to delete it" do - get :destroy, params: { id: oc.id } - expect(response).to redirect_to admin_order_cycles_path - expect(flash[:error]) - .to eq 'That order cycle has been selected by a customer and cannot be deleted. ' \ - 'To prevent customers from accessing it, please close it instead.' - end - end - - describe "when an order cycle becomes non-deletable because it is linked to a schedule" do - let!(:schedule) { create(:schedule, order_cycles: [oc]) } - - it "displays an error message when we attempt to delete it" do - get :destroy, params: { id: oc.id } - expect(response).to redirect_to admin_order_cycles_path - expect(flash[:error]) - .to eq 'That order cycle is linked to a schedule and cannot be deleted. ' \ - 'Please unlink or delete the schedule first.' - end - end - - describe "when an order cycle has any coordinator_fees" do - let(:enterprise_fee1) { create(:enterprise_fee) } - - before do - oc.coordinator_fees << enterprise_fee1 - end - - it "actually delete the order cycle" do - get :destroy, params: { id: oc.id } - expect(OrderCycle.find_by(id: oc.id)).to be nil - expect(response).to redirect_to admin_order_cycles_path - end - - describe "when the order_cycle was previously cloned" do - let(:cloned) { oc.clone! } - - it "actually delete the order cycle" do - get :destroy, params: { id: cloned.id } - - expect(OrderCycle.find_by(id: cloned.id)).to be nil - expect(OrderCycle.find_by(id: oc.id)).not_to be nil - expect(EnterpriseFee.find_by(id: enterprise_fee1.id)).not_to be nil - expect(response).to redirect_to admin_order_cycles_path + it "loads order cycles that meet all conditions" do + get :index, format: :json, params: { q: } + expect(assigns(:collection)).not_to include oc1, oc2, oc4 + expect(assigns(:collection)).to include oc3 + end end end end end end + + describe "new" do + describe "when the user manages a single distributor enterprise suitable for coordinator" do + let!(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } + + it "renders the new template" do + get :new + expect(response).to render_template :new + end + end + + describe "when a user manages multiple enterprises suitable for coordinator" do + let!(:distributor1) { create(:distributor_enterprise, owner: distributor_owner) } + let!(:distributor2) { create(:distributor_enterprise, owner: distributor_owner) } + let!(:distributor3) { create(:distributor_enterprise) } + + it "renders the set_coordinator template" do + get :new + expect(response).to render_template :set_coordinator + end + + describe "and a coordinator_id is submitted as part of the request" do + describe "when the user manages the enterprise" do + it "renders the new template" do + get :new, params: { coordinator_id: distributor1.id } + expect(response).to render_template :new + end + end + + describe "when the user does not manage the enterprise" do + it "renders the set_coordinator template and sets a flash error" do + get :new, params: { coordinator_id: distributor3.id } + expect(response).to render_template :set_coordinator + expect(flash[:error]) + .to eq "You don't have permission to create an order cycle " \ + "coordinated by that enterprise" + end + end + end + end + end + + describe "show" do + context 'a distributor manages an order cycle' do + let(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } + let(:oc) { create(:simple_order_cycle, coordinator: distributor) } + + context "distributor navigates to order cycle show page" do + it 'redirects to edit page' do + get :show, params: { id: oc.id } + expect(response).to redirect_to edit_admin_order_cycle_path(oc.id) + end + end + end + + describe "queries" do + context "as manager, when order cycle has multiple exchanges" do + let!(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: distributor) } + before do + order_cycle.exchanges.create! sender: distributor, receiver: distributor, + incoming: true, receival_instructions: 'A', tag_list: "A" + order_cycle.exchanges.create! sender: distributor, receiver: distributor, + incoming: false, pickup_instructions: 'B', tag_list: "B" + controller_login_as_enterprise_user([distributor]) + end + + it do + expect { + get :show, params: { id: order_cycle.id }, as: :json + }.to query_database( + { + select: { + enterprise_fees: 3, + enterprise_groups: 1, + enterprises: 22, + exchanges: 7, + order_cycles: 6, + proxy_orders: 1, + schedules: 1, + spree_variants: 8, + tags: 1 + }, + update: { spree_users: 1 } + } + ) + end + end + end + end + + describe "create" do + let(:shop) { create(:distributor_enterprise) } + + context "as a manager of a shop" do + let(:form_mock) { instance_double(OrderCycles::FormService) } + let(:params) { { as: :json, order_cycle: {} } } + + before do + controller_login_as_enterprise_user([shop]) + allow(OrderCycles::FormService).to receive(:new) { form_mock } + end + + context "when creation is successful" do + before { allow(form_mock).to receive(:save) { true } } + + # mock build_resource so that we can control the edit_path + Admin::OrderCyclesController.class_eval do + def build_resource + order_cycle = OrderCycle.new + order_cycle.id = 1 + order_cycle + end + end + + it "returns success: true and a valid edit path" do + spree_post :create, params + json_response = response.parsed_body + expect(json_response['success']).to be true + expect(json_response['edit_path']).to eq "/admin/order_cycles/1/incoming" + end + end + + context "when an error occurs" do + before { allow(form_mock).to receive(:save) { false } } + + it "returns an errors hash" do + spree_post :create, params + json_response = response.parsed_body + expect(json_response['errors']).to be + end + end + end + end + + describe "update" do + let(:order_cycle) { create(:simple_order_cycle) } + let(:coordinator) { order_cycle.coordinator } + let(:form_mock) { instance_double(OrderCycles::FormService) } + + before do + allow(OrderCycles::FormService).to receive(:new) { form_mock } + end + + context "as a manager of the coordinator" do + before { controller_login_as_enterprise_user([coordinator]) } + let(:params) { { format: :json, id: order_cycle.id, order_cycle: {} } } + + context "when order cycle has subscriptions" do + let(:coordinator) { order_cycle.coordinator } + let(:producer) { create(:supplier_enterprise) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let(:v) { create(:variant) } + let!(:incoming_exchange) { + create(:exchange, order_cycle:, sender: producer, receiver: coordinator, + incoming: true, variants: [v]) + } + let!(:outgoing_exchange) { + create(:exchange, order_cycle:, sender: coordinator, receiver: coordinator, + incoming: false, variants: [v]) + } + let!(:subscription) { create(:subscription, shop: coordinator, schedule:) } + let!(:subscription_line_item) { + create(:subscription_line_item, subscription:, variant: v) + } + + before do + allow(form_mock).to receive(:save) { true } + v.destroy + end + + it "can update order cycle even if the variant has been deleted" do + spree_put :update, { format: :json, id: order_cycle.id, order_cycle: {} } + expect(response).to have_http_status :ok + end + end + + context "when updating succeeds" do + before { allow(form_mock).to receive(:save) { true } } + + context "when the page is reloading" do + before { params[:reloading] = '1' } + + it "sets flash message" do + spree_put :update, params + expect(flash[:success]).to eq('Your order cycle has been updated.') + end + end + + context "when the page is not reloading" do + it "does not set flash message" do + spree_put :update, params + expect(flash[:success]).to be nil + end + end + end + + context "when a validation error occurs" do + before { allow(form_mock).to receive(:save) { false } } + + it "returns an error message" do + spree_put :update, params + + json_response = response.parsed_body + expect(json_response['errors']).to be + end + end + + it "can update preference product_selection_from_coordinator_inventory_only" do + expect(OrderCycles::FormService).to receive(:new). + with(order_cycle, + { "preferred_product_selection_from_coordinator_inventory_only" => true, + **datetime_confirmation_attrs }, + anything) { form_mock } + allow(form_mock).to receive(:save) { true } + + spree_put :update, params. + merge(order_cycle: { + preferred_product_selection_from_coordinator_inventory_only: true + }) + end + + it "can update preference automatic_notifications" do + expect(OrderCycles::FormService).to receive(:new). + with(order_cycle, + { "automatic_notifications" => true, **datetime_confirmation_attrs }, + anything) { form_mock } + allow(form_mock).to receive(:save) { true } + + spree_put :update, params. + merge(order_cycle: { automatic_notifications: true }) + end + end + end + + describe "limiting update scope" do + let(:order_cycle) { create(:simple_order_cycle) } + let(:producer) { create(:supplier_enterprise) } + let(:coordinator) { order_cycle.coordinator } + let(:hub) { create(:distributor_enterprise) } + let(:v) { create(:variant) } + let!(:incoming_exchange) { + create(:exchange, order_cycle:, sender: producer, receiver: coordinator, + incoming: true, variants: [v]) + } + let!(:outgoing_exchange) { + create(:exchange, order_cycle:, sender: coordinator, receiver: hub, + incoming: false, variants: [v]) + } + + let(:allowed) { { incoming_exchanges: [], outgoing_exchanges: [] } } + let(:restricted) { + { name: 'some name', orders_open_at: 1.day.from_now.to_s, orders_close_at: 1.day.ago.to_s } + } + let(:params) { + { + format: :json, id: order_cycle.id, order_cycle: allowed.merge(restricted) + } + } + let(:form_mock) { + instance_double(OrderCycles::FormService, save: true) + } + + before { allow(controller).to receive(:spree_current_user) { user } } + + context "as a manager of the coordinator" do + let(:user) { coordinator.owner } + let(:expected) { + [order_cycle, allowed.merge(restricted).merge(datetime_confirmation_attrs), user] + } + + it "allows me to update exchange information for exchanges, name and dates" do + expect(OrderCycles::FormService).to receive(:new).with(*expected) { form_mock } + spree_put :update, params + end + end + + context "as a producer supplying to an order cycle" do + let(:user) { producer.owner } + let(:expected) { [order_cycle, allowed.merge(datetime_confirmation_attrs), user] } + + it "allows me to update exchange information for exchanges, but not name or dates" do + expect(OrderCycles::FormService).to receive(:new).with(*expected) { form_mock } + spree_put :update, params + end + end + end + + describe "bulk_update" do + let(:oc) { create(:simple_order_cycle) } + let!(:coordinator) { oc.coordinator } + + context "when I manage the coordinator of an order cycle" do + let(:params) do + { format: :json, order_cycle_set: { collection_attributes: { '0' => { + id: oc.id, + name: "Updated Order Cycle", + orders_open_at: Date.current - 21.days, + orders_close_at: Date.current + 21.days, + } } } } + end + + before { create(:enterprise_role, user: distributor_owner, enterprise: coordinator) } + + it "updates order cycle properties" do + spree_put :bulk_update, params + oc.reload + expect(oc.name).to eq "Updated Order Cycle" + expect(oc.orders_open_at.to_date).to eq Date.current - 21.days + expect(oc.orders_close_at.to_date).to eq Date.current + 21.days + end + + it "does nothing when no data is supplied" do + expect do + spree_put :bulk_update, format: :json + end.to change { oc.orders_open_at }.by(0) + json_response = response.parsed_body + expect(json_response['errors']) + .to eq 'Hm, something went wrong. No order cycle data found.' + end + + context "when a validation error occurs" do + let(:params) do + { format: :json, order_cycle_set: { collection_attributes: { '0' => { + id: oc.id, + name: "Updated Order Cycle", + orders_open_at: Date.current + 25.days, + orders_close_at: Date.current + 21.days, + } } } } + end + + it "returns an error message" do + spree_put :bulk_update, params + json_response = response.parsed_body + expect(json_response['errors']).to be_present + end + end + end + + context "when I do not manage the coordinator of an order cycle" do + # I need to manage a hub in order to access the bulk_update action + let!(:another_distributor) { create(:distributor_enterprise, users: [distributor_owner]) } + + it "doesn't update order cycle properties" do + spree_put :bulk_update, + format: :json, + order_cycle_set: { collection_attributes: { '0' => { + id: oc.id, + name: "Updated Order Cycle", + orders_open_at: Date.current - 21.days, + orders_close_at: Date.current + 21.days, + } } } + + oc.reload + expect(oc.name).not_to eq "Updated Order Cycle" + expect(oc.orders_open_at.to_date).not_to eq Date.current - 21.days + expect(oc.orders_close_at.to_date).not_to eq Date.current + 21.days + end + end + end + + describe "notifying producers" do + let(:user) { create(:user) } + let(:admin_user) { create(:admin_user) } + let(:order_cycle) { create(:simple_order_cycle) } + + before do + allow(controller).to receive_messages spree_current_user: admin_user + end + + it "enqueues a job" do + expect do + spree_post :notify_producers, id: order_cycle.id + end.to enqueue_job OrderCycleNotificationJob + end + + it "redirects back to the order cycles path with a success message" do + spree_post :notify_producers, id: order_cycle.id + expect(response).to redirect_to admin_order_cycles_path + expect(flash[:success]).to eq( + 'Emails to be sent to producers have been queued for sending.' + ) + end + end + + describe "destroy" do + let(:distributor) { create(:distributor_enterprise, owner: distributor_owner) } + let(:oc) { create(:simple_order_cycle, coordinator: distributor) } + + describe "when an order cycle is deleteable" do + it "allows the order_cycle to be destroyed" do + get :destroy, params: { id: oc.id } + expect(OrderCycle.find_by(id: oc.id)).to be nil + end + end + + describe "when an order cycle becomes non-deletable due to the presence of an order" do + let!(:order) { create(:order, order_cycle: oc) } + + it "displays an error message when we attempt to delete it" do + get :destroy, params: { id: oc.id } + expect(response).to redirect_to admin_order_cycles_path + expect(flash[:error]) + .to eq 'That order cycle has been selected by a customer and cannot be deleted. ' \ + 'To prevent customers from accessing it, please close it instead.' + end + end + + describe "when an order cycle becomes non-deletable because it is linked to a schedule" do + let!(:schedule) { create(:schedule, order_cycles: [oc]) } + + it "displays an error message when we attempt to delete it" do + get :destroy, params: { id: oc.id } + expect(response).to redirect_to admin_order_cycles_path + expect(flash[:error]) + .to eq 'That order cycle is linked to a schedule and cannot be deleted. ' \ + 'Please unlink or delete the schedule first.' + end + end + + describe "when an order cycle has any coordinator_fees" do + let(:enterprise_fee1) { create(:enterprise_fee) } + + before do + oc.coordinator_fees << enterprise_fee1 + end + + it "actually delete the order cycle" do + get :destroy, params: { id: oc.id } + expect(OrderCycle.find_by(id: oc.id)).to be nil + expect(response).to redirect_to admin_order_cycles_path + end + + describe "when the order_cycle was previously cloned" do + let(:cloned) { oc.clone! } + + it "actually delete the order cycle" do + get :destroy, params: { id: cloned.id } + + expect(OrderCycle.find_by(id: cloned.id)).to be nil + expect(OrderCycle.find_by(id: oc.id)).not_to be nil + expect(EnterpriseFee.find_by(id: enterprise_fee1.id)).not_to be nil + expect(response).to redirect_to admin_order_cycles_path + end + end + end + end end From 035e67c33f36a6acb4e1ce3ed2f94780e4cbe4f0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:43:47 +1000 Subject: [PATCH 4/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .../api/v0/order_cycles_controller_spec.rb | 564 +++++++++--------- 1 file changed, 281 insertions(+), 283 deletions(-) diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index b42c8d28e8..4082232282 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -2,313 +2,311 @@ require "spec_helper" -module Api - RSpec.describe V0::OrderCyclesController do - let!(:distributor) { create(:distributor_enterprise) } - let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } - let!(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } - let!(:taxon1) { create(:taxon, name: 'Meat') } - let!(:taxon2) { create(:taxon, name: 'Vegetables') } - let!(:taxon3) { create(:taxon, name: 'Cake') } - let!(:property1) { create(:property, presentation: 'Organic') } - let!(:property2) { create(:property, presentation: 'Dairy-Free') } - let!(:property3) { create(:property, presentation: 'May Contain Nuts') } - let!(:product1) { - create(:product, name: "Kangaroo", primary_taxon: taxon1, properties: [property1]) - } - let!(:product2) { - create(:product, name: "Parsnips", primary_taxon: taxon2, properties: [property2]) - } - let!(:product3) { create(:product, primary_taxon: taxon2) } - let!(:product4) { create(:product, primary_taxon: taxon3, properties: [property3]) } - let!(:user) { create(:user) } - let(:customer) { create(:customer, user:, enterprise: distributor) } +RSpec.describe Api::V0::OrderCyclesController do + let!(:distributor) { create(:distributor_enterprise) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } + let!(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } + let!(:taxon1) { create(:taxon, name: 'Meat') } + let!(:taxon2) { create(:taxon, name: 'Vegetables') } + let!(:taxon3) { create(:taxon, name: 'Cake') } + let!(:property1) { create(:property, presentation: 'Organic') } + let!(:property2) { create(:property, presentation: 'Dairy-Free') } + let!(:property3) { create(:property, presentation: 'May Contain Nuts') } + let!(:product1) { + create(:product, name: "Kangaroo", primary_taxon: taxon1, properties: [property1]) + } + let!(:product2) { + create(:product, name: "Parsnips", primary_taxon: taxon2, properties: [property2]) + } + let!(:product3) { create(:product, primary_taxon: taxon2) } + let!(:product4) { create(:product, primary_taxon: taxon3, properties: [property3]) } + let!(:user) { create(:user) } + let(:customer) { create(:customer, user:, enterprise: distributor) } - before do - exchange.variants << product1.variants.first - exchange.variants << product2.variants.first - exchange.variants << product3.variants.first - allow(controller).to receive(:spree_current_user) { user } + before do + exchange.variants << product1.variants.first + exchange.variants << product2.variants.first + exchange.variants << product3.variants.first + allow(controller).to receive(:spree_current_user) { user } + end + + describe "#products" do + it "loads products for distributed products in the order cycle" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to include product1.id, product2.id, product3.id end - describe "#products" do + context "when product's variant unit scale is not in the available units list" do + before do + allow(Spree::Config).to receive(:available_units).and_return("lb,oz,kg,T,mL,L,kL") + end + it "loads products for distributed products in the order cycle" do api_get :products, id: order_cycle.id, distributor: distributor.id expect(product_ids).to include product1.id, product2.id, product3.id end + end - context "when product's variant unit scale is not in the available units list" do - before do - allow(Spree::Config).to receive(:available_units).and_return("lb,oz,kg,T,mL,L,kL") - end + it "returns products that were searched for" do + ransack_param = "name_or_meta_keywords_or_variants_display_as_or_" \ + "variants_display_name_or_variants_supplier_name_cont" + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { ransack_param => "Kangaroo" } - it "loads products for distributed products in the order cycle" do - api_get :products, id: order_cycle.id, distributor: distributor.id + expect(product_ids).to include product1.id + expect(product_ids).not_to include product2.id + end - expect(product_ids).to include product1.id, product2.id, product3.id - end + context "with variant overrides" do + let!(:vo1) { + create(:variant_override, + hub: distributor, + variant: product1.variants.first, + price: 1234.56) + } + let!(:vo2) { + create(:variant_override, + hub: distributor, + variant: product2.variants.first, + count_on_hand: 0) + } + + it "returns results scoped with variant overrides" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + overidden_product = json_response.select{ |product| product['id'] == product1.id } + expect(overidden_product[0]['variants'][0]['price']).to eq vo1.price.to_s end - it "returns products that were searched for" do - ransack_param = "name_or_meta_keywords_or_variants_display_as_or_" \ - "variants_display_name_or_variants_supplier_name_cont" - api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { ransack_param => "Kangaroo" } + it "does not return products where the variant overrides are out of stock" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).not_to include product2.id + end + end + + context "with property filters" do + before do + product1.update!(properties: [property1, property2]) + end + + it "filters by product property" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { with_properties: [property1.id, property2.id] } + + expect(response).to have_http_status :ok + expect(product_ids).to eq [product1.id, product2.id] + expect(product_ids).not_to include product3.id + end + + context "with supplier properties" do + let!(:supplier_property) { create(:property, presentation: 'Certified Organic') } + let!(:supplier) { create(:supplier_enterprise, properties: [supplier_property]) } + + before do + product1.variants.first.update!(supplier:) + product2.variants.first.update!(supplier:) + product3.update!(inherits_properties: false) + product3.variants.first.update!(supplier:) + end + + it "filter out the product that don't inherits from supplier properties" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { with_variants_supplier_properties: [supplier_property.id] } + + expect(response).to have_http_status :ok + expect(product_ids).to match_array [product1.id, product2.id] + expect(product_ids).not_to include product3.id + end + end + end + + context "with taxon filters" do + it "filters by taxon" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { variants_primary_taxon_id_in_any: [taxon2.id] } + + expect(product_ids).to include product2.id, product3.id + expect(product_ids).not_to include product1.id, product4.id + end + end + + context "when tag rules apply" do + let!(:vo1) { + create(:variant_override, + hub: distributor, + variant: product1.variants.first) + } + let!(:vo2) { + create(:variant_override, + hub: distributor, + variant: product2.variants.first) + } + let!(:vo3) { + create(:variant_override, + hub: distributor, + variant: product3.variants.first) + } + let(:default_hide_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + is_default: true, + preferred_variant_tags: "hide_these_variants_from_everyone", + preferred_matched_variants_visibility: "hidden") + } + let!(:hide_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + preferred_variant_tags: "hide_these_variants", + preferred_customer_tags: "hide_from_these_customers", + preferred_matched_variants_visibility: "hidden" ) + } + let!(:show_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + preferred_variant_tags: "show_these_variants", + preferred_customer_tags: "show_for_these_customers", + preferred_matched_variants_visibility: "visible" ) + } + + it "does not return variants hidden by general rules" do + vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) + + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).not_to include product1.id + end + + it "does not return variants hidden for this specific customer" do + vo2.update_attribute(:tag_list, hide_rule.preferred_variant_tags) + customer.update_attribute(:tag_list, hide_rule.preferred_customer_tags) + + api_get :products, id: order_cycle.id, distributor: distributor.id - expect(product_ids).to include product1.id expect(product_ids).not_to include product2.id end - context "with variant overrides" do - let!(:vo1) { - create(:variant_override, - hub: distributor, - variant: product1.variants.first, - price: 1234.56) - } - let!(:vo2) { - create(:variant_override, - hub: distributor, - variant: product2.variants.first, - count_on_hand: 0) - } + it "returns hidden variants made visible for this specific customer" do + vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) + vo3.update_attribute(:tag_list, + "#{show_rule.preferred_variant_tags}," \ + "#{default_hide_rule.preferred_variant_tags}") + customer.update_attribute(:tag_list, show_rule.preferred_customer_tags) - it "returns results scoped with variant overrides" do - api_get :products, id: order_cycle.id, distributor: distributor.id - - overidden_product = json_response.select{ |product| product['id'] == product1.id } - expect(overidden_product[0]['variants'][0]['price']).to eq vo1.price.to_s - end - - it "does not return products where the variant overrides are out of stock" do - api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(product_ids).not_to include product2.id - end - end - - context "with property filters" do - before do - product1.update!(properties: [property1, property2]) - end - - it "filters by product property" do - api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { with_properties: [property1.id, property2.id] } - - expect(response).to have_http_status :ok - expect(product_ids).to eq [product1.id, product2.id] - expect(product_ids).not_to include product3.id - end - - context "with supplier properties" do - let!(:supplier_property) { create(:property, presentation: 'Certified Organic') } - let!(:supplier) { create(:supplier_enterprise, properties: [supplier_property]) } - - before do - product1.variants.first.update!(supplier:) - product2.variants.first.update!(supplier:) - product3.update!(inherits_properties: false) - product3.variants.first.update!(supplier:) - end - - it "filter out the product that don't inherits from supplier properties" do - api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { with_variants_supplier_properties: [supplier_property.id] } - - expect(response).to have_http_status :ok - expect(product_ids).to match_array [product1.id, product2.id] - expect(product_ids).not_to include product3.id - end - end - end - - context "with taxon filters" do - it "filters by taxon" do - api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { variants_primary_taxon_id_in_any: [taxon2.id] } - - expect(product_ids).to include product2.id, product3.id - expect(product_ids).not_to include product1.id, product4.id - end - end - - context "when tag rules apply" do - let!(:vo1) { - create(:variant_override, - hub: distributor, - variant: product1.variants.first) - } - let!(:vo2) { - create(:variant_override, - hub: distributor, - variant: product2.variants.first) - } - let!(:vo3) { - create(:variant_override, - hub: distributor, - variant: product3.variants.first) - } - let(:default_hide_rule) { - create(:filter_products_tag_rule, - enterprise: distributor, - is_default: true, - preferred_variant_tags: "hide_these_variants_from_everyone", - preferred_matched_variants_visibility: "hidden") - } - let!(:hide_rule) { - create(:filter_products_tag_rule, - enterprise: distributor, - preferred_variant_tags: "hide_these_variants", - preferred_customer_tags: "hide_from_these_customers", - preferred_matched_variants_visibility: "hidden" ) - } - let!(:show_rule) { - create(:filter_products_tag_rule, - enterprise: distributor, - preferred_variant_tags: "show_these_variants", - preferred_customer_tags: "show_for_these_customers", - preferred_matched_variants_visibility: "visible" ) - } - - it "does not return variants hidden by general rules" do - vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) - - api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(product_ids).not_to include product1.id - end - - it "does not return variants hidden for this specific customer" do - vo2.update_attribute(:tag_list, hide_rule.preferred_variant_tags) - customer.update_attribute(:tag_list, hide_rule.preferred_customer_tags) - - api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(product_ids).not_to include product2.id - end - - it "returns hidden variants made visible for this specific customer" do - vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) - vo3.update_attribute(:tag_list, - "#{show_rule.preferred_variant_tags}," \ - "#{default_hide_rule.preferred_variant_tags}") - customer.update_attribute(:tag_list, show_rule.preferred_customer_tags) - - api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(product_ids).not_to include product1.id - expect(product_ids).to include product3.id - end - end - - context "when the order cycle is closed" do - before do - allow(controller).to receive(:order_cycle) { order_cycle } - allow(order_cycle).to receive(:open?) { false } - end - - # Regression test for https://github.com/openfoodfoundation/openfoodnetwork/issues/6491 - it "renders no products without error" do - api_get :products, id: order_cycle.id, distributor: distributor.id - - expect(json_response).to eq({}) - expect(response).to have_http_status :not_found - end - end - end - - describe "#taxons" do - it "loads taxons for distributed products in the order cycle" do - api_get :taxons, id: order_cycle.id, distributor: distributor.id - - taxons = json_response.pluck(:name) - - expect(json_response.length).to be 2 - expect(taxons).to include taxon1.name, taxon2.name - end - end - - describe "#properties" do - it "loads properties for distributed products in the order cycle" do - api_get :properties, id: order_cycle.id, distributor: distributor.id - - properties = json_response.pluck(:name) - - expect(json_response.length).to be 2 - expect(properties).to include property1.presentation, property2.presentation - end - end - - describe "#producer_properties" do - let!(:property4) { create(:property) } - let!(:supplier) { create(:supplier_enterprise) } - let!(:producer_property) { - create(:producer_property, producer_id: supplier.id, property: property4) - } - - before { product1.variants.first.update(supplier: ) } - - it "loads producer properties for distributed products in the order cycle" do - api_get :producer_properties, id: order_cycle.id, distributor: distributor.id - - properties = json_response.pluck(:name) - - expect(json_response.length).to be 1 - expect(properties).to include producer_property.property.presentation - end - end - - context "with custom taxon ordering applied and duplicate product names in the order cycle" do - let!(:supplier) { create(:supplier_enterprise) } - let!(:product5) { - create(:product, name: "Duplicate name", primary_taxon_id: taxon3.id, - supplier_id: supplier.id) - } - let!(:product6) { - create(:product, name: "Duplicate name", primary_taxon_id: taxon3.id, - supplier_id: supplier.id) - } - let!(:product7) { - create(:product, name: "Duplicate name", primary_taxon_id: taxon2.id, - supplier_id: supplier.id) - } - let!(:product8) { - create(:product, name: "Duplicate name", primary_taxon_id: taxon2.id, - supplier_id: supplier.id) - } - - before do - distributor.preferred_shopfront_taxon_order = "#{taxon2.id},#{taxon3.id},#{taxon1.id}" - exchange.variants << product5.variants.first - exchange.variants << product6.variants.first - exchange.variants << product7.variants.first - exchange.variants << product8.variants.first - end - - it "displays products in new order" do api_get :products, id: order_cycle.id, distributor: distributor.id - expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, - product6.id, product1.id] - end - it "displays products in correct order across multiple pages" do - api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 - expect(product_ids).to eq [product7.id, product8.id, product2.id] - - api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 2 - expect(product_ids).to eq [product3.id, product5.id, product6.id] - - api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 3 - expect(product_ids).to eq [product1.id] + expect(product_ids).not_to include product1.id + expect(product_ids).to include product3.id end end - private + context "when the order cycle is closed" do + before do + allow(controller).to receive(:order_cycle) { order_cycle } + allow(order_cycle).to receive(:open?) { false } + end - def product_ids - json_response.pluck(:id) + # Regression test for https://github.com/openfoodfoundation/openfoodnetwork/issues/6491 + it "renders no products without error" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(json_response).to eq({}) + expect(response).to have_http_status :not_found + end end end + + describe "#taxons" do + it "loads taxons for distributed products in the order cycle" do + api_get :taxons, id: order_cycle.id, distributor: distributor.id + + taxons = json_response.pluck(:name) + + expect(json_response.length).to be 2 + expect(taxons).to include taxon1.name, taxon2.name + end + end + + describe "#properties" do + it "loads properties for distributed products in the order cycle" do + api_get :properties, id: order_cycle.id, distributor: distributor.id + + properties = json_response.pluck(:name) + + expect(json_response.length).to be 2 + expect(properties).to include property1.presentation, property2.presentation + end + end + + describe "#producer_properties" do + let!(:property4) { create(:property) } + let!(:supplier) { create(:supplier_enterprise) } + let!(:producer_property) { + create(:producer_property, producer_id: supplier.id, property: property4) + } + + before { product1.variants.first.update(supplier: ) } + + it "loads producer properties for distributed products in the order cycle" do + api_get :producer_properties, id: order_cycle.id, distributor: distributor.id + + properties = json_response.pluck(:name) + + expect(json_response.length).to be 1 + expect(properties).to include producer_property.property.presentation + end + end + + context "with custom taxon ordering applied and duplicate product names in the order cycle" do + let!(:supplier) { create(:supplier_enterprise) } + let!(:product5) { + create(:product, name: "Duplicate name", primary_taxon_id: taxon3.id, + supplier_id: supplier.id) + } + let!(:product6) { + create(:product, name: "Duplicate name", primary_taxon_id: taxon3.id, + supplier_id: supplier.id) + } + let!(:product7) { + create(:product, name: "Duplicate name", primary_taxon_id: taxon2.id, + supplier_id: supplier.id) + } + let!(:product8) { + create(:product, name: "Duplicate name", primary_taxon_id: taxon2.id, + supplier_id: supplier.id) + } + + before do + distributor.preferred_shopfront_taxon_order = "#{taxon2.id},#{taxon3.id},#{taxon1.id}" + exchange.variants << product5.variants.first + exchange.variants << product6.variants.first + exchange.variants << product7.variants.first + exchange.variants << product8.variants.first + end + + it "displays products in new order" do + api_get :products, id: order_cycle.id, distributor: distributor.id + expect(product_ids).to eq [product7.id, product8.id, product2.id, product3.id, product5.id, + product6.id, product1.id] + end + + it "displays products in correct order across multiple pages" do + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3 + expect(product_ids).to eq [product7.id, product8.id, product2.id] + + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 2 + expect(product_ids).to eq [product3.id, product5.id, product6.id] + + api_get :products, id: order_cycle.id, distributor: distributor.id, per_page: 3, page: 3 + expect(product_ids).to eq [product1.id] + end + end + + private + + def product_ids + json_response.pluck(:id) + end end From 77b7b5ea47cf6c599126b9dce89ebc2a561d0aa5 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:45:00 +1000 Subject: [PATCH 5/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 2 - .../api/v0/orders_controller_spec.rb | 738 +++++++++--------- 2 files changed, 368 insertions(+), 372 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 32c3f25017..f7a1aa64b1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,8 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/api/v0/order_cycles_controller_spec.rb' - - 'spec/controllers/api/v0/orders_controller_spec.rb' - 'spec/controllers/spree/admin/adjustments_controller_spec.rb' - 'spec/controllers/spree/admin/payment_methods_controller_spec.rb' - 'spec/controllers/spree/admin/variants_controller_spec.rb' diff --git a/spec/controllers/api/v0/orders_controller_spec.rb b/spec/controllers/api/v0/orders_controller_spec.rb index e0245dceff..418d45d8d4 100644 --- a/spec/controllers/api/v0/orders_controller_spec.rb +++ b/spec/controllers/api/v0/orders_controller_spec.rb @@ -2,415 +2,413 @@ require 'spec_helper' -module Api - RSpec.describe V0::OrdersController do - include AuthenticationHelper - render_views +RSpec.describe Api::V0::OrdersController do + include AuthenticationHelper + render_views - let!(:regular_user) { create(:user) } - let!(:admin_user) { create(:admin_user) } + let!(:regular_user) { create(:user) } + let!(:admin_user) { create(:admin_user) } - let!(:distributor) { create(:distributor_enterprise) } - let!(:coordinator) { create(:distributor_enterprise) } - let!(:order_cycle) { create(:simple_order_cycle, coordinator:) } + let!(:distributor) { create(:distributor_enterprise) } + let!(:coordinator) { create(:distributor_enterprise) } + let!(:order_cycle) { create(:simple_order_cycle, coordinator:) } - describe '#index' do - let!(:distributor2) { create(:distributor_enterprise) } - let!(:coordinator2) { create(:distributor_enterprise) } - let!(:supplier) { create(:supplier_enterprise) } - let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) } - let!(:order1) do - create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, - distributor:, billing_address: create(:address, lastname: "c"), - total: 5.0) - end - let!(:order2) do - create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, - distributor: distributor2, billing_address: create(:address, lastname: "a"), - total: 10.0) - end - let!(:order3) do - create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, - distributor:, billing_address: create(:address, lastname: "b"), - total: 1.0 ) - end - let!(:order4) do - create(:completed_order_with_fees, order_cycle: order_cycle2, distributor: distributor2, - billing_address: create(:address, lastname: "d"), - total: 15.0) - end - let!(:order5) { create(:order, state: 'cart', completed_at: nil) } - let!(:line_item1) do - create(:line_item_with_shipment, order: order1, - product: create(:product, supplier_id: supplier.id)) - end - let!(:line_item2) do - create(:line_item_with_shipment, order: order2, - product: create(:product, supplier_id: supplier.id)) - end - let!(:line_item3) do - create(:line_item_with_shipment, order: order2, - product: create(:product, supplier_id: supplier.id)) - end - let!(:line_item4) do - create(:line_item_with_shipment, order: order3, - product: create(:product, supplier_id: supplier.id)) + describe '#index' do + let!(:distributor2) { create(:distributor_enterprise) } + let!(:coordinator2) { create(:distributor_enterprise) } + let!(:supplier) { create(:supplier_enterprise) } + let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) } + let!(:order1) do + create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, + distributor:, billing_address: create(:address, lastname: "c"), + total: 5.0) + end + let!(:order2) do + create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, + distributor: distributor2, billing_address: create(:address, lastname: "a"), + total: 10.0) + end + let!(:order3) do + create(:order, order_cycle:, state: 'complete', completed_at: Time.zone.now, + distributor:, billing_address: create(:address, lastname: "b"), + total: 1.0 ) + end + let!(:order4) do + create(:completed_order_with_fees, order_cycle: order_cycle2, distributor: distributor2, + billing_address: create(:address, lastname: "d"), + total: 15.0) + end + let!(:order5) { create(:order, state: 'cart', completed_at: nil) } + let!(:line_item1) do + create(:line_item_with_shipment, order: order1, + product: create(:product, supplier_id: supplier.id)) + end + let!(:line_item2) do + create(:line_item_with_shipment, order: order2, + product: create(:product, supplier_id: supplier.id)) + end + let!(:line_item3) do + create(:line_item_with_shipment, order: order2, + product: create(:product, supplier_id: supplier.id)) + end + let!(:line_item4) do + create(:line_item_with_shipment, order: order3, + product: create(:product, supplier_id: supplier.id)) + end + + context 'as a regular user' do + before do + allow(controller).to receive(:spree_current_user) { regular_user } + get :index end - context 'as a regular user' do - before do - allow(controller).to receive(:spree_current_user) { regular_user } - get :index - end + it "returns unauthorized" do + assert_unauthorized! + end + end - it "returns unauthorized" do - assert_unauthorized! - end + context 'as an admin user' do + before do + allow(controller).to receive(:spree_current_user) { admin_user } end - context 'as an admin user' do - before do - allow(controller).to receive(:spree_current_user) { admin_user } - end - - it "retrieves a list of orders with appropriate attributes, + it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do + get :index + returns_orders(json_response) + end + end + + context 'as an enterprise user' do + context 'producer enterprise' do + before do + allow(controller).to receive(:spree_current_user) { supplier.owner } + end + + context "with no distributor allows to edit orders" do + before { get :index } + + it "does not display line items for which my enterprise is a supplier" do + assert_unauthorized! + end + end + + context "with distributor allows to edit orders" do + before do + distributor.update_columns(enable_producers_to_edit_orders: true) + get :index + end + + it "retrieves a list of orders which have my supplied products" do + returns_orders(json_response) + end + end + end + + context 'coordinator enterprise' do + before do + allow(controller).to receive(:spree_current_user) { coordinator.owner } get :index + end + + it "retrieves a list of orders" do returns_orders(json_response) end end - context 'as an enterprise user' do - context 'producer enterprise' do - before do - allow(controller).to receive(:spree_current_user) { supplier.owner } - end - - context "with no distributor allows to edit orders" do - before { get :index } - - it "does not display line items for which my enterprise is a supplier" do - assert_unauthorized! - end - end - - context "with distributor allows to edit orders" do - before do - distributor.update_columns(enable_producers_to_edit_orders: true) - get :index - end - - it "retrieves a list of orders which have my supplied products" do - returns_orders(json_response) - end - end + context 'hub enterprise' do + before do + allow(controller).to receive(:spree_current_user) { distributor.owner } + get :index end - context 'coordinator enterprise' do - before do - allow(controller).to receive(:spree_current_user) { coordinator.owner } - get :index - end - - it "retrieves a list of orders" do - returns_orders(json_response) - end - end - - context 'hub enterprise' do - before do - allow(controller).to receive(:spree_current_user) { distributor.owner } - get :index - end - - it "retrieves a list of orders" do - returns_orders(json_response) - end + it "retrieves a list of orders" do + returns_orders(json_response) end end + end - context 'using search filters' do - before do - allow(controller).to receive(:spree_current_user) { admin_user } - end - - it 'can show only completed orders' do - get :index, params: { q: { completed_at_not_null: true, s: 'created_at desc' } }, - as: :json - - expect(json_response['orders']).to eq serialized_orders([order4, order3, order2, order1]) - end + context 'using search filters' do + before do + allow(controller).to receive(:spree_current_user) { admin_user } end - context 'sorting' do - before do - allow(controller).to receive(:spree_current_user) { admin_user } - end + it 'can show only completed orders' do + get :index, params: { q: { completed_at_not_null: true, s: 'created_at desc' } }, + as: :json - it 'can sort orders by total' do - get :index, params: { q: { completed_at_not_null: true, s: 'total desc' } }, + expect(json_response['orders']).to eq serialized_orders([order4, order3, order2, order1]) + end + end + + context 'sorting' do + before do + allow(controller).to receive(:spree_current_user) { admin_user } + end + + it 'can sort orders by total' do + get :index, params: { q: { completed_at_not_null: true, s: 'total desc' } }, + as: :json + + expect(json_response['orders']).to eq serialized_orders([order4, order2, order1, order3]) + end + + it 'can sort orders by bill_address.lastname' do + get :index, params: { q: { completed_at_not_null: true, + s: 'bill_address_lastname ASC' } }, + as: :json + + expect(json_response['orders'] + .pluck('id')).to eq serialized_orders([order2, order3, order1, order4]) + .pluck('id') + end + + context "with an order without billing address" do + let!(:order7) { + create(:order_with_line_items, billing_address: nil, order_cycle:, + distributor:) + } + let(:expected_order_ids) { + serialized_orders([order2, order3, order1, order4, order7]).pluck('id') + } + + it 'can show orders without bill address' do + get :index, params: { q: {} }, as: :json - expect(json_response['orders']).to eq serialized_orders([order4, order2, order1, order3]) + expect(json_response[:orders].pluck(:id)).to match_array(expected_order_ids) end it 'can sort orders by bill_address.lastname' do - get :index, params: { q: { completed_at_not_null: true, - s: 'bill_address_lastname ASC' } }, + get :index, params: { q: { s: 'bill_address_lastname ASC' } }, as: :json - expect(json_response['orders'] - .pluck('id')).to eq serialized_orders([order2, order3, order1, order4]) - .pluck('id') - end - - context "with an order without billing address" do - let!(:order7) { - create(:order_with_line_items, billing_address: nil, order_cycle:, - distributor:) - } - let(:expected_order_ids) { - serialized_orders([order2, order3, order1, order4, order7]).pluck('id') - } - - it 'can show orders without bill address' do - get :index, params: { q: {} }, - as: :json - - expect(json_response[:orders].pluck(:id)).to match_array(expected_order_ids) - end - - it 'can sort orders by bill_address.lastname' do - get :index, params: { q: { s: 'bill_address_lastname ASC' } }, - as: :json - - expect(json_response[:orders].pluck(:id)).to eq expected_order_ids - end - end - end - - context 'with pagination' do - before do - allow(controller).to receive(:spree_current_user) { distributor.owner } - end - - it 'returns pagination data when query params contain :per_page]' do - get :index, params: { per_page: 15, page: 1 } - - pagination_data = { - 'results' => 2, - 'pages' => 1, - 'page' => 1, - 'per_page' => 15 - } - - expect(json_response['pagination']).to eq pagination_data + expect(json_response[:orders].pluck(:id)).to eq expected_order_ids end end end - describe "#show" do - let!(:order) { - create(:completed_order_with_totals, order_cycle:, distributor: ) - } - - context "Resource not found" do - before { allow(controller).to receive(:spree_current_user) { admin_user } } - - it "when no order number is given" do - get :show, params: { id: "" } - expect(response).to have_http_status(:not_found) - end - - it "when order number given is not in the systen" do - get :show, params: { id: "X1321313232" } - expect(response).to have_http_status(:not_found) - end + context 'with pagination' do + before do + allow(controller).to receive(:spree_current_user) { distributor.owner } end - context "access" do - it "returns unauthorized, as a regular user" do - allow(controller).to receive(:spree_current_user) { regular_user } - get :show, params: { id: order.number } - assert_unauthorized! - end + it 'returns pagination data when query params contain :per_page]' do + get :index, params: { per_page: 15, page: 1 } - it "returns the order, as an admin user" do - allow(controller).to receive(:spree_current_user) { admin_user } - get :show, params: { id: order.number } - expect_order - end - - it "returns the order, as the order distributor owner" do - allow(controller).to receive(:spree_current_user) { order.distributor.owner } - get :show, params: { id: order.number } - expect_order - end - - it "returns unauthorized, as the order product's supplier owner" do - allow(controller).to receive(:spree_current_user) { - order.line_items.first.variant.supplier.owner - } - get :show, params: { id: order.number } - assert_unauthorized! - end - - it "returns the order, as the Order Cycle coorinator owner" do - allow(controller).to receive(:spree_current_user) { order.order_cycle.coordinator.owner } - get :show, params: { id: order.number } - expect_order - end - end - - context "as distributor owner" do - let!(:order) { - create(:completed_order_with_fees, order_cycle:, distributor: ) + pagination_data = { + 'results' => 2, + 'pages' => 1, + 'page' => 1, + 'per_page' => 15 } - before { allow(controller).to receive(:spree_current_user) { order.distributor.owner } } - - it "can view an order not in a standard state" do - order.update(completed_at: nil, state: 'shipped') - get :show, params: { id: order.number } - expect_order - end - - it "can view an order with weight calculator (this validates case " \ - "where options[current_order] is nil on the shipping method serializer)" do - order.shipping_method.update_attribute(:calculator, - create(:weight_calculator, calculable: order)) - allow(controller).to receive(:current_order).and_return order - get :show, params: { id: order.number } - expect_order - end - - it "returns an order with all required fields" do - get :show, params: { id: order.number } - - expect_order - expect(json_response.symbolize_keys.keys).to include(*order_detailed_attributes) - - expect(json_response[:bill_address]).to include( - 'address1' => order.bill_address.address1, - 'lastname' => order.bill_address.lastname - ) - expect(json_response[:ship_address]).to include( - 'address1' => order.ship_address.address1, - 'lastname' => order.ship_address.lastname - ) - expect(json_response[:shipping_method][:name]).to eq order.shipping_method.name - - expect(json_response[:adjustments].first).to include( - 'label' => "Transaction fee", - 'amount' => order.all_adjustments.payment_fee.first.amount.to_s - ) - expect(json_response[:adjustments].second).to include( - 'label' => "Shipping", - 'amount' => order.shipment_adjustments.first.amount.to_s - ) - - expect(json_response[:payments].first[:amount]).to eq order.payments.first.amount.to_s - expect(json_response[:line_items].size).to eq order.line_items.size - expect(json_response[:line_items].first[:variant][:product_name]) - .to eq order.line_items.first.variant.product.name - expect(json_response[:line_items].first[:tax_category_id]) - .to eq order.line_items.first.variant.tax_category_id - end + expect(json_response['pagination']).to eq pagination_data end end - - describe "#capture and #ship actions" do - let(:user) { create(:user) } - let(:product) { create(:simple_product) } - let(:distributor) { create(:distributor_enterprise, owner: user) } - let(:order_cycle) { - create(:simple_order_cycle, - distributors: [distributor], variants: [product.variants.first]) - } - let!(:order) { - create(:order_with_totals_and_distribution, - user:, distributor:, order_cycle:, - state: 'complete', payment_state: 'balance_due') - } - - before do - order.finalize! - order.payments << create(:check_payment, order:, amount: order.total) - allow(controller).to receive(:spree_current_user) { order.distributor.owner } - end - - describe "#capture" do - it "captures payments and returns an updated order object" do - put :capture, params: { id: order.number } - - expect(order.reload.pending_payments.empty?).to be true - expect_order - end - - context "when payment is not required" do - before do - allow_any_instance_of(Spree::Order).to receive(:payment_required?) { false } - end - - it "returns an error" do - put :capture, params: { id: order.number } - - expect(json_response['error']) - .to eq 'Payment could not be processed, please check the details you entered' - end - end - end - - describe "#ship" do - before do - order.payments.first.capture! - end - - it "marks orders as shipped and returns an updated order object" do - put :ship, params: { id: order.number } - - expect(order.reload.shipments.any?(&:shipped?)).to be true - expect_order - end - end - end - - private - - def expect_order - expect(response).to have_http_status :ok - expect(json_response[:number]).to eq order.number - end - - def serialized_orders(orders) - serialized_orders = ActiveModel::ArraySerializer.new( - orders, - each_serializer: Api::Admin::OrderSerializer, - root: false - ) - - JSON.parse(serialized_orders.to_json) - end - - def returns_orders(response) - keys = response['orders'].first.keys.map(&:to_sym) - expect(order_attributes.all?{ |attr| keys.include? attr }).to be_truthy - end - - def order_attributes - [ - :id, :number, :full_name, :email, :phone, :completed_at, :display_total, - :edit_path, :state, :payment_state, :shipment_state, - :payments_path, :ready_to_ship, :ready_to_capture, :created_at, - :distributor_name, :special_instructions - ] - end - - def order_detailed_attributes - [ - :number, :item_total, :total, :state, :adjustment_total, :payment_total, - :completed_at, :shipment_state, :payment_state, :email, :special_instructions - ] - end + end + + describe "#show" do + let!(:order) { + create(:completed_order_with_totals, order_cycle:, distributor: ) + } + + context "Resource not found" do + before { allow(controller).to receive(:spree_current_user) { admin_user } } + + it "when no order number is given" do + get :show, params: { id: "" } + expect(response).to have_http_status(:not_found) + end + + it "when order number given is not in the systen" do + get :show, params: { id: "X1321313232" } + expect(response).to have_http_status(:not_found) + end + end + + context "access" do + it "returns unauthorized, as a regular user" do + allow(controller).to receive(:spree_current_user) { regular_user } + get :show, params: { id: order.number } + assert_unauthorized! + end + + it "returns the order, as an admin user" do + allow(controller).to receive(:spree_current_user) { admin_user } + get :show, params: { id: order.number } + expect_order + end + + it "returns the order, as the order distributor owner" do + allow(controller).to receive(:spree_current_user) { order.distributor.owner } + get :show, params: { id: order.number } + expect_order + end + + it "returns unauthorized, as the order product's supplier owner" do + allow(controller).to receive(:spree_current_user) { + order.line_items.first.variant.supplier.owner + } + get :show, params: { id: order.number } + assert_unauthorized! + end + + it "returns the order, as the Order Cycle coorinator owner" do + allow(controller).to receive(:spree_current_user) { order.order_cycle.coordinator.owner } + get :show, params: { id: order.number } + expect_order + end + end + + context "as distributor owner" do + let!(:order) { + create(:completed_order_with_fees, order_cycle:, distributor: ) + } + + before { allow(controller).to receive(:spree_current_user) { order.distributor.owner } } + + it "can view an order not in a standard state" do + order.update(completed_at: nil, state: 'shipped') + get :show, params: { id: order.number } + expect_order + end + + it "can view an order with weight calculator (this validates case " \ + "where options[current_order] is nil on the shipping method serializer)" do + order.shipping_method.update_attribute(:calculator, + create(:weight_calculator, calculable: order)) + allow(controller).to receive(:current_order).and_return order + get :show, params: { id: order.number } + expect_order + end + + it "returns an order with all required fields" do + get :show, params: { id: order.number } + + expect_order + expect(json_response.symbolize_keys.keys).to include(*order_detailed_attributes) + + expect(json_response[:bill_address]).to include( + 'address1' => order.bill_address.address1, + 'lastname' => order.bill_address.lastname + ) + expect(json_response[:ship_address]).to include( + 'address1' => order.ship_address.address1, + 'lastname' => order.ship_address.lastname + ) + expect(json_response[:shipping_method][:name]).to eq order.shipping_method.name + + expect(json_response[:adjustments].first).to include( + 'label' => "Transaction fee", + 'amount' => order.all_adjustments.payment_fee.first.amount.to_s + ) + expect(json_response[:adjustments].second).to include( + 'label' => "Shipping", + 'amount' => order.shipment_adjustments.first.amount.to_s + ) + + expect(json_response[:payments].first[:amount]).to eq order.payments.first.amount.to_s + expect(json_response[:line_items].size).to eq order.line_items.size + expect(json_response[:line_items].first[:variant][:product_name]) + .to eq order.line_items.first.variant.product.name + expect(json_response[:line_items].first[:tax_category_id]) + .to eq order.line_items.first.variant.tax_category_id + end + end + end + + describe "#capture and #ship actions" do + let(:user) { create(:user) } + let(:product) { create(:simple_product) } + let(:distributor) { create(:distributor_enterprise, owner: user) } + let(:order_cycle) { + create(:simple_order_cycle, + distributors: [distributor], variants: [product.variants.first]) + } + let!(:order) { + create(:order_with_totals_and_distribution, + user:, distributor:, order_cycle:, + state: 'complete', payment_state: 'balance_due') + } + + before do + order.finalize! + order.payments << create(:check_payment, order:, amount: order.total) + allow(controller).to receive(:spree_current_user) { order.distributor.owner } + end + + describe "#capture" do + it "captures payments and returns an updated order object" do + put :capture, params: { id: order.number } + + expect(order.reload.pending_payments.empty?).to be true + expect_order + end + + context "when payment is not required" do + before do + allow_any_instance_of(Spree::Order).to receive(:payment_required?) { false } + end + + it "returns an error" do + put :capture, params: { id: order.number } + + expect(json_response['error']) + .to eq 'Payment could not be processed, please check the details you entered' + end + end + end + + describe "#ship" do + before do + order.payments.first.capture! + end + + it "marks orders as shipped and returns an updated order object" do + put :ship, params: { id: order.number } + + expect(order.reload.shipments.any?(&:shipped?)).to be true + expect_order + end + end + end + + private + + def expect_order + expect(response).to have_http_status :ok + expect(json_response[:number]).to eq order.number + end + + def serialized_orders(orders) + serialized_orders = ActiveModel::ArraySerializer.new( + orders, + each_serializer: Api::Admin::OrderSerializer, + root: false + ) + + JSON.parse(serialized_orders.to_json) + end + + def returns_orders(response) + keys = response['orders'].first.keys.map(&:to_sym) + expect(order_attributes.all?{ |attr| keys.include? attr }).to be_truthy + end + + def order_attributes + [ + :id, :number, :full_name, :email, :phone, :completed_at, :display_total, + :edit_path, :state, :payment_state, :shipment_state, + :payments_path, :ready_to_ship, :ready_to_capture, :created_at, + :distributor_name, :special_instructions + ] + end + + def order_detailed_attributes + [ + :number, :item_total, :total, :state, :adjustment_total, :payment_total, + :completed_at, :shipment_state, :payment_state, :email, :special_instructions + ] end end From 5a497bc6eec83969c5f331dee3eacc1fcd0f4e2f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:46:30 +1000 Subject: [PATCH 6/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 1 - .../admin/adjustments_controller_spec.rb | 432 +++++++++--------- 2 files changed, 215 insertions(+), 218 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f7a1aa64b1..6d75389ff2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,7 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/spree/admin/adjustments_controller_spec.rb' - 'spec/controllers/spree/admin/payment_methods_controller_spec.rb' - 'spec/controllers/spree/admin/variants_controller_spec.rb' - 'spec/lib/open_food_network/address_finder_spec.rb' diff --git a/spec/controllers/spree/admin/adjustments_controller_spec.rb b/spec/controllers/spree/admin/adjustments_controller_spec.rb index abccf7d87a..7806ba3c05 100644 --- a/spec/controllers/spree/admin/adjustments_controller_spec.rb +++ b/spec/controllers/spree/admin/adjustments_controller_spec.rb @@ -2,254 +2,252 @@ require 'spec_helper' -module Spree - RSpec.describe Admin::AdjustmentsController do - include AuthenticationHelper +RSpec.describe Spree::Admin::AdjustmentsController do + include AuthenticationHelper - before { controller_login_as_admin } + before { controller_login_as_admin } - describe "index" do - let!(:order) { create(:completed_order_with_totals) } - let!(:adjustment1) { - create(:adjustment, originator_type: "Spree::ShippingMethod", order:, - adjustable: order.shipment) - } - let!(:adjustment2) { - create(:adjustment, originator_type: "Spree::PaymentMethod", eligible: true, order:) - } - let!(:adjustment3) { - create(:adjustment, originator_type: "Spree::PaymentMethod", eligible: false, order:) - } - let!(:adjustment4) { create(:adjustment, originator_type: "EnterpriseFee", order:) } - let!(:adjustment5) { create(:adjustment, originator: nil, adjustable: order, order:) } + describe "index" do + let!(:order) { create(:completed_order_with_totals) } + let!(:adjustment1) { + create(:adjustment, originator_type: "Spree::ShippingMethod", order:, + adjustable: order.shipment) + } + let!(:adjustment2) { + create(:adjustment, originator_type: "Spree::PaymentMethod", eligible: true, order:) + } + let!(:adjustment3) { + create(:adjustment, originator_type: "Spree::PaymentMethod", eligible: false, order:) + } + let!(:adjustment4) { create(:adjustment, originator_type: "EnterpriseFee", order:) } + let!(:adjustment5) { create(:adjustment, originator: nil, adjustable: order, order:) } - it "displays eligible adjustments" do - spree_get :index, order_id: order.number + it "displays eligible adjustments" do + spree_get :index, order_id: order.number - expect(assigns(:collection)).to include adjustment1, adjustment2 - expect(assigns(:collection)).not_to include adjustment3 - end - - it "displays admin adjustments" do - spree_get :index, order_id: order.number - - expect(assigns(:collection)).to include adjustment5 - end - - it "does not display enterprise fee adjustments" do - spree_get :index, order_id: order.number - - expect(assigns(:collection)).not_to include adjustment4 - end + expect(assigns(:collection)).to include adjustment1, adjustment2 + expect(assigns(:collection)).not_to include adjustment3 end - describe "setting the adjustment's tax" do - let(:order) { create(:order) } - let(:zone) { create(:zone_with_member) } - let(:tax_rate) { create(:tax_rate, amount: 0.1, zone:, included_in_price: true ) } + it "displays admin adjustments" do + spree_get :index, order_id: order.number - describe "creating an adjustment" do - let(:tax_category_param) { '' } - let(:params) { - { - order_id: order.number, - adjustment: { - label: 'Testing included tax', amount: '110', tax_category_id: tax_category_param - } - } - } - - context "when no tax category is specified" do - it "doesn't apply tax" do - spree_post :create, params - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - - new_adjustment = Adjustment.admin.last - - expect(new_adjustment.label).to eq('Testing included tax') - expect(new_adjustment.amount).to eq(110) - expect(new_adjustment.tax_category).to be_nil - expect(new_adjustment.order_id).to eq(order.id) - - expect(order.reload.total).to eq 110 - expect(order.included_tax_total).to eq 0 - end - end - - context "when a tax category is provided" do - let(:tax_category_param) { tax_rate.tax_category.id.to_s } - - it "applies tax" do - spree_post :create, params - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - - new_adjustment = Adjustment.admin.last - - expect(new_adjustment.label).to eq('Testing included tax') - expect(new_adjustment.amount).to eq(110) - expect(new_adjustment.tax_category).to eq tax_rate.tax_category - expect(new_adjustment.order_id).to eq(order.id) - - expect(order.reload.total).to eq 110 - expect(order.included_tax_total).to eq 10 - end - end - - context "when the tax category has multiple rates for the same tax zone" do - let(:tax_category) { create(:tax_category) } - let!(:tax_rate1) { - create(:tax_rate, amount: 0.1, zone:, included_in_price: false, - tax_category: ) - } - let!(:tax_rate2) { - create(:tax_rate, amount: 0.2, zone:, included_in_price: false, - tax_category: ) - } - let(:tax_category_param) { tax_category.id.to_s } - let(:params) { - { - order_id: order.number, - adjustment: { - label: 'Testing multiple rates', amount: '100', tax_category_id: tax_category_param - } - } - } - - it "applies both rates" do - spree_post :create, params - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - - new_adjustment = Adjustment.admin.last - - expect(new_adjustment.amount).to eq(100) - expect(new_adjustment.tax_category).to eq tax_category - expect(new_adjustment.order_id).to eq(order.id) - expect(new_adjustment.adjustments.tax.count).to eq 2 - - expect(order.reload.total).to eq 130 - expect(order.additional_tax_total).to eq 30 - end - end - end - - describe "updating an adjustment" do - let(:old_tax_category) { create(:tax_category) } - let(:tax_category_param) { '' } - let(:params) { - { - id: adjustment.id, - order_id: order.number, - adjustment: { - label: 'Testing included tax', amount: '110', tax_category_id: tax_category_param - } - } - } - let(:adjustment) { - create(:adjustment, adjustable: order, order:, - amount: 1100, tax_category: old_tax_category) - } - - context "when no tax category is specified" do - it "doesn't apply tax" do - spree_put :update, params - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - - adjustment = Adjustment.admin.last - - expect(adjustment.label).to eq('Testing included tax') - expect(adjustment.amount).to eq(110) - expect(adjustment.tax_category).to be_nil - expect(adjustment.order_id).to eq(order.id) - - expect(order.reload.total).to eq 110 - expect(order.included_tax_total).to eq 0 - end - end - - context "when a tax category is provided" do - let(:tax_category_param) { tax_rate.tax_category.id.to_s } - - it "applies tax" do - spree_put :update, params - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - - adjustment = Adjustment.admin.last - - expect(adjustment.label).to eq('Testing included tax') - expect(adjustment.amount).to eq(110) - expect(adjustment.tax_category).to eq tax_rate.tax_category - expect(adjustment.order_id).to eq(order.id) - - expect(order.reload.total).to eq 110 - expect(order.included_tax_total).to eq 10 - end - end - end + expect(assigns(:collection)).to include adjustment5 end - describe "#delete" do - let!(:order) { create(:completed_order_with_totals) } - let(:payment_fee) { - create(:adjustment, amount: 0.50, order:, adjustable: order.payments.first) + it "does not display enterprise fee adjustments" do + spree_get :index, order_id: order.number + + expect(assigns(:collection)).not_to include adjustment4 + end + end + + describe "setting the adjustment's tax" do + let(:order) { create(:order) } + let(:zone) { create(:zone_with_member) } + let(:tax_rate) { create(:tax_rate, amount: 0.1, zone:, included_in_price: true ) } + + describe "creating an adjustment" do + let(:tax_category_param) { '' } + let(:params) { + { + order_id: order.number, + adjustment: { + label: 'Testing included tax', amount: '110', tax_category_id: tax_category_param + } + } } - context "as an enterprise user with edit permissions on the order" do - before do - order.adjustments << payment_fee - controller_login_as_enterprise_user([order.distributor]) - end - - it "deletes the adjustment" do - spree_delete :destroy, order_id: order.number, id: payment_fee.id - + context "when no tax category is specified" do + it "doesn't apply tax" do + spree_post :create, params expect(response).to redirect_to spree.admin_order_adjustments_path(order) - expect(order.reload.all_adjustments.count).to be_zero + + new_adjustment = Spree::Adjustment.admin.last + + expect(new_adjustment.label).to eq('Testing included tax') + expect(new_adjustment.amount).to eq(110) + expect(new_adjustment.tax_category).to be_nil + expect(new_adjustment.order_id).to eq(order.id) + + expect(order.reload.total).to eq 110 + expect(order.included_tax_total).to eq 0 end end - context "as an enterprise user with no permissions on the order" do - before do - order.adjustments << payment_fee - controller_login_as_enterprise_user([create(:enterprise)]) + context "when a tax category is provided" do + let(:tax_category_param) { tax_rate.tax_category.id.to_s } + + it "applies tax" do + spree_post :create, params + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + + new_adjustment = Spree::Adjustment.admin.last + + expect(new_adjustment.label).to eq('Testing included tax') + expect(new_adjustment.amount).to eq(110) + expect(new_adjustment.tax_category).to eq tax_rate.tax_category + expect(new_adjustment.order_id).to eq(order.id) + + expect(order.reload.total).to eq 110 + expect(order.included_tax_total).to eq 10 end + end - it "is unauthorized, does not delete the adjustment" do - spree_delete :destroy, order_id: order.number, id: payment_fee.id + context "when the tax category has multiple rates for the same tax zone" do + let(:tax_category) { create(:tax_category) } + let!(:tax_rate1) { + create(:tax_rate, amount: 0.1, zone:, included_in_price: false, + tax_category: ) + } + let!(:tax_rate2) { + create(:tax_rate, amount: 0.2, zone:, included_in_price: false, + tax_category: ) + } + let(:tax_category_param) { tax_category.id.to_s } + let(:params) { + { + order_id: order.number, + adjustment: { + label: 'Testing multiple rates', amount: '100', tax_category_id: tax_category_param + } + } + } - expect(response).to redirect_to unauthorized_path - expect(order.reload.all_adjustments.count).to eq 1 + it "applies both rates" do + spree_post :create, params + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + + new_adjustment = Spree::Adjustment.admin.last + + expect(new_adjustment.amount).to eq(100) + expect(new_adjustment.tax_category).to eq tax_category + expect(new_adjustment.order_id).to eq(order.id) + expect(new_adjustment.adjustments.tax.count).to eq 2 + + expect(order.reload.total).to eq 130 + expect(order.additional_tax_total).to eq 30 end end end - describe "with a cancelled order" do - let(:order) { create(:completed_order_with_totals) } - let(:tax_rate) { create(:tax_rate, amount: 0.1, calculator: ::Calculator::DefaultTax.new) } + describe "updating an adjustment" do + let(:old_tax_category) { create(:tax_category) } + let(:tax_category_param) { '' } + let(:params) { + { + id: adjustment.id, + order_id: order.number, + adjustment: { + label: 'Testing included tax', amount: '110', tax_category_id: tax_category_param + } + } + } let(:adjustment) { - create(:adjustment, adjustable: order, order:, amount: 1100) + create(:adjustment, adjustable: order, order:, + amount: 1100, tax_category: old_tax_category) } - before do - expect(order.cancel).to eq true + context "when no tax category is specified" do + it "doesn't apply tax" do + spree_put :update, params + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + + adjustment = Spree::Adjustment.admin.last + + expect(adjustment.label).to eq('Testing included tax') + expect(adjustment.amount).to eq(110) + expect(adjustment.tax_category).to be_nil + expect(adjustment.order_id).to eq(order.id) + + expect(order.reload.total).to eq 110 + expect(order.included_tax_total).to eq 0 + end end - it "doesn't create adjustments" do - expect { - spree_post :create, order_id: order.number, - adjustment: { label: "Testing", amount: "110" } - }.not_to change { [Adjustment.count, order.reload.total] } + context "when a tax category is provided" do + let(:tax_category_param) { tax_rate.tax_category.id.to_s } - expect(response).to redirect_to spree.admin_order_adjustments_path(order) - end + it "applies tax" do + spree_put :update, params + expect(response).to redirect_to spree.admin_order_adjustments_path(order) - it "doesn't change adjustments" do - expect { - spree_put :update, order_id: order.number, id: adjustment.id, - adjustment: { label: "Testing", amount: "110" } - }.not_to change { [adjustment.reload.amount, order.reload.total] } + adjustment = Spree::Adjustment.admin.last - expect(response).to redirect_to spree.admin_order_adjustments_path(order) + expect(adjustment.label).to eq('Testing included tax') + expect(adjustment.amount).to eq(110) + expect(adjustment.tax_category).to eq tax_rate.tax_category + expect(adjustment.order_id).to eq(order.id) + + expect(order.reload.total).to eq 110 + expect(order.included_tax_total).to eq 10 + end end end end + + describe "#delete" do + let!(:order) { create(:completed_order_with_totals) } + let(:payment_fee) { + create(:adjustment, amount: 0.50, order:, adjustable: order.payments.first) + } + + context "as an enterprise user with edit permissions on the order" do + before do + order.adjustments << payment_fee + controller_login_as_enterprise_user([order.distributor]) + end + + it "deletes the adjustment" do + spree_delete :destroy, order_id: order.number, id: payment_fee.id + + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + expect(order.reload.all_adjustments.count).to be_zero + end + end + + context "as an enterprise user with no permissions on the order" do + before do + order.adjustments << payment_fee + controller_login_as_enterprise_user([create(:enterprise)]) + end + + it "is unauthorized, does not delete the adjustment" do + spree_delete :destroy, order_id: order.number, id: payment_fee.id + + expect(response).to redirect_to unauthorized_path + expect(order.reload.all_adjustments.count).to eq 1 + end + end + end + + describe "with a cancelled order" do + let(:order) { create(:completed_order_with_totals) } + let(:tax_rate) { create(:tax_rate, amount: 0.1, calculator: Calculator::DefaultTax.new) } + let(:adjustment) { + create(:adjustment, adjustable: order, order:, amount: 1100) + } + + before do + expect(order.cancel).to eq true + end + + it "doesn't create adjustments" do + expect { + spree_post :create, order_id: order.number, + adjustment: { label: "Testing", amount: "110" } + }.not_to change { [Spree::Adjustment.count, order.reload.total] } + + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + end + + it "doesn't change adjustments" do + expect { + spree_put :update, order_id: order.number, id: adjustment.id, + adjustment: { label: "Testing", amount: "110" } + }.not_to change { [adjustment.reload.amount, order.reload.total] } + + expect(response).to redirect_to spree.admin_order_adjustments_path(order) + end + end end From b2fd4ccb11aea29172d22a959819304d50052938 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:50:38 +1000 Subject: [PATCH 7/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 1 - .../admin/payment_methods_controller_spec.rb | 497 +++++++++--------- 2 files changed, 248 insertions(+), 250 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 6d75389ff2..591414153e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,7 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/spree/admin/payment_methods_controller_spec.rb' - 'spec/controllers/spree/admin/variants_controller_spec.rb' - 'spec/lib/open_food_network/address_finder_spec.rb' - 'spec/lib/open_food_network/order_cycle_form_applicator_spec.rb' diff --git a/spec/controllers/spree/admin/payment_methods_controller_spec.rb b/spec/controllers/spree/admin/payment_methods_controller_spec.rb index 6e14f8d3cd..e826174e8b 100644 --- a/spec/controllers/spree/admin/payment_methods_controller_spec.rb +++ b/spec/controllers/spree/admin/payment_methods_controller_spec.rb @@ -2,301 +2,300 @@ require 'spec_helper' -module Spree - class GatewayWithPassword < PaymentMethod - preference :password, :string, default: "password" +class GatewayWithPassword < Spree::PaymentMethod + preference :password, :string, default: "password" +end + +RSpec.describe Spree::Admin::PaymentMethodsController do + let(:user) { + create(:user, enterprises: [create(:distributor_enterprise)]) + } + + describe "#new" do + before { allow(controller).to receive(:spree_current_user) { user } } + + it "allows to select from a range of payment gateways" do + spree_get :new + providers = assigns(:providers).map(&:to_s) + + expect(providers).to eq %w[ + Spree::Gateway::PayPalExpress + Spree::PaymentMethod::Check + ] + end + + it "allows to select StripeSCA when configured" do + allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(true) + + spree_get :new + providers = assigns(:providers).map(&:to_s) + + expect(providers).to include "Spree::Gateway::StripeSCA" + end end - RSpec.describe Admin::PaymentMethodsController do - let(:user) { - create(:user, enterprises: [create(:distributor_enterprise)]) + describe "#edit" do + let(:stripe) { + create( + :stripe_sca_payment_method, + distributor_ids: [enterprise_id], + preferred_enterprise_id: enterprise_id + ) } + let(:enterprise_id) { user.enterprise_ids.first } - describe "#new" do - before { allow(controller).to receive(:spree_current_user) { user } } + before { allow(controller).to receive(:spree_current_user) { user } } - it "allows to select from a range of payment gateways" do - spree_get :new - providers = assigns(:providers).map(&:to_s) + it "shows the current gateway type even if not enabled" do + allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(false) - expect(providers).to eq %w[ - Spree::Gateway::PayPalExpress - Spree::PaymentMethod::Check - ] - end + spree_get :edit, id: stripe.id + providers = assigns(:providers).map(&:to_s) - it "allows to select StripeSCA when configured" do - allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(true) + expect(providers).to include "Spree::Gateway::StripeSCA" + end + end - spree_get :new - providers = assigns(:providers).map(&:to_s) + describe "#create and #update" do + let!(:enterprise) { create(:distributor_enterprise, owner: user) } + let(:payment_method) { + GatewayWithPassword.create!(name: "Bogus", preferred_password: "haxme", + distributor_ids: [enterprise.id]) + } + let!(:user) { create(:user) } - expect(providers).to include "Spree::Gateway::StripeSCA" + before { allow(controller).to receive(:spree_current_user) { user } } + + it "does not clear password on update" do + expect(payment_method.preferred_password).to eq "haxme" + spree_put :update, id: payment_method.id, + payment_method: { + type: payment_method.class.to_s, + preferred_password: "" + } + expect(response).to redirect_to spree.edit_admin_payment_method_path(payment_method) + + payment_method.reload + expect(payment_method.preferred_password).to eq "haxme" + end + + context "tries to save invalid payment" do + it "doesn't break, responds nicely" do + expect { + spree_post :create, payment_method: { name: "", type: "Spree::Gateway::PayPalExpress" } + }.not_to raise_error end end - describe "#edit" do - let(:stripe) { - create( - :stripe_sca_payment_method, - distributor_ids: [enterprise_id], - preferred_enterprise_id: enterprise_id - ) - } - let(:enterprise_id) { user.enterprise_ids.first } + it "can create a payment method of a valid type" do + expect { + spree_post :create, + payment_method: { name: "Test Method", type: "Spree::Gateway::PayPalExpress", + distributor_ids: [enterprise.id] } + }.to change { Spree::PaymentMethod.count }.by(1) - before { allow(controller).to receive(:spree_current_user) { user } } - - it "shows the current gateway type even if not enabled" do - allow(Spree::Config).to receive(:stripe_connect_enabled).and_return(false) - - spree_get :edit, id: stripe.id - providers = assigns(:providers).map(&:to_s) - - expect(providers).to include "Spree::Gateway::StripeSCA" - end + expect(response).to be_redirect + expect(response).to redirect_to spree + .edit_admin_payment_method_path(assigns(:payment_method)) end - describe "#create and #update" do - let!(:enterprise) { create(:distributor_enterprise, owner: user) } - let(:payment_method) { - GatewayWithPassword.create!(name: "Bogus", preferred_password: "haxme", - distributor_ids: [enterprise.id]) + it "can not create a payment method of an invalid type" do + expect { + spree_post :create, + payment_method: { name: "Invalid Payment Method", type: "Spree::InvalidType", + distributor_ids: [enterprise.id] } + }.to change { Spree::PaymentMethod.count }.by(0) + + expect(response).to be_redirect + expect(response).to redirect_to spree.new_admin_payment_method_path + end + end + + describe "#update" do + context "updating a payment method" do + let!(:payment_method) { create(:payment_method, :flat_rate) } + let(:params) { + { + id: payment_method.id, + payment_method: { + name: "Updated", + description: "Updated", + type: "Spree::PaymentMethod::Check", + calculator_attributes: { + id: payment_method.calculator.id, + preferred_amount: 456, + } + } + } } - let!(:user) { create(:user) } - before { allow(controller).to receive(:spree_current_user) { user } } - - it "does not clear password on update" do - expect(payment_method.preferred_password).to eq "haxme" - spree_put :update, id: payment_method.id, - payment_method: { - type: payment_method.class.to_s, - preferred_password: "" - } - expect(response).to redirect_to spree.edit_admin_payment_method_path(payment_method) + before { controller_login_as_admin } + it "updates the payment method" do + spree_post :update, params payment_method.reload - expect(payment_method.preferred_password).to eq "haxme" + + expect(payment_method.name).to eq "Updated" + expect(payment_method.description).to eq "Updated" + expect(payment_method.calculator.preferred_amount).to eq 456 end - context "tries to save invalid payment" do - it "doesn't break, responds nicely" do - expect { - spree_post :create, payment_method: { name: "", type: "Spree::Gateway::PayPalExpress" } - }.not_to raise_error - end - end - - it "can create a payment method of a valid type" do - expect { - spree_post :create, - payment_method: { name: "Test Method", type: "Spree::Gateway::PayPalExpress", - distributor_ids: [enterprise.id] } - }.to change { Spree::PaymentMethod.count }.by(1) - - expect(response).to be_redirect - expect(response).to redirect_to spree - .edit_admin_payment_method_path(assigns(:payment_method)) - end - - it "can not create a payment method of an invalid type" do - expect { - spree_post :create, - payment_method: { name: "Invalid Payment Method", type: "Spree::InvalidType", - distributor_ids: [enterprise.id] } - }.to change { Spree::PaymentMethod.count }.by(0) - - expect(response).to be_redirect - expect(response).to redirect_to spree.new_admin_payment_method_path - end - end - - describe "#update" do - context "updating a payment method" do - let!(:payment_method) { create(:payment_method, :flat_rate) } + context "when the given payment method type does not match" do let(:params) { { id: payment_method.id, payment_method: { - name: "Updated", - description: "Updated", - type: "Spree::PaymentMethod::Check", - calculator_attributes: { - id: payment_method.calculator.id, - preferred_amount: 456, - } + type: "Spree::Gateway::PayPalExpress" } } } - before { controller_login_as_admin } - - it "updates the payment method" do + it "updates the payment method type" do spree_post :update, params - payment_method.reload - expect(payment_method.name).to eq "Updated" - expect(payment_method.description).to eq "Updated" - expect(payment_method.calculator.preferred_amount).to eq 456 - end - - context "when the given payment method type does not match" do - let(:params) { - { - id: payment_method.id, - payment_method: { - type: "Spree::Gateway::PayPalExpress" - } - } - } - - it "updates the payment method type" do - spree_post :update, params - - expect(PaymentMethod.find(payment_method.id).type).to eq "Spree::Gateway::PayPalExpress" - end - end - end - - context "on a StripeSCA payment method" do - let!(:user) { create(:user, enterprise_limit: 2) } - let!(:enterprise1) { create(:distributor_enterprise, owner: user) } - let!(:enterprise2) { create(:distributor_enterprise, owner: create(:user)) } - let!(:payment_method) { - create( - :stripe_sca_payment_method, - distributor_ids: [enterprise1.id, enterprise2.id], - preferred_enterprise_id: enterprise2.id - ) - } - - before { allow(controller).to receive(:spree_current_user) { user } } - - context "when an attempt is made to change " \ - "the stripe account holder (preferred_enterprise_id)" do - let(:params) { - { - id: payment_method.id, - payment_method: { - type: "Spree::Gateway::StripeSCA", - preferred_enterprise_id: enterprise1.id - } - } - } - - context "as a user that does not manage the existing stripe account holder" do - it "prevents the stripe account holder from being updated" do - spree_put :update, params - expect(payment_method.reload.preferred_enterprise_id).to eq enterprise2.id - end - end - - context "as a user that manages the existing stripe account holder" do - before { enterprise2.update!(owner_id: user.id) } - - it "allows the stripe account holder to be updated" do - spree_put :update, params - expect(payment_method.reload.preferred_enterprise_id).to eq enterprise1.id - end - - context "when no enterprise is selected as the account holder" do - before { payment_method.update_attribute(:preferred_enterprise_id, nil) } - - context "id not provided at all" do - before { params[:payment_method].delete(:preferred_enterprise_id) } - - it "does not save the payment method" do - spree_put :update, params - expect(response).to render_template :edit - expect(assigns(:payment_method).errors - .messages[:stripe_account_owner]).to include 'can\'t be blank' - end - end - - context "enterprise_id of 0" do - before { params[:payment_method][:preferred_enterprise_id] = 0 } - - it "does not save the payment method" do - spree_put :update, params - expect(response).to render_template :edit - expect(assigns(:payment_method).errors - .messages[:stripe_account_owner]).to include 'can\'t be blank' - end - end - end - end + type = Spree::PaymentMethod.find(payment_method.id).type + expect(type).to eq "Spree::Gateway::PayPalExpress" end end end - context "Requesting provider preference fields" do - let(:enterprise) { create(:distributor_enterprise) } - let(:user) do - new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', - password_confirmation: 'blahblah', ) - new_user.enterprise_roles.build(enterprise:).save - new_user.save - new_user - end + context "on a StripeSCA payment method" do + let!(:user) { create(:user, enterprise_limit: 2) } + let!(:enterprise1) { create(:distributor_enterprise, owner: user) } + let!(:enterprise2) { create(:distributor_enterprise, owner: create(:user)) } + let!(:payment_method) { + create( + :stripe_sca_payment_method, + distributor_ids: [enterprise1.id, enterprise2.id], + preferred_enterprise_id: enterprise2.id + ) + } - before do - allow(controller).to receive_messages spree_current_user: user - end + before { allow(controller).to receive(:spree_current_user) { user } } - context "on an existing payment method" do - let(:payment_method) { create(:payment_method) } + context "when an attempt is made to change " \ + "the stripe account holder (preferred_enterprise_id)" do + let(:params) { + { + id: payment_method.id, + payment_method: { + type: "Spree::Gateway::StripeSCA", + preferred_enterprise_id: enterprise1.id + } + } + } - context "where I have permission" do - before do - payment_method.distributors << user.enterprises.is_distributor.first - end - - context "without an altered provider type" do - it "renders provider settings with same payment method" do - spree_get :show_provider_preferences, - pm_id: payment_method.id, - provider_type: "Spree::PaymentMethod::Check" - expect(assigns(:payment_method)).to eq payment_method - expect(response).to render_template partial: '_provider_settings' - end - end - - context "with an altered provider type" do - it "renders provider settings with a different payment method" do - spree_get :show_provider_preferences, - pm_id: payment_method.id, - provider_type: "Spree::Gateway::PayPalExpress" - expect(assigns(:payment_method)).not_to eq payment_method - expect(response).to render_template partial: '_provider_settings' - end + context "as a user that does not manage the existing stripe account holder" do + it "prevents the stripe account holder from being updated" do + spree_put :update, params + expect(payment_method.reload.preferred_enterprise_id).to eq enterprise2.id end end - context "where I do not have permission" do - before do - payment_method.distributors = [] + context "as a user that manages the existing stripe account holder" do + before { enterprise2.update!(owner_id: user.id) } + + it "allows the stripe account holder to be updated" do + spree_put :update, params + expect(payment_method.reload.preferred_enterprise_id).to eq enterprise1.id end - it "renders unauthorised" do - spree_get :show_provider_preferences, - pm_id: payment_method.id, - provider_type: "Spree::PaymentMethod::Check" - expect(assigns(:payment_method)).to eq payment_method - expect(flash[:error]).to eq "Authorization Failure" - end - end - end + context "when no enterprise is selected as the account holder" do + before { payment_method.update_attribute(:preferred_enterprise_id, nil) } - context "on a new payment method" do - it "renders provider settings with a new payment method of type" do - spree_get :show_provider_preferences, - pm_id: "", - provider_type: "Spree::Gateway::PayPalExpress" - expect(assigns(:payment_method)).to be_a_new Spree::Gateway::PayPalExpress - expect(response).to render_template partial: '_provider_settings' + context "id not provided at all" do + before { params[:payment_method].delete(:preferred_enterprise_id) } + + it "does not save the payment method" do + spree_put :update, params + expect(response).to render_template :edit + expect(assigns(:payment_method).errors + .messages[:stripe_account_owner]).to include 'can\'t be blank' + end + end + + context "enterprise_id of 0" do + before { params[:payment_method][:preferred_enterprise_id] = 0 } + + it "does not save the payment method" do + spree_put :update, params + expect(response).to render_template :edit + expect(assigns(:payment_method).errors + .messages[:stripe_account_owner]).to include 'can\'t be blank' + end + end + end end end end end + + context "Requesting provider preference fields" do + let(:enterprise) { create(:distributor_enterprise) } + let(:user) do + new_user = create(:user, email: 'enterprise@hub.com', password: 'blahblah', + password_confirmation: 'blahblah', ) + new_user.enterprise_roles.build(enterprise:).save + new_user.save + new_user + end + + before do + allow(controller).to receive_messages spree_current_user: user + end + + context "on an existing payment method" do + let(:payment_method) { create(:payment_method) } + + context "where I have permission" do + before do + payment_method.distributors << user.enterprises.is_distributor.first + end + + context "without an altered provider type" do + it "renders provider settings with same payment method" do + spree_get :show_provider_preferences, + pm_id: payment_method.id, + provider_type: "Spree::PaymentMethod::Check" + expect(assigns(:payment_method)).to eq payment_method + expect(response).to render_template partial: '_provider_settings' + end + end + + context "with an altered provider type" do + it "renders provider settings with a different payment method" do + spree_get :show_provider_preferences, + pm_id: payment_method.id, + provider_type: "Spree::Gateway::PayPalExpress" + expect(assigns(:payment_method)).not_to eq payment_method + expect(response).to render_template partial: '_provider_settings' + end + end + end + + context "where I do not have permission" do + before do + payment_method.distributors = [] + end + + it "renders unauthorised" do + spree_get :show_provider_preferences, + pm_id: payment_method.id, + provider_type: "Spree::PaymentMethod::Check" + expect(assigns(:payment_method)).to eq payment_method + expect(flash[:error]).to eq "Authorization Failure" + end + end + end + + context "on a new payment method" do + it "renders provider settings with a new payment method of type" do + spree_get :show_provider_preferences, + pm_id: "", + provider_type: "Spree::Gateway::PayPalExpress" + expect(assigns(:payment_method)).to be_a_new Spree::Gateway::PayPalExpress + expect(response).to render_template partial: '_provider_settings' + end + end + end end From da5b147d297c119aa70c2ef17b57c766a6ef02f0 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Mon, 16 Jun 2025 16:51:50 +1000 Subject: [PATCH 8/8] Style Metrics/ModuleLength in spec file Best viewed without whitespace changes. --- .rubocop_todo.yml | 1 - .../spree/admin/variants_controller_spec.rb | 362 +++++++++--------- 2 files changed, 179 insertions(+), 184 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 591414153e..a3359eb36e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -171,7 +171,6 @@ Metrics/ModuleLength: - 'engines/order_management/spec/services/order_management/subscriptions/validator_spec.rb' - 'engines/order_management/spec/services/order_management/subscriptions/variants_list_spec.rb' - 'lib/open_food_network/column_preference_defaults.rb' - - 'spec/controllers/spree/admin/variants_controller_spec.rb' - 'spec/lib/open_food_network/address_finder_spec.rb' - 'spec/lib/open_food_network/order_cycle_form_applicator_spec.rb' - 'spec/lib/open_food_network/order_cycle_permissions_spec.rb' diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 5ca4491838..21f0413bcd 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -2,205 +2,201 @@ require 'spec_helper' -module Spree - module Admin - RSpec.describe VariantsController do - context "log in as admin user" do - before { controller_login_as_admin } +RSpec.describe Spree::Admin::VariantsController do + context "log in as admin user" do + before { controller_login_as_admin } - describe "#index" do - describe "deleted variants" do - let(:product) { create(:product, name: 'Product A') } - let(:deleted_variant) do - deleted_variant = product.variants.create( - unit_value: "2", variant_unit: "weight", variant_unit_scale: 1, price: 1, - primary_taxon: create(:taxon), supplier: create(:supplier_enterprise) - ) - deleted_variant.delete - deleted_variant - end - - it "lists only non-deleted variants with params[:deleted] == off" do - spree_get :index, product_id: product.id, deleted: "off" - expect(assigns(:variants)).to eq(product.variants) - end - - it "lists only deleted variants with params[:deleted] == on" do - spree_get :index, product_id: product.id, deleted: "on" - expect(assigns(:variants)).to eq([deleted_variant]) - end - end + describe "#index" do + describe "deleted variants" do + let(:product) { create(:product, name: 'Product A') } + let(:deleted_variant) do + deleted_variant = product.variants.create( + unit_value: "2", variant_unit: "weight", variant_unit_scale: 1, price: 1, + primary_taxon: create(:taxon), supplier: create(:supplier_enterprise) + ) + deleted_variant.delete + deleted_variant end - describe "#update" do - let!(:variant) { - create( - :variant, - display_name: "Tomatoes", - sku: 123, - supplier: producer - ) - } - let(:producer) { create(:enterprise) } - - it "updates the variant" do - expect { - spree_put( - :update, - id: variant.id, - product_id: variant.product.id, - variant: { display_name: "Better tomatoes", sku: 456 } - ) - variant.reload - }.to change { variant.display_name }.to("Better tomatoes") - .and change { variant.sku }.to(456.to_s) - end - - context "when updating supplier" do - let(:new_producer) { create(:enterprise) } - - it "updates the supplier" do - expect { - spree_put( - :update, - id: variant.id, - product_id: variant.product.id, - variant: { supplier_id: new_producer.id } - ) - variant.reload - }.to change { variant.supplier_id }.to(new_producer.id) - end - - it "removes associated product from existing Order Cycles" do - distributor = create(:distributor_enterprise) - order_cycle = create( - :simple_order_cycle, - variants: [variant], - coordinator: distributor, - distributors: [distributor] - ) - - spree_put( - :update, - id: variant.id, - product_id: variant.product.id, - variant: { supplier_id: new_producer.id } - ) - - expect(order_cycle.reload.distributed_variants).not_to include variant - end - end + it "lists only non-deleted variants with params[:deleted] == off" do + spree_get :index, product_id: product.id, deleted: "off" + expect(assigns(:variants)).to eq(product.variants) end - describe "#search" do - let(:supplier) { create(:supplier_enterprise) } - let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier.id) } - let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier.id) } - let!(:v1) { p1.variants.first } - let!(:v2) { p2.variants.first } - let!(:vo) { create(:variant_override, variant: v1, hub: d, count_on_hand: 44) } - let!(:d) { create(:distributor_enterprise) } - let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1]) } - - it "filters by distributor" do - spree_get :search, q: 'Prod', distributor_id: d.id.to_s - expect(assigns(:variants)).to eq([v1]) - end - - it "applies variant overrides" do - spree_get :search, q: 'Prod', distributor_id: d.id.to_s - expect(assigns(:variants)).to eq([v1]) - expect(assigns(:variants).first.on_hand).to eq(44) - end - - it "filters by order cycle" do - spree_get :search, q: 'Prod', order_cycle_id: oc.id.to_s - expect(assigns(:variants)).to eq([v1]) - end - - it "does not filter when no distributor or order cycle is specified" do - spree_get :search, q: 'Prod' - expect(assigns(:variants)).to match_array [v1, v2] - end - end - - describe '#destroy' do - let(:variant) { create(:variant) } - - context 'when requesting with html' do - before do - allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant } - allow(variant).to receive(:destroy).and_call_original - end - - it 'deletes the variant' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(variant).to have_received(:destroy) - end - - it 'shows a success flash message' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(flash[:success]).to be - end - - it 'redirects to admin_product_variants_url' do - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(response).to redirect_to spree.admin_product_variants_url(variant.product.id) - end - - it 'destroys all its exchanges' do - exchange = create(:exchange) - variant.exchanges << exchange - - spree_delete :destroy, id: variant.id, product_id: variant.product.id, - format: 'html' - expect(variant.exchanges.reload).to be_empty - end - end + it "lists only deleted variants with params[:deleted] == on" do + spree_get :index, product_id: product.id, deleted: "on" + expect(assigns(:variants)).to eq([deleted_variant]) end end + end - context "log in as supplier and distributor enable_producers_to_edit_orders" do - let(:supplier1) { create(:supplier_enterprise) } - let(:supplier2) { create(:supplier_enterprise) } - let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier1.id) } - let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier2.id) } - let!(:v1) { p1.variants.first } - let!(:v2) { p2.variants.first } - let!(:d) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } - let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1, v2]) } + describe "#update" do + let!(:variant) { + create( + :variant, + display_name: "Tomatoes", + sku: 123, + supplier: producer + ) + } + let(:producer) { create(:enterprise) } + it "updates the variant" do + expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { display_name: "Better tomatoes", sku: 456 } + ) + variant.reload + }.to change { variant.display_name }.to("Better tomatoes") + .and change { variant.sku }.to(456.to_s) + end + + context "when updating supplier" do + let(:new_producer) { create(:enterprise) } + + it "updates the supplier" do + expect { + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { supplier_id: new_producer.id } + ) + variant.reload + }.to change { variant.supplier_id }.to(new_producer.id) + end + + it "removes associated product from existing Order Cycles" do + distributor = create(:distributor_enterprise) + order_cycle = create( + :simple_order_cycle, + variants: [variant], + coordinator: distributor, + distributors: [distributor] + ) + + spree_put( + :update, + id: variant.id, + product_id: variant.product.id, + variant: { supplier_id: new_producer.id } + ) + + expect(order_cycle.reload.distributed_variants).not_to include variant + end + end + end + + describe "#search" do + let(:supplier) { create(:supplier_enterprise) } + let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier.id) } + let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier.id) } + let!(:v1) { p1.variants.first } + let!(:v2) { p2.variants.first } + let!(:vo) { create(:variant_override, variant: v1, hub: d, count_on_hand: 44) } + let!(:d) { create(:distributor_enterprise) } + let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1]) } + + it "filters by distributor" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) + end + + it "applies variant overrides" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) + expect(assigns(:variants).first.on_hand).to eq(44) + end + + it "filters by order cycle" do + spree_get :search, q: 'Prod', order_cycle_id: oc.id.to_s + expect(assigns(:variants)).to eq([v1]) + end + + it "does not filter when no distributor or order cycle is specified" do + spree_get :search, q: 'Prod' + expect(assigns(:variants)).to match_array [v1, v2] + end + end + + describe '#destroy' do + let(:variant) { create(:variant) } + + context 'when requesting with html' do before do - order = create(:order_with_line_items, distributor: d, line_items_count: 1) - order.line_items.take.variant.update_attribute(:supplier_id, supplier1.id) - controller_login_as_enterprise_user([supplier1]) + allow(Spree::Variant).to receive(:find).with(variant.id.to_s) { variant } + allow(variant).to receive(:destroy).and_call_original end - describe "#search" do - it "filters by distributor and supplier1 products" do - spree_get :search, q: 'Prod', distributor_id: d.id.to_s - expect(assigns(:variants)).to eq([v1]) - end + it 'deletes the variant' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(variant).to have_received(:destroy) end - describe "#update" do - it "updates the variant" do - expect { - spree_put( - :update, - id: v1.id, - product_id: v1.product.id, - variant: { display_name: "Better tomatoes", sku: 456 } - ) - v1.reload - }.to change { v1.display_name }.to("Better tomatoes") - .and change { v1.sku }.to(456.to_s) - end + it 'shows a success flash message' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(flash[:success]).to be + end + + it 'redirects to admin_product_variants_url' do + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(response).to redirect_to spree.admin_product_variants_url(variant.product.id) + end + + it 'destroys all its exchanges' do + exchange = create(:exchange) + variant.exchanges << exchange + + spree_delete :destroy, id: variant.id, product_id: variant.product.id, + format: 'html' + expect(variant.exchanges.reload).to be_empty end end end end + + context "log in as supplier and distributor enable_producers_to_edit_orders" do + let(:supplier1) { create(:supplier_enterprise) } + let(:supplier2) { create(:supplier_enterprise) } + let!(:p1) { create(:simple_product, name: 'Product 1', supplier_id: supplier1.id) } + let!(:p2) { create(:simple_product, name: 'Product 2', supplier_id: supplier2.id) } + let!(:v1) { p1.variants.first } + let!(:v2) { p2.variants.first } + let!(:d) { create(:distributor_enterprise, enable_producers_to_edit_orders: true) } + let!(:oc) { create(:simple_order_cycle, distributors: [d], variants: [v1, v2]) } + + before do + order = create(:order_with_line_items, distributor: d, line_items_count: 1) + order.line_items.take.variant.update_attribute(:supplier_id, supplier1.id) + controller_login_as_enterprise_user([supplier1]) + end + + describe "#search" do + it "filters by distributor and supplier1 products" do + spree_get :search, q: 'Prod', distributor_id: d.id.to_s + expect(assigns(:variants)).to eq([v1]) + end + end + + describe "#update" do + it "updates the variant" do + expect { + spree_put( + :update, + id: v1.id, + product_id: v1.product.id, + variant: { display_name: "Better tomatoes", sku: 456 } + ) + v1.reload + }.to change { v1.display_name }.to("Better tomatoes") + .and change { v1.sku }.to(456.to_s) + end + end + end end