Skip to content

Commit c7a6ee5

Browse files
adelowolunny
authored andcommitted
Don't fail silently if trying to add a collaborator twice (#4533)
* don't fail silently if trying to add a collaborator twice * fix translation text * added collaborator test * improvee testcases * Added tests to make sure a collaborator cannot be added twice
1 parent 7cb1c1c commit c7a6ee5

File tree

3 files changed

+109
-0
lines changed

3 files changed

+109
-0
lines changed

options/locale/locale_en-US.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,7 @@ settings.transfer_succeed = The repository has been transferred.
10251025
settings.confirm_delete = Delete Repository
10261026
settings.add_collaborator = Add Collaborator
10271027
settings.add_collaborator_success = The collaborator has been added.
1028+
settings.add_collaborator_duplicate =The collaborator is already added to this repository.
10281029
settings.delete_collaborator = Remove
10291030
settings.collaborator_deletion = Remove Collaborator
10301031
settings.collaborator_deletion_desc = Removing a collaborator will revoke their access to this repository. Continue?

routers/repo/setting.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,12 @@ func CollaborationPost(ctx *context.Context) {
401401
}
402402
}
403403

404+
if got, err := ctx.Repo.Repository.IsCollaborator(u.ID); err == nil && got {
405+
ctx.Flash.Error(ctx.Tr("repo.settings.add_collaborator_duplicate"))
406+
ctx.Redirect(ctx.Repo.RepoLink + "/settings/collaboration")
407+
return
408+
}
409+
404410
if err = ctx.Repo.Repository.AddCollaborator(u); err != nil {
405411
ctx.ServerError("AddCollaborator", err)
406412
return

routers/repo/settings_test.go

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010

1111
"code.gitea.io/gitea/models"
1212
"code.gitea.io/gitea/modules/auth"
13+
"code.gitea.io/gitea/modules/context"
1314
"code.gitea.io/gitea/modules/test"
1415

1516
"github.com/stretchr/testify/assert"
@@ -59,3 +60,104 @@ func TestAddReadWriteOnlyDeployKey(t *testing.T) {
5960
Mode: models.AccessModeWrite,
6061
})
6162
}
63+
64+
func TestCollaborationPost(t *testing.T) {
65+
66+
models.PrepareTestEnv(t)
67+
ctx := test.MockContext(t, "user2/repo1/issues/labels")
68+
test.LoadUser(t, ctx, 2)
69+
test.LoadUser(t, ctx, 4)
70+
test.LoadRepo(t, ctx, 1)
71+
72+
ctx.Req.Form.Set("collaborator", "user4")
73+
74+
u := &models.User{
75+
LowerName: "user2",
76+
Type: models.UserTypeIndividual,
77+
}
78+
79+
re := &models.Repository{
80+
ID: 2,
81+
Owner: u,
82+
}
83+
84+
repo := &context.Repository{
85+
Owner: u,
86+
Repository: re,
87+
}
88+
89+
ctx.Repo = repo
90+
91+
CollaborationPost(ctx)
92+
93+
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())
94+
95+
exists, err := re.IsCollaborator(4)
96+
assert.NoError(t, err)
97+
assert.True(t, exists)
98+
}
99+
100+
func TestCollaborationPost_AddCollaboratorTwice(t *testing.T) {
101+
102+
models.PrepareTestEnv(t)
103+
ctx := test.MockContext(t, "user2/repo1/issues/labels")
104+
test.LoadUser(t, ctx, 2)
105+
test.LoadUser(t, ctx, 4)
106+
test.LoadRepo(t, ctx, 1)
107+
108+
ctx.Req.Form.Set("collaborator", "user4")
109+
110+
u := &models.User{
111+
LowerName: "user2",
112+
Type: models.UserTypeIndividual,
113+
}
114+
115+
re := &models.Repository{
116+
ID: 2,
117+
Owner: u,
118+
}
119+
120+
repo := &context.Repository{
121+
Owner: u,
122+
Repository: re,
123+
}
124+
125+
ctx.Repo = repo
126+
127+
CollaborationPost(ctx)
128+
129+
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())
130+
131+
exists, err := re.IsCollaborator(4)
132+
assert.NoError(t, err)
133+
assert.True(t, exists)
134+
135+
// Try adding the same collaborator again
136+
CollaborationPost(ctx)
137+
138+
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())
139+
assert.NotEmpty(t, ctx.Flash.ErrorMsg)
140+
}
141+
142+
func TestCollaborationPost_NonExistentUser(t *testing.T) {
143+
144+
models.PrepareTestEnv(t)
145+
ctx := test.MockContext(t, "user2/repo1/issues/labels")
146+
test.LoadUser(t, ctx, 2)
147+
test.LoadRepo(t, ctx, 1)
148+
149+
ctx.Req.Form.Set("collaborator", "user34")
150+
151+
repo := &context.Repository{
152+
Owner: &models.User{
153+
LowerName: "user2",
154+
},
155+
}
156+
157+
ctx.Repo = repo
158+
159+
CollaborationPost(ctx)
160+
161+
assert.EqualValues(t, http.StatusFound, ctx.Resp.Status())
162+
assert.NotEmpty(t, ctx.Flash.ErrorMsg)
163+
}

0 commit comments

Comments
 (0)