Unify user update methods (#28733)

Fixes #28660
Fixes an admin api bug related to `user.LoginSource`
Fixed `/user/emails` response not identical to GitHub api

This PR unifies the user update methods. The goal is to keep the logic
only at one place (having audit logs in mind). For example, do the
password checks only in one method not everywhere a password is updated.

After that PR is merged, the user creation should be next.

7 files changed, 105 insertions, 397 deletions

diff --git a/models/fixtures/email_address.yml b/models/fixtures/email_address.yml
index ce4d5208df..67a99f43e2 100644
--- a/models/fixtures/email_address.yml
+++ b/models/fixtures/email_address.yml
@@ -285,3 +285,11 @@
   lower_email: abcde@gitea.com
   is_activated: true
   is_primary: false
+
+-
+  id: 37
+  uid: 37
+  email: user37@example.com
+  lower_email: user37@example.com
+  is_activated: true
+  is_primary: true
diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml
index 79fbb981f6..aa0daedd85 100644
--- a/models/fixtures/user.yml
+++ b/models/fixtures/user.yml
@@ -1095,7 +1095,7 @@
   allow_git_hook: false
   allow_import_local: false
   allow_create_organization: true
-  prohibit_login: true
+  prohibit_login: false
   avatar: avatar29
   avatar_email: user30@example.com
   use_custom_avatar: false
@@ -1332,3 +1332,40 @@
   repo_admin_change_team_access: false
   theme: ""
   keep_activity_private: false
+
+-
+  id: 37
+  lower_name: user37
+  name: user37
+  full_name: User 37
+  email: user37@example.com
+  keep_email_private: false
+  email_notifications_preference: enabled
+  passwd: ZogKvWdyEx:password
+  passwd_hash_algo: dummy
+  must_change_password: false
+  login_source: 0
+  login_name: user37
+  type: 0
+  salt: ZogKvWdyEx
+  max_repo_creation: -1
+  is_active: true
+  is_admin: false
+  is_restricted: false
+  allow_git_hook: false
+  allow_import_local: false
+  allow_create_organization: true
+  prohibit_login: true
+  avatar: avatar29
+  avatar_email: user37@example.com
+  use_custom_avatar: false
+  num_followers: 0
+  num_following: 0
+  num_stars: 0
+  num_repos: 0
+  num_teams: 0
+  num_members: 0
+  visibility: 0
+  repo_admin_change_team_access: false
+  theme: ""
+  keep_activity_private: false
diff --git a/models/user/email_address.go b/models/user/email_address.go
index 2af2621f5f..957e72fe89 100644
--- a/models/user/email_address.go
+++ b/models/user/email_address.go
@@ -142,12 +142,24 @@ func (email *EmailAddress) BeforeInsert() {
 	}
 }
 
+func InsertEmailAddress(ctx context.Context, email *EmailAddress) (*EmailAddress, error) {
+	if err := db.Insert(ctx, email); err != nil {
+		return nil, err
+	}
+	return email, nil
+}
+
+func UpdateEmailAddress(ctx context.Context, email *EmailAddress) error {
+	_, err := db.GetEngine(ctx).ID(email.ID).AllCols().Update(email)
+	return err
+}
+
 var emailRegexp = regexp.MustCompile("^[a-zA-Z0-9.!#$%&'*+-/=?^_`{|}~]*@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$")
 
 // ValidateEmail check if email is a allowed address
 func ValidateEmail(email string) error {
 	if len(email) == 0 {
-		return nil
+		return ErrEmailInvalid{email}
 	}
 
 	if !emailRegexp.MatchString(email) {
@@ -177,6 +189,36 @@ func ValidateEmail(email string) error {
 	return nil
 }
 
+func GetEmailAddressByEmail(ctx context.Context, email string) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{email}
+	}
+	return ea, nil
+}
+
+func GetEmailAddressOfUser(ctx context.Context, email string, uid int64) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("lower_email=? AND uid=?", strings.ToLower(email), uid).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{email}
+	}
+	return ea, nil
+}
+
+func GetPrimaryEmailAddressOfUser(ctx context.Context, uid int64) (*EmailAddress, error) {
+	ea := &EmailAddress{}
+	if has, err := db.GetEngine(ctx).Where("uid=? AND is_primary=?", uid, true).Get(ea); err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrEmailAddressNotExist{}
+	}
+	return ea, nil
+}
+
 // GetEmailAddresses returns all email addresses belongs to given user.
 func GetEmailAddresses(ctx context.Context, uid int64) ([]*EmailAddress, error) {
 	emails := make([]*EmailAddress, 0, 5)
@@ -235,91 +277,6 @@ func IsEmailUsed(ctx context.Context, email string) (bool, error) {
 	return db.GetEngine(ctx).Where("lower_email=?", strings.ToLower(email)).Get(&EmailAddress{})
 }
 
-// AddEmailAddress adds an email address to given user.
-func AddEmailAddress(ctx context.Context, email *EmailAddress) error {
-	email.Email = strings.TrimSpace(email.Email)
-	used, err := IsEmailUsed(ctx, email.Email)
-	if err != nil {
-		return err
-	} else if used {
-		return ErrEmailAlreadyUsed{email.Email}
-	}
-
-	if err = ValidateEmail(email.Email); err != nil {
-		return err
-	}
-
-	return db.Insert(ctx, email)
-}
-
-// AddEmailAddresses adds an email address to given user.
-func AddEmailAddresses(ctx context.Context, emails []*EmailAddress) error {
-	if len(emails) == 0 {
-		return nil
-	}
-
-	// Check if any of them has been used
-	for i := range emails {
-		emails[i].Email = strings.TrimSpace(emails[i].Email)
-		used, err := IsEmailUsed(ctx, emails[i].Email)
-		if err != nil {
-			return err
-		} else if used {
-			return ErrEmailAlreadyUsed{emails[i].Email}
-		}
-		if err = ValidateEmail(emails[i].Email); err != nil {
-			return err
-		}
-	}
-
-	if err := db.Insert(ctx, emails); err != nil {
-		return fmt.Errorf("Insert: %w", err)
-	}
-
-	return nil
-}
-
-// DeleteEmailAddress deletes an email address of given user.
-func DeleteEmailAddress(ctx context.Context, email *EmailAddress) (err error) {
-	if email.IsPrimary {
-		return ErrPrimaryEmailCannotDelete{Email: email.Email}
-	}
-
-	var deleted int64
-	// ask to check UID
-	address := EmailAddress{
-		UID: email.UID,
-	}
-	if email.ID > 0 {
-		deleted, err = db.GetEngine(ctx).ID(email.ID).Delete(&address)
-	} else {
-		if email.Email != "" && email.LowerEmail == "" {
-			email.LowerEmail = strings.ToLower(email.Email)
-		}
-		deleted, err = db.GetEngine(ctx).
-			Where("lower_email=?", email.LowerEmail).
-			Delete(&address)
-	}
-
-	if err != nil {
-		return err
-	} else if deleted != 1 {
-		return ErrEmailAddressNotExist{Email: email.Email}
-	}
-	return nil
-}
-
-// DeleteEmailAddresses deletes multiple email addresses
-func DeleteEmailAddresses(ctx context.Context, emails []*EmailAddress) (err error) {
-	for i := range emails {
-		if err = DeleteEmailAddress(ctx, emails[i]); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // DeleteInactiveEmailAddresses deletes inactive email addresses
 func DeleteInactiveEmailAddresses(ctx context.Context) error {
 	_, err := db.GetEngine(ctx).
diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go
index 7f3ca75cfd..140443f82f 100644
--- a/models/user/email_address_test.go
+++ b/models/user/email_address_test.go
@@ -42,96 +42,6 @@ func TestIsEmailUsed(t *testing.T) {
 	assert.False(t, isExist)
 }
 
-func TestAddEmailAddress(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	assert.NoError(t, user_model.AddEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		Email:       "user1234567890@example.com",
-		LowerEmail:  "user1234567890@example.com",
-		IsPrimary:   true,
-		IsActivated: true,
-	}))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.AddEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		Email:      "user1234567890@example.com",
-		LowerEmail: "user1234567890@example.com",
-	})
-	assert.Error(t, err)
-	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
-}
-
-func TestAddEmailAddresses(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	// insert multiple email address
-	emails := make([]*user_model.EmailAddress, 2)
-	emails[0] = &user_model.EmailAddress{
-		Email:       "user1234@example.com",
-		LowerEmail:  "user1234@example.com",
-		IsActivated: true,
-	}
-	emails[1] = &user_model.EmailAddress{
-		Email:       "user5678@example.com",
-		LowerEmail:  "user5678@example.com",
-		IsActivated: true,
-	}
-	assert.NoError(t, user_model.AddEmailAddresses(db.DefaultContext, emails))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.AddEmailAddresses(db.DefaultContext, emails)
-	assert.Error(t, err)
-	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
-}
-
-func TestDeleteEmailAddress(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	assert.NoError(t, user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		ID:         int64(33),
-		Email:      "user1-2@example.com",
-		LowerEmail: "user1-2@example.com",
-	}))
-
-	assert.NoError(t, user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		Email:      "user1-3@example.com",
-		LowerEmail: "user1-3@example.com",
-	}))
-
-	// Email address does not exist
-	err := user_model.DeleteEmailAddress(db.DefaultContext, &user_model.EmailAddress{
-		UID:        int64(1),
-		Email:      "user1234567890@example.com",
-		LowerEmail: "user1234567890@example.com",
-	})
-	assert.Error(t, err)
-}
-
-func TestDeleteEmailAddresses(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-
-	// delete multiple email address
-	emails := make([]*user_model.EmailAddress, 2)
-	emails[0] = &user_model.EmailAddress{
-		UID:        int64(2),
-		ID:         int64(3),
-		Email:      "user2@example.com",
-		LowerEmail: "user2@example.com",
-	}
-	emails[1] = &user_model.EmailAddress{
-		UID:        int64(2),
-		Email:      "user2-2@example.com",
-		LowerEmail: "user2-2@example.com",
-	}
-	assert.NoError(t, user_model.DeleteEmailAddresses(db.DefaultContext, emails))
-
-	// ErrEmailAlreadyUsed
-	err := user_model.DeleteEmailAddresses(db.DefaultContext, emails)
-	assert.Error(t, err)
-}
-
 func TestMakeEmailPrimary(t *testing.T) {
 	assert.NoError(t, unittest.PrepareTestDatabase())
 
diff --git a/models/user/error.go b/models/user/error.go
index f512994169..ef572c178a 100644
--- a/models/user/error.go
+++ b/models/user/error.go
@@ -108,18 +108,3 @@ func IsErrUserIsNotLocal(err error) bool {
 	_, ok := err.(ErrUserIsNotLocal)
 	return ok
 }
-
-type ErrUsernameNotChanged struct {
-	UID  int64
-	Name string
-}
-
-func (err ErrUsernameNotChanged) Error() string {
-	return fmt.Sprintf("username hasn't been changed[uid: %d, name: %s]", err.UID, err.Name)
-}
-
-// IsErrUsernameNotChanged
-func IsErrUsernameNotChanged(err error) bool {
-	_, ok := err.(ErrUsernameNotChanged)
-	return ok
-}
diff --git a/models/user/user.go b/models/user/user.go
index 269a1be725..e5245dfbb0 100644
--- a/models/user/user.go
+++ b/models/user/user.go
@@ -196,18 +196,6 @@ func (u *User) SetLastLogin() {
 	u.LastLoginUnix = timeutil.TimeStampNow()
 }
 
-// UpdateUserDiffViewStyle updates the users diff view style
-func UpdateUserDiffViewStyle(ctx context.Context, u *User, style string) error {
-	u.DiffViewStyle = style
-	return UpdateUserCols(ctx, u, "diff_view_style")
-}
-
-// UpdateUserTheme updates a users' theme irrespective of the site wide theme
-func UpdateUserTheme(ctx context.Context, u *User, themeName string) error {
-	u.Theme = themeName
-	return UpdateUserCols(ctx, u, "theme")
-}
-
 // GetPlaceholderEmail returns an noreply email
 func (u *User) GetPlaceholderEmail() string {
 	return fmt.Sprintf("%s@%s", u.LowerName, setting.Service.NoReplyAddress)
@@ -378,13 +366,6 @@ func (u *User) NewGitSig() *git.Signature {
 // SetPassword hashes a password using the algorithm defined in the config value of PASSWORD_HASH_ALGO
 // change passwd, salt and passwd_hash_algo fields
 func (u *User) SetPassword(passwd string) (err error) {
-	if len(passwd) == 0 {
-		u.Passwd = ""
-		u.Salt = ""
-		u.PasswdHashAlgo = ""
-		return nil
-	}
-
 	if u.Salt, err = GetUserSalt(); err != nil {
 		return err
 	}
@@ -488,21 +469,6 @@ func (u *User) IsMailable() bool {
 	return u.IsActive
 }
 
-// EmailNotifications returns the User's email notification preference
-func (u *User) EmailNotifications() string {
-	return u.EmailNotificationsPreference
-}
-
-// SetEmailNotifications sets the user's email notification preference
-func SetEmailNotifications(ctx context.Context, u *User, set string) error {
-	u.EmailNotificationsPreference = set
-	if err := UpdateUserCols(ctx, u, "email_notifications_preference"); err != nil {
-		log.Error("SetEmailNotifications: %v", err)
-		return err
-	}
-	return nil
-}
-
 // IsUserExist checks if given user name exist,
 // the user name should be noncased unique.
 // If uid is presented, then check will rule out that one,
@@ -705,8 +671,13 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
 	if u.Rands, err = GetUserSalt(); err != nil {
 		return err
 	}
-	if err = u.SetPassword(u.Passwd); err != nil {
-		return err
+	if u.Passwd != "" {
+		if err = u.SetPassword(u.Passwd); err != nil {
+			return err
+		}
+	} else {
+		u.Salt = ""
+		u.PasswdHashAlgo = ""
 	}
 
 	// save changes to database
@@ -817,24 +788,6 @@ func VerifyUserActiveCode(ctx context.Context, code string) (user *User) {
 	return nil
 }
 
-// checkDupEmail checks whether there are the same email with the user
-func checkDupEmail(ctx context.Context, u *User) error {
-	u.Email = strings.ToLower(u.Email)
-	has, err := db.GetEngine(ctx).
-		Where("id!=?", u.ID).
-		And("type=?", u.Type).
-		And("email=?", u.Email).
-		Get(new(User))
-	if err != nil {
-		return err
-	} else if has {
-		return ErrEmailAlreadyUsed{
-			Email: u.Email,
-		}
-	}
-	return nil
-}
-
 // ValidateUser check if user is valid to insert / update into database
 func ValidateUser(u *User, cols ...string) error {
 	if len(cols) == 0 || util.SliceContainsString(cols, "visibility", true) {
@@ -843,81 +796,9 @@ func ValidateUser(u *User, cols ...string) error {
 		}
 	}
 
-	if len(cols) == 0 || util.SliceContainsString(cols, "email", true) {
-		u.Email = strings.ToLower(u.Email)
-		if err := ValidateEmail(u.Email); err != nil {
-			return err
-		}
-	}
 	return nil
 }
 
-// UpdateUser updates user's information.
-func UpdateUser(ctx context.Context, u *User, changePrimaryEmail bool, cols ...string) error {
-	err := ValidateUser(u, cols...)
-	if err != nil {
-		return err
-	}
-
-	e := db.GetEngine(ctx)
-
-	if changePrimaryEmail {
-		var emailAddress EmailAddress
-		has, err := e.Where("lower_email=?", strings.ToLower(u.Email)).Get(&emailAddress)
-		if err != nil {
-			return err
-		}
-		if has && emailAddress.UID != u.ID {
-			return ErrEmailAlreadyUsed{
-				Email: u.Email,
-			}
-		}
-		// 1. Update old primary email
-		if _, err = e.Where("uid=? AND is_primary=?", u.ID, true).Cols("is_primary").Update(&EmailAddress{
-			IsPrimary: false,
-		}); err != nil {
-			return err
-		}
-
-		if !has {
-			emailAddress.Email = u.Email
-			emailAddress.UID = u.ID
-			emailAddress.IsActivated = true
-			emailAddress.IsPrimary = true
-			if _, err := e.Insert(&emailAddress); err != nil {
-				return err
-			}
-		} else if _, err := e.ID(emailAddress.ID).Cols("is_primary").Update(&EmailAddress{
-			IsPrimary: true,
-		}); err != nil {
-			return err
-		}
-	} else if !u.IsOrganization() { // check if primary email in email_address table
-		primaryEmailExist, err := e.Where("uid=? AND is_primary=?", u.ID, true).Exist(&EmailAddress{})
-		if err != nil {
-			return err
-		}
-
-		if !primaryEmailExist {
-			if _, err := e.Insert(&EmailAddress{
-				Email:       u.Email,
-				UID:         u.ID,
-				IsActivated: true,
-				IsPrimary:   true,
-			}); err != nil {
-				return err
-			}
-		}
-	}
-
-	if len(cols) == 0 {
-		_, err = e.ID(u.ID).AllCols().Update(u)
-	} else {
-		_, err = e.ID(u.ID).Cols(cols...).Update(u)
-	}
-	return err
-}
-
 // UpdateUserCols update user according special columns
 func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
 	if err := ValidateUser(u, cols...); err != nil {
@@ -928,25 +809,6 @@ func UpdateUserCols(ctx context.Context, u *User, cols ...string) error {
 	return err
 }
 
-// UpdateUserSetting updates user's settings.
-func UpdateUserSetting(ctx context.Context, u *User) (err error) {
-	ctx, committer, err := db.TxContext(ctx)
-	if err != nil {
-		return err
-	}
-	defer committer.Close()
-
-	if !u.IsOrganization() {
-		if err = checkDupEmail(ctx, u); err != nil {
-			return err
-		}
-	}
-	if err = UpdateUser(ctx, u, false); err != nil {
-		return err
-	}
-	return committer.Commit()
-}
-
 // GetInactiveUsers gets all inactive users
 func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, error) {
 	var cond builder.Cond = builder.Eq{"is_active": false}
@@ -1044,7 +906,7 @@ func GetUserEmailsByNames(ctx context.Context, names []string) []string {
 		if err != nil {
 			continue
 		}
-		if u.IsMailable() && u.EmailNotifications() != EmailNotificationsDisabled {
+		if u.IsMailable() && u.EmailNotificationsPreference != EmailNotificationsDisabled {
 			mails = append(mails, u.Email)
 		}
 	}
diff --git a/models/user/user_test.go b/models/user/user_test.go
index 65aebea43a..f3e5a95b1e 100644
--- a/models/user/user_test.go
+++ b/models/user/user_test.go
@@ -101,13 +101,13 @@ func TestSearchUsers(t *testing.T) {
 	}
 
 	testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
-		[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34})
+		[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolFalse},
 		[]int64{9})
 
 	testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
-		[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34})
+		[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37})
 
 	testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue},
 		[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
@@ -123,7 +123,7 @@ func TestSearchUsers(t *testing.T) {
 		[]int64{29})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: util.OptionalBoolTrue},
-		[]int64{30})
+		[]int64{37})
 
 	testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: util.OptionalBoolTrue},
 		[]int64{24})
@@ -147,20 +147,7 @@ func TestEmailNotificationPreferences(t *testing.T) {
 		{user_model.EmailNotificationsOnMention, 9},
 	} {
 		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: test.userID})
-		assert.Equal(t, test.expected, user.EmailNotifications())
-
-		// Try all possible settings
-		assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsEnabled))
-		assert.Equal(t, user_model.EmailNotificationsEnabled, user.EmailNotifications())
-
-		assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsOnMention))
-		assert.Equal(t, user_model.EmailNotificationsOnMention, user.EmailNotifications())
-
-		assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsDisabled))
-		assert.Equal(t, user_model.EmailNotificationsDisabled, user.EmailNotifications())
-
-		assert.NoError(t, user_model.SetEmailNotifications(db.DefaultContext, user, user_model.EmailNotificationsAndYourOwn))
-		assert.Equal(t, user_model.EmailNotificationsAndYourOwn, user.EmailNotifications())
+		assert.Equal(t, test.expected, user.EmailNotificationsPreference)
 	}
 }
 
@@ -343,42 +330,6 @@ func TestGetMaileableUsersByIDs(t *testing.T) {
 	}
 }
 
-func TestUpdateUser(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-
-	user.KeepActivityPrivate = true
-	assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, false))
-	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	assert.True(t, user.KeepActivityPrivate)
-
-	setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, false}
-	user.KeepActivityPrivate = false
-	user.Visibility = structs.VisibleTypePrivate
-	assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, false))
-	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	assert.True(t, user.KeepActivityPrivate)
-
-	newEmail := "new_" + user.Email
-	user.Email = newEmail
-	assert.NoError(t, user_model.UpdateUser(db.DefaultContext, user, true))
-	user = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	assert.Equal(t, newEmail, user.Email)
-
-	user.Email = "no mail@mail.org"
-	assert.Error(t, user_model.UpdateUser(db.DefaultContext, user, true))
-}
-
-func TestUpdateUserEmailAlreadyUsed(t *testing.T) {
-	assert.NoError(t, unittest.PrepareTestDatabase())
-	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
-	org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
-
-	user2.Email = org3.Email
-	err := user_model.UpdateUser(db.DefaultContext, user2, true)
-	assert.True(t, user_model.IsErrEmailAlreadyUsed(err))
-}
-
 func TestNewUserRedirect(t *testing.T) {
 	// redirect to a completely new name
 	assert.NoError(t, unittest.PrepareTestDatabase())
@@ -534,14 +485,12 @@ func Test_ValidateUser(t *testing.T) {
 	}()
 	setting.Service.AllowedUserVisibilityModesSlice = []bool{true, false, true}
 	kases := map[*user_model.User]bool{
-		{ID: 1, Visibility: structs.VisibleTypePublic}:                            true,
-		{ID: 2, Visibility: structs.VisibleTypeLimited}:                           false,
-		{ID: 2, Visibility: structs.VisibleTypeLimited, Email: "invalid"}:         false,
-		{ID: 2, Visibility: structs.VisibleTypePrivate, Email: "valid@valid.com"}: true,
+		{ID: 1, Visibility: structs.VisibleTypePublic}:  true,
+		{ID: 2, Visibility: structs.VisibleTypeLimited}: false,
+		{ID: 2, Visibility: structs.VisibleTypePrivate}: true,
 	}
 	for kase, expected := range kases {
-		err := user_model.ValidateUser(kase)
-		assert.EqualValues(t, expected, err == nil, fmt.Sprintf("case: %+v", kase))
+		assert.EqualValues(t, expected, nil == user_model.ValidateUser(kase), fmt.Sprintf("case: %+v", kase))
 	}
 }