mirror of
https://github.com/openfoodfoundation/openfoodnetwork
synced 2026-01-30 21:27:17 +00:00
BPUR: Don't use master variant to update attributes. Changes to product filtering.
This commit is contained in:
@@ -50,16 +50,15 @@ productsApp.controller('AdminBulkProductsCtrl', function($scope, $timeout, $http
|
||||
});
|
||||
};
|
||||
|
||||
//accessible from scope
|
||||
$scope.onHand = function(products){
|
||||
return onHand(products);
|
||||
$scope.updateOnHand = function(product){
|
||||
product.on_hand = onHand(product);
|
||||
}
|
||||
|
||||
$scope.updateStatusMessage = {
|
||||
text: "",
|
||||
style: {}
|
||||
}
|
||||
|
||||
|
||||
$scope.updateProducts = function(productsToSubmit){
|
||||
$scope.displayUpdating();
|
||||
$http({
|
||||
@@ -226,12 +225,9 @@ function filterSubmitProducts(productsToFilter){
|
||||
for (i in productsToFilter) {
|
||||
if (productsToFilter[i].hasOwnProperty("id")){
|
||||
var filteredProduct = {};
|
||||
filteredProduct.id = productsToFilter[i].id;
|
||||
if (productsToFilter[i].hasOwnProperty("supplier_id")) filteredProduct.supplier_id = productsToFilter[i].supplier_id;
|
||||
if (productsToFilter[i].hasOwnProperty("name")) filteredProduct.name = productsToFilter[i].name;
|
||||
if (productsToFilter[i].hasOwnProperty("available_on")) filteredProduct.available_on = productsToFilter[i].available_on;
|
||||
var filteredVariants = [];
|
||||
|
||||
if (productsToFilter[i].hasOwnProperty("variants")){
|
||||
var filteredVariants = [];
|
||||
for (j in productsToFilter[i].variants){
|
||||
if (productsToFilter[i].variants[j].deleted_at == null && productsToFilter[i].variants[j].hasOwnProperty("id")){
|
||||
filteredVariants[j] = {};
|
||||
@@ -240,10 +236,19 @@ function filterSubmitProducts(productsToFilter){
|
||||
if (productsToFilter[i].variants[j].hasOwnProperty("price")) filteredVariants[j].price = productsToFilter[i].variants[j].price;
|
||||
}
|
||||
}
|
||||
filteredProduct.variants_attributes = filteredVariants; // Note that the name of the property changes to enable mass assignment of variants attributes with rails
|
||||
}
|
||||
if (productsToFilter[i].hasOwnProperty("master")) filteredProduct.master_attributes = productsToFilter[i].master
|
||||
filteredProducts.push(filteredProduct);
|
||||
|
||||
var hasUpdatableProperty = false;
|
||||
filteredProduct.id = productsToFilter[i].id;
|
||||
if (productsToFilter[i].hasOwnProperty("name")) { filteredProduct.name = productsToFilter[i].name; hasUpdatableProperty = true; }
|
||||
if (productsToFilter[i].hasOwnProperty("supplier_id")) { filteredProduct.supplier_id = productsToFilter[i].supplier_id; hasUpdatableProperty = true; }
|
||||
//if (productsToFilter[i].hasOwnProperty("master")) filteredProduct.master_attributes = productsToFilter[i].master
|
||||
if (productsToFilter[i].hasOwnProperty("price")) { filteredProduct.price = productsToFilter[i].price; hasUpdatableProperty = true; }
|
||||
if (productsToFilter[i].hasOwnProperty("on_hand") && filteredVariants.length == 0) { filteredProduct.on_hand = productsToFilter[i].on_hand; hasUpdatableProperty = true; } //only update if no variants present
|
||||
if (productsToFilter[i].hasOwnProperty("available_on")) { filteredProduct.available_on = productsToFilter[i].available_on; hasUpdatableProperty = true; }
|
||||
if (filteredVariants.length > 0) { filteredProduct.variants_attributes = filteredVariants; hasUpdatableProperty = true; } // Note that the name of the property changes to enable mass assignment of variants attributes with rails
|
||||
|
||||
if (hasUpdatableProperty) filteredProducts.push(filteredProduct);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,9 +5,8 @@ Spree::Product.class_eval do
|
||||
has_many :distributors, :through => :product_distributions
|
||||
|
||||
accepts_nested_attributes_for :product_distributions, :allow_destroy => true
|
||||
accepts_nested_attributes_for :master, :allow_destroy => false
|
||||
|
||||
attr_accessible :supplier_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size, :master_attributes
|
||||
attr_accessible :supplier_id, :distributor_ids, :product_distributions_attributes, :group_buy, :group_buy_unit_size
|
||||
|
||||
validates_presence_of :supplier
|
||||
|
||||
|
||||
@@ -27,10 +27,10 @@
|
||||
%td
|
||||
%select.select2{ :name => 'supplier_id', 'ng-model' => 'product.supplier_id', 'ng-options' => 's.id as s.name for s in suppliers' }
|
||||
%td
|
||||
%input{ 'ng-model' => 'product.master.price', 'ng-decimal' => :true, :name => 'master_price', :type => 'text' }
|
||||
%input{ 'ng-model' => 'product.price', 'ng-decimal' => :true, :name => 'price', :type => 'text' }
|
||||
%td
|
||||
%span{ 'ng-bind' => 'onHand(product)', :name => 'on_hand', 'ng-show' => 'product.variants.length > 0' }
|
||||
%input.field{ 'ng-model' => 'product.master.on_hand', :name => 'master_on_hand', 'ng-show' => 'product.variants.length == 0', :type => 'number' }
|
||||
%span{ 'ng-bind' => 'product.on_hand', :name => 'on_hand', 'ng-show' => 'product.variants.length > 0' }
|
||||
%input.field{ 'ng-model' => 'product.on_hand', :name => 'on_hand', 'ng-show' => 'product.variants.length == 0', :type => 'number' }
|
||||
%td
|
||||
%input{ 'ng-model' => 'product.available_on', :name => 'available_on', :type => 'text' }
|
||||
%tr{ 'ng-repeat' => 'variant in product.variants' }
|
||||
@@ -40,7 +40,7 @@
|
||||
%td
|
||||
%input{ 'ng-model' => 'variant.price', 'ng-decimal' => :true, :name => 'variant_price', :type => 'text' }
|
||||
%td
|
||||
%input.field{ 'ng-model' => 'variant.on_hand', :name => 'variant_on_hand', :type => 'number' }
|
||||
%input.field{ 'ng-model' => 'variant.on_hand', 'ng-change' => 'updateOnHand(product)', :name => 'variant_on_hand', :type => 'number' }
|
||||
%td
|
||||
%input{:type => 'button', :value => 'Update', 'ng-click' => 'prepareProductsForSubmit()'}
|
||||
%span{ id: "update-status-message", 'ng-style' => 'updateStatusMessage.style' }
|
||||
|
||||
@@ -3,11 +3,8 @@ r.list_of :products, @collection do
|
||||
r.element :name
|
||||
r.element :supplier_id
|
||||
r.element :available_on, Proc.new{ |product| product.available_on.strftime("%F %T") }
|
||||
r.element :master do
|
||||
r.element :id
|
||||
r.element :price
|
||||
r.element :on_hand
|
||||
end
|
||||
r.element :price
|
||||
r.element :on_hand
|
||||
r.list_of :variants, Proc.new{ |product| product.variants.sort { |a,b| a.id <=> b.id } } do
|
||||
r.element :id
|
||||
r.element :options_text
|
||||
|
||||
@@ -13,7 +13,6 @@ describe Spree::Admin::ProductsController do
|
||||
p1 = FactoryGirl.create(:product)
|
||||
p2 = FactoryGirl.create(:product)
|
||||
spree_get :bulk_index, { format: :json }
|
||||
#process :bulk_index, {:use_route=> :spree}, nil, nil, "GET"
|
||||
|
||||
assigns[:collection].should_not be_empty
|
||||
assigns[:collection].should == [ p1, p2 ]
|
||||
@@ -33,11 +32,8 @@ describe Spree::Admin::ProductsController do
|
||||
"name" => p1.name,
|
||||
"supplier_id" => p1.supplier_id,
|
||||
"available_on" => p1.available_on.strftime("%F %T"),
|
||||
"master" => {
|
||||
"id" => p1.master.id,
|
||||
"price" => p1.master.price.to_s,
|
||||
"on_hand" => p1.master.on_hand
|
||||
},
|
||||
"price" => p1.price.to_s,
|
||||
"on_hand" => ( v11.on_hand + v12.on_hand + v13.on_hand ),
|
||||
"variants" => [ #ordered by id
|
||||
{ "id" => v11.id, "options_text" => v11.options_text, "price" => v11.price.to_s, "on_hand" => v11.on_hand },
|
||||
{ "id" => v12.id, "options_text" => v12.options_text, "price" => v12.price.to_s, "on_hand" => v12.on_hand },
|
||||
@@ -49,17 +45,13 @@ describe Spree::Admin::ProductsController do
|
||||
"name" => p2.name,
|
||||
"supplier_id" => p2.supplier_id,
|
||||
"available_on" => p2.available_on.strftime("%F %T"),
|
||||
"master" => {
|
||||
"id" => p2.master.id,
|
||||
"price" => p2.master.price.to_s,
|
||||
"on_hand" => p2.master.on_hand
|
||||
},
|
||||
"price" => p2.price.to_s,
|
||||
"on_hand" => v21.on_hand,
|
||||
"variants" => [ #ordered by id
|
||||
{ "id" => v21.id, "options_text" => v21.options_text, "price" => v21.price.to_s, "on_hand" => v21.on_hand }
|
||||
]
|
||||
}
|
||||
json_response = JSON.parse(response.body)
|
||||
#json_response = Hash[json_response.map{ |k, v| [k.to_sym, v] }]
|
||||
json_response.should == [ p1r, p2r ]
|
||||
end
|
||||
end
|
||||
|
||||
@@ -54,49 +54,49 @@ feature %q{
|
||||
page.should have_field "available_on", with: p2.available_on.strftime("%F %T")
|
||||
end
|
||||
|
||||
it "displays a price input (for master variant) for each product" do
|
||||
it "displays a price input for each product (ie. for master variant)" do
|
||||
p1 = FactoryGirl.create(:product)
|
||||
p2 = FactoryGirl.create(:product)
|
||||
p1.master.price = 22.00
|
||||
p2.master.price = 44.00
|
||||
p1.price = 22.00
|
||||
p2.price = 44.00
|
||||
p1.save!
|
||||
p2.save!
|
||||
|
||||
visit '/admin/products/bulk_index'
|
||||
|
||||
page.should have_field "master_price", with: "22.0"
|
||||
page.should have_field "master_price", with: "44.0"
|
||||
page.should have_field "price", with: "22.0"
|
||||
page.should have_field "price", with: "44.0"
|
||||
end
|
||||
|
||||
it "displays an on hand count input (for master variant) for each product if no other variants exist" do
|
||||
it "displays an on hand count input for each product (ie. for master variant) if no regular variants exist" do
|
||||
p1 = FactoryGirl.create(:product)
|
||||
p2 = FactoryGirl.create(:product)
|
||||
p1.master.on_hand = 15
|
||||
p2.master.on_hand = 12
|
||||
p1.on_hand = 15
|
||||
p2.on_hand = 12
|
||||
p1.save!
|
||||
p2.save!
|
||||
|
||||
visit '/admin/products/bulk_index'
|
||||
|
||||
page.should_not have_selector "span[name='on_hand']", text: "0"
|
||||
page.should have_field "master_on_hand", with: "15"
|
||||
page.should have_field "master_on_hand", with: "12"
|
||||
page.should have_field "on_hand", with: "15"
|
||||
page.should have_field "on_hand", with: "12"
|
||||
end
|
||||
|
||||
it "displays an on hand count in a span (for master variant) for each product if other variants exist" do
|
||||
it "displays an on hand count in a span for each product (ie. for master variant) if other variants exist" do
|
||||
p1 = FactoryGirl.create(:product)
|
||||
p2 = FactoryGirl.create(:product)
|
||||
v1 = FactoryGirl.create(:variant, product: p1, is_master: false, on_hand: 4)
|
||||
p1.master.on_hand = 15
|
||||
p2.master.on_hand = 12
|
||||
p1.on_hand = 15
|
||||
p2.on_hand = 12
|
||||
p1.save!
|
||||
p2.save!
|
||||
|
||||
visit '/admin/products/bulk_index'
|
||||
|
||||
page.should_not have_field "master_on_hand", with: "15"
|
||||
page.should_not have_field "on_hand", with: "15"
|
||||
page.should have_selector "span[name='on_hand']", text: "4"
|
||||
page.should have_field "master_on_hand", with: "12"
|
||||
page.should have_field "on_hand", with: "12"
|
||||
end
|
||||
end
|
||||
|
||||
@@ -137,7 +137,7 @@ feature %q{
|
||||
|
||||
visit '/admin/products/bulk_index'
|
||||
|
||||
page.should have_field "master_price", with: "2.0"
|
||||
page.should have_field "price", with: "2.0"
|
||||
page.should have_field "variant_price", with: "12.75"
|
||||
page.should have_field "variant_price", with: "2.5"
|
||||
end
|
||||
@@ -172,8 +172,8 @@ feature %q{
|
||||
s1 = FactoryGirl.create(:supplier_enterprise)
|
||||
s2 = FactoryGirl.create(:supplier_enterprise)
|
||||
p = FactoryGirl.create(:product, supplier: s1, available_on: Date.today)
|
||||
p.master.price = 10.0
|
||||
p.master.on_hand = 6;
|
||||
p.price = 10.0
|
||||
p.on_hand = 6;
|
||||
p.save!
|
||||
|
||||
login_to_admin_section
|
||||
@@ -183,14 +183,14 @@ feature %q{
|
||||
page.should have_field "product_name", with: p.name
|
||||
page.should have_select "supplier_id", selected: s1.name
|
||||
page.should have_field "available_on", with: p.available_on.strftime("%F %T")
|
||||
page.should have_field "master_price", with: "10.0"
|
||||
page.should have_field "master_on_hand", with: "6"
|
||||
page.should have_field "price", with: "10.0"
|
||||
page.should have_field "on_hand", with: "6"
|
||||
|
||||
fill_in "product_name", with: "Big Bag Of Potatoes"
|
||||
select(s2.name, :from => 'supplier_id')
|
||||
fill_in "available_on", with: (Date.today-3).strftime("%F %T")
|
||||
fill_in "master_price", with: "20"
|
||||
fill_in "master_on_hand", with: "18"
|
||||
fill_in "price", with: "20"
|
||||
fill_in "on_hand", with: "18"
|
||||
|
||||
click_button 'Update'
|
||||
page.find("span#update-status-message").should have_content "Update complete"
|
||||
@@ -200,8 +200,8 @@ feature %q{
|
||||
page.should have_field "product_name", with: "Big Bag Of Potatoes"
|
||||
page.should have_select "supplier_id", selected: s2.name
|
||||
page.should have_field "available_on", with: (Date.today-3).strftime("%F %T")
|
||||
page.should have_field "master_price", with: "20.0"
|
||||
page.should have_field "master_on_hand", with: "18"
|
||||
page.should have_field "price", with: "20.0"
|
||||
page.should have_field "on_hand", with: "18"
|
||||
end
|
||||
|
||||
scenario "updating a product with variants" do
|
||||
|
||||
@@ -140,7 +140,7 @@ describe("Auxillary functions", function(){
|
||||
expect( getDirtyObjects(c, d) ).toEqual( [ { id: 2, value: 12 }, { id: 3, value2: 15 } ] );
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe("filtering products", function(){
|
||||
it("only accepts and returns an array", function(){
|
||||
expect( filterSubmitProducts( [] ) ).toEqual([]);
|
||||
@@ -155,9 +155,14 @@ describe("Auxillary functions", function(){
|
||||
expect( filterSubmitProducts( [ { id: 1, name: "p1" }, { notanid: 2, name: "p2"} ] ) ).toEqual( [ { id: 1, name: "p1" } ]);
|
||||
});
|
||||
|
||||
it("returns variants as variants_attributes", function(){
|
||||
it("does not return a product object for products which have no propeties other than an id", function(){
|
||||
expect( filterSubmitProducts( [ { id: 1, someunwantedproperty: "something" }, { id: 2, name: "p2"} ] ) ).toEqual( [ { id: 2, name: "p2" } ]);
|
||||
});
|
||||
|
||||
it("does not return an on_hand count when a product has variants", function(){
|
||||
var testProduct = {
|
||||
id: 1,
|
||||
on_hand: 5,
|
||||
variants: [ {
|
||||
id: 1,
|
||||
on_hand: 5,
|
||||
@@ -174,10 +179,10 @@ describe("Auxillary functions", function(){
|
||||
} ] );
|
||||
});
|
||||
|
||||
it("returns master as master_attributes", function(){
|
||||
it("returns variants as variants_attributes", function(){
|
||||
var testProduct = {
|
||||
id: 1,
|
||||
master: [ {
|
||||
variants: [ {
|
||||
id: 1,
|
||||
on_hand: 5,
|
||||
price: 12.0
|
||||
@@ -185,7 +190,7 @@ describe("Auxillary functions", function(){
|
||||
};
|
||||
expect( filterSubmitProducts( [ testProduct ] ) ).toEqual( [ {
|
||||
id: 1,
|
||||
master_attributes: [ {
|
||||
variants_attributes: [ {
|
||||
id: 1,
|
||||
on_hand: 5,
|
||||
price: 12.0
|
||||
@@ -222,6 +227,18 @@ describe("Auxillary functions", function(){
|
||||
} ] );
|
||||
});
|
||||
|
||||
it("does not return variants_attributes property if variants is an empty array", function(){
|
||||
var testProduct = {
|
||||
id: 1,
|
||||
price: 10,
|
||||
variants: []
|
||||
};
|
||||
expect( filterSubmitProducts( [ testProduct ] ) ).toEqual( [ {
|
||||
id: 1,
|
||||
price: 10
|
||||
} ] );
|
||||
});
|
||||
|
||||
// TODO Not an exhaustive test, is there a better way to do this?
|
||||
it("only returns properties the properties of products which ought to be updated", function(){
|
||||
var testProduct = {
|
||||
|
||||
Reference in New Issue
Block a user