diff --git a/Gemfile b/Gemfile index bdfd0b3410..c9d34cd0bb 100644 --- a/Gemfile +++ b/Gemfile @@ -99,6 +99,7 @@ group :test, :development do gem 'rspec-retry' gem 'json_spec' gem 'unicorn-rails' + gem 'atomic' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 2157de2b77..54f7daf530 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -155,6 +155,7 @@ GEM angularjs-rails (1.2.13) ansi (1.4.2) arel (3.0.3) + atomic (1.1.99) awesome_nested_set (2.1.5) activerecord (>= 3.0.0) awesome_print (1.0.2) @@ -547,6 +548,7 @@ DEPENDENCIES angular-rails-templates (~> 0.2.0) angularjs-file-upload-rails (~> 1.1.0) angularjs-rails (= 1.2.13) + atomic awesome_print aws-sdk blockenspiel diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb index 986dc07d22..63eb63f0b3 100644 --- a/app/models/enterprise_group.rb +++ b/app/models/enterprise_group.rb @@ -1,6 +1,8 @@ require 'open_food_network/locking' +require 'open_food_network/permalink_generator' class EnterpriseGroup < ActiveRecord::Base + include PermalinkGenerator acts_as_list has_and_belongs_to_many :enterprises @@ -81,25 +83,10 @@ class EnterpriseGroup < ActiveRecord::Base private - def self.find_available_value(existing, requested) - return requested unless existing.include?(requested) - used_indices = existing.map do |p| - p.slice!(/^#{requested}/) - p.match(/^\d+$/).to_s.to_i - end - options = (1..used_indices.length + 1).to_a - used_indices - requested + options.first.to_s - end - - def find_available_permalink(requested) - existing = self.class.where(id: !id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) - self.class.find_available_value(existing, requested) - end - def sanitize_permalink if permalink.blank? || permalink_changed? requested = permalink.presence || permalink_was.presence || name.presence || 'group' - self.permalink = find_available_permalink(requested.parameterize) + self.permalink = create_unique_permalink(requested.parameterize) end end end diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 2cf13baf84..53f960ba63 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -1,4 +1,7 @@ +require 'open_food_network/permalink_generator' + Spree::Product.class_eval do + include PermalinkGenerator # We have an after_destroy callback on Spree::ProductOptionType. However, if we # don't specify dependent => destroy on this association, it is not called. See: # https://github.com/rails/rails/issues/7618 @@ -19,6 +22,8 @@ Spree::Product.class_eval do attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value attr_accessible :inherits_properties, :sku + before_validation :sanitize_permalink + # validates_presence_of :variants, unless: :new_record?, message: "Product must have at least one variant" validates_presence_of :supplier validates :primary_taxon, presence: { message: "^Product Category can't be blank" } @@ -239,4 +244,11 @@ Spree::Product.class_eval do raise end end + + def sanitize_permalink + if permalink.blank? || permalink_changed? + requested = permalink.presence || permalink_was.presence || name.presence || 'product' + self.permalink = create_unique_permalink(requested.parameterize) + end + end end diff --git a/config/environments/test.rb b/config/environments/test.rb index 46d5dcf7c6..67131b6489 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -41,6 +41,11 @@ Openfoodnetwork::Application.configure do # Print deprecation notices to the stderr config.active_support.deprecation = :stderr config.action_mailer.default_url_options = { :host => "test.host" } + + # To block requests before running the database cleaner + require 'open_food_network/rack_request_blocker' + # Make sure the middleware is inserted first in middleware chain + config.middleware.insert_before('ActionDispatch::Static', 'RackRequestBlocker') end # Allows us to use _url helpers in Rspec diff --git a/lib/open_food_network/permalink_generator.rb b/lib/open_food_network/permalink_generator.rb new file mode 100644 index 0000000000..99d923fd1e --- /dev/null +++ b/lib/open_food_network/permalink_generator.rb @@ -0,0 +1,22 @@ +module PermalinkGenerator + def self.included(base) + base.extend(ClassMethods) + end + + module ClassMethods + def find_available_value(existing, requested) + return requested unless existing.include?(requested) + used_indices = existing.map do |p| + p.slice!(/^#{requested}/) + p.match(/^\d+$/).to_s.to_i + end + options = (1..used_indices.length + 1).to_a - used_indices + requested + options.first.to_s + end + end + + def create_unique_permalink(requested) + existing = self.class.where('id != ?', id).where("permalink LIKE ?", "#{requested}%").pluck(:permalink) + self.class.find_available_value(existing, requested) + end +end diff --git a/lib/open_food_network/rack_request_blocker.rb b/lib/open_food_network/rack_request_blocker.rb new file mode 100644 index 0000000000..71f1c450e2 --- /dev/null +++ b/lib/open_food_network/rack_request_blocker.rb @@ -0,0 +1,77 @@ +# Copied from http://blog.salsify.com/engineering/tearing-capybara-ajax-tests +# https://gist.github.com/jturkel/9317269/raw/ff7838684370fd8a468ffe1e5ce1f3e46ba39951/rack_request_blocker.rb + +require 'atomic' + +# Rack middleware that keeps track of the number of active requests and can block new requests. +class RackRequestBlocker + + @@num_active_requests = Atomic.new(0) + @@block_requests = Atomic.new(false) + + # Returns the number of requests the server is currently processing. + def self.num_active_requests + @@num_active_requests.value + end + + # Prevents the server from accepting new requests. Any new requests will return an HTTP + # 503 status. + def self.block_requests! + @@block_requests.value = true + end + + # Allows the server to accept requests again. + def self.allow_requests! + @@block_requests.value = false + end + + def initialize(app) + @app = app + end + + def call(env) + increment_active_requests + if block_requests? + block_request(env) + else + @app.call(env) + end + ensure + decrement_active_requests + end + + def self.wait_for_requests_complete + self.block_requests! + max_wait_time = 30 + polling_interval = 0.01 + wait_until = Time.now + max_wait_time.seconds + while true + return if self.num_active_requests == 0 + if Time.now > wait_until + raise "Failed waiting for completing requests, #{self.num_active_requests} running." + else + sleep(polling_interval) + end + end + ensure + self.allow_requests! + end + + private + + def block_requests? + @@block_requests.value + end + + def block_request(env) + [503, {}, []] + end + + def increment_active_requests + @@num_active_requests.update { |v| v + 1 } + end + + def decrement_active_requests + @@num_active_requests.update { |v| v - 1 } + end +end diff --git a/script/ci/includes.sh b/script/ci/includes.sh index a0599504c2..619d03153e 100644 --- a/script/ci/includes.sh +++ b/script/ci/includes.sh @@ -5,18 +5,33 @@ function load_environment { fi } +function master_merged { + if [[ `git tag -l "$BUILDKITE_BRANCH"` != '' ]]; then + echo "'$BUILDKITE_BRANCH' is a tag." + if [[ `git tag -l --contains origin/master "$BUILDKITE_BRANCH"` != '' ]]; then + echo "This tag contains the current master." + return 0 + else + echo "This tag does not contain the current master." + return 1 + fi + fi + if [[ `git branch -r --merged origin/$BUILDKITE_BRANCH` == *origin/master* ]]; then + echo "This branch already has the current master merged." + return 0 + fi + return 1 +} + function exit_unless_master_merged { - if [[ `git branch -a --merged origin/$BUILDKITE_BRANCH` != *origin/master* ]]; then + if ! master_merged; then echo "This branch does not have the current master merged. Please merge master and push again." exit 1 fi } function succeed_if_master_merged { - if [[ `git branch -a --merged origin/$BUILDKITE_BRANCH` == *origin/master* ]]; then - echo "This branch already has the current master merged." - exit 0 - fi + master_merged && exit 0 } function set_ofn_commit { diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index bf77ab415a..8d52f21b48 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -124,7 +124,7 @@ feature %q{ visit '/admin/orders/bulk_management' end - pending "displays an update button which submits pending changes" do + it "displays an update button which submits pending changes" do fill_in "quantity", :with => 2 page.should have_selector "input[name='quantity'].update-pending" page.should_not have_selector "input[name='quantity'].update-success" @@ -132,6 +132,8 @@ feature %q{ click_button "Update" page.should_not have_selector "input[name='quantity'].update-pending" page.should have_selector "input[name='quantity'].update-success" + page.should have_selector "input[name='final_weight_volume'].update-success", visible: false + page.should have_selector "input[name='price'].update-success", visible: false end end end @@ -288,9 +290,8 @@ feature %q{ visit '/admin/orders/bulk_management' end - pending "displays a select box for order cycles, which filters line items by the selected order cycle" do - order_cycle_names = ["All"] - OrderCycle.all.each{ |oc| order_cycle_names << oc.name } + it "displays a select box for order cycles, which filters line items by the selected order cycle" do + order_cycle_names = OrderCycle.pluck(:name).push "All" find("div.select2-container#s2id_order_cycle_filter").click order_cycle_names.each { |ocn| page.should have_selector "div.select2-drop-active ul.select2-results li", text: ocn } find("div.select2-container#s2id_order_cycle_filter").click @@ -440,13 +441,15 @@ feature %q{ page.should have_button "SAVE" end - pending "saves pendings changes when 'SAVE' button is clicked" do + it "saves pendings changes when 'SAVE' button is clicked" do within("tr#li_#{li2.id} td.quantity") do page.fill_in "quantity", :with => (li2.quantity + 1).to_s end fill_in "start_date_filter", :with => (Date.today - 9).strftime("%F %T") + page.should have_selector "input[name='quantity'].update-pending" click_button "SAVE" - page.should_not have_selector "input[name='quantity'].update-pending" + page.should have_no_selector "input.update-pending" + page.should have_selector "input[name='quantity'].update-success" within("tr#li_#{li2.id} td.quantity") do page.should have_field "quantity", :with => ( li2.quantity + 1 ).to_s end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 79d5c02d86..068a2184de 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -80,6 +80,11 @@ RSpec.configure do |config| config.before(:each, js: true) { DatabaseCleaner.strategy = :deletion, {except: ['spree_countries', 'spree_states']} } config.before(:each) { DatabaseCleaner.start } config.after(:each) { DatabaseCleaner.clean } + config.after(:each, js:true) do + Capybara.reset_sessions! + RackRequestBlocker.wait_for_requests_complete + DatabaseCleaner.clean + end # Geocoding config.before(:each) { Spree::Address.any_instance.stub(:geocode).and_return([1,1]) }