Sort repositories by watch status #19

ghost · 2016-11-03T02:47:26Z

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.

tboerger · 2016-11-03T08:21:08Z

Please reopen against master.

tboerger · 2016-11-04T08:44:06Z

models/access.go

 	if limit > 0 {
 		sess.Limit(limit)
 		repos = make([]*Repository, 0, limit)
 	} else {
 		repos = make([]*Repository, 0, 10)
 	}
+
+	if setting.UsePostgreSQL {


I really dislike this part to check for the used DB

It has to be done, PostgreSQL uses different syntax.

But you don't need to do that if you use XORM properly

Would you like to suggest changes? I replicated what @unknwon did previously.

Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)

@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's no need to keep these separate, go for the one PostgreSQL needs, since MySQL and SQLite handles that format as well.

tboerger · 2016-11-04T08:44:11Z

models/repo.go

-	sess := x.Where("owner_id = ?", userID).Desc("updated_unix")
+	sess := x.Where("owner_id = ?", userID).Desc("uid").Desc("updated_unix")
+
+	if setting.UsePostgreSQL {


I really dislike this part to check for the used DB

It has to be done, PostgreSQL uses different syntax.

But you don't need to do that if you use XORM properly

Would you like to suggest changes? I replicated what @unknwon did previously.

Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)

@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.

tboerger · 2016-11-04T08:44:18Z

models/repo.go

-	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")
+
+	if setting.UsePostgreSQL {


I really dislike this part to check for the used DB

It has to be done, PostgreSQL uses different syntax.

But you don't need to do that if you use XORM properly

Would you like to suggest changes? I replicated what @unknwon did previously.

Just look at the XORM docs, or maybe @lunny can give you a hint since he's the XORM author ;)

@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.

lunny · 2016-11-03T07:21:51Z

models/access.go

@@ -124,13 +125,20 @@ func (u *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 // GetAccessibleRepositories finds repositories which the user has access but does not own.
 // 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")


order by uid desc will disable order by updated_unix desc.

Will it? How do I get it to do an ORDER BY "uid" desc, "updated_unix" desc?

I mean it will disable on logic not the generated SQL. The code is right.

@lunny this seems to be the only thing left to resolve here, how do you get multi-column sort with xorm ?

LGTM then. That's right!

@LefsFlarey @strk seems useless, since UID are never duplicates :)

@bkcsoft good point ! Besides, how does that have to do with "watch status" sorting in the title of this PR ?

strk · 2016-11-08T08:01:42Z

LGTM (where's the automatic labels addition by lgtm gone ?)

lunny · 2016-11-08T09:50:01Z

@LefsFlarey there are file conflicts.

This fix would sort user repositories by whether they have starred them, themselves.

codecov-io · 2016-11-08T14:13:22Z

Current coverage is 3.13% (diff: 0.00%)

Merging #19 into master will decrease coverage by <.01%

@@            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

Powered by Codecov. Last update f6d53ec...0eea5f5

thibaultmeyer · 2016-11-13T08:43:12Z

models/access.go

@@ -124,13 +125,20 @@ func (u *User) GetRepositoryAccesses() (map[*Repository]AccessMode, error) {
 // GetAccessibleRepositories finds repositories which the user has access but does not own.
 // 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")


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")

thibaultmeyer · 2016-11-13T08:43:43Z

models/repo.go

@@ -1450,7 +1450,14 @@ func GetRepositoryByID(id int64) (*Repository, error) {

 // 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")


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")

thibaultmeyer · 2016-11-13T08:44:28Z

models/repo.go

@@ -1467,7 +1474,15 @@ func GetUserRepositories(userID int64, private bool, page, pageSize int) ([]*Rep
 // GetUserRepositories returns a list of mirror repositories of given user.
 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")


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")

thibaultmeyer · 2016-11-13T08:46:11Z

They are file conflicts since all raw query has been refactored to use XORM in a previous PR

tboerger · 2016-11-15T21:08:52Z

@LefsFlarey why have you closed this?

Add lint check

ghost added the type/enhancement An improvement of existing functionality label Nov 3, 2016

tboerger closed this Nov 3, 2016

tboerger added invalid and removed type/enhancement An improvement of existing functionality labels Nov 3, 2016

bkcsoft changed the base branch from develop to master November 4, 2016 04:17

bkcsoft added type/enhancement An improvement of existing functionality and removed reviewed/invalid labels Nov 4, 2016

bkcsoft reopened this Nov 4, 2016

tboerger added this to the 1.0.0 milestone Nov 4, 2016

tboerger suggested changes Nov 4, 2016

View reviewed changes

lunny reviewed Nov 4, 2016

View reviewed changes

strk modified the milestones: 1.1.0, 1.0.0 Nov 6, 2016

DblK added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 8, 2016

LefsFlare added 4 commits November 8, 2016 21:59

Sort repositories by user's stars

a27909a

This fix would sort user repositories by whether they have starred them, themselves.

Sort collaborative repositories by user star

0329f15

Sort mirror repositories by star status

2a0aec3

Fixed compile errors

0eea5f5

thibaultmeyer suggested changes Nov 13, 2016

View reviewed changes

ghost closed this Nov 14, 2016

tboerger removed the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Nov 15, 2016

ghost deleted the feature/sort-repos-by-star branch November 25, 2016 01:38

tboerger added reviewed/invalid and removed type/enhancement An improvement of existing functionality labels Nov 29, 2016

tboerger removed this from the 1.1.0 milestone Nov 29, 2016

ghost restored the feature/sort-repos-by-star branch December 11, 2016 05:39

lunny referenced this pull request in lunny/gitea Feb 7, 2019

Merge pull request #19 from lunny/lunny/add_lint_check

c5b4183

Add lint check

go-gitea locked and limited conversation to collaborators Nov 23, 2020

delvh added issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea and removed reviewed/invalid labels Oct 7, 2023

This pull request was closed.

Sort repositories by watch status #19

Sort repositories by watch status #19

Conversation

ghost commented Nov 3, 2016

TL;DR

Why?

tboerger commented Nov 3, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

strk commented Nov 8, 2016

lunny commented Nov 8, 2016

codecov-io commented Nov 8, 2016 • edited Loading

Current coverage is 3.13% (diff: 0.00%)

thibaultmeyer Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

thibaultmeyer Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

thibaultmeyer Nov 13, 2016 • edited Loading

Choose a reason for hiding this comment

thibaultmeyer commented Nov 13, 2016

tboerger commented Nov 15, 2016

codecov-io commented Nov 8, 2016 •

edited

Loading

thibaultmeyer Nov 13, 2016 •

edited

Loading

thibaultmeyer Nov 13, 2016 •

edited

Loading

thibaultmeyer Nov 13, 2016 •

edited

Loading