From ce1a6df89ca3b0eefc9d4866025795329d48c80b Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sat, 22 Apr 2023 22:21:02 -0400 Subject: [PATCH 1/9] Sort users and orgs on explore by recency by default --- routers/web/explore/user.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index e00493c87b766..1a56feb997a66 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -24,7 +24,7 @@ const ( ) // UserSearchDefaultSortType is the default sort type for user search -const UserSearchDefaultSortType = "alphabetically" +const UserSearchDefaultSortType = "newest" var nullByte = []byte{0x00} @@ -58,8 +58,6 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, // we can not set orderBy to `models.SearchOrderByXxx`, because there may be a JOIN in the statement, different tables may have the same name columns ctx.Data["SortType"] = ctx.FormString("sort") switch ctx.FormString("sort") { - case "newest": - orderBy = "`user`.id DESC" case "oldest": orderBy = "`user`.id ASC" case "recentupdate": @@ -72,9 +70,12 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, orderBy = "`user`.last_login_unix ASC" case "reverselastlogin": orderBy = "`user`.last_login_unix DESC" - case UserSearchDefaultSortType: // "alphabetically" - default: + case "alphabetically": orderBy = "`user`.name ASC" + case UserSearchDefaultSortType: + fallthrough + default: + orderBy = "`user`.id DESC" ctx.Data["SortType"] = UserSearchDefaultSortType } From 1b467c2585cec0aa2559f5c7aec707cd1200a530 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sat, 22 Apr 2023 22:31:18 -0400 Subject: [PATCH 2/9] repo actually uses recent update, this now matches --- routers/web/explore/user.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 1a56feb997a66..c09647d5c9d56 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -24,7 +24,7 @@ const ( ) // UserSearchDefaultSortType is the default sort type for user search -const UserSearchDefaultSortType = "newest" +const UserSearchDefaultSortType = "recentupdate" var nullByte = []byte{0x00} @@ -58,10 +58,10 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, // we can not set orderBy to `models.SearchOrderByXxx`, because there may be a JOIN in the statement, different tables may have the same name columns ctx.Data["SortType"] = ctx.FormString("sort") switch ctx.FormString("sort") { + case "newest": + orderBy = "`user`.id DESC" case "oldest": orderBy = "`user`.id ASC" - case "recentupdate": - orderBy = "`user`.updated_unix DESC" case "leastupdate": orderBy = "`user`.updated_unix ASC" case "reversealphabetically": @@ -75,7 +75,7 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, case UserSearchDefaultSortType: fallthrough default: - orderBy = "`user`.id DESC" + orderBy = "`user`.updated_unix DESC" ctx.Data["SortType"] = UserSearchDefaultSortType } From 850b3e46f0435e75307f3334e38c78ca8e990427 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 23 Apr 2023 23:58:48 -0400 Subject: [PATCH 3/9] alpha sort for admin --- routers/web/admin/orgs.go | 4 ++-- routers/web/admin/users.go | 6 +++--- routers/web/explore/user.go | 19 +++++++++++++++---- 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/routers/web/admin/orgs.go b/routers/web/admin/orgs.go index 6a3617d67f9bc..f63897481d216 100644 --- a/routers/web/admin/orgs.go +++ b/routers/web/admin/orgs.go @@ -23,12 +23,12 @@ func Organizations(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("admin.organizations") ctx.Data["PageIsAdminOrganizations"] = true - explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{ + explore.RenderUserSearchWithSort(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeOrganization, ListOptions: db.ListOptions{ PageSize: setting.UI.Admin.OrgPagingNum, }, Visible: []structs.VisibleType{structs.VisibleTypePublic, structs.VisibleTypeLimited, structs.VisibleTypePrivate}, - }, tplOrgs) + }, tplOrgs, "alphabetically") } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 2150bc42f7baf..59f4e3160b969 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -53,14 +53,14 @@ func Users(ctx *context.Context) { sortType := ctx.FormString("sort") if sortType == "" { - sortType = explore.UserSearchDefaultSortType + sortType = "alphabetically" } ctx.PageData["adminUserListSearchForm"] = map[string]interface{}{ "StatusFilterMap": statusFilterMap, "SortType": sortType, } - explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{ + explore.RenderUserSearchWithSort(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeIndividual, ListOptions: db.ListOptions{ @@ -73,7 +73,7 @@ func Users(ctx *context.Context) { IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]), IsProhibitLogin: util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]), ExtraParamStrings: extraParamStrings, - }, tplUsers) + }, tplUsers, "alphabetically") } // NewUser render adding a new user page diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index c09647d5c9d56..86b426b643788 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -34,6 +34,11 @@ func isKeywordValid(keyword string) bool { // RenderUserSearch render user search page func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, tplName base.TplName) { + RenderUserSearchWithSort(ctx, opts, tplName, UserSearchDefaultSortType) +} + +// RenderUserSearchWithSort render user search page with specific default sort +func RenderUserSearchWithSort(ctx *context.Context, opts *user_model.SearchUserOptions, tplName base.TplName, defaultSort string) { // Sitemap index for sitemap paths opts.Page = int(ctx.ParamsInt64("idx")) isSitemap := ctx.Params("idx") != "" @@ -56,8 +61,13 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, ) // we can not set orderBy to `models.SearchOrderByXxx`, because there may be a JOIN in the statement, different tables may have the same name columns - ctx.Data["SortType"] = ctx.FormString("sort") - switch ctx.FormString("sort") { + + sortType := ctx.FormString("sort") + if sortType == "" { + sortType = defaultSort + } + ctx.Data["SortType"] = sortType + switch sortType { case "newest": orderBy = "`user`.id DESC" case "oldest": @@ -72,11 +82,12 @@ func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, orderBy = "`user`.last_login_unix DESC" case "alphabetically": orderBy = "`user`.name ASC" - case UserSearchDefaultSortType: + case "recentupdate": fallthrough default: + // in case the sortType is not valid, we set it to recentupdate + ctx.Data["SortType"] = "recentupdate" orderBy = "`user`.updated_unix DESC" - ctx.Data["SortType"] = UserSearchDefaultSortType } opts.Keyword = ctx.FormTrim("q") From ab84be001ff7457d4c3879e0b2f9a7ca768bf334 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 24 Apr 2023 00:13:23 -0400 Subject: [PATCH 4/9] use const --- routers/web/admin/orgs.go | 2 +- routers/web/admin/users.go | 2 +- routers/web/explore/user.go | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/routers/web/admin/orgs.go b/routers/web/admin/orgs.go index f63897481d216..c40a5baeff34c 100644 --- a/routers/web/admin/orgs.go +++ b/routers/web/admin/orgs.go @@ -30,5 +30,5 @@ func Organizations(ctx *context.Context) { PageSize: setting.UI.Admin.OrgPagingNum, }, Visible: []structs.VisibleType{structs.VisibleTypePublic, structs.VisibleTypeLimited, structs.VisibleTypePrivate}, - }, tplOrgs, "alphabetically") + }, tplOrgs, explore.UserSearchDefaultAdminSort) } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 59f4e3160b969..f817bdd81836f 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -73,7 +73,7 @@ func Users(ctx *context.Context) { IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]), IsProhibitLogin: util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]), ExtraParamStrings: extraParamStrings, - }, tplUsers, "alphabetically") + }, tplUsers, explore.UserSearchDefaultAdminSort) } // NewUser render adding a new user page diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 86b426b643788..71142257ab15d 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -25,6 +25,7 @@ const ( // UserSearchDefaultSortType is the default sort type for user search const UserSearchDefaultSortType = "recentupdate" +const UserSearchDefaultAdminSort = "alphabetically" var nullByte = []byte{0x00} From 610ae060940c65a7b2caaa2ba4152f19f66f6ece Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 24 Apr 2023 00:52:19 -0400 Subject: [PATCH 5/9] make fmt --- routers/web/explore/user.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 71142257ab15d..f3f923520a077 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -24,8 +24,10 @@ const ( ) // UserSearchDefaultSortType is the default sort type for user search -const UserSearchDefaultSortType = "recentupdate" -const UserSearchDefaultAdminSort = "alphabetically" +const ( + UserSearchDefaultSortType = "recentupdate" + UserSearchDefaultAdminSort = "alphabetically" +) var nullByte = []byte{0x00} From 196abe2bf92303013b780c8dda6a43246d00f6e3 Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Mon, 24 Apr 2023 10:14:08 -0400 Subject: [PATCH 6/9] update test --- tests/integration/setting_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/setting_test.go b/tests/integration/setting_test.go index cb8248e6e2e52..a824bd7f2fc95 100644 --- a/tests/integration/setting_test.go +++ b/tests/integration/setting_test.go @@ -20,7 +20,7 @@ func TestSettingShowUserEmailExplore(t *testing.T) { setting.UI.ShowUserEmail = true session := loginUser(t, "user2") - req := NewRequest(t, "GET", "/explore/users") + req := NewRequest(t, "GET", "/explore/users?sort=alphabetically") resp := session.MakeRequest(t, req, http.StatusOK) htmlDoc := NewHTMLParser(t, resp.Body) assert.Contains(t, @@ -30,7 +30,7 @@ func TestSettingShowUserEmailExplore(t *testing.T) { setting.UI.ShowUserEmail = false - req = NewRequest(t, "GET", "/explore/users") + req = NewRequest(t, "GET", "/explore/users?sort=alphabetically") resp = session.MakeRequest(t, req, http.StatusOK) htmlDoc = NewHTMLParser(t, resp.Body) assert.NotContains(t, From 40b0087a3d89d21bbd9790899a0ba378df5d599e Mon Sep 17 00:00:00 2001 From: techknowlogick Date: Sun, 30 Apr 2023 00:04:14 -0400 Subject: [PATCH 7/9] Update routers/web/admin/users.go Co-authored-by: wxiaoguang --- routers/web/admin/users.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index f817bdd81836f..48e9a2b3b11cc 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -53,7 +53,7 @@ func Users(ctx *context.Context) { sortType := ctx.FormString("sort") if sortType == "" { - sortType = "alphabetically" + sortType = explore.UserSearchDefaultAdminSort } ctx.PageData["adminUserListSearchForm"] = map[string]interface{}{ "StatusFilterMap": statusFilterMap, From 8b29f4090fdb0f755b4a28cf8e7a91b22fdc41ac Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 6 May 2023 12:40:51 +0800 Subject: [PATCH 8/9] improve the user search sort --- modules/context/form.go | 5 +++++ routers/web/admin/orgs.go | 8 ++++++-- routers/web/admin/users.go | 5 +++-- routers/web/explore/org.go | 4 ++++ routers/web/explore/user.go | 17 ++++++----------- 5 files changed, 24 insertions(+), 15 deletions(-) diff --git a/modules/context/form.go b/modules/context/form.go index f9c4ab6a9845a..7020c37f229c8 100644 --- a/modules/context/form.go +++ b/modules/context/form.go @@ -65,3 +65,8 @@ func (ctx *Context) FormOptionalBool(key string) util.OptionalBool { v = v || strings.EqualFold(s, "on") return util.OptionalBoolOf(v) } + +func (ctx *Context) SetFormValue(key, value string) { + _ = ctx.Req.FormValue(key) // force parse form + ctx.Req.Form.Set(key, value) +} diff --git a/routers/web/admin/orgs.go b/routers/web/admin/orgs.go index c40a5baeff34c..5447acf03bf15 100644 --- a/routers/web/admin/orgs.go +++ b/routers/web/admin/orgs.go @@ -23,12 +23,16 @@ func Organizations(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("admin.organizations") ctx.Data["PageIsAdminOrganizations"] = true - explore.RenderUserSearchWithSort(ctx, &user_model.SearchUserOptions{ + if ctx.FormString("sort") == "" { + ctx.SetFormValue("sort", explore.UserSearchDefaultAdminSort) + } + + explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeOrganization, ListOptions: db.ListOptions{ PageSize: setting.UI.Admin.OrgPagingNum, }, Visible: []structs.VisibleType{structs.VisibleTypePublic, structs.VisibleTypeLimited, structs.VisibleTypePrivate}, - }, tplOrgs, explore.UserSearchDefaultAdminSort) + }, tplOrgs) } diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 48e9a2b3b11cc..f1bb985f8b89f 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -54,13 +54,14 @@ func Users(ctx *context.Context) { sortType := ctx.FormString("sort") if sortType == "" { sortType = explore.UserSearchDefaultAdminSort + ctx.SetFormValue("sort", sortType) } ctx.PageData["adminUserListSearchForm"] = map[string]interface{}{ "StatusFilterMap": statusFilterMap, "SortType": sortType, } - explore.RenderUserSearchWithSort(ctx, &user_model.SearchUserOptions{ + explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeIndividual, ListOptions: db.ListOptions{ @@ -73,7 +74,7 @@ func Users(ctx *context.Context) { IsTwoFactorEnabled: util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]), IsProhibitLogin: util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]), ExtraParamStrings: extraParamStrings, - }, tplUsers, explore.UserSearchDefaultAdminSort) + }, tplUsers) } // NewUser render adding a new user page diff --git a/routers/web/explore/org.go b/routers/web/explore/org.go index c9fb05ff3a883..aa9de8893b8c5 100644 --- a/routers/web/explore/org.go +++ b/routers/web/explore/org.go @@ -30,6 +30,10 @@ func Organizations(ctx *context.Context) { visibleTypes = append(visibleTypes, structs.VisibleTypeLimited, structs.VisibleTypePrivate) } + if ctx.FormString("sort") == "" { + ctx.SetFormValue("sort", UserSearchDefaultSortType) + } + RenderUserSearch(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeOrganization, diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index f3f923520a077..ad446e5a1cc3a 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -37,11 +37,6 @@ func isKeywordValid(keyword string) bool { // RenderUserSearch render user search page func RenderUserSearch(ctx *context.Context, opts *user_model.SearchUserOptions, tplName base.TplName) { - RenderUserSearchWithSort(ctx, opts, tplName, UserSearchDefaultSortType) -} - -// RenderUserSearchWithSort render user search page with specific default sort -func RenderUserSearchWithSort(ctx *context.Context, opts *user_model.SearchUserOptions, tplName base.TplName, defaultSort string) { // Sitemap index for sitemap paths opts.Page = int(ctx.ParamsInt64("idx")) isSitemap := ctx.Params("idx") != "" @@ -65,12 +60,8 @@ func RenderUserSearchWithSort(ctx *context.Context, opts *user_model.SearchUserO // we can not set orderBy to `models.SearchOrderByXxx`, because there may be a JOIN in the statement, different tables may have the same name columns - sortType := ctx.FormString("sort") - if sortType == "" { - sortType = defaultSort - } - ctx.Data["SortType"] = sortType - switch sortType { + ctx.Data["SortType"] = ctx.FormString("sort") + switch ctx.FormString("sort") { case "newest": orderBy = "`user`.id DESC" case "oldest": @@ -142,6 +133,10 @@ func Users(ctx *context.Context) { ctx.Data["PageIsExploreUsers"] = true ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled + if ctx.FormString("sort") == "" { + ctx.SetFormValue("sort", UserSearchDefaultSortType) + } + RenderUserSearch(ctx, &user_model.SearchUserOptions{ Actor: ctx.Doer, Type: user_model.UserTypeIndividual, From 5016e18727fa4773ad64329362f0a90c8efa5320 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 6 May 2023 20:17:14 +0800 Subject: [PATCH 9/9] Rename SetFormValue -> SetFormString --- modules/context/form.go | 2 +- routers/web/admin/orgs.go | 2 +- routers/web/admin/users.go | 2 +- routers/web/explore/org.go | 2 +- routers/web/explore/user.go | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/context/form.go b/modules/context/form.go index 7020c37f229c8..5c02152582250 100644 --- a/modules/context/form.go +++ b/modules/context/form.go @@ -66,7 +66,7 @@ func (ctx *Context) FormOptionalBool(key string) util.OptionalBool { return util.OptionalBoolOf(v) } -func (ctx *Context) SetFormValue(key, value string) { +func (ctx *Context) SetFormString(key, value string) { _ = ctx.Req.FormValue(key) // force parse form ctx.Req.Form.Set(key, value) } diff --git a/routers/web/admin/orgs.go b/routers/web/admin/orgs.go index 5447acf03bf15..d0fd0d5002507 100644 --- a/routers/web/admin/orgs.go +++ b/routers/web/admin/orgs.go @@ -24,7 +24,7 @@ func Organizations(ctx *context.Context) { ctx.Data["PageIsAdminOrganizations"] = true if ctx.FormString("sort") == "" { - ctx.SetFormValue("sort", explore.UserSearchDefaultAdminSort) + ctx.SetFormString("sort", explore.UserSearchDefaultAdminSort) } explore.RenderUserSearch(ctx, &user_model.SearchUserOptions{ diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index f1bb985f8b89f..bd31d9d632f2e 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -54,7 +54,7 @@ func Users(ctx *context.Context) { sortType := ctx.FormString("sort") if sortType == "" { sortType = explore.UserSearchDefaultAdminSort - ctx.SetFormValue("sort", sortType) + ctx.SetFormString("sort", sortType) } ctx.PageData["adminUserListSearchForm"] = map[string]interface{}{ "StatusFilterMap": statusFilterMap, diff --git a/routers/web/explore/org.go b/routers/web/explore/org.go index aa9de8893b8c5..c8b26c35efb2e 100644 --- a/routers/web/explore/org.go +++ b/routers/web/explore/org.go @@ -31,7 +31,7 @@ func Organizations(ctx *context.Context) { } if ctx.FormString("sort") == "" { - ctx.SetFormValue("sort", UserSearchDefaultSortType) + ctx.SetFormString("sort", UserSearchDefaultSortType) } RenderUserSearch(ctx, &user_model.SearchUserOptions{ diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index ad446e5a1cc3a..a2b5f80099429 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -134,7 +134,7 @@ func Users(ctx *context.Context) { ctx.Data["IsRepoIndexerEnabled"] = setting.Indexer.RepoIndexerEnabled if ctx.FormString("sort") == "" { - ctx.SetFormValue("sort", UserSearchDefaultSortType) + ctx.SetFormString("sort", UserSearchDefaultSortType) } RenderUserSearch(ctx, &user_model.SearchUserOptions{