diff --git a/models/activities/notification.go b/models/activities/notification.go index 5931bd3cb2d..2ca1051ed3e 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -14,6 +14,7 @@ import ( 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/timeutil" @@ -241,6 +242,9 @@ 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 } @@ -284,6 +288,31 @@ 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) @@ -452,8 +481,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 } diff --git a/models/activities/notification_list.go b/models/activities/notification_list.go index a95ad638b65..d729534589e 100644 --- a/models/activities/notification_list.go +++ b/models/activities/notification_list.go @@ -172,18 +172,28 @@ type NotificationList []*Notification // LoadAttributes load Repo Issue User and Comment if not loaded func (nl NotificationList) LoadAttributes(ctx context.Context) error { - if _, _, err := nl.LoadRepos(ctx); err != nil { + repos, _, err := nl.LoadRepos(ctx) + if err != nil { + return err + } + if err := repos.LoadAttributes(ctx); err != nil { return err } if _, err := nl.LoadIssues(ctx); err != nil { return err } + if err = nl.LoadIssuePullRequests(ctx); err != nil { + return err + } if _, err := nl.LoadUsers(ctx); err != nil { return err } if _, err := nl.LoadComments(ctx); err != nil { return err } + if _, err = nl.LoadCommits(ctx); err != nil { + return err + } if _, err := nl.LoadReleases(ctx); err != nil { return err } diff --git a/modules/structs/notifications.go b/modules/structs/notifications.go index 20c0c02b4c3..b47fec25786 100644 --- a/modules/structs/notifications.go +++ b/modules/structs/notifications.go @@ -25,7 +25,7 @@ type NotificationSubject struct { LatestCommentURL string `json:"latest_comment_url"` HTMLURL string `json:"html_url"` LatestCommentHTMLURL string `json:"latest_comment_html_url"` - Type NotifySubjectType `json:"type" binding:"In(Issue,Pull,Commit,Repository)"` + Type NotifySubjectType `json:"type" binding:"In(Issue,Pull,Commit,Repository,Release)"` State StateType `json:"state"` } diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go index 12bae5dd84b..cb722d7948b 100644 --- a/routers/api/v1/notify/repo.go +++ b/routers/api/v1/notify/repo.go @@ -214,14 +214,20 @@ func ReadRepoNotifications(ctx *context.APIContext) { changed := make([]*structs.NotificationThread, 0, len(nl)) + if err := activities_model.NotificationList(nl).LoadAttributes(ctx); err != nil { + ctx.APIErrorInternal(err) + return + } + for _, n := range nl { notif, err := activities_model.SetNotificationStatus(ctx, n.ID, ctx.Doer, targetStatus) if err != nil { ctx.APIErrorInternal(err) return } - _ = notif.LoadAttributes(ctx) - changed = append(changed, convert.ToNotificationThread(ctx, notif)) + n.Status = notif.Status + n.UpdatedUnix = notif.UpdatedUnix + changed = append(changed, convert.ToNotificationThread(ctx, n)) } ctx.JSON(http.StatusResetContent, changed) } diff --git a/routers/api/v1/notify/user.go b/routers/api/v1/notify/user.go index 9e43b0fc754..c840fb44d72 100644 --- a/routers/api/v1/notify/user.go +++ b/routers/api/v1/notify/user.go @@ -161,14 +161,20 @@ func ReadNotifications(ctx *context.APIContext) { changed := make([]*structs.NotificationThread, 0, len(nl)) + if err := activities_model.NotificationList(nl).LoadAttributes(ctx); err != nil { + ctx.APIErrorInternal(err) + return + } + for _, n := range nl { notif, err := activities_model.SetNotificationStatus(ctx, n.ID, ctx.Doer, targetStatus) if err != nil { ctx.APIErrorInternal(err) return } - _ = notif.LoadAttributes(ctx) - changed = append(changed, convert.ToNotificationThread(ctx, notif)) + n.Status = notif.Status + n.UpdatedUnix = notif.UpdatedUnix + changed = append(changed, convert.ToNotificationThread(ctx, n)) } ctx.JSON(http.StatusResetContent, changed) diff --git a/services/convert/notification.go b/services/convert/notification.go index e32f6997795..549f6cd6f6c 100644 --- a/services/convert/notification.go +++ b/services/convert/notification.go @@ -6,6 +6,7 @@ package convert import ( "context" "net/url" + "strings" activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/perm" @@ -71,7 +72,7 @@ func ToNotificationThread(ctx context.Context, n *activities_model.Notification) url := n.Repository.HTMLURL() + "/commit/" + url.PathEscape(n.CommitID) result.Subject = &api.NotificationSubject{ Type: api.NotifySubjectCommit, - Title: n.CommitID, + Title: strings.TrimSpace(n.Commit.CommitMessage), URL: url, HTMLURL: url, } diff --git a/services/uinotification/notify.go b/services/uinotification/notify.go index 80e6b40aeaa..87065a2b009 100644 --- a/services/uinotification/notify.go +++ b/services/uinotification/notify.go @@ -5,6 +5,7 @@ package uinotification import ( "context" + "slices" activities_model "code.gitea.io/gitea/models/activities" "code.gitea.io/gitea/models/db" @@ -363,6 +364,19 @@ func (ns *notificationService) UpdateRelease(ctx context.Context, doer *user_mod return } + repo, err := repo_model.GetRepositoryByID(ctx, rel.RepoID) + if err != nil { + log.Error("GetRepositoryByID: %v", err) + return + } + if err := repo.LoadOwner(ctx); err != nil { + log.Error("LoadOwner: %v", err) + return + } + if !repo.Owner.IsOrganization() && !slices.Contains(repoWatcherIDs, repo.Owner.ID) && repo.Owner.ID != doer.ID { + repoWatcherIDs = append(repoWatcherIDs, repo.Owner.ID) + } + for _, watcherID := range repoWatcherIDs { if watcherID == doer.ID { // Do not notify the publisher of the release diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index a037b9584f0..982cef28a9f 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -1523,7 +1523,8 @@ "issue", "pull", "commit", - "repository" + "repository", + "release" ], "type": "string" }, @@ -13244,7 +13245,8 @@ "issue", "pull", "commit", - "repository" + "repository", + "release" ], "type": "string" }, diff --git a/tests/integration/api_notification_test.go b/tests/integration/api_notification_test.go index e6bc1424768..f556e1912c7 100644 --- a/tests/integration/api_notification_test.go +++ b/tests/integration/api_notification_test.go @@ -4,9 +4,12 @@ package integration import ( + "encoding/base64" "fmt" "net/http" + "net/url" "testing" + "time" activities_model "code.gitea.io/gitea/models/activities" auth_model "code.gitea.io/gitea/models/auth" @@ -15,6 +18,7 @@ import ( "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" api "code.gitea.io/gitea/modules/structs" + repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -213,3 +217,137 @@ func TestAPINotificationPUT(t *testing.T) { assert.True(t, apiNL[0].Unread) assert.False(t, apiNL[0].Pinned) } + +func TestAPICommitNotification(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + session := loginUser(t, user2.Name) + token1 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository) + + content := "This is a test commit" + contentEncoded := base64.StdEncoding.EncodeToString([]byte(content)) + // push a commit with @user2 in the commit message, it's expected to create a notification + createFileOptions := api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: "master", + NewBranchName: "master", + Message: "This is a test commit to mention @user2", + Author: api.Identity{ + Name: "Anne Doe", + Email: "annedoe@example.com", + }, + Committer: api.Identity{ + Name: "John Doe", + Email: "johndoe@example.com", + }, + Dates: api.CommitDateOptions{ + Author: time.Unix(946684810, 0), + Committer: time.Unix(978307190, 0), + }, + }, + ContentBase64: contentEncoded, + } + + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/new_commit_notification.txt", user2.Name, repo1.Name), &createFileOptions). + AddTokenAuth(token1) + resp := MakeRequest(t, req, http.StatusCreated) + + // Check notifications are as expected + token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteNotification) + req = NewRequest(t, "GET", "/api/v1/notifications?all=true"). + AddTokenAuth(token2) + resp = MakeRequest(t, req, http.StatusOK) + var apiNL []api.NotificationThread + DecodeJSON(t, resp, &apiNL) + + assert.Equal(t, api.NotifySubjectCommit, apiNL[0].Subject.Type) + assert.Equal(t, "This is a test commit to mention @user2", apiNL[0].Subject.Title) + assert.True(t, apiNL[0].Unread) + assert.False(t, apiNL[0].Pinned) + }) +} + +func TestAPIReleaseNotification(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + session1 := loginUser(t, user1.Name) + token1 := getTokenForLoggedInUser(t, session1, auth_model.AccessTokenScopeWriteRepository) + + // user1 create a release, it's expected to create a notification + createNewReleaseUsingAPI(t, token1, user2, repo1, "v0.0.2", "", "v0.0.2 is released", "test notification release") + + // user2 login to check notifications + session2 := loginUser(t, user2.Name) + + // Check notifications are as expected + token2 := getTokenForLoggedInUser(t, session2, auth_model.AccessTokenScopeWriteNotification) + req := NewRequest(t, "GET", "/api/v1/notifications?all=true"). + AddTokenAuth(token2) + resp := MakeRequest(t, req, http.StatusOK) + var apiNL []api.NotificationThread + DecodeJSON(t, resp, &apiNL) + + assert.Equal(t, api.NotifySubjectRelease, apiNL[0].Subject.Type) + assert.Equal(t, "v0.0.2 is released", apiNL[0].Subject.Title) + assert.True(t, apiNL[0].Unread) + assert.False(t, apiNL[0].Pinned) + }) +} + +func TestAPIRepoTransferNotification(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + + session1 := loginUser(t, user2.Name) + token1 := getTokenForLoggedInUser(t, session1, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser) + + // create repo to move + repoName := "moveME" + apiRepo := new(api.Repository) + req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos", &api.CreateRepoOption{ + Name: repoName, + Description: "repo move around", + Private: false, + Readme: "Default", + AutoInit: true, + }).AddTokenAuth(token1) + resp := MakeRequest(t, req, http.StatusCreated) + DecodeJSON(t, resp, apiRepo) + + defer func() { + _ = repo_service.DeleteRepositoryDirectly(db.DefaultContext, apiRepo.ID) + }() + + // repo user1/moveME created, now transfer it to org6 + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID}) + session2 := loginUser(t, user2.Name) + token2 := getTokenForLoggedInUser(t, session2, auth_model.AccessTokenScopeWriteRepository) + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/transfer", repo.OwnerName, repo.Name), &api.TransferRepoOption{ + NewOwner: "org6", + TeamIDs: nil, + }).AddTokenAuth(token2) + MakeRequest(t, req, http.StatusCreated) + + // user5 login to check notifications, because user5 is a member of org6's owners team + user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) + session5 := loginUser(t, user5.Name) + + // Check notifications are as expected + token5 := getTokenForLoggedInUser(t, session5, auth_model.AccessTokenScopeWriteNotification) + req = NewRequest(t, "GET", "/api/v1/notifications?all=true"). + AddTokenAuth(token5) + resp = MakeRequest(t, req, http.StatusOK) + var apiNL []api.NotificationThread + DecodeJSON(t, resp, &apiNL) + + assert.Equal(t, api.NotifySubjectRepository, apiNL[0].Subject.Type) + assert.Equal(t, "user2/moveME", apiNL[0].Subject.Title) + assert.True(t, apiNL[0].Unread) + assert.False(t, apiNL[0].Pinned) + }) +}