Skip to content

Commit e48cb86

Browse files
EmyrkEmyrk
committed
fixup query and test
1 parent 395edca commit e48cb86

File tree

4 files changed

+61
-21
lines changed

4 files changed

+61
-21
lines changed

‎coderd/database/dbmem/dbmem.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2615,13 +2615,21 @@ func (q *FakeQuerier) GetGroups(_ context.Context, arg database.GetGroupsParams)
26152615
groupIDs[member.GroupID] = struct{}{}
26162616
}
26172617
}
2618+
2619+
// Handle the everyone group
2620+
for _, orgMember := range q.organizationMembers {
2621+
if orgMember.UserID == arg.HasMemberID {
2622+
groupIDs[orgMember.OrganizationID] = struct{}{}
2623+
}
2624+
}
26182625
}
26192626

26202627
filtered := make([]database.Group, 0)
26212628
for _, group := range q.groups {
26222629
if arg.OrganizationID != uuid.Nil && group.OrganizationID != arg.OrganizationID {
26232630
continue
26242631
}
2632+
26252633
_, ok := groupIDs[group.ID]
26262634
if arg.HasMemberID != uuid.Nil && !ok {
26272635
continue

‎coderd/database/queries.sql.go

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/groups.sql

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,12 @@ WHERE
3939
SELECT
4040
1
4141
FROM
42-
group_members
42+
-- this view handles the 'everyone' group in orgs.
43+
group_members_expanded
4344
WHERE
44-
group_members.group_id = groups.id
45+
group_members_expanded.group_id = groups.id
4546
AND
46-
group_members.user_id = @has_member_id
47+
group_members_expanded.user_id = @has_member_id
4748
)
4849
ELSE true
4950
END

‎enterprise/coderd/groups_test.go

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import (
44
"net/http"
55
"sort"
66
"testing"
7+
"time"
78

89
"github.com/google/uuid"
910
"github.com/stretchr/testify/require"
1011

1112
"github.com/coder/coder/v2/coderd/audit"
1213
"github.com/coder/coder/v2/coderd/coderdtest"
1314
"github.com/coder/coder/v2/coderd/database"
15+
"github.com/coder/coder/v2/coderd/database/db2sdk"
1416
"github.com/coder/coder/v2/coderd/rbac"
1517
"github.com/coder/coder/v2/coderd/util/ptr"
1618
"github.com/coder/coder/v2/codersdk"
@@ -568,13 +570,19 @@ func TestPatchGroup(t *testing.T) {
568570
})
569571
}
570572

571-
func sortAllGroups(groups []codersdk.Group) {
573+
func normalizeAllGroups(groups []codersdk.Group) {
572574
for i := range groups {
573-
sortGroupMembers(&groups[i])
575+
normalizeGroupMembers(&groups[i])
574576
}
575577
}
576578

577-
func sortGroupMembers(group *codersdk.Group) {
579+
// normalizeGroupMembers removes comparison noise from the group members.
580+
func normalizeGroupMembers(group *codersdk.Group) {
581+
for i := range group.Members {
582+
group.Members[i].LastSeenAt = time.Time{}
583+
group.Members[i].CreatedAt = time.Time{}
584+
group.Members[i].UpdatedAt = time.Time{}
585+
}
578586
sort.Slice(group.Members, func(i, j int) bool {
579587
return group.Members[i].ID.String() < group.Members[j].ID.String()
580588
})
@@ -651,8 +659,8 @@ func TestGroup(t *testing.T) {
651659

652660
ggroup, err := userAdminClient.Group(ctx, group.ID)
653661
require.NoError(t, err)
654-
sortGroupMembers(&group)
655-
sortGroupMembers(&ggroup)
662+
normalizeGroupMembers(&group)
663+
normalizeGroupMembers(&ggroup)
656664

657665
require.Equal(t, group, ggroup)
658666
})
@@ -813,7 +821,7 @@ func TestGroups(t *testing.T) {
813821
_, user2 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
814822
_, user3 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
815823
_, user4 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
816-
_, user5 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
824+
user5Client, user5 := coderdtest.CreateAnotherUser(t, client, user.OrganizationID)
817825

818826
ctx := testutil.Context(t, testutil.WaitLong)
819827
group1, err := userAdminClient.CreateGroup(ctx, user.OrganizationID, codersdk.CreateGroupRequest{
@@ -830,41 +838,64 @@ func TestGroups(t *testing.T) {
830838
AddUsers: []string{user2.ID.String(), user3.ID.String()},
831839
})
832840
require.NoError(t, err)
833-
sortGroupMembers(&group1)
841+
normalizeGroupMembers(&group1)
834842

835843
group2, err = userAdminClient.PatchGroup(ctx, group2.ID, codersdk.PatchGroupRequest{
836844
AddUsers: []string{user4.ID.String(), user5.ID.String()},
837845
})
838846
require.NoError(t, err)
839-
sortGroupMembers(&group2)
847+
normalizeGroupMembers(&group2)
840848

841849
// Fetch everyone group for comparison
842850
everyoneGroup, err := userAdminClient.Group(ctx, user.OrganizationID)
843851
require.NoError(t, err)
844-
sortGroupMembers(&everyoneGroup)
852+
normalizeGroupMembers(&everyoneGroup)
845853

846854
groups, err := userAdminClient.Groups(ctx, codersdk.GroupArguments{
847855
Organization: user.OrganizationID.String(),
848856
})
849857
require.NoError(t, err)
850-
sortAllGroups(groups)
858+
normalizeAllGroups(groups)
851859

852860
// 'Everyone' group + 2 custom groups.
853-
require.Len(t, groups, 3)
854-
require.Contains(t, groups, group1)
855-
require.Contains(t, groups, group2)
861+
require.ElementsMatch(t, []codersdk.Group{
862+
everyoneGroup,
863+
group1,
864+
group2,
865+
}, groups)
856866

857867
// Filter by user
858868
user5Groups, err := userAdminClient.Groups(ctx, codersdk.GroupArguments{
859869
HasMember: user5.Username,
860870
})
861871
require.NoError(t, err)
872+
normalizeAllGroups(user5Groups)
862873
// Everyone group and group 2
863-
require.ElementsMatch(t, user5Groups, []codersdk.Group{
874+
require.ElementsMatch(t, []codersdk.Group{
864875
everyoneGroup,
865876
group2,
866-
})
877+
}, user5Groups)
867878

879+
// Query from the user's perspective
880+
user5View, err := user5Client.Groups(ctx, codersdk.GroupArguments{})
881+
require.NoError(t, err)
882+
normalizeAllGroups(user5Groups)
883+
884+
// Everyone group and group 2
885+
require.Len(t, user5View, 2)
886+
user5ViewIDs := db2sdk.List(user5View, func(g codersdk.Group) uuid.UUID {
887+
return g.ID
888+
})
889+
890+
require.ElementsMatch(t, []uuid.UUID{
891+
everyoneGroup.ID,
892+
group2.ID,
893+
}, user5ViewIDs)
894+
for _, g := range user5View {
895+
// Only expect the 1 member, themselves
896+
require.Len(t, g.Members, 1)
897+
require.Equal(t, user5.ReducedUser.ID, g.Members[0].MinimalUser.ID)
898+
}
868899
})
869900
}
870901

0 commit comments

Comments
 (0)