Sort repositories by watch status#19
Conversation
|
Please reopen against master. |
models/access.go
Outdated
There was a problem hiding this comment.
I really dislike this part to check for the used DB
There was a problem hiding this comment.
It has to be done, PostgreSQL uses different syntax.
There was a problem hiding this comment.
But you don't need to do that if you use XORM properly
There was a problem hiding this comment.
Would you like to suggest changes? I replicated what @unknwon did previously.
There was a problem hiding this comment.
Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)
There was a problem hiding this comment.
@tboerger I understand where you are coming from but this style of compensating for PostgreSQL is present throughout the existing codebase. I want to propose this be merged first before optimisations be carried out throughout.
There was a problem hiding this comment.
There's no need to keep these separate, go for the one PostgreSQL needs, since MySQL and SQLite handles that format as well.
models/repo.go
Outdated
There was a problem hiding this comment.
I really dislike this part to check for the used DB
There was a problem hiding this comment.
It has to be done, PostgreSQL uses different syntax.
There was a problem hiding this comment.
But you don't need to do that if you use XORM properly
There was a problem hiding this comment.
Would you like to suggest changes? I replicated what @unknwon did previously.
There was a problem hiding this comment.
Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)
There was a problem hiding this comment.
@tboerger I understand where you are coming from but this style of compensating for PostgreSQL is present throughout the existing codebase. I want to propose this be merged first before optimisations be carried out throughout.
models/repo.go
Outdated
There was a problem hiding this comment.
I really dislike this part to check for the used DB
There was a problem hiding this comment.
It has to be done, PostgreSQL uses different syntax.
There was a problem hiding this comment.
But you don't need to do that if you use XORM properly
There was a problem hiding this comment.
Would you like to suggest changes? I replicated what @unknwon did previously.
There was a problem hiding this comment.
Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)
There was a problem hiding this comment.
@tboerger I understand where you are coming from but this style of compensating for PostgreSQL is present throughout the existing codebase. I want to propose this be merged first before optimisations be carried out throughout.
models/access.go
Outdated
There was a problem hiding this comment.
order by uid desc will disable order by updated_unix desc.
There was a problem hiding this comment.
Will it? How do I get it to do an ORDER BY "uid" desc, "updated_unix" desc?
There was a problem hiding this comment.
I mean it will disable on logic not the generated SQL. The code is right.
There was a problem hiding this comment.
@lunny this seems to be the only thing left to resolve here, how do you get multi-column sort with xorm ?
There was a problem hiding this comment.
@LefsFlarey @strk seems useless, since UID are never duplicates :)
There was a problem hiding this comment.
@bkcsoft good point ! Besides, how does that have to do with "watch status" sorting in the title of this PR ?
|
LGTM (where's the automatic labels addition by lgtm gone ?) |
|
@LefsFlarey there are file conflicts. |
This fix would sort user repositories by whether they have starred them, themselves.
Current coverage is 3.13% (diff: 0.00%)@@ master #19 diff @@
========================================
Files 33 33
Lines 7823 7836 +13
Methods 0 0
Messages 0 0
Branches 0 0
========================================
Hits 246 246
- Misses 7557 7570 +13
Partials 20 20
|
| // If limit is smaller than 1 means returns all found results. | ||
| func (user *User) GetAccessibleRepositories(limit int) (repos []*Repository, _ error) { | ||
| sess := x.Where("owner_id !=? ", user.ID).Desc("updated_unix") | ||
| sess := x.Where("owner_id !=? ", user.ID).Desc("uid").Desc("updated_unix") |
There was a problem hiding this comment.
For a better readability and to get the real line / instruction in case of error, please set one XORM instruction per line
eg :
x.Where("owner_id !=? ", user.ID).
Desc("uid").
Desc("updated_unix")
or
x.
Where("owner_id !=? ", user.ID).
Desc("uid").
Desc("updated_unix")
| // GetUserRepositories returns a list of repositories of given user. | ||
| func GetUserRepositories(userID int64, private bool, page, pageSize int) ([]*Repository, error) { | ||
| sess := x.Where("owner_id = ?", userID).Desc("updated_unix") | ||
| sess := x.Where("owner_id = ?", userID).Desc("uid").Desc("updated_unix") |
There was a problem hiding this comment.
For a better readability and to get the real line / instruction in case of error, please set one XORM instruction per line
eg :
x.Where("owner_id !=? ", user.ID).
Desc("uid").
Desc("updated_unix")
or
x.
Where("owner_id !=? ", user.ID).
Desc("uid").
Desc("updated_unix")
| func GetUserMirrorRepositories(userID int64) ([]*Repository, error) { | ||
| repos := make([]*Repository, 0, 10) | ||
| return repos, x.Where("owner_id = ?", userID).And("is_mirror = ?", true).Find(&repos) | ||
| sess := x.Where("owner_id = ?", userID).And("is_mirror = ?", true).Desc("uid") |
There was a problem hiding this comment.
For a better readability and to get the real line / instruction in case of error, please set one XORM instruction per line
eg :
sess := x.Where("owner_id = ?", userID).
And("is_mirror = ?", true).
Desc("uid")
or
sess := x.
Where("owner_id = ?", userID).
And("is_mirror = ?", true).
Desc("uid")
|
They are file conflicts since all raw query has been refactored to use XORM in a previous PR |
|
@LefsFlarey why have you closed this? |
TL;DR
This PR is a feature addition to sort user repositories by their star/favourite status.
Why?
Many times, I find myself with over 20-ish repositories cluttering up my main page when I am only actively maintaining 2-3 repositories. I have always wanted a simple way to sort them thus I decided to code a quick fix to sort the repositories according to whether I starred them.
Note that this PR does not sort repositories based on number of stars but rather if you, the creator, have starred them yourself.