Using db.SearchOrderBy in internal instead of sortType #11195

New Issue

GiteaMirror · 2025-11-02T09:30:28-06:00

GiteaMirror commented

2025-11-02 09:30:28 -06:00

Originally created by @yp05327 on GitHub (Jul 10, 2023).

Feature Description

As we have a globally db.SearchOrderBy for sorting, in some places we still using xorm.Session.Asc(col_name) and xorm.Session.Desc(col_name) which may make mistakes like #25806 and hard to maintain.

I have made a simple keyword search, and found there are two places is using Asc and Desc directly:

models/git/commit_status.go: func sortCommitStatusesSession
models/issues/issue_search.go: func applySorts

Screenshots

No response

Originally created by @yp05327 on GitHub (Jul 10, 2023). ### Feature Description As we have a globally `db.SearchOrderBy` for sorting, in some places we still using `xorm.Session.Asc(col_name)` and `xorm.Session.Desc(col_name)` which may make mistakes like #25806 and hard to maintain. I have made a simple keyword search, and found there are two places is using `Asc` and `Desc` directly: - models/git/commit_status.go: func sortCommitStatusesSession - models/issues/issue_search.go: func applySorts ### Screenshots _No response_

GiteaMirror added the type/proposal label 2025-11-02 09:30:28 -06:00

GiteaMirror commented

2025-11-02 09:30:29 -06:00

@CaiCandong commented on GitHub (Jul 13, 2023):

I understand what you mean, reuse db.SearchOrderBy as much as possible to avoid bug, but we can only reuse as much as we can, there are still some complex cases where we can't avoid the need to use xorm.Session.Asc(col_name) and xorm.Session.Desc(col_name) directly. For example models/issues/issue_search.go: func applySorts.
So I think your previous modification #25806 was somewhat inappropriate : you change SortType string to OrderBy db.SearchOrderBy ,I don't think SearchOrderBy can cover all cases, and the role of SearchOrderBy should just be to provide some reusable sorting logic。 I'm going to propose a PR to implement it, and I'll probably make some changes to the commit you made in #25806.

@CaiCandong commented on GitHub (Jul 13, 2023): I understand what you mean, reuse db.SearchOrderBy as much as possible to avoid bug, but we can only reuse as much as we can, there are still some complex cases where we can't avoid the need to use `xorm.Session.Asc(col_name)` and `xorm.Session.Desc(col_name)` directly. For example `models/issues/issue_search.go: func applySorts`. So I think your previous modification #25806 was somewhat inappropriate : you change `SortType string` to `OrderBy db.SearchOrderBy` ,I don't think `SearchOrderBy` can cover all cases, and the role of `SearchOrderBy` should just be to provide some reusable sorting logic。 I'm going to propose a PR to implement it, and I'll probably make some changes to the commit you made in #25806.

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/gitea#11195