Browse Source

Implemented issue #562 - Ticket creation - allow to create users without email, issue #181 - Allow customers without email addresses (was incoming phone call) and issue #247 telephone ticket without email.

Martin Edenhofer 7 years ago
parent
commit
77d3f8bf67

+ 35 - 0
app/assets/javascripts/app/models/_application_model.coffee

@@ -32,6 +32,10 @@ class App.Model extends Spine.Model
       return @title
     if @subject
       return @subject
+    if @phone
+      return @phone
+    if @login
+      return @login
     return '???'
 
   displayNameLong: ->
@@ -57,6 +61,12 @@ class App.Model extends Spine.Model
       return @email
     if @title
       return @title
+    if @subject
+      return @subject
+    if @phone
+      return @phone
+    if @login
+      return @login
     return '???'
 
   icon: (user) ->
@@ -165,6 +175,31 @@ class App.Model extends Spine.Model
 
   ###
 
+set new attributes of model (remove already available attributes)
+
+  App.Model.attributesSet(attributes)
+
+  ###
+
+  @attributesSet: (attributes) ->
+
+    configure_attributes = App[ @.className ].configure_attributes
+    attributesNew = []
+    for localAttribute in configure_attributes
+      found = false
+      for attribute in attributes
+        if attribute.name is localAttribute.name
+          found = true
+          break
+      if !found
+        attributesNew.push localAttribute
+    for attribute in attributes
+      App[@.className].attributes.push attribute.name
+      attributesNew.push attribute
+    App[ @.className ].configure_attributes = attributesNew
+
+  ###
+
   attributes = App.Model.attributesGet(optionalScreen, optionalAttributesList)
 
   returns

+ 12 - 6
app/controllers/users_controller.rb

@@ -126,6 +126,12 @@ class UsersController < ApplicationController
       if admin_account_exists && !params[:signup]
         raise Exceptions::UnprocessableEntity, 'Only signup with not authenticate user possible!'
       end
+
+      # check if user already exists
+      if clean_params[:email].blank?
+        raise Exceptions::UnprocessableEntity, 'Attribute \'email\' required!'
+      end
+
       user = User.new(clean_params)
       user.associations_from_param(params)
       user.updated_by_id = 1
@@ -166,7 +172,7 @@ class UsersController < ApplicationController
     end
 
     # check if user already exists
-    if !user.email.empty?
+    if user.email.present?
       exists = User.where(email: user.email.downcase).first
       raise Exceptions::UnprocessableEntity, 'User already exists!' if exists
     end
@@ -177,7 +183,7 @@ class UsersController < ApplicationController
       Setting.set('system_init_done', true)
 
       # fetch org logo
-      if !user.email.empty?
+      if user.email.present?
         Service::Image.organization_suggest(user.email)
       end
 
@@ -363,7 +369,7 @@ class UsersController < ApplicationController
       limit: params[:limit],
       current_user: current_user,
     }
-    if params[:role_ids] && !params[:role_ids].empty?
+    if params[:role_ids].present?
       query_params[:role_ids] = params[:role_ids]
     end
 
@@ -449,10 +455,10 @@ class UsersController < ApplicationController
     end
 
     # do query
-    user_all = if params[:role_ids] && !params[:role_ids].empty?
-                 User.joins(:roles).where( 'roles.id' => params[:role_ids] ).where('users.id != 1').order('users.created_at DESC').limit( params[:limit] || 20 )
+    user_all = if params[:role_ids].present?
+                 User.joins(:roles).where('roles.id' => params[:role_ids]).where('users.id != 1').order('users.created_at DESC').limit(params[:limit] || 20)
                else
-                 User.where('id != 1').order('created_at DESC').limit( params[:limit] || 20 )
+                 User.where('id != 1').order('created_at DESC').limit(params[:limit] || 20)
                end
 
     # build result list

+ 11 - 5
app/models/user.rb

@@ -38,7 +38,7 @@ class User < ApplicationModel
   load 'user/search_index.rb'
   include User::SearchIndex
 
-  before_validation :check_name, :check_email, :check_login, :ensure_password, :ensure_roles
+  before_validation :check_name, :check_email, :check_login, :ensure_password, :ensure_roles, :ensure_identifier
   before_create   :check_preferences_default, :validate_roles, :domain_based_assignment, :set_locale
   before_update   :check_preferences_default, :validate_roles, :reset_login_failed
   after_create    :avatar_for_email_check
@@ -867,9 +867,9 @@ returns
       end
     end
 
-    # if no email, complain about missing login
-    if id != 1 && login.blank?
-      raise Exceptions::UnprocessableEntity, 'Attribute \'login\' required!'
+    # generate auto login
+    if login.blank?
+      self.login = "auto-#{Time.zone.now.to_i}-#{rand(999_999)}"
     end
 
     # check if login already exists
@@ -878,7 +878,7 @@ returns
     while check
       exists = User.find_by(login: login)
       if exists && exists.id != id
-        self.login = login + rand(999).to_s
+        self.login = "#{login}#{rand(999)}"
       else
         check = false
       end
@@ -891,6 +891,12 @@ returns
     self.role_ids = Role.signup_role_ids
   end
 
+  def ensure_identifier
+    return true if email.present? || firstname.present? || lastname.present? || phone.present?
+    return true if login.present? && !login.start_with?('auto-')
+    raise Exceptions::UnprocessableEntity, 'Minimum one identifier (login, firstname, lastname, phone or email) for user is required.'
+  end
+
   def validate_roles
     return true if !role_ids
     role_ids.each { |role_id|

+ 54 - 0
db/migrate/20170714000001_object_manager_user_email_optional.rb

@@ -0,0 +1,54 @@
+class ObjectManagerUserEmailOptional < ActiveRecord::Migration
+  def up
+
+    # return if it's a new setup
+    return if !Setting.find_by(name: 'system_init_done')
+
+    ObjectManager::Attribute.add(
+      force: true,
+      object: 'User',
+      name: 'email',
+      display: 'Email',
+      data_type: 'input',
+      data_option: {
+        type: 'email',
+        maxlength: 150,
+        null: true,
+        item_class: 'formGroup--halfSize',
+      },
+      editable: false,
+      active: true,
+      screens: {
+        signup: {
+          '-all-' => {
+            null: false,
+          },
+        },
+        invite_agent: {
+          '-all-' => {
+            null: false,
+          },
+        },
+        invite_customer: {
+          '-all-' => {
+            null: false,
+          },
+        },
+        edit: {
+          '-all-' => {
+            null: true,
+          },
+        },
+        view: {
+          '-all-' => {
+            shown: true,
+          },
+        },
+      },
+      to_create: false,
+      to_migrate: false,
+      to_delete: false,
+      position: 400,
+    )
+  end
+end

+ 2 - 2
db/seeds/object_manager_attributes.rb

@@ -604,7 +604,7 @@ ObjectManager::Attribute.add(
   data_option: {
     type: 'email',
     maxlength: 150,
-    null: false,
+    null: true,
     item_class: 'formGroup--halfSize',
   },
   editable: false,
@@ -627,7 +627,7 @@ ObjectManager::Attribute.add(
     },
     edit: {
       '-all-' => {
-        null: false,
+        null: true,
       },
     },
     view: {

+ 23 - 7
test/controllers/user_organization_controller_test.rb

@@ -128,6 +128,14 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest
     assert(result['error'])
     assert_equal('User already exists!', result['error'])
 
+    # email missing with enabled feature
+    params = { firstname: 'some firstname', signup: true }
+    post '/api/v1/users', params.to_json, headers
+    assert_response(422)
+    result = JSON.parse(@response.body)
+    assert(result['error'])
+    assert_equal('Attribute \'email\' required!', result['error'])
+
     # create user with enabled feature (take customer role)
     params = { firstname: 'Me First', lastname: 'Me Last', email: 'new_here@example.com', signup: true }
     post '/api/v1/users', params.to_json, headers
@@ -330,22 +338,30 @@ class UserOrganizationControllerTest < ActionDispatch::IntegrationTest
     assert_response(422)
     result = JSON.parse(@response.body)
     assert(result)
-    assert_equal('Attribute \'login\' required!', result['error'])
+    assert_equal('Minimum one identifier (login, firstname, lastname, phone or email) for user is required.', result['error'])
 
-    params = { firstname: 'newfirstname123', note: 'some note' }
+    # invalid email
+    params = { firstname: 'newfirstname123', email: 'some_what', note: 'some note' }
     post '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
     assert_response(422)
     result = JSON.parse(@response.body)
     assert(result)
-    assert_equal('Attribute \'login\' required!', result['error'])
+    assert_equal('Invalid email', result['error'])
 
-    params = { firstname: 'newfirstname123', email: 'some_what', note: 'some note' }
+    # with valid attributes
+    params = { firstname: 'newfirstname123', note: 'some note' }
     post '/api/v1/users', params.to_json, @headers.merge('Authorization' => credentials)
-    assert_response(422)
+    assert_response(201)
     result = JSON.parse(@response.body)
     assert(result)
-    assert_equal('Invalid email', result['error'])
-
+    user = User.find(result['id'])
+    assert_not(user.role?('Admin'))
+    assert_not(user.role?('Agent'))
+    assert(user.role?('Customer'))
+    assert(result['login'].start_with?('auto-'))
+    assert_equal('', result['email'])
+    assert_equal('newfirstname123', result['firstname'])
+    assert_equal('', result['lastname'])
   end
 
   test 'user index and create with agent' do

+ 75 - 0
test/unit/user_test.rb

@@ -293,6 +293,81 @@ class UserTest < ActiveSupport::TestCase
     }
   end
 
+  test 'without email - but login eq email' do
+    name = rand(999_999_999)
+
+    login = "admin-role_without_email#{name}@example.com"
+    email = "admin-role_without_email#{name}@example.com"
+    admin = User.create_or_update(
+      login: login,
+      firstname: 'Role',
+      lastname: "Admin#{name}",
+      #email: "",
+      password: 'adminpw',
+      active: true,
+      roles: Role.where(name: %w(Admin Agent)),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    assert(admin.id)
+    assert_equal(admin.login, login)
+    assert_equal(admin.email, '')
+
+    admin.email = email
+    admin.save!
+
+    assert_equal(admin.login, login)
+    assert_equal(admin.email, email)
+
+    admin.email = ''
+    admin.save!
+
+    assert(admin.id)
+    assert(admin.login)
+    assert_not_equal(admin.login, login)
+    assert_equal(admin.email, '')
+
+    admin.destroy!
+  end
+
+  test 'without email - but login ne email' do
+    name = rand(999_999_999)
+
+    login = "admin-role_without_email#{name}"
+    email = "admin-role_without_email#{name}@example.com"
+    admin = User.create_or_update(
+      login: login,
+      firstname: 'Role',
+      lastname: "Admin#{name}",
+      #email: "",
+      password: 'adminpw',
+      active: true,
+      roles: Role.where(name: %w(Admin Agent)),
+      updated_by_id: 1,
+      created_by_id: 1,
+    )
+
+    assert(admin.id)
+    assert_equal(admin.login, login)
+    assert_equal(admin.email, '')
+
+    admin.email = email
+    admin.save!
+
+    assert_equal(admin.login, login)
+    assert_equal(admin.email, email)
+
+    admin.email = ''
+    admin.save!
+
+    assert(admin.id)
+    assert_equal(admin.login, login)
+    assert_equal(admin.email, '')
+
+    admin.destroy!
+  end
+
   test 'ensure roles' do
     name = rand(999_999_999)