refactor(db): migrate `UpdateUser` off `user.go` (#7267)

CHANGELOG.md

@@ -27,6 +27,7 @@ All notable changes to Gogs are documented in this file.
 ### Fixed
 
 - Unable to use LDAP authentication on ARM machines. [#6761](https://github.com/gogs/gogs/issues/6761)
+- Unable to choose "Lookup Avatar by mail" in user settings without deleting custom avatar. [#7267](https://github.com/gogs/gogs/pull/7267)
 
 ### Removed
 

internal/db/repo.go

@@ -34,6 +34,7 @@ import (
 	"gogs.io/gogs/internal/avatar"
 	"gogs.io/gogs/internal/conf"
 	dberrors "gogs.io/gogs/internal/db/errors"
+	"gogs.io/gogs/internal/dbutil"
 	"gogs.io/gogs/internal/errutil"
 	"gogs.io/gogs/internal/markup"
 	"gogs.io/gogs/internal/osutil"
@@ -1112,11 +1113,9 @@ func createRepository(e *xorm.Session, doer, owner *User, repo *Repository) (err
 		return err
 	}
 
-	owner.NumRepos++
-	// Remember visibility preference.
-	owner.LastRepoVisibility = repo.IsPrivate
-	if err = updateUser(e, owner); err != nil {
-		return fmt.Errorf("updateUser: %v", err)
+	_, err = e.Exec(dbutil.Quote("UPDATE %s SET num_repos = num_repos + 1 WHERE id = ?", "user"), owner.ID)
+	if err != nil {
+		return errors.Wrap(err, "increase owned repository count")
 	}
 
 	// Give access to all members in owner team.
@@ -1222,8 +1221,17 @@ func CreateRepository(doer, owner *User, opts CreateRepoOptionsLegacy) (_ *Repos
 			return nil, fmt.Errorf("CreateRepository 'git update-server-info': %s", stderr)
 		}
 	}
+	if err = sess.Commit(); err != nil {
+		return nil, err
+	}
 
-	return repo, sess.Commit()
+	// Remember visibility preference
+	err = Users.Update(context.TODO(), owner.ID, UpdateUserOptions{LastRepoVisibility: &repo.IsPrivate})
+	if err != nil {
+		return nil, errors.Wrap(err, "update user")
+	}
+
+	return repo, nil
 }
 
 func countRepositories(userID int64, private bool) int64 {
@@ -2544,6 +2552,12 @@ func ForkRepository(doer, owner *User, baseRepo *Repository, name, desc string)
 		return nil, fmt.Errorf("Commit: %v", err)
 	}
 
+	// Remember visibility preference
+	err = Users.Update(context.TODO(), owner.ID, UpdateUserOptions{LastRepoVisibility: &repo.IsPrivate})
+	if err != nil {
+		return nil, errors.Wrap(err, "update user")
+	}
+
 	if err = repo.UpdateSize(); err != nil {
 		log.Error("UpdateSize [repo_id: %d]: %v", repo.ID, err)
 	}

internal/db/user.go

@@ -19,10 +19,7 @@ import (
 
 	"gogs.io/gogs/internal/conf"
 	"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"
 )
 
@@ -42,48 +39,6 @@ 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() {
-		u.Email = strings.ToLower(u.Email)
-		has, err := e.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{args: errutil.Args{"email": u.Email}}
-		}
-
-		if u.AvatarEmail == "" {
-			u.AvatarEmail = u.Email
-		}
-		u.Avatar = tool.HashEmail(u.AvatarEmail)
-	}
-
-	u.LowerName = strings.ToLower(u.Name)
-	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
-}
-
-// TODO(unknwon): Refactoring together with methods that do updates.
-func (u *User) BeforeUpdate() {
-	if u.MaxRepoCreation < -1 {
-		u.MaxRepoCreation = -1
-	}
-	u.UpdatedUnix = time.Now().Unix()
-}
-
-// 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)
-}
-
 // deleteBeans deletes all given beans, beans should contain delete conditions.
 func deleteBeans(e Engine, beans ...interface{}) (err error) {
 	for i := range beans {

internal/db/user_mail.go

@@ -11,7 +11,6 @@ import (
 
 	"gogs.io/gogs/internal/db/errors"
 	"gogs.io/gogs/internal/errutil"
-	"gogs.io/gogs/internal/userutil"
 )
 
 // EmailAddresses is the list of all email addresses of a user. Can contain the
@@ -120,28 +119,11 @@ func AddEmailAddresses(emails []*EmailAddress) error {
 }
 
 func (email *EmailAddress) Activate() error {
-	user, err := Users.GetByID(context.TODO(), email.UserID)
-	if err != nil {
-		return err
-	}
-	if user.Rands, err = userutil.RandomSalt(); err != nil {
-		return err
-	}
-
-	sess := x.NewSession()
-	defer sess.Close()
-	if err = sess.Begin(); err != nil {
-		return err
-	}
-
 	email.IsActivated = true
-	if _, err := sess.ID(email.ID).AllCols().Update(email); err != nil {
-		return err
-	} else if err = updateUser(sess, user); err != nil {
+	if _, err := x.ID(email.ID).AllCols().Update(email); err != nil {
 		return err
 	}
-
-	return sess.Commit()
+	return Users.Update(context.TODO(), email.UserID, UpdateUserOptions{GenerateNewRands: true})
 }
 
 func DeleteEmailAddress(email *EmailAddress) (err error) {

internal/db/users.go

@@ -87,8 +87,7 @@ 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 updates fields for the given user.
 	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
@@ -324,8 +323,10 @@ type ErrEmailAlreadyUsed struct {
 	args errutil.Args
 }
 
+// IsErrEmailAlreadyUsed returns true if the underlying error has the type
+// ErrEmailAlreadyUsed.
 func IsErrEmailAlreadyUsed(err error) bool {
-	_, ok := err.(ErrEmailAlreadyUsed)
+	_, ok := errors.Cause(err).(ErrEmailAlreadyUsed)
 	return ok
 }
 
@@ -415,8 +416,10 @@ type ErrUserNotExist struct {
 	args errutil.Args
 }
 
+// IsErrUserNotExist returns true if the underlying error has the type
+// ErrUserNotExist.
 func IsErrUserNotExist(err error) bool {
-	_, ok := err.(ErrUserNotExist)
+	_, ok := errors.Cause(err).(ErrUserNotExist)
 	return ok
 }
 
@@ -549,30 +552,117 @@ func (db *users) ListFollowings(ctx context.Context, userID int64, page, pageSiz
 }
 
 type UpdateUserOptions struct {
-	FullName    string
-	Website     string
-	Location    string
-	Description string
+	LoginSource *int64
+	LoginName   *string
 
-	MaxRepoCreation int
+	Password *string
+	// GenerateNewRands indicates whether to force generate new rands for the user.
+	GenerateNewRands bool
+
+	FullName    *string
+	Email       *string
+	Website     *string
+	Location    *string
+	Description *string
+
+	MaxRepoCreation    *int
+	LastRepoVisibility *bool
+
+	IsActivated      *bool
+	IsAdmin          *bool
+	AllowGitHook     *bool
+	AllowImportLocal *bool
+	ProhibitLogin    *bool
+
+	Avatar      *string
+	AvatarEmail *string
 }
 
 func (db *users) Update(ctx context.Context, userID int64, opts UpdateUserOptions) error {
-	if opts.MaxRepoCreation < -1 {
-		opts.MaxRepoCreation = -1
+	updates := map[string]any{
+		"updated_unix": db.NowFunc().Unix(),
 	}
-	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
+
+	if opts.LoginSource != nil {
+		updates["login_source"] = *opts.LoginSource
+	}
+	if opts.LoginName != nil {
+		updates["login_name"] = *opts.LoginName
+	}
+
+	if opts.Password != nil {
+		salt, err := userutil.RandomSalt()
+		if err != nil {
+			return errors.Wrap(err, "generate salt")
+		}
+		updates["salt"] = salt
+		updates["passwd"] = userutil.EncodePassword(*opts.Password, salt)
+		opts.GenerateNewRands = true
+	}
+	if opts.GenerateNewRands {
+		rands, err := userutil.RandomSalt()
+		if err != nil {
+			return errors.Wrap(err, "generate rands")
+		}
+		updates["rands"] = rands
+	}
+
+	if opts.FullName != nil {
+		updates["full_name"] = strutil.Truncate(*opts.FullName, 255)
+	}
+	if opts.Email != nil {
+		_, err := db.GetByEmail(ctx, *opts.Email)
+		if err == nil {
+			return ErrEmailAlreadyUsed{args: errutil.Args{"email": *opts.Email}}
+		} else if !IsErrUserNotExist(err) {
+			return errors.Wrap(err, "check email")
+		}
+		updates["email"] = *opts.Email
+	}
+	if opts.Website != nil {
+		updates["website"] = strutil.Truncate(*opts.Website, 255)
+	}
+	if opts.Location != nil {
+		updates["location"] = strutil.Truncate(*opts.Location, 255)
+	}
+	if opts.Description != nil {
+		updates["description"] = strutil.Truncate(*opts.Description, 255)
+	}
+
+	if opts.MaxRepoCreation != nil {
+		if *opts.MaxRepoCreation < -1 {
+			*opts.MaxRepoCreation = -1
+		}
+		updates["max_repo_creation"] = *opts.MaxRepoCreation
+	}
+	if opts.LastRepoVisibility != nil {
+		updates["last_repo_visibility"] = *opts.LastRepoVisibility
+	}
+
+	if opts.IsActivated != nil {
+		updates["is_active"] = *opts.IsActivated
+	}
+	if opts.IsAdmin != nil {
+		updates["is_admin"] = *opts.IsAdmin
+	}
+	if opts.AllowGitHook != nil {
+		updates["allow_git_hook"] = *opts.AllowGitHook
+	}
+	if opts.AllowImportLocal != nil {
+		updates["allow_import_local"] = *opts.AllowImportLocal
+	}
+	if opts.ProhibitLogin != nil {
+		updates["prohibit_login"] = *opts.ProhibitLogin
+	}
+
+	if opts.Avatar != nil {
+		updates["avatar"] = strutil.Truncate(*opts.Avatar, 2048)
+	}
+	if opts.AvatarEmail != nil {
+		updates["avatar_email"] = strutil.Truncate(*opts.AvatarEmail, 255)
+	}
+
+	return db.WithContext(ctx).Model(&User{}).Where("id = ?", userID).Updates(updates).Error
 }
 
 func (db *users) UseCustomAvatar(ctx context.Context, userID int64, avatar []byte) error {

internal/db/users_test.go

@@ -731,16 +731,69 @@ func usersListFollowings(t *testing.T, db *users) {
 func usersUpdate(t *testing.T, db *users) {
 	ctx := context.Background()
 
-	alice, err := db.Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
+	const oldPassword = "Password"
+	alice, err := db.Create(
+		ctx,
+		"alice",
+		"alice@example.com",
+		CreateUserOptions{
+			FullName:    "FullName",
+			Password:    oldPassword,
+			LoginSource: 9,
+			LoginName:   "LoginName",
+			Location:    "Location",
+			Website:     "Website",
+			Activated:   false,
+			Admin:       false,
+		},
+	)
 	require.NoError(t, err)
 
-	overLimitStr := strings.Repeat("a", 300)
+	t.Run("update password", func(t *testing.T) {
+		got := userutil.ValidatePassword(alice.Password, alice.Salt, oldPassword)
+		require.True(t, got)
+
+		newPassword := "NewPassword"
+		err = db.Update(ctx, alice.ID, UpdateUserOptions{Password: &newPassword})
+		require.NoError(t, err)
+		alice, err = db.GetByID(ctx, alice.ID)
+		require.NoError(t, err)
+
+		got = userutil.ValidatePassword(alice.Password, alice.Salt, oldPassword)
+		assert.False(t, got, "Old password should stop working")
+
+		got = userutil.ValidatePassword(alice.Password, alice.Salt, newPassword)
+		assert.True(t, got, "New password should work")
+	})
+
+	t.Run("update email but already used", func(t *testing.T) {
+		// todo
+	})
+
+	loginSource := int64(1)
+	maxRepoCreation := 99
+	lastRepoVisibility := true
+	overLimitStr := strings.Repeat("a", 2050)
 	opts := UpdateUserOptions{
-		FullName:        overLimitStr,
-		Website:         overLimitStr,
-		Location:        overLimitStr,
-		Description:     overLimitStr,
-		MaxRepoCreation: 1,
+		LoginSource: &loginSource,
+		LoginName:   &alice.Name,
+
+		FullName:    &overLimitStr,
+		Website:     &overLimitStr,
+		Location:    &overLimitStr,
+		Description: &overLimitStr,
+
+		MaxRepoCreation:    &maxRepoCreation,
+		LastRepoVisibility: &lastRepoVisibility,
+
+		IsActivated:      &lastRepoVisibility,
+		IsAdmin:          &lastRepoVisibility,
+		AllowGitHook:     &lastRepoVisibility,
+		AllowImportLocal: &lastRepoVisibility,
+		ProhibitLogin:    &lastRepoVisibility,
+
+		Avatar:      &overLimitStr,
+		AvatarEmail: &overLimitStr,
 	}
 	err = db.Update(ctx, alice.ID, opts)
 	require.NoError(t, err)
@@ -748,28 +801,34 @@ func usersUpdate(t *testing.T, db *users) {
 	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",
+	assertValues := func() {
+		assert.Equal(t, loginSource, alice.LoginSource)
+		assert.Equal(t, alice.Name, alice.LoginName)
+		wantStr255 := strings.Repeat("a", 255)
+		assert.Equal(t, wantStr255, alice.FullName)
+		assert.Equal(t, wantStr255, alice.Website)
+		assert.Equal(t, wantStr255, alice.Location)
+		assert.Equal(t, wantStr255, alice.Description)
+		assert.Equal(t, maxRepoCreation, alice.MaxRepoCreation)
+		assert.Equal(t, lastRepoVisibility, alice.LastRepoVisibility)
+		assert.Equal(t, lastRepoVisibility, alice.IsActive)
+		assert.Equal(t, lastRepoVisibility, alice.IsAdmin)
+		assert.Equal(t, lastRepoVisibility, alice.AllowGitHook)
+		assert.Equal(t, lastRepoVisibility, alice.AllowImportLocal)
+		assert.Equal(t, lastRepoVisibility, alice.ProhibitLogin)
+		wantStr2048 := strings.Repeat("a", 2048)
+		assert.Equal(t, wantStr2048, alice.Avatar)
+		assert.Equal(t, wantStr255, alice.AvatarEmail)
 	}
-	err = db.Update(ctx, alice.ID, opts)
+	assertValues()
+
+	// Test ignored values
+	err = db.Update(ctx, alice.ID, UpdateUserOptions{})
 	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)
+	assertValues()
 }
 
 func usersUseCustomAvatar(t *testing.T, db *users) {

internal/email/message.go

@@ -232,6 +232,10 @@ func NewContext() {
 // It returns without confirmation (mail processed asynchronously) in normal cases,
 // but waits/blocks under hook mode to make sure mail has been sent.
 func Send(msg *Message) {
+	if !conf.Email.Enabled {
+		return
+	}
+
 	mailQueue <- msg
 
 	if conf.HookMode {

internal/form/user.go

@@ -104,8 +104,8 @@ func (f *UpdateProfile) Validate(ctx *macaron.Context, errs binding.Errors) bind
 }
 
 const (
-	AVATAR_LOCAL  string = "local"
-	AVATAR_BYMAIL string = "bymail"
+	AvatarLocal  string = "local"
+	AvatarLookup string = "lookup"
 )
 
 type Avatar struct {

internal/route/admin/users.go

@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"strings"
 
-	"github.com/unknwon/com"
 	log "unknwon.dev/clog/v2"
 
 	"gogs.io/gogs/internal/conf"
@@ -17,7 +16,6 @@ import (
 	"gogs.io/gogs/internal/email"
 	"gogs.io/gogs/internal/form"
 	"gogs.io/gogs/internal/route"
-	"gogs.io/gogs/internal/userutil"
 )
 
 const (
@@ -176,38 +174,37 @@ func EditUserPost(c *context.Context, f form.AdminEditUser) {
 		return
 	}
 
+	opts := db.UpdateUserOptions{
+		LoginName:        &f.LoginName,
+		FullName:         &f.FullName,
+		Website:          &f.Website,
+		Location:         &f.Location,
+		MaxRepoCreation:  &f.MaxRepoCreation,
+		IsActivated:      &f.Active,
+		IsAdmin:          &f.Admin,
+		AllowGitHook:     &f.AllowGitHook,
+		AllowImportLocal: &f.AllowImportLocal,
+		ProhibitLogin:    &f.ProhibitLogin,
+	}
+
 	fields := strings.Split(f.LoginType, "-")
 	if len(fields) == 2 {
-		loginSource := com.StrTo(fields[1]).MustInt64()
-
+		loginSource, _ := strconv.ParseInt(fields[1], 10, 64)
 		if u.LoginSource != loginSource {
-			u.LoginSource = loginSource
+			opts.LoginSource = &loginSource
 		}
 	}
 
-	if len(f.Password) > 0 {
-		u.Password = f.Password
-		var err error
-		if u.Salt, err = userutil.RandomSalt(); err != nil {
-			c.Error(err, "get user salt")
-			return
-		}
-		u.Password = userutil.EncodePassword(u.Password, u.Salt)
+	if f.Password != "" {
+		opts.Password = &f.Password
 	}
 
-	u.LoginName = f.LoginName
-	u.FullName = f.FullName
-	u.Email = f.Email
-	u.Website = f.Website
-	u.Location = f.Location
-	u.MaxRepoCreation = f.MaxRepoCreation
-	u.IsActive = f.Active
-	u.IsAdmin = f.Admin
-	u.AllowGitHook = f.AllowGitHook
-	u.AllowImportLocal = f.AllowImportLocal
-	u.ProhibitLogin = f.ProhibitLogin
+	if u.Email != f.Email {
+		opts.Email = &f.Email
+	}
 
-	if err := db.UpdateUser(u); err != nil {
+	err := db.Users.Update(c.Req.Context(), u.ID, opts)
+	if err != nil {
 		if db.IsErrEmailAlreadyUsed(err) {
 			c.Data["Err_Email"] = true
 			c.RenderWithErr(c.Tr("form.email_been_used"), USER_EDIT, &f)
@@ -216,7 +213,7 @@ func EditUserPost(c *context.Context, f form.AdminEditUser) {
 		}
 		return
 	}
-	log.Trace("Account profile updated by admin (%s): %s", c.User.Name, u.Name)
+	log.Trace("Account updated by admin %q: %s", c.User.Name, u.Name)
 
 	c.Flash.Success(c.Tr("admin.users.update_profile_success"))
 	c.Redirect(conf.Server.Subpath + "/admin/users/" + c.Params(":userid"))

internal/route/api/v1/admin/user.go

@@ -15,7 +15,6 @@ import (
 	"gogs.io/gogs/internal/db"
 	"gogs.io/gogs/internal/email"
 	"gogs.io/gogs/internal/route/api/v1/user"
-	"gogs.io/gogs/internal/userutil"
 )
 
 func parseLoginSource(c *context.APIContext, sourceID int64) {
@@ -83,39 +82,30 @@ func EditUser(c *context.APIContext, form api.EditUserOption) {
 		return
 	}
 
-	if len(form.Password) > 0 {
-		u.Password = form.Password
-		var err error
-		if u.Salt, err = userutil.RandomSalt(); err != nil {
-			c.Error(err, "get user salt")
-			return
-		}
-		u.Password = userutil.EncodePassword(u.Password, u.Salt)
+	opts := db.UpdateUserOptions{
+		LoginSource:      &form.SourceID,
+		LoginName:        &form.LoginName,
+		FullName:         &form.FullName,
+		Website:          &form.Website,
+		Location:         &form.Location,
+		MaxRepoCreation:  form.MaxRepoCreation,
+		IsActivated:      form.Active,
+		IsAdmin:          form.Admin,
+		AllowGitHook:     form.AllowGitHook,
+		AllowImportLocal: form.AllowImportLocal,
+		ProhibitLogin:    nil, // TODO: Add this option to API
 	}
 
-	u.LoginSource = form.SourceID
-	u.LoginName = form.LoginName
-	u.FullName = form.FullName
-	u.Email = form.Email
-	u.Website = form.Website
-	u.Location = form.Location
-	if form.Active != nil {
-		u.IsActive = *form.Active
-	}
-	if form.Admin != nil {
-		u.IsAdmin = *form.Admin
-	}
-	if form.AllowGitHook != nil {
-		u.AllowGitHook = *form.AllowGitHook
-	}
-	if form.AllowImportLocal != nil {
-		u.AllowImportLocal = *form.AllowImportLocal
-	}
-	if form.MaxRepoCreation != nil {
-		u.MaxRepoCreation = *form.MaxRepoCreation
+	if form.Password != "" {
+		opts.Password = &form.Password
 	}
 
-	if err := db.UpdateUser(u); err != nil {
+	if u.Email != form.Email {
+		opts.Email = &form.Email
+	}
+
+	err := db.Users.Update(c.Req.Context(), u.ID, opts)
+	if err != nil {
 		if db.IsErrEmailAlreadyUsed(err) {
 			c.ErrorStatus(http.StatusUnprocessableEntity, err)
 		} else {
@@ -123,8 +113,13 @@ func EditUser(c *context.APIContext, form api.EditUserOption) {
 		}
 		return
 	}
-	log.Trace("Account profile updated by admin %q: %s", c.User.Name, u.Name)
+	log.Trace("Account updated by admin %q: %s", c.User.Name, u.Name)
 
+	u, err = db.Users.GetByID(c.Req.Context(), u.ID)
+	if err != nil {
+		c.Error(err, "get user")
+		return
+	}
 	c.JSONSuccess(u.APIFormat())
 }
 

internal/route/api/v1/org/org.go

@@ -89,14 +89,25 @@ func Edit(c *context.APIContext, form api.EditOrgOption) {
 		return
 	}
 
-	org.FullName = form.FullName
-	org.Description = form.Description
-	org.Website = form.Website
-	org.Location = form.Location
-	if err := db.UpdateUser(org); err != nil {
-		c.Error(err, "update user")
+	err := db.Users.Update(
+		c.Req.Context(),
+		c.Org.Organization.ID,
+		db.UpdateUserOptions{
+			FullName:    &form.FullName,
+			Website:     &form.Website,
+			Location:    &form.Location,
+			Description: &form.Description,
+		},
+	)
+	if err != nil {
+		c.Error(err, "update organization")
 		return
 	}
 
+	org, err = db.GetOrgByName(org.Name)
+	if err != nil {
+		c.Error(err, "get organization")
+		return
+	}
 	c.JSONSuccess(convert.ToOrganization(org))
 }

internal/route/org/setting.go

@@ -63,16 +63,15 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) {
 	}
 
 	opts := db.UpdateUserOptions{
-		FullName:        f.FullName,
-		Website:         f.Website,
-		Location:        f.Location,
-		Description:     f.Description,
-		MaxRepoCreation: org.MaxRepoCreation,
+		FullName:    &f.FullName,
+		Website:     &f.Website,
+		Location:    &f.Location,
+		Description: &f.Description,
 	}
 	if c.User.IsAdmin {
-		opts.MaxRepoCreation = f.MaxRepoCreation
+		opts.MaxRepoCreation = &f.MaxRepoCreation
 	}
-	err := db.Users.Update(c.Req.Context(), c.User.ID, opts)
+	err := db.Users.Update(c.Req.Context(), c.Org.Organization.ID, opts)
 	if err != nil {
 		c.Error(err, "update organization")
 		return
@@ -83,7 +82,7 @@ func SettingsPost(c *context.Context, f form.UpdateOrgSetting) {
 }
 
 func SettingsAvatar(c *context.Context, f form.Avatar) {
-	f.Source = form.AVATAR_LOCAL
+	f.Source = form.AvatarLocal
 	if err := user.UpdateAvatarSetting(c, f, c.Org.Organization); err != nil {
 		c.Flash.Error(err.Error())
 	} else {

internal/route/repo/setting.go

@@ -309,7 +309,7 @@ func SettingsAvatar(c *context.Context) {
 }
 
 func SettingsAvatarPost(c *context.Context, f form.Avatar) {
-	f.Source = form.AVATAR_LOCAL
+	f.Source = form.AvatarLocal
 	if err := UpdateAvatarSetting(c, f, c.Repo.Repository); err != nil {
 		c.Flash.Error(err.Error())
 	} else {

internal/route/user/auth.go

@@ -367,9 +367,16 @@ func SignUpPost(c *context.Context, cpt *captcha.Captcha, f form.Register) {
 	//
 	// Auto-set admin for the only user.
 	if db.Users.Count(c.Req.Context()) == 1 {
-		user.IsAdmin = true
-		user.IsActive = true
-		if err := db.UpdateUser(user); err != nil {
+		v := true
+		err := db.Users.Update(
+			c.Req.Context(),
+			user.ID,
+			db.UpdateUserOptions{
+				IsActivated: &v,
+				IsAdmin:     &v,
+			},
+		)
+		if err != nil {
 			c.Error(err, "update user")
 			return
 		}
@@ -476,13 +483,16 @@ func Activate(c *context.Context) {
 
 	// Verify code.
 	if user := verifyUserActiveCode(code); user != nil {
-		user.IsActive = true
-		var err error
-		if user.Rands, err = userutil.RandomSalt(); err != nil {
-			c.Error(err, "get user salt")
-			return
-		}
-		if err := db.UpdateUser(user); err != nil {
+		v := true
+		err := db.Users.Update(
+			c.Req.Context(),
+			user.ID,
+			db.UpdateUserOptions{
+				GenerateNewRands: true,
+				IsActivated:      &v,
+			},
+		)
+		if err != nil {
 			c.Error(err, "update user")
 			return
 		}
@@ -601,26 +611,16 @@ func ResetPasswdPost(c *context.Context) {
 
 	if u := verifyUserActiveCode(code); u != nil {
 		// Validate password length.
-		passwd := c.Query("password")
-		if len(passwd) < 6 {
+		password := c.Query("password")
+		if len(password) < 6 {
 			c.Data["IsResetForm"] = true
 			c.Data["Err_Password"] = true
 			c.RenderWithErr(c.Tr("auth.password_too_short"), RESET_PASSWORD, nil)
 			return
 		}
 
-		u.Password = passwd
-		var err error
-		if u.Rands, err = userutil.RandomSalt(); err != nil {
-			c.Error(err, "get user salt")
-			return
-		}
-		if u.Salt, err = userutil.RandomSalt(); err != nil {
-			c.Error(err, "get user salt")
-			return
-		}
-		u.Password = userutil.EncodePassword(u.Password, u.Salt)
-		if err := db.UpdateUser(u); err != nil {
+		err := db.Users.Update(c.Req.Context(), u.ID, db.UpdateUserOptions{Password: &password})
+		if err != nil {
 			c.Error(err, "update user")
 			return
 		}

internal/route/user/setting.go

@@ -96,10 +96,9 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) {
 		c.Req.Context(),
 		c.User.ID,
 		db.UpdateUserOptions{
-			FullName:        f.FullName,
-			Website:         f.Website,
-			Location:        f.Location,
-			MaxRepoCreation: c.User.MaxRepoCreation,
+			FullName: &f.FullName,
+			Website:  &f.Website,
+			Location: &f.Location,
 		},
 	)
 	if err != nil {
@@ -113,13 +112,23 @@ func SettingsPost(c *context.Context, f form.UpdateProfile) {
 
 // FIXME: limit upload size
 func UpdateAvatarSetting(c *context.Context, f form.Avatar, ctxUser *db.User) error {
-	if f.Source == form.AVATAR_BYMAIL && len(f.Gravatar) > 0 {
-		ctxUser.UseCustomAvatar = false
-		ctxUser.Avatar = cryptoutil.MD5(f.Gravatar)
-		ctxUser.AvatarEmail = f.Gravatar
+	if f.Source == form.AvatarLookup && f.Gravatar != "" {
+		avatar := cryptoutil.MD5(f.Gravatar)
+		err := db.Users.Update(
+			c.Req.Context(),
+			ctxUser.ID,
+			db.UpdateUserOptions{
+				Avatar:      &avatar,
+				AvatarEmail: &f.Gravatar,
+			},
+		)
+		if err != nil {
+			return errors.Wrap(err, "update user")
+		}
 
-		if err := db.UpdateUser(ctxUser); err != nil {
-			return fmt.Errorf("update user: %v", err)
+		err = db.Users.DeleteCustomAvatar(c.Req.Context(), c.User.ID)
+		if err != nil {
+			return errors.Wrap(err, "delete custom avatar")
 		}
 		return nil
 	}
@@ -193,14 +202,14 @@ func SettingsPasswordPost(c *context.Context, f form.ChangePassword) {
 	} else if f.Password != f.Retype {
 		c.Flash.Error(c.Tr("form.password_not_match"))
 	} else {
-		c.User.Password = f.Password
-		var err error
-		if c.User.Salt, err = userutil.RandomSalt(); err != nil {
-			c.Errorf(err, "get user salt")
-			return
-		}
-		c.User.Password = userutil.EncodePassword(c.User.Password, c.User.Salt)
-		if err := db.UpdateUser(c.User); err != nil {
+		err := db.Users.Update(
+			c.Req.Context(),
+			c.User.ID,
+			db.UpdateUserOptions{
+				Password: &f.Password,
+			},
+		)
+		if err != nil {
 			c.Errorf(err, "update user")
 			return
 		}