Merge pull request #5905 from mkllnk/lost-decorators

Load previously lost decorator code and don't delete orders

.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'

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

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

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'

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'

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'

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

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,

lib/open_food_network/model_class_from_controller_name.rb

@@ -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

lib/open_food_network/reports/report.rb

@@ -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

lib/open_food_network/reports/row.rb

@@ -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

lib/open_food_network/reports/rule.rb

@@ -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

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

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

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

spec/lib/open_food_network/reports/report_spec.rb

@@ -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

spec/lib/open_food_network/reports/row_spec.rb

@@ -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

spec/lib/open_food_network/reports/rule_spec.rb

@@ -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

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