From d3ec647189fc25b2818af787875c01d5e2ebbbf9 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Sun, 29 May 2016 19:48:28 +0200 Subject: [PATCH] Added browser reload support if only config changes have changed. Added support to undo config changes. --- .../object_manager_attribute.coffee | 5 ++ .../app/views/object_manager/index.jst.eco | 2 + app/models/object_manager/attribute.rb | 74 +++++++++++++++---- db/migrate/20120101000001_create_base.rb | 2 + .../20160529000001_update_object_manager2.rb | 10 +++ test/browser/admin_object_manager_test.rb | 7 +- test/unit/object_manager_test.rb | 64 ++++++++++++++++ 7 files changed, 148 insertions(+), 16 deletions(-) create mode 100644 db/migrate/20160529000001_update_object_manager2.rb 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 0ba864fa5..77065db3b 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 @@ -2,6 +2,11 @@ # coffeelint: disable=camel_case_classes class App.UiElement.object_manager_attribute extends App.UiElement.ApplicationUiElement @render: (attribute, params = {}) -> + + # if we have already changed settings, use them in edit screen + if params.data_option_new && !_.isEmpty(params.data_option_new) + params.data_option = params.data_option_new + item = $(App.view('object_manager/attribute')(attribute: attribute)) updateDataMap = (localParams, localAttribute, localAttributes, localClassname, localForm, localA) => diff --git a/app/assets/javascripts/app/views/object_manager/index.jst.eco b/app/assets/javascripts/app/views/object_manager/index.jst.eco index 375767ffe..d934bcee3 100644 --- a/app/assets/javascripts/app/views/object_manager/index.jst.eco +++ b/app/assets/javascripts/app/views/object_manager/index.jst.eco @@ -27,6 +27,8 @@ <%- @T('Create') %>: <%= item.object %>.<%= item.name %> (<%= item.data_type %>) <% else if item.to_delete is true: %> <%- @T('Delete') %>: <%= item.object %>.<%= item.name %> (<%= item.data_type %>) + <% else if item.to_migrate || item.to_config is true: %> + <%- @T('Changed') %>: <%= item.object %>.<%= item.name %> (<%= item.data_type %>) <% end %> <% end %>

diff --git a/app/models/object_manager/attribute.rb b/app/models/object_manager/attribute.rb index f5b5014d2..23f0fa926 100644 --- a/app/models/object_manager/attribute.rb +++ b/app/models/object_manager/attribute.rb @@ -4,6 +4,7 @@ class ObjectManager::Attribute < ApplicationModel validates :name, presence: true store :screens store :data_option + store :data_option_new notify_clients_support @@ -75,6 +76,7 @@ add a new attribute entry for an object to_migrate: false, to_create: false, to_delete: false, + to_config: false, ) preserved name are @@ -103,6 +105,7 @@ possible types 'aa' => 'aa (comment)', 'bb' => 'bb (comment)', }, + maxlength: 200, nulloption: true, null: false, multiple: false, # currently only "false" supported @@ -218,7 +221,24 @@ possible types data.delete(:to_create) data.delete(:to_migrate) data.delete(:to_delete) + data.delete(:to_config) end + + # if data_option has changed, store it for next migration + if !force + if record[:data_option] != data[:data_option] + + # do we need a database migration? + if record[:data_option][:maxlength] && data[:data_option][:maxlength] && record[:data_option][:maxlength].to_s != data[:data_option][:maxlength].to_s + data[:to_migrate] = true + end + + record[:data_option_new] = data[:data_option] + data.delete(:data_option) + data[:to_config] = true + end + end + # update attributes data.each {|key, value| record[key.to_sym] = value @@ -423,8 +443,11 @@ returns def self.discard_changes ObjectManager::Attribute.where('to_create = ?', true).each(&:destroy) - ObjectManager::Attribute.where('to_delete = ?', true).each {|attribute| + ObjectManager::Attribute.where('to_delete = ? OR to_config = ?', true, true).each {|attribute| + attribute.to_migrate = false attribute.to_delete = false + attribute.to_config = false + attribute.data_option_new = {} attribute.save } true @@ -460,7 +483,7 @@ returns =end def self.migrations - ObjectManager::Attribute.where('to_create = ? OR to_migrate = ? OR to_delete = ?', true, true, true) + ObjectManager::Attribute.where('to_create = ? OR to_migrate = ? OR to_delete = ? OR to_config = ?', true, true, true, true) end =begin @@ -482,7 +505,8 @@ to send no browser reload event, pass false def self.migration_execute(send_event = true) # check if field already exists - execute_count = 0 + execute_db_count = 0 + execute_config_count = 0 migrations.each {|attribute| model = Kernel.const_get(attribute.object_lookup.name) @@ -492,11 +516,21 @@ to send no browser reload event, pass false ActiveRecord::Migration.remove_column model.table_name, attribute.name reset_database_info(model) end - execute_count += 1 + execute_db_count += 1 attribute.destroy next end + # config changes + if attribute.to_config + execute_config_count += 1 + attribute.data_option = attribute.data_option_new + attribute.data_option_new = {} + attribute.to_config = false + attribute.save! + next if !attribute.to_create && !attribute.to_migrate && !attribute.to_delete + end + data_type = nil if attribute.data_type =~ /^input|select|richtext|textarea|checkbox$/ data_type = :string @@ -544,7 +578,7 @@ to send no browser reload event, pass false attribute.to_migrate = false attribute.save! reset_database_info(model) - execute_count += 1 + execute_db_count += 1 next end @@ -592,17 +626,21 @@ to send no browser reload event, pass false attribute.save! reset_database_info(model) - execute_count += 1 + execute_db_count += 1 } # sent maintenance message to clients - if send_event && execute_count != 0 - if ENV['APP_RESTART_CMD'] - AppVersion.set(true, 'restart_auto') - sleep 4 - Delayed::Job.enqueue(Observer::AppVersionRestartJob.new(ENV['APP_RESTART_CMD'])) - else - AppVersion.set(true, 'restart_manual') + if send_event + if execute_db_count != 0 + if ENV['APP_RESTART_CMD'] + AppVersion.set(true, 'restart_auto') + sleep 4 + Delayed::Job.enqueue(Observer::AppVersionRestartJob.new(ENV['APP_RESTART_CMD'])) + else + AppVersion.set(true, 'restart_manual') + end + elsif execute_config_count != 0 + AppVersion.set(true, 'config_changed') end end true @@ -652,7 +690,12 @@ to send no browser reload event, pass false # validate data_option if data_type == 'input' raise 'Need data_option[:type] param' if !data_option[:type] - raise "Invalid data_option[:type] param '#{data_option[:type]}'" if data_option[:type] !~ /^(text|password|phone|fax|email|url)$/ + raise "Invalid data_option[:type] param '#{data_option[:type]}'" if data_option[:type] !~ /^(text|password|tel|fax|email|url)$/ + raise 'Need data_option[:maxlength] param' if !data_option[:maxlength] + raise "Invalid data_option[:maxlength] param #{data_option[:maxlength]}" if data_option[:maxlength].to_s !~ /^\d+?$/ + end + + if data_type == 'richtext' raise 'Need data_option[:maxlength] param' if !data_option[:maxlength] raise "Invalid data_option[:maxlength] param #{data_option[:maxlength]}" if data_option[:maxlength].to_s !~ /^\d+?$/ end @@ -667,6 +710,9 @@ to send no browser reload event, pass false if data_type == 'select' || data_type == 'checkbox' raise 'Need data_option[:default] param' if !data_option.key?(:default) raise 'Invalid data_option[:options] or data_option[:relation] param' if data_option[:options].nil? && data_option[:relation].nil? + if !data_option.key?(:maxlength) + data_option[:maxlength] = 255 + end if !data_option.key?(:nulloption) data_option[:nulloption] = true end diff --git a/db/migrate/20120101000001_create_base.rb b/db/migrate/20120101000001_create_base.rb index 4b4fcab96..b588ef91f 100644 --- a/db/migrate/20120101000001_create_base.rb +++ b/db/migrate/20120101000001_create_base.rb @@ -467,12 +467,14 @@ class CreateBase < ActiveRecord::Migration t.column :display, :string, limit: 200, null: false t.column :data_type, :string, limit: 100, null: false t.column :data_option, :string, limit: 8000, null: true + t.column :data_option_new, :string, limit: 8000, null: true t.column :editable, :boolean, null: false, default: true t.column :active, :boolean, null: false, default: true t.column :screens, :string, limit: 2000, null: true t.column :to_create, :boolean, null: false, default: false t.column :to_migrate, :boolean, null: false, default: false t.column :to_delete, :boolean, null: false, default: false + t.column :to_config, :boolean, null: false, default: false t.column :position, :integer, null: false t.column :created_by_id, :integer, null: false t.column :updated_by_id, :integer, null: false diff --git a/db/migrate/20160529000001_update_object_manager2.rb b/db/migrate/20160529000001_update_object_manager2.rb new file mode 100644 index 000000000..5917ec0c9 --- /dev/null +++ b/db/migrate/20160529000001_update_object_manager2.rb @@ -0,0 +1,10 @@ +class UpdateObjectManager2 < ActiveRecord::Migration + def up + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + add_column :object_manager_attributes, :to_config, :boolean, null: false, default: false + add_column :object_manager_attributes, :data_option_new, :string, limit: 8000, null: true, default: false + + end +end diff --git a/test/browser/admin_object_manager_test.rb b/test/browser/admin_object_manager_test.rb index 99e21ef0c..bae55d9d7 100644 --- a/test/browser/admin_object_manager_test.rb +++ b/test/browser/admin_object_manager_test.rb @@ -120,8 +120,11 @@ class AdminObjectManagerTest < TestCase css: '#content', value: 'Database Update required', ) - click(css: '#content .tab-pane.active table tbody tr:last-child .js-delete') - sleep 4 + object_manager_attribute_delete( + data: { + name: 'browser_test1', + }, + ) watch_for( css: '#content', value: 'Database Update required', diff --git a/test/unit/object_manager_test.rb b/test/unit/object_manager_test.rb index 2d8040bdd..68bb42634 100644 --- a/test/unit/object_manager_test.rb +++ b/test/unit/object_manager_test.rb @@ -546,6 +546,70 @@ class ObjectManagerTest < ActiveSupport::TestCase assert_equal(Time.zone.parse('2016-05-12 00:59:59 UTC'), ticket2.attribute3) assert_equal(Date.parse('2016-05-11'), ticket2.attribute4) + # update data_option null -> to_config + attribute1 = ObjectManager::Attribute.add( + object: 'Ticket', + name: 'attribute1', + display: 'Attribute 1', + data_type: 'input', + data_option: { + maxlength: 200, + type: 'text', + null: false, + }, + active: true, + screens: {}, + position: 20, + created_by_id: 1, + updated_by_id: 1, + ) + assert(attribute1) + + assert_equal(true, ObjectManager::Attribute.pending_migration?) + assert_equal(0, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(1, ObjectManager::Attribute.where(to_config: true).count) + assert_equal(1, ObjectManager::Attribute.migrations.count) + + # execute migrations + assert(ObjectManager::Attribute.migration_execute) + + assert_equal(false, ObjectManager::Attribute.pending_migration?) + assert_equal(0, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(0, ObjectManager::Attribute.where(to_config: true).count) + assert_equal(0, ObjectManager::Attribute.migrations.count) + + # update data_option maxlength -> to_config && to_migrate + attribute1 = ObjectManager::Attribute.add( + object: 'Ticket', + name: 'attribute1', + display: 'Attribute 1', + data_type: 'input', + data_option: { + maxlength: 250, + type: 'text', + null: false, + }, + active: true, + screens: {}, + position: 20, + created_by_id: 1, + updated_by_id: 1, + ) + assert(attribute1) + + assert_equal(true, ObjectManager::Attribute.pending_migration?) + assert_equal(1, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(1, ObjectManager::Attribute.where(to_config: true).count) + assert_equal(1, ObjectManager::Attribute.migrations.count) + + # execute migrations + assert(ObjectManager::Attribute.migration_execute) + + assert_equal(false, ObjectManager::Attribute.pending_migration?) + assert_equal(0, ObjectManager::Attribute.where(to_migrate: true).count) + assert_equal(0, ObjectManager::Attribute.where(to_config: true).count) + assert_equal(0, ObjectManager::Attribute.migrations.count) + # remove attribute ObjectManager::Attribute.remove( object: 'Ticket',