Skip to content

Fix SQL Query for SearchTeam #20844

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

Merged
merged 11 commits into from
Aug 21, 2022
Merged
Changes from all 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
2 changes: 1 addition & 1 deletion integrations/api_team_test.go
Original file line number Diff line number Diff line change
@@ -223,7 +223,7 @@ func TestAPITeamSearch(t *testing.T) {
defer prepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17})

var results TeamSearchResults

22 changes: 22 additions & 0 deletions integrations/api_user_orgs_test.go
Original file line number Diff line number Diff line change
@@ -26,8 +26,19 @@ func TestUserOrgs(t *testing.T) {
orgs := getUserOrgs(t, adminUsername, normalUsername)

user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"})
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})

assert.Equal(t, []*api.Organization{
{
ID: 17,
UserName: user17.Name,
FullName: user17.FullName,
AvatarURL: user17.AvatarLink(),
Description: "",
Website: "",
Location: "",
Visibility: "public",
},
{
ID: 3,
UserName: user3.Name,
@@ -82,8 +93,19 @@ func TestMyOrgs(t *testing.T) {
var orgs []*api.Organization
DecodeJSON(t, resp, &orgs)
user3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user3"})
user17 := unittest.AssertExistsAndLoadBean(t, &user_model.User{Name: "user17"})

assert.Equal(t, []*api.Organization{
{
ID: 17,
UserName: user17.Name,
FullName: user17.FullName,
AvatarURL: user17.AvatarLink(),
Description: "",
Website: "",
Location: "",
Visibility: "public",
},
{
ID: 3,
UserName: user3.Name,
9 changes: 5 additions & 4 deletions integrations/org_test.go
Original file line number Diff line number Diff line change
@@ -197,8 +197,8 @@ func TestOrgRestrictedUser(t *testing.T) {
func TestTeamSearch(t *testing.T) {
defer prepareTestEnv(t)()

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 15})
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 17})

var results TeamSearchResults

@@ -209,8 +209,9 @@ func TestTeamSearch(t *testing.T) {
resp := session.MakeRequest(t, req, http.StatusOK)
DecodeJSON(t, resp, &results)
assert.NotEmpty(t, results.Data)
assert.Len(t, results.Data, 1)
assert.Equal(t, "test_team", results.Data[0].Name)
assert.Len(t, results.Data, 2)
assert.Equal(t, "review_team", results.Data[0].Name)
assert.Equal(t, "test_team", results.Data[1].Name)

// no access if not organization member
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
6 changes: 6 additions & 0 deletions models/fixtures/org_user.yml
Original file line number Diff line number Diff line change
@@ -63,3 +63,9 @@
uid: 29
org_id: 17
is_public: true

-
id: 12
uid: 2
org_id: 17
is_public: true
2 changes: 1 addition & 1 deletion models/fixtures/user.yml
Original file line number Diff line number Diff line change
@@ -309,7 +309,7 @@
avatar_email: [email protected]
num_repos: 2
is_active: true
num_members: 3
num_members: 4
num_teams: 3

-
37 changes: 25 additions & 12 deletions models/organization/team.go
Original file line number Diff line number Diff line change
@@ -96,16 +96,7 @@ type SearchTeamOptions struct {
IncludeDesc bool
}

// SearchTeam search for teams. Caller is responsible to check permissions.
func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
if opts.Page <= 0 {
opts.Page = 1
}
if opts.PageSize == 0 {
// Default limit
opts.PageSize = 10
}

func (opts *SearchTeamOptions) toCond() builder.Cond {
cond := builder.NewCond()

if len(opts.Keyword) > 0 {
@@ -117,18 +108,39 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
cond = cond.And(keywordCond)
}

cond = cond.And(builder.Eq{"org_id": opts.OrgID})
if opts.OrgID > 0 {
cond = cond.And(builder.Eq{"`team`.org_id": opts.OrgID})
}

if opts.UserID > 0 {
cond = cond.And(builder.Eq{"team_user.uid": opts.UserID})
}

return cond
}

// SearchTeam search for teams. Caller is responsible to check permissions.
func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {
sess := db.GetEngine(db.DefaultContext)

opts.SetDefaultValues()
cond := opts.toCond()

if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}

count, err := sess.
Where(cond).
Count(new(Team))
if err != nil {
return nil, 0, err
}

sess = sess.Where(cond)
if opts.UserID > 0 {
sess = sess.Join("INNER", "team_user", "team_user.team_id = team.id")
}

if opts.PageSize == -1 {
opts.PageSize = int(count)
} else {
@@ -137,6 +149,7 @@ func SearchTeam(opts *SearchTeamOptions) ([]*Team, int64, error) {

teams := make([]*Team, 0, opts.PageSize)
if err = sess.
Where(cond).
OrderBy("lower_name").
Find(&teams); err != nil {
return nil, 0, err
2 changes: 1 addition & 1 deletion routers/web/org/teams.go
Original file line number Diff line number Diff line change
@@ -339,7 +339,7 @@ func SearchTeam(ctx *context.Context) {
}

opts := &organization.SearchTeamOptions{
UserID: ctx.Doer.ID,
// UserID is not set because the router already requires the doer to be an org admin. Thus, we don't need to restrict to teams that the user belongs in
Keyword: ctx.FormTrim("q"),
OrgID: ctx.Org.Organization.ID,
IncludeDesc: ctx.FormString("include_desc") == "" || ctx.FormBool("include_desc"),