raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-01-24 20:36:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #3394 from mkllnk/3021-update-soft-delete

Browse Source 
[Spree upgrade] 3021 update soft delete
This commit is contained in:
Pau Pérez Fabregat
2019-02-25 11:58:41 +01:00
committed by GitHub
parent 439ea618df 8374c7dea4
commit aed9c41b0d
18 changed files with 67 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
app/controllers/spree/admin/products_controller_decorator.rb
View File

@@ -78,8 +78,13 @@ Spree::Admin::ProductsController.class_eval do
params[:q][:deleted_at_null] ||= "1"
params[:q][:s] ||= "name asc"
@search = Spree::Product.ransack(params[:q]) # this line is modified - hit Spree::Product instead of super, avoiding cancan error for fetching records with block permissions via accessible_by
# The next line is modified.
# Hit Spree::Product instead of super, avoiding cancan error for fetching
# records with block permissions via accessible_by.
@collection = Spree::Product
@collection = @collection.with_deleted if params[:q].delete(:deleted_at_null).blank?
# @search needs to be defined as this is passed to search_form_for
@search = @collection.ransack(params[:q])
@collection = @search.result.
managed_by(spree_current_user). # this line is added to the original spree code!!!!!
group_by_products_id.

7
app/controllers/spree/admin/variants_controller_decorator.rb
View File

@@ -10,8 +10,11 @@ Spree::Admin::VariantsController.class_eval do
def destroy
@variant = Spree::Variant.find(params[:id])
@variant.delete # This line changed, as well as removal of following conditional
flash[:success] = I18n.t('notice_messages.variant_deleted')
if VariantDeleter.new.delete(@variant) # This line changed
flash[:success] = Spree.t('notice_messages.variant_deleted')
else
flash[:success] = Spree.t('notice_messages.variant_not_deleted')
end
respond_with(@variant) do |format|
format.html { redirect_to admin_product_variants_url(params[:product_id]) }

6
app/controllers/spree/api/products_controller_decorator.rb
View File

@@ -33,7 +33,7 @@ Spree::Api::ProductsController.class_eval do
authorize! :delete, Spree::Product
@product = find_product(params[:product_id])
authorize! :delete, @product
@product.delete
@product.destroy
respond_with(@product, :status => 204)
end
@@ -56,8 +56,8 @@ Spree::Api::ProductsController.class_eval do
def product_scope
if current_api_user.has_spree_role?("admin") || current_api_user.enterprises.present? # This line modified
scope = Spree::Product
unless params[:show_deleted]
scope = scope.not_deleted
if params[:show_deleted]
scope = scope.with_deleted
end
else
scope = Spree::Product.active

2
app/controllers/spree/api/variants_controller_decorator.rb
View File

@@ -3,7 +3,7 @@ Spree::Api::VariantsController.class_eval do
@variant = scope.find(params[:variant_id])
authorize! :delete, @variant
@variant.delete
VariantDeleter.new.delete(@variant)
respond_with @variant, status: 204
end
end

4
app/models/order_cycle.rb
View File

@@ -146,7 +146,6 @@ class OrderCycle < ActiveRecord::Base
Spree::Variant.
joins(:exchanges).
merge(Exchange.in_order_cycle(self)).
not_deleted.
select('DISTINCT spree_variants.*').
to_a # http://stackoverflow.com/q/15110166
end
@@ -163,9 +162,8 @@ class OrderCycle < ActiveRecord::Base
def variants_distributed_by(distributor)
return Spree::Variant.where("1=0") unless distributor.present?
Spree::Variant.
not_deleted.
merge(distributor.inventory_variants).
joins(:exchanges).
merge(distributor.inventory_variants).
merge(Exchange.in_order_cycle(self)).
merge(Exchange.outgoing).
merge(Exchange.to_enterprise(distributor))

1
app/models/product_import/entry_processor.rb
View File

@@ -49,7 +49,6 @@ module ProductImport
VariantOverride.for_hubs([enterprise_id]).count
else
Spree::Variant.
not_deleted.
not_master.
joins(:product).
where('spree_products.supplier_id IN (?)', enterprise_id).

8
app/models/spree/product_decorator.rb
View File

@@ -202,20 +202,18 @@ Spree::Product.class_eval do
end
end
def delete_with_delete_from_order_cycles
def destroy_with_delete_from_order_cycles
transaction do
OpenFoodNetwork::ProductsCache.product_deleted(self) do
# Touch supplier and distributors as we would on #destroy
self.supplier.touch
touch_distributors
ExchangeVariant.where('exchange_variants.variant_id IN (?)', self.variants_including_master.with_deleted).destroy_all
delete_without_delete_from_order_cycles
destroy_without_delete_from_order_cycles
end
end
end
alias_method_chain :delete, :delete_from_order_cycles
alias_method_chain :destroy, :delete_from_order_cycles
def refresh_products_cache

14
app/models/spree/variant_decorator.rb
View File

@@ -35,7 +35,6 @@ Spree::Variant.class_eval do
scope :with_order_cycles_inner, joins(exchanges: :order_cycle)
scope :not_deleted, where(deleted_at: nil)
scope :not_master, where(is_master: false)
scope :in_order_cycle, lambda { |order_cycle|
with_order_cycles_inner.
@@ -105,19 +104,6 @@ Spree::Variant.class_eval do
OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for self
end
def delete
if product.variants == [self] # Only variant left on product
errors.add :product, I18n.t(:spree_variant_product_error)
false
else
transaction do
self.update_column(:deleted_at, Time.zone.now)
ExchangeVariant.where(variant_id: self).destroy_all
self
end
end
end
def refresh_products_cache
if is_master?
product.refresh_products_cache

4
app/serializers/api/admin/for_order_cycle/enterprise_serializer.rb
View File

@@ -31,9 +31,9 @@ class Api::Admin::ForOrderCycle::EnterpriseSerializer < ActiveModel::Serializer
def products
return @products unless @products.nil?
@products = if order_cycle.prefers_product_selection_from_coordinator_inventory_only?
object.supplied_products.not_deleted.visible_for(order_cycle.coordinator)
object.supplied_products.visible_for(order_cycle.coordinator)
else
object.supplied_products.not_deleted
object.supplied_products
end
end

17
app/services/variant_deleter.rb Normal file
View File

@@ -0,0 +1,17 @@
# Checks the validity of a soft-delete call.
class VariantDeleter
def delete(variant)
if only_variant_on_product?(variant)
variant.errors.add :product, I18n.t(:spree_variant_product_error)
return false
end
variant.destroy
end
private
def only_variant_on_product?(variant)
variant.product.variants == [variant]
end
end

6
lib/open_food_network/products_and_inventory_report_base.rb
View File

@@ -31,11 +31,7 @@ module OpenFoodNetwork
end
def filter(variants)
filter_on_hand filter_to_distributor filter_to_order_cycle filter_to_supplier filter_not_deleted variants
end
def filter_not_deleted(variants)
variants.not_deleted
filter_on_hand filter_to_distributor filter_to_order_cycle filter_to_supplier variants
end
# Using the `in_stock?` method allows overrides by distributors.

10
spec/controllers/spree/api/variants_controller_spec.rb
View File

@@ -69,6 +69,16 @@ module Spree
lambda { variant.reload }.should_not raise_error
variant.deleted_at.should_not be_nil
end
it "doesn't delete the only variant of the product" do
product = create(:product)
variant = product.variants.first
spree_delete :soft_delete, {variant_id: variant.to_param, product_id: product.to_param, format: :json}
expect(variant.reload).to_not be_deleted
expect(assigns(:variant).errors[:product]).to include "must have at least one variant"
end
end
end
end

4
spec/lib/open_food_network/products_and_inventory_report_spec.rb
View File

@@ -1,7 +1,7 @@
require 'spec_helper'
module OpenFoodNetwork
xdescribe ProductsAndInventoryReport do
describe ProductsAndInventoryReport do
context "As a site admin" do
let(:user) do
user = create(:user)
@@ -105,7 +105,7 @@ module OpenFoodNetwork
it "should filter deleted products" do
product1 = create(:simple_product, supplier: supplier)
product2 = create(:simple_product, supplier: supplier)
product2.delete
product2.destroy
subject.filter(Spree::Variant.scoped).should match_array [product1.master, product1.variants.first]
end
describe "based on report type" do

4
spec/lib/open_food_network/products_cache_spec.rb
View File

@@ -98,7 +98,7 @@ module OpenFoodNetwork
it "refreshes the cache based on exchanges the variant was in before destruction" do
expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc)
product.delete
product.destroy
end
it "performs the cache refresh after the product has been removed from the order cycle" do
@@ -106,7 +106,7 @@ module OpenFoodNetwork
expect(product.reload.deleted_at).not_to be_nil
end
product.delete
product.destroy
end
end

23
spec/models/spree/product_spec.rb
View File

@@ -55,15 +55,6 @@ module Spree
end
end
it "does not allow the last variant to be deleted" do
product = create(:simple_product)
expect(product.variants(:reload).length).to eq 1
v = product.variants.last
v.delete
expect(v.errors[:product]).to include "must have at least one variant"
end
context "when the product has variants" do
let(:product) do
product = create(:simple_product)
@@ -172,7 +163,7 @@ module Spree
it "refreshes the products cache on delete" do
expect(OpenFoodNetwork::ProductsCache).to receive(:product_deleted).with(product)
product.delete
product.destroy
end
# On destroy, all distributed variants are refreshed by a Variant around_destroy
@@ -185,11 +176,15 @@ module Spree
let!(:oc) { create(:simple_order_cycle, distributors: [distributor], variants: [product.variants.first]) }
it "touches the supplier" do
expect { product.delete }.to change { supplier.reload.updated_at }
expect { product.destroy }.to change { supplier.reload.updated_at }
end
it "touches all distributors" do
expect { product.delete }.to change { distributor.reload.updated_at }
expect { product.destroy }.to change { distributor.reload.updated_at }
end
it "removes variants from order cycles" do
expect { product.destroy }.to change { ExchangeVariant.count }
end
end
@@ -701,13 +696,13 @@ module Spree
it "removes the master variant from all order cycles" do
e.variants << p.master
p.delete
p.destroy
e.variants(true).should be_empty
end
it "removes all other variants from order cycles" do
e.variants << v
p.delete
p.destroy
e.variants(true).should be_empty
end
end

27
spec/models/spree/variant_spec.rb
View File

@@ -13,16 +13,6 @@ module Spree
end
describe "scopes" do
it "finds non-deleted variants" do
v_not_deleted = create(:variant)
v_deleted = create(:variant)
v_deleted.deleted_at = Time.zone.now
v_deleted.save
Spree::Variant.not_deleted.should include v_not_deleted
Spree::Variant.not_deleted.should_not include v_deleted
end
describe "finding variants in a distributor" do
let!(:d1) { create(:distributor_enterprise) }
let!(:d2) { create(:distributor_enterprise) }
@@ -535,22 +525,5 @@ module Spree
v.destroy
e.reload.variant_ids.should be_empty
end
context "as the last variant of a product" do
let!(:extra_variant) { create(:variant) }
let!(:product) { extra_variant.product }
let!(:first_variant) { product.variants.first }
before { product.reload }
it "cannot be deleted" do
expect(product.variants.length).to eq 2
expect(extra_variant.delete).to eq extra_variant
expect(product.variants(:reload).length).to eq 1
expect(first_variant.delete).to be false
expect(product.variants(:reload).length).to eq 1
expect(first_variant.errors[:product]).to include "must have at least one variant"
end
end
end
end

2
spec/requests/shop_spec.rb
View File

@@ -24,7 +24,7 @@ describe "Shop API", type: :request do
set_order order
v61.update_attribute(:count_on_hand, 1)
p6.delete
p6.destroy
v71.update_attribute(:count_on_hand, 1)
v41.update_attribute(:count_on_hand, 1)
v42.update_attribute(:count_on_hand, 0)

4
spec/serializers/admin/for_order_cycle/enterprise_serializer_spec.rb
View File

@@ -1,3 +1,5 @@
require "spec_helper"
describe Api::Admin::ForOrderCycle::EnterpriseSerializer do
let(:coordinator) { create(:distributor_enterprise) }
let(:order_cycle) { double(:order_cycle, coordinator: coordinator) }
@@ -14,7 +16,7 @@ describe Api::Admin::ForOrderCycle::EnterpriseSerializer do
let(:serialized_enterprise) do
Api::Admin::ForOrderCycle::EnterpriseSerializer.new(
enterprise,
enterprise.reload, # load the products that were created after the enterprise
order_cycle: order_cycle,
spree_current_user: enterprise.owner
).to_json