From 28fe4fa80be73ac897dfa84dbe5e9ebf8f6199c2 Mon Sep 17 00:00:00 2001 From: Martin Edenhofer Date: Thu, 29 Sep 2016 13:25:20 +0200 Subject: [PATCH] Update user with default preferences based on permissions. --- app/models/user.rb | 54 +++++++++---- config/application.rb | 6 +- .../20160920000001_user_preferences_update.rb | 9 +++ test/unit/user_test.rb | 81 ++++++++++--------- 4 files changed, 96 insertions(+), 54 deletions(-) create mode 100644 db/migrate/20160920000001_user_preferences_update.rb diff --git a/app/models/user.rb b/app/models/user.rb index dabdc7f41..96e2c16d3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -656,7 +656,7 @@ returns update/sync default preferences of users in a dedecated role - result = User.update_default_preferences('Agent') + result = User.update_default_preferences('Agent', force) returns @@ -664,35 +664,61 @@ returns =end - def self.update_default_preferences(role_name) + def self.update_default_preferences(role_name, force = false) role = Role.lookup(name: role_name) - User.of_role(role_name).each { |user| - user.check_notifications(role) - user.check_preferences_default - user.save + default = Rails.configuration.preferences_default_by_permission + return false if !default + default.deep_stringify_keys! + role.permissions.each { |permission| + User.with_permissions(permission.name).each { |user| + next if !default[permission.name] + has_changed = false + default[permission.name].each { |key, value| + next if !force && user.preferences[key] + has_changed = true + user.preferences[key] = value + } + if has_changed + user.save! + end + } } true end def check_notifications(o) - default = Rails.configuration.preferences_default_by_role + default = Rails.configuration.preferences_default_by_permission return if !default default.deep_stringify_keys! - return if !default[o.name] - if !@preferences_default - @preferences_default = {} - end - default[o.name].each { |key, value| - next if @preferences_default[key] - @preferences_default[key] = value + has_changed = false + o.permissions.each { |permission| + next if !default[permission.name] + default[permission.name].each { |key, value| + next if preferences[key] + preferences[key] = value + has_changed = true + } } + + return true if !has_changed + + if id + save! + return true + end + + @preferences_default = preferences + true end def check_preferences_default return if !@preferences_default return if @preferences_default.empty? + preferences_tmp = @preferences_default.merge(preferences) self.preferences = preferences_tmp + @preferences_default = nil + true end private diff --git a/config/application.rb b/config/application.rb index 05dc172b3..5b8a77bc5 100644 --- a/config/application.rb +++ b/config/application.rb @@ -48,9 +48,9 @@ module Zammad # define cache store config.cache_store = :file_store, "#{Rails.root}/tmp/cache_file_store_#{Rails.env}" - # default preferences by role - config.preferences_default_by_role = { - Agent: { + # default preferences by permission + config.preferences_default_by_permission = { + 'ticket.agent' => { notification_config: { matrix: { create: { diff --git a/db/migrate/20160920000001_user_preferences_update.rb b/db/migrate/20160920000001_user_preferences_update.rb new file mode 100644 index 000000000..7dbf636d5 --- /dev/null +++ b/db/migrate/20160920000001_user_preferences_update.rb @@ -0,0 +1,9 @@ + +class UserPreferencesUpdate < ActiveRecord::Migration + def up + # return if it's a new setup + return if !Setting.find_by(name: 'system_init_done') + + User.update_default_preferences('Agent') + end +end diff --git a/test/unit/user_test.rb b/test/unit/user_test.rb index b15dbd160..e193f6f4b 100644 --- a/test/unit/user_test.rb +++ b/test/unit/user_test.rb @@ -289,13 +289,14 @@ class UserTest < ActiveSupport::TestCase end test 'user default preferences' do + name = rand(999_999_999) groups = Group.where(name: 'Users') roles = Role.where(name: 'Agent') agent1 = User.create_or_update( - login: 'agent-default-preferences1@example.com', + login: "agent-default-preferences#{name}@example.com", firstname: 'Preferences', - lastname: 'Agent1', - email: 'agent-default-preferences1@example.com', + lastname: "Agent#{name}", + email: "agent-default-preferences#{name}@example.com", password: 'agentpw', active: true, roles: roles, @@ -306,6 +307,7 @@ class UserTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) + agent1 = User.find(agent1.id) assert(agent1.preferences) assert(agent1.preferences['locale']) assert_equal(agent1.preferences['locale'], 'de-de') @@ -316,10 +318,10 @@ class UserTest < ActiveSupport::TestCase roles = Role.where(name: 'Customer') customer1 = User.create_or_update( - login: 'customer-default-preferences1@example.com', + login: "customer-default-preferences#{name}@example.com", firstname: 'Preferences', - lastname: 'Customer1', - email: 'customer-default-preferences1@example.com', + lastname: "Customer#{name}", + email: "customer-default-preferences#{name}@example.com", password: 'customerpw', active: true, roles: roles, @@ -329,13 +331,16 @@ class UserTest < ActiveSupport::TestCase updated_by_id: 1, created_by_id: 1, ) + customer1 = User.find(customer1.id) assert(customer1.preferences) assert(customer1.preferences['locale']) assert_equal(customer1.preferences['locale'], 'de-de') assert_not(customer1.preferences['notification_config']) + customer1 = User.find(customer1.id) customer1.roles = Role.where(name: 'Agent') - customer1.save + customer1 = User.find(customer1.id) + assert(customer1.preferences) assert(customer1.preferences['locale']) assert_equal(customer1.preferences['locale'], 'de-de') @@ -380,12 +385,13 @@ class UserTest < ActiveSupport::TestCase created_by_id: 1, updated_by_id: 1, ) + name = rand(999_999_999) assert_raises(RuntimeError) { User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_1, test_role_3], @@ -395,10 +401,10 @@ class UserTest < ActiveSupport::TestCase } assert_raises(RuntimeError) { User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_2, test_role_3], @@ -407,10 +413,10 @@ class UserTest < ActiveSupport::TestCase ) } user1 = User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_1, test_role_2], @@ -422,10 +428,10 @@ class UserTest < ActiveSupport::TestCase assert_not(user1.role_ids.include?(test_role_3.id)) assert_not(user1.role_ids.include?(test_role_4.id)) user1 = User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_1, test_role_4], @@ -438,10 +444,10 @@ class UserTest < ActiveSupport::TestCase assert(user1.role_ids.include?(test_role_4.id)) assert_raises(RuntimeError) { User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_1, test_role_3], @@ -451,10 +457,10 @@ class UserTest < ActiveSupport::TestCase } assert_raises(RuntimeError) { User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: [test_role_2, test_role_3], @@ -470,36 +476,37 @@ class UserTest < ActiveSupport::TestCase end test 'permission default' do - admin_count = User.of_role('Admin').count + name = rand(999_999_999) + admin_count = User.with_permissions('admin').count admin = User.create_or_update( - login: 'admin-role1@example.com', + login: "admin-role#{name}@example.com", firstname: 'Role', - lastname: 'Admin1', - email: 'admin-role1@example.com', + lastname: "Admin#{name}", + email: "admin-role#{name}@example.com", password: 'adminpw', active: true, roles: Role.where(name: %w(Admin Agent)), updated_by_id: 1, created_by_id: 1, ) - agent_count = User.of_role('Agent').count + agent_count = User.with_permissions('ticket.agent').count agent = User.create_or_update( - login: 'agent-role1@example.com', + login: "agent-role#{name}@example.com", firstname: 'Role', - lastname: 'Agent1', - email: 'agent-role1@example.com', + lastname: "Agent#{name}", + email: "agent-role#{name}@example.com", password: 'agentpw', active: true, roles: Role.where(name: 'Agent'), updated_by_id: 1, created_by_id: 1, ) - customer_count = User.of_role('Customer').count + customer_count = User.with_permissions('ticket.customer').count customer = User.create_or_update( - login: 'customer-role1@example.com', + login: "customer-role#{name}@example.com", firstname: 'Role', - lastname: 'Customer1', - email: 'customer-role1@example.com', + lastname: "Customer#{name}", + email: "customer-role#{name}@example.com", password: 'customerpw', active: true, roles: Role.where(name: 'Customer'),