Skip to content
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

Pin repos on profile #30961

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5f890b5
feat(pin): implemented base code for pins on the backend
carlosfelgueiras May 10, 2024
fd47c82
feat(pin): implemented the list of pinned repositories on the profile
carlosfelgueiras May 10, 2024
e8dca1e
feat(pin): implemented the pin/unpin button on repo main page
carlosfelgueiras May 10, 2024
0754ed7
test(pin): created basic tests for the pin/unpin feature
carlosfelgueiras May 10, 2024
f8b1756
fix(lint): fixed and added copyright and license comment on top of files
carlosfelgueiras May 13, 2024
5fba27c
fix(table name): Changed repository pins database name
DanielMatiasCarvalho May 13, 2024
0887277
Update templates/repo/pin_unpin.tmpl
carlosfelgueiras May 15, 2024
c9d2f49
fix(pin): removed octicon-custom-pin-off and replaced the places that…
carlosfelgueiras May 15, 2024
dae3c8a
feat(pin): made the gap between pin card half the previous size
carlosfelgueiras May 15, 2024
2140062
fix(user pin service): converted max number of pins to a constant
DanielMatiasCarvalho May 17, 2024
b941b04
fix(user pin service): fixed CanPin function to return an error
DanielMatiasCarvalho May 17, 2024
a019e44
fix(pin db): added a function to get the count of pinned repos
DanielMatiasCarvalho May 17, 2024
3704d23
fix(view): Added a missing return function
DanielMatiasCarvalho May 17, 2024
805dbc7
fix(transaction): changed the use of the transaction to WithTx instea…
DanielMatiasCarvalho May 17, 2024
859b1ce
refactor(pin): changed method used to check if pin exists from Get to…
carlosfelgueiras May 28, 2024
764b08b
fix(pin): pins not being deleted when user gets deleted
carlosfelgueiras May 28, 2024
a392779
Merge branch 'main' into feature-pin-repos
carlosfelgueiras May 31, 2024
9479423
Merge branch 'main' into feature-pin-repos
carlosfelgueiras Jun 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 62 additions & 0 deletions models/repo/pin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package repo

import (
"context"

"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/timeutil"
)

type Pin struct {
ID int64 `xorm:"pk autoincr"`
UID int64 `xorm:"UNIQUE(s)"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not UserID ? i do not think UID is a good name. It looks like UUID

RepoID int64 `xorm:"UNIQUE(s)"`
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
}

// TableName sets the table name for the pin struct
func (s *Pin) TableName() string {
return "repository_pin"
}

func init() {
db.RegisterModel(new(Pin))
}

func IsPinned(ctx context.Context, userID, repoID int64) bool {
exists, _ := db.GetEngine(ctx).Exist(&Pin{UID: userID, RepoID: repoID})

return exists
}

func PinRepo(ctx context.Context, doer *user_model.User, repo *Repository, pin bool) error {
return db.WithTx(ctx, func(ctx context.Context) error {
pinned := IsPinned(ctx, doer.ID, repo.ID)

if pin {
// Already pinned, nothing to do
if pinned {
return nil
}

if err := db.Insert(ctx, &Pin{UID: doer.ID, RepoID: repo.ID}); err != nil {
return err
}
} else {
// Not pinned, nothing to do
if !pinned {
return nil
}

if _, err := db.DeleteByBean(ctx, &Pin{UID: doer.ID, RepoID: repo.ID}); err != nil {
return err
}
}

return nil
})
}
48 changes: 48 additions & 0 deletions models/repo/pin_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2024 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package repo_test

import (
"testing"

"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"
user_model "code.gitea.io/gitea/models/user"

"github.com/stretchr/testify/assert"
)

func TestPinRepoFunctionality(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

unittest.AssertNotExistsBean(t, &repo_model.Pin{UID: user.ID, RepoID: repo.ID})
assert.NoError(t, repo_model.PinRepo(db.DefaultContext, user, repo, true))
unittest.AssertExistsAndLoadBean(t, &repo_model.Pin{UID: user.ID, RepoID: repo.ID})
assert.NoError(t, repo_model.PinRepo(db.DefaultContext, user, repo, true))
unittest.AssertExistsAndLoadBean(t, &repo_model.Pin{UID: user.ID, RepoID: repo.ID})
assert.NoError(t, repo_model.PinRepo(db.DefaultContext, user, repo, false))
unittest.AssertNotExistsBean(t, &repo_model.Star{UID: user.ID, RepoID: repo.ID})
}

func TestIsPinned(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())

user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})

unittest.AssertNotExistsBean(t, &repo_model.Pin{UID: user.ID, RepoID: repo.ID})
assert.NoError(t, repo_model.PinRepo(db.DefaultContext, user, repo, true))
unittest.AssertExistsAndLoadBean(t, &repo_model.Pin{UID: user.ID, RepoID: repo.ID})

assert.True(t, repo_model.IsPinned(db.DefaultContext, user.ID, repo.ID))

assert.NoError(t, repo_model.PinRepo(db.DefaultContext, user, repo, false))
unittest.AssertNotExistsBean(t, &repo_model.Star{UID: user.ID, RepoID: repo.ID})

assert.False(t, repo_model.IsPinned(db.DefaultContext, user.ID, repo.ID))
}
35 changes: 35 additions & 0 deletions models/repo/user_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,41 @@ func GetStarredRepos(ctx context.Context, opts *StarredReposOptions) ([]*Reposit
return db.Find[Repository](ctx, opts)
}

type PinnedReposOptions struct {
db.ListOptions
PinnerID int64
RepoOwnerID int64
}

func (opts *PinnedReposOptions) ToConds() builder.Cond {
var cond builder.Cond = builder.Eq{
"repository_pin.uid": opts.PinnerID,
}
if opts.RepoOwnerID != 0 {
cond = cond.And(builder.Eq{
"repository.owner_id": opts.RepoOwnerID,
})
}
return cond
}

func (opts *PinnedReposOptions) ToJoins() []db.JoinFunc {
return []db.JoinFunc{
func(e db.Engine) error {
e.Join("INNER", "repository_pin", "`repository`.id=`repository_pin`.repo_id")
return nil
},
}
}

func GetPinnedRepos(ctx context.Context, opts *PinnedReposOptions) (RepositoryList, error) {
return db.Find[Repository](ctx, opts)
}

func CountPinnedRepos(ctx context.Context, opts *PinnedReposOptions) (int64, error) {
return db.Count[Repository](ctx, opts)
}

type WatchedReposOptions struct {
db.ListOptions
WatcherID int64
Expand Down
6 changes: 6 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1102,6 +1102,7 @@ transfer.no_permission_to_reject = You do not have permission to reject this tra
desc.private = Private
desc.public = Public
desc.template = Template
desc.private_template = Private Template
desc.internal = Internal
desc.archived = Archived
desc.sha256 = SHA256
Expand Down Expand Up @@ -1187,10 +1188,15 @@ fork_from_self = You cannot fork a repository you own.
fork_guest_user = Sign in to fork this repository.
watch_guest_user = Sign in to watch this repository.
star_guest_user = Sign in to star this repository.
pin_guest_user = Sign in to pin this repository.
unwatch = Unwatch
watch = Watch
unstar = Unstar
star = Star
pin = Pin
unpin = Unpin
pin-org = Pin to %s
unpin-org = Unpin from %s
fork = Fork
action.blocked_user = Cannot perform action because you are blocked by the repository owner.
download_archive = Download Repository
Expand Down
20 changes: 17 additions & 3 deletions routers/web/org/home.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"code.gitea.io/gitea/modules/util"
shared_user "code.gitea.io/gitea/routers/web/shared/user"
"code.gitea.io/gitea/services/context"
user_service "code.gitea.io/gitea/services/user"
)

const (
Expand Down Expand Up @@ -101,9 +102,11 @@ func Home(ctx *context.Context) {
ctx.Data["IsPrivate"] = private

var (
repos []*repo_model.Repository
count int64
err error
repos []*repo_model.Repository
count int64
pinnedRepos []*repo_model.Repository
pinnedCount int64
err error
)
repos, count, err = repo_model.SearchRepository(ctx, &repo_model.SearchRepoOptions{
ListOptions: db.ListOptions{
Expand Down Expand Up @@ -139,8 +142,19 @@ func Home(ctx *context.Context) {
return
}

// Get pinned repos
pinnedRepos, err = user_service.GetUserPinnedRepos(ctx, org.AsUser(), ctx.Doer)
if err != nil {
ctx.ServerError("GetUserPinnedRepos", err)
return
}

pinnedCount = int64(len(pinnedRepos))

ctx.Data["Repos"] = repos
ctx.Data["Total"] = count
ctx.Data["PinnedRepos"] = pinnedRepos
ctx.Data["PinnedTotal"] = pinnedCount
ctx.Data["Members"] = members
ctx.Data["Teams"] = ctx.Org.Teams
ctx.Data["DisableNewPullMirrors"] = setting.Mirror.DisableNewPull
Expand Down
9 changes: 9 additions & 0 deletions routers/web/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
repo_service "code.gitea.io/gitea/services/repository"
archiver_service "code.gitea.io/gitea/services/repository/archiver"
commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus"
user_service "code.gitea.io/gitea/services/user"
)

const (
Expand Down Expand Up @@ -321,6 +322,14 @@ func Action(ctx *context.Context) {
err = repo_model.StarRepo(ctx, ctx.Doer, ctx.Repo.Repository, true)
case "unstar":
err = repo_model.StarRepo(ctx, ctx.Doer, ctx.Repo.Repository, false)
case "pin":
err = user_service.PinRepo(ctx, ctx.Doer, ctx.Repo.Repository, true, false)
case "unpin":
err = user_service.PinRepo(ctx, ctx.Doer, ctx.Repo.Repository, false, false)
case "pin-org":
err = user_service.PinRepo(ctx, ctx.Doer, ctx.Repo.Repository, true, true)
case "unpin-org":
err = user_service.PinRepo(ctx, ctx.Doer, ctx.Repo.Repository, false, true)
case "accept_transfer":
err = acceptOrRejectRepoTransfer(ctx, true)
case "reject_transfer":
Expand Down
44 changes: 44 additions & 0 deletions routers/web/repo/view.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"code.gitea.io/gitea/models/db"
git_model "code.gitea.io/gitea/models/git"
issue_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
access_model "code.gitea.io/gitea/models/perm/access"
repo_model "code.gitea.io/gitea/models/repo"
unit_model "code.gitea.io/gitea/models/unit"
Expand All @@ -51,6 +52,7 @@ import (
"code.gitea.io/gitea/services/context"
issue_service "code.gitea.io/gitea/services/issue"
files_service "code.gitea.io/gitea/services/repository/files"
user_service "code.gitea.io/gitea/services/user"

"github.com/nektos/act/pkg/model"

Expand Down Expand Up @@ -792,6 +794,14 @@ func Home(ctx *context.Context) {
return
}

if ctx.IsSigned {
err := loadPinData(ctx)
if err != nil {
ctx.ServerError("loadPinData", err)
carlosfelgueiras marked this conversation as resolved.
Show resolved Hide resolved
return
}
}

renderHomeCode(ctx)
}

Expand Down Expand Up @@ -1179,3 +1189,37 @@ func Forks(ctx *context.Context) {

ctx.HTML(http.StatusOK, tplForks)
}

func loadPinData(ctx *context.Context) error {
lunny marked this conversation as resolved.
Show resolved Hide resolved
// First, cleanup any pins that are no longer valid
err := user_service.CleanupPins(ctx, ctx.Doer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why cleanup pins when user home page be visited? Looks wired.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are cleaning the pins to avoid an edge case.
If we have a user1, with 6 pins, being one of those pins a repo belonging to user2. If user 2 changes the repo visibility to private, user1 can no longer have it as a pin. If user1 now visited a new repo page, without visiting his own homepage, the pin button would be disabled because he already has the maximum number of pins. However, if we clean the pins, the pin to the repo of user2 will be deleted, and the button will be enabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When changing the visibility and user has no permission to access this pin, means user2 forcibly unpinned it, so this should be handled when user2 changes the repo visibility, not the moment user visiting the repo home page.
Also for other cases, like remove the user as the member of an organization and so on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still a concern. Cleaning should not be done when a page is visiting.

if err != nil {
return err
}

ctx.Data["IsPinningRepo"] = repo_model.IsPinned(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID)
ctx.Data["CanPinRepo"], err = user_service.CanPin(ctx, ctx.Doer, ctx.Repo.Repository)
if err != nil {
return err
}

if ctx.Repo.Repository.Owner.IsOrganization() {
org := organization.OrgFromUser(ctx.Repo.Repository.Owner)

isAdmin, err := org.IsOrgAdmin(ctx, ctx.Doer.ID)
if err != nil {
return err
}

if isAdmin {
ctx.Data["CanUserPinToOrg"] = true
ctx.Data["IsOrgPinningRepo"] = repo_model.IsPinned(ctx, ctx.Repo.Repository.OwnerID, ctx.Repo.Repository.ID)
ctx.Data["CanOrgPinRepo"], err = user_service.CanPin(ctx, ctx.Repo.Repository.Owner, ctx.Repo.Repository)
if err != nil {
return err
}
}
}

return nil
}
21 changes: 17 additions & 4 deletions routers/web/user/profile.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"code.gitea.io/gitea/routers/web/org"
shared_user "code.gitea.io/gitea/routers/web/shared/user"
"code.gitea.io/gitea/services/context"
user_service "code.gitea.io/gitea/services/user"
)

const (
Expand Down Expand Up @@ -104,10 +105,12 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
pagingNum := setting.UI.User.RepoPagingNum
topicOnly := ctx.FormBool("topic")
var (
repos []*repo_model.Repository
count int64
total int
orderBy db.SearchOrderBy
repos []*repo_model.Repository
pinnedRepos []*repo_model.Repository
count int64
pinnedCount int64
total int
orderBy db.SearchOrderBy
)

ctx.Data["SortType"] = ctx.FormString("sort")
Expand Down Expand Up @@ -312,9 +315,19 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
}

total = int(count)

pinnedRepos, err = user_service.GetUserPinnedRepos(ctx, ctx.ContextUser, ctx.Doer)
if err != nil {
ctx.ServerError("GetUserPinnedRepos", err)
return
}

pinnedCount = int64(len(pinnedRepos))
}
ctx.Data["Repos"] = repos
ctx.Data["Total"] = total
ctx.Data["PinnedRepos"] = pinnedRepos
ctx.Data["PinnedCount"] = pinnedCount

err = shared_user.LoadHeaderCount(ctx)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions services/repository/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
&repo_model.Redirect{RedirectRepoID: repoID},
&repo_model.RepoUnit{RepoID: repoID},
&repo_model.Star{RepoID: repoID},
&repo_model.Pin{RepoID: repoID},
&admin_model.Task{RepoID: repoID},
&repo_model.Watch{RepoID: repoID},
&webhook.Webhook{RepoID: repoID},
Expand Down
1 change: 1 addition & 0 deletions services/user/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
&user_model.Blocking{BlockerID: u.ID},
&user_model.Blocking{BlockeeID: u.ID},
&actions_model.ActionRunnerToken{OwnerID: u.ID},
&repo_model.Pin{UID: u.ID},
); err != nil {
return fmt.Errorf("deleteBeans: %w", err)
}
Expand Down
Loading