Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare for solidus 3.0 #52

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/spree/admin/sale_prices_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions app/models/spree/sale_price.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
module Spree
class SalePrice < ActiveRecord::Base
acts_as_paranoid
include Spree::SoftDeletable

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

Expand All @@ -16,7 +16,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
Expand Down Expand Up @@ -49,11 +49,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
Expand Down
2 changes: 1 addition & 1 deletion app/views/spree/admin/sale_prices/_prices_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion app/views/spree/admin/sale_prices/_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion solidus_sale_prices.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '~> 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)/}) }
Expand Down
2 changes: 1 addition & 1 deletion spec/features/admin/sale_prices_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions spec/models/price_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 6 additions & 6 deletions spec/models/sale_price_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
let(:price) { sale_price.price }

before do
price.destroy
price.discard
sale_price.reload
end

Expand All @@ -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) } }

Expand All @@ -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 }
Expand Down
20 changes: 10 additions & 10 deletions spec/models/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

expect(variant.prices.count).not_to eql 0
variant.prices.each do |p|
expect(p.price).to eql BigDecimal.new(10.95, 4)
expect(p.price).to eql BigDecimal(10.95, 4)
end
end

Expand All @@ -29,8 +29,8 @@
variant.prices.each do |p|
variant.put_on_sale 10.95, { currencies: [ p.currency ] }

expect(variant.price_in(p.currency).price).to eq BigDecimal.new(10.95, 4)
expect(variant.original_price_in(p.currency).price).to eql BigDecimal.new(19.99, 4)
expect(variant.price_in(p.currency).price).to eq BigDecimal(10.95, 4)
expect(variant.original_price_in(p.currency).price).to eql BigDecimal(19.99, 4)
end
end

Expand All @@ -46,7 +46,7 @@

some_prices.each do |p|
expect(variant.price_in(p.currency).price).to be_within(0.01).of(10.95)
expect(variant.original_price_in(p.currency).price).to eql BigDecimal.new(19.99, 4)
expect(variant.original_price_in(p.currency).price).to eql BigDecimal(19.99, 4)
end
end

Expand All @@ -59,9 +59,9 @@

variant.prices.each do |p|
expect(p.on_sale?).to be true
expect(p.price).to eq BigDecimal.new(10.95, 4)
expect(p.sale_price).to eq BigDecimal.new(10.95, 4)
expect(p.original_price).to eq BigDecimal.new(12.90, 4)
expect(p.price).to eq BigDecimal(10.95, 4)
expect(p.sale_price).to eq BigDecimal(10.95, 4)
expect(p.original_price).to eq BigDecimal(12.90, 4)
end
end

Expand All @@ -74,9 +74,9 @@

variant.prices.each do |p|
expect(p.on_sale?).to be false
expect(p.price).to eq BigDecimal.new(9.90, 4)
expect(p.price).to eq BigDecimal(9.90, 4)
expect(p.sale_price).to eq nil
expect(p.original_price).to eq BigDecimal.new(9.90, 4)
expect(p.original_price).to eq BigDecimal(9.90, 4)
end
end

Expand Down Expand Up @@ -149,7 +149,7 @@
end

it 'destroys all sale prices when it is destroyed' do
expect { @variant.destroy }
expect { @variant.discard }
.to change { Spree::SalePrice.all.size }
.from(3).to(0)
end
Expand Down