Skip to content

Improve notifications to fix commit notification and support release notification #34803

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.

Sign up for GitHub

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

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft

Improve notifications to fix commit notification and support release notification #34803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6b055dd
Add release notification and fix repository transfer/commit notification
lunny Jun 20, 2025
803a3a4
fix
lunny Jun 20, 2025
4163c6d
Fix bug
lunny Jun 20, 2025
01a0b8a
Add tests and fix lint
lunny Jun 21, 2025
49e9ab0
Fix lint
lunny Jun 21, 2025
60227c5
Merge branch 'main' into lunny/improve_notification
lunny Jun 21, 2025
fd17f05
improvements
lunny Jun 21, 2025
7a1cc34
Merge branch 'main' into lunny/improve_notification
lunny Jun 21, 2025
11bd0ea
improvements
lunny Jul 3, 2025
6f79a1a
Merge branch 'main' into lunny/improve_notification
lunny Jul 3, 2025
7aaa2d6
remove nolint
lunny Jul 3, 2025
5dc32a3
Fix typo
lunny Jul 3, 2025
8b0071d
Merge branch 'main' into lunny/improve_notification
lunny Jul 3, 2025
5676169
Merge branch 'main' into lunny/improve_notification
lunny Jul 21, 2025
886c476
Merge branch 'main' into lunny/improve_notification
lunny Jul 31, 2025
fa0262e
Merge branch 'main' into lunny/improve_notification
lunny Aug 1, 2025
2ffcd2e
improvement
lunny Aug 3, 2025
df24a1f
Merge branch 'main' into lunny/improve_notification
lunny Aug 3, 2025
f37de47
Use commit title instead in notification
lunny Aug 12, 2025
d3b6d84
Some improvements
lunny Aug 23, 2025
88143cd
Merge branch 'main' into lunny/improve_notification
lunny Aug 23, 2025
ff70703
Merge branch 'main' into lunny/improve_notification
lunny Aug 28, 2025
0614610
add contexts
lunny Aug 28, 2025
b48620b
Revert "add contexts"
lunny Aug 30, 2025
a024ad9
Merge branch 'main' into lunny/improve_notification
lunny Aug 30, 2025
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
177 changes: 145 additions & 32 deletions models/activities/notification.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,18 @@ package activities
import (
"context"
"fmt"
"html/template"
"net/url"
"strconv"

"code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues"
"code.gitea.io/gitea/models/organization"
repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/svg"
"code.gitea.io/gitea/modules/timeutil"

"xorm.io/builder"
Expand Down Expand Up @@ -46,6 +49,8 @@ const (
NotificationSourceCommit
// NotificationSourceRepository is a notification for a repository
NotificationSourceRepository
// NotificationSourceRelease is a notification for a release
NotificationSourceRelease
)

// Notification represents a notification
Expand All @@ -60,13 +65,16 @@ type Notification struct {
IssueID int64 `xorm:"NOT NULL"`
CommitID string
CommentID int64
ReleaseID int64

UpdatedBy int64 `xorm:"NOT NULL"`

Issue *issues_model.Issue `xorm:"-"`
Repository *repo_model.Repository `xorm:"-"`
Comment *issues_model.Comment `xorm:"-"`
User *user_model.User `xorm:"-"`
Release *repo_model.Release `xorm:"-"`
Commit *git.Commit `xorm:"-"`

CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"`
UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"`
Expand Down Expand Up @@ -104,6 +112,10 @@ func (n *Notification) TableIndices() []*schemas.Index {
commitIDIndex.AddColumn("commit_id")
indices = append(indices, commitIDIndex)

releaseIDIndex := schemas.NewIndex("idx_notification_release_id", schemas.IndexType)
releaseIDIndex.AddColumn("release_id")
indices = append(indices, releaseIDIndex)

Copy link
Contributor

Choose a reason for hiding this comment

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

The database design looks strange.

If I understand correctly, what you need is "a unique key to avoid duplication and help to mark the notification as read". I think it could be resolved by a more flexible approach like:

unique_key = 'commit-{CommitID}'
unique_key = 'release-{ReleaseID}'
unique_key = 'comment-{IssueID}-{CommentID}'

Then we only need one unique index (user_id, unique_key)

And, some legacy indices like status, source seem not able to help to improve querying performance because if user_id is used, then these indices won't be used.

And, the user_id index is redundancy because there is already (user_id, status, updated_unix). If you would take more care about performance, these problems should be handled.

Copy link
Member Author

Choose a reason for hiding this comment

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

The source should also be part of the unique key. Currently, issue and release notifications require a unique key, while commit and repository notifications do not.

updatedByIndex := schemas.NewIndex("idx_notification_updated_by", schemas.IndexType)
updatedByIndex.AddColumn("updated_by")
indices = append(indices, updatedByIndex)
Expand All @@ -116,36 +128,55 @@ func init() {
}

// CreateRepoTransferNotification creates notification for the user a repository was transferred to
func CreateRepoTransferNotification(ctx context.Context, doer, newOwner *user_model.User, repo *repo_model.Repository) error {
return db.WithTx(ctx, func(ctx context.Context) error {
var notify []*Notification

if newOwner.IsOrganization() {
users, err := organization.GetUsersWhoCanCreateOrgRepo(ctx, newOwner.ID)
if err != nil || len(users) == 0 {
return err
}
for i := range users {
notify = append(notify, &Notification{
UserID: i,
RepoID: repo.ID,
Status: NotificationStatusUnread,
UpdatedBy: doer.ID,
Source: NotificationSourceRepository,
})
}
} else {
notify = []*Notification{{
UserID: newOwner.ID,
RepoID: repo.ID,
Status: NotificationStatusUnread,
UpdatedBy: doer.ID,
Source: NotificationSourceRepository,
}}
}
func CreateRepoTransferNotification(ctx context.Context, doerID, repoID, receiverID int64) error {
notify := &Notification{
UserID: receiverID,
RepoID: repoID,
Status: NotificationStatusUnread,
UpdatedBy: doerID,
Source: NotificationSourceRepository,
}
return db.Insert(ctx, notify)
}

func CreateCommitNotifications(ctx context.Context, doerID, repoID int64, commitID string, receiverID int64) error {
notification := &Notification{
Source: NotificationSourceCommit,
UserID: receiverID,
RepoID: repoID,
CommitID: commitID,
Status: NotificationStatusUnread,
UpdatedBy: doerID,
}

return db.Insert(ctx, notification)
}

func CreateOrUpdateReleaseNotifications(ctx context.Context, doerID, repoID, releaseID, receiverID int64) error {
notification := new(Notification)
if _, err := db.GetEngine(ctx).
Where("user_id = ?", receiverID).
And("repo_id = ?", repoID).
And("release_id = ?", releaseID).
Get(notification); err != nil {
return err
}
if notification.ID > 0 {
notification.Status = NotificationStatusUnread
notification.UpdatedBy = doerID
_, err := db.GetEngine(ctx).ID(notification.ID).Cols("status", "updated_by").Update(notification)
return err
}

return db.Insert(ctx, notify)
})
notification = &Notification{
Source: NotificationSourceRelease,
RepoID: repoID,
UserID: receiverID,
Status: NotificationStatusUnread,
ReleaseID: releaseID,
UpdatedBy: doerID,
}
return db.Insert(ctx, notification)
}

func createIssueNotification(ctx context.Context, userID int64, issue *issues_model.Issue, commentID, updatedByID int64) error {
Expand Down Expand Up @@ -213,6 +244,12 @@ func (n *Notification) LoadAttributes(ctx context.Context) (err error) {
if err = n.loadComment(ctx); err != nil {
return err
}
if err = n.loadCommit(ctx); err != nil {
return err
}
if err = n.loadRelease(ctx); err != nil {
return err
}
return err
}

Expand Down Expand Up @@ -253,6 +290,41 @@ func (n *Notification) loadComment(ctx context.Context) (err error) {
return nil
}

func (n *Notification) loadCommit(ctx context.Context) (err error) {
if n.Source != NotificationSourceCommit || n.CommitID == "" || n.Commit != nil {
return nil
}

if n.Repository == nil {
_ = n.loadRepo(ctx)
if n.Repository == nil {
return fmt.Errorf("repository not found for notification %d", n.ID)
}
}

repo, err := gitrepo.OpenRepository(ctx, n.Repository)
if err != nil {
return fmt.Errorf("OpenRepository [%d]: %w", n.Repository.ID, err)
}
defer repo.Close()

n.Commit, err = repo.GetCommit(n.CommitID)
if err != nil {
return fmt.Errorf("Notification[%d]: Failed to get repo for commit %s: %v", n.ID, n.CommitID, err)
}
return nil
}

func (n *Notification) loadRelease(ctx context.Context) (err error) {
if n.Release == nil && n.ReleaseID != 0 {
n.Release, err = repo_model.GetReleaseByID(ctx, n.ReleaseID)
if err != nil {
return fmt.Errorf("GetReleaseByID [%d]: %w", n.ReleaseID, err)
}
}
return nil
}

func (n *Notification) loadUser(ctx context.Context) (err error) {
if n.User == nil {
n.User, err = user_model.GetUserByID(ctx, n.UserID)
Expand Down Expand Up @@ -285,6 +357,8 @@ func (n *Notification) HTMLURL(ctx context.Context) string {
return n.Repository.HTMLURL(ctx) + "/commit/" + url.PathEscape(n.CommitID)
case NotificationSourceRepository:
return n.Repository.HTMLURL(ctx)
case NotificationSourceRelease:
return n.Release.HTMLURL()
}
return ""
}
Expand All @@ -301,10 +375,28 @@ func (n *Notification) Link(ctx context.Context) string {
return n.Repository.Link() + "/commit/" + url.PathEscape(n.CommitID)
case NotificationSourceRepository:
return n.Repository.Link()
case NotificationSourceRelease:
return n.Release.Link()
}
return ""
}

func (n *Notification) IconHTML(ctx context.Context) template.HTML {
switch n.Source {
case NotificationSourceIssue, NotificationSourcePullRequest:
// n.Issue should be loaded before calling this method
return n.Issue.IconHTML(ctx)
case NotificationSourceCommit:
return svg.RenderHTML("octicon-commit", 16, "text grey")
case NotificationSourceRepository:
return svg.RenderHTML("octicon-repo", 16, "text grey")
case NotificationSourceRelease:
return svg.RenderHTML("octicon-tag", 16, "text grey")
default:
return ""
}
}

// APIURL formats a URL-string to the notification
func (n *Notification) APIURL() string {
return setting.AppURL + "api/v1/notifications/threads/" + strconv.FormatInt(n.ID, 10)
Expand Down Expand Up @@ -373,6 +465,28 @@ func SetRepoReadBy(ctx context.Context, userID, repoID int64) error {
return err
}

// SetReleaseReadBy sets issue to be read by given user.
func SetReleaseReadBy(ctx context.Context, releaseID, userID int64) error {
_, err := db.GetEngine(ctx).Where(builder.Eq{
"user_id": userID,
"status": NotificationStatusUnread,
"source": NotificationSourceRelease,
"release_id": releaseID,
}).Cols("status").Update(&Notification{Status: NotificationStatusRead})
return err
}

// SetCommitReadBy sets issue to be read by given user.
func SetCommitReadBy(ctx context.Context, repoID, userID int64, commitID string) error {
_, err := db.GetEngine(ctx).Where(builder.Eq{
"user_id": userID,
"status": NotificationStatusUnread,
"source": NotificationSourceCommit,
"commit_id": commitID,
}).Cols("status").Update(&Notification{Status: NotificationStatusRead})
return err
}

// SetNotificationStatus change the notification status
func SetNotificationStatus(ctx context.Context, notificationID int64, user *user_model.User, status NotificationStatus) (*Notification, error) {
notification, err := GetNotificationByID(ctx, notificationID)
Expand All @@ -385,8 +499,7 @@ func SetNotificationStatus(ctx context.Context, notificationID int64, user *user
}

notification.Status = status

_, err = db.GetEngine(ctx).ID(notificationID).Update(notification)
_, err = db.GetEngine(ctx).ID(notificationID).Cols("status").Update(notification)
return notification, err
}

Expand Down
Loading