diff --git a/Gemfile b/Gemfile index a2fce9bad..c9fb824ba 100644 --- a/Gemfile +++ b/Gemfile @@ -55,7 +55,7 @@ group :test do gem 'email_spec' gem 'faker' gem 'fake_stripe' - gem 'webdrivers','~> 5.0', require: false + gem 'webdrivers','>= 5.2.0', require: false gem 'rspec-json_expectations' gem 'simplecov' gem 'timecop' diff --git a/Gemfile.lock b/Gemfile.lock index ea37211ef..c961ec826 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -65,7 +65,6 @@ GEM xpath (~> 3.2) celluloid (0.16.0) timers (~> 4.0.0) - childprocess (4.1.0) chronic (0.10.2) coderay (1.1.2) concurrent-ruby (1.1.5) @@ -171,7 +170,7 @@ GEM mime-types-data (~> 3.2015) mime-types-data (3.2019.0331) mini_mime (1.0.2) - mini_portile2 (2.8.1) + mini_portile2 (2.8.5) minitest (5.14.0) moneta (1.0.0) multi_json (1.13.1) @@ -179,8 +178,8 @@ GEM multipart-post (2.0.0) newrelic_rpm (5.0.0.342) nio4r (2.5.8) - nokogiri (1.14.2) - mini_portile2 (~> 2.8.0) + nokogiri (1.15.5) + mini_portile2 (~> 2.8.2) racc (~> 1.4) parallel (1.12.1) parser (3.2.2.0) @@ -200,7 +199,7 @@ GEM nio4r (~> 2.0) puma-heroku (2.0.0) puma (>= 5.0, < 6.0) - racc (1.6.2) + racc (1.7.3) rack (1.6.12) rack-protection (1.5.5) rack @@ -245,7 +244,7 @@ GEM responders (2.4.0) actionpack (>= 4.2.0, < 5.3) railties (>= 4.2.0, < 5.3) - rexml (3.2.5) + rexml (3.2.6) rspec (3.8.0) rspec-core (~> 3.8.0) rspec-expectations (~> 3.8.0) @@ -292,10 +291,10 @@ GEM tilt scout_apm (5.3.3) parser - selenium-webdriver (4.1.0) - childprocess (>= 0.5, < 5.0) + selenium-webdriver (4.9.0) rexml (~> 3.2, >= 3.2.5) - rubyzip (>= 1.2.2) + rubyzip (>= 1.2.2, < 3.0) + websocket (~> 1.0) simplecov (0.17.0) docile (~> 1.1) json (>= 1.8, < 3) @@ -336,15 +335,16 @@ GEM binding_of_caller (>= 0.7.2) railties (>= 4.0) sprockets-rails (>= 2.0, < 4.0) - webdrivers (5.0.0) + webdrivers (5.3.1) nokogiri (~> 1.6) rubyzip (>= 1.3.0) - selenium-webdriver (~> 4.0) + selenium-webdriver (~> 4.0, < 4.11) webmock (3.10.0) addressable (>= 2.3.6) crack (>= 0.3.2) hashdiff (>= 0.4.0, < 2.0.0) webrick (1.6.1) + websocket (1.2.10) websocket-driver (0.7.1) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.4) @@ -417,7 +417,7 @@ DEPENDENCIES uglifier vcr web-console (~> 2.0) - webdrivers (~> 5.0) + webdrivers (>= 5.2.0) webmock where-or will_paginate diff --git a/app/controllers/valid_vouchers_controller.rb b/app/controllers/valid_vouchers_controller.rb index 0ed335da2..ee22328ca 100644 --- a/app/controllers/valid_vouchers_controller.rb +++ b/app/controllers/valid_vouchers_controller.rb @@ -32,10 +32,7 @@ def edit def update @valid_voucher = ValidVoucher.find(params[:id]) args = params[:valid_voucher] - # max_sales_for_type if blank should be "infinity" - if args[:max_sales_for_type].blank? - args[:max_sales_for_type] = ValidVoucher::INFINITE - end + set_arg_defaults_if_blank!(args) if @valid_voucher.update_attributes(args) redirect_to edit_show_path(@valid_voucher.showdate.show), :notice => 'Update successful.' else @@ -53,9 +50,9 @@ def create return redirect_to(back, :alert => t('season_setup.must_select_vouchertypes')) if vt.blank? vouchertypes = Vouchertype.find vt showdates = Showdate.find sd - # max_sales_for_type if blank should be "infinity" - args[:max_sales_for_type] = ValidVoucher::INFINITE if args[:max_sales_for_type].blank? - + set_arg_defaults_if_blank!(args) + # min_sales_for_type if blank should default to 1 + args[:min_sales_per_txn] = 1 if args[:min_sales_per_txn].blank? # params[:before_or_after] is either '+1' or '-1' to possibly negate minutes_before_curtain # (so the value stored is "Minutes before", but may be negative to indicate "minutes after") args[:before_showtime] = (params[:minutes_before].to_i * params[:before_or_after].to_i).minutes @@ -82,4 +79,15 @@ def destroy end end + private + + def set_arg_defaults_if_blank!(args) + # this should be done with strong params + # supply defaults for min/max_sales_for_type and max_sales_per_txn if blank + args[:min_sales_per_txn] = 1 if args[:min_sales_per_txn].blank? + [:max_sales_for_type, :max_sales_per_txn].each do |attrib| + args[attrib] = ValidVoucher::INFINITE if args[attrib].blank? + end + end + end diff --git a/app/models/order.rb b/app/models/order.rb index 4263b7a1e..df8243e47 100644 --- a/app/models/order.rb +++ b/app/models/order.rb @@ -149,7 +149,8 @@ def add_tickets_from_params(valid_voucher_params, customer, promo_code: '', seat seats = seats2.slice!(0,qty) # is this order-placer allowed to exercise this redemption? redemption = vv.adjust_for_customer - if (vv.customer.try(:is_boxoffice) || redemption.max_sales_for_this_patron >= qty) + if (vv.customer.try(:is_boxoffice) || # boxoffice can do anything + redemption.max_sales_for_this_patron >= qty) # there's enough seats for this patron's limit add_tickets_without_capacity_checks(vv, qty, seats) else self.errors.add(:base, I18n.translate('store.errors.not_enough_seats', :count => saleable_seats)) diff --git a/app/models/valid_voucher.rb b/app/models/valid_voucher.rb index 12b654e59..8fbe06f74 100644 --- a/app/models/valid_voucher.rb +++ b/app/models/valid_voucher.rb @@ -10,10 +10,13 @@ class ValidVoucher < ActiveRecord::Base + # Capacity is infinite if it is left blank + INFINITE = 100_000 + class InvalidRedemptionError < RuntimeError ; end class InvalidProcessedByError < RuntimeError ; end - attr_accessible :showdate_id, :showdate, :vouchertype_id, :vouchertype, :promo_code, :start_sales, :end_sales, :max_sales_for_type + attr_accessible :showdate_id, :showdate, :vouchertype_id, :vouchertype, :promo_code, :start_sales, :end_sales, :max_sales_for_type, :min_sales_per_txn, :max_sales_per_txn # auxiliary attributes that aren't persisted attr_accessible :explanation, :visible, :supplied_promo_code, :customer, :max_sales_for_this_patron belongs_to :showdate @@ -22,14 +25,15 @@ class InvalidProcessedByError < RuntimeError ; end validates_associated :showdate, :if => lambda { |v| !(v.vouchertype.bundle?) } validates_associated :vouchertype validates_numericality_of :max_sales_for_type, :allow_nil => true, :greater_than_or_equal_to => 0 + validates_numericality_of :min_sales_per_txn, :greater_than_or_equal_to => 1, :less_than_or_equal_to => INFINITE, :message => "must be blank, or greater than or equal to 1" + validates_numericality_of :max_sales_per_txn, :greater_than_or_equal_to => 1, :less_than_or_equal_to => INFINITE, :message => "must be blank, or greater than or equal to 1" + validate :min_max_sales_constraints validates_presence_of :start_sales validates_presence_of :end_sales scope :for_ticket_products, -> { joins(:vouchertype).where('vouchertypes.category != ?', 'nonticket') } scope :sorted, -> { joins(:vouchertype).order('vouchertypes.display_order,vouchertypes.name') } - # Capacity is infinite if it is left blank - INFINITE = 100_000 def max_sales_for_type ; self[:max_sales_for_type] || INFINITE ; end def sales_unlimited? ; max_sales_for_type >= INFINITE ; end @@ -71,6 +75,36 @@ def max_sales_for_this_patron end end + # min and max dropdown menu options, plus an option to purchase zero tickets, + # that respects min and max sales per txn as well as max of remaining seats etc. + # Make the menu contain at most max choices. + def min_and_max_sales_for_this_txn(max_choices = INFINITE) + max_sales = [self.max_sales_for_this_patron, + self.max_sales_per_txn, + self.seats_of_type_remaining]. + min + min_sales = self.min_sales_per_txn + if (max_sales.zero? || # maybe no seats of this type remaining + max_sales < min_sales) # or just not enough + [0] + else # at this point, we know max_sales >= min_sales, and neither is zero + range = [max_sales - min_sales + 1, max_choices].min + [0] + (min_sales .. min_sales+range-1).to_a + end + end + + def display_min_and_max_sales_per_txn + if min_sales_per_txn == 1 + max_sales_per_txn == INFINITE ? '' : "(max #{max_sales_per_txn} per order)" + else # minimum order + case max_sales_per_txn + when min_sales_per_txn then "(#{min_sales_per_txn} per order)" + when INFINITE then "(#{min_sales_per_txn}+ per order)" + else "(#{min_sales_per_txn}-#{max_sales_per_txn} per order)" + end + end + end + private # A zero-price vouchertype that is marked as "available to public" @@ -81,6 +115,16 @@ def self_service_comps_must_have_promo_code end end + # Min/max sales per transaction cannot contradict min/max sales for type. + # This is checked *after* each attribute is individually range-checked + def min_max_sales_constraints + errors.add :min_sales_per_txn, "cannot be greater than max allowed sales of this type" if + min_sales_per_txn > max_sales_for_type + errors.add :min_sales_per_txn, "cannot be greater than maximum purchase per transaction" if + min_sales_per_txn > max_sales_per_txn + errors.empty? + end + # Vouchertype's valid date must not be later than valid_voucher start date # Vouchertype expiration date must not be earlier than valid_voucher end date def check_dates @@ -156,6 +200,7 @@ def adjust_for_capacity when INFINITE then "No performance-specific limit applies" else "#{max_sales_for_this_patron} remaining" end + self.explanation << " " << display_min_and_max_sales_per_txn self.visible = true end @@ -201,10 +246,10 @@ def self.bundles(seasons = [Time.this_season-1, Time.this_season+1]) def self.bundles_available_to(customer = Customer.walkup_customer, promo_code=nil) bundles = ValidVoucher. - where('? BETWEEN start_sales AND end_sales', Time.current). - includes(:vouchertype,:showdate).references(:vouchertypes). - where('vouchertypes.category' => 'bundle'). - order("season DESC,display_order,price DESC") + where('? BETWEEN start_sales AND end_sales', Time.current). + includes(:vouchertype,:showdate).references(:vouchertypes). + where('vouchertypes.category' => 'bundle'). + order("season DESC,display_order,price DESC") bundles = bundles.map do |b| b.customer = customer b.supplied_promo_code = promo_code diff --git a/app/views/shows/_showdate.html.haml b/app/views/shows/_showdate.html.haml index 0ac1252d7..620e59747 100644 --- a/app/views/shows/_showdate.html.haml +++ b/app/views/shows/_showdate.html.haml @@ -24,7 +24,8 @@ - nsales = showdate.sales_by_type(v.vouchertype_id) .col-3 = link_to v.vouchertype.name_with_price, {:controller => 'valid_vouchers', :action => 'edit', :id => v.id} - = "(#{h(v.promo_code)})" unless v.promo_code.blank? + = v.display_min_and_max_sales_per_txn + = "(#{v.promo_code})" unless v.promo_code.blank? .col-3= time_in_words_relative_to(v.start_sales,showdate.thedate) .col-3= showdate_time_limit_for(v, :end_sales) .col-3 diff --git a/app/views/store/_ticket_menus.html.haml b/app/views/store/_ticket_menus.html.haml index a4f622a01..d9bb031d7 100644 --- a/app/views/store/_ticket_menus.html.haml +++ b/app/views/store/_ticket_menus.html.haml @@ -33,7 +33,6 @@ - @store.valid_vouchers.each do |v| %div{:id => "vouchertype_#{v.vouchertype_id}",:class => ['form-group', 'form-row', (v.promo_code.blank? ? 'no-promo' : 'promo')]} - fieldname = "valid_voucher[#{v.id}]" - - max_sales = [30, v.max_sales_for_this_patron].min %label.col-form-label.text-right.col-sm-4{:for => "valid_voucher_#{v.id}"}= v.name_with_price - ticket_class = if v.vouchertype.reservable? then 'ticket' else '' end - if @gAdminDisplay @@ -42,9 +41,11 @@ .col-sm-7 %label.col-form-label.form-control-sm.alert-warning.s-explain= v.explanation - else - = select_tag(fieldname, options_for_select(0..max_sales), 'data-price' => v.price, 'data-zone' => v.zone_short_name, :class => "itemQty #{ticket_class} form-control form-control-sm col-sm-1") - - if max_sales.zero? - %span.text-info.col-sm-7.border.border-danger.s-explain= v.explanation + - num_allowed = v.min_and_max_sales_for_this_txn(max_choices = 20) + - if num_allowed.last.zero? + %span.text-info.col-sm-4.border.border-danger.s-explain No seats remaining for tickets of this type + - else + = select_tag(fieldname, options_for_select(num_allowed), 'data-price' => v.price, 'data-zone' => v.zone_short_name, :class => "itemQty #{ticket_class} form-control form-control-sm col-sm-1") = hidden_field_tag "price[#{v.id}]", v.price, {:id => "valid_voucher_#{v.id}_price"} - if ! @store.valid_vouchers.empty? && !@store.sd.stream? diff --git a/app/views/valid_vouchers/edit.html.haml b/app/views/valid_vouchers/edit.html.haml index 34e6f2a6b..91b222bac 100644 --- a/app/views/valid_vouchers/edit.html.haml +++ b/app/views/valid_vouchers/edit.html.haml @@ -22,7 +22,7 @@ .col-md-5.text-right = popup_help_for :valid_voucher_min_sales_per_txn %label.col-form-label{:for=>:valid_voucher_min_sales_per_txn} Min purchase per transaction - = number_field_tag 'valid_voucher_min_sales_per_txn', @valid_voucher.min_sales_per_txn, :name => 'valid_voucher[min_sales_per_txn]', :class => 'form-control col-md-2' + = number_field_tag 'valid_voucher_min_sales_per_txn', (@valid_voucher.min_sales_per_txn unless @valid_voucher.min_sales_per_txn == 1), :name => 'valid_voucher[min_sales_per_txn]', :class => 'form-control col-md-2' .form-group.form-row .col-md-5.text-right diff --git a/config/locales/en.popup_help.yml b/config/locales/en.popup_help.yml index 2aa311e62..ab6ab8dc6 100644 --- a/config/locales/en.popup_help.yml +++ b/config/locales/en.popup_help.yml @@ -366,6 +366,17 @@ en: Maximum number of bundles/subscriptions of this type that may be sold. Leave blank for unlimited. + valid_voucher_min_sales_per_txn: + + Minimum number of tickets of this type that can be purchased in one transaction. + For example, set it to 2 to offer a 2-ticket pack. Leave blank for 1. + + valid_voucher_max_sales_per_txn: + + Maximum number of tickets of this type that can be purchased in one transaction. + For example, you can limit sales of a promo or discount ticket to a maximum + per transaction. Leave blank for unlimited. + valid_voucher_promo_code: > If you enter a promo code here, customers (but not box office staff) diff --git a/config/locales/en.yml b/config/locales/en.yml index a2cef03ce..02de4eee2 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -1,4 +1,10 @@ en: + activerecord: + attributes: + valid_voucher: + min_sales_per_txn: "Minimum purchase per transaction" + max_sales_per_txn: "Maximum purchase per transaction" + reports: revenue_details: csv_error: > diff --git a/db/migrate/20230803004141_add_min_max_purchase_to_valid_voucher.rb b/db/migrate/20230803004141_add_min_max_purchase_to_valid_voucher.rb index 1859ab53c..a5b154b97 100644 --- a/db/migrate/20230803004141_add_min_max_purchase_to_valid_voucher.rb +++ b/db/migrate/20230803004141_add_min_max_purchase_to_valid_voucher.rb @@ -1,4 +1,8 @@ class AddMinMaxPurchaseToValidVoucher < ActiveRecord::Migration def change + change_table :valid_vouchers do |t| + t.integer :min_sales_per_txn, :default => 1, :allow_nil => false + t.integer :max_sales_per_txn, :default => ValidVoucher::INFINITE, :allow_nil => false + end end end diff --git a/db/migrate/20230925030503_adjust_infinity_in_max_sales_for_type.rb b/db/migrate/20230925030503_adjust_infinity_in_max_sales_for_type.rb new file mode 100644 index 000000000..fd871fddf --- /dev/null +++ b/db/migrate/20230925030503_adjust_infinity_in_max_sales_for_type.rb @@ -0,0 +1,10 @@ +class AdjustInfinityInMaxSalesForType < ActiveRecord::Migration + # previous code used '999' as an upper bound on max_sales_for_type that means "infinity". + # it should be 10_000 to be consistent with the value of ValidVoucher::INFINITE. + def change + ValidVoucher.where("max_sales_for_type=999"). + update_all(:max_sales_for_type => ValidVoucher::INFINITE) + ValidVoucher.where("max_sales_per_txn=999"). + update_all(:max_sales_per_txn => ValidVoucher::INFINITE) + end +end diff --git a/db/schema.rb b/db/schema.rb index fc8212787..dc62535de 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20230803004141) do +ActiveRecord::Schema.define(version: 20230925030503) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -320,8 +320,10 @@ t.datetime "start_sales" t.datetime "end_sales" t.integer "max_sales_for_type" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.integer "min_sales_per_txn", default: 1 + t.integer "max_sales_per_txn", default: 999 end add_index "valid_vouchers", ["showdate_id"], name: "index_valid_vouchers_on_showdate_id", using: :btree diff --git a/features/step_definitions/showdate_has_vouchers_steps.rb b/features/step_definitions/showdate_has_vouchers_steps.rb index 4ed10e051..74a572149 100644 --- a/features/step_definitions/showdate_has_vouchers_steps.rb +++ b/features/step_definitions/showdate_has_vouchers_steps.rb @@ -14,11 +14,11 @@ Given /^a show "(.*)" with the following tickets available:$/ do |show_name, tickets| tickets.hashes.each do |t| steps %Q{Given a show "#{show_name}" with #{t[:qty]} "#{t[:type]}" tickets for #{t[:price]} on "#{t[:showdate]}"} + @showdate = Showdate.find_by!(:thedate => Time.parse(t[:showdate])) # if sales cutoff time is specified, modify the valid-voucher once created if (cutoff = t[:sales_cutoff]) - sd = Showdate.find_by!(:thedate => Time.parse(t[:showdate])) - vv = ValidVoucher.find_by!(:showdate => sd, :vouchertype => Vouchertype.find_by!(:name => t[:type])) - vv.update_attributes(:end_sales => sd.thedate - cutoff.to_i.minutes) + vv = ValidVoucher.find_by!(:showdate => @showdate, :vouchertype => Vouchertype.find_by!(:name => t[:type])) + vv.update_attributes(:end_sales => @showdate.thedate - cutoff.to_i.minutes) end end end @@ -34,6 +34,12 @@ make_valid_tickets(@showdate, @vouchertype, n.to_i) end +Given /"(.*)" tickets for that performance must be purchased at least (\d+) and at most (\d+) at a time/ do |vtype,min,max| + expect(@showdate).to be_a_kind_of Showdate + vv = ValidVoucher.find_by!(:showdate => @showdate, :vouchertype => Vouchertype.find_by!(:name => vtype)) + vv.update_attributes!(:min_sales_per_txn => min.to_i, :max_sales_per_txn => max.to_i) +end + Given /^the "(.*)" tickets for "(.*)" require promo code "(.*)"$/ do |ticket_type,date,promo| vouchertype = Vouchertype.find_by_name! ticket_type showdate = Showdate.find_by_thedate! Time.zone.parse date diff --git a/features/store/min_and_max_sales_per_transaction.feature b/features/store/min_and_max_sales_per_transaction.feature new file mode 100644 index 000000000..4c92ac8ee --- /dev/null +++ b/features/store/min_and_max_sales_per_transaction.feature @@ -0,0 +1,31 @@ +Feature: enforce minimum and maximum sales per transaction for a particular voucher type + + As a boxoffice manager + So that I can offer Buy One Get One and similar promotions + I want to enforce both a minimum and maximum number of tickets that can be purchased in one sale + + Background: performance with minimum and maximum purchase limits on a promotional ticket + + Given a show "The Nerd" with 5 "General" tickets for $10.00 on "Oct 1, 2010, 7pm" + And "General" tickets for that performance must be purchased at least 3 and at most 4 at a time + + Scenario: cannot add fewer than minimum tickets per transaction to cart + + Given 1 "General" tickets have been sold for "Oct 1, 2010, 7pm" + When I go to the store page + Then the "General - $10.00" menu should have options: 0;3;4 + + Scenario: cannot add fewer than minimum tickets per transaction to cart, nor more than available + + Given 2 "General" tickets have been sold for "Oct 1, 2010, 7pm" + When I go to the store page + Then the "General - $10.00" menu should have options: 0;3 + + Scenario: when minimum purchase per txn exceeds number of tickets of that type remaining, should show as sold out + + Given 3 "General" tickets have been sold for "Oct 1, 2010, 7pm" + When I go to the store page + Then the "General - $10.00" menu should have options: 0 + + + diff --git a/features/support/env.rb b/features/support/env.rb index a1b479c34..ef3683e00 100644 --- a/features/support/env.rb +++ b/features/support/env.rb @@ -26,9 +26,10 @@ Capybara.default_selector = :css Capybara.server = :webrick Capybara.register_driver :selenium do |app| - Webdrivers::Chromedriver.required_version = '114.0.5735.90' + Webdrivers::Chromedriver.required_version = '119.0.6045.105' + webdriver_args = %w[--headless --no-sandbox --disable-gpu --window-size=1024,1024] options = Selenium::WebDriver::Chrome::Options.new( - args: %w[--headless --no-sandbox --disable-gpu --window-size=1024,1024] + args: webdriver_args ) # When an "unexpected" alert/confirm is displayed, accept it (ie user clicks OK). # Expected ones can be handled with accept_alert do...end or accept_confirm do...end diff --git a/features/support/webmock.rb b/features/support/webmock.rb index acf743dca..a9709c5e7 100644 --- a/features/support/webmock.rb +++ b/features/support/webmock.rb @@ -3,4 +3,8 @@ WebMock.disable_net_connect!( net_http_connect_on_start: true, # @see https://github.com/bblimke/webmock/blob/master/README.md#connecting-on-nethttpstart allow_localhost: true, - allow: "chromedriver.storage.googleapis.com") + # 186585553: When upgrade to Ruby3.0, stop requiring webdrivers gem, upgrade to Selenium 4.11+, + # and hopefully remove the whitelist URIs below + allow: ["googlechromelabs.github.io", 'edgedl.me.gvt1.com'] +) + diff --git a/spec/models/valid_voucher_spec.rb b/spec/models/valid_voucher_spec.rb index a055663f9..05d0735e0 100644 --- a/spec/models/valid_voucher_spec.rb +++ b/spec/models/valid_voucher_spec.rb @@ -10,6 +10,14 @@ end end + it "has a friendly error message for min/max_sales_per_txn" do + @v = build(:valid_voucher) + @v.min_sales_per_txn = @v.max_sales_per_txn = -1 + expect(@v).not_to be_valid + expect(@v.errors.full_messages).to include_match_for /^Minimum purchase per transaction must be blank, or greater than or equal to 1$/ + expect(@v.errors.full_messages).to include_match_for /^Maximum purchase per transaction must be blank, or greater than or equal to 1$/ + end + describe "seats remaining" do subject do ValidVoucher.new(