diff --git a/Gemfile b/Gemfile index 898aca82f..10d2b96d5 100644 --- a/Gemfile +++ b/Gemfile @@ -29,7 +29,7 @@ gem 'activeadmin' gem 'active_admin_date_range_preset', github: 'activeadmin-plugins/active_admin_date_range_preset' gem 'active_admin_datetimepicker' gem 'active_admin_import' -gem 'active_admin_scoped_collection_actions' +gem 'active_admin_scoped_collection_actions', github: 'yeti-switch/active_admin_scoped_collection_actions', branch: 'confirmation_with_summary' gem 'active_admin_theme', github: 'yeti-switch/active_admin_theme', branch: 'dark_mode' gem 'draper' gem 'ransack' diff --git a/Gemfile.lock b/Gemfile.lock index 8a268d48b..e39f11e9c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -73,6 +73,14 @@ GIT mustache (~> 1.0, >= 0.99.4) rspec (~> 3.0) +GIT + remote: https://github.com/yeti-switch/active_admin_scoped_collection_actions.git + revision: e8572f032bcd809c743ef912c10a328bb60a583b + branch: confirmation_with_summary + specs: + active_admin_scoped_collection_actions (2.0.0) + activeadmin (>= 3.0, < 4.0) + GIT remote: https://github.com/yeti-switch/active_admin_theme.git revision: 2043bf60325b3cbcd45cf0285f377ad576af0b98 @@ -148,8 +156,6 @@ GEM activerecord-import (>= 2.0) rchardet (>= 1.6) rubyzip (>= 1.2) - active_admin_scoped_collection_actions (2.0.0) - activeadmin (>= 3.0, < 4.0) active_admin_sidebar (3.0.0) activeadmin (>= 3.0, < 4.0) active_record_extended (3.5.0) @@ -1068,7 +1074,7 @@ DEPENDENCIES active_admin_date_range_preset! active_admin_datetimepicker active_admin_import - active_admin_scoped_collection_actions + active_admin_scoped_collection_actions! active_admin_sidebar (~> 3.0) active_admin_theme! active_record_extended diff --git a/app/admin/routing/destinations.rb b/app/admin/routing/destinations.rb index c226762ea..5dc1c9f7d 100644 --- a/app/admin/routing/destinations.rb +++ b/app/admin/routing/destinations.rb @@ -59,6 +59,7 @@ scope :low_quality, show_count: false scope :time_valid, show_count: false + scope :expired, show_count: false scope :scheduled, show_count: false filter :id diff --git a/app/assets/javascripts/active_admin.js b/app/assets/javascripts/active_admin.js index d14264daf..5f9e1ad18 100644 --- a/app/assets/javascripts/active_admin.js +++ b/app/assets/javascripts/active_admin.js @@ -50,6 +50,7 @@ //= require credential_generator //= require vendor/jquery.serialize-object.min.js //= require build_tags +//= require batch_update_datetimepicker //= require ajax_tab //= require rtp_diagram diff --git a/app/assets/javascripts/batch_update_datetimepicker.js b/app/assets/javascripts/batch_update_datetimepicker.js new file mode 100644 index 000000000..2963ae1e5 --- /dev/null +++ b/app/assets/javascripts/batch_update_datetimepicker.js @@ -0,0 +1,15 @@ +// Build date time picker for datetime fields on update batch action form + +// modal dialog pulls focus back into itself, so picker popup must be whitelisted, +// same as jquery ui does it for its own datepicker +var dialogAllowInteraction = $.ui.dialog.prototype._allowInteraction; +$.ui.dialog.prototype._allowInteraction = function (event) { + return dialogAllowInteraction.call(this, event) || !!$(event.target).closest('.xdsoft_datetimepicker').length; +}; + +// dialog is built after the page is loaded, so inputs of BatchUpdateForm::Base#form_data_datetime +// are not picked up by active_admin_datetimepicker itself. setupDateTimePicker is a global +// defined by that gem in app/assets/javascripts/active_admin_datetimepicker.js +$(document).on('mass_update_modal_dialog:after_open', function (event, form) { + setupDateTimePicker(form); +}); diff --git a/app/forms/batch_update_form/base.rb b/app/forms/batch_update_form/base.rb index 6ade9e7ce..fb608d025 100644 --- a/app/forms/batch_update_form/base.rb +++ b/app/forms/batch_update_form/base.rb @@ -115,6 +115,11 @@ def form_data_date(_options) 'datepicker' end + # rendered as a plain input, turned into date time picker by batch_update_datetimepicker.js + def form_data_datetime(_options) + { type: 'text', class: 'date-time-picker' } + end + def form_data_boolean(_options) [%w[Yes true], %w[No false]] end diff --git a/app/forms/batch_update_form/destination.rb b/app/forms/batch_update_form/destination.rb index 974e1cb9b..48a074e2f 100644 --- a/app/forms/batch_update_form/destination.rb +++ b/app/forms/batch_update_form/destination.rb @@ -12,8 +12,8 @@ class BatchUpdateForm::Destination < BatchUpdateForm::Base attribute :reject_calls, type: :boolean attribute :quality_alarm, type: :boolean attribute :rate_group_id, type: :foreign_key, class_name: 'Routing::RateGroup' - attribute :valid_from, type: :date - attribute :valid_till, type: :date + attribute :valid_from, type: :datetime + attribute :valid_till, type: :datetime attribute :rate_policy_id, type: :integer_collection, collection: Routing::DestinationRatePolicy::POLICIES.invert.to_a attribute :initial_interval attribute :initial_rate @@ -108,7 +108,7 @@ class BatchUpdateForm::Destination < BatchUpdateForm::Base validates :next_rate, numericality: { allow_blank: true }, if: :next_rate_changed? # date validations - validates_date :valid_from, on_or_before: :valid_till, if: -> { valid_from.present? && valid_till.present? } + validates_datetime :valid_from, on_or_before: :valid_till, if: -> { valid_from.present? && valid_till.present? } # format validations validates :prefix, format: { without: /\s/, message: I18n.t('activerecord.errors.models.routing\destination.attributes.prefix.with_spaces') }, if: :prefix_changed? diff --git a/app/forms/batch_update_form/dialpeer.rb b/app/forms/batch_update_form/dialpeer.rb index ad4718d2a..b281e27a3 100644 --- a/app/forms/batch_update_form/dialpeer.rb +++ b/app/forms/batch_update_form/dialpeer.rb @@ -25,8 +25,8 @@ class BatchUpdateForm::Dialpeer < BatchUpdateForm::Base attribute :vendor_id, type: :foreign_key, class_name: 'Contractor', scope: :vendors attribute :account_id, type: :foreign_key, class_name: 'Account' attribute :routeset_discriminator_id, type: :foreign_key, class_name: 'Routing::RoutesetDiscriminator' - attribute :valid_from, type: :date - attribute :valid_till, type: :date + attribute :valid_from, type: :datetime + attribute :valid_till, type: :datetime attribute :asr_limit attribute :acd_limit attribute :short_calls_limit @@ -166,5 +166,5 @@ def gateway_is_shared? Gateway.find_by(id: gateway_id)&.is_shared? end - validates_date :valid_from, on_or_before: :valid_till, if: -> { valid_from.present? && valid_till.present? } + validates_datetime :valid_from, on_or_before: :valid_till, if: -> { valid_from.present? && valid_till.present? } end diff --git a/app/models/routing/destination.rb b/app/models/routing/destination.rb index fdec3c355..761fa9505 100644 --- a/app/models/routing/destination.rb +++ b/app/models/routing/destination.rb @@ -73,7 +73,8 @@ class Routing::Destination < ApplicationRecord include RoutingTagIdsScopeable scope :low_quality, -> { where quality_alarm: true } - scope :time_valid, -> { where('valid_till >= :time AND valid_from < :time', time: Time.now) } + scope :time_valid, -> { where('valid_till >= NOW() AND valid_from < NOW()') } + scope :expired, -> { where('valid_till < NOW()') } scope :rateplan_id_filter, lambda { |value| rate_group_ids = Routing::RatePlanGroup.where(rateplan_id: value).pluck(:rate_group_id).uniq diff --git a/lib/resource_dsl/acts_as_async_destroy.rb b/lib/resource_dsl/acts_as_async_destroy.rb index 345aea9ef..99ae688d4 100644 --- a/lib/resource_dsl/acts_as_async_destroy.rb +++ b/lib/resource_dsl/acts_as_async_destroy.rb @@ -9,6 +9,7 @@ def acts_as_async_destroy(model_class) scoped_collection_action :async_destroy, title: 'Delete batch', + confirm_summary: -> { I18n.t('active_admin_scoped_collection_actions.confirm_destroy_summary') }, if: proc { authorized?(:batch_destroy, resource_class) } do AsyncBatchDestroyJob.perform_later(model_class, scoped_collection_records.except(:eager_load).to_sql, @paper_trail_info) flash[:notice] = I18n.t('flash.actions.batch_actions.batch_destroy.job_scheduled') diff --git a/lib/resource_dsl/acts_as_async_update.rb b/lib/resource_dsl/acts_as_async_update.rb index 9df78080f..9a0e3c9b4 100644 --- a/lib/resource_dsl/acts_as_async_update.rb +++ b/lib/resource_dsl/acts_as_async_update.rb @@ -12,6 +12,7 @@ def acts_as_async_update(form_class) title: 'Update batch', class: 'scoped_collection_action_button ui', form: -> { form_class.form_data }, + confirm_submit: true, if: proc { authorized?(:batch_update, resource_class) } do attrs = params[:changes]&.permit! # if there is no changes just reload page quietly diff --git a/spec/features/billing/accounts/batch_update_spec.rb b/spec/features/billing/accounts/batch_update_spec.rb index 1ac665908..fbbd2837f 100644 --- a/spec/features/billing/accounts/batch_update_spec.rb +++ b/spec/features/billing/accounts/batch_update_spec.rb @@ -96,6 +96,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'should check validates' do diff --git a/spec/features/billing/contacts/batch_update_spec.rb b/spec/features/billing/contacts/batch_update_spec.rb index 237f6d352..15fcfbeec 100644 --- a/spec/features/billing/contacts/batch_update_spec.rb +++ b/spec/features/billing/contacts/batch_update_spec.rb @@ -47,6 +47,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'should check validates' do diff --git a/spec/features/billing/contractors/batch_update_spec.rb b/spec/features/billing/contractors/batch_update_spec.rb index 53614f7de..d1a43cd9a 100644 --- a/spec/features/billing/contractors/batch_update_spec.rb +++ b/spec/features/billing/contractors/batch_update_spec.rb @@ -17,6 +17,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end let(:assign_params) do diff --git a/spec/features/equipment/gateway_groups/batch_update_spec.rb b/spec/features/equipment/gateway_groups/batch_update_spec.rb index f4b4a9eee..babe2a745 100644 --- a/spec/features/equipment/gateway_groups/batch_update_spec.rb +++ b/spec/features/equipment/gateway_groups/batch_update_spec.rb @@ -23,6 +23,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'when all field filled with valid values' do diff --git a/spec/features/equipment/gateways/batch_update_spec.rb b/spec/features/equipment/gateways/batch_update_spec.rb index 8a033d1a1..5617955f3 100644 --- a/spec/features/equipment/gateways/batch_update_spec.rb +++ b/spec/features/equipment/gateways/batch_update_spec.rb @@ -16,6 +16,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end let(:assign_params) do diff --git a/spec/features/routing/customers_auths/batch_update_spec.rb b/spec/features/routing/customers_auths/batch_update_spec.rb index c4f376ba9..eb8a97893 100644 --- a/spec/features/routing/customers_auths/batch_update_spec.rb +++ b/spec/features/routing/customers_auths/batch_update_spec.rb @@ -105,6 +105,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'check validations' do diff --git a/spec/features/routing/destinations/batch_update_spec.rb b/spec/features/routing/destinations/batch_update_spec.rb index 349acd363..83195e34d 100644 --- a/spec/features/routing/destinations/batch_update_spec.rb +++ b/spec/features/routing/destinations/batch_update_spec.rb @@ -12,6 +12,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end let(:assign_params) do @@ -24,8 +25,8 @@ reject_calls: false, quality_alarm: true, rate_group_id: rate_group.id.to_s, - valid_from: '2020-01-10', - valid_till: '2020-01-20', + valid_from: '2020-01-10 10:15', + valid_till: '2020-01-20 20:45', rate_policy_id: rate_policy_id.to_s, initial_interval: '1', initial_rate: '1', @@ -218,7 +219,10 @@ end context 'when user wants to change routing_tag_ids to "NOT TAGGED"' do - subject { click_button :OK } + subject do + click_button :OK + confirm_batch_update + end let(:_destinations) { nil } let(:assign_params) { {} } diff --git a/spec/features/routing/destinations/index_destination_spec.rb b/spec/features/routing/destinations/index_destination_spec.rb index f5ce793ed..1d97dd36f 100644 --- a/spec/features/routing/destinations/index_destination_spec.rb +++ b/spec/features/routing/destinations/index_destination_spec.rb @@ -15,6 +15,28 @@ end end + context 'with expired scope' do + subject do + visit destinations_path(scope: 'expired') + end + + let!(:expired_destinations) do + create_list(:destination, 3, valid_from: 2.days.ago, valid_till: 1.hour.ago) + end + + before do + create(:destination, valid_from: 1.day.ago, valid_till: 1.day.from_now) + end + + it 'responds with correct rows' do + subject + expect(page).to have_table_row(count: expired_destinations.size) + expired_destinations.each do |destination| + expect(page).to have_table_cell(column: 'ID', exact_text: destination.id.to_s) + end + end + end + context 'when filter by country and network' do let!(:country) { System::Country.find_by!(name: 'United States') } let!(:network) { create(:network, name: 'some network') } diff --git a/spec/features/routing/dialpeers/batch_update_spec.rb b/spec/features/routing/dialpeers/batch_update_spec.rb index 91a851f4b..6c20d001c 100644 --- a/spec/features/routing/dialpeers/batch_update_spec.rb +++ b/spec/features/routing/dialpeers/batch_update_spec.rb @@ -47,8 +47,8 @@ vendor_id: vendor_main.id.to_s, account_id: account_vendors.id.to_s, routeset_discriminator_id: routeset_discriminator.id.to_s, - valid_from: '2020-01-10', - valid_till: '2020-01-20', + valid_from: '2020-01-10 10:15', + valid_till: '2020-01-20 20:45', asr_limit: '0.9', acd_limit: '0.9', short_calls_limit: '0.9', @@ -239,6 +239,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'check validation' do @@ -279,7 +280,10 @@ end context 'when user wants to change routing_tag_ids to "ANY TAG"' do - subject { click_button :OK } + subject do + click_button :OK + confirm_batch_update + end let(:_dialpeers) { nil } let(:routing_group) { nil } diff --git a/spec/features/routing/numberlists/batch_update_spec.rb b/spec/features/routing/numberlists/batch_update_spec.rb index 6610b00d3..0f36ebcc1 100644 --- a/spec/features/routing/numberlists/batch_update_spec.rb +++ b/spec/features/routing/numberlists/batch_update_spec.rb @@ -17,6 +17,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end let(:assign_params) do diff --git a/spec/features/routing/routing_plan_lnp_rules/batch_update_spec.rb b/spec/features/routing/routing_plan_lnp_rules/batch_update_spec.rb index 17570b166..7c2b56ea1 100644 --- a/spec/features/routing/routing_plan_lnp_rules/batch_update_spec.rb +++ b/spec/features/routing/routing_plan_lnp_rules/batch_update_spec.rb @@ -58,6 +58,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'should check validates for the field:' do diff --git a/spec/features/routing/routing_plan_static_routes/batch_update_spec.rb b/spec/features/routing/routing_plan_static_routes/batch_update_spec.rb index e9fd305d3..475884e54 100644 --- a/spec/features/routing/routing_plan_static_routes/batch_update_spec.rb +++ b/spec/features/routing/routing_plan_static_routes/batch_update_spec.rb @@ -54,6 +54,7 @@ subject do fill_batch_form click_button :OK + confirm_batch_update end context 'should check validates' do diff --git a/spec/features/routing/routing_plans/batch_update_spec.rb b/spec/features/routing/routing_plans/batch_update_spec.rb index 938e44091..ae9e4e540 100644 --- a/spec/features/routing/routing_plans/batch_update_spec.rb +++ b/spec/features/routing/routing_plans/batch_update_spec.rb @@ -39,6 +39,7 @@ subject do fill_batch_form click_button 'OK' + confirm_batch_update end context 'should check validates for the field:' do diff --git a/spec/models/batch_update_form/destination_spec.rb b/spec/models/batch_update_form/destination_spec.rb index 3ec6d022e..8f0133724 100644 --- a/spec/models/batch_update_form/destination_spec.rb +++ b/spec/models/batch_update_form/destination_spec.rb @@ -108,12 +108,24 @@ it { is_expected.to_not allow_value('string test').for(:prefix).with_message(prefix_err_message) } + # :valid_till as it is displayed in the validation error + let(:formatted_valid_till) { Time.zone.parse(assign_params[:valid_till]).strftime('%Y-%m-%d %H:%M:%S') } + context 'when :valid_from date is later than :valid_till date' do let(:assign_params) { { valid_from: '2020-09-09', valid_till: '2020-01-01' } } it 'should have error: :valid_from must be before or equal to' do subject - expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{assign_params[:valid_till]}" + expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{formatted_valid_till}" + end + end + + context 'when :valid_from time is later than :valid_till time of the same date' do + let(:assign_params) { { valid_from: '2020-01-01 15:00', valid_till: '2020-01-01 10:30' } } + + it 'should have error: :valid_from must be before or equal to' do + subject + expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{formatted_valid_till}" end end diff --git a/spec/models/batch_update_form/dialpeer_spec.rb b/spec/models/batch_update_form/dialpeer_spec.rb index 9c5863065..618e0d119 100644 --- a/spec/models/batch_update_form/dialpeer_spec.rb +++ b/spec/models/batch_update_form/dialpeer_spec.rb @@ -222,12 +222,24 @@ it 'should pass validations' do expect(subject).to be_valid end end + # :valid_till as it is displayed in the validation error + let(:formatted_valid_till) { Time.zone.parse(assign_params[:valid_till]).strftime('%Y-%m-%d %H:%M:%S') } + context 'when :valid_till is before than :valid_from' do let(:assign_params) { { valid_from: '2020-05-05', valid_till: '2020-01-01' } } it 'should have error:' do subject - expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{assign_params[:valid_till]}" + expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{formatted_valid_till}" + end + end + + context 'when :valid_till time is before than :valid_from time of the same date' do + let(:assign_params) { { valid_from: '2020-01-01 15:00', valid_till: '2020-01-01 10:30' } } + + it 'should have error:' do + subject + expect(subject.errors.to_a).to contain_exactly "Valid from must be before or equal to #{formatted_valid_till}" end end diff --git a/spec/support/helpers/feature_test_helper.rb b/spec/support/helpers/feature_test_helper.rb index 476d3b5a1..04d8e3f61 100644 --- a/spec/support/helpers/feature_test_helper.rb +++ b/spec/support/helpers/feature_test_helper.rb @@ -71,4 +71,11 @@ def table_select_row(id) def within_main_content(&block) within('#main_content_wrapper', &block) end + + # confirms "Update batch" dialog after it was submitted + def confirm_batch_update + within('.active_admin_dialog_confirm_submit') do + click_button 'OK' + end + end end