diff --git a/app/assets/javascripts/darkswarm/controllers/registration/registration_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/registration/registration_controller.js.coffee index 29a60c3f15..c935461d91 100644 --- a/app/assets/javascripts/darkswarm/controllers/registration/registration_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/registration/registration_controller.js.coffee @@ -5,7 +5,20 @@ Darkswarm.controller "RegistrationCtrl", ($scope, RegistrationService, Enterpris $scope.steps = ['details', 'contact', 'type', 'about', 'images', 'social'] - $scope.countries = availableCountries + # Filter countries without states since the form requires a state to be selected. + # Consider changing the form to require a state only if a country requires them (Spree option). + # Invalid countries still need to be filtered (better server-side). + $scope.countries = availableCountries.filter (country) -> + country.states.length > 0 + + $scope.countriesById = $scope.countries.reduce (obj, country) -> + obj[country.id] = country + obj + , {} + + $scope.setDefaultCountry = (id) -> + country = $scope.countriesById[id] + $scope.enterprise.country = country if country $scope.countryHasStates = -> $scope.enterprise.country.states.length > 0 diff --git a/app/assets/javascripts/darkswarm/filters/filter_products.js.coffee b/app/assets/javascripts/darkswarm/filters/filter_products.js.coffee index 18402edc02..3e9f0b4821 100644 --- a/app/assets/javascripts/darkswarm/filters/filter_products.js.coffee +++ b/app/assets/javascripts/darkswarm/filters/filter_products.js.coffee @@ -5,4 +5,4 @@ Darkswarm.filter 'products', (Matcher) -> return products if text == "" products.filter (product) => propertiesToMatch = [product.name, product.variant_names, product.supplier.name, product.primary_taxon.name] - Matcher.match propertiesToMatch, text + Matcher.matchBeginning propertiesToMatch, text diff --git a/app/assets/javascripts/darkswarm/services/matcher.js.coffee b/app/assets/javascripts/darkswarm/services/matcher.js.coffee index 9360afdd1f..db871f6a0d 100644 --- a/app/assets/javascripts/darkswarm/services/matcher.js.coffee +++ b/app/assets/javascripts/darkswarm/services/matcher.js.coffee @@ -1,7 +1,14 @@ Darkswarm.factory "Matcher", -> - # Match text fragment in an array of strings. new class Matcher + + # Match text fragment in an array of strings. match: (properties, text)-> properties.some (prop)-> prop ||= "" prop.toLowerCase().indexOf(text.toLowerCase()) != -1 + + # Return true if text occurs at the beginning of any word present in an array of strings + matchBeginning: (properties, text) -> + text = text.trim() + regexp = new RegExp("(?:^|[\\s-])#{text}", "i") + properties.some (prop) -> (prop || "").search(regexp) >= 0 diff --git a/app/assets/javascripts/darkswarm/services/products.js.coffee b/app/assets/javascripts/darkswarm/services/products.js.coffee index 63e015e5ba..6c6e222330 100644 --- a/app/assets/javascripts/darkswarm/services/products.js.coffee +++ b/app/assets/javascripts/darkswarm/services/products.js.coffee @@ -42,9 +42,10 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Pro registerVariants: -> for product in @products if product.variants + product.variant_names = "" product.variants = for variant in product.variants variant = Variants.register variant if product.name != variant.name_to_display - product.variant_names += variant.name_to_display + product.variant_names += variant.name_to_display + " " variant.product = product variant diff --git a/app/assets/stylesheets/darkswarm/producer_node.css.scss b/app/assets/stylesheets/darkswarm/producer_node.css.scss index 921d8df544..770a76564f 100644 --- a/app/assets/stylesheets/darkswarm/producer_node.css.scss +++ b/app/assets/stylesheets/darkswarm/producer_node.css.scss @@ -32,6 +32,11 @@ color: $clr-turquoise; } + .vertical-align-middle { + display: flex; + align-items: center; + } + a { &:hover, &:active, &:focus { color: $clr-turquoise-bright; diff --git a/app/views/producers/_skinny.html.haml b/app/views/producers/_skinny.html.haml index 311de9daf5..28fbd5439f 100644 --- a/app/views/producers/_skinny.html.haml +++ b/app/views/producers/_skinny.html.haml @@ -1,20 +1,24 @@ .row.active_table_row{"ng-click" => "toggle($event)", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}"} - .columns.small-12.medium-4.large-4.skinny-head + .columns.small-12.medium-8.large-8.skinny-head %span{"ng-if" => "::producer.is_distributor" } - %a.is_distributor{"ng-href" => "{{::producer.path}}" } - %i{ng: {class: "::producer.producer_icon_font"}} - %span.margin-top - %strong{"ng-bind" => "::producer.name"} + %a.is_distributor{"ng-href" => "{{::producer.path}}"} + .row.vertical-align-middle + .columns.small-2.medium-2.large-2 + %i{ng: {class: "::producer.producer_icon_font"}} + .columns.small-10.medium-10.large-10 + %span.margin-top + %strong{"ng-bind" => "::producer.name"} %span.producer-name{"ng-if" => "::!producer.is_distributor" } - %i{ng: {class: "::producer.producer_icon_font"}} - %span.margin-top - %strong{"ng-bind" => "::producer.name"} - - - .columns.small-6.medium-3.large-3 + .row.vertical-align-middle + .columns.small-2.medium-2.large-2 + %i{ng: {class: "::producer.producer_icon_font"}} + .columns.small-10.medium-10.large-10 + %span.margin-top + %strong{"ng-bind" => "::producer.name"} + .columns.small-6.medium-2.large-2 %span.margin-top{"ng-bind" => "::producer.address.city"} - .columns.small-4.medium-3.large-4 + .columns.small-4.medium-1.large-1 %span.margin-top{"ng-bind" => "::producer.address.state_name | uppercase"} - .columns.small-2.medium-2.large-1.text-right + .columns.small-2.medium-1.large-1.text-right %span.margin-top %i{"ng-class" => "{'ofn-i_005-caret-down' : !open(), 'ofn-i_006-caret-up' : open()}"} diff --git a/app/views/registration/steps/_details.html.haml b/app/views/registration/steps/_details.html.haml index dd3ee65085..4a01d92385 100644 --- a/app/views/registration/steps/_details.html.haml +++ b/app/views/registration/steps/_details.html.haml @@ -54,7 +54,7 @@ .small-12.medium-8.large-8.columns .field %label{ for: 'enterprise_country' } {{'registration_detail_country' | t}} - %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { model: 'enterprise.country', options: 'c as c.name for c in countries' } } + %select.chunky{ id: 'enterprise_country', name: 'country', required: true, ng: { init: "setDefaultCountry(#{Spree::Config[:default_country_id]})", model: 'enterprise.country', options: 'c as c.name for c in countries' } } %span.error{ ng: { show: "details.country.$error.required && submitted" } } {{'registration_detail_country_error' | t}} diff --git a/config/locales/en.yml b/config/locales/en.yml index 73173f9c2e..760785c858 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -13,6 +13,45 @@ # # # See http://community.openfoodnetwork.org/t/localisation-ofn-in-your-language/397 +# +# Changing this file +# ================== +# +# You are welcome to fix typos, add missing translations and remove unused ones. +# Here are some guidelines to make sure that this file is becoming more beautiful +# with every change we do. +# +# * Change text: No problem. Fix the typo. And please enclose the text in quotes +# to avoid any accidents. +# +# Example 1: "When you're using double quotes, they look like \"this\"" +# Example 2: "When you’re using double quotes, they look like “this”" +# +# The second example uses unicode to make it look prettier and avoid backslashes. +# +# * Add translations: Cool, every bit of text in the application should be here. +# If you add a translation for a view or mailer, please make use of the nested +# structure. Use the "lazy" lookup. See: http://guides.rubyonrails.org/i18n.html#looking-up-translations +# +# Avoid global keys. There are a lot already. And some are okay, for example +# "enterprises" should be the same everywhere on the page. But in doubt, +# create a new translation and give it a meaningful scope. +# +# Don't worry about duplication. We may use the same word in different contexts, +# but another language could use different words. So don't try to re-use +# translations between files. +# +# Don't move big parts around or rename scopes with a lot of entries without +# a really good reason. In some cases that causes a lot of translations in +# other languages to be lost. That causes more work for translators. +# +# * Remove translations: If you are sure that they are not used anywhere, +# please remove them. Be aware that some translations are looked up with +# variables. For example app/views/admin/contents/_fieldset.html.haml looks +# up labels for preferences. Unfortunately, they don't have a scope. +# +# * Participate in the community discussions: +# - https://community.openfoodnetwork.org/t/workflow-to-structure-translations/932 en: activerecord: diff --git a/spec/features/consumer/registration_spec.rb b/spec/features/consumer/registration_spec.rb index 4a7b815737..dbdff739d1 100644 --- a/spec/features/consumer/registration_spec.rb +++ b/spec/features/consumer/registration_spec.rb @@ -6,7 +6,20 @@ feature "Registration", js: true do describe "Registering a Profile" do let(:user) { create(:user, password: "password", password_confirmation: "password") } - before { Spree::Config.enterprises_require_tos = false } + + before do + Spree::Config.enterprises_require_tos = false + + albania = Spree::Country.create!({ name: "Albania", iso3: "ALB", iso: "AL", iso_name: "ALBANIA", numcode: "8" }, without_protection: true) + Spree::State.create!({ name: "Berat", abbr: "BRA", country: albania }, without_protection: true) + Spree::Country.create!({ name: "Chad", iso3: "TCD", iso: "TD", iso_name: "CHAD", numcode: "148" }, without_protection: true) + end + + after do + Spree::State.where(name: 'Berat').delete_all + Spree::Country.where(name: 'Albania').delete_all + Spree::Country.where(name: 'Chad').delete_all + end it "Allows a logged in user to register a profile" do visit registration_path @@ -35,7 +48,7 @@ feature "Registration", js: true do fill_in 'enterprise_address', with: '123 Abc Street' fill_in 'enterprise_city', with: 'Northcote' fill_in 'enterprise_zipcode', with: '3070' - select 'Australia', from: 'enterprise_country' + expect(page).to have_select('enterprise_country', options: %w(Albania Australia), selected: 'Australia') select 'VIC', from: 'enterprise_state' perform_and_ensure(:click_button, "Continue", lambda { page.has_content? 'Who is responsible for managing My Awesome Enterprise?' }) diff --git a/spec/javascripts/unit/darkswarm/services/matcher_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/matcher_spec.js.coffee new file mode 100644 index 0000000000..4da9cb506e --- /dev/null +++ b/spec/javascripts/unit/darkswarm/services/matcher_spec.js.coffee @@ -0,0 +1,54 @@ +describe 'Matcher service', -> + Matcher = null + + beforeEach -> + module 'Darkswarm' + inject ($injector)-> + Matcher = $injector.get("Matcher") + + describe '#match', -> + it "matches full word", -> + expect(Matcher.match ["product_name"], "product_name").toEqual true + + it "matches second word", -> + expect(Matcher.match ["product_name", 'variant'], "variant").toEqual true + + it "matches word with underscore", -> + expect(Matcher.match ["product_name"], "ct_na").toEqual true + + it "matches word if property has two words", -> + expect(Matcher.match ["product name"], "nam").toEqual true + + it "matches word with dash", -> + expect(Matcher.match ["product-name"], "ct-na").toEqual true + + it "finds in any part of properties", -> + expect(Matcher.match ["keyword"], "word").toEqual true + + it "finds beginning of property", -> + expect(Matcher.match ["keyword"], "key").toEqual true + + it "doesn't find non-sense or mistypes", -> + expect(Matcher.match ["keyword"], "keywrd").toEqual false + + describe '#matchBeginning', -> + it "matches full word", -> + expect(Matcher.matchBeginning ["product_name"], "product_name").toEqual true + + it "matches second word", -> + expect(Matcher.matchBeginning ["product_name", 'variant'], "variant").toEqual true + + it "matches word if property has two words", -> + expect(Matcher.matchBeginning ["product name"], "nam").toEqual true + + it "matches second part of word separated by dash", -> + expect(Matcher.matchBeginning ["product-name"], "name").toEqual true + + it "matches beginning of property", -> + expect(Matcher.matchBeginning ["keyword"], "key").toEqual true + + it "doesn't match in any part of property", -> + expect(Matcher.matchBeginning ["keyword"], "word").toEqual false + + it "doesn't match non-sense or mistypes", -> + expect(Matcher.matchBeginning ["keyword"], "keywrd").toEqual false diff --git a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee index c5f9d64f12..62eeaf823f 100644 --- a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee @@ -82,6 +82,12 @@ describe 'Products service', -> $httpBackend.flush() expect(Products.products[0].variants[0]).toBe Variants.variants[1] + it "stores variant names", -> + product.variants = [{id: 1, name_to_display: "one"}, {id: 2, name_to_display: "two"}] + $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.flush() + expect(Products.products[0].variant_names).toEqual "one two " + it "sets primaryImageOrMissing when no images are provided", -> $httpBackend.expectGET("/shop/products").respond([product]) $httpBackend.flush() diff --git a/spec/jobs/products_cache_integrity_checker_job_spec.rb b/spec/jobs/products_cache_integrity_checker_job_spec.rb index 1770f9bf84..b77baf8018 100644 --- a/spec/jobs/products_cache_integrity_checker_job_spec.rb +++ b/spec/jobs/products_cache_integrity_checker_job_spec.rb @@ -6,9 +6,10 @@ describe ProductsCacheIntegrityCheckerJob do let(:distributor) { create(:distributor_enterprise) } let(:order_cycle) { create(:simple_order_cycle) } let(:job) { ProductsCacheIntegrityCheckerJob.new distributor.id, order_cycle.id } + let(:cache_key) { "products-json-#{distributor.id}-#{order_cycle.id}" } before do - Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", "[1, 2, 3]\n" + Rails.cache.write(cache_key, "[1, 2, 3]\n") OpenFoodNetwork::ProductsRenderer.stub(:new) { double(:pr, products_json: "[1, 3]\n") } end @@ -18,7 +19,7 @@ describe ProductsCacheIntegrityCheckerJob do end it "deals with nil cached_json" do - Rails.cache.clear + Rails.cache.delete(cache_key) expect(Bugsnag).to receive(:notify) run_job job end diff --git a/spec/lib/open_food_network/cached_products_renderer_spec.rb b/spec/lib/open_food_network/cached_products_renderer_spec.rb index 6e096fc6d1..5346db610a 100644 --- a/spec/lib/open_food_network/cached_products_renderer_spec.rb +++ b/spec/lib/open_food_network/cached_products_renderer_spec.rb @@ -48,11 +48,12 @@ module OpenFoodNetwork end describe "when the products JSON is not cached" do - let(:cached_json) { Rails.cache.read "products-json-#{distributor.id}-#{order_cycle.id}" } - let(:cache_present) { Rails.cache.exist? "products-json-#{distributor.id}-#{order_cycle.id}" } + let(:cache_key) { "products-json-#{distributor.id}-#{order_cycle.id}" } + let(:cached_json) { Rails.cache.read(cache_key) } + let(:cache_present) { Rails.cache.exist?(cache_key) } before do - Rails.cache.clear + Rails.cache.delete(cache_key) cpr.stub(:uncached_products_json) { 'fresh products' } end diff --git a/spec/performance/injection_helper_spec.rb b/spec/performance/injection_helper_spec.rb index ef8d937ce1..7553836947 100644 --- a/spec/performance/injection_helper_spec.rb +++ b/spec/performance/injection_helper_spec.rb @@ -18,7 +18,7 @@ describe InjectionHelper, type: :helper, performance: true do results = [] 4.times do |i| ActiveRecord::Base.connection.query_cache.clear - Rails.cache.clear + Rails.cache.delete_matched('api\/cached_enterprise_serializer\/enterprises') result = Benchmark.measure { helper.inject_enterprises } results << result.total if i > 0 puts result diff --git a/spec/performance/shop_controller_spec.rb b/spec/performance/shop_controller_spec.rb index bfe49cd178..396abb574d 100644 --- a/spec/performance/shop_controller_spec.rb +++ b/spec/performance/shop_controller_spec.rb @@ -14,6 +14,12 @@ describe ShopController, type: :controller, performance: true do describe "fetching products" do let(:exchange) { order_cycle.exchanges.to_enterprises(d).outgoing.first } let(:image) { File.open(File.expand_path('../../../app/assets/images/logo-white.png', __FILE__)) } + let(:cache_key_patterns) do + [ + 'api\/taxon_serializer\/spree\/taxons', + 'enterprise' + ] + end before do 11.times do @@ -28,7 +34,7 @@ describe ShopController, type: :controller, performance: true do end it "returns products via json" do - results = multi_benchmark(3) do + results = multi_benchmark(3, cache_key_patterns: cache_key_patterns) do xhr :get, :products response.should be_success end diff --git a/spec/support/performance_helper.rb b/spec/support/performance_helper.rb index a2d4fe3630..767691fa06 100644 --- a/spec/support/performance_helper.rb +++ b/spec/support/performance_helper.rb @@ -1,9 +1,9 @@ module OpenFoodNetwork module PerformanceHelper - def multi_benchmark(num_samples) + def multi_benchmark(num_samples, cache_key_patterns: []) results = (0..num_samples).map do |i| ActiveRecord::Base.connection.query_cache.clear - Rails.cache.clear + delete_cache_keys(cache_key_patterns) result = Benchmark.measure { yield } @@ -16,5 +16,16 @@ module OpenFoodNetwork results end + + # Looks for matching keys and deletes them + # Blindly running `Rails.cache.clear` is harmful since it alters application + # state outside executing spec example + # + # @param cache_key_patterns [Array] + def delete_cache_keys(cache_key_patterns) + cache_key_patterns.each do |pattern| + Rails.cache.delete_matched(pattern) + end + end end end diff --git a/spec/support/request/authentication_workflow.rb b/spec/support/request/authentication_workflow.rb index 79b6d81452..61ba32e1b0 100644 --- a/spec/support/request/authentication_workflow.rb +++ b/spec/support/request/authentication_workflow.rb @@ -19,11 +19,6 @@ module AuthenticationWorkflow admin_user end - def stub_authorization! - before(:all) { Spree::Ability.register_ability(AuthorizationHelpers::Request::SuperAbility) } - after(:all) { Spree::Ability.remove_ability(AuthorizationHelpers::Request::SuperAbility) } - end - def login_to_admin_section admin_role = Spree::Role.find_or_create_by_name!('admin') admin_user = create(:user,