Ordering team by name ascending except for 'Owners' #48
Conversation
| @@ -495,7 +495,7 @@ func (org *User) GetUserRepositories(userID int64, page, pageSize int) ([]*Repos | |||
| repos := make([]*Repository, 0, pageSize) | |||
| // FIXME: use XORM chain operations instead of raw SQL. | |||
| if err = x.Sql(fmt.Sprintf(`SELECT repository.* FROM repository | |||
| INNER JOIN team_repo | |||
| INNER JOIN team_repo | |||
There was a problem hiding this comment.
Need to test it but I think the space should stay.
There was a problem hiding this comment.
no problem, the query still working cause SQL threat \n like space. You can see other files where space has been trimmed and the query still working
| @@ -507,7 +507,7 @@ func (org *User) GetUserRepositories(userID int64, page, pageSize int) ([]*Repos | |||
| } | |||
|
|
|||
| results, err := x.Query(fmt.Sprintf(`SELECT repository.id FROM repository | |||
| INNER JOIN team_repo | |||
| INNER JOIN team_repo | |||
There was a problem hiding this comment.
Same here about the pending space since the SQL Query is multi-lines
There was a problem hiding this comment.
no problem, the query still working cause SQL threat \n like space. You can see other files where space has been trimmed and the query still working
| @@ -534,7 +534,7 @@ func (org *User) GetUserMirrorRepositories(userID int64) ([]*Repository, error) | |||
|
|
|||
| repos := make([]*Repository, 0, 10) | |||
| if err = x.Sql(fmt.Sprintf(`SELECT repository.* FROM repository | |||
| INNER JOIN team_repo | |||
| INNER JOIN team_repo | |||
There was a problem hiding this comment.
Same here about the pending space since the SQL Query is multi-lines
There was a problem hiding this comment.
no problem, the query still working cause SQL threat \n like space. You can see other files where space has been trimmed and the query still working
Current coverage is 2.18% (diff: 0.00%)@@ master #48 diff @@
========================================
Files 31 31
Lines 7508 7508
Methods 0 0
Messages 0 0
Branches 0 0
========================================
- Hits 166 164 -2
- Misses 7326 7327 +1
- Partials 16 17 +1
|
|
LGTM, can you please update/rebase the branch? |
|
You could choose "Rebase and Merge" during the merge process : https://github.com/blog/2243-rebase-and-merge-pull-requests |
But that adds an unneeded additional commit. I prefer to have the commits rebased without any additional merge commit... |
|
And what the problem ? that Git ! |
|
I can understand that github takes the conservative approach |
|
@tboerger LGTM |
| @@ -50,7 +50,7 @@ func (org *User) GetOwnerTeam() (*Team, error) { | |||
| } | |||
|
|
|||
| func (org *User) getTeams(e Engine) error { | |||
| return e.Where("org_id=?", org.ID).Find(&org.Teams) | |||
| return e.Where("org_id=?", org.ID).OrderBy("CASE WHEN name LIKE '" + OWNER_TEAM + "' THEN '' ELSE name END").Find(&org.Teams) | |||
There was a problem hiding this comment.
Use proper SQL params instead of concatenating strings. Like this:
OrderBy("name LIKE ?", variable)Also, why LIKE and not =?
There was a problem hiding this comment.
This is not possible.
There was a problem hiding this comment.
OK then, I thought it was possible.
There was a problem hiding this comment.
I agree it should not be LIKE but equality operator.
Is OWNER_TEAM guaranteed to be SQL-safe ? I'd feel more comfortable if we did not rely on that and use some xorm-provided quoting function, is it available @lunny ?
|
LGTM |
DblK
added
the
lgtm/done
|
For some reason codecov/ tests are failing, not sure why. Who set those up ? |
|
Beside nitpicking, better get things moved. Github did let me commit even if coverage tests were failing. |
tboerger
removed
the
lgtm/done
tboerger
added
the
lgtm/done
Relative to the issue #3841, this pull request force team names to be ascending ordered. Owners is now always on top of the list.
Tested on MySQL 5, PostgreSQL 9 ans SQlite 3