From 4d24fec6fb32cf99367ac48c4459ca9485478306 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 13 Feb 2014 09:26:07 +1100 Subject: [PATCH] BPE: Add a variant. Do not show edit on unsaved variants. Delete unsaved variants. --- .../admin/bulk_product_update.js.coffee | 41 +++- .../spree/admin/products/bulk_edit.html.haml | 8 +- .../admin/bulk_product_update_spec.rb | 42 +++- .../unit/bulk_product_update_spec.js.coffee | 223 ++++++++++++------ 4 files changed, 230 insertions(+), 84 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 6ef5b2a267..eb809d084f 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -285,6 +285,17 @@ productEditModule.controller "AdminProductEditCtrl", [ window.location = "/admin/products/" + product.permalink_live + ((if variant then "/variants/" + variant.id else "")) + "/edit" + $scope.addVariant = (product) -> + product.variants.push {id: $scope.nextVariantId()} + $scope.displayProperties[product.id].showVariants = true + + + $scope.nextVariantId = -> + $scope.variantIdCounter = 0 unless $scope.variantIdCounter? + $scope.variantIdCounter -= 1 + $scope.variantIdCounter + + $scope.deleteProduct = (product) -> if confirm("Are you sure?") $http( @@ -297,14 +308,20 @@ productEditModule.controller "AdminProductEditCtrl", [ $scope.deleteVariant = (product, variant) -> - if confirm("Are you sure?") - $http( - method: "DELETE" - url: "/api/products/" + product.id + "/variants/" + variant.id - ).success (data) -> - product.variants.splice product.variants.indexOf(variant), 1 - delete $scope.dirtyProducts[product.id].variants[variant.id] if $scope.dirtyProducts.hasOwnProperty(product.id) and $scope.dirtyProducts[product.id].hasOwnProperty("variants") and $scope.dirtyProducts[product.id].variants.hasOwnProperty(variant.id) - $scope.displayDirtyProducts() + if !$scope.variantSaved(variant) + $scope.removeVariant(product, variant) + else + if confirm("Are you sure?") + $http( + method: "DELETE" + url: "/api/products/" + product.id + "/variants/" + variant.id + ).success (data) -> + $scope.removeVariant(product, variant) + + $scope.removeVariant = (product, variant) -> + product.variants.splice product.variants.indexOf(variant), 1 + delete $scope.dirtyProducts[product.id].variants[variant.id] if $scope.dirtyProducts.hasOwnProperty(product.id) and $scope.dirtyProducts[product.id].hasOwnProperty("variants") and $scope.dirtyProducts[product.id].variants.hasOwnProperty(variant.id) + $scope.displayDirtyProducts() $scope.cloneProduct = (product) -> @@ -325,6 +342,14 @@ productEditModule.controller "AdminProductEditCtrl", [ Object.keys(product.variants).length > 0 + $scope.hasUnit = (product) -> + product.variant_unit_with_scale? + + + $scope.variantSaved = (variant) -> + variant.hasOwnProperty('id') && variant.id > 0 + + $scope.hasOnDemandVariants = (product) -> (variant for id, variant of product.variants when variant.on_demand).length > 0 diff --git a/app/views/spree/admin/products/bulk_edit.html.haml b/app/views/spree/admin/products/bulk_edit.html.haml index 68e73e5da3..28cefe564f 100644 --- a/app/views/spree/admin/products/bulk_edit.html.haml +++ b/app/views/spree/admin/products/bulk_edit.html.haml @@ -111,6 +111,7 @@ %tr.product %td.left-actions %a{ 'ofn-toggle-variants' => 'true', :class => "view-variants icon-chevron-right", 'ng-show' => 'hasVariants(product)' } + %a{ :class => "add-variant icon-plus-sign", 'ng-click' => "addVariant(product)", 'ng-show' => "!hasVariants(product) && hasUnit(product)" } %td.supplier{ 'ng-show' => 'columns.supplier.visible' } %select.select2{ 'ng-model' => 'product.supplier', :name => 'supplier', 'ofn-track-product' => 'supplier', 'ng-options' => 's.name for s in suppliers' } %td{ 'ng-show' => 'columns.name.visible' } @@ -118,7 +119,7 @@ %td.unit{ 'ng-show' => 'columns.unit.visible' } %select.select2{ 'ng-model' => 'product.variant_unit_with_scale', :name => 'variant_unit_with_scale', 'ofn-track-product' => 'variant_unit_with_scale', 'ng-options' => 'unit[1] as unit[0] for unit in variant_unit_options' } %option{'value' => '', 'ng-hide' => "hasVariants(product)"} - %input{ 'ng-model' => 'product.master.unit_value_with_description', :name => 'master_unit_value_with_description', 'ofn-track-product' => 'master.unit_value_with_description', :type => 'text', :placeholder => 'value', 'ng-show' => "product.variant_unit_with_scale != undefined && !hasVariants(product)" } + %input{ 'ng-model' => 'product.master.unit_value_with_description', :name => 'master_unit_value_with_description', 'ofn-track-product' => 'master.unit_value_with_description', :type => 'text', :placeholder => 'value', 'ng-show' => "!hasVariants(product) && hasUnit(product)" } %input{ 'ng-model' => 'product.variant_unit_name', :name => 'variant_unit_name', 'ofn-track-product' => 'variant_unit_name', :placeholder => 'unit', 'ng-show' => "product.variant_unit_with_scale == 'items'", :type => 'text' } %td{ 'ng-show' => 'columns.price.visible' } %input{ 'ng-model' => 'product.price', 'ofn-decimal' => :true, :name => 'price', 'ofn-track-product' => 'price', :type => 'text' } @@ -135,7 +136,8 @@ %a{ 'ng-click' => 'deleteProduct(product)', :class => "delete-product icon-trash no-text" } %tr.variant{ 'ng-repeat' => 'variant in product.variants', 'ng-show' => 'displayProperties[product.id].showVariants', 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'" } %td.left-actions - %a{ :class => "variant-item icon-caret-right" } + %a{ :class => "variant-item icon-caret-right", 'ng-hide' => "$last" } + %a{ :class => "add-variant icon-plus-sign", 'ng-click' => "addVariant(product)", 'ng-show' => "$last" } %td{ 'ng-show' => 'columns.supplier.visible' } %td{ 'ng-show' => 'columns.name.visible' } {{ variant.options_text }} @@ -148,7 +150,7 @@ %span{ 'ng-bind' => 'variant.on_hand', :name => 'variant_on_hand', 'ng-show' => 'variant.on_demand' } %td{ 'ng-show' => 'columns.available_on.visible' } %td.actions - %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text" } + %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text", 'ng-show' => "variantSaved(variant)" } %td.actions %td.actions %a{ 'ng-click' => 'deleteVariant(product,variant)', :class => "delete-variant icon-trash no-text" } diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index fb57fa7254..0ff07ca0ab 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -229,7 +229,8 @@ feature %q{ end end - scenario "create a new product" do + + scenario "creating a new product" do s = FactoryGirl.create(:supplier_enterprise) d = FactoryGirl.create(:distributor_enterprise) @@ -253,6 +254,44 @@ feature %q{ page.should have_field "product_name", with: 'Big Bag Of Apples' end + + scenario "creating new variants" do + # Given a product without variants or a unit + p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil) + login_to_admin_section + visit '/admin/products/bulk_edit' + + # I should not see an add variant button + page.should_not have_selector 'a.add-variant', visible: true + + # When I set the unit + select "Weight (kg)", from: "variant_unit_with_scale" + + # I should see an add variant button + page.should have_selector 'a.add-variant', visible: true + + # When I add three variants + page.find('a.add-variant', visible: true).click + page.find('a.add-variant', visible: true).click + page.find('a.add-variant', visible: true).click + + # They should be added, and should see no edit buttons + page.all("tr.variant").count.should == 3 + page.should_not have_selector "a.edit-variant", visible: true + + # When I remove one, it should be removed + page.all('a.delete-variant').first.click + page.all("tr.variant").count.should == 2 + + # When I fill out variant details and hit update + + # Then the variants should be saved + + # And I should see edit buttons for the new variants + + end + + scenario "updating a product with no variants (except master)" do s1 = FactoryGirl.create(:supplier_enterprise) s2 = FactoryGirl.create(:supplier_enterprise) @@ -340,6 +379,7 @@ feature %q{ page.should have_field "variant_unit_value_with_description", with: "123 abc" end + describe "setting the master unit value for a product without variants" do it "sets the master unit value" do p = FactoryGirl.create(:product, variant_unit: nil, variant_unit_scale: nil) diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index 604fb4e9b4..f87c13a21a 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee @@ -603,6 +603,43 @@ describe "AdminProductEditCtrl", -> expect(scope.hasOnDemandVariants(product)).toBe(false) + describe "determining whether a product has variants", -> + it "returns true when it does", -> + product = + variants: [{id: 1}, {id: 2}] + expect(scope.hasVariants(product)).toBe(true) + + it "returns false when it does not", -> + product = + variants: [] + expect(scope.hasVariants(product)).toBe(false) + + + describe "determining whether a product has a unit", -> + it "returns true when it does", -> + product = + variant_unit_with_scale: 'weight_1000' + expect(scope.hasUnit(product)).toBe(true) + + it "returns false when its unit is undefined", -> + product = {} + expect(scope.hasUnit(product)).toBe(false) + + + describe "determining whether a variant has been saved", -> + it "returns true when it has a positive id", -> + variant = {id: 1} + expect(scope.variantSaved(variant)).toBe(true) + + it "returns false when it has no id", -> + variant = {} + expect(scope.variantSaved(variant)).toBe(false) + + it "returns false when it has a negative id", -> + variant = {id: -1} + expect(scope.variantSaved(variant)).toBe(false) + + describe "submitting products to be updated", -> describe "packing products", -> it "extracts variant_unit_with_scale into variant_unit and variant_unit_scale", -> @@ -889,6 +926,31 @@ describe "AdminProductEditCtrl", -> expect(scope.findProduct(123)).toBeNull() + describe "adding variants", -> + beforeEach -> + scope.displayProperties ||= {123: {}} + + it "adds the first variant", -> + product = {id: 123, variants: []} + scope.addVariant(product) + expect(product).toEqual + id: 123 + variants: [{id: -1}] + + it "adds subsequent variants", -> + product = {id: 123, variants: []} + scope.addVariant(product) + scope.addVariant(product) + expect(product).toEqual + id: 123 + variants: [{id: -1}, {id: -2}] + + it "shows the variant(s)", -> + product = {id: 123, variants: []} + scope.addVariant(product) + expect(scope.displayProperties[123].showVariants).toBe(true) + + describe "deleting products", -> it "deletes products with a http delete request to /api/products/id", -> spyOn(window, "confirm").andReturn true @@ -942,83 +1004,100 @@ describe "AdminProductEditCtrl", -> describe "deleting variants", -> - it "deletes variants with a http delete request to /api/products/product_id/variants/(variant_id)", -> - spyOn(window, "confirm").andReturn true - scope.products = [ - { - id: 9 - permalink_live: "apples" - variants: [ - id: 3 - price: 12 - ] - } - { - id: 13 - permalink_live: "oranges" - } - ] - scope.dirtyProducts = {} - httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data" - scope.deleteVariant scope.products[0], scope.products[0].variants[0] - httpBackend.flush() + describe "when the variant has not been saved", -> + it "removes the variant from products and dirtyProducts", -> + spyOn(window, "confirm").andReturn true + scope.products = [ + {id: 1, variants: [{id: -1}]} + ] + scope.dirtyProducts = + 1: {id: 1, variants: {'-1': {id: -1}}} + scope.deleteVariant scope.products[0], scope.products[0].variants[0] + expect(scope.products).toEqual([ + {id: 1, variants: []} + ]) + expect(scope.dirtyProducts).toEqual + 1: {id: 1, variants: {}} - it "removes the specified variant from both the variants object and scope.dirtyProducts (if it exists there)", -> - spyOn(window, "confirm").andReturn true - scope.products = [ - { - id: 9 - permalink_live: "apples" - variants: [ - { + + describe "when the variant has been saved", -> + it "deletes variants with a http delete request to /api/products/product_id/variants/(variant_id)", -> + spyOn(window, "confirm").andReturn true + scope.products = [ + { + id: 9 + permalink_live: "apples" + variants: [ id: 3 - price: 12.0 - } - { - id: 4 - price: 6.0 - } - ] - } - { - id: 13 - permalink_live: "oranges" - } - ] - scope.dirtyProducts = - 9: - id: 9 - variants: - 3: - id: 3 - price: 12.0 + price: 12 + ] + } + { + id: 13 + permalink_live: "oranges" + } + ] + scope.dirtyProducts = {} + httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data" + scope.deleteVariant scope.products[0], scope.products[0].variants[0] + httpBackend.flush() - 4: - id: 4 - price: 6.0 + it "removes the specified variant from both the variants object and scope.dirtyProducts (if it exists there)", -> + spyOn(window, "confirm").andReturn true + scope.products = [ + { + id: 9 + permalink_live: "apples" + variants: [ + { + id: 3 + price: 12.0 + } + { + id: 4 + price: 6.0 + } + ] + } + { + id: 13 + permalink_live: "oranges" + } + ] + scope.dirtyProducts = + 9: + id: 9 + variants: + 3: + id: 3 + price: 12.0 + + 4: + id: 4 + price: 6.0 + + 13: + id: 13 + name: "P1" - 13: - id: 13 - name: "P1" + httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data" + scope.deleteVariant scope.products[0], scope.products[0].variants[0] + httpBackend.flush() + expect(scope.products[0].variants).toEqual [ + id: 4 + price: 6.0 + ] + expect(scope.dirtyProducts).toEqual + 9: + id: 9 + variants: + 4: + id: 4 + price: 6.0 - httpBackend.expectDELETE("/api/products/9/variants/3").respond 200, "data" - scope.deleteVariant scope.products[0], scope.products[0].variants[0] - httpBackend.flush() - expect(scope.products[0].variants).toEqual [ - id: 4 - price: 6.0 - ] - expect(scope.dirtyProducts).toEqual - 9: - id: 9 - variants: - 4: - id: 4 - price: 6.0 - - 13: - id: 13 - name: "P1" + 13: + id: 13 + name: "P1"