Quellcode durchsuchen

refactor(db): add `Users.Update` (#7263)

Joe Chen vor 2 Jahren
Ursprung
Commit
13099a7e4f

+ 14 - 6
internal/db/email_addresses.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/pkg/errors"
 	"gorm.io/gorm"
 
 	"gogs.io/gogs/internal/errutil"
@@ -17,9 +18,11 @@ import (
 //
 // NOTE: All methods are sorted in alphabetical order.
 type EmailAddressesStore interface {
-	// GetByEmail returns the email address with given email. It may return
-	// unverified email addresses and returns ErrEmailNotExist when not found.
-	GetByEmail(ctx context.Context, email string) (*EmailAddress, error)
+	// GetByEmail returns the email address with given email. If `needsActivated` is
+	// true, only activated email will be returned, otherwise, it may return
+	// inactivated email addresses. It returns ErrEmailNotExist when no qualified
+	// email is not found.
+	GetByEmail(ctx context.Context, email string, needsActivated bool) (*EmailAddress, error)
 }
 
 var EmailAddresses EmailAddressesStore
@@ -43,7 +46,7 @@ type ErrEmailNotExist struct {
 }
 
 func IsErrEmailAddressNotExist(err error) bool {
-	_, ok := err.(ErrEmailNotExist)
+	_, ok := errors.Cause(err).(ErrEmailNotExist)
 	return ok
 }
 
@@ -55,9 +58,14 @@ func (ErrEmailNotExist) NotFound() bool {
 	return true
 }
 
-func (db *emailAddresses) GetByEmail(ctx context.Context, email string) (*EmailAddress, error) {
+func (db *emailAddresses) GetByEmail(ctx context.Context, email string, needsActivated bool) (*EmailAddress, error) {
+	tx := db.WithContext(ctx).Where("email = ?", email)
+	if needsActivated {
+		tx = tx.Where("is_activated = ?", true)
+	}
+
 	emailAddress := new(EmailAddress)
-	err := db.WithContext(ctx).Where("email = ?", email).First(emailAddress).Error
+	err := tx.First(emailAddress).Error
 	if err != nil {
 		if err == gorm.ErrRecordNotFound {
 			return nil, ErrEmailNotExist{

+ 14 - 3
internal/db/email_addresses_test.go

@@ -49,7 +49,7 @@ func emailAddressesGetByEmail(t *testing.T, db *emailAddresses) {
 	ctx := context.Background()
 
 	const testEmail = "[email protected]"
-	_, err := db.GetByEmail(ctx, testEmail)
+	_, err := db.GetByEmail(ctx, testEmail, false)
 	wantErr := ErrEmailNotExist{
 		args: errutil.Args{
 			"email": testEmail,
@@ -58,9 +58,20 @@ func emailAddressesGetByEmail(t *testing.T, db *emailAddresses) {
 	assert.Equal(t, wantErr, err)
 
 	// TODO: Use EmailAddresses.Create to replace SQL hack when the method is available.
-	err = db.Exec(`INSERT INTO email_address (uid, email) VALUES (1, ?)`, testEmail).Error
+	err = db.Exec(`INSERT INTO email_address (uid, email, is_activated) VALUES (1, ?, FALSE)`, testEmail).Error
 	require.NoError(t, err)
-	got, err := db.GetByEmail(ctx, testEmail)
+	got, err := db.GetByEmail(ctx, testEmail, false)
+	require.NoError(t, err)
+	assert.Equal(t, testEmail, got.Email)
+
+	// Should not return if we only want activated emails
+	_, err = db.GetByEmail(ctx, testEmail, true)
+	assert.Equal(t, wantErr, err)
+
+	// TODO: Use EmailAddresses.MarkActivated to replace SQL hack when the method is available.
+	err = db.Exec(`UPDATE email_address SET is_activated = TRUE WHERE email = ?`, testEmail).Error
+	require.NoError(t, err)
+	got, err = db.GetByEmail(ctx, testEmail, true)
 	require.NoError(t, err)
 	assert.Equal(t, testEmail, got.Email)
 }

+ 1 - 1
internal/db/org.go

@@ -106,7 +106,7 @@ func CreateOrganization(org, owner *User) (err error) {
 		return err
 	}
 
-	if Users.IsUsernameUsed(context.TODO(), org.Name) {
+	if Users.IsUsernameUsed(context.TODO(), org.Name, 0) {
 		return ErrUserAlreadyExist{
 			args: errutil.Args{
 				"name": org.Name,

+ 7 - 3
internal/db/user.go

@@ -21,6 +21,7 @@ import (
 	"gogs.io/gogs/internal/db/errors"
 	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/repoutil"
+	"gogs.io/gogs/internal/strutil"
 	"gogs.io/gogs/internal/tool"
 	"gogs.io/gogs/internal/userutil"
 )
@@ -41,6 +42,7 @@ func (u *User) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+// TODO(unknwon): Update call sites to use refactored methods and delete this one.
 func updateUser(e Engine, u *User) error {
 	// Organization does not need email
 	if !u.IsOrganization() {
@@ -59,9 +61,9 @@ func updateUser(e Engine, u *User) error {
 	}
 
 	u.LowerName = strings.ToLower(u.Name)
-	u.Location = tool.TruncateString(u.Location, 255)
-	u.Website = tool.TruncateString(u.Website, 255)
-	u.Description = tool.TruncateString(u.Description, 255)
+	u.Location = strutil.Truncate(u.Location, 255)
+	u.Website = strutil.Truncate(u.Website, 255)
+	u.Description = strutil.Truncate(u.Description, 255)
 
 	_, err := e.ID(u.ID).AllCols().Update(u)
 	return err
@@ -76,6 +78,8 @@ func (u *User) BeforeUpdate() {
 }
 
 // UpdateUser updates user's information.
+//
+// TODO(unknwon): Update call sites to use refactored methods and delete this one.
 func UpdateUser(u *User) error {
 	return updateUser(x, u)
 }

+ 45 - 13
internal/db/users.go

@@ -72,8 +72,10 @@ type UsersStore interface {
 	GetByUsername(ctx context.Context, username string) (*User, error)
 	// HasForkedRepository returns true if the user has forked given repository.
 	HasForkedRepository(ctx context.Context, userID, repoID int64) bool
-	// IsUsernameUsed returns true if the given username has been used.
-	IsUsernameUsed(ctx context.Context, username string) bool
+	// IsUsernameUsed returns true if the given username has been used other than
+	// the excluded user (a non-positive ID effectively meaning check against all
+	// users).
+	IsUsernameUsed(ctx context.Context, username string, excludeUserId int64) bool
 	// List returns a list of users. Results are paginated by given page and page
 	// size, and sorted by primary key (id) in ascending order.
 	List(ctx context.Context, page, pageSize int) ([]*User, error)
@@ -85,6 +87,9 @@ type UsersStore interface {
 	// Results are paginated by given page and page size, and sorted by the time of
 	// follow in descending order.
 	ListFollowings(ctx context.Context, userID int64, page, pageSize int) ([]*User, error)
+	// Update updates all fields for the given user, all values are persisted as-is
+	// (i.e. empty values would overwrite/wipe out existing values).
+	Update(ctx context.Context, userID int64, opts UpdateUserOptions) error
 	// UseCustomAvatar uses the given avatar as the user custom avatar.
 	UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error
 }
@@ -201,7 +206,7 @@ func (db *users) ChangeUsername(ctx context.Context, userID int64, newUsername s
 		return err
 	}
 
-	if db.IsUsernameUsed(ctx, newUsername) {
+	if db.IsUsernameUsed(ctx, newUsername, userID) {
 		return ErrUserAlreadyExist{
 			args: errutil.Args{
 				"name": newUsername,
@@ -226,6 +231,11 @@ func (db *users) ChangeUsername(ctx context.Context, userID int64, newUsername s
 			return errors.Wrap(err, "update user name")
 		}
 
+		// Stop here if it's just a case-change of the username
+		if strings.EqualFold(user.Name, newUsername) {
+			return nil
+		}
+
 		// Update all references to the user name in pull requests
 		err = tx.Model(&PullRequest{}).
 			Where("head_user_name = ?", user.LowerName).
@@ -328,7 +338,7 @@ func (db *users) Create(ctx context.Context, username, email string, opts Create
 		return nil, err
 	}
 
-	if db.IsUsernameUsed(ctx, username) {
+	if db.IsUsernameUsed(ctx, username, 0) {
 		return nil, ErrUserAlreadyExist{
 			args: errutil.Args{
 				"name": username,
@@ -428,18 +438,13 @@ func (db *users) GetByEmail(ctx context.Context, email string) (*User, error) {
 	}
 
 	// Otherwise, check activated email addresses
-	emailAddress := new(EmailAddress)
-	err = db.WithContext(ctx).
-		Where("email = ? AND is_activated = ?", email, true).
-		First(emailAddress).
-		Error
+	emailAddress, err := NewEmailAddressesStore(db.DB).GetByEmail(ctx, email, true)
 	if err != nil {
-		if err == gorm.ErrRecordNotFound {
+		if IsErrEmailAddressNotExist(err) {
 			return nil, ErrUserNotExist{args: errutil.Args{"email": email}}
 		}
 		return nil, err
 	}
-
 	return db.GetByID(ctx, emailAddress.UserID)
 }
 
@@ -473,13 +478,13 @@ func (db *users) HasForkedRepository(ctx context.Context, userID, repoID int64)
 	return count > 0
 }
 
-func (db *users) IsUsernameUsed(ctx context.Context, username string) bool {
+func (db *users) IsUsernameUsed(ctx context.Context, username string, excludeUserId int64) bool {
 	if username == "" {
 		return false
 	}
 	return db.WithContext(ctx).
 		Select("id").
-		Where("lower_name = ?", strings.ToLower(username)).
+		Where("lower_name = ? AND id != ?", strings.ToLower(username), excludeUserId).
 		First(&User{}).
 		Error != gorm.ErrRecordNotFound
 }
@@ -534,6 +539,33 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz
 		Error
 }
 
+type UpdateUserOptions struct {
+	FullName    string
+	Website     string
+	Location    string
+	Description string
+
+	MaxRepoCreation int
+}
+
+func (db *users) Update(ctx context.Context, userID int64, opts UpdateUserOptions) error {
+	if opts.MaxRepoCreation < -1 {
+		opts.MaxRepoCreation = -1
+	}
+	return db.WithContext(ctx).
+		Model(&User{}).
+		Where("id = ?", userID).
+		Updates(map[string]any{
+			"full_name":         strutil.Truncate(opts.FullName, 255),
+			"website":           strutil.Truncate(opts.Website, 255),
+			"location":          strutil.Truncate(opts.Location, 255),
+			"description":       strutil.Truncate(opts.Description, 255),
+			"max_repo_creation": opts.MaxRepoCreation,
+			"updated_unix":      db.NowFunc().Unix(),
+		}).
+		Error
+}
+
 func (db *users) UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error {
 	err := userutil.SaveAvatar(userID, avatar)
 	if err != nil {

+ 109 - 7
internal/db/users_test.go

@@ -104,6 +104,7 @@ func TestUsers(t *testing.T) {
 		{"List", usersList},
 		{"ListFollowers", usersListFollowers},
 		{"ListFollowings", usersListFollowings},
+		{"Update", usersUpdate},
 		{"UseCustomAvatar", usersUseCustomAvatar},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -241,10 +242,20 @@ func usersChangeUsername(t *testing.T, db *users) {
 	})
 
 	t.Run("name already exists", func(t *testing.T) {
-		err := db.ChangeUsername(ctx, alice.ID, alice.Name)
+		bob, err := db.Create(
+			ctx,
+			"bob",
+			"[email protected]",
+			CreateUserOptions{
+				Activated: true,
+			},
+		)
+		require.NoError(t, err)
+
+		err = db.ChangeUsername(ctx, alice.ID, bob.Name)
 		wantErr := ErrUserAlreadyExist{
 			args: errutil.Args{
-				"name": alice.Name,
+				"name": bob.Name,
 			},
 		}
 		assert.Equal(t, wantErr, err)
@@ -303,7 +314,7 @@ func usersChangeUsername(t *testing.T, db *users) {
 	assert.Equal(t, headUserName, alice.Name)
 
 	var updatedUnix int64
-	err = db.Model(&User{}).Select("updated_unix").Row().Scan(&updatedUnix)
+	err = db.Model(&User{}).Select("updated_unix").Where("id = ?", alice.ID).Row().Scan(&updatedUnix)
 	require.NoError(t, err)
 	assert.Equal(t, int64(0), updatedUnix)
 
@@ -329,6 +340,13 @@ func usersChangeUsername(t *testing.T, db *users) {
 	require.NoError(t, err)
 	assert.Equal(t, newUsername, alice.Name)
 	assert.Equal(t, db.NowFunc().Unix(), alice.UpdatedUnix)
+
+	// Change the cases of the username should just be fine
+	err = db.ChangeUsername(ctx, alice.ID, strings.ToUpper(newUsername))
+	require.NoError(t, err)
+	alice, err = db.GetByID(ctx, alice.ID)
+	require.NoError(t, err)
+	assert.Equal(t, strings.ToUpper(newUsername), alice.Name)
 }
 
 func usersCount(t *testing.T, db *users) {
@@ -561,10 +579,50 @@ func usersIsUsernameUsed(t *testing.T, db *users) {
 	alice, err := db.Create(ctx, "alice", "[email protected]", CreateUserOptions{})
 	require.NoError(t, err)
 
-	got := db.IsUsernameUsed(ctx, alice.Name)
-	assert.True(t, got)
-	got = db.IsUsernameUsed(ctx, "bob")
-	assert.False(t, got)
+	tests := []struct {
+		name          string
+		username      string
+		excludeUserID int64
+		want          bool
+	}{
+		{
+			name:          "no change",
+			username:      alice.Name,
+			excludeUserID: alice.ID,
+			want:          false,
+		},
+		{
+			name:          "change case",
+			username:      strings.ToUpper(alice.Name),
+			excludeUserID: alice.ID,
+			want:          false,
+		},
+		{
+			name:          "not used",
+			username:      "bob",
+			excludeUserID: alice.ID,
+			want:          false,
+		},
+		{
+			name:          "not used when not excluded",
+			username:      "bob",
+			excludeUserID: 0,
+			want:          false,
+		},
+
+		{
+			name:          "used when not excluded",
+			username:      alice.Name,
+			excludeUserID: 0,
+			want:          true,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := db.IsUsernameUsed(ctx, test.username, test.excludeUserID)
+			assert.Equal(t, test.want, got)
+		})
+	}
 }
 
 func usersList(t *testing.T, db *users) {
@@ -670,6 +728,50 @@ func usersListFollowings(t *testing.T, db *users) {
 	assert.Equal(t, alice.ID, got[0].ID)
 }
 
+func usersUpdate(t *testing.T, db *users) {
+	ctx := context.Background()
+
+	alice, err := db.Create(ctx, "alice", "[email protected]", CreateUserOptions{})
+	require.NoError(t, err)
+
+	overLimitStr := strings.Repeat("a", 300)
+	opts := UpdateUserOptions{
+		FullName:        overLimitStr,
+		Website:         overLimitStr,
+		Location:        overLimitStr,
+		Description:     overLimitStr,
+		MaxRepoCreation: 1,
+	}
+	err = db.Update(ctx, alice.ID, opts)
+	require.NoError(t, err)
+
+	alice, err = db.GetByID(ctx, alice.ID)
+	require.NoError(t, err)
+
+	wantStr := strings.Repeat("a", 255)
+	assert.Equal(t, wantStr, alice.FullName)
+	assert.Equal(t, wantStr, alice.Website)
+	assert.Equal(t, wantStr, alice.Location)
+	assert.Equal(t, wantStr, alice.Description)
+	assert.Equal(t, 1, alice.MaxRepoCreation)
+
+	// Test empty values
+	opts = UpdateUserOptions{
+		FullName: "Alice John",
+		Website:  "https://gogs.io",
+	}
+	err = db.Update(ctx, alice.ID, opts)
+	require.NoError(t, err)
+
+	alice, err = db.GetByID(ctx, alice.ID)
+	require.NoError(t, err)
+	assert.Equal(t, opts.FullName, alice.FullName)
+	assert.Equal(t, opts.Website, alice.Website)
+	assert.Empty(t, alice.Location)
+	assert.Empty(t, alice.Description)
+	assert.Empty(t, alice.MaxRepoCreation)
+}
+
 func usersUseCustomAvatar(t *testing.T, db *users) {
 	ctx := context.Background()
 

+ 0 - 1
internal/form/user.go

@@ -95,7 +95,6 @@ func (f *SignIn) Validate(ctx *macaron.Context, errs binding.Errors) binding.Err
 type UpdateProfile struct {
 	Name     string `binding:"Required;AlphaDashDot;MaxSize(35)"`
 	FullName string `binding:"MaxSize(100)"`
-	Email    string `binding:"Required;Email;MaxSize(254)"`
 	Website  string `binding:"Url;MaxSize(100)"`
 	Location string `binding:"MaxSize(50)"`
 }

+ 139 - 13
internal/route/lfs/mocks_test.go

@@ -2331,6 +2331,9 @@ type MockUsersStore struct {
 	// ListFollowingsFunc is an instance of a mock function object
 	// controlling the behavior of the method ListFollowings.
 	ListFollowingsFunc *UsersStoreListFollowingsFunc
+	// UpdateFunc is an instance of a mock function object controlling the
+	// behavior of the method Update.
+	UpdateFunc *UsersStoreUpdateFunc
 	// UseCustomAvatarFunc is an instance of a mock function object
 	// controlling the behavior of the method UseCustomAvatar.
 	UseCustomAvatarFunc *UsersStoreUseCustomAvatarFunc
@@ -2386,7 +2389,7 @@ func NewMockUsersStore() *MockUsersStore {
 			},
 		},
 		IsUsernameUsedFunc: &UsersStoreIsUsernameUsedFunc{
-			defaultHook: func(context.Context, string) (r0 bool) {
+			defaultHook: func(context.Context, string, int64) (r0 bool) {
 				return
 			},
 		},
@@ -2405,6 +2408,11 @@ func NewMockUsersStore() *MockUsersStore {
 				return
 			},
 		},
+		UpdateFunc: &UsersStoreUpdateFunc{
+			defaultHook: func(context.Context, int64, db.UpdateUserOptions) (r0 error) {
+				return
+			},
+		},
 		UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{
 			defaultHook: func(context.Context, int64, []byte) (r0 error) {
 				return
@@ -2463,7 +2471,7 @@ func NewStrictMockUsersStore() *MockUsersStore {
 			},
 		},
 		IsUsernameUsedFunc: &UsersStoreIsUsernameUsedFunc{
-			defaultHook: func(context.Context, string) bool {
+			defaultHook: func(context.Context, string, int64) bool {
 				panic("unexpected invocation of MockUsersStore.IsUsernameUsed")
 			},
 		},
@@ -2482,6 +2490,11 @@ func NewStrictMockUsersStore() *MockUsersStore {
 				panic("unexpected invocation of MockUsersStore.ListFollowings")
 			},
 		},
+		UpdateFunc: &UsersStoreUpdateFunc{
+			defaultHook: func(context.Context, int64, db.UpdateUserOptions) error {
+				panic("unexpected invocation of MockUsersStore.Update")
+			},
+		},
 		UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{
 			defaultHook: func(context.Context, int64, []byte) error {
 				panic("unexpected invocation of MockUsersStore.UseCustomAvatar")
@@ -2533,6 +2546,9 @@ func NewMockUsersStoreFrom(i db.UsersStore) *MockUsersStore {
 		ListFollowingsFunc: &UsersStoreListFollowingsFunc{
 			defaultHook: i.ListFollowings,
 		},
+		UpdateFunc: &UsersStoreUpdateFunc{
+			defaultHook: i.Update,
+		},
 		UseCustomAvatarFunc: &UsersStoreUseCustomAvatarFunc{
 			defaultHook: i.UseCustomAvatar,
 		},
@@ -3518,24 +3534,24 @@ func (c UsersStoreHasForkedRepositoryFuncCall) Results() []interface{} {
 // UsersStoreIsUsernameUsedFunc describes the behavior when the
 // IsUsernameUsed method of the parent MockUsersStore instance is invoked.
 type UsersStoreIsUsernameUsedFunc struct {
-	defaultHook func(context.Context, string) bool
-	hooks       []func(context.Context, string) bool
+	defaultHook func(context.Context, string, int64) bool
+	hooks       []func(context.Context, string, int64) bool
 	history     []UsersStoreIsUsernameUsedFuncCall
 	mutex       sync.Mutex
 }
 
 // IsUsernameUsed delegates to the next hook function in the queue and
 // stores the parameter and result values of this invocation.
-func (m *MockUsersStore) IsUsernameUsed(v0 context.Context, v1 string) bool {
-	r0 := m.IsUsernameUsedFunc.nextHook()(v0, v1)
-	m.IsUsernameUsedFunc.appendCall(UsersStoreIsUsernameUsedFuncCall{v0, v1, r0})
+func (m *MockUsersStore) IsUsernameUsed(v0 context.Context, v1 string, v2 int64) bool {
+	r0 := m.IsUsernameUsedFunc.nextHook()(v0, v1, v2)
+	m.IsUsernameUsedFunc.appendCall(UsersStoreIsUsernameUsedFuncCall{v0, v1, v2, r0})
 	return r0
 }
 
 // SetDefaultHook sets function that is called when the IsUsernameUsed
 // method of the parent MockUsersStore instance is invoked and the hook
 // queue is empty.
-func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context, string) bool) {
+func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context, string, int64) bool) {
 	f.defaultHook = hook
 }
 
@@ -3543,7 +3559,7 @@ func (f *UsersStoreIsUsernameUsedFunc) SetDefaultHook(hook func(context.Context,
 // IsUsernameUsed method of the parent MockUsersStore instance invokes the
 // hook at the front of the queue and discards it. After the queue is empty,
 // the default hook function is invoked for any future action.
-func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, string) bool) {
+func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, string, int64) bool) {
 	f.mutex.Lock()
 	f.hooks = append(f.hooks, hook)
 	f.mutex.Unlock()
@@ -3552,19 +3568,19 @@ func (f *UsersStoreIsUsernameUsedFunc) PushHook(hook func(context.Context, strin
 // SetDefaultReturn calls SetDefaultHook with a function that returns the
 // given values.
 func (f *UsersStoreIsUsernameUsedFunc) SetDefaultReturn(r0 bool) {
-	f.SetDefaultHook(func(context.Context, string) bool {
+	f.SetDefaultHook(func(context.Context, string, int64) bool {
 		return r0
 	})
 }
 
 // PushReturn calls PushHook with a function that returns the given values.
 func (f *UsersStoreIsUsernameUsedFunc) PushReturn(r0 bool) {
-	f.PushHook(func(context.Context, string) bool {
+	f.PushHook(func(context.Context, string, int64) bool {
 		return r0
 	})
 }
 
-func (f *UsersStoreIsUsernameUsedFunc) nextHook() func(context.Context, string) bool {
+func (f *UsersStoreIsUsernameUsedFunc) nextHook() func(context.Context, string, int64) bool {
 	f.mutex.Lock()
 	defer f.mutex.Unlock()
 
@@ -3603,6 +3619,9 @@ type UsersStoreIsUsernameUsedFuncCall struct {
 	// Arg1 is the value of the 2nd argument passed to this method
 	// invocation.
 	Arg1 string
+	// Arg2 is the value of the 3rd argument passed to this method
+	// invocation.
+	Arg2 int64
 	// Result0 is the value of the 1st result returned from this method
 	// invocation.
 	Result0 bool
@@ -3611,7 +3630,7 @@ type UsersStoreIsUsernameUsedFuncCall struct {
 // Args returns an interface slice containing the arguments of this
 // invocation.
 func (c UsersStoreIsUsernameUsedFuncCall) Args() []interface{} {
-	return []interface{}{c.Arg0, c.Arg1}
+	return []interface{}{c.Arg0, c.Arg1, c.Arg2}
 }
 
 // Results returns an interface slice containing the results of this
@@ -3958,6 +3977,113 @@ func (c UsersStoreListFollowingsFuncCall) Results() []interface{} {
 	return []interface{}{c.Result0, c.Result1}
 }
 
+// UsersStoreUpdateFunc describes the behavior when the Update method of the
+// parent MockUsersStore instance is invoked.
+type UsersStoreUpdateFunc struct {
+	defaultHook func(context.Context, int64, db.UpdateUserOptions) error
+	hooks       []func(context.Context, int64, db.UpdateUserOptions) error
+	history     []UsersStoreUpdateFuncCall
+	mutex       sync.Mutex
+}
+
+// Update delegates to the next hook function in the queue and stores the
+// parameter and result values of this invocation.
+func (m *MockUsersStore) Update(v0 context.Context, v1 int64, v2 db.UpdateUserOptions) error {
+	r0 := m.UpdateFunc.nextHook()(v0, v1, v2)
+	m.UpdateFunc.appendCall(UsersStoreUpdateFuncCall{v0, v1, v2, r0})
+	return r0
+}
+
+// SetDefaultHook sets function that is called when the Update method of the
+// parent MockUsersStore instance is invoked and the hook queue is empty.
+func (f *UsersStoreUpdateFunc) SetDefaultHook(hook func(context.Context, int64, db.UpdateUserOptions) error) {
+	f.defaultHook = hook
+}
+
+// PushHook adds a function to the end of hook queue. Each invocation of the
+// Update method of the parent MockUsersStore instance invokes the hook at
+// the front of the queue and discards it. After the queue is empty, the
+// default hook function is invoked for any future action.
+func (f *UsersStoreUpdateFunc) PushHook(hook func(context.Context, int64, db.UpdateUserOptions) error) {
+	f.mutex.Lock()
+	f.hooks = append(f.hooks, hook)
+	f.mutex.Unlock()
+}
+
+// SetDefaultReturn calls SetDefaultHook with a function that returns the
+// given values.
+func (f *UsersStoreUpdateFunc) SetDefaultReturn(r0 error) {
+	f.SetDefaultHook(func(context.Context, int64, db.UpdateUserOptions) error {
+		return r0
+	})
+}
+
+// PushReturn calls PushHook with a function that returns the given values.
+func (f *UsersStoreUpdateFunc) PushReturn(r0 error) {
+	f.PushHook(func(context.Context, int64, db.UpdateUserOptions) error {
+		return r0
+	})
+}
+
+func (f *UsersStoreUpdateFunc) nextHook() func(context.Context, int64, db.UpdateUserOptions) error {
+	f.mutex.Lock()
+	defer f.mutex.Unlock()
+
+	if len(f.hooks) == 0 {
+		return f.defaultHook
+	}
+
+	hook := f.hooks[0]
+	f.hooks = f.hooks[1:]
+	return hook
+}
+
+func (f *UsersStoreUpdateFunc) appendCall(r0 UsersStoreUpdateFuncCall) {
+	f.mutex.Lock()
+	f.history = append(f.history, r0)
+	f.mutex.Unlock()
+}
+
+// History returns a sequence of UsersStoreUpdateFuncCall objects describing
+// the invocations of this function.
+func (f *UsersStoreUpdateFunc) History() []UsersStoreUpdateFuncCall {
+	f.mutex.Lock()
+	history := make([]UsersStoreUpdateFuncCall, len(f.history))
+	copy(history, f.history)
+	f.mutex.Unlock()
+
+	return history
+}
+
+// UsersStoreUpdateFuncCall is an object that describes an invocation of
+// method Update on an instance of MockUsersStore.
+type UsersStoreUpdateFuncCall struct {
+	// Arg0 is the value of the 1st argument passed to this method
+	// invocation.
+	Arg0 context.Context
+	// Arg1 is the value of the 2nd argument passed to this method
+	// invocation.
+	Arg1 int64
+	// Arg2 is the value of the 3rd argument passed to this method
+	// invocation.
+	Arg2 db.UpdateUserOptions
+	// Result0 is the value of the 1st result returned from this method
+	// invocation.
+	Result0 error
+}
+
+// Args returns an interface slice containing the arguments of this
+// invocation.
+func (c UsersStoreUpdateFuncCall) Args() []interface{} {
+	return []interface{}{c.Arg0, c.Arg1, c.Arg2}
+}
+
+// Results returns an interface slice containing the results of this
+// invocation.
+func (c UsersStoreUpdateFuncCall) Results() []interface{} {
+	return []interface{}{c.Result0}
+}
+
 // UsersStoreUseCustomAvatarFunc describes the behavior when the
 // UseCustomAvatar method of the parent MockUsersStore instance is invoked.
 type UsersStoreUseCustomAvatarFunc struct {

+ 24 - 22
internal/route/org/setting.go

@@ -5,8 +5,6 @@
 package org
 
 import (
-	"strings"
-
 	"github.com/pkg/errors"
 	log "unknwon.dev/clog/v2"
 
@@ -40,43 +38,47 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) {
 
 	org := c.Org.Organization
 
-	// Check if organization name has been changed.
-	if org.LowerName != strings.ToLower(f.Name) {
-		if db.Users.IsUsernameUsed(c.Req.Context(), f.Name) {
-			c.Data["OrgName"] = true
-			c.RenderWithErr(c.Tr("form.username_been_taken"), SETTINGS_OPTIONS, &f)
-			return
-		} else if err := db.Users.ChangeUsername(c.Req.Context(), org.ID, f.Name); err != nil {
+	// Check if the organization username (including cases) had been changed
+	if org.Name != f.Name {
+		err := db.Users.ChangeUsername(c.Req.Context(), c.Org.Organization.ID, f.Name)
+		if err != nil {
 			c.Data["OrgName"] = true
+			var msg string
 			switch {
+			case db.IsErrUserAlreadyExist(errors.Cause(err)):
+				msg = c.Tr("form.username_been_taken")
 			case db.IsErrNameNotAllowed(errors.Cause(err)):
-				c.RenderWithErr(c.Tr("user.form.name_not_allowed", err.(db.ErrNameNotAllowed).Value()), SETTINGS_OPTIONS, &f)
+				msg = c.Tr("user.form.name_not_allowed", err.(db.ErrNameNotAllowed).Value())
 			default:
 				c.Error(err, "change organization name")
+				return
 			}
+
+			c.RenderWithErr(msg, SETTINGS_OPTIONS, &f)
 			return
 		}
+
 		// reset c.org.OrgLink with new name
 		c.Org.OrgLink = conf.Server.Subpath + "/org/" + f.Name
 		log.Trace("Organization name changed: %s -> %s", org.Name, f.Name)
 	}
-	// In case it's just a case change.
-	org.Name = f.Name
-	org.LowerName = strings.ToLower(f.Name)
 
+	opts := db.UpdateUserOptions{
+		FullName:        f.FullName,
+		Website:         f.Website,
+		Location:        f.Location,
+		Description:     f.Description,
+		MaxRepoCreation: org.MaxRepoCreation,
+	}
 	if c.User.IsAdmin {
-		org.MaxRepoCreation = f.MaxRepoCreation
+		opts.MaxRepoCreation = f.MaxRepoCreation
 	}
-
-	org.FullName = f.FullName
-	org.Description = f.Description
-	org.Website = f.Website
-	org.Location = f.Location
-	if err := db.UpdateUser(org); err != nil {
-		c.Error(err, "update user")
+	err := db.Users.Update(c.Req.Context(), c.User.ID, opts)
+	if err != nil {
+		c.Error(err, "update organization")
 		return
 	}
-	log.Trace("Organization setting updated: %s", org.Name)
+
 	c.Flash.Success(c.Tr("org.settings.update_setting_success"))
 	c.Redirect(c.Org.OrgLink + "/settings")
 }

+ 1 - 1
internal/route/repo/setting.go

@@ -225,7 +225,7 @@ func SettingsPost(c *context.Context, f form.RepoSetting) {
 		}
 
 		newOwner := c.Query("new_owner_name")
-		if !db.Users.IsUsernameUsed(c.Req.Context(), newOwner) {
+		if !db.Users.IsUsernameUsed(c.Req.Context(), newOwner, c.Repo.Owner.ID) {
 			c.RenderWithErr(c.Tr("form.enterred_invalid_owner_name"), SETTINGS_OPTIONS, nil)
 			return
 		}

+ 1 - 1
internal/route/user/auth.go

@@ -439,7 +439,7 @@ func verifyActiveEmailCode(code, email string) *db.EmailAddress {
 		data := com.ToStr(user.ID) + email + user.LowerName + user.Password + user.Rands
 
 		if tool.VerifyTimeLimitCode(data, minutes, prefix) {
-			emailAddress, err := db.EmailAddresses.GetByEmail(gocontext.TODO(), email)
+			emailAddress, err := db.EmailAddresses.GetByEmail(gocontext.TODO(), email, false)
 			if err == nil {
 				return emailAddress
 			}

+ 16 - 19
internal/route/user/setting.go

@@ -11,7 +11,6 @@ import (
 	"html/template"
 	"image/png"
 	"io"
-	"strings"
 
 	"github.com/pkg/errors"
 	"github.com/pquerna/otp"
@@ -69,9 +68,10 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) {
 
 	// Non-local users are not allowed to change their username
 	if c.User.IsLocal() {
-		// Check if username characters have been changed
-		if c.User.LowerName != strings.ToLower(f.Name) {
-			if err := db.Users.ChangeUsername(c.Req.Context(), c.User.ID, f.Name); err != nil {
+		// Check if the username (including cases) had been changed
+		if c.User.Name != f.Name {
+			err := db.Users.ChangeUsername(c.Req.Context(), c.User.ID, f.Name)
+			if err != nil {
 				c.FormErr("Name")
 				var msg string
 				switch {
@@ -90,23 +90,20 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) {
 
 			log.Trace("Username changed: %s -> %s", c.User.Name, f.Name)
 		}
-
-		// In case it's just a case change
-		c.User.Name = f.Name
-		c.User.LowerName = strings.ToLower(f.Name)
 	}
 
-	c.User.FullName = f.FullName
-	c.User.Email = f.Email
-	c.User.Website = f.Website
-	c.User.Location = f.Location
-	if err := db.UpdateUser(c.User); err != nil {
-		if db.IsErrEmailAlreadyUsed(err) {
-			msg := c.Tr("form.email_been_used")
-			c.RenderWithErr(msg, SETTINGS_PROFILE, &f)
-			return
-		}
-		c.Errorf(err, "update user")
+	err := db.Users.Update(
+		c.Req.Context(),
+		c.User.ID,
+		db.UpdateUserOptions{
+			FullName:        f.FullName,
+			Website:         f.Website,
+			Location:        f.Location,
+			MaxRepoCreation: c.User.MaxRepoCreation,
+		},
+	)
+	if err != nil {
+		c.Error(err, "update user")
 		return
 	}
 

+ 9 - 0
internal/strutil/strutil.go

@@ -54,3 +54,12 @@ func Ellipsis(str string, threshold int) string {
 	}
 	return str[:threshold] + "..."
 }
+
+// Truncate returns a truncated string if its length is over the limit.
+// Otherwise, it returns the original string.
+func Truncate(str string, limit int) string {
+	if len(str) < limit {
+		return str
+	}
+	return str[:limit]
+}

+ 40 - 0
internal/strutil/strutil_test.go

@@ -95,3 +95,43 @@ func TestEllipsis(t *testing.T) {
 		})
 	}
 }
+
+func TestTruncate(t *testing.T) {
+	tests := []struct {
+		name  string
+		str   string
+		limit int
+		want  string
+	}{
+		{
+			name:  "empty string with zero limit",
+			str:   "",
+			limit: 0,
+			want:  "",
+		},
+		{
+			name:  "smaller length than limit",
+			str:   "ab",
+			limit: 3,
+			want:  "ab",
+		},
+		{
+			name:  "same length as limit",
+			str:   "abc",
+			limit: 3,
+			want:  "abc",
+		},
+		{
+			name:  "greater length than limit",
+			str:   "ab",
+			limit: 1,
+			want:  "a",
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			got := Truncate(test.str, test.limit)
+			assert.Equal(t, test.want, got)
+		})
+	}
+}

+ 2 - 14
internal/tool/tool.go

@@ -5,7 +5,6 @@
 package tool
 
 import (
-	"crypto/md5"
 	"crypto/sha1"
 	"encoding/base64"
 	"encoding/hex"
@@ -23,6 +22,7 @@ import (
 	"github.com/gogs/chardet"
 
 	"gogs.io/gogs/internal/conf"
+	"gogs.io/gogs/internal/cryptoutil"
 )
 
 // ShortSHA1 truncates SHA1 string length to at most 10.
@@ -125,10 +125,7 @@ func CreateTimeLimitCode(data string, minutes int, startInf interface{}) string
 // HashEmail hashes email address to MD5 string.
 // https://en.gravatar.com/site/implement/hash/
 func HashEmail(email string) string {
-	email = strings.ToLower(strings.TrimSpace(email))
-	h := md5.New()
-	_, _ = h.Write([]byte(email))
-	return hex.EncodeToString(h.Sum(nil))
+	return cryptoutil.MD5(strings.ToLower(strings.TrimSpace(email)))
 }
 
 // AvatarLink returns relative avatar link to the site domain by given email,
@@ -358,15 +355,6 @@ func Subtract(left, right interface{}) interface{} {
 	}
 }
 
-// TruncateString returns a truncated string with given limit,
-// it returns input string if length is not reached limit.
-func TruncateString(str string, limit int) string {
-	if len(str) < limit {
-		return str
-	}
-	return str[:limit]
-}
-
 // StringsToInt64s converts a slice of string to a slice of int64.
 func StringsToInt64s(strs []string) []int64 {
 	ints := make([]int64, len(strs))

+ 0 - 4
templates/user/settings/profile.tmpl

@@ -23,10 +23,6 @@
 							<label for="full_name">{{.i18n.Tr "settings.full_name"}}</label>
 							<input id="full_name" name="full_name" value="{{.full_name}}">
 						</div>
-						<div class="required field {{if .Err_Email}}error{{end}}">
-							<label for="email">{{.i18n.Tr "email"}}</label>
-							<input id="email" name="email" value="{{.email}}" required>
-						</div>
 						<div class="field {{if .Err_Website}}error{{end}}">
 							<label for="website">{{.i18n.Tr "settings.website"}}</label>
 							<input id="website" name="website" type="url" value="{{.website}}">