Merge pull request #55 from cpfergus1/solidus-3-preparation

Solidus 3 preparation

Gemfile

@@ -6,11 +6,6 @@ git_source(:github) { |repo| "https://github.com/#{repo}.git" }
 branch = ENV.fetch('SOLIDUS_BRANCH', 'master')
 gem 'solidus', github: 'solidusio/solidus', branch: branch
 
-# Needed to help Bundler figure out how to resolve dependencies,
-# otherwise it takes forever to resolve them.
-# See https://github.com/bundler/bundler/issues/6677
-gem 'rails', '>0.a'
-
 # Provides basic authentication functionality for testing parts of your engine
 gem 'solidus_auth_devise'
 

app/controllers/spree/admin/sale_prices_controller.rb

@@ -22,7 +22,7 @@ def create
 
       def destroy
         @sale_price = Spree::SalePrice.find(params[:id])
-        @sale_price.destroy
+        @sale_price.discard
         render_js_for_destroy
       end
 

app/decorators/models/solidus_sale_prices/spree/price_decorator.rb

@@ -5,6 +5,9 @@ def self.prepended(base)
         base.has_many :sale_prices, dependent: :destroy
         base.has_many :active_sale_prices, -> { merge(::Spree::SalePrice.active) }, class_name: '::Spree::SalePrice'
         base.after_save :update_calculated_sale_prices
+        base.after_discard do
+          sale_prices.discard_all
+        end
       end
 
       def update_calculated_sale_prices

app/decorators/models/solidus_sale_prices/spree/variant_decorator.rb

@@ -23,29 +23,29 @@ def put_on_sale(value, params = {})
       # TODO make update_sale method
 
       def active_sale_in(currency)
-        price_in(currency).active_sale
+        SolidusSalePrices::PriceMethod.price_for_options(self, currency).active_sale
       end
       alias :current_sale_in :active_sale_in
 
       def next_active_sale_in(currency)
-        price_in(currency).next_active_sale
+        SolidusSalePrices::PriceMethod.price_for_options(self, currency).next_active_sale
       end
       alias :next_current_sale_in :next_active_sale_in
 
       def sale_price_in(currency)
-        ::Spree::Price.new variant_id: self.id, currency: currency, amount: price_in(currency).sale_price
+        ::Spree::Price.new variant_id: self.id, currency: currency, amount: SolidusSalePrices::PriceMethod.price_for_options(self, currency).sale_price
       end
 
       def discount_percent_in(currency)
-        price_in(currency).discount_percent
+        SolidusSalePrices::PriceMethod.price_for_options(self, currency).discount_percent
       end
 
       def on_sale_in?(currency)
-        price_in(currency).on_sale?
+        SolidusSalePrices::PriceMethod.price_for_options(self, currency).on_sale?
       end
 
       def original_price_in(currency)
-        ::Spree::Price.new variant_id: self.id, currency: currency, amount: price_in(currency).original_price
+        ::Spree::Price.new variant_id: self.id, currency: currency, amount: SolidusSalePrices::PriceMethod.price_for_options(self, currency).original_price
       end
 
       def enable_sale(currencies = nil)

app/helpers/solidus_sale_prices/price_method.rb

@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+# This module implements functionality found in Solidus 3.1 as an intermediate bridge between earlier versions of Solidus.
+# This should updated and deprecated as Solidus versions 2.11, and 3.0 are no longer supported.
+module SolidusSalePrices
+  module PriceMethod
+    def self.price_for_options(variant, currency, country_iso = nil)
+      options = ::Spree::Config.pricing_options_class.new(currency: currency,
+        country_iso: country_iso)
+      price_search(variant, options)
+    end
+
+    def self.price_search(variant, options)
+      variant.currently_valid_prices.detect do |price|
+        (price.country_iso == options.desired_attributes[:country_iso] ||
+         price.country_iso.nil?
+        ) && price.currency == options.desired_attributes[:currency]
+      end
+    end
+  end
+end

app/models/spree/sale_price.rb

@@ -1,9 +1,20 @@
 module Spree
   class SalePrice < ActiveRecord::Base
+    # The following code enables soft-deletion. In Solidus v2.11+ there is a mixin
+    # that handles soft-deletion consistently across all Solidus records that need
+    # it. Once we no longer support v2.10, we can remove the dependency on paranoia
+    # and replace these lines with:
+    #
+    #     include Spree::SoftDeletable
+    #
+    # However, this will be a breaking change as it will change the behaviour of
+    # calling `destroy` on these records.
     acts_as_paranoid
+    include Discard::Model
+    self.discard_column = :deleted_at
 
     belongs_to :price, class_name: "Spree::Price", touch: true
-    belongs_to :price_with_deleted, -> { with_deleted }, class_name: "Spree::Price", foreign_key: :price_id
+    belongs_to :price_with_deleted, -> { with_discarded }, class_name: "Spree::Price", foreign_key: :price_id
 
     delegate :currency, :currency=, to: :price, allow_nil: true
 
@@ -16,7 +27,7 @@ class SalePrice < ActiveRecord::Base
 
     before_save :compute_calculated_price
 
-    scope :ordered, -> { order('start_at IS NOT NULL, start_at ASC') }
+    scope :ordered, -> { order(Arel.sql('start_at IS NOT NULL, start_at ASC')) }
     scope :active, -> { where(enabled: true).where('(start_at <= ? OR start_at IS NULL) AND (end_at >= ? OR end_at IS NULL)', Time.now, Time.now) }
 
     # TODO make this work or remove it
@@ -49,11 +60,11 @@ def start(end_time = nil)
       end_time = nil if end_time.present? && end_time <= Time.now # if end_time is not in the future then make it nil (no end)
       attr = { end_at: end_time, enabled: true }
       attr[:start_at] = Time.now if self.start_at.present? && self.start_at > Time.now # only set start_at if it's not set in the past
-      update_attributes(attr)
+      update(attr)
     end
 
     def stop
-      update_attributes({ end_at: Time.now, enabled: false })
+      update({ end_at: Time.now, enabled: false })
     end
 
     # Convenience method for displaying the price of a given sale_price in the table

app/views/spree/admin/sale_prices/_prices_table.html.erb

@@ -14,7 +14,7 @@
         </thead>
         <tbody>
           <% prices.each do |price| %>
-            <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.deleted? %>">
+            <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.discarded? %>">
               <td><%= check_box_tag 'price_ids[]', price.id %></td>
               <td><%= price.variant.descriptive_name %></td>
               <td><%= price.display_country %></td>

app/views/spree/admin/sale_prices/_table.html.erb

@@ -16,7 +16,7 @@
       </thead>
       <tbody>
         <% sale_prices.each do |price| %>
-          <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.deleted? %>">
+          <tr id="<%= spree_dom_id price %>" data-hook="prices_row" class="<%= "deleted" if price.discarded? %>">
             <td>
               <% if price.start_at&.future? %>
                 <span class="pill pill-warning"><%=t 'spree.sale_price_scheduled' %></span>

lib/solidus_sale_prices/engine.rb

@@ -4,7 +4,7 @@
 
 module SolidusSalePrices
   class Engine < Rails::Engine
-    include SolidusSupport::EngineExtensions::Decorators
+    include SolidusSupport::EngineExtensions
 
     isolate_namespace ::Spree
 

lib/solidus_sale_prices/factories.rb → lib/solidus_sale_prices/testing_support/factories.rb

@@ -36,6 +36,7 @@
     after(:create) do |variant, _evaluator|
       create(:eur_price, variant: variant)
       create(:usd_price, variant: variant)
+      variant.reload
     end
   end
 end

solidus_sale_prices.gemspec

@@ -19,7 +19,7 @@ Gem::Specification.new do |s|
     s.metadata["source_code_uri"] = s.homepage if s.homepage
   end
 
-  s.required_ruby_version = '~> 2.4'
+  s.required_ruby_version = Gem::Requirement.new('>= 2.5')
 
   s.files = Dir.chdir(File.expand_path(__dir__)) do
     `git ls-files -z`.split("\x0").reject { |f| f.match(%r{^(test|spec|features)/}) }
@@ -29,9 +29,10 @@ Gem::Specification.new do |s|
   s.executables = s.files.grep(%r{^exe/}) { |f| File.basename(f) }
   s.require_paths = ["lib"]
 
-  solidus_version = [">= 1.0", "< 3"]
+  solidus_version = [">= 1.0", "< 4"]
 
   s.add_dependency 'deface', '~> 1.0'
+  s.add_dependency 'discard'
   s.add_dependency 'solidus_api', solidus_version
   s.add_dependency 'solidus_backend', solidus_version
   s.add_dependency 'solidus_core', solidus_version

spec/features/admin/sale_prices_spec.rb

@@ -80,12 +80,12 @@
       fill_in('sale_price_value', with: 32.33)
 
       click_button('Create')
-
+      
       within all('[data-hook="sale_prices"] [data-hook="prices_row"]').last do
         expect(page).to have_content(variant.descriptive_name)
         expect(page).to have_content(32.33)
         expect(page).to have_content(country.name)
-        expect(page).to have_content(variant.prices.last.currency)
+        expect(page).to have_content("EUR")
         expect(page).to have_content(start_at.strftime('%B %d, %Y'))
         expect(page).to have_content(end_at.strftime('%B %d, %Y'))
       end
@@ -97,7 +97,7 @@
 
     within('[data-hook="sale_prices"]') do
       expect(page).to have_selector('[data-hook="prices_row"]', count: 1)
-      find('.delete-resource').click
+      accept_alert { find('.delete-resource').click }
       expect(page).to have_selector('[data-hook="prices_row"]', count: 0)
     end
   end

spec/models/price_spec.rb

@@ -11,7 +11,7 @@
 
     it 'builds a new sale' do
       is_expected.to have_attributes({
-        value: BigDecimal.new(sale_price_value, 4),
+        value: BigDecimal(sale_price_value, 4),
         start_at: be_within(1.second).of(Time.now),
         end_at: nil,
         enabled: true,
@@ -32,7 +32,7 @@
       end
 
       it "updates the price's price" do
-        expect { put_on_sale }.to change { price.reload.price }.from(price_amount).to(BigDecimal.new(sale_price_value, 4))
+        expect { put_on_sale }.to change { price.reload.price }.from(price_amount).to(BigDecimal(sale_price_value, 4))
       end
 
       it "sets original_price" do
@@ -89,7 +89,7 @@
       before { price.put_on_sale 10 }
 
       it 'destroys all sale prices when it is destroyed' do
-        expect { price.destroy }
+        expect { price.discard }
           .to change { Spree::SalePrice.all.size }
           .from(1).to(0)
       end

spec/models/sale_price_spec.rb

@@ -39,7 +39,7 @@
     end
 
     it 'sets the end time' do
-      expect(sale_price.end_at).to be_within(1.second).of(Time.now)
+      expect(sale_price.end_at).to be_within(2.second).of(Time.now)
     end
   end
 
@@ -66,7 +66,7 @@
       let(:price) { sale_price.price }
 
       before do
-        price.destroy
+        price.discard
         sale_price.reload
       end
 
@@ -81,18 +81,18 @@
     context 'when the price has been soft-deleted' do
       before do
         sale = create :sale_price
-        sale.price.destroy
+        sale.price.discard
       end
 
       it 'preloads the variant via SQL also for soft-deleted records' do
-        records = Spree::SalePrice.with_deleted.includes(:variant)
+        records = Spree::SalePrice.with_discarded.includes(:variant)
         expect(records.first.variant).to be_present
       end
     end
   end
 
   context 'touching associated product when destroyed' do
-    subject { -> { sale_price.reload.destroy } }
+    subject { -> { sale_price.reload.discard } }
     let!(:product) { sale_price.product }
     let(:sale_price) { Timecop.travel(1.day.ago) { create(:sale_price) } }
 
@@ -107,15 +107,15 @@
     end
 
     context 'when associated variant has been destroyed' do
-      before { sale_price.variant.destroy }
+      before { sale_price.variant.discard }
 
       it 'does not touch product' do
         expect(subject).not_to change { product.reload.updated_at }
       end
     end
 
     context 'when associated price has been destroyed' do
-      before { sale_price.price.destroy }
+      before { sale_price.price.discard }
 
       it 'does not touch product' do
         expect(subject).not_to change { product.reload.updated_at }