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 316515828..5212bc356 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 @@ -244,7 +244,7 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi params: params ) configureAttributes = [ - { name: 'data_option::diff', display: 'Default time Diff (minutes)', tag: 'integer', null: false, default: 24 }, + { name: 'data_option::diff', display: 'Default time Diff (minutes)', tag: 'integer', null: true }, ] datetimeDiff = new App.ControllerForm( model: @@ -258,7 +258,7 @@ class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUi @date: (item, localParams, params) -> configureAttributes = [ - { name: 'data_option::diff', display: 'Default time Diff (hours)', tag: 'integer', null: false, default: 24 }, + { name: 'data_option::diff', display: 'Default time Diff (hours)', tag: 'integer', null: true }, ] dateDiff = new App.ControllerForm( model: diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 2b21267df..5f16d65ac 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -946,12 +946,7 @@ is certain attribute used by triggers, overviews or schedulers [{ 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?, - message: 'must have integer value for :diff (in hours)' }] - when 'date' - [{ failed: local_data_option[:diff].nil?, - message: 'must have integer value for :diff (in days)' }] + message: 'must have boolean value for :past' }] else [] end diff --git a/app/models/object_manager/attribute/set_defaults.rb b/app/models/object_manager/attribute/set_defaults.rb index e8fb41a78..915033565 100644 --- a/app/models/object_manager/attribute/set_defaults.rb +++ b/app/models/object_manager/attribute/set_defaults.rb @@ -15,25 +15,30 @@ class ObjectManager method_name = "#{attr}=" return if !record.respond_to? method_name - return if record.send(attr).present? + return if record.send("#{attr}_came_from_user?") record.send method_name, build_value(config) end def build_value(config) - case config[:data_type] - when 'date' - config[:diff].days.from_now - when 'datetime' - config[:diff].hours.from_now - else - config[:default] - end + method_name = "build_value_#{config[:data_type]}" + + return send(method_name, config) if respond_to?(method_name, true) + + config[:default] + end + + def build_value_date(config) + config[:diff]&.days&.from_now + end + + def build_value_datetime(config) + config[:diff]&.hours&.from_now&.change(usec: 0, sec: 0) end def attributes_for(record) query = ObjectManager::Attribute.active.editable.for_object(record.class) - cache_key = "#{query.cache_key}/attribute_defaults" + cache_key = "#{query.cache_key_with_version}/attribute_defaults" Rails.cache.fetch cache_key do query diff --git a/db/migrate/20211020131134_issue_3810_custom_date_attribute_no_default.rb b/db/migrate/20211020131134_issue_3810_custom_date_attribute_no_default.rb new file mode 100644 index 000000000..edde66611 --- /dev/null +++ b/db/migrate/20211020131134_issue_3810_custom_date_attribute_no_default.rb @@ -0,0 +1,18 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class Issue3810CustomDateAttributeNoDefault < ActiveRecord::Migration[6.0] + def up + return if !Setting.exists?(name: 'system_init_done') + + ObjectManager::Attribute + .where(data_type: %i[date datetime]) + .each { |elem| update_single(elem) } + end + + def update_single(elem) + elem.data_option[:diff] = nil + elem.save! + rescue => e + Rails.logger.error e + end +end diff --git a/public/assets/tests/qunit/form.js b/public/assets/tests/qunit/form.js index cd7edc261..1562b53cf 100644 --- a/public/assets/tests/qunit/form.js +++ b/public/assets/tests/qunit/form.js @@ -1125,7 +1125,7 @@ QUnit.test("object manager form 1", assert => { params = App.ControllerForm.params(el) var test_params = { data_option: { - diff: 24, + diff: null, future: true, past: true }, diff --git a/spec/db/migrate/issue_3810_custom_date_attribute_no_default_spec.rb b/spec/db/migrate/issue_3810_custom_date_attribute_no_default_spec.rb new file mode 100644 index 000000000..46cefe948 --- /dev/null +++ b/spec/db/migrate/issue_3810_custom_date_attribute_no_default_spec.rb @@ -0,0 +1,21 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +RSpec.describe Issue3810CustomDateAttributeNoDefault, type: :db_migration, db_strategy: :reset_all do + before :all do # rubocop:disable RSpec/BeforeAfterAll + create('object_manager_attribute_date', name: 'rspec_date', default: 24) + create('object_manager_attribute_datetime', name: 'rspec_datetime', default: 24) + + ObjectManager::Attribute.migration_execute + end + + after :all do # rubocop:disable RSpec/BeforeAfterAll + ObjectManager::Attribute.where('name LIKE ?', 'rspec_%').destroy_all + end + + it 'unsets diff migration' do + migrate + expect(create(:ticket)).to have_attributes(rspec_date: nil, rspec_datetime: nil) + end +end diff --git a/spec/factories/object_manager_attribute.rb b/spec/factories/object_manager_attribute.rb index 6920bd886..807374d1b 100644 --- a/spec/factories/object_manager_attribute.rb +++ b/spec/factories/object_manager_attribute.rb @@ -41,6 +41,8 @@ FactoryBot.define do end factory :object_manager_attribute_text, parent: :object_manager_attribute do + default { '' } + data_type { 'input' } data_option do { @@ -48,7 +50,7 @@ FactoryBot.define do 'maxlength' => 200, 'null' => true, 'translate' => false, - 'default' => default || '', + 'default' => default, 'options' => {}, 'relation' => '', } @@ -71,10 +73,12 @@ FactoryBot.define do end factory :object_manager_attribute_integer, parent: :object_manager_attribute do + default { 0 } + data_type { 'integer' } data_option do { - 'default' => default || 0, + 'default' => default, 'min' => 0, 'max' => 9999, } @@ -82,10 +86,12 @@ FactoryBot.define do end factory :object_manager_attribute_boolean, parent: :object_manager_attribute do + default { false } + data_type { 'boolean' } data_option do { - default: default || false, + default: default, options: { true => 'yes', false => 'no', @@ -95,34 +101,40 @@ FactoryBot.define do end factory :object_manager_attribute_date, parent: :object_manager_attribute do + default { 24 } + name { 'date_attribute' } data_type { 'date' } data_option do { - 'diff' => default || 24, + 'diff' => default, 'null' => true, } end end factory :object_manager_attribute_datetime, parent: :object_manager_attribute do + default { 24 } + name { 'datetime_attribute' } data_type { 'datetime' } data_option do { 'future' => true, 'past' => true, - 'diff' => default || 24, + 'diff' => default, 'null' => true, } end end factory :object_manager_attribute_select, parent: :object_manager_attribute do + default { '' } + data_type { 'select' } data_option do { - 'default' => default || '', + 'default' => default, 'options' => { 'key_1' => 'value_1', 'key_2' => 'value_2', @@ -139,6 +151,8 @@ FactoryBot.define do end factory :object_manager_attribute_tree_select, parent: :object_manager_attribute do + default { '' } + data_type { 'tree_select' } data_option do { diff --git a/spec/models/object_manager/attribute/set_defaults_spec.rb b/spec/models/object_manager/attribute/set_defaults_spec.rb index 9e5a1cd3e..7fb9a98fe 100644 --- a/spec/models/object_manager/attribute/set_defaults_spec.rb +++ b/spec/models/object_manager/attribute/set_defaults_spec.rb @@ -2,18 +2,21 @@ require 'rails_helper' +DEFAULT_VALUES = { + text: 'rspec', + boolean: true, + date: 1, + datetime: 12, + integer: 123, + select: 'key_1' +}.freeze + RSpec.describe ObjectManager::Attribute::SetDefaults, type: :model do describe 'setting default', db_strategy: :reset_all do before :all do # rubocop:disable RSpec/BeforeAfterAll - { - text: 'rspec', - boolean: true, - date: 1, - datetime: 12, - integer: 123, - select: 'key_1' - }.each do |key, value| + DEFAULT_VALUES.each do |key, value| create("object_manager_attribute_#{key}", name: "rspec_#{key}", default: value) + create("object_manager_attribute_#{key}", name: "rspec_#{key}_no_default", default: nil) end create('object_manager_attribute_text', name: 'rspec_empty', default: '') @@ -77,7 +80,7 @@ RSpec.describe ObjectManager::Attribute::SetDefaults, type: :model do end it 'datetime is set' do - freeze_time + travel_to Time.current.change(usec: 0, sec: 0) expect(example.rspec_datetime).to eq 12.hours.from_now end @@ -89,5 +92,28 @@ RSpec.describe ObjectManager::Attribute::SetDefaults, type: :model do expect(example.rspec_select).to eq 'key_1' end end + + context 'when overriding default to empty value' do + subject(:example) do + params = DEFAULT_VALUES.keys.each_with_object({}) { |elem, memo| memo["rspec_#{elem}"] = nil } + create :ticket, params + end + + DEFAULT_VALUES.each_key do |elem| + it "#{elem} is empty" do + expect(example.send("rspec_#{elem}")).to be_nil + end + end + end + + context 'when default is not set' do + subject(:example) { create :ticket } + + DEFAULT_VALUES.each_key do |elem| + it "#{elem} is empty" do + expect(example.send("rspec_#{elem}_no_default")).to be_nil + end + end + end end end diff --git a/spec/system/system/object_manager_spec.rb b/spec/system/system/object_manager_spec.rb index 3fa859bdc..d69ef5eb1 100644 --- a/spec/system/system/object_manager_spec.rb +++ b/spec/system/system/object_manager_spec.rb @@ -489,4 +489,30 @@ RSpec.describe 'System > Objects', type: :system do end end end + + context 'when creating with no diff' do + before do + visit '/#system/object_manager' + page.find('.js-new').click + + in_modal disappears: false do + fill_in 'Name', with: 'nodiff' + fill_in 'Display', with: 'NoDiff' + end + end + + it 'date attribute' do + page.find('select[name=data_type]').select('Date') + fill_in 'Default time Diff (hours)', with: '' + + expect { page.find('.js-submit').click }.to change(ObjectManager::Attribute, :count).by(1) + end + + it 'datetime attribute' do + page.find('select[name=data_type]').select('Datetime') + fill_in 'Default time Diff (minutes)', with: '' + + expect { page.find('.js-submit').click }.to change(ObjectManager::Attribute, :count).by(1) + end + end end