From 295da25dd23aef786b845eee8ecf26025bee2e2c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Apr 2015 17:17:28 +1000 Subject: [PATCH 01/49] insert clone after cloned product --- .../javascripts/admin/services/bulk_products.js.coffee | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/services/bulk_products.js.coffee b/app/assets/javascripts/admin/services/bulk_products.js.coffee index d898356846..6085d48fcc 100644 --- a/app/assets/javascripts/admin/services/bulk_products.js.coffee +++ b/app/assets/javascripts/admin/services/bulk_products.js.coffee @@ -19,7 +19,7 @@ angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher) # when a respond_overrride for the clone action is used. id = data.product.id dataFetcher("/api/products/" + id + "?template=bulk_show").then (newProduct) => - @addProducts [newProduct] + @insertProductAfter(product, newProduct) updateVariantLists: (serverProducts, productsWithUnsavedVariants) -> for product in productsWithUnsavedVariants @@ -39,6 +39,10 @@ angular.module("ofn.admin").factory "BulkProducts", (PagedFetcher, dataFetcher) @unpackProduct product @products.push product + insertProductAfter: (product, newProduct) -> + index = @products.indexOf(product) + @products.splice(index + 1, 0, newProduct) + unpackProduct: (product) -> #$scope.matchProducer product @loadVariantUnit product From 66f847f673cd47c82238b094f7620a7c4ab1e983 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 30 Apr 2015 17:22:54 +1000 Subject: [PATCH 02/49] showing save button at the bottom as well --- app/views/spree/admin/products/bulk_edit.html.haml | 1 + 1 file changed, 1 insertion(+) diff --git a/app/views/spree/admin/products/bulk_edit.html.haml b/app/views/spree/admin/products/bulk_edit.html.haml index f230d1c331..383e080177 100644 --- a/app/views/spree/admin/products/bulk_edit.html.haml +++ b/app/views/spree/admin/products/bulk_edit.html.haml @@ -8,3 +8,4 @@ = render 'spree/admin/products/bulk_edit/actions' = render 'spree/admin/products/bulk_edit/indicators' = render 'spree/admin/products/bulk_edit/products' + = render 'spree/admin/products/bulk_edit/actions' From baabb5c07fa11d94a9ddbff464bdcf929152e823 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 May 2015 11:30:26 +1000 Subject: [PATCH 03/49] fixing BPE feature spec --- .../spree/admin/products/bulk_edit.html.haml | 2 +- .../bulk_edit/_save_button_row.html.haml | 3 ++ .../admin/bulk_product_update_spec.rb | 28 +++++++++---------- 3 files changed, 18 insertions(+), 15 deletions(-) create mode 100644 app/views/spree/admin/products/bulk_edit/_save_button_row.html.haml diff --git a/app/views/spree/admin/products/bulk_edit.html.haml b/app/views/spree/admin/products/bulk_edit.html.haml index 383e080177..22b0a195da 100644 --- a/app/views/spree/admin/products/bulk_edit.html.haml +++ b/app/views/spree/admin/products/bulk_edit.html.haml @@ -8,4 +8,4 @@ = render 'spree/admin/products/bulk_edit/actions' = render 'spree/admin/products/bulk_edit/indicators' = render 'spree/admin/products/bulk_edit/products' - = render 'spree/admin/products/bulk_edit/actions' + = render 'spree/admin/products/bulk_edit/save_button_row' diff --git a/app/views/spree/admin/products/bulk_edit/_save_button_row.html.haml b/app/views/spree/admin/products/bulk_edit/_save_button_row.html.haml new file mode 100644 index 0000000000..a5f18986c0 --- /dev/null +++ b/app/views/spree/admin/products/bulk_edit/_save_button_row.html.haml @@ -0,0 +1,3 @@ +%div.sixteen.columns.alpha{ 'ng-hide' => 'loading || products.length == 0', style: "margin-bottom: 10px" } + %div.four.columns.alpha + %input.four.columns.alpha{ :type => 'button', :value => 'Save Changes', 'ng-click' => 'submitProducts()'} diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index ac101da5d9..089f3188f7 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -266,7 +266,7 @@ feature %q{ fill_in "variant_display_as", with: "Case" fill_in "variant_price", with: "4.0" fill_in "variant_on_hand", with: "10" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." updated_variant = Spree::Variant.where(deleted_at: nil).last @@ -325,7 +325,7 @@ feature %q{ fill_in "product_sku", with: "NEW SKU" end - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload @@ -354,7 +354,7 @@ feature %q{ select "Items", from: "variant_unit_with_scale" fill_in "variant_unit_name", with: "loaf" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload @@ -377,7 +377,7 @@ feature %q{ first("a.view-variants").trigger('click') fill_in "variant_unit_value_with_description", with: '123 abc' - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload @@ -402,7 +402,7 @@ feature %q{ select "Weight (kg)", from: "variant_unit_with_scale" fill_in "master_unit_value_with_description", with: '123 abc' - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload @@ -450,7 +450,7 @@ feature %q{ expect(page).to have_selector "span[name='on_hand']", text: "10" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." v.reload @@ -474,7 +474,7 @@ feature %q{ fill_in "variant_price", with: "10.0" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." v.reload @@ -491,21 +491,21 @@ feature %q{ fill_in "product_name", with: "new name 1" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload expect(p.name).to eq "new name 1" fill_in "product_name", with: "new name 2" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload expect(p.name).to eq "new name 2" fill_in "product_name", with: "original name" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload expect(p.name).to eq "original name" @@ -521,7 +521,7 @@ feature %q{ fill_in "product_name", :with => "new product name" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload expect(p.name).to eq "new product name" @@ -534,7 +534,7 @@ feature %q{ visit '/admin/products/bulk_edit' - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "No changes to save." end end @@ -553,7 +553,7 @@ feature %q{ expect(page).to have_no_field "product_name", with: p2.name fill_in "product_name", :with => "new product1" - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p1.reload expect(p1.name).to eq "new product1" @@ -829,7 +829,7 @@ feature %q{ fill_in "display_as", with: "Big Bag" end - click_button 'Save Changes' + first(:button, 'Save Changes').click expect(page.find("#status-message")).to have_content "Changes saved." p.reload From 993183f2f57b79c8d3d981e60a4d47fde759df60 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 1 May 2015 15:04:12 +1000 Subject: [PATCH 04/49] updating js spec: cloning product calls insertProductAfter now --- .../unit/admin/services/bulk_products_spec.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee index 70c22c80e0..0b3c0841a6 100644 --- a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee @@ -61,7 +61,7 @@ describe "BulkProducts service", -> clonedProduct = id: 17 - spyOn(BulkProducts, "addProducts") + spyOn(BulkProducts, "insertProductAfter") BulkProducts.products = [originalProduct] $httpBackend.expectGET("/admin/products/oranges/clone.json").respond 200, product: clonedProduct From 6b956a8a38c904347fd95a482b80f12eed3a1377 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 May 2015 10:19:37 +1000 Subject: [PATCH 05/49] Updating product clone spec --- .../unit/admin/services/bulk_products_spec.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee index 0b3c0841a6..54b57aed48 100644 --- a/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/bulk_products_spec.js.coffee @@ -68,7 +68,7 @@ describe "BulkProducts service", -> $httpBackend.expectGET("/api/products/17?template=bulk_show").respond 200, clonedProduct BulkProducts.cloneProduct BulkProducts.products[0] $httpBackend.flush() - expect(BulkProducts.addProducts).toHaveBeenCalledWith [clonedProduct] + expect(BulkProducts.insertProductAfter).toHaveBeenCalledWith originalProduct, clonedProduct describe "preparing products", -> From 828456118b23429d9c48696fb88e4fdc08d5f697 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 May 2015 11:15:18 +1000 Subject: [PATCH 06/49] Remove forgotten binding.pry --- app/models/spree/ability_decorator.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 7bc6fb6535..e652825911 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -154,7 +154,6 @@ class AbilityDecorator can [:admin, :index, :read, :create, :edit, :update, :fire], Spree::ReturnAuthorization can [:destroy], Spree::Adjustment do |adjustment| # Sharing code with destroying a line item. This should be unified and probably applied for other actions as well. - binding.pry if user.admin? true elsif adjustment.adjustable.instance_of? Spree::Order From 0a0bb67277ed6ee1572b71fe6317320ad4663444 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 May 2015 14:05:14 +1000 Subject: [PATCH 07/49] No SKU for cloned products. Community topic 175 --- app/models/spree/product_decorator.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 7846a0c1e1..fcaf30c44c 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -108,6 +108,12 @@ Spree::Product.class_eval do # -- Methods + # Called by Spree::Product::duplicate before saving. + def duplicate_extra(parent) + # Spree sets the SKU to "COPY OF #{parent sku}". + self.master.sku = '' + end + def properties_including_inherited # Product properties override producer properties ps = product_properties.all From 6953f6193974f404ee6058a195e73f4100675b1c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 May 2015 15:07:22 +1000 Subject: [PATCH 08/49] bulk product edit: new col "on demand" --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 4 ++++ .../spree/admin/products/bulk_edit/_products_head.html.haml | 2 ++ .../admin/products/bulk_edit/_products_product.html.haml | 2 ++ 3 files changed, 8 insertions(+) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 8cac0abeda..673197ae6d 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -10,6 +10,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout unit: {name: "Unit", visible: true} price: {name: "Price", visible: true} on_hand: {name: "On Hand", visible: true} + on_demand: {name: "On Demand", visible: false} category: {name: "Category", visible: false} inherits_properties: {name: "Inherits Properties?", visible: false} available_on: {name: "Available On", visible: false} @@ -309,6 +310,9 @@ filterSubmitProducts = (productsToFilter) -> if product.hasOwnProperty("on_hand") and filteredVariants.length == 0 #only update if no variants present filteredProduct.on_hand = product.on_hand hasUpdatableProperty = true + if product.hasOwnProperty("on_demand") + filteredProduct.on_demand = product.on_demand + hasUpdatableProperty = true if product.hasOwnProperty("category_id") filteredProduct.primary_taxon_id = product.category_id hasUpdatableProperty = true diff --git a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml index 2e37da2bc8..18bd000473 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml @@ -7,6 +7,7 @@ %col.display_as{ ng: { show: 'columns.unit.visible' } } %col.price{ ng: { show: 'columns.price.visible'} } %col.on_hand{ ng: { show: 'columns.on_hand.visible' } } + %col.on_demand{ ng: { show: 'columns.on_demand.visible' } } %col.category{ ng: { show: 'columns.category.visible' } } %col.inherits_properties{ ng: { show: 'columns.inherits_properties.visible' } } %col.available_on{ ng: { show: 'columns.available_on.visible' } } @@ -24,6 +25,7 @@ %th.display_as{ 'ng-show' => 'columns.unit.visible' } Display As %th.price{ 'ng-show' => 'columns.price.visible' } Price %th.on_hand{ 'ng-show' => 'columns.on_hand.visible' } On Hand + %th.on_demand{ 'ng-show' => 'columns.on_demand.visible' } On Demand %th.category{ 'ng-show' => 'columns.category.visible' } Category %th.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' } Inherits Properties? %th.available_on{ 'ng-show' => 'columns.available_on.visible' } Av. On diff --git a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml index 0eb9ff5f58..48564991dd 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml @@ -20,6 +20,8 @@ %td.on_hand{ 'ng-show' => 'columns.on_hand.visible' } %span{ 'ng-bind' => 'product.on_hand', :name => 'on_hand', 'ng-show' => '!hasOnDemandVariants(product) && (hasVariants(product) || product.on_demand)' } %input.field{ 'ng-model' => 'product.on_hand', :name => 'on_hand', 'ofn-track-product' => 'on_hand', 'ng-hide' => 'hasVariants(product) || product.on_demand', :type => 'number' } + %td.on_demand{ 'ng-show' => 'columns.on_demand.visible' } + %input.field{ 'ng-model' => 'product.on_demand', :name => 'on_demand', 'ofn-track-product' => 'on_demand', :type => 'checkbox' } %td.category{ 'ng-if' => 'columns.category.visible' } %input.fullwidth{ :type => 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'product.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-product' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' } %td.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' } From 1e26466d119134a48cf73c5bee87d4215bbb47ee Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 20 May 2015 15:29:35 +1000 Subject: [PATCH 09/49] bulk product edit: "on demand" for new product --- .../admin/products/new/replace_form.html.haml.deface | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface index 33c70b728e..3fd5a74cb1 100644 --- a/app/overrides/spree/admin/products/new/replace_form.html.haml.deface +++ b/app/overrides/spree/admin/products/new/replace_form.html.haml.deface @@ -38,20 +38,26 @@ .twelve.columns.alpha .six.columns.alpha = render 'spree/admin/products/primary_taxon_form', f: f - .three.columns + .two.columns = f.field_container :price do = f.label :price, t(:price) %span.required * %br/ = f.text_field :price, class: 'fullwidth' = f.error_message_on :price - .three.columns.omega + .two.columns = f.field_container :on_hand do = f.label :on_hand, t(:on_hand) %span.required * %br/ = f.text_field :on_hand, class: 'fullwidth' = f.error_message_on :on_hand + .two.columns.omega + = f.field_container :on_demand do + = f.label :on_demand, t(:on_demand) + %br/ + = f.check_box :on_demand + = f.error_message_on :on_demand .twelve.columns.alpha .six.columns.alpha   .three.columns From 19367670845a9bc63a619bee11dd27af6be283bd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 21 May 2015 11:06:08 +1000 Subject: [PATCH 10/49] BPE: new col for tax category --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 7 ++++++- app/helpers/admin/injection_helper.rb | 4 ++++ app/serializers/api/admin/product_serializer.rb | 2 +- app/serializers/api/admin/tax_category_serializer.rb | 3 +++ app/views/spree/admin/products/bulk_edit/_data.html.haml | 1 + .../admin/products/bulk_edit/_products_head.html.haml | 2 ++ .../admin/products/bulk_edit/_products_product.html.haml | 3 +++ 7 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 app/serializers/api/admin/tax_category_serializer.rb diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 673197ae6d..5098152551 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -1,4 +1,4 @@ -angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $http, BulkProducts, DisplayProperties, dataFetcher, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, SpreeApiAuth) -> +angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $http, BulkProducts, DisplayProperties, dataFetcher, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, SpreeApiAuth, tax_categories) -> $scope.loading = true $scope.StatusMessage = StatusMessage @@ -12,6 +12,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout on_hand: {name: "On Hand", visible: true} on_demand: {name: "On Demand", visible: false} category: {name: "Category", visible: false} + tax_category: {name: "Tax Category", visible: false} inherits_properties: {name: "Inherits Properties?", visible: false} available_on: {name: "Available On", visible: false} @@ -33,6 +34,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout $scope.producers = producers $scope.taxons = Taxons.taxons + $scope.tax_categories = tax_categories $scope.filterProducers = [{id: "0", name: ""}].concat $scope.producers $scope.filterTaxons = [{id: "0", name: ""}].concat $scope.taxons $scope.producerFilter = "0" @@ -316,6 +318,9 @@ filterSubmitProducts = (productsToFilter) -> if product.hasOwnProperty("category_id") filteredProduct.primary_taxon_id = product.category_id hasUpdatableProperty = true + if product.hasOwnProperty("tax_category_id") + filteredProduct.tax_category_id = product.tax_category_id + hasUpdatableProperty = true if product.hasOwnProperty("inherits_properties") filteredProduct.inherits_properties = product.inherits_properties hasUpdatableProperty = true diff --git a/app/helpers/admin/injection_helper.rb b/app/helpers/admin/injection_helper.rb index 36ddccdb9a..961bc60afb 100644 --- a/app/helpers/admin/injection_helper.rb +++ b/app/helpers/admin/injection_helper.rb @@ -50,6 +50,10 @@ module Admin admin_inject_json_ams_array "ofn.admin", "products", @products, Api::Admin::ProductSerializer end + def admin_inject_tax_categories + admin_inject_json_ams_array "ofn.admin", "tax_categories", @tax_categories, Api::Admin::TaxCategorySerializer + end + def admin_inject_taxons admin_inject_json_ams_array "admin.taxons", "taxons", @taxons, Api::Admin::TaxonSerializer end diff --git a/app/serializers/api/admin/product_serializer.rb b/app/serializers/api/admin/product_serializer.rb index b1f233c044..65dd34ba6b 100644 --- a/app/serializers/api/admin/product_serializer.rb +++ b/app/serializers/api/admin/product_serializer.rb @@ -1,7 +1,7 @@ class Api::Admin::ProductSerializer < ActiveModel::Serializer attributes :id, :name, :sku, :variant_unit, :variant_unit_scale, :variant_unit_name, :on_demand, :inherits_properties - attributes :on_hand, :price, :available_on, :permalink_live + attributes :on_hand, :price, :available_on, :permalink_live, :tax_category_id has_one :supplier, key: :producer_id, embed: :id has_one :primary_taxon, key: :category_id, embed: :id diff --git a/app/serializers/api/admin/tax_category_serializer.rb b/app/serializers/api/admin/tax_category_serializer.rb new file mode 100644 index 0000000000..37e99578b8 --- /dev/null +++ b/app/serializers/api/admin/tax_category_serializer.rb @@ -0,0 +1,3 @@ +class Api::Admin::TaxCategorySerializer < ActiveModel::Serializer + attributes :id, :name +end diff --git a/app/views/spree/admin/products/bulk_edit/_data.html.haml b/app/views/spree/admin/products/bulk_edit/_data.html.haml index 8b727be3eb..3624421870 100644 --- a/app/views/spree/admin/products/bulk_edit/_data.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_data.html.haml @@ -1,3 +1,4 @@ = admin_inject_producers = admin_inject_taxons += admin_inject_tax_categories = admin_inject_spree_api_key diff --git a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml index 18bd000473..6e22aef8ff 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml @@ -9,6 +9,7 @@ %col.on_hand{ ng: { show: 'columns.on_hand.visible' } } %col.on_demand{ ng: { show: 'columns.on_demand.visible' } } %col.category{ ng: { show: 'columns.category.visible' } } + %col.tax_category{ ng: { show: 'columns.tax_category.visible' } } %col.inherits_properties{ ng: { show: 'columns.inherits_properties.visible' } } %col.available_on{ ng: { show: 'columns.available_on.visible' } } %col.actions @@ -27,6 +28,7 @@ %th.on_hand{ 'ng-show' => 'columns.on_hand.visible' } On Hand %th.on_demand{ 'ng-show' => 'columns.on_demand.visible' } On Demand %th.category{ 'ng-show' => 'columns.category.visible' } Category + %th.tax_category{ 'ng-show' => 'columns.tax_category.visible' } Tax Category %th.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' } Inherits Properties? %th.available_on{ 'ng-show' => 'columns.available_on.visible' } Av. On %th.actions diff --git a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml index 48564991dd..eab9936558 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml @@ -24,6 +24,9 @@ %input.field{ 'ng-model' => 'product.on_demand', :name => 'on_demand', 'ofn-track-product' => 'on_demand', :type => 'checkbox' } %td.category{ 'ng-if' => 'columns.category.visible' } %input.fullwidth{ :type => 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'product.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-product' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' } + %td.tax_category{ 'ng-if' => 'columns.tax_category.visible' } + %select.select2{ name: 'product_tax_category_id', 'ofn-track-product' => 'tax_category_id', ng: {model: 'product.tax_category_id', options: 'tax_category.id as tax_category.name for tax_category in tax_categories'} } + %option{value: ''} None %td.inherits_properties{ 'ng-show' => 'columns.inherits_properties.visible' } %input{ 'ng-model' => 'product.inherits_properties', :name => 'inherits_properties', 'ofn-track-product' => 'inherits_properties', type: "checkbox" } %td.available_on{ 'ng-show' => 'columns.available_on.visible' } From 2ed519ef50b93d1b6c6d0c743533cd6da8fa981b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 21 May 2015 12:37:10 +1000 Subject: [PATCH 11/49] on_demand checkbox for variants --- app/assets/javascripts/admin/bulk_product_update.js.coffee | 5 ++++- .../admin/products/bulk_edit/_products_product.html.haml | 2 +- .../admin/products/bulk_edit/_products_variant.html.haml | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 5098152551..c902328af4 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -312,7 +312,7 @@ filterSubmitProducts = (productsToFilter) -> if product.hasOwnProperty("on_hand") and filteredVariants.length == 0 #only update if no variants present filteredProduct.on_hand = product.on_hand hasUpdatableProperty = true - if product.hasOwnProperty("on_demand") + if product.hasOwnProperty("on_demand") and filteredVariants.length == 0 #only update if no variants present filteredProduct.on_demand = product.on_demand hasUpdatableProperty = true if product.hasOwnProperty("category_id") @@ -346,6 +346,9 @@ filterSubmitVariant = (variant) -> if variant.hasOwnProperty("on_hand") filteredVariant.on_hand = variant.on_hand hasUpdatableProperty = true + if variant.hasOwnProperty("on_demand") + filteredVariant.on_demand = variant.on_demand + hasUpdatableProperty = true if variant.hasOwnProperty("price") filteredVariant.price = variant.price hasUpdatableProperty = true diff --git a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml index eab9936558..376e88071a 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml @@ -21,7 +21,7 @@ %span{ 'ng-bind' => 'product.on_hand', :name => 'on_hand', 'ng-show' => '!hasOnDemandVariants(product) && (hasVariants(product) || product.on_demand)' } %input.field{ 'ng-model' => 'product.on_hand', :name => 'on_hand', 'ofn-track-product' => 'on_hand', 'ng-hide' => 'hasVariants(product) || product.on_demand', :type => 'number' } %td.on_demand{ 'ng-show' => 'columns.on_demand.visible' } - %input.field{ 'ng-model' => 'product.on_demand', :name => 'on_demand', 'ofn-track-product' => 'on_demand', :type => 'checkbox' } + %input.field{ 'ng-model' => 'product.on_demand', :name => 'on_demand', 'ofn-track-product' => 'on_demand', :type => 'checkbox', 'ng-hide' => 'hasVariants(product)' } %td.category{ 'ng-if' => 'columns.category.visible' } %input.fullwidth{ :type => 'text', id: "p{{product.id}}_category_id", 'ng-model' => 'product.category_id', 'ofn-taxon-autocomplete' => '', 'ofn-track-product' => 'category_id', 'multiple-selection' => 'false', placeholder: 'Category' } %td.tax_category{ 'ng-if' => 'columns.tax_category.visible' } diff --git a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml index 7e60cbfd4c..757b64e781 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml @@ -15,7 +15,10 @@ %td{ 'ng-show' => 'columns.on_hand.visible' } %input.field{ 'ng-model' => 'variant.on_hand', 'ng-change' => 'updateOnHand(product)', :name => 'variant_on_hand', 'ng-hide' => 'variant.on_demand', 'ofn-track-variant' => 'on_hand', :type => 'number' } %span{ 'ng-bind' => 'variant.on_hand', :name => 'variant_on_hand', 'ng-show' => 'variant.on_demand' } + %td{ 'ng-show' => 'columns.on_demand.visible' } + %input.field{ 'ng-model' => 'variant.on_demand', :name => 'variant_on_demand', 'ofn-track-variant' => 'on_demand', :type => 'checkbox' } %td{ 'ng-show' => 'columns.category.visible' } + %td{ 'ng-show' => 'columns.tax_category.visible' } %td{ 'ng-show' => 'columns.inherits_properties.visible' } %td{ 'ng-show' => 'columns.available_on.visible' } %td.actions From 73884d4f019bebb3ffc0a3df94e1341f1fc81645 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 21 May 2015 13:01:28 +1000 Subject: [PATCH 12/49] BPE: Display notice about variant overrides --- app/models/spree/variant_decorator.rb | 1 + app/serializers/api/admin/variant_serializer.rb | 1 + .../spree/admin/products/bulk_edit/_products_variant.html.haml | 1 + 3 files changed, 3 insertions(+) diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index 03543793b2..f35927ccf7 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -7,6 +7,7 @@ Spree::Variant.class_eval do has_many :exchange_variants, dependent: :destroy has_many :exchanges, through: :exchange_variants + has_many :variant_overrides attr_accessible :unit_value, :unit_description, :images_attributes, :display_as, :display_name accepts_nested_attributes_for :images diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 26ad2d7f37..510f7af333 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -1,6 +1,7 @@ class Api::Admin::VariantSerializer < ActiveModel::Serializer attributes :id, :options_text, :unit_value, :unit_description, :unit_to_display, :on_demand, :display_as, :display_name, :name_to_display attributes :on_hand, :price + has_many :variant_overrides def on_hand object.on_hand.nil? ? 0 : ( object.on_hand.to_f.finite? ? object.on_hand : "On demand" ) diff --git a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml index 757b64e781..853d8e1520 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_variant.html.haml @@ -24,5 +24,6 @@ %td.actions %a{ 'ng-click' => 'editWarn(product,variant)', :class => "edit-variant icon-edit no-text", 'ng-show' => "variantSaved(variant)" } %td.actions + %span.icon-warning-sign.with-tip{ 'ng-if' => 'variant.variant_overrides', title: "This variant has {{variant.variant_overrides.length}} override(s)" } %td.actions %a{ 'ng-click' => 'deleteVariant(product,variant)', :class => "delete-variant icon-trash no-text" } From f017197221121b65866b7d7c560c0390b39a9aa6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 21 May 2015 18:00:14 +1000 Subject: [PATCH 13/49] orders list: filter by distributor and order cycle --- .../spree/admin/orders_controller_decorator.rb | 7 +++++++ ...r_and_order_cycle_filter_inputs.html.haml.deface | 13 +++++++++++++ 2 files changed, 20 insertions(+) create mode 100644 app/overrides/spree/admin/orders/index/add_distributor_and_order_cycle_filter_inputs.html.haml.deface diff --git a/app/controllers/spree/admin/orders_controller_decorator.rb b/app/controllers/spree/admin/orders_controller_decorator.rb index 42e7068afc..ac3d0fa9e0 100644 --- a/app/controllers/spree/admin/orders_controller_decorator.rb +++ b/app/controllers/spree/admin/orders_controller_decorator.rb @@ -23,6 +23,13 @@ Spree::Admin::OrdersController.class_eval do distributed_by_user(spree_current_user). page(params[:page]). per(params[:per_page] || Spree::Config[:orders_per_page]) + # Filter orders by distributor + if params[:distributor_ids] + @orders = @orders.where(distributor_id: params[:distributor_ids]) + end + if params[:order_cycle_ids] + @orders = @orders.where(order_cycle_id: params[:order_cycle_ids]) + end } } } # Overwrite to use confirm_email_for_customer instead of confirm_email. diff --git a/app/overrides/spree/admin/orders/index/add_distributor_and_order_cycle_filter_inputs.html.haml.deface b/app/overrides/spree/admin/orders/index/add_distributor_and_order_cycle_filter_inputs.html.haml.deface new file mode 100644 index 0000000000..f9e5e54e88 --- /dev/null +++ b/app/overrides/spree/admin/orders/index/add_distributor_and_order_cycle_filter_inputs.html.haml.deface @@ -0,0 +1,13 @@ +/ insert_before "div.clearfix" + +.field-block.alpha.eight.columns + = label_tag nil, t(:distributors) + = select_tag(:distributor_ids, + options_for_select(Enterprise.is_distributor.managed_by(spree_current_user).map {|e| [e.name, e.id]}, params[:distributor_ids]), + {class: "select2 fullwidth", multiple: true}) + +.field-block.alpha.eight.columns + = label_tag nil, t(:order_cycles) + = select_tag(:order_cycle_ids, + options_for_select(OrderCycle.managed_by(spree_current_user).map {|oc| [oc.name, oc.id]}, params[:order_cycle_ids]), + {class: "select2 fullwidth", multiple: true}) From 7e4751cb3ae549fd3d1ad7bac8fd550ad38e9cbb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 21 May 2015 22:30:23 +1000 Subject: [PATCH 14/49] updating bulk product js spec --- spec/javascripts/unit/bulk_product_update_spec.js.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index a6fb0fa432..6048814505 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee @@ -215,6 +215,7 @@ describe "filtering products for submission to database", -> variant_unit_scale: 1 variant_unit_name: 'loaf' available_on: available_on + tax_category_id: null master_attributes: id: 2 unit_value: 250 From 9c137ccf0faafde9f0c26fd9ad8a1b373dc7220f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 22 May 2015 11:03:21 +1000 Subject: [PATCH 15/49] provide tax_categories in spec --- spec/javascripts/unit/bulk_product_update_spec.js.coffee | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index 6048814505..fb439f1c0c 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee @@ -239,6 +239,7 @@ describe "AdminProductEditCtrl", -> module ($provide)-> $provide.value "producers", [] $provide.value "taxons", [] + $provide.value "tax_categories", [] $provide.value 'SpreeApiKey', 'API_KEY' null From 99cb09c6f75bfc0889ccac2e9ec09f6958e45b44 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 20 May 2015 14:05:41 +1000 Subject: [PATCH 16/49] When loading products for shopfront, load all variants in one go --- app/controllers/shop_controller.rb | 28 ++++++++++++++++++--- app/serializers/api/product_serializer.rb | 14 +++-------- spec/controllers/shop_controller_spec.rb | 14 +++++++++++ spec/serializers/product_serializer_spec.rb | 14 ----------- 4 files changed, 42 insertions(+), 28 deletions(-) delete mode 100644 spec/serializers/product_serializer_spec.rb diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index 73861def5c..35aece5e1d 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -10,12 +10,15 @@ class ShopController < BaseController end def products - # Can we make this query less slow? - # if @products = products_for_shop + render status: 200, - json: ActiveModel::ArraySerializer.new(@products, each_serializer: Api::ProductSerializer, - current_order_cycle: current_order_cycle, current_distributor: current_distributor).to_json + json: ActiveModel::ArraySerializer.new(@products, + each_serializer: Api::ProductSerializer, + current_order_cycle: current_order_cycle, + current_distributor: current_distributor, + variants: variants_for_shop_by_id).to_json + else render json: "", status: 404 end @@ -46,6 +49,23 @@ class ShopController < BaseController end end + def variants_for_shop_by_id + # We use the in_stock? method here instead of the in_stock scope because we need to + # look up the stock as overridden by VariantOverrides, and the scope method is not affected + # by them. + variants = Spree::Variant. + where(is_master: false). + for_distribution(current_order_cycle, current_distributor). + each { |v| v.scope_to_hub current_distributor }. + select(&:in_stock?) + + variants.inject({}) do |vs, v| + vs[v.product_id] ||= [] + vs[v.product_id] << v + vs + end + end + def taxon_order if current_distributor.preferred_shopfront_taxon_order.present? current_distributor diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index 0de794796b..b707aa281c 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -30,8 +30,9 @@ class Api::CachedProductSerializer < ActiveModel::Serializer #cached #delegate :cache_key, to: :object - attributes :id, :name, :permalink, :count_on_hand, :on_demand, :group_buy, - :notes, :description, :properties_with_values + attributes :id, :name, :permalink, :count_on_hand + attributes :on_demand, :group_buy, :notes, :description + attributes :properties_with_values has_many :variants, serializer: Api::VariantSerializer has_many :taxons, serializer: Api::IdSerializer @@ -46,13 +47,6 @@ class Api::CachedProductSerializer < ActiveModel::Serializer end def variants - # We use the in_stock? method here instead of the in_stock scope because we need to - # look up the stock as overridden by VariantOverrides, and the scope method is not affected - # by them. - - object.variants. - for_distribution(options[:current_order_cycle], options[:current_distributor]). - each { |v| v.scope_to_hub options[:current_distributor] }. - select(&:in_stock?) + options[:variants][object.id] || [] end end diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb index 4b7c53da19..10f0eed2cd 100644 --- a/spec/controllers/shop_controller_spec.rb +++ b/spec/controllers/shop_controller_spec.rb @@ -175,4 +175,18 @@ describe ShopController do end end end + + describe "loading variants" do + let(:hub) { create(:distributor_enterprise) } + let(:oc) { create(:simple_order_cycle, distributors: [hub], variants: [v1]) } + let(:p) { create(:simple_product) } + let!(:v1) { create(:variant, product: p, unit_value: 3) } + let!(:v2) { create(:variant, product: p, unit_value: 5) } + + it "scopes variants to distribution" do + controller.stub(:current_order_cycle) { oc } + controller.stub(:current_distributor) { hub } + controller.send(:variants_for_shop_by_id).should == {p.id => [v1]} + end + end end diff --git a/spec/serializers/product_serializer_spec.rb b/spec/serializers/product_serializer_spec.rb deleted file mode 100644 index 0091668dbb..0000000000 --- a/spec/serializers/product_serializer_spec.rb +++ /dev/null @@ -1,14 +0,0 @@ -describe Api::ProductSerializer do - let(:hub) { create(:distributor_enterprise) } - let(:oc) { create(:simple_order_cycle, distributors: [hub], variants: [v1]) } - let(:p) { create(:simple_product) } - let!(:v1) { create(:variant, product: p, unit_value: 3) } - let!(:v2) { create(:variant, product: p, unit_value: 5) } - - it "scopes variants to distribution" do - s = Api::ProductSerializer.new p, current_distributor: hub, current_order_cycle: oc - json = s.to_json - json.should include v1.options_text - json.should_not include v2.options_text - end -end From c5f00d87bd3b63a750f5282583084b84e6e31df6 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 20 May 2015 15:22:01 +1000 Subject: [PATCH 17/49] When loading products for shopfront, load all master variants in one go --- app/controllers/shop_controller.rb | 46 ++++++++++++++--------- app/serializers/api/product_serializer.rb | 5 +++ 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index 35aece5e1d..0655624a0b 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -17,7 +17,8 @@ class ShopController < BaseController each_serializer: Api::ProductSerializer, current_order_cycle: current_order_cycle, current_distributor: current_distributor, - variants: variants_for_shop_by_id).to_json + variants: variants_for_shop_by_id, + master_variants: master_variants_for_shop_by_id).to_json else render json: "", status: 404 @@ -49,23 +50,6 @@ class ShopController < BaseController end end - def variants_for_shop_by_id - # We use the in_stock? method here instead of the in_stock scope because we need to - # look up the stock as overridden by VariantOverrides, and the scope method is not affected - # by them. - variants = Spree::Variant. - where(is_master: false). - for_distribution(current_order_cycle, current_distributor). - each { |v| v.scope_to_hub current_distributor }. - select(&:in_stock?) - - variants.inject({}) do |vs, v| - vs[v.product_id] ||= [] - vs[v.product_id] << v - vs - end - end - def taxon_order if current_distributor.preferred_shopfront_taxon_order.present? current_distributor @@ -76,4 +60,30 @@ class ShopController < BaseController "name ASC" end end + + def all_variants_for_shop + # We use the in_stock? method here instead of the in_stock scope because we need to + # look up the stock as overridden by VariantOverrides, and the scope method is not affected + # by them. + Spree::Variant. + for_distribution(current_order_cycle, current_distributor). + each { |v| v.scope_to_hub current_distributor }. + select(&:in_stock?) + end + + def variants_for_shop_by_id + index_by_product_id all_variants_for_shop.reject(&:is_master) + end + + def master_variants_for_shop_by_id + index_by_product_id all_variants_for_shop.select(&:is_master) + end + + def index_by_product_id(variants) + variants.inject({}) do |vs, v| + vs[v.product_id] ||= [] + vs[v.product_id] << v + vs + end + end end diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index b707aa281c..4c4aec2310 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -49,4 +49,9 @@ class Api::CachedProductSerializer < ActiveModel::Serializer def variants options[:variants][object.id] || [] end + + def master + options[:master_variants][object.id].andand.first + end + end From 769edbe9d52aebbb6cef71c3a06e90069f789b29 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 12:04:59 +1000 Subject: [PATCH 18/49] Find the earliest closing times for each distributor in an active order cycle --- app/models/order_cycle.rb | 16 +++++++++++++++- spec/models/order_cycle_spec.rb | 31 +++++++++++++++++++++++++------ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 88e4953edc..dfd45230b6 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -92,11 +92,25 @@ class OrderCycle < ActiveRecord::Base with_distributor(distributor).soonest_closing.first end - def self.most_recently_closed_for(distributor) with_distributor(distributor).most_recently_closed.first end + # Find the earliest closing times for each distributor in an active order cycle, and return + # them in the format {distributor_id => closing_time, ...} + def self.earliest_closing_times + Hash[ + Exchange. + outgoing. + joins(:order_cycle). + merge(OrderCycle.active). + group('exchanges.receiver_id'). + select('exchanges.receiver_id AS receiver_id, MIN(order_cycles.orders_close_at) AS earliest_close_at'). + map { |ex| [ex.receiver_id, ex.earliest_close_at.to_time] } + ] + end + + def clone! oc = self.dup oc.name = "COPY OF #{oc.name}" diff --git a/spec/models/order_cycle_spec.rb b/spec/models/order_cycle_spec.rb index 817d2e8e93..507f9977b1 100644 --- a/spec/models/order_cycle_spec.rb +++ b/spec/models/order_cycle_spec.rb @@ -313,7 +313,7 @@ describe OrderCycle do @oc.pickup_time_for(@d2).should == '2-8pm Friday' end end - + describe "finding pickup instructions for a distributor" do it "returns the pickup instructions" do @oc.pickup_instructions_for(@d1).should == "Come get it!" @@ -375,7 +375,7 @@ describe OrderCycle do occ.coordinator_fee_ids.should_not be_empty occ.coordinator_fee_ids.should == oc.coordinator_fee_ids - + # to_h gives us a unique hash for each exchange # check that the clone has no additional exchanges occ.exchanges.map(&:to_h).all? do |ex| @@ -402,7 +402,7 @@ describe OrderCycle do describe "finding order cycles opening in the future" do it "should give the soonest opening order cycle for a distributor" do distributor = create(:distributor_enterprise) - oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now) + oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 10.days.from_now, orders_close_at: 11.days.from_now) OrderCycle.first_opening_for(distributor).should == oc end @@ -411,13 +411,32 @@ describe OrderCycle do OrderCycle.first_opening_for(distributor).should == nil end end - + describe "finding open order cycles" do it "should give the soonest closing order cycle for a distributor" do distributor = create(:distributor_enterprise) - oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now) - oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now) + oc = create(:simple_order_cycle, name: 'oc 1', distributors: [distributor], orders_open_at: 1.days.ago, orders_close_at: 11.days.from_now) + oc2 = create(:simple_order_cycle, name: 'oc 2', distributors: [distributor], orders_open_at: 2.days.ago, orders_close_at: 12.days.from_now) OrderCycle.first_closing_for(distributor).should == oc end end + + describe "finding the earliest closing times for each distributor" do + let(:time1) { 1.week.from_now } + let(:time2) { 2.weeks.from_now } + let(:time3) { 3.weeks.from_now } + let(:e1) { create(:distributor_enterprise) } + let(:e2) { create(:distributor_enterprise) } + let!(:oc1) { create(:simple_order_cycle, orders_close_at: time1, distributors: [e1]) } + let!(:oc2) { create(:simple_order_cycle, orders_close_at: time2, distributors: [e2]) } + let!(:oc3) { create(:simple_order_cycle, orders_close_at: time3, distributors: [e2]) } + + it "returns the closing time, indexed by enterprise id" do + OrderCycle.earliest_closing_times[e1.id].should == time1 + end + + it "returns the earliest closing time" do + OrderCycle.earliest_closing_times[e2.id].should == time2 + end + end end From f940984ca311313e3457da6ec98eb5c044c609f0 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 12:07:11 +1000 Subject: [PATCH 19/49] Pull earliest closing time computations out of the serialization loop --- app/helpers/injection_helper.rb | 2 +- app/serializers/api/enterprise_serializer.rb | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 37794cef9d..a17d6e81f9 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,6 +1,6 @@ module InjectionHelper def inject_enterprises - inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, active_distributors: @active_distributors + inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, active_distributors: @active_distributors, earliest_closing_times: OrderCycle.earliest_closing_times end def inject_current_order diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index 532887ae01..626fe77cad 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -18,14 +18,12 @@ class Api::UncachedEnterpriseSerializer < ActiveModel::Serializer attributes :orders_close_at, :active def orders_close_at - OrderCycle.first_closing_for(object).andand.orders_close_at + options[:earliest_closing_times][object.id] end def active - @options[:active_distributors].andand.include? object + options[:active_distributors].andand.include? object end - - end class Api::CachedEnterpriseSerializer < ActiveModel::Serializer From f0e909c92bc7a029ca9fb607accfaa9b4799c871 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 12:38:33 +1000 Subject: [PATCH 20/49] Look up the shipping services (pickup, delivery) that different hubs provide --- app/models/spree/shipping_method_decorator.rb | 16 +++++++++++ spec/models/spree/shipping_method_spec.rb | 27 +++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/app/models/spree/shipping_method_decorator.rb b/app/models/spree/shipping_method_decorator.rb index 4a2cf75b47..b8be603048 100644 --- a/app/models/spree/shipping_method_decorator.rb +++ b/app/models/spree/shipping_method_decorator.rb @@ -25,6 +25,22 @@ Spree::ShippingMethod.class_eval do scope :by_name, order('spree_shipping_methods.name ASC') + + # Return the services (pickup, delivery) that different distributors provide, in the format: + # {distributor_id => {pickup: true, delivery: false}, ...} + def self.services + Hash[ + Spree::ShippingMethod. + joins(:distributor_shipping_methods). + group('distributor_id'). + select("distributor_id"). + select("BOOL_OR(spree_shipping_methods.require_ship_address = 'f') AS pickup"). + select("BOOL_OR(spree_shipping_methods.require_ship_address = 't') AS delivery"). + map { |sm| [sm.distributor_id.to_i, {pickup: sm.pickup == 't', delivery: sm.delivery == 't'}] } + ] + end + + def available_to_order_with_distributor_check?(order, display_on=nil) available_to_order_without_distributor_check?(order, display_on) && self.distributors.include?(order.distributor) diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index d6b821e890..8b7f397191 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -55,5 +55,32 @@ module Spree sm.should be_available_to_order o end end + + describe "finding services offered by all distributors" do + let!(:d1) { create(:distributor_enterprise) } + let!(:d2) { create(:distributor_enterprise) } + let!(:d3) { create(:distributor_enterprise) } + let!(:d4) { create(:distributor_enterprise) } + let!(:d1_pickup) { create(:shipping_method, require_ship_address: false, distributors: [d1]) } + let!(:d1_delivery) { create(:shipping_method, require_ship_address: true, distributors: [d1]) } + let!(:d2_pickup) { create(:shipping_method, require_ship_address: false, distributors: [d2]) } + let!(:d3_delivery) { create(:shipping_method, require_ship_address: true, distributors: [d3]) } + + it "reports when the services are available" do + ShippingMethod.services[d1.id].should == {pickup: true, delivery: true} + end + + it "reports when only pickup is available" do + ShippingMethod.services[d2.id].should == {pickup: true, delivery: false} + end + + it "reports when only delivery is available" do + ShippingMethod.services[d3.id].should == {pickup: false, delivery: true} + end + + it "returns no entry when no service is available" do + ShippingMethod.services[d4.id].should be_nil + end + end end end From ee8db23fd99e08dc4037b8feef03079253276e77 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 12:40:04 +1000 Subject: [PATCH 21/49] Pull shipping method service computations out of the serialization loop --- app/helpers/injection_helper.rb | 2 +- app/serializers/api/enterprise_serializer.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index a17d6e81f9..39831711ff 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,6 +1,6 @@ module InjectionHelper def inject_enterprises - inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, active_distributors: @active_distributors, earliest_closing_times: OrderCycle.earliest_closing_times + inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, active_distributors: @active_distributors, earliest_closing_times: OrderCycle.earliest_closing_times, shipping_method_services: Spree::ShippingMethod.services end def inject_current_order diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index 626fe77cad..f7bd06f239 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -42,11 +42,13 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer has_one :address, serializer: Api::AddressSerializer def pickup - object.shipping_methods.where(:require_ship_address => false).present? + services = options[:shipping_method_services][object.id] + services ? services[:pickup] : false end def delivery - object.shipping_methods.where(:require_ship_address => true).present? + services = options[:shipping_method_services][object.id] + services ? services[:delivery] : false end def email From 704955a1854fbf2739779e5a5a6f6f2845535bba Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 15:47:52 +1000 Subject: [PATCH 22/49] Load active distributors where they're needed rather than in most controllers --- app/controllers/base_controller.rb | 3 --- app/controllers/checkout_controller.rb | 3 --- app/controllers/enterprises_controller.rb | 2 +- app/controllers/groups_controller.rb | 1 - app/controllers/home_controller.rb | 4 +--- app/controllers/map_controller.rb | 1 - app/controllers/producers_controller.rb | 3 +-- app/helpers/injection_helper.rb | 18 +++++++++++++++++- app/views/groups/show.html.haml | 6 +++--- spec/controllers/base_controller_spec.rb | 5 ----- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/app/controllers/base_controller.rb b/app/controllers/base_controller.rb index 1d74df5706..392e2fef64 100644 --- a/app/controllers/base_controller.rb +++ b/app/controllers/base_controller.rb @@ -12,9 +12,6 @@ class BaseController < ApplicationController before_filter :check_order_cycle_expiry - def load_active_distributors - @active_distributors ||= Enterprise.distributors_with_active_order_cycles - end private diff --git a/app/controllers/checkout_controller.rb b/app/controllers/checkout_controller.rb index 0c42ceb987..11be28ef04 100644 --- a/app/controllers/checkout_controller.rb +++ b/app/controllers/checkout_controller.rb @@ -12,9 +12,6 @@ class CheckoutController < Spree::CheckoutController include EnterprisesHelper def edit - # Because this controller doesn't inherit from our BaseController - # We need to duplicate the code here - @active_distributors ||= Enterprise.distributors_with_active_order_cycles end def update diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index 097139556c..75ad5c475b 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -4,7 +4,7 @@ class EnterprisesController < BaseController include OrderCyclesHelper # These prepended filters are in the reverse order of execution - prepend_before_filter :load_active_distributors, :set_order_cycles, :require_distributor_chosen, :reset_order, only: :shop + prepend_before_filter :set_order_cycles, :require_distributor_chosen, :reset_order, only: :shop before_filter :clean_permalink, only: :check_permalink respond_to :js, only: :permalink_checker diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 8653131b5a..6930632966 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,6 +1,5 @@ class GroupsController < BaseController layout 'darkswarm' - before_filter :load_active_distributors def index @groups = EnterpriseGroup.on_front_page.by_position diff --git a/app/controllers/home_controller.rb b/app/controllers/home_controller.rb index 76e179ed22..3bb7a68538 100644 --- a/app/controllers/home_controller.rb +++ b/app/controllers/home_controller.rb @@ -1,11 +1,9 @@ class HomeController < BaseController layout 'darkswarm' - before_filter :load_active_distributors - + def index end def about_us end end - diff --git a/app/controllers/map_controller.rb b/app/controllers/map_controller.rb index a980ba8f40..46a6f5852a 100644 --- a/app/controllers/map_controller.rb +++ b/app/controllers/map_controller.rb @@ -1,6 +1,5 @@ class MapController < BaseController layout 'darkswarm' - before_filter :load_active_distributors def index end diff --git a/app/controllers/producers_controller.rb b/app/controllers/producers_controller.rb index b101a95b7f..42d1d401e5 100644 --- a/app/controllers/producers_controller.rb +++ b/app/controllers/producers_controller.rb @@ -1,7 +1,6 @@ class ProducersController < BaseController layout 'darkswarm' - before_filter :load_active_distributors - + def index end end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 39831711ff..2229888f98 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,6 +1,10 @@ module InjectionHelper def inject_enterprises - inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, active_distributors: @active_distributors, earliest_closing_times: OrderCycle.earliest_closing_times, shipping_method_services: Spree::ShippingMethod.services + inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, enterprise_injection_data + end + + def inject_group_enterprises + inject_json_ams "group_enterprises", @group.enterprises, Api::EnterpriseSerializer, enterprise_injection_data end def inject_current_order @@ -53,4 +57,16 @@ module InjectionHelper end render partial: "json/injection_ams", locals: {name: name, json: json} end + + + private + + def enterprise_injection_data + @active_distributors ||= Enterprise.distributors_with_active_order_cycles + @earliest_closing_times ||= OrderCycle.earliest_closing_times + @shipping_method_services ||= Spree::ShippingMethod.services + + {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services} + end + end diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 1bc965dfb3..80101d9e8c 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -3,8 +3,8 @@ = inject_enterprises -# inject enterprises in this group --# further hubs and producers of these enterprises can't be resoleved within this small subset -= inject_json_ams "group_enterprises", @group.enterprises, Api::EnterpriseSerializer, active_distributors: @active_distributors +-# further hubs and producers of these enterprises can't be resolved within this small subset += inject_group_enterprises #group-page.row.pad-top{"ng-controller" => "GroupPageCtrl"} .small-12.columns.pad-top @@ -95,7 +95,7 @@ = render partial: 'home/fat' = render partial: 'shared/components/enterprise_no_results' - + .small-12.medium-12.large-3.columns = render partial: 'contact' diff --git a/spec/controllers/base_controller_spec.rb b/spec/controllers/base_controller_spec.rb index 1040b0594c..b5ef006c5b 100644 --- a/spec/controllers/base_controller_spec.rb +++ b/spec/controllers/base_controller_spec.rb @@ -24,9 +24,4 @@ describe BaseController do response.should redirect_to root_url flash[:info].should == "The order cycle you've selected has just closed. Please try again!" end - - it "loads active_distributors" do - Enterprise.stub(:distributors_with_active_order_cycles) { 'active distributors' } - controller.load_active_distributors.should == 'active distributors' - end end From 4a59c85b3e83bcf02837c3665f911f90f2072411 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 15:48:40 +1000 Subject: [PATCH 23/49] Inject current hub from AMS rather than RABL --- app/helpers/injection_helper.rb | 4 ++++ app/views/json/_current_hub.rabl | 6 ------ app/views/layouts/darkswarm.html.haml | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) delete mode 100644 app/views/json/_current_hub.rabl diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 2229888f98..a0aa2f87f1 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -7,6 +7,10 @@ module InjectionHelper inject_json_ams "group_enterprises", @group.enterprises, Api::EnterpriseSerializer, enterprise_injection_data end + def inject_current_hub + inject_json_ams "currentHub", current_distributor, Api::EnterpriseSerializer, enterprise_injection_data + end + def inject_current_order inject_json_ams "currentOrder", current_order, Api::CurrentOrderSerializer, current_distributor: current_distributor, current_order_cycle: current_order_cycle end diff --git a/app/views/json/_current_hub.rabl b/app/views/json/_current_hub.rabl deleted file mode 100644 index 103baf9fb3..0000000000 --- a/app/views/json/_current_hub.rabl +++ /dev/null @@ -1,6 +0,0 @@ -object current_distributor -extends 'json/partials/enterprise' - -child suppliers: :producers do - attributes :id -end diff --git a/app/views/layouts/darkswarm.html.haml b/app/views/layouts/darkswarm.html.haml index a627ba4896..5169efa942 100644 --- a/app/views/layouts/darkswarm.html.haml +++ b/app/views/layouts/darkswarm.html.haml @@ -24,7 +24,7 @@ = render partial: "shared/ie_warning" = javascript_include_tag "iehack" - = inject_json "currentHub", "current_hub" + = inject_current_hub = inject_json "user", "current_user" = inject_json "railsFlash", "flash" = inject_taxons From cf79b90044c219757dc5e2044b9ea1e70944e3b1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 16:24:13 +1000 Subject: [PATCH 24/49] Load relatives of all enterprises in one go --- app/models/enterprise_relationship.rb | 22 +++++++++++++++++++++ spec/models/enterprise_relationship_spec.rb | 12 +++++++++++ 2 files changed, 34 insertions(+) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index fbdef9d52c..22d04a6cd1 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -25,6 +25,28 @@ class EnterpriseRelationship < ActiveRecord::Base scope :by_name, with_enterprises.order('child_enterprises.name, parent_enterprises.name') + # Load an array of the relatives of each enterprise (ie. any enterprise related to it in + # either direction). This array is split into distributors and producers, and has the format: + # {enterprise_id => {distributors: [id, ...], producers: [id, ...]} } + def self.relatives + relationships = EnterpriseRelationship.includes(:child, :parent) + relatives = {} + + relationships.each do |r| + relatives[r.parent_id] ||= {distributors: [], producers: []} + relatives[r.child_id] ||= {distributors: [], producers: []} + + relatives[r.parent_id][:producers] << r.child_id if r.child.is_primary_producer + relatives[r.parent_id][:distributors] << r.child_id if r.child.is_distributor + + relatives[r.child_id][:producers] << r.parent_id if r.parent.is_primary_producer + relatives[r.child_id][:distributors] << r.parent_id if r.parent.is_distributor + end + + relatives + end + + def permissions_list=(perms) perms.andand.each { |name| permissions.build name: name } end diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index e87c204037..cb1743dec1 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -69,4 +69,16 @@ describe EnterpriseRelationship do EnterpriseRelationship.with_permission('two').sort.should == [er1, er2].sort end end + + describe "finding relatives" do + let(:e1) { create(:supplier_enterprise) } + let(:e2) { create(:supplier_enterprise, sells: 'any') } + let!(:er) { create(:enterprise_relationship, parent: e1, child: e2) } + + it "categorises enterprises into distributors and producers" do + EnterpriseRelationship.relatives.should == + {e1.id => {distributors: [e2.id], producers: [e2.id]}, + e2.id => {distributors: [], producers: [e1.id]}} + end + end end From 3afd636577ad2348687a098b01c0dec4d457126e Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Thu, 21 May 2015 16:25:05 +1000 Subject: [PATCH 25/49] Pull relatives computation out of the serialization loop --- app/helpers/injection_helper.rb | 3 ++- app/serializers/api/enterprise_serializer.rb | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index a0aa2f87f1..1d5f4d19e2 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -69,8 +69,9 @@ module InjectionHelper @active_distributors ||= Enterprise.distributors_with_active_order_cycles @earliest_closing_times ||= OrderCycle.earliest_closing_times @shipping_method_services ||= Spree::ShippingMethod.services + @relatives ||= EnterpriseRelationship.relatives - {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services} + {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services, relatives: @relatives} end end diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index f7bd06f239..a97544b3ee 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -72,11 +72,13 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer end def producers - ActiveModel::ArraySerializer.new(object.suppliers.activated, {each_serializer: Api::IdSerializer}) + relatives = options[:relatives][object.id] + relatives ? relatives[:producers] : [] end def hubs - ActiveModel::ArraySerializer.new(object.distributors.activated, {each_serializer: Api::IdSerializer}) + relatives = options[:relatives][object.id] + relatives ? relatives[:distributors] : [] end # Map svg icons. From 2c92b5a7516b888d87583ad6e35a876bc583d504 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 10:57:14 +1000 Subject: [PATCH 26/49] Find all supplied and distributed taxons --- app/models/spree/taxon_decorator.rb | 36 +++++++++++++++++++++++++++++ spec/models/spree/taxon_spec.rb | 28 ++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 spec/models/spree/taxon_spec.rb diff --git a/app/models/spree/taxon_decorator.rb b/app/models/spree/taxon_decorator.rb index 10ee0b4719..1a26ce73a8 100644 --- a/app/models/spree/taxon_decorator.rb +++ b/app/models/spree/taxon_decorator.rb @@ -9,4 +9,40 @@ Spree::Taxon.class_eval do #fs << Spree::ProductFilters.distributor_filter if Spree::ProductFilters.respond_to? :distributor_filter fs end + + # Find all the taxons of supplied products for each enterprise, indexed by enterprise. + # Format: {enterprise_id => [taxon_id, ...]} + def self.supplied_taxons + taxons = {} + + Spree::Taxon. + joins(:products => :supplier). + select('spree_taxons.*, enterprises.id AS enterprise_id'). + each do |t| + + taxons[t.enterprise_id.to_i] ||= Set.new + taxons[t.enterprise_id.to_i] << t.id + end + + taxons + end + + # Find all the taxons of distributed products for each enterprise, indexed by enterprise. + # Format: {enterprise_id => [taxon_id, ...]} + def self.distributed_taxons + taxons = {} + + Spree::Taxon. + joins(:products). + merge(Spree::Product.with_order_cycles_outer). + where('o_exchanges.incoming = ?', false). + select('spree_taxons.*, o_exchanges.receiver_id AS enterprise_id'). + each do |t| + + taxons[t.enterprise_id.to_i] ||= Set.new + taxons[t.enterprise_id.to_i] << t.id + end + + taxons + end end diff --git a/spec/models/spree/taxon_spec.rb b/spec/models/spree/taxon_spec.rb new file mode 100644 index 0000000000..a0d729c054 --- /dev/null +++ b/spec/models/spree/taxon_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +module Spree + describe Taxon do + let(:e) { create(:supplier_enterprise) } + let(:t0) { p1.taxons.order('id ASC').first } + let(:t1) { create(:taxon) } + let(:t2) { create(:taxon) } + + describe "finding all supplied taxons" do + let!(:p1) { create(:simple_product, supplier: e, taxons: [t1, t2]) } + + it "finds taxons" do + Taxon.supplied_taxons.should == {e.id => Set.new([t0.id, t1.id, t2.id])} + end + end + + describe "finding all distributed taxons" do + let!(:oc) { create(:simple_order_cycle, distributors: [e], variants: [p1.master]) } + let(:s) { create(:supplier_enterprise) } + let(:p1) { create(:simple_product, supplier: s, taxons: [t1, t2]) } + + it "finds taxons" do + Taxon.distributed_taxons.should == {e.id => Set.new([t0.id, t1.id, t2.id])} + end + end + end +end From 1a887df4127d932f2f0f40d54a4ffc9f416aa65b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 11:03:53 +1000 Subject: [PATCH 27/49] Pull taxon computation out of the serialization loop --- app/helpers/injection_helper.rb | 4 +++- app/serializers/api/enterprise_serializer.rb | 11 +++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 1d5f4d19e2..c2e4e6c91a 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -70,8 +70,10 @@ module InjectionHelper @earliest_closing_times ||= OrderCycle.earliest_closing_times @shipping_method_services ||= Spree::ShippingMethod.services @relatives ||= EnterpriseRelationship.relatives + @supplied_taxons ||= Spree::Taxon.supplied_taxons + @distributed_taxons ||= Spree::Taxon.distributed_taxons - {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services, relatives: @relatives} + {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services, relatives: @relatives, supplied_taxons: @supplied_taxons, distributed_taxons: @distributed_taxons} end end diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index a97544b3ee..d15ee7860a 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -36,11 +36,18 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer :email, :hash, :logo, :promo_image, :path, :pickup, :delivery, :icon, :icon_font, :producer_icon_font, :category, :producers, :hubs - has_many :distributed_taxons, key: :taxons, serializer: Api::IdSerializer - has_many :supplied_taxons, serializer: Api::IdSerializer + attributes :taxons, :supplied_taxons has_one :address, serializer: Api::AddressSerializer + def taxons + options[:distributed_taxons][object.id] + end + + def supplied_taxons + options[:supplied_taxons][object.id] + end + def pickup services = options[:shipping_method_services][object.id] services ? services[:pickup] : false From dd761719eea337c807645ae32a3f7909a81f2e54 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 11:27:47 +1000 Subject: [PATCH 28/49] Fix undefined Api::IdSerializer error --- app/serializers/api/enterprise_serializer.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index d15ee7860a..6f46d62eda 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -1,4 +1,7 @@ class Api::EnterpriseSerializer < ActiveModel::Serializer + # We reference this here because otherwise the serializer complains about its absence + Api::IdSerializer + def serializable_hash cached_serializer_hash.merge uncached_serializer_hash end @@ -40,6 +43,7 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer has_one :address, serializer: Api::AddressSerializer + def taxons options[:distributed_taxons][object.id] end From 31b726613d5116dc2a1f615aa0f16f55d6f58559 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 12:17:09 +1000 Subject: [PATCH 29/49] Avoid loading enterprise injection data when it's not be needed due to caching --- app/helpers/injection_helper.rb | 14 ++++------ app/serializers/api/enterprise_serializer.rb | 16 +++++------ .../enterprise_injection_data.rb | 27 +++++++++++++++++++ 3 files changed, 40 insertions(+), 17 deletions(-) create mode 100644 lib/open_food_network/enterprise_injection_data.rb diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index c2e4e6c91a..05057c136b 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,6 +1,8 @@ +require 'open_food_network/enterprise_injection_data' + module InjectionHelper def inject_enterprises - inject_json_ams "enterprises", Enterprise.activated.all, Api::EnterpriseSerializer, enterprise_injection_data + inject_json_ams "enterprises", Enterprise.activated.includes(:address).all, Api::EnterpriseSerializer, enterprise_injection_data end def inject_group_enterprises @@ -66,14 +68,8 @@ module InjectionHelper private def enterprise_injection_data - @active_distributors ||= Enterprise.distributors_with_active_order_cycles - @earliest_closing_times ||= OrderCycle.earliest_closing_times - @shipping_method_services ||= Spree::ShippingMethod.services - @relatives ||= EnterpriseRelationship.relatives - @supplied_taxons ||= Spree::Taxon.supplied_taxons - @distributed_taxons ||= Spree::Taxon.distributed_taxons - - {active_distributors: @active_distributors, earliest_closing_times: @earliest_closing_times, shipping_method_services: @shipping_method_services, relatives: @relatives, supplied_taxons: @supplied_taxons, distributed_taxons: @distributed_taxons} + @enterprise_injection_data ||= OpenFoodNetwork::EnterpriseInjectionData.new + {data: @enterprise_injection_data} end end diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index 6f46d62eda..4f38c12964 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -21,11 +21,11 @@ class Api::UncachedEnterpriseSerializer < ActiveModel::Serializer attributes :orders_close_at, :active def orders_close_at - options[:earliest_closing_times][object.id] + options[:data].earliest_closing_times[object.id] end def active - options[:active_distributors].andand.include? object + options[:data].active_distributors.andand.include? object end end @@ -45,20 +45,20 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer def taxons - options[:distributed_taxons][object.id] + options[:data].distributed_taxons[object.id] end def supplied_taxons - options[:supplied_taxons][object.id] + options[:data].supplied_taxons[object.id] end def pickup - services = options[:shipping_method_services][object.id] + services = options[:data].shipping_method_services[object.id] services ? services[:pickup] : false end def delivery - services = options[:shipping_method_services][object.id] + services = options[:data].shipping_method_services[object.id] services ? services[:delivery] : false end @@ -83,12 +83,12 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer end def producers - relatives = options[:relatives][object.id] + relatives = options[:data].relatives[object.id] relatives ? relatives[:producers] : [] end def hubs - relatives = options[:relatives][object.id] + relatives = options[:data].relatives[object.id] relatives ? relatives[:distributors] : [] end diff --git a/lib/open_food_network/enterprise_injection_data.rb b/lib/open_food_network/enterprise_injection_data.rb new file mode 100644 index 0000000000..9862418b98 --- /dev/null +++ b/lib/open_food_network/enterprise_injection_data.rb @@ -0,0 +1,27 @@ +module OpenFoodNetwork + class EnterpriseInjectionData + def active_distributors + @active_distributors ||= Enterprise.distributors_with_active_order_cycles + end + + def earliest_closing_times + @earliest_closing_times ||= OrderCycle.earliest_closing_times + end + + def shipping_method_services + @shipping_method_services ||= Spree::ShippingMethod.services + end + + def relatives + @relatives ||= EnterpriseRelationship.relatives + end + + def supplied_taxons + @supplied_taxons ||= Spree::Taxon.supplied_taxons + end + + def distributed_taxons + @distributed_taxons ||= Spree::Taxon.distributed_taxons + end + end +end From e1b4c3b1e4bb75efe713c8b246fdb835004caa36 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 13:47:24 +1000 Subject: [PATCH 30/49] Add benchmarking test for inject_enterprises --- spec/performance/injection_helper_spec.rb | 29 +++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 spec/performance/injection_helper_spec.rb diff --git a/spec/performance/injection_helper_spec.rb b/spec/performance/injection_helper_spec.rb new file mode 100644 index 0000000000..4ba22d738f --- /dev/null +++ b/spec/performance/injection_helper_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe InjectionHelper, type: :helper do + let(:oc) { create(:simple_order_cycle) } + let(:relative_supplier) { create(:supplier_enterprise) } + let(:relative_distributor) { create(:distributor_enterprise) } + + before do + 50.times do + e = create(:enterprise) + oc.distributors << e + create(:enterprise_relationship, parent: e, child: relative_supplier) + create(:enterprise_relationship, parent: e, child: relative_distributor) + end + end + + it "is performant in injecting enterprises" do + results = [] + 4.times do |i| + ActiveRecord::Base.connection.query_cache.clear + Rails.cache.clear + result = Benchmark.measure { helper.inject_enterprises } + results << result.total if i > 0 + puts result + end + + puts (results.sum / results.count * 1000).round 0 + end +end From 41bc67e2d8e601b111c8e3820206b79e8cf1e04f Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 22 May 2015 14:46:33 +1000 Subject: [PATCH 31/49] Add benchmark for product serialisation --- spec/performance/shop_controller_spec.rb | 46 ++++++++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 spec/performance/shop_controller_spec.rb diff --git a/spec/performance/shop_controller_spec.rb b/spec/performance/shop_controller_spec.rb new file mode 100644 index 0000000000..14e258d18d --- /dev/null +++ b/spec/performance/shop_controller_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe ShopController, type: :controller do + let(:d) { create(:distributor_enterprise) } + let(:enterprise_fee) { create(:enterprise_fee) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator_fees: [enterprise_fee]) } + + before do + controller.stub(:current_distributor) { d } + controller.stub(:current_order_cycle) { order_cycle } + end + + describe "fetching products" do + let(:exchange) { order_cycle.exchanges.to_enterprises(d).outgoing.first } + let(:image) { File.open(File.expand_path('../../../app/assets/images/logo.jpg', __FILE__)) } + + before do + 11.times do + p = create(:simple_product) + p.set_property 'Organic Certified', 'NASAA 12345' + v1 = create(:variant, product: p) + v2 = create(:variant, product: p) + Spree::Image.create! viewable_id: p.master.id, viewable_type: 'Spree::Variant', attachment: image + + exchange.variants << [v1, v2] + end + end + + it "returns products via json" do + results = [] + 4.times do |i| + ActiveRecord::Base.connection.query_cache.clear + Rails.cache.clear + result = Benchmark.measure do + xhr :get, :products + response.should be_success + end + + results << result.total if i > 0 + puts result + end + + puts (results.sum / results.count * 1000).round 0 + end + end +end From e74390a013b9b614375c98bd63eb847b804923ed Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 May 2015 16:26:08 +1000 Subject: [PATCH 32/49] Remove controller specs for @active_distributors, now set via helper --- spec/controllers/home_controller_spec.rb | 12 ------------ spec/controllers/map_controller_spec.rb | 13 ------------- spec/controllers/producers_controller_spec.rb | 15 --------------- 3 files changed, 40 deletions(-) delete mode 100644 spec/controllers/map_controller_spec.rb delete mode 100644 spec/controllers/producers_controller_spec.rb diff --git a/spec/controllers/home_controller_spec.rb b/spec/controllers/home_controller_spec.rb index bbbff9a7b1..924462741f 100644 --- a/spec/controllers/home_controller_spec.rb +++ b/spec/controllers/home_controller_spec.rb @@ -9,21 +9,9 @@ describe HomeController do Enterprise.stub(:distributors_with_active_order_cycles) { [distributor] } end - it "sets active distributors" do - get :index - assigns[:active_distributors].should == [distributor] - end - # Exclusion from actual rendered view handled in features/consumer/home it "shows JSON for invisible hubs" do get :index response.body.should have_content invisible_distributor.name end - - # This is done inside the json/hubs Serializer - it "gets the next order cycle for each hub" do - OrderCycle.should_receive(:first_closing_for).twice - get :index - end end - diff --git a/spec/controllers/map_controller_spec.rb b/spec/controllers/map_controller_spec.rb deleted file mode 100644 index fab9b7ac22..0000000000 --- a/spec/controllers/map_controller_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spec_helper' - -describe MapController do - it "loads active distributors" do - active_distributors = double(:distributors) - - Enterprise.stub(:distributors_with_active_order_cycles) { active_distributors } - - get :index - - assigns(:active_distributors).should == active_distributors - end -end diff --git a/spec/controllers/producers_controller_spec.rb b/spec/controllers/producers_controller_spec.rb deleted file mode 100644 index ec3c39036c..0000000000 --- a/spec/controllers/producers_controller_spec.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'spec_helper' - -describe ProducersController do - let!(:distributor) { create(:distributor_enterprise) } - - before do - Enterprise.stub(:distributors_with_active_order_cycles) { [distributor] } - Enterprise.stub(:all).and_return([distributor]) - end - - it "sets active distributors" do - get :index - assigns[:active_distributors].should == [distributor] - end -end From 75f1f673ad3d63a6c366947876ce3479b62f050a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 May 2015 16:26:31 +1000 Subject: [PATCH 33/49] Update spec for EnterpriseSerializer --- spec/serializers/enterprise_serializer_spec.rb | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/spec/serializers/enterprise_serializer_spec.rb b/spec/serializers/enterprise_serializer_spec.rb index 1063a042e7..7c467d9c3d 100644 --- a/spec/serializers/enterprise_serializer_spec.rb +++ b/spec/serializers/enterprise_serializer_spec.rb @@ -3,19 +3,18 @@ describe Api::EnterpriseSerializer do let(:enterprise) { create(:distributor_enterprise) } let(:taxon) { create(:taxon) } + let(:data_class) { Struct.new(:earliest_closing_times, :active_distributors, + :distributed_taxons, :supplied_taxons, + :shipping_method_services, :relatives) } + let(:data) { data_class.new({}, [], {}, {}, {}, {producers: [], distributors: []}) } + it "serializes an enterprise" do - serializer = Api::EnterpriseSerializer.new enterprise + serializer = Api::EnterpriseSerializer.new enterprise, data: data serializer.to_json.should match enterprise.name end - it "includes distributed taxons" do - enterprise.stub(:distributed_taxons).and_return [taxon] - serializer = Api::EnterpriseSerializer.new enterprise - serializer.to_json.should match taxon.id.to_s - end - it "will render urls" do - serializer = Api::EnterpriseSerializer.new enterprise + serializer = Api::EnterpriseSerializer.new enterprise, data: data serializer.to_json.should match "map_005-hub.svg" end end From 3ab7df88e68bf024039f1a59ca64283b3daaef95 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 27 May 2015 16:26:40 +1000 Subject: [PATCH 34/49] Allow serialization of nil enterprise --- app/serializers/api/enterprise_serializer.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index 4f38c12964..fc843a5138 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -3,17 +3,18 @@ class Api::EnterpriseSerializer < ActiveModel::Serializer Api::IdSerializer def serializable_hash + cached_serializer_hash.merge uncached_serializer_hash end private def cached_serializer_hash - Api::CachedEnterpriseSerializer.new(object, @options).serializable_hash + Api::CachedEnterpriseSerializer.new(object, @options).serializable_hash || {} end def uncached_serializer_hash - Api::UncachedEnterpriseSerializer.new(object, @options).serializable_hash + Api::UncachedEnterpriseSerializer.new(object, @options).serializable_hash || {} end end @@ -31,7 +32,12 @@ end class Api::CachedEnterpriseSerializer < ActiveModel::Serializer cached - delegate :cache_key, to: :object + #delegate :cache_key, to: :object + + def cache_key + object.andand.cache_key + end + attributes :name, :id, :description, :latitude, :longitude, :long_description, :website, :instagram, :linkedin, :twitter, From cdbf02ca20f57b5d776bbe65422c620097e39476 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 12:07:43 +1000 Subject: [PATCH 35/49] EnterpriseRelationship.relatives can find activated enterprises only --- app/models/enterprise_relationship.rb | 14 +++++++++----- spec/models/enterprise_relationship_spec.rb | 10 ++++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 22d04a6cd1..49c26cb0f8 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -28,7 +28,7 @@ class EnterpriseRelationship < ActiveRecord::Base # Load an array of the relatives of each enterprise (ie. any enterprise related to it in # either direction). This array is split into distributors and producers, and has the format: # {enterprise_id => {distributors: [id, ...], producers: [id, ...]} } - def self.relatives + def self.relatives(activated_only=false) relationships = EnterpriseRelationship.includes(:child, :parent) relatives = {} @@ -36,11 +36,15 @@ class EnterpriseRelationship < ActiveRecord::Base relatives[r.parent_id] ||= {distributors: [], producers: []} relatives[r.child_id] ||= {distributors: [], producers: []} - relatives[r.parent_id][:producers] << r.child_id if r.child.is_primary_producer - relatives[r.parent_id][:distributors] << r.child_id if r.child.is_distributor + if !activated_only || r.child.activated? + relatives[r.parent_id][:producers] << r.child_id if r.child.is_primary_producer + relatives[r.parent_id][:distributors] << r.child_id if r.child.is_distributor + end - relatives[r.child_id][:producers] << r.parent_id if r.parent.is_primary_producer - relatives[r.child_id][:distributors] << r.parent_id if r.parent.is_distributor + if !activated_only || r.parent.activated? + relatives[r.child_id][:producers] << r.parent_id if r.parent.is_primary_producer + relatives[r.child_id][:distributors] << r.parent_id if r.parent.is_distributor + end end relatives diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index cb1743dec1..59e36c37e5 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -80,5 +80,15 @@ describe EnterpriseRelationship do {e1.id => {distributors: [e2.id], producers: [e2.id]}, e2.id => {distributors: [], producers: [e1.id]}} end + + it "finds inactive enterprises by default" do + e1.update_attribute :confirmed_at, nil + EnterpriseRelationship.relatives[e2.id][:producers].should == [e1.id] + end + + it "does not find inactive enterprises when requested" do + e1.update_attribute :confirmed_at, nil + EnterpriseRelationship.relatives(true)[e2.id][:producers].should be_empty + end end end From 69c54e1d704ebd15f5b935e0c93be619b24dc456 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 12:08:21 +1000 Subject: [PATCH 36/49] Only load activated relatives for EnterpriseInjectionData --- app/models/enterprise.rb | 4 ++++ .../enterprise_injection_data.rb | 2 +- .../enterprise_injection_data_spec.rb | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 spec/lib/open_food_network/enterprise_injection_data_spec.rb diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index e921167cf0..61d441fb2e 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -178,6 +178,10 @@ class Enterprise < ActiveRecord::Base count(distinct: true) end + def activated? + confirmed_at.present? && sells != 'unspecified' + end + def set_producer_property(property_name, property_value) transaction do property = Spree::Property.where(name: property_name).first_or_create!(presentation: property_name) diff --git a/lib/open_food_network/enterprise_injection_data.rb b/lib/open_food_network/enterprise_injection_data.rb index 9862418b98..87516007c6 100644 --- a/lib/open_food_network/enterprise_injection_data.rb +++ b/lib/open_food_network/enterprise_injection_data.rb @@ -13,7 +13,7 @@ module OpenFoodNetwork end def relatives - @relatives ||= EnterpriseRelationship.relatives + @relatives ||= EnterpriseRelationship.relatives(true) end def supplied_taxons diff --git a/spec/lib/open_food_network/enterprise_injection_data_spec.rb b/spec/lib/open_food_network/enterprise_injection_data_spec.rb new file mode 100644 index 0000000000..cb94f2374a --- /dev/null +++ b/spec/lib/open_food_network/enterprise_injection_data_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +module OpenFoodNetwork + describe EnterpriseInjectionData do + describe "relatives" do + let!(:enterprise) { create(:distributor_enterprise) } + let!(:producer) { create(:supplier_enterprise) } + let!(:producer_inactive) { create(:supplier_enterprise, confirmed_at: nil) } + let!(:er_p) { create(:enterprise_relationship, parent: producer, child: enterprise) } + let!(:er_pi) { create(:enterprise_relationship, parent: producer_inactive, child: enterprise) } + + it "only loads activated relatives" do + subject.relatives[enterprise.id][:producers].should_not include producer_inactive.id + end + end + end +end From 3f4f8afacd8b8ca432861da0fa8f6b92e68953df Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 12:19:38 +1000 Subject: [PATCH 37/49] EnterpriseRelationship.relatives does not show duplicates --- app/models/enterprise_relationship.rb | 4 ++-- spec/models/enterprise_relationship_spec.rb | 12 +++++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 49c26cb0f8..de06a0578e 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -33,8 +33,8 @@ class EnterpriseRelationship < ActiveRecord::Base relatives = {} relationships.each do |r| - relatives[r.parent_id] ||= {distributors: [], producers: []} - relatives[r.child_id] ||= {distributors: [], producers: []} + relatives[r.parent_id] ||= {distributors: Set.new, producers: Set.new} + relatives[r.child_id] ||= {distributors: Set.new, producers: Set.new} if !activated_only || r.child.activated? relatives[r.parent_id][:producers] << r.child_id if r.child.is_primary_producer diff --git a/spec/models/enterprise_relationship_spec.rb b/spec/models/enterprise_relationship_spec.rb index 59e36c37e5..96f80a65a4 100644 --- a/spec/models/enterprise_relationship_spec.rb +++ b/spec/models/enterprise_relationship_spec.rb @@ -74,21 +74,27 @@ describe EnterpriseRelationship do let(:e1) { create(:supplier_enterprise) } let(:e2) { create(:supplier_enterprise, sells: 'any') } let!(:er) { create(:enterprise_relationship, parent: e1, child: e2) } + let(:er_reverse) { create(:enterprise_relationship, parent: e2, child: e1) } it "categorises enterprises into distributors and producers" do EnterpriseRelationship.relatives.should == - {e1.id => {distributors: [e2.id], producers: [e2.id]}, - e2.id => {distributors: [], producers: [e1.id]}} + {e1.id => {distributors: Set.new([e2.id]), producers: Set.new([e2.id])}, + e2.id => {distributors: Set.new([]), producers: Set.new([e1.id])}} end it "finds inactive enterprises by default" do e1.update_attribute :confirmed_at, nil - EnterpriseRelationship.relatives[e2.id][:producers].should == [e1.id] + EnterpriseRelationship.relatives[e2.id][:producers].should == Set.new([e1.id]) end it "does not find inactive enterprises when requested" do e1.update_attribute :confirmed_at, nil EnterpriseRelationship.relatives(true)[e2.id][:producers].should be_empty end + + it "does not show duplicates" do + er_reverse + EnterpriseRelationship.relatives[e2.id][:producers].should == Set.new([e1.id]) + end end end From d478cc1f69a1ed7d4436ef5e323acaec5de5d39c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 14:03:44 +1000 Subject: [PATCH 38/49] Serialize taxons and relatives in expected format --- app/serializers/api/enterprise_serializer.rb | 16 ++++++++---- .../serializers/enterprise_serializer_spec.rb | 25 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index fc843a5138..44364c4dbc 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -3,7 +3,6 @@ class Api::EnterpriseSerializer < ActiveModel::Serializer Api::IdSerializer def serializable_hash - cached_serializer_hash.merge uncached_serializer_hash end @@ -51,11 +50,11 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer def taxons - options[:data].distributed_taxons[object.id] + ids_to_objs options[:data].distributed_taxons[object.id] end def supplied_taxons - options[:data].supplied_taxons[object.id] + ids_to_objs options[:data].supplied_taxons[object.id] end def pickup @@ -90,12 +89,12 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer def producers relatives = options[:data].relatives[object.id] - relatives ? relatives[:producers] : [] + relatives ? ids_to_objs(relatives[:producers]) : [] end def hubs relatives = options[:data].relatives[object.id] - relatives ? relatives[:distributors] : [] + relatives ? ids_to_objs(relatives[:distributors]) : [] end # Map svg icons. @@ -135,4 +134,11 @@ class Api::CachedEnterpriseSerializer < ActiveModel::Serializer } icon_fonts[object.category] end + + + private + + def ids_to_objs(ids) + ids.andand.map { |id| {id: id} } + end end diff --git a/spec/serializers/enterprise_serializer_spec.rb b/spec/serializers/enterprise_serializer_spec.rb index 7c467d9c3d..f70852d973 100644 --- a/spec/serializers/enterprise_serializer_spec.rb +++ b/spec/serializers/enterprise_serializer_spec.rb @@ -1,20 +1,31 @@ #require 'spec_helper' describe Api::EnterpriseSerializer do + let(:serializer) { Api::EnterpriseSerializer.new enterprise, data: data } let(:enterprise) { create(:distributor_enterprise) } let(:taxon) { create(:taxon) } - let(:data_class) { Struct.new(:earliest_closing_times, :active_distributors, - :distributed_taxons, :supplied_taxons, - :shipping_method_services, :relatives) } - let(:data) { data_class.new({}, [], {}, {}, {}, {producers: [], distributors: []}) } + let(:data) { OpenStruct.new(earliest_closing_times: {}, + active_distributors: [], + distributed_taxons: {enterprise.id => [123]}, + supplied_taxons: {enterprise.id => [456]}, + shipping_method_services: {}, + relatives: {enterprise.id => {producers: [123], distributors: [456]}}) } it "serializes an enterprise" do - serializer = Api::EnterpriseSerializer.new enterprise, data: data serializer.to_json.should match enterprise.name end - it "will render urls" do - serializer = Api::EnterpriseSerializer.new enterprise, data: data + it "serializes taxons as ids only" do + serializer.serializable_hash[:taxons].should == [{id: 123}] + serializer.serializable_hash[:supplied_taxons].should == [{id: 456}] + end + + it "serializes producers and hubs as ids only" do + serializer.serializable_hash[:producers].should == [{id: 123}] + serializer.serializable_hash[:hubs].should == [{id: 456}] + end + + it "serializes icons" do serializer.to_json.should match "map_005-hub.svg" end end From 50ae331d9459af558319558ea913b5a5562aaa5c Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 16:03:16 +1000 Subject: [PATCH 39/49] ng-cloak mobile menu --- app/views/shared/menu/_mobile_menu.html.haml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/app/views/shared/menu/_mobile_menu.html.haml b/app/views/shared/menu/_mobile_menu.html.haml index 5ed57917fd..e5ba9af83b 100644 --- a/app/views/shared/menu/_mobile_menu.html.haml +++ b/app/views/shared/menu/_mobile_menu.html.haml @@ -2,7 +2,7 @@ %section.left %a.left-off-canvas-toggle.menu-icon %span - %section.right + %section.right{"ng-cloak" => true} .cart = render partial: "shared/menu/cart" %a{href: main_app.shop_path} @@ -11,34 +11,34 @@ %aside.left-off-canvas-menu.show-for-medium-down %ul.off-canvas-list %li.ofn-logo - %a{href: root_path} + %a{href: root_path} %img{src: "/assets/open-food-network-beta.png", srcset: "/assets/open-food-network-beta.svg", width: "110", height: "26"} - + - if current_page? root_path %li.li-menu %a{"ofn-scroll-to" => "hubs"} - %span.nav-primary + %span.nav-primary %i.ofn-i_040-hub Hubs - else %li.li-menu %a{href: root_path + "#/#hubs"} - %span.nav-primary + %span.nav-primary %i.ofn-i_040-hub Hubs %li.li-menu %a{href: main_app.map_path} - %span.nav-primary + %span.nav-primary %i.ofn-i_037-map Map %li.li-menu %a{href: main_app.producers_path} - %span.nav-primary + %span.nav-primary %i.ofn-i_036-producers Producers %li.li-menu %a{href: main_app.groups_path} - %span.nav-primary + %span.nav-primary %i.ofn-i_035-groups Groups From 5c3a59acabaf2a340e1fc084e221eeac5cba84ad Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 29 May 2015 16:39:41 +1000 Subject: [PATCH 40/49] ng-cloak order cycles selector, tabs and shopfront --- app/views/enterprises/shop.html.haml | 2 +- app/views/shop/products/_form.html.haml | 2 +- app/views/shopping_shared/_tabs.html.haml | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/enterprises/shop.html.haml b/app/views/enterprises/shop.html.haml index 89ce1a30ac..08e25417cb 100644 --- a/app/views/enterprises/shop.html.haml +++ b/app/views/enterprises/shop.html.haml @@ -3,7 +3,7 @@ %shop.darkswarm - content_for :order_cycle_form do - %div{"ng-controller" => "OrderCycleChangeCtrl"} + %div{"ng-controller" => "OrderCycleChangeCtrl", "ng-cloak" => true} %closing{"ng-if" => "OrderCycle.selected()"} Next order closing %strong {{ OrderCycle.orders_close_at() | date_in_words }} diff --git a/app/views/shop/products/_form.html.haml b/app/views/shop/products/_form.html.haml index 0df4f022aa..bdaded0f0d 100644 --- a/app/views/shop/products/_form.html.haml +++ b/app/views/shop/products/_form.html.haml @@ -1,4 +1,4 @@ -%products.small-12.columns{"ng-controller" => "ProductsCtrl", "ng-show" => "order_cycle.order_cycle_id != null", +%products.small-12.columns{"ng-controller" => "ProductsCtrl", "ng-show" => "order_cycle.order_cycle_id != null", "ng-cloak" => true, "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1"} // TODO: Needs an ng-show to slide content down diff --git a/app/views/shopping_shared/_tabs.html.haml b/app/views/shopping_shared/_tabs.html.haml index 3641e0cbd5..48c0a1d7b5 100644 --- a/app/views/shopping_shared/_tabs.html.haml +++ b/app/views/shopping_shared/_tabs.html.haml @@ -1,11 +1,11 @@ -#tabs{"ng-controller" => "TabsCtrl"} +#tabs{"ng-controller" => "TabsCtrl", "ng-cloak" => true} .row %tabset -# Build all tabs. - - for name, heading_cols in { about: ["About #{current_distributor.name}", 6], - producers: ["Producers",2], + - for name, heading_cols in { about: ["About #{current_distributor.name}", 6], + producers: ["Producers",2], contact: ["Contact",2], - groups: ["Groups",2]} + groups: ["Groups",2]} -# tabs take tab path in 'active' and 'select' functions defined in TabsCtrl. - heading, cols = heading_cols %tab.columns{heading: heading, From 0d3cdb9c694b0346912acf0064b008b5b2cfa17a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 29 May 2015 16:51:55 +1000 Subject: [PATCH 41/49] Expand All button to show all variants in BPE --- .../javascripts/admin/bulk_product_update.js.coffee | 6 ++++++ .../admin/directives/toggle_variants.js.coffee | 4 +--- .../admin/services/display_properties.js.coffee | 8 +++----- .../admin/products/bulk_edit/_products_head.html.haml | 2 ++ .../admin/products/bulk_edit/_products_product.html.haml | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index c902328af4..c557c0c791 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -109,6 +109,12 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout window.location = "/admin/products/" + product.permalink_live + ((if variant then "/variants/" + variant.id else "")) + "/edit" + $scope.toggleShowAllVariants = -> + showVariants = !DisplayProperties.showVariants 0 + $scope.filteredProducts.forEach (product) -> + DisplayProperties.setShowVariants product.id, showVariants + DisplayProperties.setShowVariants 0, showVariants + $scope.addVariant = (product) -> product.variants.push id: $scope.nextVariantId() diff --git a/app/assets/javascripts/admin/directives/toggle_variants.js.coffee b/app/assets/javascripts/admin/directives/toggle_variants.js.coffee index 410df8d7e9..f5b18ae5cb 100644 --- a/app/assets/javascripts/admin/directives/toggle_variants.js.coffee +++ b/app/assets/javascripts/admin/directives/toggle_variants.js.coffee @@ -1,10 +1,8 @@ angular.module("ofn.admin").directive "ofnToggleVariants", (DisplayProperties) -> link: (scope, element, attrs) -> if DisplayProperties.showVariants scope.product.id - element.removeClass "icon-chevron-right" element.addClass "icon-chevron-down" else - element.removeClass "icon-chevron-down" element.addClass "icon-chevron-right" element.on "click", -> @@ -16,4 +14,4 @@ angular.module("ofn.admin").directive "ofnToggleVariants", (DisplayProperties) - else DisplayProperties.setShowVariants scope.product.id, true element.removeClass "icon-chevron-right" - element.addClass "icon-chevron-down" \ No newline at end of file + element.addClass "icon-chevron-down" diff --git a/app/assets/javascripts/admin/services/display_properties.js.coffee b/app/assets/javascripts/admin/services/display_properties.js.coffee index 7288706032..3037c9f068 100644 --- a/app/assets/javascripts/admin/services/display_properties.js.coffee +++ b/app/assets/javascripts/admin/services/display_properties.js.coffee @@ -3,12 +3,10 @@ angular.module("ofn.admin").factory "DisplayProperties", -> displayProperties: {} showVariants: (product_id) -> - @initProduct product_id - @displayProperties[product_id].showVariants + @productProperties(product_id).showVariants setShowVariants: (product_id, showVariants) -> - @initProduct product_id - @displayProperties[product_id].showVariants = showVariants + @productProperties(product_id).showVariants = showVariants - initProduct: (product_id) -> + productProperties: (product_id) -> @displayProperties[product_id] ||= {showVariants: false} diff --git a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml index 6e22aef8ff..97b4f47c83 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_head.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_head.html.haml @@ -19,6 +19,8 @@ %thead %tr %th.left-actions + %a{ 'ng-click' => 'toggleShowAllVariants()', :style => 'color: red' } + Expand All %th.producer{ 'ng-show' => 'columns.producer.visible' } Producer %th.sku{ 'ng-show' => 'columns.sku.visible' } SKU %th.name{ 'ng-show' => 'columns.name.visible' } Name diff --git a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml index 376e88071a..6ac25ae286 100644 --- a/app/views/spree/admin/products/bulk_edit/_products_product.html.haml +++ b/app/views/spree/admin/products/bulk_edit/_products_product.html.haml @@ -1,6 +1,6 @@ %tr.product{ :id => "p_{{product.id}}" } %td.left-actions - %a{ 'ofn-toggle-variants' => 'true', :class => "view-variants icon-chevron-right", 'ng-show' => 'hasVariants(product)' } + %a{ 'ofn-toggle-variants' => 'true', :class => "view-variants", 'ng-show' => 'hasVariants(product)' } %a{ :class => "add-variant icon-plus-sign", 'ng-click' => "addVariant(product)", 'ng-show' => "!hasVariants(product) && hasUnit(product)" } %td.producer{ 'ng-show' => 'columns.producer.visible' } %select.select2.fullwidth{ 'ng-model' => 'product.producer_id', :name => 'producer_id', 'ofn-track-product' => 'producer_id', 'ng-options' => 'producer.id as producer.name for producer in producers' } From 8afffdae9ab7d205475ede289213524c5ed14ac1 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 12:13:42 +1000 Subject: [PATCH 42/49] Fix error when product does not have a master variant --- app/assets/javascripts/darkswarm/services/products.js.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/darkswarm/services/products.js.coffee b/app/assets/javascripts/darkswarm/services/products.js.coffee index a07ae1f466..4785adae85 100644 --- a/app/assets/javascripts/darkswarm/services/products.js.coffee +++ b/app/assets/javascripts/darkswarm/services/products.js.coffee @@ -32,8 +32,9 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Pro if product.variants product.variants = (Variants.register variant for variant in product.variants) variant.product = product for variant in product.variants - product.master.product = product - product.master = Variants.register product.master if product.master + if product.master + product.master.product = product + product.master = Variants.register product.master registerVariantsWithCart: -> for product in @products From 473322c7e6b14a4c24927bff36440364d9138206 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 12:25:28 +1000 Subject: [PATCH 43/49] CI: Add more robust merge-to-master script --- script/ci/merge_branch_to_master.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/script/ci/merge_branch_to_master.sh b/script/ci/merge_branch_to_master.sh index 3eaae17d56..fd9c602802 100755 --- a/script/ci/merge_branch_to_master.sh +++ b/script/ci/merge_branch_to_master.sh @@ -6,5 +6,9 @@ source ./script/ci/includes.sh echo "--- Verifying branch is based on current master" exit_unless_master_merged -echo "--- Pushing branch" -git push origin $BUILDKITE_COMMIT:master +echo "--- Merging and pushing branch" +git checkout master +git merge origin/master +git merge origin/$BUILDKITE_BRANCH +git push origin master +git checkout origin/$BUILDKITE_BRANCH From c6f6c11a43143d3c820be511b8c940937696c49a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 12:51:15 +1000 Subject: [PATCH 44/49] Add wait between clicks to fix race condition --- spec/features/admin/bulk_product_update_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 4c52ec2256..f5134f4e81 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -205,8 +205,9 @@ feature %q{ expect(page).to have_selector "a.edit-variant", count: 1 # When I remove two, they should be removed - page.all('a.delete-variant').first.click - page.all('a.delete-variant').first.click + page.all('a.delete-variant', visible: true).first.click + expect(page).to have_selector "tr.variant", count: 2 + page.all('a.delete-variant', visible: true).first.click expect(page).to have_selector "tr.variant", count: 1 # When I fill out variant details and hit update From b3878b126b3c73b6baa1cefd35feb06e6813ce7a Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 12:53:46 +1000 Subject: [PATCH 45/49] Decouple generic injection spec from EnterpriseSerializer --- spec/helpers/injection_helper_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/helpers/injection_helper_spec.rb b/spec/helpers/injection_helper_spec.rb index 96d9279ef5..362ca6479b 100644 --- a/spec/helpers/injection_helper_spec.rb +++ b/spec/helpers/injection_helper_spec.rb @@ -4,7 +4,7 @@ describe InjectionHelper do let!(:enterprise) { create(:distributor_enterprise, facebook: "roger") } it "will inject via AMS" do - helper.inject_json_ams("test", [enterprise], Api::EnterpriseSerializer).should match enterprise.name + helper.inject_json_ams("test", [enterprise], Api::IdSerializer).should match /#{enterprise.id}/ end it "injects enterprises" do From 36dc0d5ccd767446a63c9b4dd07bfb4f41a41c5b Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 13:00:07 +1000 Subject: [PATCH 46/49] Do not run performance specs in CI --- script/ci/run_tests.sh | 2 +- spec/performance/injection_helper_spec.rb | 2 +- spec/performance/shop_controller_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/script/ci/run_tests.sh b/script/ci/run_tests.sh index 5539935a8b..f973c139ad 100755 --- a/script/ci/run_tests.sh +++ b/script/ci/run_tests.sh @@ -16,4 +16,4 @@ echo "--- Loading test database" bundle exec rake db:test:load echo "--- Running tests" -bundle exec rspec spec +bundle exec rspec --tag ~performance spec diff --git a/spec/performance/injection_helper_spec.rb b/spec/performance/injection_helper_spec.rb index 4ba22d738f..ef8d937ce1 100644 --- a/spec/performance/injection_helper_spec.rb +++ b/spec/performance/injection_helper_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe InjectionHelper, type: :helper do +describe InjectionHelper, type: :helper, performance: true do let(:oc) { create(:simple_order_cycle) } let(:relative_supplier) { create(:supplier_enterprise) } let(:relative_distributor) { create(:distributor_enterprise) } diff --git a/spec/performance/shop_controller_spec.rb b/spec/performance/shop_controller_spec.rb index 14e258d18d..984581a2ab 100644 --- a/spec/performance/shop_controller_spec.rb +++ b/spec/performance/shop_controller_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe ShopController, type: :controller do +describe ShopController, type: :controller, performance: true do let(:d) { create(:distributor_enterprise) } let(:enterprise_fee) { create(:enterprise_fee) } let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator_fees: [enterprise_fee]) } From db47c01784df955cff00560f3c4b5454467025dd Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Fri, 1 May 2015 09:50:12 +1000 Subject: [PATCH 47/49] Initial config for parallel spec running --- .rspec_parallel | 2 ++ Gemfile | 1 + Gemfile.lock | 4 ++++ config/database.yml | 2 +- 4 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 .rspec_parallel diff --git a/.rspec_parallel b/.rspec_parallel new file mode 100644 index 0000000000..54b33804ae --- /dev/null +++ b/.rspec_parallel @@ -0,0 +1,2 @@ +--format progress +--format ParallelTests::RSpec::SummaryLogger --out tmp/spec_summary.log diff --git a/Gemfile b/Gemfile index b63961a3ef..3a698cf366 100644 --- a/Gemfile +++ b/Gemfile @@ -112,4 +112,5 @@ group :development do gem 'guard-rails' gem 'guard-zeus' gem 'guard-rspec' + gem 'parallel_tests' end diff --git a/Gemfile.lock b/Gemfile.lock index 1b62201120..09c5f866f3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -362,6 +362,9 @@ GEM activesupport (>= 3.0.0) cocaine (~> 0.5.3) mime-types + parallel (1.4.1) + parallel_tests (1.3.7) + parallel paypal-sdk-core (0.2.10) multi_json (~> 1.0) xml-simple @@ -575,6 +578,7 @@ DEPENDENCIES newrelic_rpm oj paperclip + parallel_tests pg poltergeist pry-debugger diff --git a/config/database.yml b/config/database.yml index d74ed6256a..7eef2396f9 100644 --- a/config/database.yml +++ b/config/database.yml @@ -10,7 +10,7 @@ development: test: adapter: postgresql encoding: unicode - database: open_food_network_test + database: open_food_network_test<%= ENV['TEST_ENV_NUMBER'] %> pool: 5 host: localhost username: ofn From 2b3689fd933f99349f5284c61fc51ff1e875fb34 Mon Sep 17 00:00:00 2001 From: Rohan Mitchell Date: Wed, 3 Jun 2015 14:29:24 +1000 Subject: [PATCH 48/49] Run CI specs in parallel --- script/ci/run_tests.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/script/ci/run_tests.sh b/script/ci/run_tests.sh index 5539935a8b..9f9cdfa30d 100755 --- a/script/ci/run_tests.sh +++ b/script/ci/run_tests.sh @@ -13,7 +13,8 @@ echo "--- Bundling" bundle install echo "--- Loading test database" -bundle exec rake db:test:load +bundle exec rake db:drop db:create db:schema:load +bundle exec rake parallel:drop parallel:create parallel:load_schema echo "--- Running tests" -bundle exec rspec spec +bundle exec rake parallel:spec From 73029636055c609959feeca03d29a621eb1a1124 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 3 Jun 2015 15:11:32 +1000 Subject: [PATCH 49/49] inventory report: filter was broken because filter_to_order_cycle returned nil [skip ci] --- .../products_and_inventory_report.rb | 14 ++------------ .../products_and_inventory_report_spec.rb | 12 +----------- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/lib/open_food_network/products_and_inventory_report.rb b/lib/open_food_network/products_and_inventory_report.rb index 163f9a4f50..2b797bd944 100644 --- a/lib/open_food_network/products_and_inventory_report.rb +++ b/lib/open_food_network/products_and_inventory_report.rb @@ -47,7 +47,7 @@ module OpenFoodNetwork end def variants - filter(child_variants) + filter(master_variants) + filter(child_variants) end def child_variants @@ -57,16 +57,6 @@ module OpenFoodNetwork .order("spree_products.name") end - def master_variants - Spree::Variant.where(:is_master => true) - .joins(:product) - .where("(select spree_variants.id from spree_variants as other_spree_variants - WHERE other_spree_variants.product_id = spree_variants.product_id - AND other_spree_variants.is_master = 'f' LIMIT 1) IS NULL") - .merge(visible_products) - .order("spree_products.name") - end - def filter(variants) # NOTE: Ordering matters. # filter_to_order_cycle and filter_to_distributor return Arrays not Arel @@ -107,7 +97,7 @@ module OpenFoodNetwork def filter_to_order_cycle(variants) if params[:order_cycle_id].to_i > 0 order_cycle = OrderCycle.find params[:order_cycle_id] - variants.select! { |v| order_cycle.variants.include? v } + variants.select { |v| order_cycle.variants.include? v } else variants end diff --git a/spec/lib/open_food_network/products_and_inventory_report_spec.rb b/spec/lib/open_food_network/products_and_inventory_report_spec.rb index 368eb4d4d7..13796c10f6 100644 --- a/spec/lib/open_food_network/products_and_inventory_report_spec.rb +++ b/spec/lib/open_food_network/products_and_inventory_report_spec.rb @@ -54,10 +54,8 @@ module OpenFoodNetwork it "fetches variants for some params" do subject.should_receive(:child_variants).and_return ["children"] - subject.should_receive(:master_variants).and_return ["masters"] subject.should_receive(:filter).with(['children']).and_return ["filter_children"] - subject.should_receive(:filter).with(['masters']).and_return ["filter_masters"] - subject.variants.should == ["filter_children", "filter_masters"] + subject.variants.should == ["filter_children"] end end @@ -92,14 +90,6 @@ module OpenFoodNetwork end end - describe "fetching master variants" do - it "doesn't return master variants with siblings" do - product = create(:simple_product, supplier: supplier) - - subject.master_variants.should be_empty - end - end - describe "Filtering variants" do let(:variants) { Spree::Variant.scoped.joins(:product).where(is_master: false) } it "should return unfiltered variants sans-params" do