Fix bug

Author: lunny

routers/api/v1/api.go

@@ -1172,7 +1172,7 @@ func Routes() *web.Router {
 					m.Get("", reqAnyRepoReader(), repo.ListCollaborators)
 					m.Group("/{collaborator}", func() {
 						m.Combo("").Get(reqAnyRepoReader(), repo.IsCollaborator).
-							Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddCollaborator).
+							Put(reqAdmin(), bind(api.AddCollaboratorOption{}), repo.AddOrUpdateCollaborator).
 							Delete(reqAdmin(), repo.DeleteCollaborator)
 						m.Get("/permission", repo.GetRepoPermissions)
 					})

routers/api/v1/repo/collaborators.go

@@ -122,11 +122,11 @@ func IsCollaborator(ctx *context.APIContext) {
 	}
 }
 
-// AddCollaborator add a collaborator to a repository
-func AddCollaborator(ctx *context.APIContext) {
+// AddOrUpdateCollaborator add or update a collaborator to a repository
+func AddOrUpdateCollaborator(ctx *context.APIContext) {
 	// swagger:operation PUT /repos/{owner}/{repo}/collaborators/{collaborator} repository repoAddCollaborator
 	// ---
-	// summary: Add a collaborator to a repository
+	// summary: Add or Update a collaborator to a repository
 	// produces:
 	// - application/json
 	// parameters:
@@ -181,11 +181,11 @@ func AddCollaborator(ctx *context.APIContext) {
 		p = perm.ParseAccessMode(*form.Permission)
 	}
 
-	if err := repo_service.AddCollaborator(ctx, ctx.Repo.Repository, collaborator, p); err != nil {
+	if err := repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, collaborator, p); err != nil {
 		if errors.Is(err, user_model.ErrBlockedUser) {
-			ctx.Error(http.StatusForbidden, "AddCollaborator", err)
+			ctx.Error(http.StatusForbidden, "AddOrUpdateCollaborator", err)
 		} else {
-			ctx.Error(http.StatusInternalServerError, "AddCollaborator", err)
+			ctx.Error(http.StatusInternalServerError, "AddOrUpdateCollaborator", err)
 		}
 		return
 	}

routers/web/repo/setting/collaboration.go

@@ -99,12 +99,12 @@ func CollaborationPost(ctx *context.Context) {
 		}
 	}
 
-	if err = repo_service.AddCollaborator(ctx, ctx.Repo.Repository, u, perm.AccessModeWrite); err != nil {
+	if err = repo_service.AddOrUpdateCollaborator(ctx, ctx.Repo.Repository, u, perm.AccessModeWrite); err != nil {
 		if errors.Is(err, user_model.ErrBlockedUser) {
 			ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator.blocked_user"))
 			ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration")
 		} else {
-			ctx.ServerError("AddCollaborator", err)
+			ctx.ServerError("AddOrUpdateCollaborator", err)
 		}
 		return
 	}

services/repository/collaboration.go

@@ -17,7 +17,7 @@ import (
 	"xorm.io/builder"
 )
 
-func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User, mode perm.AccessMode) error {
+func AddOrUpdateCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User, mode perm.AccessMode) error {
 	if err := repo.LoadOwner(ctx); err != nil {
 		return err
 	}
@@ -27,17 +27,26 @@ func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_m
 	}
 
 	return db.WithTx(ctx, func(ctx context.Context) error {
-		has, err := db.Exist[repo_model.Collaboration](ctx, builder.Eq{
+		collaboration, has, err := db.Get[repo_model.Collaboration](ctx, builder.Eq{
 			"repo_id": repo.ID,
 			"user_id": u.ID,
 		})
 		if err != nil {
 			return err
 		} else if has {
-			return nil
-		}
-
-		if err = db.Insert(ctx, &repo_model.Collaboration{
+			if collaboration.Mode == mode {
+				return nil
+			}
+			if _, err = db.GetEngine(ctx).
+				Where("repo_id=?", repo.ID).
+				And("user_id=?", u.ID).
+				Cols("mode").
+				Update(&repo_model.Collaboration{
+					Mode: mode,
+				}); err != nil {
+				return err
+			}
+		} else if err = db.Insert(ctx, &repo_model.Collaboration{
 			RepoID: repo.ID,
 			UserID: u.ID,
 			Mode:   mode,

services/repository/collaboration_test.go

@@ -22,7 +22,7 @@ func TestRepository_AddCollaborator(t *testing.T) {
 		repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID})
 		assert.NoError(t, repo.LoadOwner(db.DefaultContext))
 		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: userID})
-		assert.NoError(t, AddCollaborator(db.DefaultContext, repo, user, perm.AccessModeWrite))
+		assert.NoError(t, AddOrUpdateCollaborator(db.DefaultContext, repo, user, perm.AccessModeWrite))
 		unittest.CheckConsistencyFor(t, &repo_model.Repository{ID: repoID}, &user_model.User{ID: userID})
 	}
 	testSuccess(1, 4)

services/repository/create.go

@@ -453,7 +453,7 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re
 			return fmt.Errorf("IsUserRepoAdmin: %w", err)
 		} else if !isAdmin {
 			// Make creator repo admin if it wasn't assigned automatically
-			if err = AddCollaborator(ctx, repo, doer, perm.AccessModeAdmin); err != nil {
+			if err = AddOrUpdateCollaborator(ctx, repo, doer, perm.AccessModeAdmin); err != nil {
 				return fmt.Errorf("AddCollaborator: %w", err)
 			}
 		}

services/repository/transfer.go

@@ -418,7 +418,7 @@ func StartRepositoryTransfer(ctx context.Context, doer, newOwner *user_model.Use
 		return err
 	}
 	if !hasAccess {
-		if err := AddCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil {
+		if err := AddOrUpdateCollaborator(ctx, repo, newOwner, perm.AccessModeRead); err != nil {
 			return err
 		}
 	}