-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Pin Repositories on user page (Fixes #10375) #19831
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.
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
Changes from 42 commits
e3fe53f
fa46bfd
11faab0
9f87fd6
5c22473
8464a09
0140a49
30a2076
cf5ef19
33c5db9
a0f3148
bfc0a3b
b6d4403
0cfd468
ef80b11
ca30e0c
4e2e304
d7cc870
2e94d4e
65c1c26
3bcfaa2
f0e8a25
8be963a
f489c68
6cbdb44
0d11561
94396a6
3ee1ade
22f2956
332c8fa
9eeb53b
16a9906
5655553
d8d7a40
70274d5
791122a
3e566e0
6e65d34
0e50a7c
58f5754
cb10529
826d921
b2a7270
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -217,6 +217,20 @@ func (repo *Repository) IsBroken() bool { | |
return repo.Status == RepositoryBroken | ||
} | ||
|
||
// IsPinned indicates that repository is pinned | ||
func (repo *Repository) IsPinned() bool { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the function should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A repo is either pinned to its owner's profile, or not at all. This is maybe the same confusion I had with @delvh ? This is not an implementation for pinning any repo you have access to to your profile - it's an implementation for pinning a repo to its owner's profile. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, the most interesting thing I happen to work on is in an org I own, not in my user namespace. So I couldn't pin that then? I'd assume that's a common use case, but maybe that's just me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It sounds like that's everyone's else's view too. This may need reimplementing with that in mind. |
||
pinned, err := user_model.GetPinnedRepositoryIDs(repo.OwnerID) | ||
if err != nil { | ||
return false | ||
} | ||
for _, r := range pinned { | ||
if r == repo.ID { | ||
return true | ||
} | ||
} | ||
return false | ||
} | ||
|
||
// AfterLoad is invoked from XORM after setting the values of all fields of this object. | ||
func (repo *Repository) AfterLoad() { | ||
// FIXME: use models migration to solve all at once. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,106 @@ | ||
// Copyright 2022 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package user | ||
|
||
import ( | ||
"fmt" | ||
|
||
"code.gitea.io/gitea/modules/json" | ||
) | ||
|
||
const maxPinnedRepos = 3 | ||
|
||
// Get all the repositories pinned by a user. If they've never | ||
// set pinned repositories, an empty array is returned. | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func GetPinnedRepositoryIDs(userID int64) ([]int64, error) { | ||
pinnedstring, err := GetUserSetting(userID, PinnedRepositories) | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
var parsedValues []int64 | ||
if pinnedstring == "" { | ||
return parsedValues, nil | ||
} | ||
|
||
err = json.Unmarshal([]byte(pinnedstring), &parsedValues) | ||
|
||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return parsedValues, nil | ||
} | ||
|
||
func setPinnedRepositories(userID int64, repos []int64) error { | ||
stringed, err := json.Marshal(repos) | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return err | ||
} | ||
|
||
return SetUserSetting(userID, PinnedRepositories, string(stringed)) | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
type TooManyPinnedReposError struct { | ||
count int | ||
} | ||
|
||
func (e *TooManyPinnedReposError) Error() string { | ||
return fmt.Sprintf("can pin at most %d repositories, %d pinned repositories is too much", maxPinnedRepos, e.count) | ||
} | ||
|
||
// Add some repos to a user's pinned repositories. | ||
// The caller must ensure all repos belong to the | ||
// owner. | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func PinRepos(ownerID int64, repoIDs ...int64) error { | ||
repos, err := GetPinnedRepositoryIDs(ownerID) | ||
if err != nil { | ||
return err | ||
} | ||
newrepos := make([]int64, 0, len(repoIDs)+len(repos)) | ||
|
||
repos = append(repos, repoIDs...) | ||
|
||
for _, toadd := range repos { | ||
alreadypresent := false | ||
for _, present := range newrepos { | ||
if toadd == present { | ||
alreadypresent = true | ||
break | ||
} | ||
} | ||
if !alreadypresent { | ||
newrepos = append(newrepos, toadd) | ||
} | ||
} | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if len(newrepos) > maxPinnedRepos { | ||
return &TooManyPinnedReposError{count: len(newrepos)} | ||
} | ||
return setPinnedRepositories(ownerID, newrepos) | ||
} | ||
|
||
// Remove some repos from a user's pinned repositories. | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
func UnpinRepos(ownerID int64, repoIDs ...int64) error { | ||
prevRepos, err := GetPinnedRepositoryIDs(ownerID) | ||
if err != nil { | ||
return err | ||
} | ||
var nextRepos []int64 | ||
|
||
for _, r := range prevRepos { | ||
keep := true | ||
for _, unp := range repoIDs { | ||
if r == unp { | ||
keep = false | ||
break | ||
} | ||
} | ||
if keep { | ||
nextRepos = append(nextRepos, r) | ||
} | ||
} | ||
|
||
return setPinnedRepositories(ownerID, nextRepos) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// Copyright 2022 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package user | ||
|
||
import ( | ||
"testing" | ||
|
||
"code.gitea.io/gitea/models/unittest" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestUserPinUnpinRepos(t *testing.T) { | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
assert.NoError(t, unittest.PrepareTestDatabase()) | ||
// User:2 pins repositories 1 and 2 | ||
{ | ||
assert.NoError(t, PinRepos(2, 1, 2)) | ||
pinned, err := GetPinnedRepositoryIDs(2) | ||
|
||
if assert.NoError(t, err) { | ||
expected := []int64{1, 2} | ||
assert.Equal(t, pinned, expected) | ||
} | ||
} | ||
// User:2 unpins repository 2, leaving just 1 | ||
{ | ||
assert.NoError(t, UnpinRepos(2, 1)) | ||
|
||
pinned, err := GetPinnedRepositoryIDs(2) | ||
|
||
if assert.NoError(t, err) { | ||
expected := []int64{2} | ||
assert.Equal(t, pinned, expected) | ||
} | ||
} | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,121 @@ | ||
// Copyright 2022 The Gitea Authors. All rights reserved. | ||
// Use of this source code is governed by a MIT-style | ||
// license that can be found in the LICENSE file. | ||
|
||
package repo | ||
|
||
import ( | ||
"testing" | ||
|
||
"code.gitea.io/gitea/models/unittest" | ||
"code.gitea.io/gitea/modules/test" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
const ( | ||
pin = true | ||
unpin = false | ||
) | ||
|
||
func TestUserPinUnpin(t *testing.T) { | ||
unittest.PrepareTestEnv(t) | ||
// These test cases run sequentially since they modify state | ||
testcases := []struct { | ||
uid int64 | ||
rid int64 | ||
action bool | ||
endstate bool | ||
failmesssage string | ||
}{ | ||
{ | ||
uid: 2, | ||
rid: 2, | ||
action: pin, | ||
endstate: pin, | ||
failmesssage: "user cannot pin repos they own", | ||
}, | ||
{ | ||
uid: 2, | ||
rid: 2, | ||
action: unpin, | ||
endstate: unpin, | ||
failmesssage: "user cannot unpin repos they own", | ||
}, | ||
|
||
{ | ||
uid: 2, | ||
rid: 5, | ||
action: pin, | ||
endstate: pin, | ||
failmesssage: "user cannot pin repos they have admin access to", | ||
}, | ||
{ | ||
uid: 2, | ||
rid: 5, | ||
action: unpin, | ||
endstate: unpin, | ||
failmesssage: "user cannot unpin repos they have admin access to", | ||
}, | ||
|
||
{ | ||
uid: 2, | ||
rid: 4, | ||
action: pin, | ||
endstate: unpin, | ||
failmesssage: "user can pin repos they don't have access to", | ||
}, | ||
|
||
{ | ||
uid: 5, | ||
rid: 4, | ||
action: pin, | ||
endstate: pin, | ||
failmesssage: "user cannot pin repos they own (this should never fail)", | ||
}, | ||
{ | ||
uid: 2, | ||
rid: 4, | ||
action: unpin, | ||
endstate: pin, | ||
failmesssage: "user can unpin repos they don't have access to", | ||
}, | ||
{ | ||
uid: 1, | ||
rid: 4, | ||
action: unpin, | ||
endstate: unpin, | ||
failmesssage: "admin can't unpin repos they don't have access to", | ||
}, | ||
{ | ||
uid: 1, | ||
rid: 4, | ||
action: pin, | ||
endstate: pin, | ||
failmesssage: "admin can't pin repos they don't have access to", | ||
Eekle marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
} | ||
|
||
for _, c := range testcases { | ||
ctx := test.MockContext(t, "") | ||
test.LoadUser(t, ctx, c.uid) | ||
test.LoadRepo(t, ctx, c.rid) | ||
|
||
switch c.action { | ||
case pin: | ||
ctx.SetParams(":action", "pin") | ||
case unpin: | ||
ctx.SetParams(":action", "unpin") | ||
} | ||
|
||
Action(ctx) | ||
ispinned := getRepository(ctx, c.rid).IsPinned() | ||
|
||
assert.Equal(t, ispinned, c.endstate, c.failmesssage) | ||
|
||
if c.endstate != ispinned { | ||
// We have to stop at first failure, state won't be coherent afterwards. | ||
return | ||
} | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.