Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the function RemoveOrgUser #27582

Merged
merged 5 commits into from
Nov 3, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions models/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,14 @@ import (
repo_model "code.gitea.io/gitea/models/repo"
)

func removeOrgUser(ctx context.Context, orgID, userID int64) error {
// RemoveOrgUser removes user from given organization.
func RemoveOrgUser(ctx context.Context, orgID, userID int64) error {
ctx, committer, err := db.TxContext(ctx)
if err != nil {
return err
lunny marked this conversation as resolved.
Show resolved Hide resolved
}
defer committer.Close()

ou := new(organization.OrgUser)

sess := db.GetEngine(ctx)
Expand Down Expand Up @@ -93,18 +100,5 @@ func removeOrgUser(ctx context.Context, orgID, userID int64) error {
}
}

return nil
}

// RemoveOrgUser removes user from given organization.
func RemoveOrgUser(orgID, userID int64) error {
ctx, committer, err := db.TxContext(db.DefaultContext)
if err != nil {
return err
}
defer committer.Close()
if err := removeOrgUser(ctx, orgID, userID); err != nil {
return err
}
return committer.Commit()
}
2 changes: 1 addition & 1 deletion models/org_team.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func removeInvalidOrgUser(ctx context.Context, userID, orgID int64) error {
}); err != nil {
return err
} else if count == 0 {
return removeOrgUser(ctx, orgID, userID)
return RemoveOrgUser(ctx, orgID, userID)
}
return nil
}
Expand Down
9 changes: 5 additions & 4 deletions models/org_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package models
import (
"testing"

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/models/organization"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"
Expand All @@ -20,15 +21,15 @@ func TestUser_RemoveMember(t *testing.T) {
// remove a user that is a member
unittest.AssertExistsAndLoadBean(t, &organization.OrgUser{UID: 4, OrgID: 3})
prevNumMembers := org.NumMembers
assert.NoError(t, RemoveOrgUser(org.ID, 4))
assert.NoError(t, RemoveOrgUser(db.DefaultContext, org.ID, 4))
unittest.AssertNotExistsBean(t, &organization.OrgUser{UID: 4, OrgID: 3})
org = unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
assert.Equal(t, prevNumMembers-1, org.NumMembers)

// remove a user that is not a member
unittest.AssertNotExistsBean(t, &organization.OrgUser{UID: 5, OrgID: 3})
prevNumMembers = org.NumMembers
assert.NoError(t, RemoveOrgUser(org.ID, 5))
assert.NoError(t, RemoveOrgUser(db.DefaultContext, org.ID, 5))
unittest.AssertNotExistsBean(t, &organization.OrgUser{UID: 5, OrgID: 3})
org = unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 3})
assert.Equal(t, prevNumMembers, org.NumMembers)
Expand All @@ -44,15 +45,15 @@ func TestRemoveOrgUser(t *testing.T) {
if unittest.BeanExists(t, &organization.OrgUser{OrgID: orgID, UID: userID}) {
expectedNumMembers--
}
assert.NoError(t, RemoveOrgUser(orgID, userID))
assert.NoError(t, RemoveOrgUser(db.DefaultContext, orgID, userID))
unittest.AssertNotExistsBean(t, &organization.OrgUser{OrgID: orgID, UID: userID})
org = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: orgID})
assert.EqualValues(t, expectedNumMembers, org.NumMembers)
}
testSuccess(3, 4)
testSuccess(3, 4)

err := RemoveOrgUser(7, 5)
err := RemoveOrgUser(db.DefaultContext, 7, 5)
assert.Error(t, err)
assert.True(t, organization.IsErrLastOrgOwner(err))
unittest.AssertExistsAndLoadBean(t, &organization.OrgUser{OrgID: 7, UID: 5})
Expand Down
2 changes: 1 addition & 1 deletion routers/api/v1/org/member.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ func DeleteMember(ctx *context.APIContext) {
if ctx.Written() {
return
}
if err := models.RemoveOrgUser(ctx.Org.Organization.ID, member.ID); err != nil {
if err := models.RemoveOrgUser(ctx, ctx.Org.Organization.ID, member.ID); err != nil {
ctx.Error(http.StatusInternalServerError, "RemoveOrgUser", err)
}
ctx.Status(http.StatusNoContent)
Expand Down
4 changes: 2 additions & 2 deletions routers/web/org/members.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,14 @@ func MembersAction(ctx *context.Context) {
ctx.Error(http.StatusNotFound)
return
}
err = models.RemoveOrgUser(org.ID, uid)
err = models.RemoveOrgUser(ctx, org.ID, uid)
if organization.IsErrLastOrgOwner(err) {
ctx.Flash.Error(ctx.Tr("form.last_org_owner"))
ctx.JSONRedirect(ctx.Org.OrgLink + "/members")
return
}
case "leave":
err = models.RemoveOrgUser(org.ID, ctx.Doer.ID)
err = models.RemoveOrgUser(ctx, org.ID, ctx.Doer.ID)
if err == nil {
ctx.Flash.Success(ctx.Tr("form.organization_leave_success", org.DisplayName()))
ctx.JSON(http.StatusOK, map[string]any{
Expand Down
2 changes: 1 addition & 1 deletion services/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
break
}
for _, org := range orgs {
if err := models.RemoveOrgUser(org.ID, u.ID); err != nil {
if err := models.RemoveOrgUser(ctx, org.ID, u.ID); err != nil {
if organization.IsErrLastOrgOwner(err) {
err = organization.DeleteOrganization(ctx, org)
}
Expand Down
2 changes: 1 addition & 1 deletion services/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestDeleteUser(t *testing.T) {
orgUsers := make([]*organization.OrgUser, 0, 10)
assert.NoError(t, db.GetEngine(db.DefaultContext).Find(&orgUsers, &organization.OrgUser{UID: userID}))
for _, orgUser := range orgUsers {
if err := models.RemoveOrgUser(orgUser.OrgID, orgUser.UID); err != nil {
if err := models.RemoveOrgUser(db.DefaultContext, orgUser.OrgID, orgUser.UID); err != nil {
assert.True(t, organization.IsErrLastOrgOwner(err))
return
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/auth_ldap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ func TestLDAPGroupTeamSyncAddMember(t *testing.T) {
assert.True(t, isMember, "Membership should be added to the right team")
err = models.RemoveTeamMember(db.DefaultContext, team, user.ID)
assert.NoError(t, err)
err = models.RemoveOrgUser(usersOrgs[0].ID, user.ID)
err = models.RemoveOrgUser(db.DefaultContext, usersOrgs[0].ID, user.ID)
assert.NoError(t, err)
} else {
// assert members of LDAP group "cn=admin_staff" keep initial team membership since mapped team does not exist
Expand Down
Loading