Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix sorting users by last login date #14947

Closed
wants to merge 2 commits into from
Closed

Fix sorting users by last login date #14947

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e05f340
fix admin user sorting
noerw Mar 10, 2021
d0c7151
fix for repo listing too
noerw Mar 10, 2021
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
12 changes: 8 additions & 4 deletions models/repo_list.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,14 @@ const (
SearchOrderBySizeReverse SearchOrderBy = "size DESC"
SearchOrderByID SearchOrderBy = "id ASC"
SearchOrderByIDReverse SearchOrderBy = "id DESC"
SearchOrderByStars SearchOrderBy = "num_stars ASC"
SearchOrderByStarsReverse SearchOrderBy = "num_stars DESC"
SearchOrderByForks SearchOrderBy = "num_forks ASC"
SearchOrderByForksReverse SearchOrderBy = "num_forks DESC"
// repo
SearchOrderByStars SearchOrderBy = "num_stars ASC"
SearchOrderByStarsReverse SearchOrderBy = "num_stars DESC"
SearchOrderByForks SearchOrderBy = "num_forks ASC"
SearchOrderByForksReverse SearchOrderBy = "num_forks DESC"
// user
SearchOrderByRecentLogin SearchOrderBy = "last_login_unix ASC"
SearchOrderByRecentLoginReverse SearchOrderBy = "last_login_unix DESC"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be here or in models/user.go? I think it's neat to have them all grouped together

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already refactored them. So no worry now.

)

// SearchRepositoryCondition creates a query condition according search repository options
Expand Down
4 changes: 4 additions & 0 deletions routers/home.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@ func RenderUserSearch(ctx *context.Context, opts *models.SearchUserOptions, tplN
orderBy = models.SearchOrderByRecentUpdated
case "leastupdate":
orderBy = models.SearchOrderByLeastUpdated
case "recentlogin":
orderBy = models.SearchOrderByRecentLogin
case "recentloginreverse":
orderBy = models.SearchOrderByRecentLoginReverse
case "reversealphabetically":
orderBy = models.SearchOrderByAlphabeticallyReverse
case "alphabetically":
Expand Down
2 changes: 2 additions & 0 deletions templates/admin/base/search.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
<a class="{{if eq .SortType "reversealphabetically"}}active{{end}} item" href="{{$.Link}}?sort=reversealphabetically&q={{$.Keyword}}">{{.i18n.Tr "repo.issues.label.filter_sort.reverse_alphabetically"}}</a>
<a class="{{if eq .SortType "recentupdate"}}active{{end}} item" href="{{$.Link}}?sort=recentupdate&q={{$.Keyword}}">{{.i18n.Tr "repo.issues.filter_sort.recentupdate"}}</a>
<a class="{{if eq .SortType "leastupdate"}}active{{end}} item" href="{{$.Link}}?sort=leastupdate&q={{$.Keyword}}">{{.i18n.Tr "repo.issues.filter_sort.leastupdate"}}</a>
<a class="{{if eq .SortType "recentlogin"}}active{{end}} item" href="{{$.Link}}?sort=recentlogin&q={{$.Keyword}}">{{.i18n.Tr "repo.issues.filter_sort.recentlogin"}}</a>
<a class="{{if eq .SortType "recentloginreverse"}}active{{end}} item" href="{{$.Link}}?sort=recentloginreverse&q={{$.Keyword}}">{{.i18n.Tr "repo.issues.filter_sort.recentloginreverse"}}</a>
Copy link
Member Author

@noerw noerw Mar 10, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should add specific sorters like this to the dropdown, it is used for orgs listing as well where it doesn't apply.
In general it's a bit weird, that this template is shared by user & org listing, while an almost identical search template explore/search.tmpl exists, that actually would benefit from being shared between admin & explore listing..
Opinions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please share templates if possible, we need to reduce duplication. What you could do is something like we already do for issue lists:

{{template "shared/issuelist" mergeinto . "listType" "dashboard"}}

This would merge a additional listType=dashboard property into the root context ., making it available to the sub-template. Keep note that mergeinto does clone the context once, so it'll induce a tiny performance hit, thought that should be okay for one-time rendered things like these lists.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need the shared templates anymore. See the latest code.

And shared templates bring more problems than they resolve.

</div>
</div>
</div>
Expand Down
5 changes: 1 addition & 4 deletions templates/admin/org/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@
<th>{{.i18n.Tr "admin.orgs.teams"}}</th>
<th>{{.i18n.Tr "admin.orgs.members"}}</th>
<th>{{.i18n.Tr "admin.users.repos"}}</th>
<th data-sortt-asc="recentupdate" data-sortt-desc="leastupdate">
{{.i18n.Tr "admin.users.created"}}
{{SortArrow "recentupdate" "leastupdate" $.SortType false}}
</th>
<th>{{.i18n.Tr "admin.users.created"}}</th>
<th>{{.i18n.Tr "admin.users.edit"}}</th>
</tr>
</thead>
Expand Down
4 changes: 2 additions & 2 deletions templates/admin/user/list.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@
<th>{{.i18n.Tr "admin.users.2fa"}}</th>
<th>{{.i18n.Tr "admin.users.repos"}}</th>
<th>{{.i18n.Tr "admin.users.created"}}</th>
<th data-sortt-asc="recentupdate" data-sortt-desc="leastupdate">
<th data-sortt-asc="recentlogin" data-sortt-desc="recentloginreverse">
{{.i18n.Tr "admin.users.last_login"}}
{{SortArrow "recentupdate" "leastupdate" $.SortType false}}
{{SortArrow "recentlogin" "recentloginreverse" $.SortType false}}
</th>
<th>{{.i18n.Tr "admin.users.edit"}}</th>
</tr>
Expand Down