diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index e95d9e89c6..aec1f024b9 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -600,13 +600,7 @@ Style/ClassAndModuleChildren: - 'app/serializers/api/taxon_serializer.rb' - 'app/serializers/api/variant_serializer.rb' - 'lib/open_food_network/locking.rb' - - 'lib/open_food_network/reports/report.rb' - - 'lib/open_food_network/reports/row.rb' - - 'lib/open_food_network/reports/rule.rb' - 'spec/controllers/spree/admin/base_controller_spec.rb' - - 'spec/lib/open_food_network/reports/report_spec.rb' - - 'spec/lib/open_food_network/reports/row_spec.rb' - - 'spec/lib/open_food_network/reports/rule_spec.rb' # Offense count: 2 Style/ClassVars: @@ -1037,7 +1031,6 @@ Style/FrozenStringLiteralComment: - 'lib/open_food_network/i18n_config.rb' - 'lib/open_food_network/lettuce_share_report.rb' - 'lib/open_food_network/locking.rb' - - 'lib/open_food_network/model_class_from_controller_name.rb' - 'lib/open_food_network/order_and_distributor_report.rb' - 'lib/open_food_network/order_cycle_form_applicator.rb' - 'lib/open_food_network/order_cycle_management_report.rb' @@ -1061,9 +1054,6 @@ Style/FrozenStringLiteralComment: - 'lib/open_food_network/referer_parser.rb' - 'lib/open_food_network/reports/line_items.rb' - 'lib/open_food_network/reports/list.rb' - - 'lib/open_food_network/reports/report.rb' - - 'lib/open_food_network/reports/row.rb' - - 'lib/open_food_network/reports/rule.rb' - 'lib/open_food_network/sales_tax_report.rb' - 'lib/open_food_network/scope_product_to_hub.rb' - 'lib/open_food_network/scope_variant_to_hub.rb' @@ -1294,9 +1284,6 @@ Style/FrozenStringLiteralComment: - 'spec/lib/open_food_network/products_and_inventory_report_spec.rb' - 'spec/lib/open_food_network/property_merge_spec.rb' - 'spec/lib/open_food_network/referer_parser_spec.rb' - - 'spec/lib/open_food_network/reports/report_spec.rb' - - 'spec/lib/open_food_network/reports/row_spec.rb' - - 'spec/lib/open_food_network/reports/rule_spec.rb' - 'spec/lib/open_food_network/sales_tax_report_spec.rb' - 'spec/lib/open_food_network/scope_variant_to_hub_spec.rb' - 'spec/lib/open_food_network/scope_variants_to_search_spec.rb' diff --git a/app/controllers/api/base_controller.rb b/app/controllers/api/base_controller.rb index 423a4fb104..e9d82d19ad 100644 --- a/app/controllers/api/base_controller.rb +++ b/app/controllers/api/base_controller.rb @@ -1,5 +1,6 @@ # Base controller for OFN's API require_dependency 'spree/api/controller_setup' +require "spree/core/controller_helpers/ssl" module Api class BaseController < ActionController::Metal diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 8fcdac8fe4..b039a2447d 100644 --- a/app/controllers/base_controller.rb +++ b/app/controllers/base_controller.rb @@ -1,8 +1,10 @@ +require 'spree/core/controller_helpers/auth' +require 'spree/core/controller_helpers/common' +require 'spree/core/controller_helpers/order' require 'spree/core/controller_helpers/respond_with' require 'open_food_network/tag_rule_applicator' class BaseController < ApplicationController - include Spree::Core::ControllerHelpers include Spree::Core::ControllerHelpers::Auth include Spree::Core::ControllerHelpers::Common include Spree::Core::ControllerHelpers::Order diff --git a/app/controllers/spree/user_passwords_controller.rb b/app/controllers/spree/user_passwords_controller.rb index bf8b13d379..4e7fc6e824 100644 --- a/app/controllers/spree/user_passwords_controller.rb +++ b/app/controllers/spree/user_passwords_controller.rb @@ -1,3 +1,10 @@ +# frozen_string_literal: true + +require "spree/core/controller_helpers/auth" +require "spree/core/controller_helpers/common" +require "spree/core/controller_helpers/order" +require "spree/core/controller_helpers/ssl" + module Spree class UserPasswordsController < Devise::PasswordsController helper 'spree/base', 'spree/store' diff --git a/app/controllers/spree/user_registrations_controller.rb b/app/controllers/spree/user_registrations_controller.rb index 98ad3c8d1c..1a2d740da7 100644 --- a/app/controllers/spree/user_registrations_controller.rb +++ b/app/controllers/spree/user_registrations_controller.rb @@ -1,3 +1,10 @@ +# frozen_string_literal: true + +require "spree/core/controller_helpers/auth" +require "spree/core/controller_helpers/common" +require "spree/core/controller_helpers/order" +require "spree/core/controller_helpers/ssl" + module Spree class UserRegistrationsController < Devise::RegistrationsController helper 'spree/base', 'spree/store' diff --git a/app/controllers/spree/user_sessions_controller.rb b/app/controllers/spree/user_sessions_controller.rb index 7ff26a3437..c17eafb892 100644 --- a/app/controllers/spree/user_sessions_controller.rb +++ b/app/controllers/spree/user_sessions_controller.rb @@ -1,3 +1,10 @@ +# frozen_string_literal: true + +require "spree/core/controller_helpers/auth" +require "spree/core/controller_helpers/common" +require "spree/core/controller_helpers/order" +require "spree/core/controller_helpers/ssl" + module Spree class UserSessionsController < Devise::SessionsController helper 'spree/base', 'spree/store' diff --git a/config/initializers/spree.rb b/config/initializers/spree.rb index 30e46ffc55..bc8005806e 100644 --- a/config/initializers/spree.rb +++ b/config/initializers/spree.rb @@ -6,6 +6,7 @@ # In order to initialize a setting do: # config.setting_name = 'new value' +require "spree/core/environment" require 'spree/product_filters' # Due to a bug in ActiveRecord we need to load the tagging code in Gateway which diff --git a/knapsack_rspec_report.json b/knapsack_rspec_report.json index 91499ba40e..f745a4438d 100644 --- a/knapsack_rspec_report.json +++ b/knapsack_rspec_report.json @@ -113,7 +113,6 @@ "spec/models/subscription_line_item_spec.rb": 0.021193265914916992, "spec/controllers/api/statuses_controller_spec.rb": 0.02451467514038086, "spec/lib/open_food_network/referer_parser_spec.rb": 0.015799283981323242, - "spec/lib/open_food_network/reports/rule_spec.rb": 0.01628732681274414, "spec/helpers/serializer_helper_spec.rb": 0.004682064056396484, "spec/jobs/heartbeat_job_spec.rb": 0.013271570205688477, "spec/services/mail_configuration_spec.rb": 0.01050567626953125, @@ -246,7 +245,6 @@ "spec/validators/integer_array_validator_spec.rb": 0.04994392395019531, "spec/validators/date_time_string_validator_spec.rb": 0.05316734313964844, "spec/models/product_import/reset_absent_spec.rb": 0.04071307182312012, - "spec/lib/open_food_network/reports/report_spec.rb": 0.04329681396484375, "spec/jobs/confirm_signup_job_spec.rb": 0.03060293197631836, "spec/services/order_cycle_distributed_variants_spec.rb": 0.02808237075805664, "spec/lib/open_food_network/feature_toggle_spec.rb": 0.0240786075592041, @@ -255,7 +253,6 @@ "spec/models/spree/calculator/flat_rate_spec.rb": 0.014808177947998047, "spec/models/spree/image_spec.rb": 0.014715909957885742, "spec/helpers/spree/admin/base_helper_spec.rb": 0.008042573928833008, - "spec/lib/open_food_network/reports/row_spec.rb": 0.005358695983886719, "engines/order_management/spec/controllers/order_management/reports/enterprise_fee_summaries_controller_spec.rb": 0.7100100517272949, "engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/parameters_spec.rb": 4.190261602401733, "engines/order_management/spec/services/order_management/reports/enterprise_fee_summary/report_data/enterprise_fee_type_total_spec.rb": 0.006247282028198242, diff --git a/lib/open_food_network/model_class_from_controller_name.rb b/lib/open_food_network/model_class_from_controller_name.rb deleted file mode 100644 index 97b92a18d7..0000000000 --- a/lib/open_food_network/model_class_from_controller_name.rb +++ /dev/null @@ -1,8 +0,0 @@ -module OpenFoodNetwork - module ModelClassFromControllerName - # Equivalent to CanCan's "authorize_resource :class => false" (see https://github.com/ryanb/cancan/blob/master/lib/cancan/controller_resource.rb#L146) - def model_class - self.class.to_s.sub("Controller", "").underscore.split('/').last.singularize.to_sym - end - end -end diff --git a/lib/open_food_network/reports/report.rb b/lib/open_food_network/reports/report.rb deleted file mode 100644 index 2c4e5900f2..0000000000 --- a/lib/open_food_network/reports/report.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'open_food_network/reports/row' -require 'open_food_network/reports/rule' - -module OpenFoodNetwork::Reports - class Report - class_attribute :_header, :_columns, :_rules_head - - # -- API - def header - _header - end - - def columns - _columns.to_a - end - - def rules - # Flatten linked list and return as hashes - rules = [] - - rule = _rules_head - while rule - rules << rule - rule = rule.next - end - - rules.map(&:to_h) - end - - # -- DSL - def self.header(*columns) - self._header = columns - end - end -end diff --git a/lib/open_food_network/reports/row.rb b/lib/open_food_network/reports/row.rb deleted file mode 100644 index 35b4bffe33..0000000000 --- a/lib/open_food_network/reports/row.rb +++ /dev/null @@ -1,15 +0,0 @@ -module OpenFoodNetwork::Reports - class Row - def initialize - @columns = [] - end - - def column(&block) - @columns << block - end - - def to_a - @columns - end - end -end diff --git a/lib/open_food_network/reports/rule.rb b/lib/open_food_network/reports/rule.rb deleted file mode 100644 index cfb8b7e67f..0000000000 --- a/lib/open_food_network/reports/rule.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'open_food_network/reports/row' - -module OpenFoodNetwork::Reports - class Rule - attr_reader :next - - def group(&block) - @group = block - end - - def sort(&block) - @sort = block - end - - def to_h - h = { group_by: @group, sort_by: @sort } - h[:summary_columns] = @summary_row.to_a if @summary_row - h - end - end -end diff --git a/lib/spree/core/controller_helpers/order.rb b/lib/spree/core/controller_helpers/order.rb index 1df01e286a..e95499bef2 100644 --- a/lib/spree/core/controller_helpers/order.rb +++ b/lib/spree/core/controller_helpers/order.rb @@ -68,8 +68,7 @@ module Spree session[:guest_token] = nil end - # Do not attempt to merge incomplete and current orders. - # Instead, destroy the incomplete orders. + # Recover incomplete orders from other sessions after logging in. def set_current_order return unless (user = spree_current_user) @@ -77,11 +76,10 @@ module Spree if session[:order_id].nil? && last_incomplete_order session[:order_id] = last_incomplete_order.id - elsif current_order(true) && - last_incomplete_order && - current_order != last_incomplete_order - last_incomplete_order.destroy end + + # Load current order and create a new one if necessary. + current_order(true) end def current_currency diff --git a/spec/controllers/base_controller_spec.rb b/spec/controllers/base_controller_spec.rb index ee7dcb4bcc..fb1d399335 100644 --- a/spec/controllers/base_controller_spec.rb +++ b/spec/controllers/base_controller_spec.rb @@ -9,6 +9,90 @@ describe BaseController, type: :controller do end end + describe "#current_order" do + let(:user) { create(:user) } + + it "doesn't change anything without a user" do + expect { + get :index + }.to_not change { Spree::Order.count } + end + + it "creates a new order" do + allow(controller).to receive(:spree_current_user).and_return(user) + + expect { + get :index + }.to change { Spree::Order.count }.by(1) + + expect(user.orders.count).to eq 1 + end + + it "uses the last incomplete order" do + last_cart = create(:order, user: user, created_by: user, state: "cart", completed_at: nil) + allow(controller).to receive(:spree_current_user).and_return(user) + + expect { + get :index + }.to_not change { Spree::Order.count } + + expect(session[:order_id]).to eq last_cart.id + end + + it "ignores the last incomplete order" do + # Spree used to merge the last order with the current one. + # And we used to override that logic to delete old incomplete orders. + # Now we are checking here that none of that is happening. + + last_cart = create(:order, user: user, created_by: user, state: "cart", completed_at: nil) + last_cart.line_items << create(:line_item) + + current_cart = create( + :order, + user: user, + created_by: user, + state: "cart", + completed_at: nil, + created_at: 1.week.ago + ) + session[:order_id] = current_cart.id + + allow(controller).to receive(:spree_current_user).and_return(user) + + expect { + get :index + }.to_not change { Spree::Order.count } + + expect(current_cart.line_items.count).to eq 0 + end + + it "doesn't recover old orders after checkout, a new empty one is created" do + last_cart = create(:order, user: user, created_by: user, state: "cart", completed_at: nil) + last_cart.line_items << create(:line_item) + + just_completed_order = create( + :order, + user: user, + created_by: user, + state: "complete", + completed_at: Time.zone.now, + created_at: 1.week.ago + ) + expect(just_completed_order.completed_at).to be_present + session[:order_id] = just_completed_order.id + + allow(controller).to receive(:spree_current_user).and_return(user) + + expect { + get :index + }.to change { Spree::Order.count }.by(1) + + expect(session[:order_id]).to_not eq just_completed_order.id + expect(session[:order_id]).to_not eq last_cart.id + expect(controller.current_order.line_items.count).to eq 0 + end + end + it "redirects to home with message if order cycle is expired" do expect(controller).to receive(:current_order_cycle).and_return(oc).twice expect(controller).to receive(:current_order).and_return(order).twice diff --git a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb index af91f3a917..f970c4b83a 100644 --- a/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb +++ b/spec/lib/open_food_network/orders_and_fulfillments_report_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'open_food_network/orders_and_fulfillments_report' +require 'open_food_network/order_grouper' describe OpenFoodNetwork::OrdersAndFulfillmentsReport do include AuthenticationHelper diff --git a/spec/lib/open_food_network/reports/report_spec.rb b/spec/lib/open_food_network/reports/report_spec.rb deleted file mode 100644 index 86e76bcec5..0000000000 --- a/spec/lib/open_food_network/reports/report_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'open_food_network/reports/report' - -module OpenFoodNetwork::Reports - class TestReport < Report - header 'One', 'Two', 'Three', 'Four' - end - - describe Report do - let(:report) { TestReport.new } - - it "returns the header" do - expect(report.header).to eq(%w(One Two Three Four)) - end - end -end diff --git a/spec/lib/open_food_network/reports/row_spec.rb b/spec/lib/open_food_network/reports/row_spec.rb deleted file mode 100644 index c541832edf..0000000000 --- a/spec/lib/open_food_network/reports/row_spec.rb +++ /dev/null @@ -1,19 +0,0 @@ -require 'spec_helper' -require 'open_food_network/reports/row' - -module OpenFoodNetwork::Reports - describe Row do - let(:row) { Row.new } - # rubocop:disable Style/Proc - let(:proc) { Proc.new {} } - # rubocop:enable Style/Proc - - it "can define a number of columns and return them as an array" do - row.column(&proc) - row.column(&proc) - row.column(&proc) - - expect(row.to_a).to eq([proc, proc, proc]) - end - end -end diff --git a/spec/lib/open_food_network/reports/rule_spec.rb b/spec/lib/open_food_network/reports/rule_spec.rb deleted file mode 100644 index eae5b26045..0000000000 --- a/spec/lib/open_food_network/reports/rule_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'spec_helper' -require 'open_food_network/reports/rule' - -module OpenFoodNetwork::Reports - describe Rule do - let(:rule) { Rule.new } - # rubocop:disable Style/Proc - let(:proc) { Proc.new {} } - # rubocop:enable Style/Proc - - it "can define a group proc and return it in a hash" do - rule.group(&proc) - expect(rule.to_h).to eq(group_by: proc, sort_by: nil) - end - - it "can define a sort proc and return it in a hash" do - rule.sort(&proc) - expect(rule.to_h).to eq(group_by: nil, sort_by: proc) - end - end -end diff --git a/spec/lib/spree/core/environment_spec.rb b/spec/lib/spree/core/environment_spec.rb new file mode 100644 index 0000000000..7eec8dc329 --- /dev/null +++ b/spec/lib/spree/core/environment_spec.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Spree::Core::Environment do + # Our version doesn't add any features we could test. + # So we just check that our file is loaded correctly. + let(:our_file) { Rails.root.join("lib/spree/core/environment.rb").to_s } + + it "is defined in our code" do + file = subject.method(:initialize).source_location.first + expect(file).to eq our_file + end + + it "used by Spree" do + file = Spree::Core::Engine.config.spree.method(:initialize).source_location.first + expect(file).to eq our_file + end +end