Skip to content

Commit

Permalink
orgsRemoveMember
Browse files Browse the repository at this point in the history
  • Loading branch information
@unknwon
unknwon committed Nov 14, 2023
1 parent be46b47 commit 4ba6291
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 20 deletions.
22 changes: 12 additions & 10 deletions internal/db/organizations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func (db *organizations) RemoveMember(ctx context.Context, orgID, userID int64)
if err != nil {
return errors.Wrap(err, "get owners team")
} else if t.NumMembers == 1 {
return ErrLastOrgOwner{args: map[string]any{"orgID": orgID, "userID": userID}}
return ErrLastOrgOwner{args: errutil.Args{"orgID": orgID, "userID": userID}}
}
}

Expand All @@ -165,7 +165,8 @@ func (db *organizations) RemoveMember(ctx context.Context, orgID, userID int64)
return errors.Wrap(err, "unwatch repositories")
}

err = tx.Table("repository").
err = tx.
Table("repository").
Where("id IN (?)", repoIDsConds).
UpdateColumn("num_watches", gorm.Expr("num_watches - 1")).
Error
Expand Down Expand Up @@ -193,8 +194,9 @@ func (db *organizations) RemoveMember(ctx context.Context, orgID, userID int64)
WHERE team_user.org_id = @orgID AND uid = @userID)
)
*/
err = tx.Table("team").
Where(`id IN (?)`, tx.
err = tx.
Table("team").
Where("id IN (?)", tx.
Select("team_id").
Table("team_user").
Where("org_id = ? AND uid = ?", orgID, userID),
Expand Down Expand Up @@ -235,7 +237,7 @@ func (*organizations) accessibleRepositoriesByUser(tx *gorm.DB, orgID, userID in
/*
Equivalent SQL for PostgreSQL:
<SELECT * FROM "repository">
SELECT * FROM "repository"
JOIN team_repo ON repository.id = team_repo.repo_id
WHERE
owner_id = @orgID
Expand All @@ -250,14 +252,14 @@ func (*organizations) accessibleRepositoriesByUser(tx *gorm.DB, orgID, userID in
[LIMIT @limit OFFSET @offset]
*/
conds := tx.
Table("repository").
Joins("JOIN team_repo ON repository.id = team_repo.repo_id").
Where("owner_id = ? AND (?)", orgID, tx.
Where("team_repo.team_id IN (?)", tx.
Select("team_id").
Where("owner_id = ? AND (team_repo.team_id IN (?) OR (repository.is_private = ? AND repository.is_unlisted = ?))",
orgID,
tx.Select("team_id").
Table("team_user").
Where("team_user.org_id = ? AND uid = ?", orgID, userID),
).
Or("repository.is_private = ? AND repository.is_unlisted = ?", false, false),
false, false,
)
if opts.orderBy == OrderByUpdatedDesc {
conds.Order("updated_unix DESC")
Expand Down
121 changes: 115 additions & 6 deletions internal/db/organizations_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func TestOrganizations(t *testing.T) {
tables := []any{
new(User), new(EmailAddress), new(OrgUser), new(Team), new(TeamUser), new(Repository), new(Watch), new(Star),
new(Follow), new(Issue), new(PublicKey), new(AccessToken), new(Collaboration), new(Access), new(Action),
new(IssueUser),
new(IssueUser), new(TeamRepo),
}
db := &organizations{
DB: dbtest.NewDB(t, "orgs", tables...),
Expand All @@ -46,6 +46,7 @@ func TestOrganizations(t *testing.T) {
{"Count", orgsCount},
{"DeleteByID", orgsDeleteByID},
{"AddMember", orgsAddMember},
{"RemoveMember", orgsRemoveMember},
} {
t.Run(tc.name, func(t *testing.T) {
t.Cleanup(func() {
Expand Down Expand Up @@ -424,21 +425,129 @@ func orgsAddMember(t *testing.T, db *organizations) {
require.NoError(t, err)

// Not yet a member
got, err := db.List(ctx, ListOrganizationsOptions{MemberID: bob.ID, IncludePrivateMembers: true})
gotOrgs, err := db.List(ctx, ListOrganizationsOptions{MemberID: bob.ID, IncludePrivateMembers: true})
require.NoError(t, err)
assert.Len(t, got, 0)
assert.Len(t, gotOrgs, 0)

// Add member
err = db.AddMember(ctx, org1.ID, bob.ID)
require.NoError(t, err)

// Now a member
got, err = db.List(ctx, ListOrganizationsOptions{MemberID: bob.ID, IncludePrivateMembers: true})
gotOrgs, err = db.List(ctx, ListOrganizationsOptions{MemberID: bob.ID, IncludePrivateMembers: true})
require.NoError(t, err)
assert.Len(t, got, 1)
assert.Equal(t, org1.ID, got[0].ID)
assert.Len(t, gotOrgs, 1)
assert.Equal(t, org1.ID, gotOrgs[0].ID)

// Add member again shouldn't fail
err = db.AddMember(ctx, org1.ID, bob.ID)
require.NoError(t, err)

gotOrg, err := db.GetByName(ctx, org1.Name)
require.NoError(t, err)
assert.Equal(t, 2, gotOrg.NumMembers)
}

func orgsRemoveMember(t *testing.T, db *organizations) {
ctx := context.Background()

usersStore := NewUsersStore(db.DB)
alice, err := usersStore.Create(ctx, "alice", "alice@example.com", CreateUserOptions{})
require.NoError(t, err)
bob, err := usersStore.Create(ctx, "bob", "bob@exmaple.com", CreateUserOptions{})
require.NoError(t, err)

tempPictureAvatarUploadPath := filepath.Join(os.TempDir(), "orgsRemoveMember-tempPictureAvatarUploadPath")
conf.SetMockPicture(t, conf.PictureOpts{AvatarUploadPath: tempPictureAvatarUploadPath})

org1, err := db.Create(ctx, "org1", alice.ID, CreateOrganizationOptions{})
require.NoError(t, err)

t.Run("remove non-existent member", func(t *testing.T) {
err = db.RemoveMember(ctx, org1.ID, bob.ID)
require.NoError(t, err)
})

t.Run("remove last owner", func(t *testing.T) {
err = db.RemoveMember(ctx, org1.ID, alice.ID)
wantErr := ErrLastOrgOwner{errutil.Args{"orgID": org1.ID, "userID": alice.ID}}
assert.Equal(t, wantErr, err)
})

err = db.AddMember(ctx, org1.ID, bob.ID)
require.NoError(t, err)

// Mock repository, watches, accesses and collaborations
reposStore := NewRepositoriesStore(db.DB)
repo1, err := reposStore.Create(ctx, org1.ID, CreateRepoOptions{Name: "repo1", Private: true})
require.NoError(t, err)
err = reposStore.Watch(ctx, bob.ID, repo1.ID)
require.NoError(t, err)
permsStore := NewPermsStore(db.DB)
err = permsStore.SetRepoPerms(ctx, repo1.ID, map[int64]AccessMode{bob.ID: AccessModeRead})
require.NoError(t, err)
// TODO: Use Repositories.AddCollaborator to replace SQL hack when the method is available.
err = db.DB.Create(
&Collaboration{
UserID: bob.ID,
RepoID: repo1.ID,
Mode: AccessModeRead,
},
).Error
require.NoError(t, err)

// Mock team membership
// TODO: Use Organizations.CreateTeam to replace SQL hack when the method is available.
team1 := &Team{
OrgID: org1.ID,
LowerName: "team1",
Name: "team1",
NumMembers: 1,
}
err = db.DB.Create(team1).Error
require.NoError(t, err)
// TODO: Use Organizations.AddTeamMember to replace SQL hack when the method is available.
err = db.DB.Create(
&TeamUser{
OrgID: org1.ID,
TeamID: team1.ID,
UID: bob.ID,
},
).Error
require.NoError(t, err)
// TODO: Use Organizations.AddTeamRepository to replace SQL hack when the method is available.
err = db.DB.Create(
&TeamRepo{
OrgID: org1.ID,
TeamID: team1.ID,
RepoID: repo1.ID,
},
).Error
require.NoError(t, err)

// Pull the trigger
err = db.RemoveMember(ctx, org1.ID, bob.ID)
require.NoError(t, err)

// Verify after-the-fact data
gotRepo, err := reposStore.GetByID(ctx, repo1.ID)
require.NoError(t, err)
assert.Equal(t, 1, gotRepo.NumWatches)

gotAccessMode := permsStore.AccessMode(ctx, repo1.ID, bob.ID, AccessModeOptions{Private: repo1.IsPrivate})
assert.Equal(t, AccessModeNone, gotAccessMode)

// TODO: Use Repositories.ListCollaborators to replace SQL hack when the method is available.
var count int64
err = db.DB.Model(&Collaboration{}).Where(&Collaboration{RepoID: repo1.ID}).Count(&count).Error
require.NoError(t, err)
assert.Equal(t, int64(0), count)

gotTeam, err := db.GetTeamByName(ctx, org1.ID, team1.Name)
require.NoError(t, err)
assert.Equal(t, 0, gotTeam.NumMembers)

gotOrg, err := db.GetByName(ctx, org1.Name)
require.NoError(t, err)
assert.Equal(t, 1, gotOrg.NumMembers)
}
12 changes: 8 additions & 4 deletions internal/db/users.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,8 @@ func (db *users) DeleteByID(ctx context.Context, userID int64, skipRewriteAuthor
SELECT repo_id FROM watch WHERE user_id = @userID
)
*/
err = tx.Table("repository").
err = tx.
Table("repository").
Where("id IN (?)", tx.
Select("repo_id").
Table("watch").
Expand All @@ -547,7 +548,8 @@ func (db *users) DeleteByID(ctx context.Context, userID int64, skipRewriteAuthor
SELECT repo_id FROM star WHERE uid = @userID
)
*/
err = tx.Table("repository").
err = tx.
Table("repository").
Where("id IN (?)", tx.
Select("repo_id").
Table("star").
Expand All @@ -568,7 +570,8 @@ func (db *users) DeleteByID(ctx context.Context, userID int64, skipRewriteAuthor
SELECT follow_id FROM follow WHERE user_id = @userID
)
*/
err = tx.Table("user").
err = tx.
Table("user").
Where("id IN (?)", tx.
Select("follow_id").
Table("follow").
Expand All @@ -589,7 +592,8 @@ func (db *users) DeleteByID(ctx context.Context, userID int64, skipRewriteAuthor
SELECT user_id FROM follow WHERE follow_id = @userID
)
*/
err = tx.Table("user").
err = tx.
Table("user").
Where("id IN (?)", tx.
Select("user_id").
Table("follow").
Expand Down

0 comments on commit 4ba6291

Please sign in to comment.