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

Replace Paranoia methods with Discard equivalents #3554

Merged
merged 3 commits into from
Mar 18, 2020
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/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def find_product(id)

def product_scope
if can?(:admin, Spree::Product)
scope = Spree::Product.with_deleted.accessible_by(current_ability, :read).includes(*product_includes)
scope = Spree::Product.with_discarded.accessible_by(current_ability, :read).includes(*product_includes)

unless params[:show_deleted]
scope = scope.not_deleted
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 @@ -64,7 +64,7 @@ def scope
end

if current_ability.can?(:manage, Variant) && params[:show_deleted]
variants = variants.with_deleted
variants = variants.with_discarded
end

in_stock_only = ActiveRecord::Type::Boolean.new.cast(params[:in_stock_only])
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 @@ -78,7 +78,7 @@ def split_params
end

def find_resource
Spree::Product.with_deleted.friendly.find(params[:id])
Spree::Product.with_discarded.friendly.find(params[:id])
end

def location_after_save
Expand Down
4 changes: 2 additions & 2 deletions backend/app/controllers/spree/admin/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def new_before

def collection
if params[:deleted] == "on"
base_variant_scope ||= super.with_deleted
base_variant_scope ||= super.with_discarded
else
base_variant_scope ||= super
end
Expand All @@ -43,7 +43,7 @@ def redirect_on_empty_option_values
end

def parent
@parent ||= Spree::Product.with_deleted.find_by(slug: params[:product_id])
@parent ||= Spree::Product.with_discarded.find_by(slug: params[:product_id])
@product = @parent
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@
</tr>
</thead>
<% master_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><%= price.display_country %></td>
<td><%= price.currency %></td>
<td><%= price.money.to_html %></td>
<td class="actions">
<% if can?(:update, price) %>
<%= link_to_edit(price, no_text: true) unless price.deleted? %>
<%= link_to_edit(price, no_text: true) unless price.discarded? %>
<% end %>
<% if can?(:destroy, price) %>
&nbsp;
<%= link_to_delete(price, no_text: true) unless price.deleted? %>
<%= link_to_delete(price, no_text: true) unless price.discarded? %>
<% end %>
</td>
</tr>
Expand Down
6 changes: 3 additions & 3 deletions backend/app/views/spree/admin/prices/_table.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@
</thead>
<tbody>
<% variant_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><%= price.variant.descriptive_name %></td>
<td><%= price.display_country %></td>
<td><%= price.currency %></td>
<td><%= price.money.to_html %></td>
<td class="actions">
<% if can?(:update, price) %>
<%= link_to_edit(price, no_text: true) unless price.deleted? %>
<%= link_to_edit(price, no_text: true) unless price.discarded? %>
<% end %>
<% if can?(:destroy, price) %>
&nbsp;
<%= link_to_delete(price, no_text: true) unless price.deleted? %>
<%= link_to_delete(price, no_text: true) unless price.discarded? %>
<% end %>
</td>
</tr>
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/products/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<div class="col-2">
<div class="field checkbox">
<label>
<%= f.check_box :with_deleted, { checked: params[:q][:with_deleted] == 'true' }, 'true', 'false' %>
<%= f.check_box :with_discarded, { checked: params[:q][:with_discarded] == 'true' }, 'true', 'false' %>
<%= t('spree.show_deleted') %>
</label>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
</div>
</div>

<% if product.variants.only_deleted.any? %>
<% if product.variants.discarded.any? %>
<div class="col-2">
<div class="field checkbox">
<label>
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/variants/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
<% end %>
<% end %>

<% if @variants.any? || @product.variants.only_deleted.any?%>
<% if @variants.any? || @product.variants.discarded.any? %>
<%= render "table_filter", product: @product %>
<%= render "table", variants: @variants %>
<% else %>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
let!(:soft_deleted_product) { create(:product, sku: "ABC123") }
before { soft_deleted_product.discard }

context 'when params[:q][:with_deleted] is not set' do
context 'when params[:q][:with_discarded] is not set' do
let(:params) { { q: {} } }

it 'filters out soft-deleted products by default' do
Expand All @@ -30,8 +30,8 @@
end
end

context 'when params[:q][:with_deleted] is set to "true"' do
let(:params) { { q: { with_deleted: 'true' } } }
context 'when params[:q][:with_discarded] is set to "true"' do
let(:params) { { q: { with_discarded: 'true' } } }

it 'includes soft-deleted products' do
get :index, params: params
Expand Down
2 changes: 1 addition & 1 deletion backend/spec/features/admin/products/products_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ def build_option_type_with_values(name, values)
expect(page).not_to have_content("apache baseball cap")
check "Show Deleted"
click_button 'Search'
expect(find('input[name="q[with_deleted]"]')).to be_checked
expect(find('input[name="q[with_discarded]"]')).to be_checked
expect(page).to have_content("zomg shirt")
expect(page).to have_content("apache baseball cap")
uncheck "Show Deleted"
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/concerns/spree/default_price.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module DefaultPrice

included do
has_one :default_price,
-> { with_deleted.currently_valid.with_default_attributes },
-> { with_discarded.currently_valid.with_default_attributes },
class_name: 'Spree::Price',
inverse_of: :variant,
dependent: :destroy,
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/inventory_unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ class InventoryUnit < Spree::Base
POST_SHIPMENT_STATES = %w(returned)
CANCELABLE_STATES = ['on_hand', 'backordered', 'shipped']

belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant", inverse_of: :inventory_units, optional: true
belongs_to :variant, -> { with_discarded }, class_name: "Spree::Variant", inverse_of: :inventory_units, optional: true
belongs_to :shipment, class_name: "Spree::Shipment", touch: true, inverse_of: :inventory_units, optional: true
belongs_to :carton, class_name: "Spree::Carton", inverse_of: :inventory_units, optional: true
belongs_to :line_item, class_name: "Spree::LineItem", inverse_of: :inventory_units, optional: true
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class LineItem < Spree::Base
class CurrencyMismatch < StandardError; end

belongs_to :order, class_name: "Spree::Order", inverse_of: :line_items, touch: true, optional: true
belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant", inverse_of: :line_items, optional: true
belongs_to :variant, -> { with_discarded }, class_name: "Spree::Variant", inverse_of: :line_items, optional: true
belongs_to :tax_category, class_name: "Spree::TaxCategory", optional: true

has_one :product, through: :variant
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/payment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Payment < Spree::Base

belongs_to :order, class_name: 'Spree::Order', touch: true, inverse_of: :payments, optional: true
belongs_to :source, polymorphic: true, optional: true
belongs_to :payment_method, -> { with_deleted }, class_name: 'Spree::PaymentMethod', inverse_of: :payments, optional: true
belongs_to :payment_method, -> { with_discarded }, class_name: 'Spree::PaymentMethod', inverse_of: :payments, optional: true

has_many :offsets, -> { offset_payment }, class_name: "Spree::Payment", foreign_key: :source_id
has_many :log_entries, as: :source
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/payment_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ def active?
where(type: to_s, active: true).count > 0
end

# @deprecated Use .with_deleted.find instead
# @deprecated Use .with_discarded.find instead
def find_with_destroyed(*args)
Spree::Deprecation.warn "#{self}.find_with_destroyed is deprecated. Use #{self}.with_deleted.find instead"
Spree::Deprecation.warn "#{self}.find_with_destroyed is deprecated. Use #{self}.with_discarded.find instead"
unscoped { find(*args) }
end
end
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/price.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Price < Spree::Base

MAXIMUM_AMOUNT = BigDecimal('99_999_999.99')

belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant', touch: true, optional: true
belongs_to :variant, -> { with_discarded }, class_name: 'Spree::Variant', touch: true, optional: true
belongs_to :country, class_name: "Spree::Country", foreign_key: "country_iso", primary_key: "iso", optional: true

delegate :product, to: :variant
Expand Down
4 changes: 2 additions & 2 deletions core/app/models/spree/product.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Product < Spree::Base
belongs_to :shipping_category, class_name: 'Spree::ShippingCategory', inverse_of: :products, optional: true

has_one :master,
-> { where(is_master: true).with_deleted },
-> { where(is_master: true).with_discarded },
inverse_of: :product,
class_name: 'Spree::Variant',
autosave: true
Expand Down Expand Up @@ -134,7 +134,7 @@ def find_or_build_master
self.whitelisted_ransackable_attributes = %w[name slug]

def self.ransackable_scopes(_auth_object = nil)
%i(with_deleted with_variant_sku_cont)
%i(with_discarded with_variant_sku_cont)
end

# @return [Boolean] true if there are any variants
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/shipping_method.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class ShippingMethod < Spree::Base
has_many :shipping_method_zones, dependent: :destroy
has_many :zones, through: :shipping_method_zones

belongs_to :tax_category, -> { with_deleted }, class_name: 'Spree::TaxCategory', optional: true
belongs_to :tax_category, -> { with_discarded }, class_name: 'Spree::TaxCategory', optional: true
has_many :shipping_method_stock_locations, dependent: :destroy, class_name: "Spree::ShippingMethodStockLocation"
has_many :stock_locations, through: :shipping_method_stock_locations

Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/shipping_rate.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Spree
#
class ShippingRate < Spree::Base
belongs_to :shipment, class_name: 'Spree::Shipment', touch: true, optional: true
belongs_to :shipping_method, -> { with_deleted }, class_name: 'Spree::ShippingMethod', inverse_of: :shipping_rates, optional: true
belongs_to :shipping_method, -> { with_discarded }, class_name: 'Spree::ShippingMethod', inverse_of: :shipping_rates, optional: true

has_many :taxes,
class_name: "Spree::ShippingRateTax",
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/stock_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class StockItem < Spree::Base
self.discard_column = :deleted_at

belongs_to :stock_location, class_name: 'Spree::StockLocation', inverse_of: :stock_items, optional: true
belongs_to :variant, -> { with_deleted }, class_name: 'Spree::Variant', inverse_of: :stock_items, optional: true
belongs_to :variant, -> { with_discarded }, class_name: 'Spree::Variant', inverse_of: :stock_items, optional: true
has_many :stock_movements, inverse_of: :stock_item

validates :stock_location, :variant, presence: true
Expand Down
2 changes: 1 addition & 1 deletion core/app/models/spree/variant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Variant < Spree::Base
attr_writer :rebuild_vat_prices
include Spree::DefaultPrice

belongs_to :product, -> { with_deleted }, touch: true, class_name: 'Spree::Product', inverse_of: :variants, optional: false
belongs_to :product, -> { with_discarded }, touch: true, class_name: 'Spree::Product', inverse_of: :variants, optional: false
belongs_to :tax_category, class_name: 'Spree::TaxCategory', optional: true

delegate :name, :description, :slug, :available_on, :shipping_category_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ module Spree
}.not_to change { Spree::Adjustment.count }

expect(adjustment.source).to eq(nil)
expect(Spree::PromotionAction.with_deleted.find(adjustment.source_id)).to be_present
expect(Spree::PromotionAction.with_discarded.find(adjustment.source_id)).to be_present
end

it "doesnt mess with unrelated adjustments" do
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/variant_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@
context 'when loading with pre-fetching of default_price' do
it 'also keeps the previous price' do
variant.discard
reloaded_variant = Spree::Variant.with_deleted.includes(:default_price).find_by(id: variant.id)
reloaded_variant = Spree::Variant.with_discarded.includes(:default_price).find_by(id: variant.id)
expect(reloaded_variant.display_price).to eq(previous_variant_price)
end
end
Expand Down
2 changes: 1 addition & 1 deletion frontend/app/controllers/spree/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def accurate_title

def load_product
if try_spree_current_user.try(:has_spree_role?, "admin")
@products = Spree::Product.with_deleted
@products = Spree::Product.with_discarded
else
@products = Spree::Product.available
end
Expand Down