Skip to content

Commit

Permalink
Merge pull request #2396 from jhawthorn/discard_products
Browse files Browse the repository at this point in the history
Convert product, variant, stock item, prices to discard
  • Loading branch information
jhawthorn authored Jan 3, 2018
2 parents 4cc17e5 + b6e8236 commit f1ffcc7
Show file tree
Hide file tree
Showing 30 changed files with 182 additions and 54 deletions.
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

0 comments on commit f1ffcc7

Please sign in to comment.