From 3f62d82afce5d6fe786dc39fe21d565636d59fa1 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 20 Dec 2018 16:50:01 +0000 Subject: [PATCH 01/46] Add more inventory testing for visibility in the inventory UI --- spec/models/product_importer_spec.rb | 124 +++++++++++++++++++-------- 1 file changed, 90 insertions(+), 34 deletions(-) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index f05816e538..b2d1a12d0d 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -465,50 +465,106 @@ describe ProductImport::ProductImporter do end describe "importing items into inventory" do - before do - csv_data = CSV.generate do |csv| - csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type"] - csv << ["Beans", "Another Enterprise", "User Enterprise", "5", "3.20", "500", "g"] - csv << ["Sprouts", "Another Enterprise", "User Enterprise", "6", "6.50", "500", "g"] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "2001", "1.50", "500", "g"] + describe "creating and updating inventory" do + before do + csv_data = CSV.generate do |csv| + csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type"] + csv << ["Beans", "Another Enterprise", "User Enterprise", "5", "3.20", "500", "g"] + csv << ["Sprouts", "Another Enterprise", "User Enterprise", "6", "6.50", "500", "g"] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "2001", "1.50", "500", "g"] + end + File.write('/tmp/test-m.csv', csv_data) + file = File.new('/tmp/test-m.csv') + settings = {'import_into' => 'inventories'} + @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) end - File.write('/tmp/test-m.csv', csv_data) - file = File.new('/tmp/test-m.csv') - settings = {'import_into' => 'inventories'} - @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) - end - after { File.delete('/tmp/test-m.csv') } + after { File.delete('/tmp/test-m.csv') } - it "validates entries" do - @importer.validate_entries - entries = JSON.parse(@importer.entries_json) + it "validates entries" do + @importer.validate_entries + entries = JSON.parse(@importer.entries_json) - expect(filter('valid', entries)).to eq 3 - expect(filter('invalid', entries)).to eq 0 - expect(filter('create_inventory', entries)).to eq 2 - expect(filter('update_inventory', entries)).to eq 1 + expect(filter('valid', entries)).to eq 3 + expect(filter('invalid', entries)).to eq 0 + expect(filter('create_inventory', entries)).to eq 2 + expect(filter('update_inventory', entries)).to eq 1 + end + + it "saves and updates inventory" do + @importer.save_entries + + expect(@importer.inventory_created_count).to eq 2 + expect(@importer.inventory_updated_count).to eq 1 + expect(@importer.updated_ids).to be_a(Array) + expect(@importer.updated_ids.count).to eq 3 + + beans_override = VariantOverride.where(variant_id: product2.variants.first.id, hub_id: enterprise2.id).first + sprouts_override = VariantOverride.where(variant_id: product3.variants.first.id, hub_id: enterprise2.id).first + cabbage_override = VariantOverride.where(variant_id: product4.variants.first.id, hub_id: enterprise2.id).first + + expect(Float(beans_override.price)).to eq 3.20 + expect(beans_override.count_on_hand).to eq 5 + + expect(Float(sprouts_override.price)).to eq 6.50 + expect(sprouts_override.count_on_hand).to eq 6 + + expect(Float(cabbage_override.price)).to eq 1.50 + expect(cabbage_override.count_on_hand).to eq 2001 + end end - it "saves and updates inventory" do - @importer.save_entries + describe "updating existing inventory referenced by display_name" do + before do + csv_data = CSV.generate do |csv| + csv << ["name", "display_name", "distributor", "producer", "on_hand", "price", "units"] + csv << ["Oats", "Porridge Oats", "Another Enterprise", "User Enterprise", "900", "", "500"] + end + File.write('/tmp/test-m.csv', csv_data) + file = File.new('/tmp/test-m.csv') + settings = {'import_into' => 'inventories'} + @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) + end + after { File.delete('/tmp/test-m.csv') } - expect(@importer.inventory_created_count).to eq 2 - expect(@importer.inventory_updated_count).to eq 1 - expect(@importer.updated_ids).to be_a(Array) - expect(@importer.updated_ids.count).to eq 3 + it "updates inventory item correctly" do + @importer.save_entries - beans_override = VariantOverride.where(variant_id: product2.variants.first.id, hub_id: enterprise2.id).first - sprouts_override = VariantOverride.where(variant_id: product3.variants.first.id, hub_id: enterprise2.id).first - cabbage_override = VariantOverride.where(variant_id: product4.variants.first.id, hub_id: enterprise2.id).first + expect(@importer.inventory_created_count).to eq 1 - expect(Float(beans_override.price)).to eq 3.20 - expect(beans_override.count_on_hand).to eq 5 + override = VariantOverride.where(variant_id: variant2.id, hub_id: enterprise2.id).first + visible = InventoryItem.where(variant_id: variant2.id, enterprise_id: enterprise2.id).first.visible - expect(Float(sprouts_override.price)).to eq 6.50 - expect(sprouts_override.count_on_hand).to eq 6 + expect(override.count_on_hand).to eq 900 + expect(visible).to be_truthy + end + end - expect(Float(cabbage_override.price)).to eq 1.50 - expect(cabbage_override.count_on_hand).to eq 2001 + describe "updating existing item that was set to hidden in inventory" do + before do + InventoryItem.create(variant_id: product4.variants.first.id, enterprise_id: enterprise2.id, visible: false) + + csv_data = CSV.generate do |csv| + csv << ["name", "distributor", "producer", "on_hand", "price", "units"] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "500"] + end + File.write('/tmp/test-m.csv', csv_data) + file = File.new('/tmp/test-m.csv') + settings = {'import_into' => 'inventories'} + @importer = ProductImport::ProductImporter.new(file, admin, start: 1, end: 100, settings: settings) + end + after { File.delete('/tmp/test-m.csv') } + + it "sets the item to visible in inventory when the item is updated" do + @importer.save_entries + + expect(@importer.inventory_updated_count).to eq 1 + + override = VariantOverride.where(variant_id: product4.variants.first.id, hub_id: enterprise2.id).first + visible = InventoryItem.where(variant_id: product4.variants.first.id, enterprise_id: enterprise2.id).first.visible + + expect(override.count_on_hand).to eq 900 + expect(visible).to be_truthy + end end end From 7dc7ffe1b7265def9969757908680619e8118211 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 20 Dec 2018 15:35:57 +0000 Subject: [PATCH 02/46] Update inventory template --- public/inventory_template.csv | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/public/inventory_template.csv b/public/inventory_template.csv index d52cecded6..dc2f10e3be 100644 --- a/public/inventory_template.csv +++ b/public/inventory_template.csv @@ -1 +1 @@ -producer,distributor,name,display_name,units,unit_type,price,on_hand +producer,distributor,name,display_name,variant_unit_name,sku,units,unit_type,price,on_hand,on_demand From 79dce01aef197ab32a78b2c7e0db46c3aec3db96 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 11 Jan 2019 13:07:49 +0000 Subject: [PATCH 03/46] Add spec for new on_demand logic --- spec/features/admin/product_import_spec.rb | 43 ++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index 3a337d93dd..edfc738602 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -293,6 +293,49 @@ feature "Product Import", js: true do expect(page).to have_content 'Cabbage' end end + + it "handles on_demand and on_hand validations with inventory" do + csv_data = CSV.generate do |csv| + csv << ["name", "distributor", "producer", "category", "on_hand", "price", "units", "on_demand"] + csv << ["Beans", "Another Enterprise", "User Enterprise", "Vegetables", nil, "3.20", "500", "true"] + csv << ["Sprouts", "Another Enterprise", "User Enterprise", "Vegetables", "6", "6.50", "500", "false"] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "Vegetables", nil, "1.50", "500", nil] + end + File.write('/tmp/test.csv', csv_data) + + visit main_app.admin_product_import_path + select2_select I18n.t('admin.product_import.index.inventories'), from: "settings_import_into" + attach_file 'file', '/tmp/test.csv' + click_button 'Upload' + + import_data + + expect(page).to have_selector '.item-count', text: "3" + expect(page).to have_no_selector '.invalid-count' + expect(page).to have_selector '.inv-create-count', text: '2' + expect(page).to have_selector '.inv-update-count', text: '1' + + save_data + + expect(page).to have_selector '.inv-created-count', text: '2' + expect(page).to have_selector '.inv-updated-count', text: '1' + + beans_override = VariantOverride.where(variant_id: product2.variants.first.id, hub_id: enterprise2.id).first + sprouts_override = VariantOverride.where(variant_id: product3.variants.first.id, hub_id: enterprise2.id).first + cabbage_override = VariantOverride.where(variant_id: product4.variants.first.id, hub_id: enterprise2.id).first + + expect(Float(beans_override.price)).to eq 3.20 + expect(beans_override.count_on_hand).to be_nil + expect(beans_override.on_demand).to be_truthy + + expect(Float(sprouts_override.price)).to eq 6.50 + expect(sprouts_override.count_on_hand).to eq 6 + expect(sprouts_override.on_demand).to eq false + + expect(Float(cabbage_override.price)).to eq 1.50 + expect(cabbage_override.count_on_hand).to be_nil + expect(cabbage_override.on_demand).to be_nil + end end describe "when dealing with uploaded files" do From a3b9936f3736efed6711dc1fe6bc7f41cb988e66 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 13 Jan 2019 21:42:29 +0000 Subject: [PATCH 04/46] Improve matching inventory using 'items' unit type --- app/models/product_import/entry_validator.rb | 2 +- spec/models/product_importer_spec.rb | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/models/product_import/entry_validator.rb b/app/models/product_import/entry_validator.rb index 11c0aab7dd..7582330a0a 100644 --- a/app/models/product_import/entry_validator.rb +++ b/app/models/product_import/entry_validator.rb @@ -189,7 +189,7 @@ module ProductImport products.flat_map(&:variants).each do |existing_variant| unit_scale = existing_variant.product.variant_unit_scale unscaled_units = entry.unscaled_units || 0 - entry.unit_value = unscaled_units * unit_scale + entry.unit_value = unscaled_units * unit_scale unless unit_scale.nil? if entry_matches_existing_variant?(entry, existing_variant) variant_override = create_inventory_item(entry, existing_variant) diff --git a/spec/models/product_importer_spec.rb b/spec/models/product_importer_spec.rb index b2d1a12d0d..7a259f53fe 100644 --- a/spec/models/product_importer_spec.rb +++ b/spec/models/product_importer_spec.rb @@ -26,7 +26,7 @@ describe ProductImport::ProductImporter do let!(:variant) { create(:variant, product_id: product.id, price: '8.50', on_hand: '100', unit_value: '500', display_name: 'Preexisting Banana') } let!(:product2) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Beans', unit_value: '500', primary_taxon_id: category.id, description: nil) } let!(:product3) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Sprouts', unit_value: '500', primary_taxon_id: category.id) } - let!(:product4) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Cabbage', unit_value: '500', primary_taxon_id: category.id) } + let!(:product4) { create(:simple_product, supplier: enterprise, on_hand: '100', name: 'Cabbage', unit_value: '1', variant_unit_scale: nil, variant_unit: "items", variant_unit_name: "Whole", primary_taxon_id: category.id) } let!(:product5) { create(:simple_product, supplier: enterprise2, on_hand: '100', name: 'Lettuce', unit_value: '500', primary_taxon_id: category.id) } let!(:product6) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Beetroot', unit_value: '500', on_demand: true, variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category.id, description: nil) } let!(:product7) { create(:simple_product, supplier: enterprise3, on_hand: '100', name: 'Tomato', unit_value: '500', variant_unit_scale: 1, variant_unit: 'weight', primary_taxon_id: category.id, description: nil) } @@ -468,10 +468,10 @@ describe ProductImport::ProductImporter do describe "creating and updating inventory" do before do csv_data = CSV.generate do |csv| - csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type"] - csv << ["Beans", "Another Enterprise", "User Enterprise", "5", "3.20", "500", "g"] - csv << ["Sprouts", "Another Enterprise", "User Enterprise", "6", "6.50", "500", "g"] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "2001", "1.50", "500", "g"] + csv << ["name", "distributor", "producer", "on_hand", "price", "units", "unit_type", "variant_unit_name"] + csv << ["Beans", "Another Enterprise", "User Enterprise", "5", "3.20", "500", "g", ""] + csv << ["Sprouts", "Another Enterprise", "User Enterprise", "6", "6.50", "500", "g", ""] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "2001", "1.50", "1", "", "Whole"] end File.write('/tmp/test-m.csv', csv_data) file = File.new('/tmp/test-m.csv') @@ -544,8 +544,8 @@ describe ProductImport::ProductImporter do InventoryItem.create(variant_id: product4.variants.first.id, enterprise_id: enterprise2.id, visible: false) csv_data = CSV.generate do |csv| - csv << ["name", "distributor", "producer", "on_hand", "price", "units"] - csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "500"] + csv << ["name", "distributor", "producer", "on_hand", "price", "units", "variant_unit_name"] + csv << ["Cabbage", "Another Enterprise", "User Enterprise", "900", "", "1", "Whole"] end File.write('/tmp/test-m.csv', csv_data) file = File.new('/tmp/test-m.csv') From 990b90009eb2e9ce5f2c94c964f1ed5a986dcaec Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Fri, 18 Jan 2019 19:17:05 +0000 Subject: [PATCH 05/46] Bump stripe from 3.3.2 to 4.5.0 Bumps [stripe](https://github.com/stripe/stripe-ruby) from 3.3.2 to 4.5.0. - [Release notes](https://github.com/stripe/stripe-ruby/releases) - [Changelog](https://github.com/stripe/stripe-ruby/blob/master/CHANGELOG.md) - [Commits](https://github.com/stripe/stripe-ruby/compare/v3.3.2...v4.5.0) Signed-off-by: dependabot[bot] --- Gemfile | 2 +- Gemfile.lock | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/Gemfile b/Gemfile index e36e5cf274..89c5c2dc4b 100644 --- a/Gemfile +++ b/Gemfile @@ -22,7 +22,7 @@ gem 'spree_auth_devise', github: 'openfoodfoundation/spree_auth_devise', branch: # - Change type of password from string to password to hide it in the form gem 'spree_paypal_express', github: "openfoodfoundation/better_spree_paypal_express", branch: "spree-upgrade-intermediate" #gem 'spree_paypal_express', github: "spree-contrib/better_spree_paypal_express", branch: "1-3-stable" -gem 'stripe', '~> 3.3.2' +gem 'stripe', '~> 4.5.0' # We need at least this version to have Digicert's root certificate # which is needed for Pin Payments (and possibly others). gem 'activemerchant', '~> 1.78' diff --git a/Gemfile.lock b/Gemfile.lock index 36c333e36d..d0b4e986f7 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -252,6 +252,7 @@ GEM compass (~> 1.0.0) sass-rails (< 5.1) sprockets (< 4.0) + connection_pool (2.2.2) crack (0.4.3) safe_yaml (~> 1.0.0) css_parser (1.6.0) @@ -540,6 +541,8 @@ GEM multi_xml (0.6.0) multipart-post (2.0.0) nenv (0.3.0) + net-http-persistent (3.0.0) + connection_pool (~> 2.2) nokogiri (1.6.8.1) mini_portile2 (~> 2.1.0) notiffany (0.1.1) @@ -716,8 +719,9 @@ GEM tilt (~> 1.1, != 1.3.0) state_machine (1.2.0) stringex (1.3.3) - stripe (3.3.2) - faraday (~> 0.9) + stripe (4.5.0) + faraday (~> 0.13) + net-http-persistent (~> 3.0) therubyracer (0.12.0) libv8 (~> 3.16.14.0) ref @@ -853,7 +857,7 @@ DEPENDENCIES spree_paypal_express! spring (= 1.7.2) spring-commands-rspec - stripe (~> 3.3.2) + stripe (~> 4.5.0) therubyracer (= 0.12.0) timecop truncate_html From ea7f62123bd4ed0599ec1e9c2a66b210e912a90f Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Mon, 21 Jan 2019 19:14:44 +0000 Subject: [PATCH 06/46] Bump jwt from 1.5.6 to 2.1.0 Bumps [jwt](https://github.com/jwt/ruby-jwt) from 1.5.6 to 2.1.0. - [Release notes](https://github.com/jwt/ruby-jwt/releases) - [Changelog](https://github.com/jwt/ruby-jwt/blob/master/CHANGELOG.md) - [Commits](https://github.com/jwt/ruby-jwt/compare/v1.5.6...v2.1.0) Signed-off-by: dependabot[bot] --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index e36e5cf274..29be48a8de 100644 --- a/Gemfile +++ b/Gemfile @@ -28,7 +28,7 @@ gem 'stripe', '~> 3.3.2' gem 'activemerchant', '~> 1.78' gem 'oauth2', '~> 1.4.1' # Used for Stripe Connect -gem 'jwt', '~> 1.5' +gem 'jwt', '~> 2.1' gem 'delayed_job_active_record' gem 'daemons' diff --git a/Gemfile.lock b/Gemfile.lock index d79b08ea13..09c2a0a311 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -508,7 +508,7 @@ GEM json_spec (1.1.5) multi_json (~> 1.0) rspec (>= 2.0, < 4.0) - jwt (1.5.6) + jwt (2.1.0) kaminari (0.13.0) actionpack (>= 3.0.0) activesupport (>= 3.0.0) @@ -813,7 +813,7 @@ DEPENDENCIES jquery-migrate-rails jquery-rails json_spec (~> 1.1.4) - jwt (~> 1.5) + jwt (~> 2.1) knapsack letter_opener (>= 1.4.1) listen (= 3.0.8) From dfa11834e331b22060c5c26f0727d6a3eb2d64d9 Mon Sep 17 00:00:00 2001 From: leandroalemao Date: Thu, 24 Jan 2019 11:16:31 +0000 Subject: [PATCH 07/46] [ISSUE-3348] LC: Fix moment.js deprecation warning --- app/assets/javascripts/darkswarm/filters/dates.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/darkswarm/filters/dates.js.coffee b/app/assets/javascripts/darkswarm/filters/dates.js.coffee index 7b5dd861d5..91efe89d1e 100644 --- a/app/assets/javascripts/darkswarm/filters/dates.js.coffee +++ b/app/assets/javascripts/darkswarm/filters/dates.js.coffee @@ -4,7 +4,7 @@ Darkswarm.filter "date_in_words", -> Darkswarm.filter "sensible_timeframe", (date_in_wordsFilter)-> (date) -> - if moment().add('days', 2) < moment(date) + if moment().add(2, 'days') < moment(date) t 'orders_open' else t('closing') + date_in_wordsFilter(date) From bf084cd8dfc14e2e1baca791b7fbc50ca73c542c Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 25 Jan 2019 00:28:56 +0800 Subject: [PATCH 08/46] Move import batch size outside method --- .../controllers/import_form_controller.js.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee index 95e6a67a5d..e645970a4f 100644 --- a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee +++ b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee @@ -10,6 +10,7 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt $scope.updated_ids = [] $scope.update_errors = [] + $scope.batchSize = 50 $scope.step = 'settings' $scope.chunks = 0 $scope.completed = 0 @@ -51,14 +52,13 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt $scope.start = () -> $scope.started = true total = ams_data.item_count - size = 50 - $scope.chunks = Math.ceil(total / size) + $scope.chunks = Math.ceil(total / $scope.batchSize) i = 0 while i < $scope.chunks - start = (i*size)+1 - end = (i+1)*size + start = (i * $scope.batchSize) + 1 + end = (i + 1) * $scope.batchSize if $scope.step == 'import' $scope.processImport(start, end) if $scope.step == 'save' From b67ba89aca2bfa2ccff5b61400717e727ca3bb8f Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 25 Jan 2019 00:32:31 +0800 Subject: [PATCH 09/46] Process batches of the import CSV one after another Start the HTTP request for processing next batches only after the previous batch has completed. A con of this approach is that the user needs to keep their browser open for later parts of the large CSV to be processed, but this is better than a higher chance of requests timing out. --- .../import_form_controller.js.coffee | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee index e645970a4f..d1454307dc 100644 --- a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee +++ b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee @@ -54,16 +54,24 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt total = ams_data.item_count $scope.chunks = Math.ceil(total / $scope.batchSize) - i = 0 + # Process only the first batch. + $scope.processBatch($scope.step, 0, $scope.chunks) - while i < $scope.chunks - start = (i * $scope.batchSize) + 1 - end = (i + 1) * $scope.batchSize - if $scope.step == 'import' - $scope.processImport(start, end) - if $scope.step == 'save' - $scope.processSave(start, end) - i++ + $scope.processBatch = (step, batchIndex, batchCount) -> + start = (batchIndex * $scope.batchSize) + 1 + end = (batchIndex + 1) * $scope.batchSize + withNextBatch = batchIndex + 1 < batchCount + + promise = if step == 'import' + $scope.processImport(start, end) + else if step == 'save' + $scope.processSave(start, end) + + processNextBatch = -> + $scope.processBatch(step, batchIndex + 1, batchCount) + + # Process next batch whether or not processing of the current batch succeeds. + promise.then(processNextBatch, processNextBatch) if withNextBatch $scope.processImport = (start, end) -> $http( From 5610f64c7a859f403da4075f40f6f998727e5717 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" Date: Thu, 24 Jan 2019 19:14:52 +0000 Subject: [PATCH 10/46] Bump redcarpet from 3.2.3 to 3.4.0 Bumps [redcarpet](https://github.com/vmg/redcarpet) from 3.2.3 to 3.4.0. - [Release notes](https://github.com/vmg/redcarpet/releases) - [Changelog](https://github.com/vmg/redcarpet/blob/master/CHANGELOG.md) - [Commits](https://github.com/vmg/redcarpet/compare/v3.2.3...v3.4.0) Signed-off-by: dependabot[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index 41df6b00f7..aba685f8c2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -629,7 +629,7 @@ GEM trollop (~> 2.1) rdoc (3.12.2) json (~> 1.4) - redcarpet (3.2.3) + redcarpet (3.4.0) ref (2.0.0) request_store (1.4.1) rack (>= 1.4) From b0efc3a2fa112538c1e399128ec406dcc6bfa9b2 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 25 Jan 2019 04:09:02 +0800 Subject: [PATCH 11/46] Add feature spec for large file import --- spec/features/admin/product_import_spec.rb | 72 +++++++++++++++++++++- 1 file changed, 71 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index d196a686e0..6649397e1c 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -361,6 +361,58 @@ feature "Product Import", js: true do end end + describe "handling a large file (120 data rows)" do + let!(:producer) { enterprise } + + let(:tmp_csv_path) { "/tmp/test.csv" } + + before do + quick_login_as admin + visit main_app.admin_product_import_path + end + + context "when importing to product list" do + def write_tmp_csv_file + CSV.open(tmp_csv_path, "w") do |csv| + csv << ["name", "producer", "category", "on_hand", "price", "units", "unit_type", + "tax_category", "shipping_category"] + 120.times do |i| + csv << ["Imported Product #{i + 1}", producer.name, category.name, 1, "1.00", "500", + "g", tax_category.name, shipping_category.name] + end + end + end + + before { write_tmp_csv_file } + + it "validates and saves all batches" do + # Upload and validate file. + attach_file "file", tmp_csv_path + click_button I18n.t("admin.product_import.index.upload") + proceed_to_validation + + # Check that all rows are validated. + heading = "120 #{I18n.t("admin.product_import.import.products_to_create")}" + find(".panel-header", text: heading).click + expect(page).to have_content "Imported Product 10" + expect(page).to have_content "Imported Product 60" + expect(page).to have_content "Imported Product 110" + + # Save file. + proceed_with_save + + # Be extra patient. + expect_progress_percentages "33%", "67%", "100%" + expect_import_completed + + # Check that all rows are saved. + expect(producer.supplied_products.find_by_name("Imported Product 10")).to be_present + expect(producer.supplied_products.find_by_name("Imported Product 60")).to be_present + expect(producer.supplied_products.find_by_name("Imported Product 110")).to be_present + end + end + end + private def import_data @@ -372,8 +424,26 @@ feature "Product Import", js: true do def save_data expect(page).to have_selector 'a.button.proceed', visible: true - click_link I18n.t('admin.product_import.import.save') + proceed_with_save expect(page).to have_selector 'div.save-results', visible: true + expect_import_completed + end + + def expect_progress_percentages(*percentages) + percentages.each do |percentage| + expect(page).to have_selector ".progress-interface", text: percentage + end + end + + def proceed_to_validation + import_data + end + + def proceed_with_save + click_link I18n.t("admin.product_import.import.save") + end + + def expect_import_completed expect(page).to have_content I18n.t('admin.product_import.save_results.final_results') end end From 4425b0416d021c958a8eaa4ec8edd61d37b05e63 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 25 Jan 2019 04:12:56 +0800 Subject: [PATCH 12/46] Rename import_data feature test method --- spec/features/admin/product_import_spec.rb | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index 6649397e1c..eefa33d007 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -46,7 +46,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_selector '.item-count', text: "2" expect(page).to have_no_selector '.invalid-count' @@ -89,7 +89,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_selector '.item-count', text: "2" expect(page).to have_selector '.invalid-count', text: "2" @@ -112,7 +112,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_selector '.item-count', text: "1" expect(page).to have_selector '.create-count', text: "1" @@ -142,7 +142,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation save_data @@ -188,7 +188,7 @@ feature "Product Import", js: true do click_button 'Upload' - import_data + proceed_to_validation save_data @@ -213,7 +213,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_selector '.item-count', text: "3" expect(page).to_not have_selector '.invalid-count' @@ -252,7 +252,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_selector '.item-count', text: "3" expect(page).to have_no_selector '.invalid-count' @@ -349,7 +349,7 @@ feature "Product Import", js: true do attach_file 'file', '/tmp/test.csv' click_button 'Upload' - import_data + proceed_to_validation expect(page).to have_content I18n.t('admin.product_import.import.validation_overview') expect(page).to have_selector '.item-count', text: "2" @@ -415,7 +415,7 @@ feature "Product Import", js: true do private - def import_data + def proceed_to_validation expect(page).to have_selector 'a.button.proceed', visible: true click_link I18n.t('admin.product_import.import.import') expect(page).to have_selector 'form.product-import', visible: true @@ -435,10 +435,6 @@ feature "Product Import", js: true do end end - def proceed_to_validation - import_data - end - def proceed_with_save click_link I18n.t("admin.product_import.import.save") end From b6d8611359ed2cd701ee7997f095487c4bdcba92 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 25 Jan 2019 09:06:25 +0800 Subject: [PATCH 13/46] Return early in JS if last batch --- .../controllers/import_form_controller.js.coffee | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee index d1454307dc..174d0049d9 100644 --- a/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee +++ b/app/assets/javascripts/admin/product_import/controllers/import_form_controller.js.coffee @@ -60,18 +60,20 @@ angular.module("admin.productImport").controller "ImportFormCtrl", ($scope, $htt $scope.processBatch = (step, batchIndex, batchCount) -> start = (batchIndex * $scope.batchSize) + 1 end = (batchIndex + 1) * $scope.batchSize - withNextBatch = batchIndex + 1 < batchCount + isLastBatch = batchCount == batchIndex + 1 promise = if step == 'import' $scope.processImport(start, end) else if step == 'save' $scope.processSave(start, end) + return if isLastBatch + processNextBatch = -> $scope.processBatch(step, batchIndex + 1, batchCount) # Process next batch whether or not processing of the current batch succeeds. - promise.then(processNextBatch, processNextBatch) if withNextBatch + promise.then(processNextBatch, processNextBatch) $scope.processImport = (start, end) -> $http( From b9492d6483ee251306201f437d22eae1a64e83ca Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Wed, 12 Sep 2018 03:36:23 +1000 Subject: [PATCH 14/46] Fix setup in tests for SubscriptionValidator --- spec/services/subscription_validator_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb index 6670d14fa4..64dbc2637b 100644 --- a/spec/services/subscription_validator_spec.rb +++ b/spec/services/subscription_validator_spec.rb @@ -1,3 +1,5 @@ +require "spec_helper" + describe SubscriptionValidator do let(:shop) { instance_double(Enterprise, name: "Shop") } From 28c70c7719f6abd7c987a1d5a7482c8fce213a85 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 18 Jan 2019 19:04:32 +0800 Subject: [PATCH 15/46] Fix nesting of specs for editing subscriptions --- spec/features/admin/subscriptions_spec.rb | 234 +++++++++++----------- 1 file changed, 117 insertions(+), 117 deletions(-) diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index 27bfaa1bfc..6610c006ee 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -285,137 +285,137 @@ feature 'Subscriptions' do expect(subscription_line_item.variant).to eq variant2 expect(subscription_line_item.quantity).to eq 3 end + end - context 'editing an existing subscription' do - let!(:customer) { create(:customer, enterprise: shop) } - let!(:product1) { create(:product, supplier: shop) } - let!(:product2) { create(:product, supplier: shop) } - let!(:product3) { create(:product, supplier: shop) } - let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) } - let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) } - let!(:variant3) { create(:variant, product: product3, unit_value: '10000', price: 22.00, option_values: []) } - let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } - let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - let!(:variant3_oc) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } - let!(:variant3_ex) { variant3_oc.exchanges.create(sender: shop, receiver: shop, variants: [variant3]) } - let!(:payment_method) { create(:payment_method, distributors: [shop]) } - let!(:stripe_payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) } - let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } - let!(:subscription) { - create(:subscription, - shop: shop, - customer: customer, - schedule: schedule, - payment_method: payment_method, - shipping_method: shipping_method, - subscription_line_items: [create(:subscription_line_item, variant: variant1, quantity: 2, price_estimate: 13.75)], - with_proxy_orders: true) - } + context 'editing an existing subscription' do + let!(:customer) { create(:customer, enterprise: shop) } + let!(:product1) { create(:product, supplier: shop) } + let!(:product2) { create(:product, supplier: shop) } + let!(:product3) { create(:product, supplier: shop) } + let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) } + let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) } + let!(:variant3) { create(:variant, product: product3, unit_value: '10000', price: 22.00, option_values: []) } + let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let!(:variant3_oc) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } + let!(:variant3_ex) { variant3_oc.exchanges.create(sender: shop, receiver: shop, variants: [variant3]) } + let!(:payment_method) { create(:payment_method, distributors: [shop]) } + let!(:stripe_payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) } + let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } + let!(:subscription) { + create(:subscription, + shop: shop, + customer: customer, + schedule: schedule, + payment_method: payment_method, + shipping_method: shipping_method, + subscription_line_items: [create(:subscription_line_item, variant: variant1, quantity: 2, price_estimate: 13.75)], + with_proxy_orders: true) + } - it "passes the smoke test" do - visit edit_admin_subscription_path(subscription) + it "passes the smoke test" do + visit edit_admin_subscription_path(subscription) - # Customer and Schedule cannot be edited - click_button 'edit-details' - expect(page).to have_selector '#s2id_customer_id.select2-container-disabled' - expect(page).to have_selector '#s2id_schedule_id.select2-container-disabled' + # Customer and Schedule cannot be edited + click_button 'edit-details' + expect(page).to have_selector '#s2id_customer_id.select2-container-disabled' + expect(page).to have_selector '#s2id_schedule_id.select2-container-disabled' - # Can't use a Stripe payment method because customer does not allow it - select2_select stripe_payment_method.name, from: 'payment_method_id' - expect(page).to have_content I18n.t('admin.subscriptions.details.charges_not_allowed') - click_button 'Save Changes' - expect(page).to have_content 'Credit card charges are not allowed by this customer' - select2_select payment_method.name, from: 'payment_method_id' - click_button 'Review' + # Can't use a Stripe payment method because customer does not allow it + select2_select stripe_payment_method.name, from: 'payment_method_id' + expect(page).to have_content I18n.t('admin.subscriptions.details.charges_not_allowed') + click_button 'Save Changes' + expect(page).to have_content 'Credit card charges are not allowed by this customer' + select2_select payment_method.name, from: 'payment_method_id' + click_button 'Review' - # Existing products should be visible - click_button 'edit-products' - within "#sli_0" do - expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}" - expect(page).to have_selector 'td.price', text: "$13.75" - expect(page).to have_input 'quantity', with: "2" - expect(page).to have_selector 'td.total', text: "$27.50" + # Existing products should be visible + click_button 'edit-products' + within "#sli_0" do + expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}" + expect(page).to have_selector 'td.price', text: "$13.75" + expect(page).to have_input 'quantity', with: "2" + expect(page).to have_selector 'td.total', text: "$27.50" - # Remove variant1 from the subscription - find("a.delete-item").click - end - - # Attempting to submit without a product - click_button 'Save Changes' - expect(page).to have_content 'Please add at least one product' - - # Add variant2 to the subscription - select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 1 - click_link 'Add' - within "#sli_0" do - expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}" - expect(page).to have_selector 'td.price', text: "$7.75" - expect(page).to have_input 'quantity', with: "1" - expect(page).to have_selector 'td.total', text: "$7.75" - end - - # Total should be $7.75 - expect(page).to have_selector '#order_form_total', text: "$7.75" - - # Add variant3 to the subscription (even though it is not available) - select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 1 - click_link 'Add' - within "#sli_1" do - expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}" - expect(page).to have_selector 'td.price', text: "$22.00" - expect(page).to have_input 'quantity', with: "1" - expect(page).to have_selector 'td.total', text: "$22.00" - end - - # Total should be $29.75 - expect(page).to have_selector '#order_form_total', text: "$29.75" - - click_button 'Save Changes' - expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule" - - # Remove variant3 from the subscription - within '#sli_1' do - find("a.delete-item").click - end - - click_button 'Save Changes' - expect(page).to have_current_path admin_subscriptions_path - - select2_select shop.name, from: "shop_id" - expect(page).to have_selector "td.items.panel-toggle" - first("td.items.panel-toggle").click - - # Total should be $7.75 - expect(page).to have_selector '#order_form_total', text: "$7.75" - expect(page).to have_selector 'tr.item', count: 1 - expect(subscription.reload.subscription_line_items.length).to eq 1 - expect(subscription.subscription_line_items.first.variant).to eq variant2 + # Remove variant1 from the subscription + find("a.delete-item").click end - context "with initialised order that has been changed" do - let(:proxy_order) { subscription.proxy_orders.first } - let(:order) { proxy_order.initialise_order! } - let(:line_item) { order.line_items.first } + # Attempting to submit without a product + click_button 'Save Changes' + expect(page).to have_content 'Please add at least one product' - before { line_item.update_attributes(quantity: 3) } + # Add variant2 to the subscription + select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' + fill_in 'add_quantity', with: 1 + click_link 'Add' + within "#sli_0" do + expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}" + expect(page).to have_selector 'td.price', text: "$7.75" + expect(page).to have_input 'quantity', with: "1" + expect(page).to have_selector 'td.total', text: "$7.75" + end - it "reports issues encountered during the update" do - visit edit_admin_subscription_path(subscription) - click_button 'edit-products' + # Total should be $7.75 + expect(page).to have_selector '#order_form_total', text: "$7.75" - within "#sli_0" do - fill_in 'quantity', with: "1" - end + # Add variant3 to the subscription (even though it is not available) + select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' + fill_in 'add_quantity', with: 1 + click_link 'Add' + within "#sli_1" do + expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}" + expect(page).to have_selector 'td.price', text: "$22.00" + expect(page).to have_input 'quantity', with: "1" + expect(page).to have_selector 'td.total', text: "$22.00" + end - click_button 'Save Changes' - expect(page).to have_content 'Saved' + # Total should be $29.75 + expect(page).to have_selector '#order_form_total', text: "$29.75" - expect(page).to have_selector "#order_update_issues_dialog .message", text: I18n.t("admin.subscriptions.order_update_issues_msg") + click_button 'Save Changes' + expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule" + + # Remove variant3 from the subscription + within '#sli_1' do + find("a.delete-item").click + end + + click_button 'Save Changes' + expect(page).to have_current_path admin_subscriptions_path + + select2_select shop.name, from: "shop_id" + expect(page).to have_selector "td.items.panel-toggle" + first("td.items.panel-toggle").click + + # Total should be $7.75 + expect(page).to have_selector '#order_form_total', text: "$7.75" + expect(page).to have_selector 'tr.item', count: 1 + expect(subscription.reload.subscription_line_items.length).to eq 1 + expect(subscription.subscription_line_items.first.variant).to eq variant2 + end + + context "with initialised order that has been changed" do + let(:proxy_order) { subscription.proxy_orders.first } + let(:order) { proxy_order.initialise_order! } + let(:line_item) { order.line_items.first } + + before { line_item.update_attributes(quantity: 3) } + + it "reports issues encountered during the update" do + visit edit_admin_subscription_path(subscription) + click_button 'edit-products' + + within "#sli_0" do + fill_in 'quantity', with: "1" end + + click_button 'Save Changes' + expect(page).to have_content 'Saved' + + expect(page).to have_selector "#order_update_issues_dialog .message", text: I18n.t("admin.subscriptions.order_update_issues_msg") end end end From 35c0bcb3df65d4604b3b95beb5fb712583d60098 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Wed, 12 Sep 2018 18:35:24 +0800 Subject: [PATCH 16/46] Add tests for eligible variants for a subscription --- spec/services/subscription_validator_spec.rb | 77 ++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb index 64dbc2637b..c4878414a5 100644 --- a/spec/services/subscription_validator_spec.rb +++ b/spec/services/subscription_validator_spec.rb @@ -463,4 +463,81 @@ describe SubscriptionValidator do end end end + + describe "variant eligibility for subscription" do + let!(:shop) { create(:distributor_enterprise) } + let!(:producer) { create(:supplier_enterprise) } + let!(:product) { create(:product, supplier: producer) } + let!(:variant) { product.variants.first } + + let!(:order_cycle) { current_order_cycle } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) } + let!(:subscription_line_item) do + create(:subscription_line_item, subscription: subscription, variant: variant) + end + + let(:current_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago, + orders_close_at: 1.week.from_now) + end + + let(:future_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now, + orders_close_at: 2.weeks.from_now) + end + + let(:past_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago, + orders_close_at: 1.week.ago) + end + + let(:validator) { SubscriptionValidator.new(subscription) } + + context "if it is not in an exchange" do + it "is not eligible" do + expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) + end + end + + context "if it is only in an incoming exchange" do + let!(:incoming_exchange) do + create(:exchange, order_cycle: order_cycle, sender: producer, receiver: shop, + incoming: true, variants: [variant]) + end + + it "is not eligible" do + expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) + end + end + + context "if is an outgoing exchange where the shop is the receiver" do + let!(:outgoing_exchange) do + create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop, + incoming: false, variants: [variant]) + end + + context "if the order cycle is currently open" do + it "is eligible" do + expect(validator.__send__(:available_variant_ids)).to include(variant.id) + end + end + + context "if the order cycle opens in the future" do + let!(:order_cycle) { future_order_cycle } + + it "is eligible" do + expect(validator.__send__(:available_variant_ids)).to include(variant.id) + end + end + + context "if the order cycle closed in the past" do + let!(:order_cycle) { past_order_cycle } + + it "is not eligible" do + expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) + end + end + end + end end From d3f20289f85e1fd3f4240af3fc9d03a8201768f9 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Wed, 12 Sep 2018 22:40:33 +0800 Subject: [PATCH 17/46] Move setup of feature specs for creating subscriptions --- spec/features/admin/subscriptions_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index 6610c006ee..be2eaa6ba5 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -156,12 +156,14 @@ feature 'Subscriptions' do let!(:payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } - it "passes the smoke test" do + before do visit admin_subscriptions_path - click_link 'New Subscription' - select2_select shop.name, from: 'new_subscription_shop_id' - click_button 'Continue' + click_link "New Subscription" + select2_select shop.name, from: "new_subscription_shop_id" + click_button "Continue" + end + it "passes the smoke test" do select2_select customer.email, from: 'customer_id' select2_select schedule.name, from: 'schedule_id' select2_select payment_method.name, from: 'payment_method_id' From 57f6a7a3b9dffb7a4b3cf7eabf72a111f4804d9c Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Wed, 19 Sep 2018 19:01:32 +0800 Subject: [PATCH 18/46] Support clicking different text for select2 helper Interaction with the variant autocomplete is not precise. The specs only search for the product name, then click the first result that matches the product name which they see. This could have been the case because searching using the full variant name does not match the variant. For example, searching "Some Product - 1kg" would not have results, while searching only "Some Product" (the product name) would list "Some Product - 1kg". Clicking the first match does not work in all scenarios. This allows using a separate text for searching and for clicking. --- spec/support/request/web_helper.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index c37014e637..912c19e445 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -105,6 +105,16 @@ module WebHelper targetted_select2(value, options) end + # Support having different texts to search for and to click in the select2 + # field. + # + # This overrides the method in Spree. + def targetted_select2_search(value, options) + page.execute_script %Q{$('#{options[:from]}').select2('open')} + page.execute_script "$('#{options[:dropdown_css]} input.select2-input').val('#{value}').trigger('keyup-change');" + select_select2_result(options[:select_text] || value) + end + def multi_select2_select(value, options) find("#s2id_#{options[:from]}").find('ul li.select2-search-field').click select_select2_result(value) From 929290fc77c34b5f4205a75d9ab8ee09e76158e4 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 20 Jan 2019 17:51:37 +0800 Subject: [PATCH 19/46] Reduce restrictions for creating subscriptions Allow the following variants: * Variants of permitted producers * Variants of hub * Variants that are in outgoing exchanges where the hub is receiver --- .../subscription_controller.js.coffee | 6 +- .../directives/variant_autocomplete.js.coffee | 1 + .../subscription_line_items_controller.rb | 7 +- app/services/subscription_validator.rb | 15 +- .../scope_variants_for_search.rb | 27 +++- lib/open_food_network/subscription_service.rb | 28 ++++ ...subscription_line_items_controller_spec.rb | 4 +- .../admin/subscriptions_controller_spec.rb | 2 +- spec/features/admin/subscriptions_spec.rb | 141 ++++++++++++++---- .../subscription_service_spec.rb | 99 ++++++++++++ spec/services/subscription_validator_spec.rb | 87 +---------- 11 files changed, 296 insertions(+), 121 deletions(-) create mode 100644 lib/open_food_network/subscription_service.rb create mode 100644 spec/lib/open_food_network/subscription_service_spec.rb diff --git a/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee b/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee index 3b26866125..aa635da8cc 100644 --- a/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee +++ b/app/assets/javascripts/admin/subscriptions/controllers/subscription_controller.js.coffee @@ -6,7 +6,11 @@ angular.module("admin.subscriptions").controller "SubscriptionController", ($sco $scope.schedules = Schedules.all $scope.paymentMethods = PaymentMethods.all $scope.shippingMethods = ShippingMethods.all - $scope.distributor_id = $scope.subscription.shop_id # variant selector requires distributor_id + + # Variant selector requires these + $scope.distributor_id = $scope.subscription.shop_id + $scope.eligible_for_subscriptions = true + $scope.view = if $scope.subscription.id? then 'review' else 'details' $scope.nextCallbacks = {} $scope.backCallbacks = {} diff --git a/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee b/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee index 9b8f12d3dc..17a467e463 100644 --- a/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee +++ b/app/assets/javascripts/admin/utils/directives/variant_autocomplete.js.coffee @@ -22,6 +22,7 @@ angular.module("admin.utils").directive "variantAutocomplete", ($timeout) -> q: term distributor_id: scope.distributor_id order_cycle_id: scope.order_cycle_id + eligible_for_subscriptions: scope.eligible_for_subscriptions results: (data, page) -> results: data formatResult: (variant) -> diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb index d981b71fe1..8c0432578d 100644 --- a/app/controllers/admin/subscription_line_items_controller.rb +++ b/app/controllers/admin/subscription_line_items_controller.rb @@ -1,6 +1,7 @@ require 'open_food_network/permissions' require 'open_food_network/order_cycle_permissions' require 'open_food_network/scope_variant_to_hub' +require "open_food_network/subscription_service" module Admin class SubscriptionLineItemsController < ResourceController @@ -26,7 +27,7 @@ module Admin @shop = Enterprise.managed_by(spree_current_user).find_by_id(params[:shop_id]) @schedule = permissions.editable_schedules.find_by_id(params[:schedule_id]) @order_cycle = @schedule.andand.current_or_next_order_cycle - @variant = Spree::Variant.stockable_by(@shop).find_by_id(params[:subscription_line_item][:variant_id]) + @variant = variant_if_eligible(params[:subscription_line_item][:variant_id]) if @shop.present? end def new_actions @@ -50,5 +51,9 @@ module Admin OpenFoodNetwork::ScopeVariantToHub.new(@shop).scope(@variant) @variant.price + fee_calculator.indexed_fees_for(@variant) end + + def variant_if_eligible(variant_id) + OpenFoodNetwork::SubscriptionService.eligible_variants(@shop).find_by_id(variant_id) + end end end diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb index 33fc2baf77..8ee43b9c4f 100644 --- a/app/services/subscription_validator.rb +++ b/app/services/subscription_validator.rb @@ -2,6 +2,8 @@ # Public interface consists of #valid? method provided by ActiveModel::Validations # and #json_errors which compiles a serializable hash of errors +require "open_food_network/subscription_service" + class SubscriptionValidator include ActiveModel::Naming include ActiveModel::Conversion @@ -97,15 +99,12 @@ class SubscriptionValidator errors.add(:subscription_line_items, :not_available, name: name) end - # TODO: Extract this into a separate class def available_variant_ids - @available_variant_ids ||= - Spree::Variant.joins(exchanges: { order_cycle: :schedules }) - .where(id: subscription_line_items.map(&:variant_id)) - .where(schedules: { id: schedule }, exchanges: { incoming: false, receiver_id: shop }) - .merge(OrderCycle.not_closed) - .select('DISTINCT spree_variants.id') - .pluck(:id) + return @available_variant_ids if @available_variant_ids.present? + + subscription_variant_ids = subscription_line_items.map(&:variant_id) + @available_variant_ids = OpenFoodNetwork::SubscriptionService.eligible_variants(shop) + .where(id: subscription_variant_ids).pluck(:id) end def build_msg_from(k, msg) diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb index 9befef5304..c1f90df7d1 100644 --- a/lib/open_food_network/scope_variants_for_search.rb +++ b/lib/open_food_network/scope_variants_for_search.rb @@ -5,6 +5,8 @@ require 'open_food_network/scope_variant_to_hub' # Further restrictions on the schedule, order_cycle or distributor through which the # products are available are also possible +require "open_food_network/subscription_service" + module OpenFoodNetwork class ScopeVariantsForSearch def initialize(params) @@ -33,6 +35,10 @@ module OpenFoodNetwork Spree::Variant.where(is_master: false).ransack(search_params.merge(m: 'or')).result end + def distributor + Enterprise.find params[:distributor_id] + end + def scope_to_schedule @variants = @variants.in_schedule(params[:schedule_id]) end @@ -42,12 +48,29 @@ module OpenFoodNetwork end def scope_to_distributor - distributor = Enterprise.find params[:distributor_id] + if params[:eligible_for_subscriptions] + scope_to_eligible_for_subscriptions_in_distributor + else + scope_to_available_for_orders_in_distributor + end + end + + def scope_to_available_for_orders_in_distributor @variants = @variants.in_distributor(distributor) + scope_variants_to_distributor(@variants, distributor) + end + + def scope_to_eligible_for_subscriptions_in_distributor + eligible_variants_scope = OpenFoodNetwork::SubscriptionService.eligible_variants(distributor) + @variants = @variants.merge(eligible_variants_scope) + scope_variants_to_distributor(@variants, distributor) + end + + def scope_variants_to_distributor(variants, distributor) scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor) # Perform scoping after all filtering is done. # Filtering could be a problem on scoped variants. - @variants.each { |v| scoper.scope(v) } + variants.each { |v| scoper.scope(v) } end end end diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb new file mode 100644 index 0000000000..e3479275c1 --- /dev/null +++ b/lib/open_food_network/subscription_service.rb @@ -0,0 +1,28 @@ +module OpenFoodNetwork + class SubscriptionService + # Includes the following variants: + # - Variants of permitted producers + # - Variants of hub + # - Variants that are in outgoing exchanges where the hub is receiver + def self.eligible_variants(distributor) + permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor) + .with_permission(:add_to_order_cycle).pluck(:parent_id) + permitted_producer_ids = Enterprise.is_primary_producer + .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id) + + outgoing_exchange_variant_ids = ExchangeVariant + .select("DISTINCT exchange_variants.variant_id") + .joins(:exchange) + .where(exchanges: { incoming: false, receiver_id: distributor.id }) + .pluck(:variant_id) + + variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]] + if outgoing_exchange_variant_ids.present? + variant_conditions[0] << " OR spree_variants.id IN (?)" + variant_conditions << outgoing_exchange_variant_ids + end + + Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions) + end + end +end diff --git a/spec/controllers/admin/subscription_line_items_controller_spec.rb b/spec/controllers/admin/subscription_line_items_controller_spec.rb index b2f3a0f432..2b42a20df7 100644 --- a/spec/controllers/admin/subscription_line_items_controller_spec.rb +++ b/spec/controllers/admin/subscription_line_items_controller_spec.rb @@ -10,9 +10,9 @@ describe Admin::SubscriptionLineItemsController, type: :controller do let(:unmanaged_shop) { create(:enterprise) } let!(:product) { create(:product) } let!(:variant) { create(:variant, product: product, unit_value: '100', price: 15.00, option_values: []) } + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], enterprise_fees: [enterprise_fee]) } let!(:enterprise_fee) { create(:enterprise_fee, amount: 3.50) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant], enterprise_fees: [enterprise_fee]) } let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } let(:unmanaged_schedule) { create(:schedule, order_cycles: [create(:simple_order_cycle, coordinator: unmanaged_shop)]) } @@ -42,6 +42,8 @@ describe Admin::SubscriptionLineItemsController, type: :controller do before { params.merge!(shop_id: shop.id) } context "but the shop doesn't have permission to sell product in question" do + let!(:outgoing_exchange) { } + it "returns an error" do spree_post :build, params json_response = JSON.parse(response.body) diff --git a/spec/controllers/admin/subscriptions_controller_spec.rb b/spec/controllers/admin/subscriptions_controller_spec.rb index 5f83bf0529..9385d453f3 100644 --- a/spec/controllers/admin/subscriptions_controller_spec.rb +++ b/spec/controllers/admin/subscriptions_controller_spec.rb @@ -341,7 +341,7 @@ describe Admin::SubscriptionsController, type: :controller do end context 'with subscription_line_items params' do - let!(:product2) { create(:product, supplier: shop) } + let!(:product2) { create(:product) } let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) } before do diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index be2eaa6ba5..0a03d629f4 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -145,13 +145,13 @@ feature 'Subscriptions' do let!(:customer_user) { create(:user) } let!(:credit_card1) { create(:credit_card, user: customer_user, cc_type: 'visa', last_digits: 1111, month: 10, year: 2030) } let!(:customer) { create(:customer, enterprise: shop, bill_address: address, user: customer_user, allow_charges: true) } - let!(:product1) { create(:product, supplier: shop) } - let!(:product2) { create(:product, supplier: shop) } - let!(:variant1) { create(:variant, product: product1, unit_value: '100', price: 12.00, option_values: []) } - let!(:variant2) { create(:variant, product: product2, unit_value: '1000', price: 6.00, option_values: []) } + let!(:test_product) { create(:product, supplier: shop, distributors: []) } + let!(:test_variant) { create(:variant, product: test_product, unit_value: "100", price: 12.00, option_values: []) } + let!(:shop_product) { create(:product, supplier: shop, distributors: [shop]) } + let!(:shop_variant) { create(:variant, product: shop_product, unit_value: "1000", price: 6.00, option_values: []) } let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.days.from_now, orders_close_at: 7.days.from_now) } - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [variant1, variant2], enterprise_fees: [enterprise_fee]) } + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: shop, receiver: shop, variants: [test_variant, shop_variant], enterprise_fees: [enterprise_fee]) } let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } let!(:payment_method) { create(:stripe_payment_method, name: 'Credit Card', distributors: [shop], preferred_enterprise_id: shop.id) } let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } @@ -217,11 +217,9 @@ feature 'Subscriptions' do expect(page).to have_content 'Please add at least one product' # Adding a product and getting a price estimate - select2_search_async product1.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 2 - click_link 'Add' + add_variant_to_subscription test_variant, 2 within 'table#subscription-line-items tr.item', match: :first do - expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}" + expect(page).to have_selector '.description', text: "#{test_product.name} - #{test_variant.full_name}" expect(page).to have_selector 'td.price', text: "$13.75" expect(page).to have_input 'quantity', with: "2" expect(page).to have_selector 'td.total', text: "$27.50" @@ -243,11 +241,9 @@ feature 'Subscriptions' do click_button('edit-products') # Adding a new product - select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 3 - click_link 'Add' + add_variant_to_subscription shop_variant, 3 within 'table#subscription-line-items tr.item', match: :first do - expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}" + expect(page).to have_selector '.description', text: "#{shop_product.name} - #{shop_variant.full_name}" expect(page).to have_selector 'td.price', text: "$7.75" expect(page).to have_input 'quantity', with: "3" expect(page).to have_selector 'td.total', text: "$23.25" @@ -266,7 +262,7 @@ feature 'Subscriptions' do # Prices are shown in the index within 'table#subscription-line-items tr.item', match: :first do - expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}" + expect(page).to have_selector '.description', text: "#{shop_product.name} - #{shop_variant.full_name}" expect(page).to have_selector 'td.price', text: "$7.75" expect(page).to have_input 'quantity', with: "3" expect(page).to have_selector 'td.total', text: "$23.25" @@ -284,7 +280,7 @@ feature 'Subscriptions' do # Standing Line Items are created expect(subscription.subscription_line_items.count).to eq 1 subscription_line_item = subscription.subscription_line_items.first - expect(subscription_line_item.variant).to eq variant2 + expect(subscription_line_item.variant).to eq shop_variant expect(subscription_line_item.quantity).to eq 3 end end @@ -336,7 +332,7 @@ feature 'Subscriptions' do # Existing products should be visible click_button 'edit-products' within "#sli_0" do - expect(page).to have_selector 'td.description', text: "#{product1.name} - #{variant1.full_name}" + expect(page).to have_selector '.description', text: "#{product1.name} - #{variant1.full_name}" expect(page).to have_selector 'td.price', text: "$13.75" expect(page).to have_input 'quantity', with: "2" expect(page).to have_selector 'td.total', text: "$27.50" @@ -350,11 +346,9 @@ feature 'Subscriptions' do expect(page).to have_content 'Please add at least one product' # Add variant2 to the subscription - select2_search_async product2.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 1 - click_link 'Add' + add_variant_to_subscription(variant2, 1) within "#sli_0" do - expect(page).to have_selector 'td.description', text: "#{product2.name} - #{variant2.full_name}" + expect(page).to have_selector '.description', text: "#{product2.name} - #{variant2.full_name}" expect(page).to have_selector 'td.price', text: "$7.75" expect(page).to have_input 'quantity', with: "1" expect(page).to have_selector 'td.total', text: "$7.75" @@ -364,11 +358,9 @@ feature 'Subscriptions' do expect(page).to have_selector '#order_form_total', text: "$7.75" # Add variant3 to the subscription (even though it is not available) - select2_search_async product3.name, from: I18n.t(:name_or_sku), dropdown_css: '.select2-drop' - fill_in 'add_quantity', with: 1 - click_link 'Add' + add_variant_to_subscription(variant3, 1) within "#sli_1" do - expect(page).to have_selector 'td.description', text: "#{product3.name} - #{variant3.full_name}" + expect(page).to have_selector '.description', text: "#{product3.name} - #{variant3.full_name}" expect(page).to have_selector 'td.price', text: "$22.00" expect(page).to have_input 'quantity', with: "1" expect(page).to have_selector 'td.total', text: "$22.00" @@ -377,9 +369,6 @@ feature 'Subscriptions' do # Total should be $29.75 expect(page).to have_selector '#order_form_total', text: "$29.75" - click_button 'Save Changes' - expect(page).to have_content "#{product3.name} - #{variant3.full_name} is not available from the selected schedule" - # Remove variant3 from the subscription within '#sli_1' do find("a.delete-item").click @@ -421,5 +410,103 @@ feature 'Subscriptions' do end end end + + describe "allowed variants" do + let!(:customer) { create(:customer, enterprise: shop, allow_charges: true) } + let!(:credit_card) { create(:credit_card, user: customer.user) } + let!(:shop_product) { create(:product, supplier: shop, distributors: [shop]) } + let!(:shop_variant) { create(:variant, product: shop_product, unit_value: "2000") } + let!(:permitted_supplier) do + create(:supplier_enterprise).tap do |supplier| + create(:enterprise_relationship, child: shop, parent: supplier, permissions_list: [:add_to_order_cycle]) + end + end + let!(:permitted_supplier_product) { create(:product, supplier: permitted_supplier, distributors: [shop]) } + let!(:permitted_supplier_variant) { create(:variant, product: permitted_supplier_product, unit_value: "2000") } + let!(:incoming_exchange_product) { create(:product, distributors: [shop]) } + let!(:incoming_exchange_variant) do + create(:variant, product: incoming_exchange_product, unit_value: "2000").tap do |variant| + create(:exchange, order_cycle: order_cycle, incoming: true, receiver: shop, variants: [variant]) + end + end + let!(:outgoing_exchange_product) { create(:product, distributors: [shop]) } + let!(:outgoing_exchange_variant) do + create(:variant, product: outgoing_exchange_product, unit_value: "2000").tap do |variant| + create(:exchange, order_cycle: order_cycle, incoming: false, receiver: shop, variants: [variant]) + end + end + let!(:enterprise_fee) { create(:enterprise_fee, amount: 1.75) } + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let!(:payment_method) { create(:stripe_payment_method, distributors: [shop], preferred_enterprise_id: shop.id) } + let!(:shipping_method) { create(:shipping_method, distributors: [shop]) } + + before do + visit admin_subscriptions_path + click_link "New Subscription" + select2_select shop.name, from: "new_subscription_shop_id" + click_button "Continue" + end + + it "permit creating and editing of the subscription" do + select2_select customer.email, from: "customer_id" + select2_select schedule.name, from: "schedule_id" + select2_select payment_method.name, from: "payment_method_id" + select2_select shipping_method.name, from: "shipping_method_id" + find_field("begins_at").click + within(".ui-datepicker-calendar") do + find(".ui-datepicker-today").click + end + click_button "Next" + + expect(page).to have_content "BILLING ADDRESS" + click_button "Next" + + # Add products + expect(page).to have_content "NAME OR SKU" + add_variant_to_subscription shop_variant, 3 + add_variant_to_subscription permitted_supplier_variant, 4 + add_variant_to_subscription incoming_exchange_variant, 5 + add_variant_to_subscription outgoing_exchange_variant, 6 + click_button "Next" + + # Submit form + expect { + click_button "Create Subscription" + expect(page).to have_current_path admin_subscriptions_path + }.to change(Subscription, :count).by(1) + + # Subscription line items are created + subscription = Subscription.last + expect(subscription.subscription_line_items.count).to eq 4 + + # Edit the subscription + visit edit_admin_subscription_path(subscription) + + # Remove shop_variant from the subscription + click_button "edit-products" + within "#sli_0" do + expect(page).to have_selector ".description", text: shop_variant.name + find("a.delete-item").click + end + + # Submit form + click_button "Save Changes" + expect(page).to have_current_path admin_subscriptions_path + + # Subscription is saved + visit edit_admin_subscription_path(subscription) + expect(page).to have_selector "#subscription-line-items .item", count: 3 + end + end + end + + def add_variant_to_subscription(variant, quantity) + row_count = all("#subscription-line-items .item").length + variant_name = variant.full_name.present? ? "#{variant.name} - #{variant.full_name}" : variant.name + select2_search variant.name, from: I18n.t(:name_or_sku), dropdown_css: ".select2-drop", select_text: variant_name + fill_in "add_quantity", with: quantity + click_link "Add" + expect(page).to have_selector("#subscription-line-items .item", count: row_count + 1) end end diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb new file mode 100644 index 0000000000..7df860cf03 --- /dev/null +++ b/spec/lib/open_food_network/subscription_service_spec.rb @@ -0,0 +1,99 @@ +require "spec_helper" +require "open_food_network/subscription_service" + +module OpenFoodNetwork + describe SubscriptionService do + describe "variant eligibility for subscription" do + let!(:shop) { create(:distributor_enterprise) } + let!(:producer) { create(:supplier_enterprise) } + let!(:product) { create(:product, supplier: producer) } + let!(:variant) { product.variants.first } + + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) } + let!(:subscription_line_item) do + create(:subscription_line_item, subscription: subscription, variant: variant) + end + + let(:current_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago, + orders_close_at: 1.week.from_now) + end + + let(:future_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now, + orders_close_at: 2.weeks.from_now) + end + + let(:past_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago, + orders_close_at: 1.week.ago) + end + + let!(:order_cycle) { current_order_cycle } + + context "if the shop is the supplier for the product" do + let!(:producer) { shop } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the supplier is permitted for the shop" do + let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the variant is involved in an exchange" do + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + + context "if it is an incoming exchange where the shop is the receiver" do + let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + + it "is not eligible" do + expect(described_class.eligible_variants(shop)).to_not include(variant) + end + end + + context "if it is an outgoing exchange where the shop is the receiver" do + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + + context "if the order cycle is currently open" do + let!(:order_cycle) { current_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the order cycle opens in the future" do + let!(:order_cycle) { future_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the order cycle closed in the past" do + let!(:order_cycle) { past_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + end + end + + context "if the variant is unrelated" do + it "is not eligible" do + expect(described_class.eligible_variants(shop)).to_not include(variant) + end + end + end + end +end diff --git a/spec/services/subscription_validator_spec.rb b/spec/services/subscription_validator_spec.rb index c4878414a5..bdcd14bea5 100644 --- a/spec/services/subscription_validator_spec.rb +++ b/spec/services/subscription_validator_spec.rb @@ -1,10 +1,11 @@ require "spec_helper" describe SubscriptionValidator do - let(:shop) { instance_double(Enterprise, name: "Shop") } + let(:owner) { create(:user) } + let(:shop) { create(:enterprise, name: "Shop", owner: owner) } describe "delegation" do - let(:subscription) { create(:subscription) } + let(:subscription) { create(:subscription, shop: shop) } let(:validator) { SubscriptionValidator.new(subscription) } it "delegates to subscription" do @@ -440,6 +441,7 @@ describe SubscriptionValidator do context "but some variants are unavailable" do let(:product) { instance_double(Spree::Product, name: "some_name") } + before do allow(validator).to receive(:available_variant_ids) { [variant2.id] } allow(variant1).to receive(:product) { product } @@ -453,7 +455,9 @@ describe SubscriptionValidator do end context "and all requested variants are available" do - before { allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] } } + before do + allow(validator).to receive(:available_variant_ids) { [variant1.id, variant2.id] } + end it "returns true" do expect(validator.valid?).to be true @@ -463,81 +467,4 @@ describe SubscriptionValidator do end end end - - describe "variant eligibility for subscription" do - let!(:shop) { create(:distributor_enterprise) } - let!(:producer) { create(:supplier_enterprise) } - let!(:product) { create(:product, supplier: producer) } - let!(:variant) { product.variants.first } - - let!(:order_cycle) { current_order_cycle } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) } - let!(:subscription_line_item) do - create(:subscription_line_item, subscription: subscription, variant: variant) - end - - let(:current_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago, - orders_close_at: 1.week.from_now) - end - - let(:future_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now, - orders_close_at: 2.weeks.from_now) - end - - let(:past_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago, - orders_close_at: 1.week.ago) - end - - let(:validator) { SubscriptionValidator.new(subscription) } - - context "if it is not in an exchange" do - it "is not eligible" do - expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) - end - end - - context "if it is only in an incoming exchange" do - let!(:incoming_exchange) do - create(:exchange, order_cycle: order_cycle, sender: producer, receiver: shop, - incoming: true, variants: [variant]) - end - - it "is not eligible" do - expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) - end - end - - context "if is an outgoing exchange where the shop is the receiver" do - let!(:outgoing_exchange) do - create(:exchange, order_cycle: order_cycle, sender: shop, receiver: shop, - incoming: false, variants: [variant]) - end - - context "if the order cycle is currently open" do - it "is eligible" do - expect(validator.__send__(:available_variant_ids)).to include(variant.id) - end - end - - context "if the order cycle opens in the future" do - let!(:order_cycle) { future_order_cycle } - - it "is eligible" do - expect(validator.__send__(:available_variant_ids)).to include(variant.id) - end - end - - context "if the order cycle closed in the past" do - let!(:order_cycle) { past_order_cycle } - - it "is not eligible" do - expect(validator.__send__(:available_variant_ids)).to_not include(variant.id) - end - end - end - end end From c23002102cc4b7b1593cf4e86023a435e4e03199 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Mon, 17 Sep 2018 01:14:40 +1000 Subject: [PATCH 20/46] Add warning for unavailable subscription items --- .../admin/pages/subscription_form.css.scss | 5 +++ .../admin/pages/subscription_review.css.scss | 5 +++ .../subscription_line_items_controller.rb | 3 +- .../subscription_line_item_serializer.rb | 19 ++++++++++- .../admin/subscriptions/_review.html.haml | 7 ++-- .../_subscription_line_items.html.haml | 7 ++-- config/locales/en.yml | 2 ++ lib/open_food_network/subscription_service.rb | 8 +++++ spec/features/admin/subscriptions_spec.rb | 9 +++++ .../subscription_service_spec.rb | 34 +++++++++++++++++++ 10 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 app/assets/stylesheets/admin/pages/subscription_form.css.scss create mode 100644 app/assets/stylesheets/admin/pages/subscription_review.css.scss diff --git a/app/assets/stylesheets/admin/pages/subscription_form.css.scss b/app/assets/stylesheets/admin/pages/subscription_form.css.scss new file mode 100644 index 0000000000..02c9fd783e --- /dev/null +++ b/app/assets/stylesheets/admin/pages/subscription_form.css.scss @@ -0,0 +1,5 @@ +.admin-subscription-form-subscription-line-items { + .not-in-open-and-upcoming-order-cycles-warning { + color: $warning-red; + } +} diff --git a/app/assets/stylesheets/admin/pages/subscription_review.css.scss b/app/assets/stylesheets/admin/pages/subscription_review.css.scss new file mode 100644 index 0000000000..ba3280aa2b --- /dev/null +++ b/app/assets/stylesheets/admin/pages/subscription_review.css.scss @@ -0,0 +1,5 @@ +.admin-subscription-review-subscription-line-items { + .not-in-open-and-upcoming-order-cycles-warning { + color: $warning-red; + } +} diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb index 8c0432578d..378b73b990 100644 --- a/app/controllers/admin/subscription_line_items_controller.rb +++ b/app/controllers/admin/subscription_line_items_controller.rb @@ -14,7 +14,8 @@ module Admin def build @subscription_line_item.assign_attributes(params[:subscription_line_item]) @subscription_line_item.price_estimate = price_estimate - render json: @subscription_line_item, serializer: Api::Admin::SubscriptionLineItemSerializer + render json: @subscription_line_item, serializer: Api::Admin::SubscriptionLineItemSerializer, + shop: @shop, schedule: @schedule end private diff --git a/app/serializers/api/admin/subscription_line_item_serializer.rb b/app/serializers/api/admin/subscription_line_item_serializer.rb index 23f4c6bc52..ca8028d7b5 100644 --- a/app/serializers/api/admin/subscription_line_item_serializer.rb +++ b/app/serializers/api/admin/subscription_line_item_serializer.rb @@ -1,7 +1,8 @@ module Api module Admin class SubscriptionLineItemSerializer < ActiveModel::Serializer - attributes :id, :variant_id, :quantity, :description, :price_estimate + attributes :id, :variant_id, :quantity, :description, :price_estimate, + :in_open_and_upcoming_order_cycles def description "#{object.variant.product.name} - #{object.variant.full_name}" @@ -10,6 +11,22 @@ module Api def price_estimate object.price_estimate.andand.to_f || "?" end + + def in_open_and_upcoming_order_cycles + OpenFoodNetwork::SubscriptionService + .in_open_and_upcoming_order_cycles?(option_or_assigned_shop, option_or_assigned_schedule, + object.variant) + end + + private + + def option_or_assigned_shop + @options[:shop] || object.subscription.andand.shop + end + + def option_or_assigned_schedule + @options[:schedule] || object.subscription.andand.schedule + end end end end diff --git a/app/views/admin/subscriptions/_review.html.haml b/app/views/admin/subscriptions/_review.html.haml index e1591e6210..7e97bd02aa 100644 --- a/app/views/admin/subscriptions/_review.html.haml +++ b/app/views/admin/subscriptions/_review.html.haml @@ -56,7 +56,7 @@ %input#edit-products{ type: "button", value: t(:edit), ng: { click: "setView('products')" } } .row .seven.columns.alpha.omega - %table#subscription-line-items + %table#subscription-line-items.admin-subscription-review-subscription-line-items %colgroup %col{:style => "width: 62%;"}/ %col{:style => "width: 14%;"}/ @@ -71,7 +71,10 @@ %span= t(:total) %tbody %tr.item{ id: "sli_{{$index}}", ng: { repeat: "item in subscription.subscription_line_items | filter:{ _destroy: '!true' }", class: { even: 'even', odd: 'odd' } } } - %td.description {{ item.description }} + %td + .description {{ item.description }} + .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } } + = t(".no_open_or_upcoming_order_cycle") %td.price.align-center {{ item.price_estimate | currency }} %td.quantity {{ item.quantity }} %td.total.align-center {{ (item.price_estimate * item.quantity) | currency }} diff --git a/app/views/admin/subscriptions/_subscription_line_items.html.haml b/app/views/admin/subscriptions/_subscription_line_items.html.haml index 0be91f2c93..b80b95a85b 100644 --- a/app/views/admin/subscriptions/_subscription_line_items.html.haml +++ b/app/views/admin/subscriptions/_subscription_line_items.html.haml @@ -1,4 +1,4 @@ -%table#subscription-line-items +%table#subscription-line-items.admin-subscription-form-subscription-line-items %colgroup %col{:style => "width: 49%;"}/ %col{:style => "width: 14%;"}/ @@ -15,7 +15,10 @@ %th.orders-actions.actions %tbody %tr.item{ id: "sli_{{$index}}", ng: { repeat: "item in subscription.subscription_line_items | filter:{ _destroy: '!true' }", class: { even: 'even', odd: 'odd' } } } - %td.description {{ item.description }} + %td + .description {{ item.description }} + .not-in-open-and-upcoming-order-cycles-warning{ ng: { if: '!item.in_open_and_upcoming_order_cycles' } } + = t(".not_in_open_and_upcoming_order_cycles_warning") %td.price.align-center {{ item.price_estimate | currency }} %td.quantity %input{ name: 'quantity', type: 'number', min: 0, ng: { model: 'item.quantity' } } diff --git a/config/locales/en.yml b/config/locales/en.yml index 0bde4fb005..366a481bac 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1088,6 +1088,7 @@ en: this_is_an_estimate: | The displayed prices are only an estimate and calculated at the time the subscription is changed. If you change prices or fees, orders will be updated, but the subscription will still display the old values. + not_in_open_and_upcoming_order_cycles_warning: "There are no open or upcoming order cycles for this product." details: details: Details invalid_error: Oops! Please fill in all of the required fields... @@ -1102,6 +1103,7 @@ en: details: Details address: Address products: Products + no_open_or_upcoming_order_cycle: "No Upcoming OC" product_already_in_order: This product has already been added to the order. Please edit the quantity directly. orders: number: Number diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb index e3479275c1..e4f48aa67a 100644 --- a/lib/open_food_network/subscription_service.rb +++ b/lib/open_food_network/subscription_service.rb @@ -24,5 +24,13 @@ module OpenFoodNetwork Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions) end + + def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant) + scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules }) + .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor }) + .merge(OrderCycle.not_closed) + scope = scope.where(schedules: { id: schedule }) + scope.any? + end end end diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index 0a03d629f4..f51de02e5f 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -465,9 +465,13 @@ feature 'Subscriptions' do # Add products expect(page).to have_content "NAME OR SKU" add_variant_to_subscription shop_variant, 3 + expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 1 add_variant_to_subscription permitted_supplier_variant, 4 + expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 2 add_variant_to_subscription incoming_exchange_variant, 5 + expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3 add_variant_to_subscription outgoing_exchange_variant, 6 + expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3 click_button "Next" # Submit form @@ -509,4 +513,9 @@ feature 'Subscriptions' do click_link "Add" expect(page).to have_selector("#subscription-line-items .item", count: row_count + 1) end + + def variant_not_in_open_or_upcoming_order_cycle_warning + I18n.t("not_in_open_and_upcoming_order_cycles_warning", + scope: "admin.subscriptions.subscription_line_items") + end end diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb index 7df860cf03..63ef72707d 100644 --- a/spec/lib/open_food_network/subscription_service_spec.rb +++ b/spec/lib/open_food_network/subscription_service_spec.rb @@ -95,5 +95,39 @@ module OpenFoodNetwork end end end + + describe "checking if variant in open and upcoming order cycles" do + let!(:shop) { create(:enterprise) } + let!(:product) { create(:product) } + let!(:variant) { product.variants.first } + let!(:schedule) { create(:schedule) } + + context "if the variant is involved in an exchange" do + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + + context "if it is an incoming exchange where the shop is the receiver" do + let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + + it "is is false" do + expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + + context "if it is an outgoing exchange where the shop is the receiver" do + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + + it "is true" do + expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + end + + context "if the variant is unrelated" do + it "is false" do + expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + end end end From b691d727a780da4f78103f1eb67e77e2e70827bc Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 20 Jan 2019 23:18:52 +1100 Subject: [PATCH 21/46] Move OFN::SubscriptionService to SubscriptionVariantsService --- .../subscription_line_items_controller.rb | 3 +- .../subscription_line_item_serializer.rb | 6 +- app/services/subscription_validator.rb | 4 +- app/services/subscription_variants_service.rb | 34 +++++ .../scope_variants_for_search.rb | 4 +- lib/open_food_network/subscription_service.rb | 36 ----- .../subscription_service_spec.rb | 133 ------------------ .../subscription_variants_service_spec.rb | 130 +++++++++++++++++ 8 files changed, 170 insertions(+), 180 deletions(-) create mode 100644 app/services/subscription_variants_service.rb delete mode 100644 lib/open_food_network/subscription_service.rb delete mode 100644 spec/lib/open_food_network/subscription_service_spec.rb create mode 100644 spec/services/subscription_variants_service_spec.rb diff --git a/app/controllers/admin/subscription_line_items_controller.rb b/app/controllers/admin/subscription_line_items_controller.rb index 378b73b990..5267374539 100644 --- a/app/controllers/admin/subscription_line_items_controller.rb +++ b/app/controllers/admin/subscription_line_items_controller.rb @@ -1,7 +1,6 @@ require 'open_food_network/permissions' require 'open_food_network/order_cycle_permissions' require 'open_food_network/scope_variant_to_hub' -require "open_food_network/subscription_service" module Admin class SubscriptionLineItemsController < ResourceController @@ -54,7 +53,7 @@ module Admin end def variant_if_eligible(variant_id) - OpenFoodNetwork::SubscriptionService.eligible_variants(@shop).find_by_id(variant_id) + SubscriptionVariantsService.eligible_variants(@shop).find_by_id(variant_id) end end end diff --git a/app/serializers/api/admin/subscription_line_item_serializer.rb b/app/serializers/api/admin/subscription_line_item_serializer.rb index ca8028d7b5..34bc00c6c0 100644 --- a/app/serializers/api/admin/subscription_line_item_serializer.rb +++ b/app/serializers/api/admin/subscription_line_item_serializer.rb @@ -13,9 +13,9 @@ module Api end def in_open_and_upcoming_order_cycles - OpenFoodNetwork::SubscriptionService - .in_open_and_upcoming_order_cycles?(option_or_assigned_shop, option_or_assigned_schedule, - object.variant) + SubscriptionVariantsService.in_open_and_upcoming_order_cycles?(option_or_assigned_shop, + option_or_assigned_schedule, + object.variant) end private diff --git a/app/services/subscription_validator.rb b/app/services/subscription_validator.rb index 8ee43b9c4f..051033dd9f 100644 --- a/app/services/subscription_validator.rb +++ b/app/services/subscription_validator.rb @@ -2,8 +2,6 @@ # Public interface consists of #valid? method provided by ActiveModel::Validations # and #json_errors which compiles a serializable hash of errors -require "open_food_network/subscription_service" - class SubscriptionValidator include ActiveModel::Naming include ActiveModel::Conversion @@ -103,7 +101,7 @@ class SubscriptionValidator return @available_variant_ids if @available_variant_ids.present? subscription_variant_ids = subscription_line_items.map(&:variant_id) - @available_variant_ids = OpenFoodNetwork::SubscriptionService.eligible_variants(shop) + @available_variant_ids = SubscriptionVariantsService.eligible_variants(shop) .where(id: subscription_variant_ids).pluck(:id) end diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb new file mode 100644 index 0000000000..eb6dcf0cf2 --- /dev/null +++ b/app/services/subscription_variants_service.rb @@ -0,0 +1,34 @@ +class SubscriptionVariantsService + # Includes the following variants: + # - Variants of permitted producers + # - Variants of hub + # - Variants that are in outgoing exchanges where the hub is receiver + def self.eligible_variants(distributor) + permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor) + .with_permission(:add_to_order_cycle).pluck(:parent_id) + permitted_producer_ids = Enterprise.is_primary_producer + .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id) + + outgoing_exchange_variant_ids = ExchangeVariant + .select("DISTINCT exchange_variants.variant_id") + .joins(:exchange) + .where(exchanges: { incoming: false, receiver_id: distributor.id }) + .pluck(:variant_id) + + variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]] + if outgoing_exchange_variant_ids.present? + variant_conditions[0] << " OR spree_variants.id IN (?)" + variant_conditions << outgoing_exchange_variant_ids + end + + Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions) + end + + def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant) + scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules }) + .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor }) + .merge(OrderCycle.not_closed) + scope = scope.where(schedules: { id: schedule }) + scope.any? + end +end diff --git a/lib/open_food_network/scope_variants_for_search.rb b/lib/open_food_network/scope_variants_for_search.rb index c1f90df7d1..24b110fb6e 100644 --- a/lib/open_food_network/scope_variants_for_search.rb +++ b/lib/open_food_network/scope_variants_for_search.rb @@ -5,8 +5,6 @@ require 'open_food_network/scope_variant_to_hub' # Further restrictions on the schedule, order_cycle or distributor through which the # products are available are also possible -require "open_food_network/subscription_service" - module OpenFoodNetwork class ScopeVariantsForSearch def initialize(params) @@ -61,7 +59,7 @@ module OpenFoodNetwork end def scope_to_eligible_for_subscriptions_in_distributor - eligible_variants_scope = OpenFoodNetwork::SubscriptionService.eligible_variants(distributor) + eligible_variants_scope = SubscriptionVariantsService.eligible_variants(distributor) @variants = @variants.merge(eligible_variants_scope) scope_variants_to_distributor(@variants, distributor) end diff --git a/lib/open_food_network/subscription_service.rb b/lib/open_food_network/subscription_service.rb deleted file mode 100644 index e4f48aa67a..0000000000 --- a/lib/open_food_network/subscription_service.rb +++ /dev/null @@ -1,36 +0,0 @@ -module OpenFoodNetwork - class SubscriptionService - # Includes the following variants: - # - Variants of permitted producers - # - Variants of hub - # - Variants that are in outgoing exchanges where the hub is receiver - def self.eligible_variants(distributor) - permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor) - .with_permission(:add_to_order_cycle).pluck(:parent_id) - permitted_producer_ids = Enterprise.is_primary_producer - .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id) - - outgoing_exchange_variant_ids = ExchangeVariant - .select("DISTINCT exchange_variants.variant_id") - .joins(:exchange) - .where(exchanges: { incoming: false, receiver_id: distributor.id }) - .pluck(:variant_id) - - variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]] - if outgoing_exchange_variant_ids.present? - variant_conditions[0] << " OR spree_variants.id IN (?)" - variant_conditions << outgoing_exchange_variant_ids - end - - Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions) - end - - def self.in_open_and_upcoming_order_cycles?(distributor, schedule, variant) - scope = ExchangeVariant.joins(exchange: { order_cycle: :schedules }) - .where(variant_id: variant, exchanges: { incoming: false, receiver_id: distributor }) - .merge(OrderCycle.not_closed) - scope = scope.where(schedules: { id: schedule }) - scope.any? - end - end -end diff --git a/spec/lib/open_food_network/subscription_service_spec.rb b/spec/lib/open_food_network/subscription_service_spec.rb deleted file mode 100644 index 63ef72707d..0000000000 --- a/spec/lib/open_food_network/subscription_service_spec.rb +++ /dev/null @@ -1,133 +0,0 @@ -require "spec_helper" -require "open_food_network/subscription_service" - -module OpenFoodNetwork - describe SubscriptionService do - describe "variant eligibility for subscription" do - let!(:shop) { create(:distributor_enterprise) } - let!(:producer) { create(:supplier_enterprise) } - let!(:product) { create(:product, supplier: producer) } - let!(:variant) { product.variants.first } - - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) } - let!(:subscription_line_item) do - create(:subscription_line_item, subscription: subscription, variant: variant) - end - - let(:current_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago, - orders_close_at: 1.week.from_now) - end - - let(:future_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now, - orders_close_at: 2.weeks.from_now) - end - - let(:past_order_cycle) do - create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago, - orders_close_at: 1.week.ago) - end - - let!(:order_cycle) { current_order_cycle } - - context "if the shop is the supplier for the product" do - let!(:producer) { shop } - - it "is eligible" do - expect(described_class.eligible_variants(shop)).to include(variant) - end - end - - context "if the supplier is permitted for the shop" do - let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) } - - it "is eligible" do - expect(described_class.eligible_variants(shop)).to include(variant) - end - end - - context "if the variant is involved in an exchange" do - let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - - context "if it is an incoming exchange where the shop is the receiver" do - let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } - - it "is not eligible" do - expect(described_class.eligible_variants(shop)).to_not include(variant) - end - end - - context "if it is an outgoing exchange where the shop is the receiver" do - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } - - context "if the order cycle is currently open" do - let!(:order_cycle) { current_order_cycle } - - it "is eligible" do - expect(described_class.eligible_variants(shop)).to include(variant) - end - end - - context "if the order cycle opens in the future" do - let!(:order_cycle) { future_order_cycle } - - it "is eligible" do - expect(described_class.eligible_variants(shop)).to include(variant) - end - end - - context "if the order cycle closed in the past" do - let!(:order_cycle) { past_order_cycle } - - it "is eligible" do - expect(described_class.eligible_variants(shop)).to include(variant) - end - end - end - end - - context "if the variant is unrelated" do - it "is not eligible" do - expect(described_class.eligible_variants(shop)).to_not include(variant) - end - end - end - - describe "checking if variant in open and upcoming order cycles" do - let!(:shop) { create(:enterprise) } - let!(:product) { create(:product) } - let!(:variant) { product.variants.first } - let!(:schedule) { create(:schedule) } - - context "if the variant is involved in an exchange" do - let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } - let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } - - context "if it is an incoming exchange where the shop is the receiver" do - let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } - - it "is is false" do - expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) - end - end - - context "if it is an outgoing exchange where the shop is the receiver" do - let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } - - it "is true" do - expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) - end - end - end - - context "if the variant is unrelated" do - it "is false" do - expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant) - end - end - end - end -end diff --git a/spec/services/subscription_variants_service_spec.rb b/spec/services/subscription_variants_service_spec.rb new file mode 100644 index 0000000000..31d0ff4ca7 --- /dev/null +++ b/spec/services/subscription_variants_service_spec.rb @@ -0,0 +1,130 @@ +require "spec_helper" + +describe SubscriptionVariantsService do + describe "variant eligibility for subscription" do + let!(:shop) { create(:distributor_enterprise) } + let!(:producer) { create(:supplier_enterprise) } + let!(:product) { create(:product, supplier: producer) } + let!(:variant) { product.variants.first } + + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + let!(:subscription) { create(:subscription, shop: shop, schedule: schedule) } + let!(:subscription_line_item) do + create(:subscription_line_item, subscription: subscription, variant: variant) + end + + let(:current_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.ago, + orders_close_at: 1.week.from_now) + end + + let(:future_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 1.week.from_now, + orders_close_at: 2.weeks.from_now) + end + + let(:past_order_cycle) do + create(:simple_order_cycle, coordinator: shop, orders_open_at: 2.weeks.ago, + orders_close_at: 1.week.ago) + end + + let!(:order_cycle) { current_order_cycle } + + context "if the shop is the supplier for the product" do + let!(:producer) { shop } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the supplier is permitted for the shop" do + let!(:enterprise_relationship) { create(:enterprise_relationship, child: shop, parent: product.supplier, permissions_list: [:add_to_order_cycle]) } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the variant is involved in an exchange" do + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + + context "if it is an incoming exchange where the shop is the receiver" do + let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + + it "is not eligible" do + expect(described_class.eligible_variants(shop)).to_not include(variant) + end + end + + context "if it is an outgoing exchange where the shop is the receiver" do + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + + context "if the order cycle is currently open" do + let!(:order_cycle) { current_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the order cycle opens in the future" do + let!(:order_cycle) { future_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + + context "if the order cycle closed in the past" do + let!(:order_cycle) { past_order_cycle } + + it "is eligible" do + expect(described_class.eligible_variants(shop)).to include(variant) + end + end + end + end + + context "if the variant is unrelated" do + it "is not eligible" do + expect(described_class.eligible_variants(shop)).to_not include(variant) + end + end + end + + describe "checking if variant in open and upcoming order cycles" do + let!(:shop) { create(:enterprise) } + let!(:product) { create(:product) } + let!(:variant) { product.variants.first } + let!(:schedule) { create(:schedule) } + + context "if the variant is involved in an exchange" do + let!(:order_cycle) { create(:simple_order_cycle, coordinator: shop) } + let!(:schedule) { create(:schedule, order_cycles: [order_cycle]) } + + context "if it is an incoming exchange where the shop is the receiver" do + let!(:incoming_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: true, variants: [variant]) } + + it "is is false" do + expect(described_class).not_to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + + context "if it is an outgoing exchange where the shop is the receiver" do + let!(:outgoing_exchange) { order_cycle.exchanges.create(sender: product.supplier, receiver: shop, incoming: false, variants: [variant]) } + + it "is true" do + expect(described_class).to be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + end + + context "if the variant is unrelated" do + it "is false" do + expect(described_class).to_not be_in_open_and_upcoming_order_cycles(shop, schedule, variant) + end + end + end +end From 9965e95c65a29c6989f1d9abb0e7693f404c7837 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 20 Jan 2019 23:46:56 +1100 Subject: [PATCH 22/46] Refactor methods in SubscriptionVariantsService --- app/services/subscription_variants_service.rb | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/app/services/subscription_variants_service.rb b/app/services/subscription_variants_service.rb index eb6dcf0cf2..855c200303 100644 --- a/app/services/subscription_variants_service.rb +++ b/app/services/subscription_variants_service.rb @@ -4,21 +4,11 @@ class SubscriptionVariantsService # - Variants of hub # - Variants that are in outgoing exchanges where the hub is receiver def self.eligible_variants(distributor) - permitted_order_cycle_enterprise_ids = EnterpriseRelationship.permitting(distributor) - .with_permission(:add_to_order_cycle).pluck(:parent_id) - permitted_producer_ids = Enterprise.is_primary_producer - .where('enterprises.id IN (?)', permitted_order_cycle_enterprise_ids).pluck(:id) - - outgoing_exchange_variant_ids = ExchangeVariant - .select("DISTINCT exchange_variants.variant_id") - .joins(:exchange) - .where(exchanges: { incoming: false, receiver_id: distributor.id }) - .pluck(:variant_id) - - variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids | [distributor.id]] - if outgoing_exchange_variant_ids.present? + variant_conditions = ["spree_products.supplier_id IN (?)", permitted_producer_ids(distributor)] + exchange_variant_ids = outgoing_exchange_variant_ids(distributor) + if exchange_variant_ids.present? variant_conditions[0] << " OR spree_variants.id IN (?)" - variant_conditions << outgoing_exchange_variant_ids + variant_conditions << exchange_variant_ids end Spree::Variant.joins(:product).where(is_master: false).where(*variant_conditions) @@ -31,4 +21,19 @@ class SubscriptionVariantsService scope = scope.where(schedules: { id: schedule }) scope.any? end + + def self.permitted_producer_ids(distributor) + other_permitted_producer_ids = EnterpriseRelationship.joins(:parent) + .permitting(distributor).with_permission(:add_to_order_cycle) + .merge(Enterprise.is_primary_producer) + .pluck(:parent_id) + + other_permitted_producer_ids | [distributor.id] + end + + def self.outgoing_exchange_variant_ids(distributor) + ExchangeVariant.select("DISTINCT exchange_variants.variant_id").joins(:exchange) + .where(exchanges: { incoming: false, receiver_id: distributor.id }) + .pluck(:variant_id) + end end From da4d6a092ac52b21fe7e95b83c201869952d8b97 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Mon, 21 Jan 2019 00:02:38 +1100 Subject: [PATCH 23/46] Add spec helper for choosing today from datepicker --- spec/features/admin/subscriptions_spec.rb | 4 +--- spec/spec_helper.rb | 1 + spec/support/features/datepicker_helper.rb | 9 +++++++++ 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 spec/support/features/datepicker_helper.rb diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index f51de02e5f..7626329ef2 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -454,9 +454,7 @@ feature 'Subscriptions' do select2_select payment_method.name, from: "payment_method_id" select2_select shipping_method.name, from: "shipping_method_id" find_field("begins_at").click - within(".ui-datepicker-calendar") do - find(".ui-datepicker-today").click - end + choose_today_from_datepicker click_button "Next" expect(page).to have_content "BILLING ADDRESS" diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 25dac67716..f404f4c95a 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -136,6 +136,7 @@ RSpec.configure do |config| config.extend Spree::Api::TestingSupport::Setup, :type => :controller config.include Spree::Api::TestingSupport::Helpers, :type => :controller config.include OpenFoodNetwork::ControllerHelper, :type => :controller + config.include Features::DatepickerHelper, type: :feature config.include OpenFoodNetwork::FeatureToggleHelper config.include OpenFoodNetwork::FiltersHelper config.include OpenFoodNetwork::EnterpriseGroupsHelper diff --git a/spec/support/features/datepicker_helper.rb b/spec/support/features/datepicker_helper.rb new file mode 100644 index 0000000000..df966cfa05 --- /dev/null +++ b/spec/support/features/datepicker_helper.rb @@ -0,0 +1,9 @@ +module Features + module DatepickerHelper + def choose_today_from_datepicker + within(".ui-datepicker-calendar") do + find(".ui-datepicker-today").click + end + end + end +end From c33808e8f5659796339f8db0712e330ea112ada6 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Mon, 21 Jan 2019 00:14:12 +1100 Subject: [PATCH 24/46] Extract some actions in tests into methods --- spec/features/admin/subscriptions_spec.rb | 31 +++++++++++++++-------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index 7626329ef2..99f32162f6 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -449,27 +449,22 @@ feature 'Subscriptions' do end it "permit creating and editing of the subscription" do - select2_select customer.email, from: "customer_id" - select2_select schedule.name, from: "schedule_id" - select2_select payment_method.name, from: "payment_method_id" - select2_select shipping_method.name, from: "shipping_method_id" - find_field("begins_at").click - choose_today_from_datepicker + # Fill in other details + fill_in_subscription_basic_details click_button "Next" - expect(page).to have_content "BILLING ADDRESS" click_button "Next" # Add products expect(page).to have_content "NAME OR SKU" add_variant_to_subscription shop_variant, 3 - expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 1 + expect_not_in_open_or_upcoming_order_cycle_warning 1 add_variant_to_subscription permitted_supplier_variant, 4 - expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 2 + expect_not_in_open_or_upcoming_order_cycle_warning 2 add_variant_to_subscription incoming_exchange_variant, 5 - expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3 + expect_not_in_open_or_upcoming_order_cycle_warning 3 add_variant_to_subscription outgoing_exchange_variant, 6 - expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: 3 + expect_not_in_open_or_upcoming_order_cycle_warning 3 click_button "Next" # Submit form @@ -503,6 +498,20 @@ feature 'Subscriptions' do end end + def fill_in_subscription_basic_details + select2_select customer.email, from: "customer_id" + select2_select schedule.name, from: "schedule_id" + select2_select payment_method.name, from: "payment_method_id" + select2_select shipping_method.name, from: "shipping_method_id" + + find_field("begins_at").click + choose_today_from_datepicker + end + + def expect_not_in_open_or_upcoming_order_cycle_warning(count) + expect(page).to have_content variant_not_in_open_or_upcoming_order_cycle_warning, count: count + end + def add_variant_to_subscription(variant, quantity) row_count = all("#subscription-line-items .item").length variant_name = variant.full_name.present? ? "#{variant.name} - #{variant.full_name}" : variant.name From 4a58aedf09b898a175132386bf82af1f22fbc6fb Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Tue, 22 Jan 2019 16:09:20 +0800 Subject: [PATCH 25/46] Use clearer translation for "No Upcoming OC" --- config/locales/en.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 366a481bac..f0c72f289a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1103,7 +1103,7 @@ en: details: Details address: Address products: Products - no_open_or_upcoming_order_cycle: "No Upcoming OC" + no_open_or_upcoming_order_cycle: "No Upcoming Order Cycle" product_already_in_order: This product has already been added to the order. Please edit the quantity directly. orders: number: Number From 5992bba97b80afa7264763c5ba5a92d647ed2c4f Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sat, 26 Jan 2019 03:29:15 +0800 Subject: [PATCH 26/46] Import color variables in new page SCSS partials "admin/all.css" which imports these SCSS partials already imports the color variables in "admin/variables", so actually there should be no need to import the variables again. However, "application.css" calls "require_tree", which means asset precompilation through Sprockets would attempt to compile each of the SCSS partials individually. When compiled individually, the color variables are not available to these partials. This is a quick solution to allow precompilation of "application.css" to complete. --- app/assets/stylesheets/admin/pages/subscription_form.css.scss | 2 ++ app/assets/stylesheets/admin/pages/subscription_review.css.scss | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/assets/stylesheets/admin/pages/subscription_form.css.scss b/app/assets/stylesheets/admin/pages/subscription_form.css.scss index 02c9fd783e..b892c4f17e 100644 --- a/app/assets/stylesheets/admin/pages/subscription_form.css.scss +++ b/app/assets/stylesheets/admin/pages/subscription_form.css.scss @@ -1,3 +1,5 @@ +@import '../variables'; + .admin-subscription-form-subscription-line-items { .not-in-open-and-upcoming-order-cycles-warning { color: $warning-red; diff --git a/app/assets/stylesheets/admin/pages/subscription_review.css.scss b/app/assets/stylesheets/admin/pages/subscription_review.css.scss index ba3280aa2b..76008afc0f 100644 --- a/app/assets/stylesheets/admin/pages/subscription_review.css.scss +++ b/app/assets/stylesheets/admin/pages/subscription_review.css.scss @@ -1,3 +1,5 @@ +@import '../variables'; + .admin-subscription-review-subscription-line-items { .not-in-open-and-upcoming-order-cycles-warning { color: $warning-red; From 777712a0e9c77ede475e1df06242f6f540f68694 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 04:52:13 +0800 Subject: [PATCH 27/46] Fix race condition in enterprise image feature specs --- spec/features/admin/enterprises/images_spec.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/spec/features/admin/enterprises/images_spec.rb b/spec/features/admin/enterprises/images_spec.rb index b2f1cb0787..ea903b4151 100644 --- a/spec/features/admin/enterprises/images_spec.rb +++ b/spec/features/admin/enterprises/images_spec.rb @@ -35,8 +35,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__logo-field-group" do - expect(page).to have_selector(".image-field-group__preview-image") - expect(html).to include("logo-white.png") + expect(page).to have_selector(".image-field-group__preview-image[src*='logo-white.png']") end # Replacing image @@ -47,8 +46,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__logo-field-group" do - expect(page).to have_selector(".image-field-group__preview-image") - expect(html).to include("logo-black.png") + expect(page).to have_selector(".image-field-group__preview-image[src*='logo-black.png']") end # Removing image @@ -73,8 +71,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect(page).to have_selector(".image-field-group__preview-image") - expect(html).to include("logo-white.jpg") + expect(page).to have_selector(".image-field-group__preview-image[src*='logo-white.jpg']") end # Replacing image @@ -85,8 +82,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect(page).to have_selector(".image-field-group__preview-image") - expect(html).to include("logo-black.jpg") + expect(page).to have_selector(".image-field-group__preview-image[src*='logo-black.jpg']") end # Removing image From a3c808a172cf68642d62975dc91a6e867192ff84 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 05:02:57 +0800 Subject: [PATCH 28/46] Refactor checking of preview image path in specs --- spec/features/admin/enterprises/images_spec.rb | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/enterprises/images_spec.rb b/spec/features/admin/enterprises/images_spec.rb index ea903b4151..31c0eca1ee 100644 --- a/spec/features/admin/enterprises/images_spec.rb +++ b/spec/features/admin/enterprises/images_spec.rb @@ -35,7 +35,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__logo-field-group" do - expect(page).to have_selector(".image-field-group__preview-image[src*='logo-white.png']") + expect_preview_image "logo-white.png" end # Replacing image @@ -46,7 +46,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__logo-field-group" do - expect(page).to have_selector(".image-field-group__preview-image[src*='logo-black.png']") + expect_preview_image "logo-black.png" end # Removing image @@ -71,7 +71,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect(page).to have_selector(".image-field-group__preview-image[src*='logo-white.jpg']") + expect_preview_image "logo-white.jpg" end # Replacing image @@ -82,7 +82,7 @@ feature "Managing enterprise images" do go_to_images within ".page-admin-enterprises-form__promo-image-field-group" do - expect(page).to have_selector(".image-field-group__preview-image[src*='logo-black.jpg']") + expect_preview_image "logo-black.jpg" end # Removing image @@ -99,4 +99,8 @@ feature "Managing enterprise images" do end end end + + def expect_preview_image(file_name) + expect(page).to have_selector(".image-field-group__preview-image[src*='#{file_name}']") + end end From 65438e2619f301eed03696160154a82ff3f85590 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 05:04:18 +0800 Subject: [PATCH 29/46] Refactor checking no preview image in specs --- spec/features/admin/enterprises/images_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/enterprises/images_spec.rb b/spec/features/admin/enterprises/images_spec.rb index 31c0eca1ee..110b8c2664 100644 --- a/spec/features/admin/enterprises/images_spec.rb +++ b/spec/features/admin/enterprises/images_spec.rb @@ -58,7 +58,7 @@ feature "Managing enterprise images" do expect(page).to have_content("Logo removed successfully") within ".page-admin-enterprises-form__logo-field-group" do - expect(page).to have_no_selector(".image-field-group__preview-image") + expect_no_preview_image end end @@ -94,7 +94,7 @@ feature "Managing enterprise images" do expect(page).to have_content("Promo image removed successfully") within ".page-admin-enterprises-form__promo-image-field-group" do - expect(page).to have_no_selector(".image-field-group__preview-image") + expect_no_preview_image end end end @@ -103,4 +103,8 @@ feature "Managing enterprise images" do def expect_preview_image(file_name) expect(page).to have_selector(".image-field-group__preview-image[src*='#{file_name}']") end + + def expect_no_preview_image + expect(page).to have_no_selector(".image-field-group__preview-image") + end end From d4d0489328c01aa54532a5efa5dd464aae5dcd21 Mon Sep 17 00:00:00 2001 From: Jefferson Faseler Date: Wed, 30 Jan 2019 14:25:04 -0500 Subject: [PATCH 30/46] Update name spaces for rake tasks to shorter 'ofn'. --- .travis.yml | 2 +- GETTING_STARTED.md | 2 +- config/schedule.rb | 6 +++--- lib/tasks/billing.rake | 2 +- lib/tasks/cache.rake | 2 +- lib/tasks/data.rake | 2 +- lib/tasks/dev.rake | 2 +- lib/tasks/enterprises.rake | 2 +- lib/tasks/specs.rake | 2 +- script/setup | 2 +- 10 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index db818fad23..9fd9d0934c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -39,7 +39,7 @@ before_script: script: - 'if [ "$KARMA" = "true" ]; then bundle exec rake karma:run; else echo "Skipping karma run"; fi' - - 'if [ "$RSPEC_ENGINES" = "true" ]; then bundle exec rake openfoodnetwork:specs:engines:rspec; else echo "Skipping RSpec run in engines"; fi' + - 'if [ "$RSPEC_ENGINES" = "true" ]; then bundle exec rake ofn:specs:engines:rspec; else echo "Skipping RSpec run in engines"; fi' - "bundle exec rake 'knapsack:rspec[--format progress --tag ~performance]'" after_success: diff --git a/GETTING_STARTED.md b/GETTING_STARTED.md index 5a2e7eea54..c30a70c13d 100644 --- a/GETTING_STARTED.md +++ b/GETTING_STARTED.md @@ -75,7 +75,7 @@ Then the main application tests can be run with: The tests of all custom engines can be run with: - bundle exec rake openfoodnetwork:specs:engines:rspec + bundle exec rake ofn:specs:engines:rspec Note: If your OS is not explicitly supported in the setup guides then not all tests may pass. However, you may still be able to develop. Get in touch with the [#dev][slack-dev] channel on Slack to troubleshoot issues and determine if they will preclude you from contributing to OFN. diff --git a/config/schedule.rb b/config/schedule.rb index edec8351ac..4bb20df545 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -13,7 +13,7 @@ job_type :enqueue_job, "cd :path; :environment_variable=:environment bundle exe every 1.hour do - rake 'openfoodnetwork:cache:check_products_integrity' + rake 'ofn:cache:check_products_integrity' end every 1.day, at: '12:05am' do @@ -35,10 +35,10 @@ every 5.minutes do end every 1.day, at: '1:00am' do - rake 'openfoodnetwork:billing:update_account_invoices' + rake 'ofn:billing:update_account_invoices' end # On the 2nd of every month at 1:30am every '30 1 2 * *' do - rake 'openfoodnetwork:billing:finalize_account_invoices' + rake 'ofn:billing:finalize_account_invoices' end diff --git a/lib/tasks/billing.rake b/lib/tasks/billing.rake index 6bfd0a53e7..4c279bc41e 100644 --- a/lib/tasks/billing.rake +++ b/lib/tasks/billing.rake @@ -1,4 +1,4 @@ -namespace :openfoodnetwork do +namespace :ofn do namespace :billing do desc 'Update enterprise user invoices' task update_account_invoices: :environment do diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake index 64a5d3909a..a88e745153 100644 --- a/lib/tasks/cache.rake +++ b/lib/tasks/cache.rake @@ -1,6 +1,6 @@ require 'open_food_network/products_cache_integrity_checker' -namespace :openfoodnetwork do +namespace :ofn do namespace :cache do desc 'check the integrity of the products cache' task :check_products_integrity => :environment do diff --git a/lib/tasks/data.rake b/lib/tasks/data.rake index 643886943c..9ae0984e3a 100644 --- a/lib/tasks/data.rake +++ b/lib/tasks/data.rake @@ -1,4 +1,4 @@ -namespace :openfoodnetwork do +namespace :ofn do namespace :data do desc "Adding relationships based on recent order cycles" task :create_order_cycle_relationships => :environment do diff --git a/lib/tasks/dev.rake b/lib/tasks/dev.rake index 19cc5b7a4b..2eff925395 100644 --- a/lib/tasks/dev.rake +++ b/lib/tasks/dev.rake @@ -1,4 +1,4 @@ -namespace :openfoodnetwork do +namespace :ofn do namespace :dev do desc 'load sample data' task load_sample_data: :environment do diff --git a/lib/tasks/enterprises.rake b/lib/tasks/enterprises.rake index 62d56fe8a5..547e179c2a 100644 --- a/lib/tasks/enterprises.rake +++ b/lib/tasks/enterprises.rake @@ -1,6 +1,6 @@ require 'csv' -namespace :openfoodnetwork do +namespace :ofn do namespace :dev do desc 'export enterprises to CSV' task :export_enterprises => :environment do diff --git a/lib/tasks/specs.rake b/lib/tasks/specs.rake index dddb5ffb54..bd6708dcf6 100644 --- a/lib/tasks/specs.rake +++ b/lib/tasks/specs.rake @@ -1,4 +1,4 @@ -namespace :openfoodnetwork do +namespace :ofn do namespace :specs do namespace :engines do def detect_engine_paths diff --git a/script/setup b/script/setup index 430c07b911..53332df685 100755 --- a/script/setup +++ b/script/setup @@ -52,7 +52,7 @@ printf '\n\n' | bundle exec rake db:setup db:test:prepare printf '\n' # Load some default data for your environment -bundle exec rake openfoodnetwork:dev:load_sample_data +bundle exec rake ofn:dev:load_sample_data printf '\n' printf "${YELLOW}WELCOME TO OPEN FOOD NETWORK!\n" From 4bff256f6d06f85bd382ecf342490d7a46ecace3 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sat, 2 Feb 2019 01:43:48 +0800 Subject: [PATCH 31/46] Match date format in spec with import date filter This was failing if the current day of month only has one digit. The test could not find the date in the import date filter. Before this commit, the resulting string in the test replaced " " with " ", so "February 2" was being changed to "February 2". The import date filter however uses two spaces in this case, even if the browser shows only one (HTML inline text renders two spaces as just one). --- spec/features/admin/product_import_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index d196a686e0..c44ac21a26 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -164,7 +164,7 @@ feature "Product Import", js: true do end expect(page).to have_selector 'div#s2id_import_date_filter' - import_time = carrots.import_date.to_date.to_formatted_s(:long).gsub(' ', ' ') + import_time = carrots.import_date.to_date.to_formatted_s(:long) select2_select import_time, from: "import_date_filter" expect(page).to have_field "product_name", with: carrots.name From 310486273963b7e0e551102fbd9c4a462fa40645 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sat, 2 Feb 2019 03:42:44 +0800 Subject: [PATCH 32/46] Fix shop accidentally becoming order coordinator The original setup of the order calls: create(:simple_order_cycle) Which picks an arbitrary distributor as coordinator: coordinator { Enterprise.is_distributor.first || ... } There is a chance that any of the existing distributor enterprises becomes coordinator, causing a test to intermittently fail. This commit makes the relationship between the reference enterprises and the order consistent. --- spec/controllers/api/orders_controller_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/controllers/api/orders_controller_spec.rb b/spec/controllers/api/orders_controller_spec.rb index 760094570d..134c0cfc5e 100644 --- a/spec/controllers/api/orders_controller_spec.rb +++ b/spec/controllers/api/orders_controller_spec.rb @@ -11,7 +11,9 @@ module Api let!(:distributor2) { create(:distributor_enterprise) } let!(:supplier) { create(:supplier_enterprise) } let!(:coordinator) { create(:distributor_enterprise) } + let!(:coordinator2) { create(:distributor_enterprise) } let!(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order_cycle2) { create(:simple_order_cycle, coordinator: coordinator2) } let!(:order1) do create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor, billing_address: create(:address) ) @@ -24,7 +26,9 @@ module Api create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.zone.now, distributor: distributor, billing_address: create(:address) ) end - let!(:order4) { create(:completed_order_with_fees) } + let!(:order4) do + create(:completed_order_with_fees, order_cycle: order_cycle2, distributor: distributor2) + end let!(:order5) { create(:order, state: 'cart', completed_at: nil) } let!(:line_item1) do create(:line_item, order: order1, @@ -148,7 +152,7 @@ module Api get :index, per_page: 15, page: 1 pagination_data = { - 'results' => 3, + 'results' => 2, 'pages' => 1, 'page' => 1, 'per_page' => 15 From 2c3eeec2b9e4e2b54788f4f73f236c7053b337bf Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 1 Feb 2019 21:44:30 +0000 Subject: [PATCH 33/46] Update cancan permissions for second iteration of bulk invoices --- app/controllers/spree/admin/invoices_controller.rb | 1 + app/models/spree/ability_decorator.rb | 3 ++- spec/controllers/spree/admin/invoices_controller_spec.rb | 5 +++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/app/controllers/spree/admin/invoices_controller.rb b/app/controllers/spree/admin/invoices_controller.rb index 230d01322b..710fda1a3a 100644 --- a/app/controllers/spree/admin/invoices_controller.rb +++ b/app/controllers/spree/admin/invoices_controller.rb @@ -2,6 +2,7 @@ module Spree module Admin class InvoicesController < Spree::Admin::BaseController respond_to :json + authorize_resource class: false def create invoice_service = BulkInvoiceService.new diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 2c68470840..f6f37560ce 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -210,9 +210,10 @@ class AbilityDecorator # during the order creation process from the admin backend order.distributor.nil? || user.enterprises.include?(order.distributor) || order.order_cycle.andand.coordinated_by?(user) end - can [:admin, :bulk_management, :managed, :bulk_invoice], Spree::Order do + can [:admin, :bulk_management, :managed], Spree::Order do user.admin? || user.enterprises.any?(&:is_distributor) end + can [:admin, :create, :show, :poll], :invoice can [:admin, :visible], Enterprise can [:admin, :index, :create, :update, :destroy], :line_item can [:admin, :index, :create], Spree::LineItem diff --git a/spec/controllers/spree/admin/invoices_controller_spec.rb b/spec/controllers/spree/admin/invoices_controller_spec.rb index 1eff65ba2c..9a7453fb70 100644 --- a/spec/controllers/spree/admin/invoices_controller_spec.rb +++ b/spec/controllers/spree/admin/invoices_controller_spec.rb @@ -2,10 +2,11 @@ require 'spec_helper' describe Spree::Admin::InvoicesController, type: :controller do let(:order) { create(:order_with_totals_and_distribution) } - let(:user) { create(:admin_user) } + let(:enterprise_user) { create(:user) } + let!(:enterprise) { create(:enterprise, owner: enterprise_user) } before do - allow(controller).to receive(:spree_current_user) { user } + allow(controller).to receive(:spree_current_user) { enterprise_user } end describe "#create" do From 3eedee313e352fb2a677f72ddab23faf91a07c49 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 21:21:30 +1100 Subject: [PATCH 34/46] Wait for datepicker to associate and open before selecting date --- spec/features/admin/bulk_order_management_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index d261405a4d..063500800f 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -743,6 +743,9 @@ feature %q{ current_month = Time.zone.today.strftime("%B") target_month = date.strftime("%B") + # Wait for datepicker to open and be associated to the datepicker trigger. + expect(page).to have_selector("#ui-datepicker-div") + find('#ui-datepicker-div .ui-datepicker-header .ui-datepicker-prev').click if current_month != target_month find('#ui-datepicker-div .ui-datepicker-calendar .ui-state-default', text: date.strftime("%e").to_s.strip, exact_text: true).click end From bb51f7e36b4d8264bb5b1a53a147cc776b9a79f2 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 21:24:11 +1100 Subject: [PATCH 35/46] Improve sync between keyword filter and selecting all --- spec/features/admin/bulk_order_management_spec.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index 063500800f..70caed9190 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -567,6 +567,7 @@ feature %q{ context "when a filter has been applied" do it "only toggles checkboxes which are in filteredLineItems" do fill_in "quick_search", with: o1.number + expect(page).to have_no_selector "tr#li_#{li2.id}" check "toggle_bulk" fill_in "quick_search", with: '' expect(find("tr#li_#{li1.id} input[type='checkbox'][name='bulk']").checked?).to be true @@ -577,11 +578,13 @@ feature %q{ it "only applies the delete action to filteredLineItems" do check "toggle_bulk" fill_in "quick_search", with: o1.number + expect(page).to have_no_selector "tr#li_#{li2.id}" find("div#bulk-actions-dropdown").click find("div#bulk-actions-dropdown div.menu_item", :text => "Delete Selected" ).click - fill_in "quick_search", with: '' expect(page).to have_no_selector "tr#li_#{li1.id}" + fill_in "quick_search", with: '' expect(page).to have_selector "tr#li_#{li2.id}" + expect(page).to have_no_selector "tr#li_#{li1.id}" end end end From fdede83086f35b1ec9677f14e9fc5c5e1160ad14 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 19:24:22 +0800 Subject: [PATCH 36/46] Support selecting date in next months This was causing failures when selecting tomorrow when running tests on the last day of the month. --- .../admin/bulk_order_management_spec.rb | 6 ++--- spec/support/features/datepicker_helper.rb | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index 70caed9190..b04acb29e0 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -743,13 +743,11 @@ feature %q{ end def select_date(date) - current_month = Time.zone.today.strftime("%B") - target_month = date.strftime("%B") - # Wait for datepicker to open and be associated to the datepicker trigger. expect(page).to have_selector("#ui-datepicker-div") - find('#ui-datepicker-div .ui-datepicker-header .ui-datepicker-prev').click if current_month != target_month + navigate_datepicker_to_month date + find('#ui-datepicker-div .ui-datepicker-calendar .ui-state-default', text: date.strftime("%e").to_s.strip, exact_text: true).click end end diff --git a/spec/support/features/datepicker_helper.rb b/spec/support/features/datepicker_helper.rb index df966cfa05..60221c6635 100644 --- a/spec/support/features/datepicker_helper.rb +++ b/spec/support/features/datepicker_helper.rb @@ -5,5 +5,29 @@ module Features find(".ui-datepicker-today").click end end + + def navigate_datepicker_to_month(date, reference_date = Time.zone.today) + month_and_year = date.strftime("%B %Y") + + until datepicker_month_and_year == month_and_year.upcase + if date < reference_date + navigate_datepicker_to_previous_month + elsif date > reference_date + navigate_datepicker_to_next_month + end + end + end + + def navigate_datepicker_to_previous_month + find('#ui-datepicker-div .ui-datepicker-header .ui-datepicker-prev').click + end + + def navigate_datepicker_to_next_month + find('#ui-datepicker-div .ui-datepicker-header .ui-datepicker-next').click + end + + def datepicker_month_and_year + find("#ui-datepicker-div .ui-datepicker-title").text + end end end From ed5856afa4ed5385007d1d95b5937fb75c3bccfd Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Thu, 31 Jan 2019 23:12:03 +0800 Subject: [PATCH 37/46] Compile edit link with higher priority --- app/views/spree/admin/orders/bulk_management.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index 50aa10634f..fd7cd1dc37 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -181,6 +181,6 @@ %input.show-dirty{ :type => 'text', :name => 'price', :id => 'price', :ng => { value: 'line_item.price * line_item.quantity | currency:""', readonly: "true", class: '{"update-error": line_item.errors.price}' } } %span.error{ ng: { bind: 'line_item.errors.price' } } %td.actions - %a{ href: "/admin/orders/{{line_item.order.number}}/edit", :class => "edit-order icon-edit no-text", 'confirm-link-click' => 'confirmRefresh()' } + %a{ ng: { href: "/admin/orders/{{line_item.order.number}}/edit" }, :class => "edit-order icon-edit no-text", 'confirm-link-click' => 'confirmRefresh()' } %td.actions %a{ 'ng-click' => "deleteLineItem(line_item)", :class => "delete-line-item icon-trash no-text" } From 35ecbe1584ca59d36259153b809da5035099aa2f Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 1 Feb 2019 00:17:53 +0800 Subject: [PATCH 38/46] Compile row ID with higher priority --- app/views/spree/admin/orders/bulk_management.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index fd7cd1dc37..32d164adc1 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -157,7 +157,7 @@ = t("admin.orders.bulk_management.ask") %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" } - %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:sorting.predicate:sorting.reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", :id => "li_{{line_item.id}}" } + %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:sorting.predicate:sorting.reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", ng: { attr: { id: "li_{{line_item.id}}" } } } %td.bulk %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked', 'ignore-dirty' => true } %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }} From 428e58f8f7dc047a74bf7444aa9d0f3c49bdfaa0 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 1 Feb 2019 00:36:32 +0800 Subject: [PATCH 39/46] Remove unused have_no_selector argument in feature test --- spec/features/admin/bulk_order_management_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index b04acb29e0..26dbc2aab0 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -439,7 +439,7 @@ feature %q{ expect(page).to have_selector "tr#li_#{li3.id}" fill_in "quick_search", :with => o1.email expect(page).to have_selector "tr#li_#{li1.id}" - expect(page).to have_no_selector "tr#li_#{li2.id}", true + expect(page).to have_no_selector "tr#li_#{li2.id}" expect(page).to have_no_selector "tr#li_#{li3.id}" end end From 00304286478d82ed9a10cff68f04dcd95dda39b5 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Fri, 1 Feb 2019 01:45:27 +0800 Subject: [PATCH 40/46] Do not show table until first time dereferencing is done --- app/views/spree/admin/orders/bulk_management.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/orders/bulk_management.html.haml b/app/views/spree/admin/orders/bulk_management.html.haml index 32d164adc1..950ec2a45a 100644 --- a/app/views/spree/admin/orders/bulk_management.html.haml +++ b/app/views/spree/admin/orders/bulk_management.html.haml @@ -112,7 +112,7 @@ .margin-bottom-50{ 'ng-hide' => 'RequestMonitor.loading || filteredLineItems.length == 0' } %form{ name: 'bulk_order_form' } - %table.index#listing_orders.bulk{ :class => "sixteen columns alpha" } + %table.index#listing_orders.bulk{ :class => "sixteen columns alpha", ng: { show: "initialized" } } %thead %tr{ ng: { controller: "ColumnsCtrl" } } %th.bulk From 93aabf674152d157415a5a3ac867f81bceccb86c Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 7 Feb 2019 19:26:00 +0000 Subject: [PATCH 41/46] Update all locales with the latest Transifex translations --- config/locales/fr_BE.yml | 2 +- config/locales/it.yml | 45 ++++++++++++++++++++++++++++++++++++++++ config/locales/nb.yml | 2 ++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/config/locales/fr_BE.yml b/config/locales/fr_BE.yml index c5cfc0054f..94291967f1 100644 --- a/config/locales/fr_BE.yml +++ b/config/locales/fr_BE.yml @@ -1105,7 +1105,7 @@ fr_BE: footer_legal_call: "Lire nos" footer_legal_tos: "Termes et conditions" footer_legal_visit: "Nous trouver sur" - footer_legal_text_html: "Open Food Network est une plateforme logicielle open source, libre et gratuite. Nos données sont protégées sous licence %{content_license} et notre code sous %{code_license}." + footer_legal_text_html: "Open Food Network est une plateforme logicielle open source et libre. Nos données sont protégées sous licence %{content_license} et notre code sous %{code_license}." footer_data_text_with_privacy_policy_html: "Nous prenons soin de vos données. Voir notre %{privacy_policy} et %{cookies_policy}." footer_data_text_without_privacy_policy_html: "Nous prenons soin de vos données. Voir notre %{cookies_policy}." footer_data_privacy_policy: "politique de confidentialité" diff --git a/config/locales/it.yml b/config/locales/it.yml index ddb3185edc..5a123f6480 100644 --- a/config/locales/it.yml +++ b/config/locales/it.yml @@ -602,6 +602,7 @@ it: desc_long_placeholder: Racconta di te ai consumatori. Questa informazione comparirà nel tuo profilo pubblico. business_details: abn: ABN + abn_placeholder: es. 99 123 456 789 acn: ACN acn_placeholder: es. 123 456 789 display_invoice_logo: Mostra logo nelle fatture @@ -1131,6 +1132,7 @@ it: total_excl_tax: "Totale (Tasse escl.):" total_incl_tax: "Totale (Tasse incl.):" abn: "CF:" + acn: "ACN:" invoice_issued_on: "Fattura emessa il" order_number: "Numero fattura:" date_of_transaction: "Data della transazione:" @@ -1149,7 +1151,9 @@ it: menu_5_title: "About" menu_5_url: "http://www.openfoodnetwork.org/" menu_6_title: "Connetti" + menu_6_url: "https://openfoodnetwork.org/au/connect/" menu_7_title: "Impara" + menu_7_url: "https://openfoodnetwork.org/au/learn/" logo: "Logo (640x130)" logo_mobile: "Mobile logo (75x26)" logo_mobile_svg: "Mobile logo (SVG)" @@ -2216,6 +2220,9 @@ it: validation_msg_tax: "^Categoria d'imposta richiesta" validation_msg_tax_category_cant_be_blank: "^Categoria di tassa non può essere vuoto" validation_msg_is_associated_with_an_exising_customer: "è associato con un cliente esistente" + content_configuration_pricing_table: "(TODO: tabella dei prezzi)" + content_configuration_case_studies: "(TODO: Casi studio)" + content_configuration_detail: "(TODO: Dettaglio)" enterprise_name_error: "è già stato utilizzato. Se questo è il nome della tua azienda e vorresti reclamarne la proprietà, o se vuoi contattare questa azienda, puoi contattare l'attuale referente di questo profilo a %{email}." enterprise_owner_error: "^%{email}non può gestire altre aziende (il limite è %{enterprise_limit})." enterprise_role_uniqueness_error: "^Questo ruolo è già presente." @@ -2253,6 +2260,7 @@ it: back_to_orders_list: "Indietro alla lista delle gentili richieste" no_orders_found: "Nessuna gentile richiesta trovata" order_information: "Informazioni Gentile Richiesta" + date_completed: "Data di completamento" amount: "Quantità" state_names: ready: Pronto @@ -2275,6 +2283,7 @@ it: choose: Scegli resolve_errors: 'Per favore risolvi i seguenti errori:' more_items: "+ %{count} ancora" + default_card_updated: Carta predefinita aggiornata admin: enterprise_limit_reached: "Hai raggiunto il limite standard di aziende per account. Scrivi a %{contact_email} se hai bisogno di aumentarlo." modals: @@ -2368,14 +2377,18 @@ it: non_producer_example: es. Botteghe, Food Coop, GAS enterprise_status: status_title: "%{name} è impostato e pronto a partire!" + severity: Gravità description: Descrizione resolve: Risolvi new_tag_rule_dialog: select_rule_type: "Seleziona un tipo di regola:" orders: index: + per_page: "%{results} per pagina" view_file: Vedi file compiling_invoices: Compilazione fatture + bulk_invoice_created: Fattura all'ingrosso creata + bulk_invoice_failed: Creazione fattura all'ingrosso fallita please_wait: Si prega di attendere che il PDF sia pronto prima di chiudere questo modale resend_user_email_confirmation: resend: "Invia nuovamente" @@ -2394,6 +2407,7 @@ it: 'yes': "A richiesta" variant_overrides: on_demand: + use_producer_settings: "Usa le impostazioni dello stock del produttore" 'yes': "Sì" 'no': "No" inventory_products: "Inventario Prodotti" @@ -2401,6 +2415,9 @@ it: new_products: "Nuovi Prodotti" reset_stock_levels: Resetta le quantità disponibili alla quantità predefinita changes_to: Cambia in + one_override: una si è sovrascritta + overrides: sovrascrive + remain_unsaved: restano da memorizzare no_changes_to_save: Nessuna modifica da salvare. no_authorisation: "Non abbiamo l'autorizzazione per salvare queste modifiche. " some_trouble: "Abbiamo avuto problemi durante il salvataggio: %{errors}" @@ -2449,16 +2466,23 @@ it: admin: orders: index: + listing_orders: "Listino Ordini" new_order: "Nuovo ordine" + capture: "Cattura" ship: "Spedizione" edit: "Modifica" + note: "Nota" + first: "Primo" + last: "Ultimo" previous: "Precedente" next: "Prossimo" loading: "Caricamento" no_orders_found: "Nessuna gentile richiesta trovata" results_found: "%{number} Risultati trovati." + viewing: "Guardando da %{start} a %{end}" print_invoices: "Stampa fatture" invoice: + issued_on: Emesso il tax_invoice: FATTURA DELLE TASSE code: Codice from: Da @@ -2490,12 +2514,18 @@ it: stripe_disabled_msg: I pagamenti Stripe sono stati disabilitati dall'amministratore di sistema. request_failed_msg: Spiacenti, qualcosa è andato storto mentre cercavamo di verificare i dettagli dell'account con Stripe... account_missing_msg: Non esiste un account Stripe per questa azienda. + connect_one: Connetti One access_revoked_msg: L'accesso a questo account Stripe è stato revocato, per favore ricollega il tuo account. status: Stato connected: Connesso account_id: Account ID business_name: Ragione sociale charges_enabled: Cambi Consentiti + payments: + source_forms: + stripe: + error_saving_payment: Errore memorizzando il pagamento + submitting_payment: Eseguendo il pagamento products: new: title: 'Nuovo prodotto' @@ -2514,7 +2544,9 @@ it: inherits_properties?: Eredita proprietà? available_on: Disponibile il av_on: "Disp. il" + import_date: "Data di importazione" products_variant: + variant_has_n_overrides: "Questa variante ha %{n} sostituzione/i" new_variant: "Nuova variante" product_name: Nome Prodotto primary_taxon_form: @@ -2523,14 +2555,19 @@ it: group_buy: "Acquisto di gruppo?" display_as: display_as: Visualizza come + reports: + table: + select_and_search: "Seleziona filtri e clicca su %{option} per accedere al tuo dato" users: index: + listing_users: "Elenco Utenti" new_user: "Nuovo utente" user: "Utente" enterprise_limit: "Limite azienda" search: "Cerca" email: "Email" edit: + editing_user: "Modificando Utente" back_to_users_list: "Torna all'elenco degli utenti" general_settings: "Impostazioni generali" form: @@ -2548,6 +2585,7 @@ it: edit: legal_settings: "Impostazioni Legali" cookies_consent_banner_toggle: "Mostra banner di consenso per i cookie" + privacy_policy_url: "Privacy Policy URL" enterprises_require_tos: "Le aziende devono accettare i Termini di Servizio" cookies_policy_matomo_section: "Visualizza la sezione di Matomo nella pagina della cookie policy" cookies_policy_ga_section: "Visualizza la sezione di Google Analytics nella pagina della cookie policy" @@ -2556,7 +2594,12 @@ it: payment: stripe: choose_one: Scegli uno + enter_new_card: Inserire dettagli per una nuova carta + used_saved_card: "usare la carta salvata" + or_enter_new_card: "Oppure, inserire dettagli di una nuova carta" + remember_this_card: Ricordare questa Carta? date_picker: + format: '%Y-%m-%d' js_format: 'aa-mm-gg' inventory: Inventario orders: @@ -2567,6 +2610,7 @@ it: order_mailer: invoice_email: hi: "Ciao %{name}" + invoice_attached_text: 'Aggiunge una fattura per il tuo recente ordine di ' order_state: address: indirizzo adjustments: aggiustamenti @@ -2659,5 +2703,6 @@ it: authorised_shops: Negozi autorizzati authorised_shops: shop_name: "Nome del negozio" + allow_charges?: "Consentire ricarichi" localized_number: invalid_format: 'Formato non valido: inserire un numero.' diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 9a19f953d9..8490c91c8f 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -1027,6 +1027,7 @@ nb: this_is_an_estimate: | De viste prisene er bare et estimat og beregnet på det tidspunktet abonnementet endres. Hvis du endrer priser eller avgifter, vil ordrer bli oppdatert, men abonnementet vil fortsatt vise de gamle verdiene. + not_in_open_and_upcoming_order_cycles_warning: "Det er ingen åpne eller kommende bestillingsrunder for dette produktet." details: details: Detaljer invalid_error: Oops! Vennligst fyll inn alle obligatoriske felter... @@ -1041,6 +1042,7 @@ nb: details: Detaljer address: Adresse products: Produkter + no_open_or_upcoming_order_cycle: "Ingen kommende bestillingsrunde" product_already_in_order: Dette produktet er allerede lagt til i bestillingen. Vennligst rediger mengden direkte. orders: number: Antall From 8ff5f9055b70de7ae229f2588e194001ee6c8ff5 Mon Sep 17 00:00:00 2001 From: Danni M Date: Fri, 8 Feb 2019 16:58:32 +1100 Subject: [PATCH 42/46] Update issue templates I've created 3 types of templates based on those in the wiki and the default (bug) issue template. These can be edited or additional ones added at your leisure. I also added the OFN software instance and version to the bugs template --- .github/ISSUE_TEMPLATE/bug_report.md | 61 ++++++++++++++++++++++ .github/ISSUE_TEMPLATE/feature-template.md | 35 +++++++++++++ .github/ISSUE_TEMPLATE/story-template.md | 18 +++++++ 3 files changed, 114 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature-template.md create mode 100644 .github/ISSUE_TEMPLATE/story-template.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000000..76ce58fccf --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,61 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: '' +assignees: '' + +--- + +## Description + + + +## Expected Behavior + + + +## Actual Behaviour + + + +## Steps to Reproduce + + + +1. +2. +3. +4. + +## Animated Gif/Screenshot + + + +## Context + + + +## Severity + + +## Your Environment + + +* Version used: +* Browser name and version: +* Operating System and version (desktop or mobile): +* OFN Platform instance where you discovered the bug, and which version of the software they are using. + +## Possible Fix + diff --git a/.github/ISSUE_TEMPLATE/feature-template.md b/.github/ISSUE_TEMPLATE/feature-template.md new file mode 100644 index 0000000000..9fb6cc1741 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature-template.md @@ -0,0 +1,35 @@ +--- +name: Feature template +about: Create feature epics that detail the larger feature or functionality to be + delivered. +title: '' +labels: '' +assignees: '' + +--- + +## What is the problem we are solving + + +## Success factors = expected outcome + + +## Useful information for inception + + +## Link to the "Product Development - Backlog" item in Discourse + + Add a custom footer + Pages 70 +Home +Development environment setup + +macOS (Sierra, HighSierra and Mojave) +OS X (El Capitan) +OS X (Mavericks) +Ubuntu +On Heroku +Rubocop +General guidelines + +Spree Commerce customisation diff --git a/.github/ISSUE_TEMPLATE/story-template.md b/.github/ISSUE_TEMPLATE/story-template.md new file mode 100644 index 0000000000..48a1c33493 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/story-template.md @@ -0,0 +1,18 @@ +--- +name: Story template +about: Create stories that are small chunks of work that devs will pick up and deliver +title: '' +labels: '' +assignees: '' + +--- + +**## Description** + + +**## Acceptance Criteria** + From f143540d0c6db23cce47bbe4ac9449e6dadf3de8 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sat, 9 Feb 2019 22:38:26 +0800 Subject: [PATCH 43/46] Do not expect modal open when checking spinner gone The element referenced in the following might no longer be visible: within "div.reveal-modal" --- spec/features/admin/bulk_product_update_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 96e3792163..1d9e2fb7b2 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -762,9 +762,9 @@ feature %q{ # Shows spinner whilst loading expect(page).to have_css "img.spinner", visible: true - expect(page).to have_no_css "img.spinner", visible: true end + expect(page).to have_no_css "img.spinner", visible: true expect(page).to have_no_selector "div.reveal-modal" within "table#listing_products tr#p_#{product.id}" do From e14c60c1c17e2b246e7c762a661bebfca06f179f Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 10 Feb 2019 18:59:36 +0800 Subject: [PATCH 44/46] Add RSpec matchers for flash messages --- .../matchers/flash_message_matchers.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 spec/support/matchers/flash_message_matchers.rb diff --git a/spec/support/matchers/flash_message_matchers.rb b/spec/support/matchers/flash_message_matchers.rb new file mode 100644 index 0000000000..b781fc1811 --- /dev/null +++ b/spec/support/matchers/flash_message_matchers.rb @@ -0,0 +1,31 @@ +RSpec::Matchers.define :have_flash_message do |message| + match do |node| + @message, @node = message, node + + # Ignore leading and trailing whitespace. Later versions of Capybara have :exact_text option. + # The :exact option is not supported in has_selector?. + message_substring_regex = substring_match_regex(message) + node.has_selector?(".flash", text: message_substring_regex, visible: false) + end + + failure_message do |actual| + "expected to find flash message ##{@message}" + end + + match_when_negated do |node| + @message, @node = message, node + + # Ignore leading and trailing whitespace. Later versions of Capybara have :exact_text option. + # The :exact option is not supported in has_selector?. + message_substring_regex = substring_match_regex(message) + node.has_no_selector?(".flash", text: message_substring_regex, visible: false) + end + + failure_message_when_negated do |actual| + "expected not to find flash message ##{@message}" + end + + def substring_match_regex(text) + /\A\s*#{Regexp.escape(text)}\s*\Z/ + end +end From 175264f41f8ffea35d5c41afa3680809ecbe3004 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 10 Feb 2019 19:03:02 +0800 Subject: [PATCH 45/46] Use flash matcher in shipping method feature specs --- spec/features/admin/shipping_methods_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index f6d4935a55..f8763a3c6b 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -32,7 +32,8 @@ feature 'shipping methods' do click_button 'Create' # Then the shipping method should have its distributor set - flash_message.should == 'Shipping method "Carrier Pidgeon" has been successfully created!' + message = "Shipping method \"Carrier Pidgeon\" has been successfully created!" + expect(page).to have_flash_message message sm = Spree::ShippingMethod.last sm.name.should == 'Carrier Pidgeon' @@ -100,7 +101,9 @@ feature 'shipping methods' do click_button 'Create' - flash_message.should == 'Shipping method "Teleport" has been successfully created!' + message = "Shipping method \"Teleport\" has been successfully created!" + expect(page).to have_flash_message message + expect(first('tags-input .tag-list ti-tag-item')).to have_content "local" shipping_method = Spree::ShippingMethod.find_by_name('Teleport') From 0032a237a284e05b11e921d41892db625dd77602 Mon Sep 17 00:00:00 2001 From: Kristina Lim Date: Sun, 10 Feb 2019 22:06:35 +0800 Subject: [PATCH 46/46] Wait for button to disappear before checking flash --- spec/features/admin/shipping_methods_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/shipping_methods_spec.rb b/spec/features/admin/shipping_methods_spec.rb index f8763a3c6b..bb65e70255 100644 --- a/spec/features/admin/shipping_methods_spec.rb +++ b/spec/features/admin/shipping_methods_spec.rb @@ -29,7 +29,9 @@ feature 'shipping methods' do fill_in 'shipping_method_name', with: 'Carrier Pidgeon' check "shipping_method_distributor_ids_#{d1.id}" check "shipping_method_distributor_ids_#{d2.id}" - click_button 'Create' + click_button I18n.t("actions.create") + + expect(page).to have_no_button I18n.t("actions.create") # Then the shipping method should have its distributor set message = "Shipping method \"Carrier Pidgeon\" has been successfully created!" @@ -99,8 +101,9 @@ feature 'shipping methods' do expect(page).to have_css '.tag-item' end - click_button 'Create' + click_button I18n.t("actions.create") + expect(page).to have_no_button I18n.t("actions.create") message = "Shipping method \"Teleport\" has been successfully created!" expect(page).to have_flash_message message