raggedstaff/openfoodnetwork
1
0
Fork 0
mirror of https://github.com/openfoodfoundation/openfoodnetwork synced 2026-03-01 02:03:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

Delete product_distributions: drop table and remove models, controllers and BO edit page

Browse Source
This commit is contained in:
luisramos0
2019-03-03 16:05:18 +00:00
parent 2dd55770fe
commit 86f9b3d663
19 changed files with 53 additions and 224 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.rubocop_manual_todo.yml
View File

@@ -99,7 +99,6 @@ Metrics/LineLength:
- app/models/exchange.rb
- app/models/inventory_item.rb
- app/models/order_cycle.rb
- app/models/product_distribution.rb
- app/models/product_import/entry_processor.rb
- app/models/product_import/entry_validator.rb
- app/models/product_import/product_importer.rb
@@ -349,7 +348,6 @@ Metrics/LineLength:
- spec/models/model_set_spec.rb
- spec/models/order_cycle_spec.rb
- spec/models/producer_property_spec.rb
- spec/models/product_distribution_spec.rb
- spec/models/product_importer_spec.rb
- spec/models/proxy_order_spec.rb
- spec/models/spree/ability_spec.rb

9
.rubocop_todo.yml
View File

@@ -153,7 +153,6 @@ Layout/EmptyLines:
- 'app/models/inventory_item.rb'
- 'app/models/order_cycle.rb'
- 'app/models/producer_property.rb'
- 'app/models/product_distribution.rb'
- 'app/models/spree/calculator_decorator.rb'
- 'app/models/spree/classification_decorator.rb'
- 'app/models/spree/gateway/migs.rb'
@@ -233,7 +232,6 @@ Layout/EmptyLines:
- 'spec/models/enterprise_fee_spec.rb'
- 'spec/models/enterprise_spec.rb'
- 'spec/models/model_set_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/spree/adjustment_spec.rb'
- 'spec/models/spree/line_item_spec.rb'
- 'spec/models/spree/order_spec.rb'
@@ -291,7 +289,6 @@ Layout/EmptyLinesAroundBlockBody:
- 'spec/lib/open_food_network/referer_parser_spec.rb'
- 'spec/lib/open_food_network/user_balance_calculator_spec.rb'
- 'spec/models/billable_period_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/spree/ability_spec.rb'
- 'spec/models/spree/product_spec.rb'
- 'spec/models/tag_rule/filter_payment_methods_spec.rb'
@@ -841,7 +838,6 @@ Layout/SpaceInsideHashLiteralBraces:
- 'spec/models/enterprise_relationship_spec.rb'
- 'spec/models/exchange_spec.rb'
- 'spec/models/model_set_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/product_importer_spec.rb'
- 'spec/models/spree/ability_spec.rb'
- 'spec/models/spree/gateway/stripe_connect_spec.rb'
@@ -1484,7 +1480,6 @@ Rails/Validation:
- 'app/models/enterprise_role.rb'
- 'app/models/exchange.rb'
- 'app/models/order_cycle.rb'
- 'app/models/product_distribution.rb'
- 'app/models/spree/product_decorator.rb'
- 'app/models/variant_override.rb'
@@ -1575,7 +1570,6 @@ Style/BracesAroundHashParameters:
- 'spec/lib/open_food_network/order_cycle_form_applicator_spec.rb'
- 'spec/lib/open_food_network/subscription_summarizer_spec.rb'
- 'spec/models/billable_period_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/spree/ability_spec.rb'
- 'spec/models/spree/order_spec.rb'
- 'spec/models/spree/product_spec.rb'
@@ -1850,7 +1844,6 @@ Style/HashSyntax:
- 'app/models/enterprise_role.rb'
- 'app/models/exchange_variant.rb'
- 'app/models/order_cycle.rb'
- 'app/models/product_distribution.rb'
- 'app/models/spree/adjustment_decorator.rb'
- 'app/models/spree/classification_decorator.rb'
- 'app/models/spree/gateway/migs.rb'
@@ -1931,7 +1924,6 @@ Style/HashSyntax:
- 'spec/models/enterprise_spec.rb'
- 'spec/models/exchange_spec.rb'
- 'spec/models/order_cycle_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/spree/calculator/flat_percent_item_total_spec.rb'
- 'spec/models/spree/calculator/flat_rate_spec.rb'
- 'spec/models/spree/image_spec.rb'
@@ -2267,7 +2259,6 @@ Style/Send:
- 'spec/models/enterprise_spec.rb'
- 'spec/models/exchange_spec.rb'
- 'spec/models/order_cycle_spec.rb'
- 'spec/models/product_distribution_spec.rb'
- 'spec/models/spree/gateway/stripe_connect_spec.rb'
- 'spec/models/spree/line_item_spec.rb'
- 'spec/models/spree/order_spec.rb'

8
app/controllers/spree/admin/shipping_methods_controller_decorator.rb
View File

@@ -29,12 +29,8 @@ module Spree
collection
end
# This method was originally written because ProductDistributions referenced shipping
# methods, and deleting a referenced shipping method would break all the reports that
# queried it.
# This has changed, and now all we're protecting is Orders, which is a spree resource.
# Do we really need to protect it ourselves? Does spree do this, or provide some means
# of preserving the shipping method information for past orders?
# Spree allows soft deletes of shipping_methods but our reports are not adapted to that.
# So, here we prevent the deletion (even soft) of shipping_methods used in orders.
def do_not_destroy_referenced_shipping_methods
order = Order.where(:shipping_method_id => @object).first
if order

27
app/models/product_distribution.rb
View File

@@ -1,27 +0,0 @@
class ProductDistribution < ActiveRecord::Base
belongs_to :product, :class_name => 'Spree::Product'
belongs_to :distributor, :class_name => 'Enterprise'
belongs_to :enterprise_fee
validates_presence_of :product_id, :on => :update
validates_presence_of :distributor_id, :enterprise_fee_id
validates_uniqueness_of :product_id, :scope => :distributor_id
def adjustment_for(line_item)
adjustments = line_item.order.adjustments.enterprise_fee.where(originator_id: enterprise_fee)
raise "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario." if adjustments.count > 1
adjustments.first
end
def create_adjustment_for(line_item)
a = enterprise_fee.create_adjustment(adjustment_label_for(line_item), line_item.order, line_item, true)
AdjustmentMetadata.create! adjustment: a, enterprise: enterprise_fee.enterprise, fee_name: enterprise_fee.name, fee_type: enterprise_fee.fee_type, enterprise_role: 'distributor'
end
def adjustment_label_for(line_item)
I18n.t(:products_distribution_adjustment_label, distributor: distributor.name, product: line_item.product.name )
end
end

5
app/models/spree/ability_decorator.rb
View File

@@ -138,7 +138,10 @@ class AbilityDecorator
def add_product_management_abilities(user)
# Enterprise User can only access products that they are a supplier for
can [:create], Spree::Product
can [:admin, :read, :index, :update, :product_distributions, :seo, :group_buy_options, :bulk_update, :clone, :delete, :destroy], Spree::Product do |product|
can [:admin, :read, :index, :update,
:seo, :group_buy_options,
:bulk_update, :clone, :delete,
:destroy], Spree::Product do |product|
OpenFoodNetwork::Permissions.new(user).managed_product_enterprises.include? product.supplier
end

5
app/overrides/spree/admin/shared/_product_tabs/add_distributions.html.haml.deface
View File

@@ -1,5 +0,0 @@
/ insert_bottom "[data-hook='admin_product_tabs']"
- klass = current == 'Product Distributions' ? 'active' : ''
%li{:class => klass}
= link_to_with_icon 'icon-tasks', t('admin.products.product_distributions'), product_distributions_admin_product_url(@product)

14
app/views/spree/admin/products/_distributors_form.html.haml
View File

@@ -1,14 +0,0 @@
%h2= t(:distributors)
= f.field_container :product_distributions do
- f.object.build_product_distributions_for_user spree_current_user
%table
= f.fields_for :product_distributions do |pd_form|
%tr
%td
= hidden_field_tag "#{pd_form.object_name}[_destroy]", 1, :id => nil
= check_box_tag "#{pd_form.object_name}[_destroy]", 0, !pd_form.object.new_record?
%td
= label_tag "#{pd_form.object_name}[_destroy]", pd_form.object.distributor.name
= pd_form.hidden_field :distributor_id
%td
= pd_form.collection_select :enterprise_fee_id, EnterpriseFee.for_enterprise(pd_form.object.distributor), :id, :name, {}, :class => "select2"

8
app/views/spree/admin/products/product_distributions.html.haml
View File

@@ -1,8 +0,0 @@
= render partial: 'admin/shared/product_sub_menu'
= render :partial => 'spree/admin/shared/product_tabs', :locals => { :current => 'Product Distributions' }
= render :partial => 'spree/shared/error_messages', :locals => { :target => @product }
= form_for [:admin, @product], :method => :put, :html => { :multipart => true } do |f|
%fieldset.no-border-top
= render :partial => 'distributors_form', :locals => { :f => f }
= render :partial => 'spree/admin/shared/edit_resource_links'

4
config/locales/en.yml
View File

@@ -534,7 +534,6 @@ en:
inherited_property: Inherited Property
variants:
to_order_tip: "Items made to order do not have a set stock level, such as loaves of bread made fresh to order."
product_distributions: "Product Distributions"
group_buy_options: "Group Buy Options"
back_to_products_list: "Back to products list"
@@ -1821,7 +1820,6 @@ See the %{link} to find out more about %{sitename}'s features and to start using
products_max_quantity: Max quantity
products_distributor: Distributor
products_distributor_info: When you select a distributor for your order, their address and pickup times will be displayed here.
products_distribution_adjustment_label: "Product distribution by %{distributor} for %{product}"
shop_trial_expires_in: "Your shopfront trial expires in"
shop_trial_expired_notice: "Good news! We have decided to extend shopfront trials until further notice."
@@ -2420,7 +2418,7 @@ See the %{link} to find out more about %{sitename}'s features and to start using
but do not have valid shipping and payment methods.
Until you set these up, customers will not be able to shop at these hubs.
enterprise_fees_update_notice: Your enterprise fees have been updated.
enterprise_fees_destroy_error: "That enterprise fee cannot be deleted as it is referenced by a product distribution: %{id} - %{name}."
enterprise_fees_destroy_error: "That enterprise fee cannot be deleted."
enterprise_register_package_error: "Please select a package"
enterprise_register_error: "Could not complete registration for %{enterprise}"
enterprise_register_success_notice: "Congratulations! Registration for %{enterprise} is complete!"

1
config/routes/spree.rb
View File

@@ -61,7 +61,6 @@ Spree::Core::Engine.routes.prepend do
get '/search/customer_addresses' => 'search#customer_addresses', :as => :search_customer_addresses
resources :products do
get :product_distributions, on: :member
get :group_buy_options, on: :member
get :seo, on: :member

37
db/migrate/20190303143409_drop_product_distributions.rb Normal file
View File

@@ -0,0 +1,37 @@
class DropProductDistributions < ActiveRecord::Migration
def up
drop_table :product_distributions
end
def down
create_table "product_distributions" do |t|
t.integer "product_id"
t.integer "distributor_id"
t.datetime "created_at"
t.datetime "updated_at"
t.integer "enterprise_fee_id"
end
add_index "product_distributions",
["distributor_id"],
name: "index_product_distributions_on_distributor_id"
add_index "product_distributions",
["enterprise_fee_id"],
name: "index_product_distributions_on_enterprise_fee_id"
add_index "product_distributions",
["product_id"],
name: "index_product_distributions_on_product_id"
add_foreign_key "product_distributions",
"enterprise_fees",
name: "product_distributions_enterprise_fee_id_fk"
add_foreign_key "product_distributions",
"enterprises",
name: "product_distributions_distributor_id_fk",
column: "distributor_id"
add_foreign_key "product_distributions",
"spree_products",
name: "product_distributions_product_id_fk",
column: "product_id"
end
end

18
db/schema.rb
View File

@@ -11,7 +11,7 @@
#
# It's strongly recommended to check this file into your version control system.
ActiveRecord::Schema.define(:version => 20181128054803) do
ActiveRecord::Schema.define(:version => 20190303143409) do
create_table "account_invoices", :force => true do |t|
t.integer "user_id", :null => false
@@ -331,18 +331,6 @@ ActiveRecord::Schema.define(:version => 20181128054803) do
add_index "producer_properties", ["producer_id"], :name => "index_producer_properties_on_producer_id"
add_index "producer_properties", ["property_id"], :name => "index_producer_properties_on_property_id"
create_table "product_distributions", :force => true do |t|
t.integer "product_id"
t.integer "distributor_id"
t.datetime "created_at"
t.datetime "updated_at"
t.integer "enterprise_fee_id"
end
add_index "product_distributions", ["distributor_id"], :name => "index_product_distributions_on_distributor_id"
add_index "product_distributions", ["enterprise_fee_id"], :name => "index_product_distributions_on_enterprise_fee_id"
add_index "product_distributions", ["product_id"], :name => "index_product_distributions_on_product_id"
create_table "proxy_orders", :force => true do |t|
t.integer "subscription_id", :null => false
t.integer "order_id"
@@ -1248,10 +1236,6 @@ ActiveRecord::Schema.define(:version => 20181128054803) do
add_foreign_key "producer_properties", "enterprises", name: "producer_properties_producer_id_fk", column: "producer_id"
add_foreign_key "producer_properties", "spree_properties", name: "producer_properties_property_id_fk", column: "property_id"
add_foreign_key "product_distributions", "enterprise_fees", name: "product_distributions_enterprise_fee_id_fk"
add_foreign_key "product_distributions", "enterprises", name: "product_distributions_distributor_id_fk", column: "distributor_id"
add_foreign_key "product_distributions", "spree_products", name: "product_distributions_product_id_fk", column: "product_id"
add_foreign_key "proxy_orders", "order_cycles", name: "proxy_orders_order_cycle_id_fk"
add_foreign_key "proxy_orders", "spree_orders", name: "order_id_fk", column: "order_id"
add_foreign_key "proxy_orders", "subscriptions", name: "proxy_orders_subscription_id_fk"

1
knapsack_rspec_report.json
View File

@@ -89,7 +89,6 @@
"spec/controllers/spree/api/variants_controller_spec.rb": 6.177022218704224,
"spec/controllers/admin/variant_overrides_controller_spec.rb": 7.0243120193481445,
"spec/lib/open_food_network/lettuce_share_report_spec.rb": 4.769379615783691,
"spec/models/product_distribution_spec.rb": 4.708523988723755,
"spec/models/producer_property_spec.rb": 4.007659435272217,
"spec/lib/open_food_network/customers_report_spec.rb": 3.967031240463257,
"spec/models/spree/shipping_method_spec.rb": 3.9209978580474854,

12
spec/factories.rb
View File

@@ -272,12 +272,6 @@ FactoryBot.define do
after(:create) { |ef| ef.calculator.save! }
end
factory :product_distribution, :class => ProductDistribution do
product { |pd| Spree::Product.first || FactoryBot.create(:product) }
distributor { |pd| Enterprise.is_distributor.first || FactoryBot.create(:distributor_enterprise) }
enterprise_fee { |pd| FactoryBot.create(:enterprise_fee, enterprise: pd.distributor) }
end
factory :adjustment_metadata, :class => AdjustmentMetadata do
adjustment { FactoryBot.create(:adjustment) }
enterprise { FactoryBot.create(:distributor_enterprise) }
@@ -296,8 +290,8 @@ FactoryBot.define do
order_cycle { create(:simple_order_cycle) }
after(:create) do |order|
p = create(:simple_product, :distributors => [order.distributor])
FactoryBot.create(:line_item, :order => order, :product => p)
product = create(:simple_product)
FactoryBot.create(:line_item, :order => order, :product => product)
order.reload
end
end
@@ -320,7 +314,7 @@ FactoryBot.define do
order.distributor.update_attribute(:charges_sales_tax, true)
Spree::Zone.global.update_attribute(:default_tax, true)
p = FactoryBot.create(:taxed_product, zone: Spree::Zone.global, price: proxy.product_price, tax_rate_amount: proxy.tax_rate_amount, tax_rate_name: proxy.tax_rate_name, distributors: [order.distributor])
p = FactoryBot.create(:taxed_product, zone: Spree::Zone.global, price: proxy.product_price, tax_rate_amount: proxy.tax_rate_amount, tax_rate_name: proxy.tax_rate_name)
FactoryBot.create(:line_item, order: order, product: p, price: p.price)
order.reload
end

21
spec/features/admin/products_spec.rb
View File

@@ -162,27 +162,6 @@ feature %q{
product.group_buy_unit_size.should == 10.0
end
scenario "editing product distributions" do
product = create(:simple_product, supplier: @supplier2)
visit spree.edit_admin_product_path product
within('#sidebar') { click_link 'Product Distributions' }
check @distributors[0].name
select @enterprise_fees[0].name, :from => 'product_product_distributions_attributes_0_enterprise_fee_id'
# Should only have distributors listed which the user can manage
within "#product_product_distributions_field" do
page.should_not have_content @distributors[1].name
page.should_not have_content @distributors[2].name
end
click_button 'Update'
flash_message.should == "Product \"#{product.name}\" has been successfully updated!"
product.distributors.should == [@distributors[0]]
end
scenario "editing product Search" do
product = product = create(:simple_product, supplier: @supplier2)
visit spree.edit_admin_product_path product

1
spec/mailers/order_mailer_spec.rb
View File

@@ -18,7 +18,6 @@ describe Spree::OrderMailer do
@distributor_address = create(:address, :address1 => "distributor address", :city => 'The Shire', :zipcode => "1234")
@distributor = create(:distributor_enterprise, :address => @distributor_address)
product = create(:product)
product_distribution = create(:product_distribution, :product => product, :distributor => @distributor)
@shipping_instructions = "pick up on thursday please!"
ship_address = create(:address, :address1 => "distributor address", :city => 'The Shire', :zipcode => "1234")
@order1 = create(:order, :distributor => @distributor, :bill_address => @bill_address, ship_address: ship_address, :special_instructions => @shipping_instructions)

84
spec/models/product_distribution_spec.rb
View File

@@ -1,84 +0,0 @@
require 'spec_helper'
describe ProductDistribution do
it "is unique for scope [product, distributor]" do
pd1 = create(:product_distribution)
expect(pd1).to be_valid
new_product = create(:product)
new_distributor = create(:distributor_enterprise)
pd2 = build(:product_distribution, :product => pd1.product, :distributor => pd1.distributor)
expect(pd2).to_not be_valid
pd2 = build(:product_distribution, :product => pd1.product, :distributor => new_distributor)
expect(pd2).to be_valid
pd2 = build(:product_distribution, :product => new_product, :distributor => pd1.distributor)
expect(pd2).to be_valid
pd2 = build(:product_distribution, :product => new_product, :distributor => new_distributor)
expect(pd2).to be_valid
end
describe "adjusting orders" do
describe "finding our adjustment for a line item" do
it "returns nil when not present" do
line_item = build(:line_item)
pd = ProductDistribution.new
expect(pd.send(:adjustment_for, line_item)).to be_nil
end
it "returns the adjustment when present" do
pd = create(:product_distribution)
line_item = create(:line_item)
adjustment = pd.enterprise_fee.create_adjustment('foo', line_item.order, line_item, true)
expect(pd.send(:adjustment_for, line_item)).to eq adjustment
end
it "raises an error when there are multiple adjustments for this enterprise fee" do
pd = create(:product_distribution)
line_item = create(:line_item)
pd.enterprise_fee.create_adjustment('one', line_item.order, line_item, true)
pd.enterprise_fee.create_adjustment('two', line_item.order, line_item, true)
expect do
pd.send(:adjustment_for, line_item)
end.to raise_error "Multiple adjustments for this enterprise fee on this line item. This method is not designed to deal with this scenario."
end
end
describe "creating an adjustment for a line item" do
it "creates the adjustment via the enterprise fee" do
pd = create(:product_distribution)
pd.stub(:adjustment_label_for) { 'label' }
line_item = create(:line_item)
expect { pd.send(:create_adjustment_for, line_item) }.to change(Spree::Adjustment, :count).by(1)
adjustment = Spree::Adjustment.last
expect(adjustment.label).to eq 'label'
expect(adjustment.adjustable).to eq line_item.order
expect(adjustment.source).to eq line_item
expect(adjustment.originator).to eq pd.enterprise_fee
expect(adjustment).to be_mandatory
md = adjustment.metadata
expect(md.enterprise).to eq pd.distributor
expect(md.fee_name).to eq pd.enterprise_fee.name
expect(md.fee_type).to eq pd.enterprise_fee.fee_type
expect(md.enterprise_role).to eq 'distributor'
end
end
end
private
def fire_order_contents_changed_event(user, order)
ActiveSupport::Notifications.instrument('spree.order.contents_changed', {user: user, order: order})
end
end

10
spec/models/spree/ability_spec.rb
View File

@@ -126,8 +126,8 @@ module Spree
let(:d1) { create(:distributor_enterprise) }
let(:d2) { create(:distributor_enterprise) }
let(:p1) { create(:product, supplier: s1, distributors:[d1, d2]) }
let(:p2) { create(:product, supplier: s2, distributors:[d1, d2]) }
let(:p1) { create(:product, supplier: s1) }
let(:p2) { create(:product, supplier: s2) }
let(:p_related) { create(:product, supplier: s_related) }
let(:er1) { create(:enterprise_relationship, parent: s1, child: d1) }
@@ -149,18 +149,18 @@ module Spree
let(:order) { create(:order) }
it "should be able to read/write their enterprises' products and variants" do
should have_ability([:admin, :read, :update, :product_distributions, :bulk_update, :clone, :destroy], for: p1)
should have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy], for: p1)
should have_ability([:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p1.master)
end
it "should be able to read/write related enterprises' products and variants with manage_products permission" do
er_ps
should have_ability([:admin, :read, :update, :product_distributions, :bulk_update, :clone, :destroy], for: p_related)
should have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy], for: p_related)
should have_ability([:admin, :index, :read, :edit, :update, :search, :destroy, :delete], for: p_related.master)
end
it "should not be able to read/write other enterprises' products and variants" do
should_not have_ability([:admin, :read, :update, :product_distributions, :bulk_update, :clone, :destroy], for: p2)
should_not have_ability([:admin, :read, :update, :bulk_update, :clone, :destroy], for: p2)
should_not have_ability([:admin, :index, :read, :edit, :update, :search, :destroy], for: p2.master)
end

10
spec/support/spree/init.rb
View File

@@ -1,10 +0,0 @@
# Initialise enterprise fee when created without one, like this:
# create(:product, :distributors => [...])
# In this case, we don't care what the fee is, but we need one for validations to pass.
ProductDistribution.class_eval do
before_validation :init_enterprise_fee
def init_enterprise_fee
self.enterprise_fee ||= EnterpriseFee.where(enterprise_id: distributor).first || FactoryBot.create(:enterprise_fee, enterprise_id: distributor)
end
end