From f76e1259fcba6acaa483b347ab394bf262091fd6 Mon Sep 17 00:00:00 2001 From: Artur Beljajev Date: Sat, 3 Jun 2017 23:12:16 +0300 Subject: [PATCH] Consider nil in price expire time as effective #522 --- .../admin/billing/prices_controller.rb | 19 ++++-- app/models/billing/price.rb | 42 ++++++++---- .../concerns/billing/price/expirable.rb | 4 -- .../billing/prices/_search_form.html.erb | 5 +- spec/factories/billing/price.rb | 2 +- .../admin/billing/prices/edit_spec.rb | 2 +- .../admin/billing/prices/expire_spec.rb | 2 +- .../admin/billing/prices/list_spec.rb | 12 ++-- spec/models/billing/price_spec.rb | 65 ++++++++++++++----- .../concerns/billing/price/expirable_spec.rb | 14 ---- .../admin/billing/prices/expire_spec.rb | 4 +- 11 files changed, 106 insertions(+), 65 deletions(-) diff --git a/app/controllers/admin/billing/prices_controller.rb b/app/controllers/admin/billing/prices_controller.rb index 041f55b1e..8b79d6eb5 100644 --- a/app/controllers/admin/billing/prices_controller.rb +++ b/app/controllers/admin/billing/prices_controller.rb @@ -6,18 +6,19 @@ module Admin helper_method :zones helper_method :operation_categories helper_method :durations + helper_method :statuses def index @search = OpenStruct.new(search_params) - unless @search.validity - @search.validity = 'unexpired' + unless @search.status + @search.status = default_status end prices = ::Billing::Price.all - if @search.validity.present? - prices = ::Billing::Price.send(@search.validity) + if @search.status.present? + prices = ::Billing::Price.send(@search.status) end @q = prices.search(params[:q]) @@ -81,7 +82,7 @@ module Admin def search_params allowed_params = %i[ - validity + status ] params.fetch(:search, {}).permit(*allowed_params) end @@ -102,6 +103,14 @@ module Admin durations = ::Billing::Price::durations durations.collect { |duration| [duration.sub('mon', 'month'), duration] } end + + def statuses + ::Billing::Price.statuses + end + + def default_status + 'effective' + end end end end diff --git a/app/models/billing/price.rb b/app/models/billing/price.rb index e5cddd76c..fd8dc2275 100644 --- a/app/models/billing/price.rb +++ b/app/models/billing/price.rb @@ -12,6 +12,7 @@ module Billing validates :operation_category, inclusion: { in: Proc.new { |price| price.class.operation_categories } } validates :duration, inclusion: { in: Proc.new { |price| price.class.durations } } + alias_attribute :effect_time, :valid_from alias_attribute :expire_time, :valid_to monetize :price_cents, allow_nil: true, numericality: { greater_than_or_equal_to: 0 } after_initialize :init_values @@ -22,22 +23,37 @@ module Billing def self.durations [ - '3 mons', - '6 mons', - '9 mons', - '1 year', - '2 years', - '3 years', - '4 years', - '5 years', - '6 years', - '7 years', - '8 years', - '9 years', - '10 years', + '3 mons', + '6 mons', + '9 mons', + '1 year', + '2 years', + '3 years', + '4 years', + '5 years', + '6 years', + '7 years', + '8 years', + '9 years', + '10 years', ] end + def self.statuses + %w[pending effective expired] + end + + def self.pending + where("#{attribute_alias(:effect_time)} > ?", Time.zone.now) + end + + def self.effective + condition = "#{attribute_alias(:effect_time)} <= :now " \ + " AND (#{attribute_alias(:expire_time)} >= :now" \ + " OR #{attribute_alias(:expire_time)} IS NULL)" + where(condition, now: Time.zone.now) + end + def self.valid where('valid_from <= ? AND (valid_to >= ? OR valid_to IS NULL)', Time.zone.now.end_of_day, Time.zone.now.beginning_of_day) diff --git a/app/models/concerns/billing/price/expirable.rb b/app/models/concerns/billing/price/expirable.rb index c51bdff92..c0e05832a 100644 --- a/app/models/concerns/billing/price/expirable.rb +++ b/app/models/concerns/billing/price/expirable.rb @@ -2,10 +2,6 @@ module Concerns::Billing::Price::Expirable extend ActiveSupport::Concern class_methods do - def unexpired - where("#{attribute_alias(:expire_time)} >= ?", Time.zone.now) - end - def expired where("#{attribute_alias(:expire_time)} < ?", Time.zone.now) end diff --git a/app/views/admin/billing/prices/_search_form.html.erb b/app/views/admin/billing/prices/_search_form.html.erb index e538ebf84..27ba0ca05 100644 --- a/app/views/admin/billing/prices/_search_form.html.erb +++ b/app/views/admin/billing/prices/_search_form.html.erb @@ -1,10 +1,9 @@ <%= form_for :search, url: admin_prices_path, method: :get, html: { class: 'form-horizontal' } do |f| %>
- <%= f.label :validity, class: 'col-sm-2 control-label' %> + <%= f.label :status, class: 'col-sm-2 control-label' %>
- <%= f.select :validity, options_for_select(%w[unexpired expired], search.validity), - { include_blank: true }, + <%= f.select :status, options_for_select(statuses, search.status), { include_blank: true }, class: 'form-control' %>
diff --git a/spec/factories/billing/price.rb b/spec/factories/billing/price.rb index 6700368ae..b7d49653d 100644 --- a/spec/factories/billing/price.rb +++ b/spec/factories/billing/price.rb @@ -7,7 +7,7 @@ FactoryGirl.define do operation_category Billing::Price.operation_categories.first zone - factory :unexpired_price do + factory :effective_price do expire_time { Time.zone.now + 1.day } end diff --git a/spec/features/admin/billing/prices/edit_spec.rb b/spec/features/admin/billing/prices/edit_spec.rb index f6c777bf4..8ef87b9a0 100644 --- a/spec/features/admin/billing/prices/edit_spec.rb +++ b/spec/features/admin/billing/prices/edit_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.feature 'Editing price in admin area', settings: false do - given!(:price) { create(:unexpired_price) } + given!(:price) { create(:effective_price) } background do sign_in_to_admin_area diff --git a/spec/features/admin/billing/prices/expire_spec.rb b/spec/features/admin/billing/prices/expire_spec.rb index 5713e607a..9a835f09c 100644 --- a/spec/features/admin/billing/prices/expire_spec.rb +++ b/spec/features/admin/billing/prices/expire_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.feature 'Expiring price in admin area', settings: false do - given!(:price) { create(:unexpired_price) } + given!(:price) { create(:effective_price) } background do sign_in_to_admin_area diff --git a/spec/features/admin/billing/prices/list_spec.rb b/spec/features/admin/billing/prices/list_spec.rb index a5f63b57f..d6551f8ac 100644 --- a/spec/features/admin/billing/prices/list_spec.rb +++ b/spec/features/admin/billing/prices/list_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.feature 'Viewing prices in admin area', settings: false do - given!(:unexpired_price) { create(:unexpired_price) } + given!(:effective_price) { create(:effective_price) } given!(:expired_price) { create(:expired_price) } background do @@ -9,17 +9,17 @@ RSpec.feature 'Viewing prices in admin area', settings: false do end describe 'search' do - context 'when validity is not selected' do - scenario 'shows unexpired prices' do + context 'when status is not selected' do + scenario 'shows effective prices' do visit admin_prices_path expect(page).to have_css('.price', count: 1) end end - context 'when validity is given' do - scenario 'filters by given validity' do + context 'when status is given' do + scenario 'filters by given status' do visit admin_prices_path - select 'unexpired', from: 'search_validity' + select 'effective', from: 'search_status' submit_search_form expect(page).to have_css('.price', count: 1) diff --git a/spec/models/billing/price_spec.rb b/spec/models/billing/price_spec.rb index 489971478..744f2be26 100644 --- a/spec/models/billing/price_spec.rb +++ b/spec/models/billing/price_spec.rb @@ -3,6 +3,7 @@ require 'rails_helper' RSpec.describe Billing::Price do it { is_expected.to monetize(:price) } it { is_expected.to be_versioned } + it { is_expected.to alias_attribute(:effect_time, :valid_from) } it { is_expected.to alias_attribute(:expire_time, :valid_to) } it 'should have one version' do @@ -14,34 +15,68 @@ RSpec.describe Billing::Price do end describe '::operation_categories', db: false do - it 'returns available operation categories' do + it 'returns operation categories' do categories = %w[create renew] expect(described_class.operation_categories).to eq(categories) end end describe '::durations', db: false do - it 'returns available durations' do + it 'returns durations' do durations = [ - '3 mons', - '6 mons', - '9 mons', - '1 year', - '2 years', - '3 years', - '4 years', - '5 years', - '6 years', - '7 years', - '8 years', - '9 years', - '10 years', + '3 mons', + '6 mons', + '9 mons', + '1 year', + '2 years', + '3 years', + '4 years', + '5 years', + '6 years', + '7 years', + '8 years', + '9 years', + '10 years', ] expect(described_class.durations).to eq(durations) end end + describe '::statuses', db: false do + it 'returns statuses' do + expect(described_class.statuses).to eq(%w[pending effective expired]) + end + end + + describe '::pending' do + before :example do + travel_to Time.zone.parse('05.07.2010 00:00') + + create(:price, id: 1, effect_time: Time.zone.parse('05.07.2010 00:00')) + create(:price, id: 2, effect_time: Time.zone.parse('05.07.2010 00:01')) + end + + it 'returns pending' do + expect(described_class.pending.ids).to eq([2]) + end + end + + describe '::effective' do + before :example do + travel_to Time.zone.parse('05.07.2010 00:00') + + create(:price, id: 1, effect_time: '05.07.2010 00:01', expire_time: '05.07.2010 00:02') + create(:price, id: 2, effect_time: '05.07.2010 00:00', expire_time: '05.07.2010 00:01') + create(:price, id: 3, effect_time: '05.07.2010 00:00', expire_time: nil) + create(:price, id: 4, effect_time: '04.07.2010', expire_time: '04.07.2010 23:59') + end + + it 'returns effective' do + expect(described_class.effective.ids).to eq([2, 3]) + end + end + describe 'zone validation', db: false do subject(:price) { described_class.new } diff --git a/spec/models/concerns/billing/price/expirable_spec.rb b/spec/models/concerns/billing/price/expirable_spec.rb index c9eb1a78e..2180af432 100644 --- a/spec/models/concerns/billing/price/expirable_spec.rb +++ b/spec/models/concerns/billing/price/expirable_spec.rb @@ -1,20 +1,6 @@ require 'rails_helper' RSpec.describe Billing::Price do - describe '::unexpired' do - before :example do - travel_to Time.zone.parse('05.07.2010 00:00') - - create(:price, id: 1, expire_time: Time.zone.parse('04.07.2010 23:59')) - create(:price, id: 2, expire_time: Time.zone.parse('05.07.2010 00:00')) - create(:price, id: 3, expire_time: Time.zone.parse('05.07.2010 00:01')) - end - - it 'returns prices with expire time in the future ' do - expect(described_class.unexpired.ids).to eq([2, 3]) - end - end - describe '::expired' do before :example do travel_to Time.zone.parse('05.07.2010 00:00') diff --git a/spec/requests/admin/billing/prices/expire_spec.rb b/spec/requests/admin/billing/prices/expire_spec.rb index b182b3143..79612bbc9 100644 --- a/spec/requests/admin/billing/prices/expire_spec.rb +++ b/spec/requests/admin/billing/prices/expire_spec.rb @@ -6,14 +6,14 @@ RSpec.describe 'admin price expire', settings: false do end it 'expires price' do - price = create(:unexpired_price) + price = create(:effective_price) expect { patch expire_admin_price_path(price); price.reload } .to change { price.expired? }.from(false).to(true) end it 'redirects to :index' do - price = create(:unexpired_price) + price = create(:effective_price) patch expire_admin_price_path(price)