From d5adeeeb33cda136906400be1691df94aef13c45 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Sun, 22 Aug 2021 18:13:24 +0800 Subject: [PATCH 01/22] Admin can filter user list by status --- models/user.go | 46 +++++++++++++++++----- options/locale/locale_en-US.ini | 12 ++++++ options/locale/locale_zh-CN.ini | 18 +++++++-- routers/web/explore/user.go | 9 +++++ templates/admin/base/search.tmpl | 2 +- templates/admin/user/list.tmpl | 62 +++++++++++++++++++++++++++++- web_src/js/features/admin-users.js | 34 ++++++++++++++++ web_src/js/index.js | 2 + 8 files changed, 171 insertions(+), 14 deletions(-) create mode 100644 web_src/js/features/admin-users.js diff --git a/models/user.go b/models/user.go index 1af9e545adee2..7b114172cdb1d 100644 --- a/models/user.go +++ b/models/user.go @@ -1588,14 +1588,15 @@ func GetUser(user *User) (bool, error) { // SearchUserOptions contains the options for searching type SearchUserOptions struct { ListOptions - Keyword string - Type UserType - UID int64 - OrderBy SearchOrderBy - Visible []structs.VisibleType - Actor *User // The user doing the search - IsActive util.OptionalBool - SearchByEmail bool // Search by email as well as username/full name + Keyword string + StatusFilterMap map[string]string // Admin can apply advanced search filters + Type UserType + UID int64 + OrderBy SearchOrderBy + Visible []structs.VisibleType + Actor *User // The user doing the search + IsActive util.OptionalBool + SearchByEmail bool // Search by email as well as username/full name } func (opts *SearchUserOptions) toConds() builder.Cond { @@ -1643,8 +1644,35 @@ func (opts *SearchUserOptions) toConds() builder.Cond { // Don't forget about self accessCond = accessCond.Or(builder.Eq{"id": opts.Actor.ID}) cond = cond.And(accessCond) + } else { + // Admin can apply advanced filters + for filterKey, filterValue := range opts.StatusFilterMap { + if filterValue == "" { + continue + } + if filterKey == "is_active" { + cond = cond.And(builder.Eq{"is_active": filterValue}) + } else if filterKey == "is_admin" { + cond = cond.And(builder.Eq{"is_admin": filterValue}) + } else if filterKey == "is_restricted" { + cond = cond.And(builder.Eq{"is_restricted": filterValue}) + } else if filterKey == "is_prohibit_login" { + cond = cond.And(builder.Eq{"prohibit_login": filterValue}) + } else if filterKey == "is_2fa_enabled" { + // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future + var twoFactorCond builder.Cond + twoFactorBuilder := builder.Select("uid").From("two_factor").Where(builder.And(builder.Expr("two_factor.uid = user.id"))) + if filterValue == "1" { + twoFactorCond = builder.In("id", twoFactorBuilder) + } else { + twoFactorCond = builder.NotIn("id", twoFactorBuilder) + } + cond = cond.And(twoFactorCond) + } else { + log.Critical("Unknown admin user search filter: %v=%v", filterKey, filterValue) + } + } } - } else { // Force visibility for privacy // Not logged in - only public users diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 3eb38257768a4..30ddb86001c01 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -2347,6 +2347,18 @@ users.still_own_repo = This user still owns one or more repositories. Delete or users.still_has_org = This user is a member of an organization. Remove the user from any organizations first. users.deletion_success = The user account has been deleted. users.reset_2fa = Reset 2FA +users.list_status_filter.menu_text = Filter +users.list_status_filter.reset = Reset +users.list_status_filter.is_active = Active +users.list_status_filter.not_active = Inactive +users.list_status_filter.is_admin = Admin +users.list_status_filter.not_admin = Not Admin +users.list_status_filter.is_restricted = Restricted +users.list_status_filter.not_restricted = Not Restricted +users.list_status_filter.is_prohibit_login = Prohibit Login +users.list_status_filter.not_prohibit_login = Allow Login +users.list_status_filter.is_2fa_enabled = 2FA Enabled +users.list_status_filter.not_2fa_enabled = 2FA Disabled emails.email_manage_panel = User Email Management emails.primary = Primary diff --git a/options/locale/locale_zh-CN.ini b/options/locale/locale_zh-CN.ini index 80bda011c1d90..2c5af058332b8 100644 --- a/options/locale/locale_zh-CN.ini +++ b/options/locale/locale_zh-CN.ini @@ -634,8 +634,8 @@ last_used=上次使用在 no_activity=没有最近活动 can_read_info=读取 can_write_info=写入 -key_state_desc=7 天内使用过该密钥 -token_state_desc=7 天内使用过该密钥 +key_state_desc=7 天内使用过该密钥 +token_state_desc=7 天内使用过该密钥 principal_state_desc=7 天内使用过该规则 show_openid=在个人信息上显示 hide_openid=在个人信息上隐藏 @@ -812,7 +812,7 @@ watchers=关注者 stargazers=称赞者 forks=派生仓库 pick_reaction=选择你的表情 -reactions_more=再加载 %d +reactions_more=再加载 %d unit_disabled=站点管理员已禁用此仓库单元。 language_other=其它 adopt_search=输入用户名以搜索未被收录的仓库... (留空以查找全部) @@ -2337,6 +2337,18 @@ users.still_own_repo=此用户仍然拥有一个或多个仓库。必须首先 users.still_has_org=此用户是组织的成员。必须先从组织中删除用户。 users.deletion_success=用户帐户已被删除。 users.reset_2fa=重置两步验证 +users.list_status_filter.menu_text = 筛选 +users.list_status_filter.reset = 重置 +users.list_status_filter.is_active = 已激活 +users.list_status_filter.not_active = 未激活 +users.list_status_filter.is_admin = 管理员 +users.list_status_filter.not_admin = 非管理员 +users.list_status_filter.is_restricted = 受限 +users.list_status_filter.not_restricted = 未受限 +users.list_status_filter.is_prohibit_login = 禁止登录 +users.list_status_filter.not_prohibit_login = 允许登录 +users.list_status_filter.is_2fa_enabled = 两步认证已开启 +users.list_status_filter.not_2fa_enabled = 两步认证已禁用 emails.email_manage_panel=邮件管理 emails.primary=主要的 diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index aeaaf92c12216..190d3fa980944 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -62,8 +62,15 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN orderBy = models.SearchOrderByAlphabetically } + statusFilterKeys := []string{"is_active", "is_admin", "is_restricted", "is_2fa_enabled", "is_prohibit_login"} + statusFilterMap := map[string]string{} + for _, filterKey := range statusFilterKeys { + statusFilterMap[filterKey] = ctx.FormString("status_filter[" + filterKey + "]") + } + opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy + opts.StatusFilterMap = statusFilterMap if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) { users, count, err = models.SearchUsers(opts) if err != nil { @@ -71,7 +78,9 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN return } } + ctx.Data["Keyword"] = opts.Keyword + ctx.Data["StatusFilterMap"] = statusFilterMap ctx.Data["Total"] = count ctx.Data["Users"] = users ctx.Data["UsersTwoFaStatus"] = models.UserList(users).GetTwoFaStatus() diff --git a/templates/admin/base/search.tmpl b/templates/admin/base/search.tmpl index e4e7e2d462523..98fd3f4a0780f 100644 --- a/templates/admin/base/search.tmpl +++ b/templates/admin/base/search.tmpl @@ -15,7 +15,7 @@ -
+
diff --git a/templates/admin/user/list.tmpl b/templates/admin/user/list.tmpl index 661d38cb03ba2..e9f96ca038df6 100644 --- a/templates/admin/user/list.tmpl +++ b/templates/admin/user/list.tmpl @@ -10,7 +10,67 @@
- {{template "admin/base/search" .}} + + + +
+ +
+ {{.i18n.Tr "admin.users.list_status_filter.menu_text"}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} +
+ {{.i18n.Tr "admin.users.list_status_filter.reset"}} +
+ + +
+ + +
+ + +
+ + +
+ + +
+
+ + +
+ + {{.i18n.Tr "repo.issues.filter_sort"}} {{svg "octicon-triangle-down" 14 "dropdown icon"}} + +
+ + + + + + +
+
+
+ + +
+ + +
+ + {{/* here we have valid go template syntax, but eslint doesn't like it and reports "error Parsing error: Unexpected token {" */}} + +
diff --git a/web_src/js/features/admin-users.js b/web_src/js/features/admin-users.js new file mode 100644 index 0000000000000..d2679c9b095cc --- /dev/null +++ b/web_src/js/features/admin-users.js @@ -0,0 +1,34 @@ +export function initAdminUserListSearchForm() { + if (!$('.admin').length) return; + if (!window.giteaContext || !window.giteaContext.adminUserListSearchForm) return; + + const $form = $('#user-list-search-form'); + if (!$form.length) return; + + const searchForm = window.giteaContext.adminUserListSearchForm; + + $form.find(`button[name=sort][value=${searchForm.sortType}]`).addClass('active'); + + if (searchForm.statusFilterMap) { + for (const [k, v] of Object.entries(searchForm.statusFilterMap)) { + if (!v) continue; + $form.find(`input[name="status_filter[${k}]"][value=${v}]`).prop('checked', true); + } + } + + $form.find(`input[type=radio]`).click(() => { + $form.submit(); + return false; + }); + + $form.find('.j-reset-status-filter').click(() => { + $form.find(`input[type=radio]`).each((_, e) => { + const $e = $(e); + if ($e.attr('name').startsWith('status_filter[')) { + $e.prop('checked', false); + } + }); + $form.submit(); + return false; + }); +} diff --git a/web_src/js/index.js b/web_src/js/index.js index d349092717b57..51248817ea9f5 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -17,6 +17,7 @@ import initMigration from './features/migration.js'; import initProject from './features/projects.js'; import initServiceWorker from './features/serviceworker.js'; import initTableSort from './features/tablesort.js'; +import {initAdminUserListSearchForm} from './features/admin-users.js'; import {createCodeEditor, createMonaco} from './features/codeeditor.js'; import {initMarkupAnchors} from './markup/anchors.js'; import {initNotificationsTable, initNotificationCount} from './features/notification.js'; @@ -2839,6 +2840,7 @@ $(document).ready(async () => { initFileViewToggle(); initReleaseEditor(); initRelease(); + initAdminUserListSearchForm(); const routes = { 'div.user.settings': initUserSettings, From 0b2604b9b3039670825055e0e273e447022b2d13 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Wed, 25 Aug 2021 20:00:32 +0800 Subject: [PATCH 02/22] introduce window.config.PageData to pass template data to javascript module and small refactor move legacy window.ActivityTopAuthors to window.config.PageData.ActivityTopAuthors make HTML structure more IDE-friendly in footer.tmpl and head.tmpl remove incorrect in head.tmpl use log.Error instead of log.Critical in admin user search --- models/user.go | 2 +- templates/admin/user/list.tmpl | 3 +-- templates/base/footer.tmpl | 5 +++-- templates/base/head.tmpl | 8 +++++--- templates/repo/activity.tmpl | 2 +- web_src/js/features/admin-users.js | 6 ++---- web_src/js/index.js | 4 ++-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/models/user.go b/models/user.go index 7b114172cdb1d..849cad531b0b0 100644 --- a/models/user.go +++ b/models/user.go @@ -1669,7 +1669,7 @@ func (opts *SearchUserOptions) toConds() builder.Cond { } cond = cond.And(twoFactorCond) } else { - log.Critical("Unknown admin user search filter: %v=%v", filterKey, filterValue) + log.Error("Unknown admin user search filter: %v=%v", filterKey, filterValue) } } } diff --git a/templates/admin/user/list.tmpl b/templates/admin/user/list.tmpl index e9f96ca038df6..c9f1761ee8f85 100644 --- a/templates/admin/user/list.tmpl +++ b/templates/admin/user/list.tmpl @@ -63,8 +63,7 @@ diff --git a/web_src/js/features/admin-users.js b/web_src/js/features/admin-users.js index d2679c9b095cc..2354f67fc709d 100644 --- a/web_src/js/features/admin-users.js +++ b/web_src/js/features/admin-users.js @@ -1,12 +1,10 @@ export function initAdminUserListSearchForm() { - if (!$('.admin').length) return; - if (!window.giteaContext || !window.giteaContext.adminUserListSearchForm) return; + const searchForm = window.config.PageData.adminUserListSearchForm; + if (!searchForm) return; const $form = $('#user-list-search-form'); if (!$form.length) return; - const searchForm = window.giteaContext.adminUserListSearchForm; - $form.find(`button[name=sort][value=${searchForm.sortType}]`).addClass('active'); if (searchForm.statusFilterMap) { diff --git a/web_src/js/index.js b/web_src/js/index.js index 51248817ea9f5..ddc02ba82e5c3 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -27,7 +27,7 @@ import {initMarkupContent, initCommentContent} from './markup/content.js'; import {stripTags, mqBinarySearch} from './utils.js'; import {svg, svgs} from './svg.js'; -const {AppSubUrl, AssetUrlPrefix, csrf} = window.config; +const {AppSubUrl, AssetUrlPrefix, csrf, PageData} = window.config; let previewFileModes; const commentMDEditors = {}; @@ -3439,7 +3439,7 @@ function initVueApp() { searchLimit: Number((document.querySelector('meta[name=_search_limit]') || {}).content), suburl: AppSubUrl, uid: Number((document.querySelector('meta[name=_context_uid]') || {}).content), - activityTopAuthors: window.ActivityTopAuthors || [], + activityTopAuthors: PageData.ActivityTopAuthors || [], }; }, }); From dbf21d2ef89c9cac7e561f1966bc27478dd7ada9 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Wed, 25 Aug 2021 20:34:22 +0800 Subject: [PATCH 03/22] revert ActivityTopAuthors related changes, maybe a new PR is needed --- templates/repo/activity.tmpl | 2 +- web_src/js/index.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/activity.tmpl b/templates/repo/activity.tmpl index 7de2175d5edfb..08e2a31115a35 100644 --- a/templates/repo/activity.tmpl +++ b/templates/repo/activity.tmpl @@ -110,7 +110,7 @@
diff --git a/web_src/js/index.js b/web_src/js/index.js index ddc02ba82e5c3..51248817ea9f5 100644 --- a/web_src/js/index.js +++ b/web_src/js/index.js @@ -27,7 +27,7 @@ import {initMarkupContent, initCommentContent} from './markup/content.js'; import {stripTags, mqBinarySearch} from './utils.js'; import {svg, svgs} from './svg.js'; -const {AppSubUrl, AssetUrlPrefix, csrf, PageData} = window.config; +const {AppSubUrl, AssetUrlPrefix, csrf} = window.config; let previewFileModes; const commentMDEditors = {}; @@ -3439,7 +3439,7 @@ function initVueApp() { searchLimit: Number((document.querySelector('meta[name=_search_limit]') || {}).content), suburl: AppSubUrl, uid: Number((document.querySelector('meta[name=_context_uid]') || {}).content), - activityTopAuthors: PageData.ActivityTopAuthors || [], + activityTopAuthors: window.ActivityTopAuthors || [], }; }, }); From 30f743006fcb1d00653ca3a2a332ab60a6bc8bbf Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Thu, 26 Aug 2021 14:06:12 +0800 Subject: [PATCH 04/22] use LEFT JOIN instead of SubQuery when admin filters users by 2fa. revert non-en locale. --- models/user.go | 76 ++++++++++++++++++++------------- options/locale/locale_zh-CN.ini | 18 ++------ 2 files changed, 49 insertions(+), 45 deletions(-) diff --git a/models/user.go b/models/user.go index 849cad531b0b0..fead306dd7359 100644 --- a/models/user.go +++ b/models/user.go @@ -16,9 +16,11 @@ import ( "os" "path/filepath" "regexp" + "strconv" "strings" "time" "unicode/utf8" + "xorm.io/xorm" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" @@ -1644,34 +1646,6 @@ func (opts *SearchUserOptions) toConds() builder.Cond { // Don't forget about self accessCond = accessCond.Or(builder.Eq{"id": opts.Actor.ID}) cond = cond.And(accessCond) - } else { - // Admin can apply advanced filters - for filterKey, filterValue := range opts.StatusFilterMap { - if filterValue == "" { - continue - } - if filterKey == "is_active" { - cond = cond.And(builder.Eq{"is_active": filterValue}) - } else if filterKey == "is_admin" { - cond = cond.And(builder.Eq{"is_admin": filterValue}) - } else if filterKey == "is_restricted" { - cond = cond.And(builder.Eq{"is_restricted": filterValue}) - } else if filterKey == "is_prohibit_login" { - cond = cond.And(builder.Eq{"prohibit_login": filterValue}) - } else if filterKey == "is_2fa_enabled" { - // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future - var twoFactorCond builder.Cond - twoFactorBuilder := builder.Select("uid").From("two_factor").Where(builder.And(builder.Expr("two_factor.uid = user.id"))) - if filterValue == "1" { - twoFactorCond = builder.In("id", twoFactorBuilder) - } else { - twoFactorCond = builder.NotIn("id", twoFactorBuilder) - } - cond = cond.And(twoFactorCond) - } else { - log.Error("Unknown admin user search filter: %v=%v", filterKey, filterValue) - } - } } } else { // Force visibility for privacy @@ -1694,7 +1668,49 @@ func (opts *SearchUserOptions) toConds() builder.Cond { // it returns results in given range and number of total results. func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { cond := opts.toConds() - count, err := x.Where(cond).Count(new(User)) + + searchFilterTwoFactorValue := "" + if opts.Actor != nil && opts.Actor.IsAdmin { + // Admin can apply advanced filters + for filterKey, filterValue := range opts.StatusFilterMap { + if filterValue == "" { + continue + } + parsedBool, _ := strconv.ParseBool(filterValue) + if filterKey == "is_active" { + cond = cond.And(builder.Eq{"is_active": parsedBool}) + } else if filterKey == "is_admin" { + cond = cond.And(builder.Eq{"is_admin": parsedBool}) + } else if filterKey == "is_restricted" { + cond = cond.And(builder.Eq{"is_restricted": parsedBool}) + } else if filterKey == "is_prohibit_login" { + cond = cond.And(builder.Eq{"prohibit_login": parsedBool}) + } else if filterKey == "is_2fa_enabled" { + searchFilterTwoFactorValue = filterValue + } else { + log.Error("Unknown admin user search filter: %v=%v", filterKey, filterValue) + } + } + } + + searchUserBaseQuery := func () (sess *xorm.Session) { + sess = x.Where(cond) + + if searchFilterTwoFactorValue != "" { + // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record + // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future + sess = sess.Join("LEFT OUTER", "two_factor", "two_factor.uid = `user`.id") + if searchFilterTwoFactorValue == "1" { + cond = cond.And(builder.Expr("two_factor.uid IS NOT NULL")) + } else { + cond = cond.And(builder.Expr("two_factor.uid IS NULL")) + } + } + + return sess + } + + count, err := searchUserBaseQuery().Count(new(User)) if err != nil { return nil, 0, fmt.Errorf("Count: %v", err) } @@ -1703,7 +1719,7 @@ func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { opts.OrderBy = SearchOrderByAlphabetically } - sess := x.Where(cond).OrderBy(opts.OrderBy.String()) + sess := searchUserBaseQuery().OrderBy(opts.OrderBy.String()) if opts.Page != 0 { sess = opts.setSessionPagination(sess) } diff --git a/options/locale/locale_zh-CN.ini b/options/locale/locale_zh-CN.ini index 1b0934db926e9..9a9746a46af2e 100644 --- a/options/locale/locale_zh-CN.ini +++ b/options/locale/locale_zh-CN.ini @@ -634,8 +634,8 @@ last_used=上次使用在 no_activity=没有最近活动 can_read_info=读取 can_write_info=写入 -key_state_desc=7 天内使用过该密钥 -token_state_desc=7 天内使用过该密钥 +key_state_desc=7 天内使用过该密钥 +token_state_desc=7 天内使用过该密钥 principal_state_desc=7 天内使用过该规则 show_openid=在个人信息上显示 hide_openid=在个人信息上隐藏 @@ -812,7 +812,7 @@ watchers=关注者 stargazers=称赞者 forks=派生仓库 pick_reaction=选择你的表情 -reactions_more=再加载 %d +reactions_more=再加载 %d unit_disabled=站点管理员已禁用此仓库单元。 language_other=其它 adopt_search=输入用户名以搜索未被收录的仓库... (留空以查找全部) @@ -2335,18 +2335,6 @@ users.still_own_repo=此用户仍然拥有一个或多个仓库。必须首先 users.still_has_org=此用户是组织的成员。必须先从组织中删除用户。 users.deletion_success=用户帐户已被删除。 users.reset_2fa=重置两步验证 -users.list_status_filter.menu_text = 筛选 -users.list_status_filter.reset = 重置 -users.list_status_filter.is_active = 已激活 -users.list_status_filter.not_active = 未激活 -users.list_status_filter.is_admin = 管理员 -users.list_status_filter.not_admin = 非管理员 -users.list_status_filter.is_restricted = 受限 -users.list_status_filter.not_restricted = 未受限 -users.list_status_filter.is_prohibit_login = 禁止登录 -users.list_status_filter.not_prohibit_login = 允许登录 -users.list_status_filter.is_2fa_enabled = 两步认证已开启 -users.list_status_filter.not_2fa_enabled = 两步认证已禁用 emails.email_manage_panel=邮件管理 emails.primary=主要的 From 6ec918ef4fb00de49e00c7f0631188f1ac06584a Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Thu, 26 Aug 2021 14:08:00 +0800 Subject: [PATCH 05/22] use OptionalBool instead of status map --- models/user.go | 69 +++++++++++++++++-------------------- modules/util/util.go | 12 ++++++- modules/util/util_test.go | 13 +++++++ routers/web/explore/user.go | 8 ++++- 4 files changed, 62 insertions(+), 40 deletions(-) diff --git a/models/user.go b/models/user.go index 267eae0d14feb..fdaf485aca5e6 100644 --- a/models/user.go +++ b/models/user.go @@ -16,11 +16,9 @@ import ( "os" "path/filepath" "regexp" - "strconv" "strings" "time" "unicode/utf8" - "xorm.io/xorm" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/git" @@ -35,7 +33,9 @@ import ( "golang.org/x/crypto/bcrypt" "golang.org/x/crypto/pbkdf2" "golang.org/x/crypto/scrypt" + "xorm.io/builder" + "xorm.io/xorm" ) // UserType defines the user type @@ -1590,15 +1590,19 @@ func GetUser(user *User) (bool, error) { // SearchUserOptions contains the options for searching type SearchUserOptions struct { ListOptions - Keyword string - StatusFilterMap map[string]string // Admin can apply advanced search filters - Type UserType - UID int64 - OrderBy SearchOrderBy - Visible []structs.VisibleType - Actor *User // The user doing the search - IsActive util.OptionalBool - SearchByEmail bool // Search by email as well as username/full name + Keyword string + Type UserType + UID int64 + OrderBy SearchOrderBy + Visible []structs.VisibleType + Actor *User // The user doing the search + SearchByEmail bool // Search by email as well as username/full name + + IsActive util.OptionalBool + IsAdmin util.OptionalBool + IsRestricted util.OptionalBool + IsTwoFactorEnabled util.OptionalBool + IsProhibitLogin util.OptionalBool } func (opts *SearchUserOptions) toConds() builder.Cond { @@ -1640,6 +1644,7 @@ func (opts *SearchUserOptions) toConds() builder.Cond { accessCond = accessCond.Or(builder.Eq{"id": opts.Actor.ID}) cond = cond.And(accessCond) } + } else { // Force visibility for privacy // Not logged in - only public users @@ -1654,6 +1659,18 @@ func (opts *SearchUserOptions) toConds() builder.Cond { cond = cond.And(builder.Eq{"is_active": opts.IsActive.IsTrue()}) } + if !opts.IsAdmin.IsNone() { + cond = cond.And(builder.Eq{"is_admin": opts.IsAdmin.IsTrue()}) + } + + if !opts.IsRestricted.IsNone() { + cond = cond.And(builder.Eq{"is_restricted": opts.IsRestricted.IsTrue()}) + } + + if !opts.IsProhibitLogin.IsNone() { + cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.IsTrue()}) + } + return cond } @@ -1662,38 +1679,14 @@ func (opts *SearchUserOptions) toConds() builder.Cond { func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { cond := opts.toConds() - searchFilterTwoFactorValue := "" - if opts.Actor != nil && opts.Actor.IsAdmin { - // Admin can apply advanced filters - for filterKey, filterValue := range opts.StatusFilterMap { - if filterValue == "" { - continue - } - parsedBool, _ := strconv.ParseBool(filterValue) - if filterKey == "is_active" { - cond = cond.And(builder.Eq{"is_active": parsedBool}) - } else if filterKey == "is_admin" { - cond = cond.And(builder.Eq{"is_admin": parsedBool}) - } else if filterKey == "is_restricted" { - cond = cond.And(builder.Eq{"is_restricted": parsedBool}) - } else if filterKey == "is_prohibit_login" { - cond = cond.And(builder.Eq{"prohibit_login": parsedBool}) - } else if filterKey == "is_2fa_enabled" { - searchFilterTwoFactorValue = filterValue - } else { - log.Error("Unknown admin user search filter: %v=%v", filterKey, filterValue) - } - } - } - - searchUserBaseQuery := func () (sess *xorm.Session) { + searchUserBaseQuery := func() (sess *xorm.Session) { sess = x.Where(cond) - if searchFilterTwoFactorValue != "" { + if !opts.IsTwoFactorEnabled.IsNone() { // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future sess = sess.Join("LEFT OUTER", "two_factor", "two_factor.uid = `user`.id") - if searchFilterTwoFactorValue == "1" { + if opts.IsTwoFactorEnabled.IsTrue() { cond = cond.And(builder.Expr("two_factor.uid IS NOT NULL")) } else { cond = cond.And(builder.Expr("two_factor.uid IS NULL")) diff --git a/modules/util/util.go b/modules/util/util.go index d26e6f13e4b6b..cbc6eb4f8a01c 100644 --- a/modules/util/util.go +++ b/modules/util/util.go @@ -9,6 +9,7 @@ import ( "crypto/rand" "errors" "math/big" + "strconv" "strings" ) @@ -17,7 +18,7 @@ type OptionalBool byte const ( // OptionalBoolNone a "null" boolean value - OptionalBoolNone = iota + OptionalBoolNone OptionalBool = iota // OptionalBoolTrue a "true" boolean value OptionalBoolTrue // OptionalBoolFalse a "false" boolean value @@ -47,6 +48,15 @@ func OptionalBoolOf(b bool) OptionalBool { return OptionalBoolFalse } +// OptionalBoolParse get the corresponding OptionalBool of a string using strconv.ParseBool +func OptionalBoolParse(s string) OptionalBool { + b, e := strconv.ParseBool(s) + if e != nil { + return OptionalBoolNone + } + return OptionalBoolOf(b) +} + // Max max of two ints func Max(a, b int) int { if a < b { diff --git a/modules/util/util_test.go b/modules/util/util_test.go index f82671787cfb0..39cf07c85543e 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -156,3 +156,16 @@ func Test_RandomString(t *testing.T) { assert.NotEqual(t, str3, str4) } + +func Test_OptionalBool(t *testing.T) { + assert.Equal(t, OptionalBoolNone, OptionalBoolParse("")) + assert.Equal(t, OptionalBoolNone, OptionalBoolParse("x")) + + assert.Equal(t, OptionalBoolFalse, OptionalBoolParse("0")) + assert.Equal(t, OptionalBoolFalse, OptionalBoolParse("f")) + assert.Equal(t, OptionalBoolFalse, OptionalBoolParse("False")) + + assert.Equal(t, OptionalBoolTrue, OptionalBoolParse("1")) + assert.Equal(t, OptionalBoolTrue, OptionalBoolParse("t")) + assert.Equal(t, OptionalBoolTrue, OptionalBoolParse("True")) +} diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 190d3fa980944..32ecd2cb1a3b5 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -68,9 +68,15 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN statusFilterMap[filterKey] = ctx.FormString("status_filter[" + filterKey + "]") } + opts.IsActive = util.OptionalBoolParse(statusFilterMap["is_active"]) + opts.IsAdmin = util.OptionalBoolParse(statusFilterMap["is_admin"]) + opts.IsRestricted = util.OptionalBoolParse(statusFilterMap["is_restricted"]) + opts.IsTwoFactorEnabled = util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]) + opts.IsProhibitLogin = util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]) + opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy - opts.StatusFilterMap = statusFilterMap + if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) { users, count, err = models.SearchUsers(opts) if err != nil { From e1336121a680e63c89b3cf31721cecf887f366a3 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Thu, 26 Aug 2021 15:03:22 +0800 Subject: [PATCH 06/22] refactor SearchUserOptions.toConds to SearchUserOptions.toSearchQueryBase --- models/user.go | 50 ++++++++++++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/models/user.go b/models/user.go index fdaf485aca5e6..cb575b43e66a1 100644 --- a/models/user.go +++ b/models/user.go @@ -1605,7 +1605,7 @@ type SearchUserOptions struct { IsProhibitLogin util.OptionalBool } -func (opts *SearchUserOptions) toConds() builder.Cond { +func (opts *SearchUserOptions) toSearchQueryBase() (sess *xorm.Session) { var cond builder.Cond = builder.Eq{"type": opts.Type} if len(opts.Keyword) > 0 { lowerKeyword := strings.ToLower(opts.Keyword) @@ -1671,32 +1671,38 @@ func (opts *SearchUserOptions) toConds() builder.Cond { cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.IsTrue()}) } - return cond -} - -// SearchUsers takes options i.e. keyword and part of user name to search, -// it returns results in given range and number of total results. -func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { - cond := opts.toConds() + type Join struct { + Operator string + Table interface{} + Condition string + Args []interface{} + } - searchUserBaseQuery := func() (sess *xorm.Session) { - sess = x.Where(cond) + var joins []*Join - if !opts.IsTwoFactorEnabled.IsNone() { - // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record - // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future - sess = sess.Join("LEFT OUTER", "two_factor", "two_factor.uid = `user`.id") - if opts.IsTwoFactorEnabled.IsTrue() { - cond = cond.And(builder.Expr("two_factor.uid IS NOT NULL")) - } else { - cond = cond.And(builder.Expr("two_factor.uid IS NULL")) - } + if !opts.IsTwoFactorEnabled.IsNone() { + // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record + // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future + if opts.IsTwoFactorEnabled.IsTrue() { + cond = cond.And(builder.Expr("two_factor.uid IS NOT NULL")) + } else { + cond = cond.And(builder.Expr("two_factor.uid IS NULL")) } + joins = append(joins, &Join{Operator: "LEFT OUTER", Table: "two_factor", Condition: "two_factor.uid = `user`.id"}) + } - return sess + sess = x.Where(cond) + for _, join := range joins { + sess = sess.Join(join.Operator, join.Table, join.Condition, join.Args...) } - count, err := searchUserBaseQuery().Count(new(User)) + return sess +} + +// SearchUsers takes options i.e. keyword and part of user name to search, +// it returns results in given range and number of total results. +func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { + count, err := opts.toSearchQueryBase().Count(new(User)) if err != nil { return nil, 0, fmt.Errorf("Count: %v", err) } @@ -1705,7 +1711,7 @@ func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { opts.OrderBy = SearchOrderByAlphabetically } - sess := searchUserBaseQuery().OrderBy(opts.OrderBy.String()) + sess := opts.toSearchQueryBase().OrderBy(opts.OrderBy.String()) if opts.Page != 0 { sess = opts.setSessionPagination(sess) } From 86a245a4f2260d2c7d85671280739bfffd9d7b78 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Thu, 26 Aug 2021 15:19:07 +0800 Subject: [PATCH 07/22] add unit test for user search --- models/fixtures/user.yml | 1 + models/user_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/models/fixtures/user.yml b/models/fixtures/user.yml index 850ee4041d81a..c49fe1b656a22 100644 --- a/models/fixtures/user.yml +++ b/models/fixtures/user.yml @@ -524,6 +524,7 @@ avatar_email: user30@example.com num_repos: 2 is_active: true + prohibit_login: true - id: 31 diff --git a/models/user_test.go b/models/user_test.go index a76bca0ed557f..5fc0c0fdc52e7 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -151,6 +151,18 @@ func TestSearchUsers(t *testing.T) { // order by name asc default testUserSuccess(&SearchUserOptions{Keyword: "user1", ListOptions: ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, []int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) + + testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsAdmin: util.OptionalBoolTrue}, + []int64{1}) + + testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsRestricted: util.OptionalBoolTrue}, + []int64{29, 30}) + + testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsProhibitLogin: util.OptionalBoolTrue}, + []int64{30}) + + testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsTwoFactorEnabled: util.OptionalBoolTrue}, + []int64{24}) } func TestDeleteUser(t *testing.T) { From 3296672e4cfe7c049e690d4f832a2dea22bba5cf Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Sat, 4 Sep 2021 01:20:16 +0800 Subject: [PATCH 08/22] only allow admin to use filters to search users --- routers/web/admin/users.go | 21 +++++++++++++++++++-- routers/web/explore/user.go | 13 ------------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index acccc516bb456..51e6be7440caf 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/web/explore" router_user_setting "code.gitea.io/gitea/routers/web/user/setting" @@ -36,14 +37,30 @@ func Users(ctx *context.Context) { ctx.Data["PageIsAdmin"] = true ctx.Data["PageIsAdminUsers"] = true - explore.RenderUserSearch(ctx, &models.SearchUserOptions{ + searchOpts := &models.SearchUserOptions{ Actor: ctx.User, Type: models.UserTypeIndividual, ListOptions: models.ListOptions{ PageSize: setting.UI.Admin.UserPagingNum, }, SearchByEmail: true, - }, tplUsers) + } + + statusFilterKeys := []string{"is_active", "is_admin", "is_restricted", "is_2fa_enabled", "is_prohibit_login"} + statusFilterMap := map[string]string{} + for _, filterKey := range statusFilterKeys { + statusFilterMap[filterKey] = ctx.FormString("status_filter[" + filterKey + "]") + } + + searchOpts.IsActive = util.OptionalBoolParse(statusFilterMap["is_active"]) + searchOpts.IsAdmin = util.OptionalBoolParse(statusFilterMap["is_admin"]) + searchOpts.IsRestricted = util.OptionalBoolParse(statusFilterMap["is_restricted"]) + searchOpts.IsTwoFactorEnabled = util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]) + searchOpts.IsProhibitLogin = util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]) + + ctx.Data["StatusFilterMap"] = statusFilterMap + + explore.RenderUserSearch(ctx, searchOpts, tplUsers) } // NewUser render adding a new user page diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 32ecd2cb1a3b5..f222d83ab7fc2 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -62,18 +62,6 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN orderBy = models.SearchOrderByAlphabetically } - statusFilterKeys := []string{"is_active", "is_admin", "is_restricted", "is_2fa_enabled", "is_prohibit_login"} - statusFilterMap := map[string]string{} - for _, filterKey := range statusFilterKeys { - statusFilterMap[filterKey] = ctx.FormString("status_filter[" + filterKey + "]") - } - - opts.IsActive = util.OptionalBoolParse(statusFilterMap["is_active"]) - opts.IsAdmin = util.OptionalBoolParse(statusFilterMap["is_admin"]) - opts.IsRestricted = util.OptionalBoolParse(statusFilterMap["is_restricted"]) - opts.IsTwoFactorEnabled = util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]) - opts.IsProhibitLogin = util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]) - opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy @@ -86,7 +74,6 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN } ctx.Data["Keyword"] = opts.Keyword - ctx.Data["StatusFilterMap"] = statusFilterMap ctx.Data["Total"] = count ctx.Data["Users"] = users ctx.Data["UsersTwoFaStatus"] = models.UserList(users).GetTwoFaStatus() From f6f06f20673484009823ace73bed59ef9ff7f942 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Sat, 4 Sep 2021 01:26:05 +0800 Subject: [PATCH 09/22] reformat --- routers/web/explore/user.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index f222d83ab7fc2..aeaaf92c12216 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -64,7 +64,6 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN opts.Keyword = ctx.FormTrim("q") opts.OrderBy = orderBy - if len(opts.Keyword) == 0 || isKeywordValid(opts.Keyword) { users, count, err = models.SearchUsers(opts) if err != nil { @@ -72,7 +71,6 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN return } } - ctx.Data["Keyword"] = opts.Keyword ctx.Data["Total"] = count ctx.Data["Users"] = users From df32fb4c37712fba93189479856b8ebe3f04eff8 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Sun, 5 Sep 2021 04:27:21 +0800 Subject: [PATCH 10/22] fix search query: Where and Join --- models/user.go | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/models/user.go b/models/user.go index cb575b43e66a1..fe3acc4892c7e 100644 --- a/models/user.go +++ b/models/user.go @@ -1671,15 +1671,7 @@ func (opts *SearchUserOptions) toSearchQueryBase() (sess *xorm.Session) { cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.IsTrue()}) } - type Join struct { - Operator string - Table interface{} - Condition string - Args []interface{} - } - - var joins []*Join - + sess = x.NewSession() if !opts.IsTwoFactorEnabled.IsNone() { // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future @@ -1688,21 +1680,18 @@ func (opts *SearchUserOptions) toSearchQueryBase() (sess *xorm.Session) { } else { cond = cond.And(builder.Expr("two_factor.uid IS NULL")) } - joins = append(joins, &Join{Operator: "LEFT OUTER", Table: "two_factor", Condition: "two_factor.uid = `user`.id"}) + sess = sess.Join("LEFT OUTER", "two_factor", "two_factor.uid = `user`.id") } - - sess = x.Where(cond) - for _, join := range joins { - sess = sess.Join(join.Operator, join.Table, join.Condition, join.Args...) - } - + sess = sess.Where(cond) return sess } // SearchUsers takes options i.e. keyword and part of user name to search, // it returns results in given range and number of total results. func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { - count, err := opts.toSearchQueryBase().Count(new(User)) + sessCount := opts.toSearchQueryBase() + defer sessCount.Close() + count, err := sessCount.Count(new(User)) if err != nil { return nil, 0, fmt.Errorf("Count: %v", err) } @@ -1711,13 +1700,14 @@ func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { opts.OrderBy = SearchOrderByAlphabetically } - sess := opts.toSearchQueryBase().OrderBy(opts.OrderBy.String()) + sessQuery := opts.toSearchQueryBase().OrderBy(opts.OrderBy.String()) + defer sessQuery.Close() if opts.Page != 0 { - sess = opts.setSessionPagination(sess) + sessQuery = opts.setSessionPagination(sessQuery) } users = make([]*User, 0, opts.PageSize) - return users, count, sess.Find(&users) + return users, count, sessQuery.Find(&users) } // GetStarredRepos returns the repos starred by a particular user From 89b35647c9171cb2d8ba8f54e1977b0c04513fe9 Mon Sep 17 00:00:00 2001 From: Xiaoguang Wang Date: Wed, 22 Sep 2021 14:18:37 +0800 Subject: [PATCH 11/22] fix merge conflict --- models/user.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/user.go b/models/user.go index 06fc0eb6433df..08a887b85793c 100644 --- a/models/user.go +++ b/models/user.go @@ -1676,7 +1676,7 @@ func (opts *SearchUserOptions) toSearchQueryBase() (sess *xorm.Session) { cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.IsTrue()}) } - sess = x.NewSession() + sess = db.DefaultContext().NewSession() if !opts.IsTwoFactorEnabled.IsNone() { // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future From 6bf9c07cca2bf05e9890b77b3c2e531b76059176 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Sep 2021 13:25:24 +0800 Subject: [PATCH 12/22] fix merge --- models/user.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/user.go b/models/user.go index 6e8d385fd8571..3b9b8d6f7a5dd 100644 --- a/models/user.go +++ b/models/user.go @@ -1677,7 +1677,7 @@ func (opts *SearchUserOptions) toSearchQueryBase() (sess *xorm.Session) { cond = cond.And(builder.Eq{"prohibit_login": opts.IsProhibitLogin.IsTrue()}) } - sess = db.DefaultContext().NewSession() + sess = db.NewSession(db.DefaultContext) if !opts.IsTwoFactorEnabled.IsNone() { // 2fa filter uses LEFT JOIN to check whether a user has a 2fa record // TODO: bad performance here, maybe there will be a column "is_2fa_enabled" in the future @@ -1709,7 +1709,7 @@ func SearchUsers(opts *SearchUserOptions) (users []*User, _ int64, _ error) { sessQuery := opts.toSearchQueryBase().OrderBy(opts.OrderBy.String()) defer sessQuery.Close() if opts.Page != 0 { - sessQuery = setSessionPagination(sessQuery, opts) + sessQuery = db.SetSessionPagination(sessQuery, opts) } users = make([]*User, 0, opts.PageSize) From a0a963abbf78bf4566b4c971fb99b443978d9193 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 26 Sep 2021 13:31:13 +0800 Subject: [PATCH 13/22] fix unit test --- models/user_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/models/user_test.go b/models/user_test.go index 9f0fc753a7b20..2dcca20346b61 100644 --- a/models/user_test.go +++ b/models/user_test.go @@ -162,16 +162,16 @@ func TestSearchUsers(t *testing.T) { testUserSuccess(&SearchUserOptions{Keyword: "user1", ListOptions: db.ListOptions{Page: 1}, IsActive: util.OptionalBoolTrue}, []int64{1, 10, 11, 12, 13, 14, 15, 16, 18}) - testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsAdmin: util.OptionalBoolTrue}, + testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsAdmin: util.OptionalBoolTrue}, []int64{1}) - testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsRestricted: util.OptionalBoolTrue}, + testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsRestricted: util.OptionalBoolTrue}, []int64{29, 30}) - testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsProhibitLogin: util.OptionalBoolTrue}, + testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsProhibitLogin: util.OptionalBoolTrue}, []int64{30}) - testUserSuccess(&SearchUserOptions{ListOptions: ListOptions{Page: 1}, IsTwoFactorEnabled: util.OptionalBoolTrue}, + testUserSuccess(&SearchUserOptions{ListOptions: db.ListOptions{Page: 1}, IsTwoFactorEnabled: util.OptionalBoolTrue}, []int64{24}) } From 2148ce50180547eba7efe3025f8dc1d42ddda61e Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 27 Sep 2021 11:17:09 +0800 Subject: [PATCH 14/22] refactor --- .eslintrc | 1 - modules/context/context.go | 11 +++++++---- modules/templates/helper.go | 6 ++++-- routers/web/admin/users.go | 9 ++++++++- routers/web/explore/user.go | 20 +++++++++++--------- templates/admin/user/list.tmpl | 11 ----------- templates/base/head.tmpl | 4 +++- web_src/js/features/admin-users.js | 8 ++++---- 8 files changed, 37 insertions(+), 33 deletions(-) diff --git a/.eslintrc b/.eslintrc index 98f84349552cc..bab34478cf464 100644 --- a/.eslintrc +++ b/.eslintrc @@ -3,7 +3,6 @@ reportUnusedDisableDirectives: true ignorePatterns: - /web_src/js/vendor - - /templates/base/head.tmpl - /templates/repo/activity.tmpl - /templates/repo/view_file.tmpl diff --git a/modules/context/context.go b/modules/context/context.go index 3dbc2bb9dcd0f..5f4cf79c376c8 100644 --- a/modules/context/context.go +++ b/modules/context/context.go @@ -48,10 +48,11 @@ type Render interface { // Context represents context of a request. type Context struct { - Resp ResponseWriter - Req *http.Request - Data map[string]interface{} - Render Render + Resp ResponseWriter + Req *http.Request + Data map[string]interface{} // data used by MVC templates + PageData map[string]interface{} // data used by JavaScript modules in one page + Render Render translation.Locale Cache cache.Cache csrf CSRF @@ -635,6 +636,8 @@ func Contexter() func(next http.Handler) http.Handler { "Link": link, }, } + ctx.PageData = map[string]interface{}{} + ctx.Data["PageData"] = ctx.PageData // by reference ctx.Req = WithContext(req, &ctx) ctx.csrf = Csrfer(csrfOpts, &ctx) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 27d81ec9d9b94..e56a9cfcdea36 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -350,12 +350,14 @@ func NewFuncMap() []template.FuncMap { } } else { // if sort arg is in url test if it correlates with column header sort arguments + // the direction of the arrow should indicate the "current sort order", up means ASC(normal), down means DESC(rev) if urlSort == normSort { // the table is sorted with this header normal - return SVG("octicon-triangle-down", 16) + return SVG("octicon-triangle-up", 16) } else if urlSort == revSort { // the table is sorted with this header reverse - return SVG("octicon-triangle-up", 16) + return SVG("octicon-triangle-down", 16) + } } // the table is NOT sorted with this header diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 62d1348528974..d7ef4e5e1e083 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -60,7 +60,14 @@ func Users(ctx *context.Context) { searchOpts.IsTwoFactorEnabled = util.OptionalBoolParse(statusFilterMap["is_2fa_enabled"]) searchOpts.IsProhibitLogin = util.OptionalBoolParse(statusFilterMap["is_prohibit_login"]) - ctx.Data["StatusFilterMap"] = statusFilterMap + sortType := ctx.FormString("sort") + if sortType == "" { + sortType = explore.UserSearchDefaultSortType + } + ctx.PageData["AdminUserListSearchForm"] = map[string]interface{}{ + "StatusFilterMap": statusFilterMap, + "SortType": sortType, + } explore.RenderUserSearch(ctx, searchOpts, tplUsers) } diff --git a/routers/web/explore/user.go b/routers/web/explore/user.go index 4ddb90132d165..f5f9836a44afb 100644 --- a/routers/web/explore/user.go +++ b/routers/web/explore/user.go @@ -22,6 +22,8 @@ const ( tplExploreUsers base.TplName = "explore/users" ) +const UserSearchDefaultSortType = "alphabetically" + var ( nullByte = []byte{0x00} ) @@ -44,23 +46,23 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN orderBy models.SearchOrderBy ) + // 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 = models.SearchOrderByIDReverse + orderBy = "user.id DESC" case "oldest": - orderBy = models.SearchOrderByID + orderBy = "user.id ASC" case "recentupdate": - orderBy = models.SearchOrderByRecentUpdated + orderBy = "user.updated_unix DESC" case "leastupdate": - orderBy = models.SearchOrderByLeastUpdated + orderBy = "user.updated_unix ASC" case "reversealphabetically": - orderBy = models.SearchOrderByAlphabeticallyReverse - case "alphabetically": - orderBy = models.SearchOrderByAlphabetically + orderBy = "user.name DESC" + case UserSearchDefaultSortType: // "alphabetically" default: - ctx.Data["SortType"] = "alphabetically" - orderBy = models.SearchOrderByAlphabetically + orderBy = "user.name ASC" + ctx.Data["SortType"] = UserSearchDefaultSortType } opts.Keyword = ctx.FormTrim("q") diff --git a/templates/admin/user/list.tmpl b/templates/admin/user/list.tmpl index c9f1761ee8f85..33ec0b19965bb 100644 --- a/templates/admin/user/list.tmpl +++ b/templates/admin/user/list.tmpl @@ -58,17 +58,6 @@ - - {{/* here we have valid go template syntax, but eslint doesn't like it and reports "error Parsing error: Unexpected token {" */}} -
diff --git a/templates/base/head.tmpl b/templates/base/head.tmpl index 8177bb915b9db..32b5a2e3a9215 100644 --- a/templates/base/head.tmpl +++ b/templates/base/head.tmpl @@ -25,7 +25,9 @@ {{end}} + {{/* here we have valid go template syntax, but eslint doesn't like it and reports "error Parsing error: Unexpected token {" */}}