From 4e0707905664241841430a6c1a10ce6c0c3045a6 Mon Sep 17 00:00:00 2001 From: Thorsten Eckel Date: Thu, 14 Mar 2019 11:06:49 +0100 Subject: [PATCH] Follow up - 444e48e3772eb4abf8fb00170dca3d08ad65955f - Removed future and past data_options of Date ObjectManager::Attribute because they are not determinable due to time zones. --- .../object_manager_attribute.coffee | 20 --------------- app/models/object_manager/attribute.rb | 6 +---- .../validation/{date.rb => future_past.rb} | 4 +-- ...nager_attribute_date_remove_future_past.rb | 12 +++++++++ ..._attribute_date_remove_future_past_spec.rb | 25 +++++++++++++++++++ spec/factories/object_manager_attribute.rb | 6 ++--- .../{date_spec.rb => future_past_spec.rb} | 4 +-- test/integration/object_manager_test.rb | 19 -------------- 8 files changed, 44 insertions(+), 52 deletions(-) rename app/models/object_manager/attribute/validation/{date.rb => future_past.rb} (76%) create mode 100644 db/migrate/20190314084909_object_manager_attribute_date_remove_future_past.rb create mode 100644 spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb rename spec/models/object_manager/attribute/validation/{date_spec.rb => future_past_spec.rb} (91%) diff --git a/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee b/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee index 320b025bd..76eec780d 100644 --- a/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee +++ b/app/assets/javascripts/app/controllers/_ui_element/object_manager_attribute.coffee @@ -231,24 +231,6 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi item.find('.js-datetimeDiff').html(datetimeDiff.form) @date: (item, localParams, params) -> - configureAttributes = [ - { name: 'data_option::future', display: 'Allow future', tag: 'boolean', null: false, default: true }, - ] - dateFuture = new App.ControllerForm( - model: - configure_attributes: configureAttributes - noFieldset: true - params: params - ) - configureAttributes = [ - { name: 'data_option::past', display: 'Allow past', tag: 'boolean', null: false, default: true }, - ] - datePast = new App.ControllerForm( - model: - configure_attributes: configureAttributes - noFieldset: true - params: params - ) configureAttributes = [ { name: 'data_option::diff', display: 'Default time Diff (hours)', tag: 'integer', null: false, default: 24 }, ] @@ -258,8 +240,6 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi noFieldset: true params: params ) - item.find('.js-dateFuture').html(dateFuture.form) - item.find('.js-datePast').html(datePast.form) item.find('.js-dateDiff').html(dateDiff.form) @integer: (item, localParams, params) -> diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 088eb2bd4..3d87aaa2a 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -1001,11 +1001,7 @@ is certain attribute used by triggers, overviews or schedulers { failed: local_data_option[:diff].nil?, message: 'must have integer value for :diff (in hours)' }] when 'date' - [{ failed: local_data_option[:future].nil?, - message: 'must have boolean value for :future' }, - { failed: local_data_option[:past].nil?, - message: 'must have boolean value for :past' }, - { failed: local_data_option[:diff].nil?, + [{ failed: local_data_option[:diff].nil?, message: 'must have integer value for :diff (in days)' }] else [] diff --git a/app/models/object_manager/attribute/validation/date.rb b/app/models/object_manager/attribute/validation/future_past.rb similarity index 76% rename from app/models/object_manager/attribute/validation/date.rb rename to app/models/object_manager/attribute/validation/future_past.rb index da64ab3fc..2715ebd9b 100644 --- a/app/models/object_manager/attribute/validation/date.rb +++ b/app/models/object_manager/attribute/validation/future_past.rb @@ -1,4 +1,4 @@ -class ObjectManager::Attribute::Validation::Date < ObjectManager::Attribute::Validation::Backend +class ObjectManager::Attribute::Validation::FuturePast < ObjectManager::Attribute::Validation::Backend def validate return if value.blank? @@ -11,7 +11,7 @@ class ObjectManager::Attribute::Validation::Date < ObjectManager::Attribute::Val private def irrelevant_attribute? - %w[date datetime].exclude?(attribute.data_type) + attribute.data_type != 'datetime'.freeze end def validate_past diff --git a/db/migrate/20190314084909_object_manager_attribute_date_remove_future_past.rb b/db/migrate/20190314084909_object_manager_attribute_date_remove_future_past.rb new file mode 100644 index 000000000..47df6274a --- /dev/null +++ b/db/migrate/20190314084909_object_manager_attribute_date_remove_future_past.rb @@ -0,0 +1,12 @@ +class ObjectManagerAttributeDateRemoveFuturePast < ActiveRecord::Migration[5.1] + def change + + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + ObjectManager::Attribute.where(data_type: 'date').each do |attribute| + attribute.data_option = attribute.data_option.except(:future, :past) + attribute.save! + end + end +end diff --git a/spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb b/spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb new file mode 100644 index 000000000..779e4ceac --- /dev/null +++ b/spec/db/migrate/object_manager_attribute_date_remove_future_past_spec.rb @@ -0,0 +1,25 @@ +require 'rails_helper' + +RSpec.describe ObjectManagerAttributeDateRemoveFuturePast, type: :db_migration do + context 'when Date ObjectManager::Attribute exists' do + + it 'removes future and past data_option' do + subject = build(:object_manager_attribute_date) + + # add data_options manually because the factory doesn't contain them anymore + subject.data_option = subject.data_option.merge( + future: false, + past: false, + ) + + # mock interfaces to save time + # otherwise we would have to reseed the database + expect(ObjectManager::Attribute).to receive(:where).and_return([subject]) + expect(subject).to receive(:save!) + + migrate + + expect(subject.data_option).to_not include(:past, :future) + end + end +end diff --git a/spec/factories/object_manager_attribute.rb b/spec/factories/object_manager_attribute.rb index 07e82943f..8168fb48f 100644 --- a/spec/factories/object_manager_attribute.rb +++ b/spec/factories/object_manager_attribute.rb @@ -69,10 +69,8 @@ FactoryBot.define do data_type 'date' data_option do { - 'future' => true, - 'past' => true, - 'diff' => 24, - 'null' => true, + 'diff' => 24, + 'null' => true, } end end diff --git a/spec/models/object_manager/attribute/validation/date_spec.rb b/spec/models/object_manager/attribute/validation/future_past_spec.rb similarity index 91% rename from spec/models/object_manager/attribute/validation/date_spec.rb rename to spec/models/object_manager/attribute/validation/future_past_spec.rb index 0b8ef4ddc..0b1d4fe19 100644 --- a/spec/models/object_manager/attribute/validation/date_spec.rb +++ b/spec/models/object_manager/attribute/validation/future_past_spec.rb @@ -1,10 +1,10 @@ require 'rails_helper' require 'models/object_manager/attribute/validation/backend_examples' -RSpec.describe ::ObjectManager::Attribute::Validation::Date do +RSpec.describe ::ObjectManager::Attribute::Validation::FuturePast do let(:record) { build(:user) } - let(:attribute) { build(:object_manager_attribute_date) } + let(:attribute) { build(:object_manager_attribute_datetime) } subject do described_class.new( record: record, diff --git a/test/integration/object_manager_test.rb b/test/integration/object_manager_test.rb index a43ebb46a..eba3abb0a 100644 --- a/test/integration/object_manager_test.rb +++ b/test/integration/object_manager_test.rb @@ -284,25 +284,6 @@ class ObjectManagerTest < ActiveSupport::TestCase object: 'Ticket', name: 'test11', ) - - assert_raises(ActiveRecord::RecordInvalid) do - attribute12 = ObjectManager::Attribute.add( - object: 'Ticket', - name: 'test12', - display: 'Test 12', - data_type: 'date', - data_option: { - past: false, - diff: 24, - null: true, - }, - active: true, - screens: {}, - position: 20, - created_by_id: 1, - updated_by_id: 1, - ) - end assert_equal(false, ObjectManager::Attribute.pending_migration?) assert_raises(RuntimeError) do