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

Convert product, variant, stock item, prices to discard #2396

Merged
merged 6 commits into from
Jan 3, 2018
Merged
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 api/app/controllers/spree/api/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def update
def destroy
@product = find_product(params[:id])
authorize! :destroy, @product
@product.paranoia_destroy
@product.discard
respond_with(@product, status: 204)
end

Expand Down
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/stock_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def update

def destroy
@stock_item = Spree::StockItem.accessible_by(current_ability, :destroy).find(params[:id])
@stock_item.paranoia_destroy
@stock_item.discard
respond_with(@stock_item, status: 204)
end

Expand Down
2 changes: 1 addition & 1 deletion api/app/controllers/spree/api/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def create

def destroy
@variant = scope.accessible_by(current_ability, :destroy).find(params[:id])
@variant.paranoia_destroy
@variant.discard
respond_with(@variant, status: 204)
end

Expand Down
2 changes: 1 addition & 1 deletion api/spec/requests/spree/api/products_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ module Spree
specify do
get spree.api_product_path(product)
expect(json_response["slug"]).to match(/and-1-ways/)
product.paranoia_destroy
product.discard

get spree.api_product_path(other_product)
expect(json_response["slug"]).to match(/droids/)
Expand Down
2 changes: 1 addition & 1 deletion api/spec/requests/spree/api/shipments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@

it 'removes a destroyed variant from a shipment' do
order.contents.add(variant, 2)
variant.paranoia_destroy
variant.discard

put spree.remove_api_shipment_path(shipment), params: { variant_id: variant.to_param, quantity: 1 }
expect(response.status).to eq(200)
Expand Down
2 changes: 1 addition & 1 deletion backend/app/controllers/spree/admin/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def update

def destroy
@product = Spree::Product.friendly.find(params[:id])
@product.paranoia_destroy!
@product.discard

flash[:success] = t('spree.notice_messages.product_deleted')

Expand Down
4 changes: 3 additions & 1 deletion backend/app/controllers/spree/admin/resource_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,9 @@ def destroy
invoke_callbacks(:destroy, :before)

destroy_result =
if @object.respond_to?(:paranoia_destroy)
if @object.respond_to?(:discard)
@object.discard
elsif @object.respond_to?(:paranoia_destroy)
@object.paranoia_destroy
else
@object.destroy
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
# Regression test for https://github.com/spree/spree/issues/1903
context 'when soft deleted products exist' do
let!(:soft_deleted_product) { create(:product, sku: "ABC123") }
before { soft_deleted_product.paranoia_destroy }
before { soft_deleted_product.discard }

context 'when params[:q][:with_deleted] is not set' do
let(:params) { { q: {} } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ module Admin
end

context "with a deleted product" do
before { product.paranoia_destroy! }
before { product.discard }

it "is the product" do
subject
Expand All @@ -31,8 +31,8 @@ module Admin
let!(:variant) { create(:variant, product: product) }
let!(:deleted_variant) { create(:variant, product: product) }

context "with deleted variants" do
before { deleted_variant.paranoia_destroy! }
context "with soft-deleted variants" do
before { deleted_variant.discard }

context "when deleted is not requested" do
it "excludes deleted variants" do
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ def insufficient_stock_lines
# Check to see if any line item variants are soft, deleted.
# If so add error and restart checkout.
def ensure_line_item_variants_are_not_deleted
if line_items.any? { |li| li.variant.paranoia_destroyed? }
if line_items.any? { |li| li.variant.discarded? }
errors.add(:base, I18n.t('spree.deleted_variants_present'))
restart_checkout_flow
false
Expand Down
6 changes: 6 additions & 0 deletions core/app/models/spree/price.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
require 'discard'

module Spree
class Price < Spree::Base
acts_as_paranoid
include Spree::ParanoiaDeprecations

include Discard::Model
self.discard_column = :deleted_at

MAXIMUM_AMOUNT = BigDecimal('99_999_999.99')

Expand Down
15 changes: 15 additions & 0 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'discard'

module Spree
# Products represent an entity for sale in a store. Products can have
# variations, called variants. Product properties include description,
Expand All @@ -12,6 +14,18 @@ class Product < Spree::Base
friendly_id :slug_candidates, use: :history

acts_as_paranoid
include Spree::ParanoiaDeprecations

include Discard::Model
self.discard_column = :deleted_at

after_discard do
variants_including_master.discard_all
self.product_option_types = []
self.product_properties = []
self.classifications = []
self.product_promotion_rules = []
end

has_many :product_option_types, dependent: :destroy, inverse_of: :product
has_many :option_types, through: :product_option_types
Expand Down Expand Up @@ -90,6 +104,7 @@ def find_or_build_master
after_create :build_variants_from_option_values_hash, if: :option_values_hash

after_destroy :punch_slug
after_discard :punch_slug

after_initialize :ensure_master

Expand Down
6 changes: 6 additions & 0 deletions core/app/models/spree/stock_item.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
require 'discard'

module Spree
class StockItem < Spree::Base
acts_as_paranoid
include Spree::ParanoiaDeprecations

include Discard::Model
self.discard_column = :deleted_at

belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items
belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant', inverse_of: :stock_items
Expand Down
16 changes: 15 additions & 1 deletion core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
require 'discard'

module Spree
# == Master Variant
#
Expand All @@ -14,9 +16,21 @@ module Spree
# option values and may have inventory units. Sum of on_hand each variant's
# inventory level determine "on_hand" level for the product.
class Variant < Spree::Base
acts_as_paranoid
acts_as_list scope: :product

acts_as_paranoid
include Spree::ParanoiaDeprecations

include Discard::Model
self.discard_column = :deleted_at

after_discard do
stock_items.discard_all
images.destroy_all
prices.discard_all
currently_valid_prices.discard_all
end

attr_writer :rebuild_vat_prices
include Spree::DefaultPrice

Expand Down
1 change: 1 addition & 0 deletions core/lib/spree/core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
require 'state_machines-activerecord'

require 'spree/deprecation'
require 'spree/paranoia_deprecations'

# This is required because ActiveModel::Validations#invalid? conflicts with the
# invalid state of a Payment. In the future this should be removed.
Expand Down
19 changes: 19 additions & 0 deletions core/lib/spree/paranoia_deprecations.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
module Spree
module ParanoiaDeprecations
def paranoia_destroy
Spree::Deprecation.warn <<-WARN.strip_heredoc, caller
Calling #destroy (or #paranoia_destroy) on a #{self.class} currently performs a soft-destroy using the paranoia gem.
In Solidus 3.0, paranoia will be removed, and this will perform a HARD destroy instead. To continue soft-deleting, use #discard instead.
WARN
super
end

def paranoia_delete
Spree::Deprecation.warn <<-WARN.strip_heredoc, caller
Calling #delete (or #paranoia_delete) on a #{self.class} currently performs a soft-destroy using the paranoia gem.
In Solidus 3.0, paranoia will be removed, and this will perform a HARD destroy instead. To continue soft-deleting, use #discard instead.
WARN
super
end
end
end
1 change: 1 addition & 0 deletions core/solidus_core.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ Gem::Specification.new do |s|
s.add_dependency 'paranoia', '~> 2.4'
s.add_dependency 'ransack', '~> 1.8'
s.add_dependency 'state_machines-activerecord', '~> 0.4'
s.add_dependency 'discard'
end
2 changes: 1 addition & 1 deletion core/spec/lib/spree/core/importer/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ module Core

context 'variant was soft-deleted' do
it 'raise error as variant shouldnt be found' do
variant.product.paranoia_destroy
variant.product.discard
hash = { sku: variant.sku }
expect {
Importer::Order.ensure_variant_id_from_params(hash)
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/customer_return_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@
end

it "should NOT raise an error when a soft-deleted stock item exists in the stock location" do
inventory_unit.find_stock_item.paranoia_destroy
inventory_unit.find_stock_item.discard
create(:customer_return_without_return_items, return_items: [return_item], stock_location_id: new_stock_location.id)
end

Expand Down
6 changes: 3 additions & 3 deletions core/spec/models/spree/inventory_unit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,17 @@
end
end

context "variants deleted" do
context "variants discarded" do
let!(:unit) { create(:inventory_unit) }

it "can still fetch variant" do
unit.variant.destroy
unit.variant.discard
expect(unit.reload.variant).to be_a Spree::Variant
end

it "can still fetch variants by eager loading (remove default_scope)" do
skip "find a way to remove default scope when eager loading associations"
unit.variant.destroy
unit.variant.discard
expect(Spree::InventoryUnit.joins(:variant).includes(:variant).first.variant).to be_a Spree::Variant
end
end
Expand Down
8 changes: 4 additions & 4 deletions core/spec/models/spree/line_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
let(:line_item) { order.line_items.first }

context '#destroy' do
it "fetches deleted products" do
line_item.product.paranoia_destroy
it "fetches soft-deleted products" do
line_item.product.discard
expect(line_item.reload.product).to be_a Spree::Product
end

it "fetches deleted variants" do
line_item.variant.paranoia_destroy
it "fetches soft-deleted variants" do
line_item.variant.discard
expect(line_item.reload.variant).to be_a Spree::Variant
end

Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
context 'when variant is destroyed' do
before do
allow(order).to receive(:restart_checkout_flow)
order.line_items.first.variant.paranoia_destroy
order.line_items.first.variant.discard
end

it 'should restart checkout flow' do
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/product/scopes_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@
end

context "with soft-deleted master price" do
before { product.master.prices.each(&:paranoia_destroy!) }
before { product.master.prices.discard_all }

it "doesn't include the product" do
expect(Spree::Product.available).to match_array([])
Expand Down
Loading