Skip to content

Commit 84fab63

Browse files
philip-petersonphilip-peterson
committed
RenderUserSearch and SearchUsers should not mutate incoming options
1 parent 498088c commit 84fab63

File tree

12 files changed

+31
-31
lines changed

12 files changed

+31
-31
lines changed

‎models/user/search.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ func (opts *SearchUserOptions) toSearchQueryBase(ctx context.Context) *xorm.Sess
137137

138138
// SearchUsers takes options i.e. keyword and part of user name to search,
139139
// it returns results in given range and number of total results.
140-
func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _ int64, _ error) {
140+
func SearchUsers(ctx context.Context, opts SearchUserOptions) (users []*User, _ int64, _ error) {
141141
sessCount := opts.toSearchQueryBase(ctx)
142142
defer sessCount.Close()
143143
count, err := sessCount.Count(new(User))
@@ -152,7 +152,7 @@ func SearchUsers(ctx context.Context, opts *SearchUserOptions) (users []*User, _
152152
sessQuery := opts.toSearchQueryBase(ctx).OrderBy(opts.OrderBy.String())
153153
defer sessQuery.Close()
154154
if opts.Page > 0 {
155-
sessQuery = db.SetSessionPagination(sessQuery, opts)
155+
sessQuery = db.SetSessionPagination(sessQuery, &opts)
156156
}
157157

158158
// the sql may contain JOIN, so we must only select User related columns

‎models/user/user_test.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestCanCreateOrganization(t *testing.T) {
8888

8989
func TestSearchUsers(t *testing.T) {
9090
assert.NoError(t, unittest.PrepareTestDatabase())
91-
testSuccess := func(opts *user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
91+
testSuccess := func(opts user_model.SearchUserOptions, expectedUserOrOrgIDs []int64) {
9292
users, _, err := user_model.SearchUsers(db.DefaultContext, opts)
9393
assert.NoError(t, err)
9494
cassText := fmt.Sprintf("ids: %v, opts: %v", expectedUserOrOrgIDs, opts)
@@ -100,61 +100,61 @@ func TestSearchUsers(t *testing.T) {
100100
}
101101

102102
// test orgs
103-
testOrgSuccess := func(opts *user_model.SearchUserOptions, expectedOrgIDs []int64) {
103+
testOrgSuccess := func(opts user_model.SearchUserOptions, expectedOrgIDs []int64) {
104104
opts.Type = user_model.UserTypeOrganization
105105
testSuccess(opts, expectedOrgIDs)
106106
}
107107

108-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
108+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1, PageSize: 2}},
109109
[]int64{3, 6})
110110

111-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
111+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 2, PageSize: 2}},
112112
[]int64{7, 17})
113113

114-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
114+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 3, PageSize: 2}},
115115
[]int64{19, 25})
116116

117-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
117+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 4, PageSize: 2}},
118118
[]int64{26, 41})
119119

120-
testOrgSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
120+
testOrgSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 5, PageSize: 2}},
121121
[]int64{42})
122122

123-
testOrgSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
123+
testOrgSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 6, PageSize: 2}},
124124
[]int64{})
125125

126126
// test users
127-
testUserSuccess := func(opts *user_model.SearchUserOptions, expectedUserIDs []int64) {
127+
testUserSuccess := func(opts user_model.SearchUserOptions, expectedUserIDs []int64) {
128128
opts.Type = user_model.UserTypeIndividual
129129
testSuccess(opts, expectedUserIDs)
130130
}
131131

132-
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
132+
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}},
133133
[]int64{1, 2, 4, 5, 8, 9, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
134134

135-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
135+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(false)},
136136
[]int64{9})
137137

138-
testUserSuccess(&user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
138+
testUserSuccess(user_model.SearchUserOptions{OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
139139
[]int64{1, 2, 4, 5, 8, 10, 11, 12, 13, 14, 15, 16, 18, 20, 21, 24, 27, 28, 29, 30, 32, 34, 37, 38, 39, 40})
140140

141-
testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
141+
testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", OrderBy: "id ASC", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
142142
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
143143

144144
// order by name asc default
145-
testUserSuccess(&user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
145+
testUserSuccess(user_model.SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: optional.Some(true)},
146146
[]int64{1, 10, 11, 12, 13, 14, 15, 16, 18})
147147

148-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
148+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: optional.Some(true)},
149149
[]int64{1})
150150

151-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
151+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: optional.Some(true)},
152152
[]int64{29})
153153

154-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
154+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: optional.Some(true)},
155155
[]int64{37})
156156

157-
testUserSuccess(&user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
157+
testUserSuccess(user_model.SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: optional.Some(true)},
158158
[]int64{24})
159159
}
160160

‎routers/api/v1/admin/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func GetAllOrgs(ctx *context.APIContext) {
101101

102102
listOptions := utils.GetListOptions(ctx)
103103

104-
users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
104+
users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
105105
Actor: ctx.Doer,
106106
Type: user_model.UserTypeOrganization,
107107
OrderBy: db.SearchOrderByAlphabetically,

‎routers/api/v1/admin/user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ func SearchUsers(ctx *context.APIContext) {
423423

424424
listOptions := utils.GetListOptions(ctx)
425425

426-
users, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
426+
users, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
427427
Actor: ctx.Doer,
428428
Type: user_model.UserTypeIndividual,
429429
LoginName: ctx.FormTrim("login_name"),

‎routers/api/v1/org/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ func GetAll(ctx *context.APIContext) {
201201

202202
listOptions := utils.GetListOptions(ctx)
203203

204-
publicOrgs, maxResults, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
204+
publicOrgs, maxResults, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
205205
Actor: ctx.Doer,
206206
ListOptions: listOptions,
207207
Type: user_model.UserTypeOrganization,

‎routers/api/v1/user/user.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ func Search(ctx *context.APIContext) {
7373
if ctx.PublicOnly {
7474
visible = []structs.VisibleType{structs.VisibleTypePublic}
7575
}
76-
users, maxResults, err = user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
76+
users, maxResults, err = user_model.SearchUsers(ctx, user_model.SearchUserOptions{
7777
Actor: ctx.Doer,
7878
Keyword: ctx.FormTrim("q"),
7979
UID: uid,

‎routers/web/admin/orgs.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func Organizations(ctx *context.Context) {
2727
ctx.SetFormString("sort", UserSearchDefaultAdminSort)
2828
}
2929

30-
explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{
30+
explore.RenderUserSearch(ctx, user_model.SearchUserOptions{
3131
Actor: ctx.Doer,
3232
Type: user_model.UserTypeOrganization,
3333
IncludeReserved: true, // administrator needs to list all accounts include reserved

‎routers/web/admin/users.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func Users(ctx *context.Context) {
6464
"SortType": sortType,
6565
}
6666

67-
explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{
67+
explore.RenderUserSearch(ctx, user_model.SearchUserOptions{
6868
Actor: ctx.Doer,
6969
Type: user_model.UserTypeIndividual,
7070
ListOptions: db.ListOptions{

‎routers/web/explore/org.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func Organizations(ctx *context.Context) {
4444
ctx.SetFormString("sort", sortOrder)
4545
}
4646

47-
RenderUserSearch(ctx, &user_model.SearchUserOptions{
47+
RenderUserSearch(ctx, user_model.SearchUserOptions{
4848
Actor: ctx.Doer,
4949
Type: user_model.UserTypeOrganization,
5050
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},

‎routers/web/explore/user.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func isKeywordValid(keyword string) bool {
3232
}
3333

3434
// RenderUserSearch render user search page
35-
func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, tplName templates.TplName) {
35+
func RenderUserSearch(ctx *context.Context, opts user_model.SearchUserOptions, tplName templates.TplName) {
3636
// Sitemap index for sitemap paths
3737
opts.Page = int(ctx.PathParamInt64("idx"))
3838
isSitemap := ctx.PathParam("idx") != ""
@@ -151,7 +151,7 @@ func Users(ctx *context.Context) {
151151
ctx.SetFormString("sort", sortOrder)
152152
}
153153

154-
RenderUserSearch(ctx, &user_model.SearchUserOptions{
154+
RenderUserSearch(ctx, user_model.SearchUserOptions{
155155
Actor: ctx.Doer,
156156
Type: user_model.UserTypeIndividual,
157157
ListOptions: db.ListOptions{PageSize: setting.UI.ExplorePagingNum},

‎routers/web/home.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ func Home(ctx *context.Context) {
6868
func HomeSitemap(ctx *context.Context) {
6969
m := sitemap.NewSitemapIndex()
7070
if !setting.Service.Explore.DisableUsersPage {
71-
_, cnt, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
71+
_, cnt, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
7272
Type: user_model.UserTypeIndividual,
7373
ListOptions: db.ListOptions{PageSize: 1},
7474
IsActive: optional.Some(true),

‎routers/web/user/search.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616

1717
// SearchCandidates searches candidate users for dropdown list
1818
func SearchCandidates(ctx *context.Context) {
19-
users, _, err := user_model.SearchUsers(ctx, &user_model.SearchUserOptions{
19+
users, _, err := user_model.SearchUsers(ctx, user_model.SearchUserOptions{
2020
Actor: ctx.Doer,
2121
Keyword: ctx.FormTrim("q"),
2222
Type: user_model.UserTypeIndividual,

0 commit comments

Comments
 (0)