diff --git a/app/assets/javascripts/app/index.coffee b/app/assets/javascripts/app/index.coffee index 73246f81f..438554c90 100644 --- a/app/assets/javascripts/app/index.coffee +++ b/app/assets/javascripts/app/index.coffee @@ -53,7 +53,7 @@ class App extends Spine.Controller @viewPrintItem: (item, attributeConfig = {}, valueRef, table, object) -> return '-' if item is undefined return '-' if item is '' - return item if item is null + return '-' if item is null result = '' items = [item] if _.isArray(item) diff --git a/app/models/application_model/can_lookup_search_index_attributes.rb b/app/models/application_model/can_lookup_search_index_attributes.rb index 11f731d62..e1f95f408 100644 --- a/app/models/application_model/can_lookup_search_index_attributes.rb +++ b/app/models/application_model/can_lookup_search_index_attributes.rb @@ -70,7 +70,7 @@ returns attributes[ attribute_ref_name ] = value end - if self.class.include?(HasObjectManagerAttributesValidation) + if is_a? HasObjectManagerAttributes RequestCache.integer_fields(self.class.to_s).each do |field| next if attributes[field].blank? diff --git a/app/models/concerns/has_object_manager_attributes.rb b/app/models/concerns/has_object_manager_attributes.rb new file mode 100644 index 000000000..011afbf7d --- /dev/null +++ b/app/models/concerns/has_object_manager_attributes.rb @@ -0,0 +1,14 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +module HasObjectManagerAttributes + extend ActiveSupport::Concern + + included do + # Disable table inheritance to allow columns with the name 'type'. + self.inheritance_column = nil + + validates_with ObjectManager::Attribute::Validation, on: %i[create update] + + after_initialize ObjectManager::Attribute::SetDefaults.new + end +end diff --git a/app/models/concerns/has_object_manager_attributes_validation.rb b/app/models/concerns/has_object_manager_attributes_validation.rb index b42875c5b..f1aa116bb 100644 --- a/app/models/concerns/has_object_manager_attributes_validation.rb +++ b/app/models/concerns/has_object_manager_attributes_validation.rb @@ -5,9 +5,8 @@ module HasObjectManagerAttributesValidation extend ActiveSupport::Concern included do - # Disable table inheritance to allow columns with the name 'type'. - self.inheritance_column = nil + ActiveSupport::Deprecation.warn("Concern 'HasObjectManagerAttributesValidation' is deprecated. Use 'HasObjectManagerValidation' instead.") - validates_with ObjectManager::Attribute::Validation, on: %i[create update] + include HasObjectManagerAttributes end end diff --git a/app/models/group.rb b/app/models/group.rb index 2b94ad01d..9686f1258 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -7,7 +7,7 @@ class Group < ApplicationModel include ChecksHtmlSanitized include ChecksLatestChangeObserved include HasHistory - include HasObjectManagerAttributesValidation + include HasObjectManagerAttributes include HasCollectionUpdate include HasTicketCreateScreenImpact include HasSearchIndexBackend diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index 244cb7140..ef37850a6 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -38,6 +38,13 @@ class ObjectManager::Attribute < ApplicationModel before_validation :set_base_options + scope :active, -> { where(active: true) } + scope :editable, -> { where(editable: true) } + scope :for_object, lambda { |name_or_klass| + id = ObjectLookup.lookup(name: name_or_klass.to_s) + where(object_lookup_id: id) + } + =begin list of all attributes diff --git a/app/models/object_manager/attribute/set_defaults.rb b/app/models/object_manager/attribute/set_defaults.rb new file mode 100644 index 000000000..e8fb41a78 --- /dev/null +++ b/app/models/object_manager/attribute/set_defaults.rb @@ -0,0 +1,62 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +class ObjectManager + class Attribute + class SetDefaults + def after_initialize(record) + return if !record.new_record? + + attributes_for(record).each { |attr, config| set_value(record, attr, config) } + end + + private + + def set_value(record, attr, config) + method_name = "#{attr}=" + + return if !record.respond_to? method_name + return if record.send(attr).present? + + 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 + end + + def attributes_for(record) + query = ObjectManager::Attribute.active.editable.for_object(record.class) + cache_key = "#{query.cache_key}/attribute_defaults" + + Rails.cache.fetch cache_key do + query + .map { |attr| { attr.name => config_of(attr) } } + .reduce({}, :merge) + .compact + end + end + + def config_of(attr) + case attr.data_type + when 'date', 'datetime' + { + data_type: attr.data_type, + diff: attr.data_option&.dig(:diff) + } + else + { + data_type: attr.data_type, + default: attr.data_option&.dig(:default) + } + end + end + end + end +end diff --git a/app/models/organization.rb b/app/models/organization.rb index 85fa3ad8d..7db54e459 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -8,7 +8,7 @@ class Organization < ApplicationModel include HasSearchIndexBackend include CanCsvImport include ChecksHtmlSanitized - include HasObjectManagerAttributesValidation + include HasObjectManagerAttributes include HasTaskbars include Organization::Assets diff --git a/app/models/ticket.rb b/app/models/ticket.rb index 1f89df3b1..71dc22eab 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -13,7 +13,7 @@ class Ticket < ApplicationModel include HasOnlineNotifications include HasKarmaActivityLog include HasLinks - include HasObjectManagerAttributesValidation + include HasObjectManagerAttributes include HasTaskbars include Ticket::CallsStatsTicketReopenLog include Ticket::EnqueuesUserTicketCounterJob diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index c33f35970..a5f8374f9 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -8,7 +8,7 @@ class Ticket::Article < ApplicationModel include ChecksHtmlSanitized include CanCsvImport include CanCloneAttachments - include HasObjectManagerAttributesValidation + include HasObjectManagerAttributes include Ticket::Article::Assets include Ticket::Article::EnqueueCommunicateEmailJob diff --git a/app/models/user.rb b/app/models/user.rb index 32007b4ad..3b32f4841 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -11,7 +11,7 @@ class User < ApplicationModel include ChecksHtmlSanitized include HasGroups include HasRoles - include HasObjectManagerAttributesValidation + include HasObjectManagerAttributes include ::HasTicketCreateScreenImpact include HasTaskbars include User::HasTicketCreateScreenImpact diff --git a/spec/factories/object_manager_attribute.rb b/spec/factories/object_manager_attribute.rb index 4abbc653b..02732b88e 100644 --- a/spec/factories/object_manager_attribute.rb +++ b/spec/factories/object_manager_attribute.rb @@ -5,6 +5,7 @@ FactoryBot.define do transient do object_name { 'Ticket' } additional_data_options { nil } + default { nil } end object_lookup_id { ObjectLookup.by_name(object_name) } @@ -47,7 +48,7 @@ FactoryBot.define do 'maxlength' => 200, 'null' => true, 'translate' => false, - 'default' => '', + 'default' => default || '', 'options' => {}, 'relation' => '', } @@ -58,7 +59,7 @@ FactoryBot.define do data_type { 'integer' } data_option do { - 'default' => 0, + 'default' => default || 0, 'min' => 0, 'max' => 9999, } @@ -69,7 +70,7 @@ FactoryBot.define do data_type { 'boolean' } data_option do { - default: false, + default: default || false, options: { true => 'yes', false => 'no', @@ -83,7 +84,7 @@ FactoryBot.define do data_type { 'date' } data_option do { - 'diff' => 24, + 'diff' => default || 24, 'null' => true, } end @@ -96,7 +97,7 @@ FactoryBot.define do { 'future' => true, 'past' => true, - 'diff' => 24, + 'diff' => default || 24, 'null' => true, } end @@ -106,7 +107,7 @@ FactoryBot.define do data_type { 'select' } data_option do { - 'default' => '', + 'default' => default || '', 'options' => { 'key_1' => 'value_1', 'key_2' => 'value_2', diff --git a/spec/models/concerns/has_object_manager_attributes_validation_examples.rb b/spec/models/concerns/has_object_manager_attributes_examples.rb similarity index 93% rename from spec/models/concerns/has_object_manager_attributes_validation_examples.rb rename to spec/models/concerns/has_object_manager_attributes_examples.rb index acdacfc06..2476d5879 100644 --- a/spec/models/concerns/has_object_manager_attributes_validation_examples.rb +++ b/spec/models/concerns/has_object_manager_attributes_examples.rb @@ -1,6 +1,6 @@ # Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ -RSpec.shared_examples 'HasObjectManagerAttributesValidation' do +RSpec.shared_examples 'HasObjectManagerAttributes' do it 'validates ObjectManager::Attributes' do expect(described_class.validators.map(&:class)).to include(ObjectManager::Attribute::Validation) end diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 0b8f5cb7a..4111da026 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' require 'models/application_model_examples' require 'models/concerns/can_be_imported_examples' -require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/concerns/has_object_manager_attributes_examples' require 'models/concerns/has_collection_update_examples' require 'models/concerns/has_ticket_create_screen_impact_examples' require 'models/concerns/has_xss_sanitized_note_examples' @@ -13,7 +13,7 @@ RSpec.describe Group, type: :model do it_behaves_like 'ApplicationModel' it_behaves_like 'CanBeImported' - it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'HasObjectManagerAttributes' it_behaves_like 'HasCollectionUpdate', collection_factory: :group it_behaves_like 'HasTicketCreateScreenImpact', create_screen_factory: :group it_behaves_like 'HasXssSanitizedNote', model_factory: :group diff --git a/spec/models/object_manager/attribute/set_defaults_spec.rb b/spec/models/object_manager/attribute/set_defaults_spec.rb new file mode 100644 index 000000000..9e5a1cd3e --- /dev/null +++ b/spec/models/object_manager/attribute/set_defaults_spec.rb @@ -0,0 +1,93 @@ +# Copyright (C) 2012-2021 Zammad Foundation, http://zammad-foundation.org/ + +require 'rails_helper' + +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| + create("object_manager_attribute_#{key}", name: "rspec_#{key}", default: value) + end + + create('object_manager_attribute_text', name: 'rspec_empty', default: '') + + ObjectManager::Attribute.migration_execute + end + + after :all do # rubocop:disable RSpec/BeforeAfterAll + ObjectManager::Attribute.where('name LIKE ?', 'rspec_%').destroy_all + end + + context 'with text type' do # on text + it 'default value is set' do + ticket = create :ticket + expect(ticket.rspec_text).to eq 'rspec' + end + + it 'empty string as default value gets saved' do + ticket = create :ticket + expect(ticket.rspec_empty).to eq '' + end + + it 'given value overrides default value' do + ticket = create :ticket, rspec_text: 'another' + expect(ticket.rspec_text).to eq 'another' + end + + # actual create works slightly differently than FactoryGirl! + it 'given value overrides default value when using native #create' do + ticket_attrs = attributes_for :ticket, rspec_text: 'another', group: Group.first + ticket_attrs[:group] = Group.first + ticket_attrs[:customer] = User.first + + ticket_created = Ticket.create! ticket_attrs + + expect(ticket_created.rspec_text).to eq 'another' + end + + it 'given nil overrides default value' do + ticket = create :ticket, rspec_text: nil + expect(ticket.rspec_text).to be_nil + end + + it 'updating attribute to nil does not instantiate default' do + ticket = create :ticket + ticket.update! rspec_text: nil + expect(ticket.rspec_text).to be_nil + end + end + + context 'when using other types' do + subject(:example) { create :ticket } + + it 'boolean is set' do + expect(example.rspec_boolean).to eq true + end + + it 'date is set' do + freeze_time + expect(example.rspec_date).to eq 1.day.from_now.to_date + end + + it 'datetime is set' do + freeze_time + expect(example.rspec_datetime).to eq 12.hours.from_now + end + + it 'integer is set' do + expect(example.rspec_integer).to eq 123 + end + + it 'select value is set' do + expect(example.rspec_select).to eq 'key_1' + end + end + end +end diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb index ff06428ae..9468127d5 100644 --- a/spec/models/organization_spec.rb +++ b/spec/models/organization_spec.rb @@ -6,7 +6,7 @@ require 'models/concerns/can_csv_import_examples' require 'models/concerns/has_history_examples' require 'models/concerns/has_search_index_backend_examples' require 'models/concerns/has_xss_sanitized_note_examples' -require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/concerns/has_object_manager_attributes_examples' require 'models/concerns/has_taskbars_examples' RSpec.describe Organization, type: :model do @@ -17,7 +17,7 @@ RSpec.describe Organization, type: :model do it_behaves_like 'HasHistory' it_behaves_like 'HasSearchIndexBackend', indexed_factory: :organization it_behaves_like 'HasXssSanitizedNote', model_factory: :organization - it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'HasObjectManagerAttributes' it_behaves_like 'HasTaskbars' describe 'Class methods:' do diff --git a/spec/models/ticket/article_spec.rb b/spec/models/ticket/article_spec.rb index eaf6622fd..abb1824d6 100644 --- a/spec/models/ticket/article_spec.rb +++ b/spec/models/ticket/article_spec.rb @@ -5,7 +5,7 @@ require 'models/application_model_examples' require 'models/concerns/can_be_imported_examples' require 'models/concerns/can_csv_import_examples' require 'models/concerns/has_history_examples' -require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/concerns/has_object_manager_attributes_examples' require 'models/ticket/article/has_ticket_contact_attributes_impact_examples' RSpec.describe Ticket::Article, type: :model do @@ -15,7 +15,7 @@ RSpec.describe Ticket::Article, type: :model do it_behaves_like 'CanBeImported' it_behaves_like 'CanCsvImport' it_behaves_like 'HasHistory' - it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'HasObjectManagerAttributes' it_behaves_like 'Ticket::Article::HasTicketContactAttributesImpact' diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index b26764a22..61047fec8 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -8,7 +8,7 @@ require 'models/concerns/has_history_examples' require 'models/concerns/has_tags_examples' require 'models/concerns/has_taskbars_examples' require 'models/concerns/has_xss_sanitized_note_examples' -require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/concerns/has_object_manager_attributes_examples' require 'models/tag/writes_to_ticket_history_examples' require 'models/ticket/calls_stats_ticket_reopen_log_examples' require 'models/ticket/enqueues_user_ticket_counter_job_examples' @@ -28,7 +28,7 @@ RSpec.describe Ticket, type: :model do it_behaves_like 'TagWritesToTicketHistory' it_behaves_like 'HasTaskbars' it_behaves_like 'HasXssSanitizedNote', model_factory: :ticket - it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'HasObjectManagerAttributes' it_behaves_like 'Ticket::Escalation' it_behaves_like 'TicketCallsStatsTicketReopenLog' it_behaves_like 'TicketEnqueuesTicketUserTicketCounterJob' diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 77666cd5f..68feb3103 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -8,7 +8,7 @@ require 'models/concerns/has_roles_examples' require 'models/concerns/has_groups_permissions_examples' require 'models/concerns/has_xss_sanitized_note_examples' require 'models/concerns/can_be_imported_examples' -require 'models/concerns/has_object_manager_attributes_validation_examples' +require 'models/concerns/has_object_manager_attributes_examples' require 'models/user/can_lookup_search_index_attributes_examples' require 'models/user/has_ticket_create_screen_impact_examples' require 'models/user/performs_geo_lookup_examples' @@ -28,7 +28,7 @@ RSpec.describe User, type: :model do it_behaves_like 'HasXssSanitizedNote', model_factory: :user it_behaves_like 'HasGroups and Permissions', group_access_no_permission_factory: :user it_behaves_like 'CanBeImported' - it_behaves_like 'HasObjectManagerAttributesValidation' + it_behaves_like 'HasObjectManagerAttributes' it_behaves_like 'User::HasTicketCreateScreenImpact' it_behaves_like 'CanLookupSearchIndexAttributes' it_behaves_like 'HasTaskbars'