Skip to content

Commit 01a0b8a

Browse files
lunnylunny
committed
Add tests and fix lint
1 parent 4163c6d commit 01a0b8a

File tree

9 files changed

+216
-11
lines changed

9 files changed

+216
-11
lines changed

‎models/activities/notification.go‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
repo_model "code.gitea.io/gitea/models/repo"
1515
user_model "code.gitea.io/gitea/models/user"
1616
"code.gitea.io/gitea/modules/git"
17+
"code.gitea.io/gitea/modules/gitrepo"
1718
"code.gitea.io/gitea/modules/setting"
1819
"code.gitea.io/gitea/modules/timeutil"
1920

@@ -241,6 +242,9 @@ func (n *Notification) LoadAttributes(ctx context.Context) (err error) {
241242
if err = n.loadComment(ctx); err != nil {
242243
return err
243244
}
245+
if err = n.loadCommit(ctx); err != nil {
246+
return err
247+
}
244248
if err = n.loadRelease(ctx); err != nil {
245249
return err
246250
}
@@ -284,6 +288,31 @@ func (n *Notification) loadComment(ctx context.Context) (err error) {
284288
return nil
285289
}
286290

291+
func (n *Notification) loadCommit(ctx context.Context) (err error) {
292+
if n.Source != NotificationSourceCommit || n.CommitID == "" || n.Commit != nil {
293+
return nil
294+
}
295+
296+
if n.Repository == nil {
297+
_ = n.loadRepo(ctx)
298+
if n.Repository == nil {
299+
return fmt.Errorf("repository not found for notification %d", n.ID)
300+
}
301+
}
302+
303+
repo, err := gitrepo.OpenRepository(ctx, n.Repository)
304+
if err != nil {
305+
return fmt.Errorf("OpenRepository [%d]: %w", n.Repository.ID, err)
306+
}
307+
defer repo.Close()
308+
309+
n.Commit, err = repo.GetCommit(n.CommitID)
310+
if err != nil {
311+
return fmt.Errorf("Notification[%d]: Failed to get repo for commit %s: %v", n.ID, n.CommitID, err)
312+
}
313+
return nil
314+
}
315+
287316
func (n *Notification) loadRelease(ctx context.Context) (err error) {
288317
if n.Release == nil && n.ReleaseID != 0 {
289318
n.Release, err = repo_model.GetReleaseByID(ctx, n.ReleaseID)
@@ -452,8 +481,7 @@ func SetNotificationStatus(ctx context.Context, notificationID int64, user *user
452481
}
453482

454483
notification.Status = status
455-
456-
_, err = db.GetEngine(ctx).ID(notificationID).Update(notification)
484+
_, err = db.GetEngine(ctx).ID(notificationID).Cols("status").Update(notification)
457485
return notification, err
458486
}
459487

‎models/activities/notification_list.go‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,18 +172,28 @@ type NotificationList []*Notification
172172

173173
// LoadAttributes load Repo Issue User and Comment if not loaded
174174
func (nl NotificationList) LoadAttributes(ctx context.Context) error {
175-
if _, _, err := nl.LoadRepos(ctx); err != nil {
175+
repos, _, err := nl.LoadRepos(ctx)
176+
if err != nil {
177+
return err
178+
}
179+
if err := repos.LoadAttributes(ctx); err != nil {
176180
return err
177181
}
178182
if _, err := nl.LoadIssues(ctx); err != nil {
179183
return err
180184
}
185+
if err = nl.LoadIssuePullRequests(ctx); err != nil {
186+
return err
187+
}
181188
if _, err := nl.LoadUsers(ctx); err != nil {
182189
return err
183190
}
184191
if _, err := nl.LoadComments(ctx); err != nil {
185192
return err
186193
}
194+
if _, err = nl.LoadCommits(ctx); err != nil {
195+
return err
196+
}
187197
if _, err := nl.LoadReleases(ctx); err != nil {
188198
return err
189199
}

‎modules/structs/notifications.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ type NotificationSubject struct {
2525
LatestCommentURL string `json:"latest_comment_url"`
2626
HTMLURL string `json:"html_url"`
2727
LatestCommentHTMLURL string `json:"latest_comment_html_url"`
28-
Type NotifySubjectType `json:"type" binding:"In(Issue,Pull,Commit,Repository)"`
28+
Type NotifySubjectType `json:"type" binding:"In(Issue,Pull,Commit,Repository,Release)"`
2929
State StateType `json:"state"`
3030
}
3131

‎routers/api/v1/notify/repo.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,20 @@ func ReadRepoNotifications(ctx *context.APIContext) {
214214

215215
changed := make([]*structs.NotificationThread, 0, len(nl))
216216

217+
if err := activities_model.NotificationList(nl).LoadAttributes(ctx); err != nil {
218+
ctx.APIErrorInternal(err)
219+
return
220+
}
221+
217222
for _, n := range nl {
218223
notif, err := activities_model.SetNotificationStatus(ctx, n.ID, ctx.Doer, targetStatus)
219224
if err != nil {
220225
ctx.APIErrorInternal(err)
221226
return
222227
}
223-
_ = notif.LoadAttributes(ctx)
224-
changed = append(changed, convert.ToNotificationThread(ctx, notif))
228+
n.Status = notif.Status
229+
n.UpdatedUnix = notif.UpdatedUnix
230+
changed = append(changed, convert.ToNotificationThread(ctx, n))
225231
}
226232
ctx.JSON(http.StatusResetContent, changed)
227233
}

‎routers/api/v1/notify/user.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,14 +161,20 @@ func ReadNotifications(ctx *context.APIContext) {
161161

162162
changed := make([]*structs.NotificationThread, 0, len(nl))
163163

164+
if err := activities_model.NotificationList(nl).LoadAttributes(ctx); err != nil {
165+
ctx.APIErrorInternal(err)
166+
return
167+
}
168+
164169
for _, n := range nl {
165170
notif, err := activities_model.SetNotificationStatus(ctx, n.ID, ctx.Doer, targetStatus)
166171
if err != nil {
167172
ctx.APIErrorInternal(err)
168173
return
169174
}
170-
_ = notif.LoadAttributes(ctx)
171-
changed = append(changed, convert.ToNotificationThread(ctx, notif))
175+
n.Status = notif.Status
176+
n.UpdatedUnix = notif.UpdatedUnix
177+
changed = append(changed, convert.ToNotificationThread(ctx, n))
172178
}
173179

174180
ctx.JSON(http.StatusResetContent, changed)

‎services/convert/notification.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package convert
66
import (
77
"context"
88
"net/url"
9+
"strings"
910

1011
activities_model "code.gitea.io/gitea/models/activities"
1112
"code.gitea.io/gitea/models/perm"
@@ -71,7 +72,7 @@ func ToNotificationThread(ctx context.Context, n *activities_model.Notification)
7172
url := n.Repository.HTMLURL() + "/commit/" + url.PathEscape(n.CommitID)
7273
result.Subject = &api.NotificationSubject{
7374
Type: api.NotifySubjectCommit,
74-
Title: n.CommitID,
75+
Title: strings.TrimSpace(n.Commit.CommitMessage),
7576
URL: url,
7677
HTMLURL: url,
7778
}

‎services/uinotification/notify.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package uinotification
55

66
import (
77
"context"
8+
"slices"
89

910
activities_model "code.gitea.io/gitea/models/activities"
1011
"code.gitea.io/gitea/models/db"
@@ -363,6 +364,19 @@ func (ns *notificationService) UpdateRelease(ctx context.Context, doer *user_mod
363364
return
364365
}
365366

367+
repo, err := repo_model.GetRepositoryByID(ctx, rel.RepoID)
368+
if err != nil {
369+
log.Error("GetRepositoryByID: %v", err)
370+
return
371+
}
372+
if err := repo.LoadOwner(ctx); err != nil {
373+
log.Error("LoadOwner: %v", err)
374+
return
375+
}
376+
if !repo.Owner.IsOrganization() && !slices.Contains(repoWatcherIDs, repo.Owner.ID) && repo.Owner.ID != doer.ID {
377+
repoWatcherIDs = append(repoWatcherIDs, repo.Owner.ID)
378+
}
379+
366380
for _, watcherID := range repoWatcherIDs {
367381
if watcherID == doer.ID {
368382
// Do not notify the publisher of the release

‎templates/swagger/v1_json.tmpl‎

Lines changed: 4 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎tests/integration/api_notification_test.go‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
package integration
55

66
import (
7+
"encoding/base64"
78
"fmt"
89
"net/http"
10+
"net/url"
911
"testing"
12+
"time"
1013

1114
activities_model "code.gitea.io/gitea/models/activities"
1215
auth_model "code.gitea.io/gitea/models/auth"
@@ -15,6 +18,7 @@ import (
1518
"code.gitea.io/gitea/models/unittest"
1619
user_model "code.gitea.io/gitea/models/user"
1720
api "code.gitea.io/gitea/modules/structs"
21+
repo_service "code.gitea.io/gitea/services/repository"
1822
"code.gitea.io/gitea/tests"
1923

2024
"github.com/stretchr/testify/assert"
@@ -213,3 +217,137 @@ func TestAPINotificationPUT(t *testing.T) {
213217
assert.True(t, apiNL[0].Unread)
214218
assert.False(t, apiNL[0].Pinned)
215219
}
220+
221+
func TestAPICommitNotification(t *testing.T) {
222+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
223+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
224+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
225+
226+
session := loginUser(t, user2.Name)
227+
token1 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
228+
229+
content := "This is a test commit"
230+
contentEncoded := base64.StdEncoding.EncodeToString([]byte(content))
231+
// push a commit with @user2 in the commit message, it's expected to create a notification
232+
createFileOptions := api.CreateFileOptions{
233+
FileOptions: api.FileOptions{
234+
BranchName: "master",
235+
NewBranchName: "master",
236+
Message: "This is a test commit to mention @user2",
237+
Author: api.Identity{
238+
Name: "Anne Doe",
239+
Email: "[email protected]",
240+
},
241+
Committer: api.Identity{
242+
Name: "John Doe",
243+
Email: "[email protected]",
244+
},
245+
Dates: api.CommitDateOptions{
246+
Author: time.Unix(946684810, 0),
247+
Committer: time.Unix(978307190, 0),
248+
},
249+
},
250+
ContentBase64: contentEncoded,
251+
}
252+
253+
req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/new_commit_notification.txt", user2.Name, repo1.Name), &createFileOptions).
254+
AddTokenAuth(token1)
255+
resp := MakeRequest(t, req, http.StatusCreated)
256+
257+
// Check notifications are as expected
258+
token2 := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteNotification)
259+
req = NewRequest(t, "GET", "/api/v1/notifications?all=true").
260+
AddTokenAuth(token2)
261+
resp = MakeRequest(t, req, http.StatusOK)
262+
var apiNL []api.NotificationThread
263+
DecodeJSON(t, resp, &apiNL)
264+
265+
assert.Equal(t, api.NotifySubjectCommit, apiNL[0].Subject.Type)
266+
assert.Equal(t, "This is a test commit to mention @user2", apiNL[0].Subject.Title)
267+
assert.True(t, apiNL[0].Unread)
268+
assert.False(t, apiNL[0].Pinned)
269+
})
270+
}
271+
272+
func TestAPIReleaseNotification(t *testing.T) {
273+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
274+
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
275+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
276+
repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
277+
278+
session1 := loginUser(t, user1.Name)
279+
token1 := getTokenForLoggedInUser(t, session1, auth_model.AccessTokenScopeWriteRepository)
280+
281+
// user1 create a release, it's expected to create a notification
282+
createNewReleaseUsingAPI(t, token1, user2, repo1, "v0.0.2", "", "v0.0.2 is released", "test notification release")
283+
284+
// user2 login to check notifications
285+
session2 := loginUser(t, user2.Name)
286+
287+
// Check notifications are as expected
288+
token2 := getTokenForLoggedInUser(t, session2, auth_model.AccessTokenScopeWriteNotification)
289+
req := NewRequest(t, "GET", "/api/v1/notifications?all=true").
290+
AddTokenAuth(token2)
291+
resp := MakeRequest(t, req, http.StatusOK)
292+
var apiNL []api.NotificationThread
293+
DecodeJSON(t, resp, &apiNL)
294+
295+
assert.Equal(t, api.NotifySubjectRelease, apiNL[0].Subject.Type)
296+
assert.Equal(t, "v0.0.2 is released", apiNL[0].Subject.Title)
297+
assert.True(t, apiNL[0].Unread)
298+
assert.False(t, apiNL[0].Pinned)
299+
})
300+
}
301+
302+
func TestAPIRepoTransferNotification(t *testing.T) {
303+
onGiteaRun(t, func(t *testing.T, u *url.URL) {
304+
user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
305+
306+
session1 := loginUser(t, user2.Name)
307+
token1 := getTokenForLoggedInUser(t, session1, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteUser)
308+
309+
// create repo to move
310+
repoName := "moveME"
311+
apiRepo := new(api.Repository)
312+
req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos", &api.CreateRepoOption{
313+
Name: repoName,
314+
Description: "repo move around",
315+
Private: false,
316+
Readme: "Default",
317+
AutoInit: true,
318+
}).AddTokenAuth(token1)
319+
resp := MakeRequest(t, req, http.StatusCreated)
320+
DecodeJSON(t, resp, apiRepo)
321+
322+
defer func() {
323+
_ = repo_service.DeleteRepositoryDirectly(db.DefaultContext, apiRepo.ID)
324+
}()
325+
326+
// repo user1/moveME created, now transfer it to org6
327+
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: apiRepo.ID})
328+
session2 := loginUser(t, user2.Name)
329+
token2 := getTokenForLoggedInUser(t, session2, auth_model.AccessTokenScopeWriteRepository)
330+
req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/transfer", repo.OwnerName, repo.Name), &api.TransferRepoOption{
331+
NewOwner: "org6",
332+
TeamIDs: nil,
333+
}).AddTokenAuth(token2)
334+
MakeRequest(t, req, http.StatusCreated)
335+
336+
// user5 login to check notifications, because user5 is a member of org6's owners team
337+
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
338+
session5 := loginUser(t, user5.Name)
339+
340+
// Check notifications are as expected
341+
token5 := getTokenForLoggedInUser(t, session5, auth_model.AccessTokenScopeWriteNotification)
342+
req = NewRequest(t, "GET", "/api/v1/notifications?all=true").
343+
AddTokenAuth(token5)
344+
resp = MakeRequest(t, req, http.StatusOK)
345+
var apiNL []api.NotificationThread
346+
DecodeJSON(t, resp, &apiNL)
347+
348+
assert.Equal(t, api.NotifySubjectRepository, apiNL[0].Subject.Type)
349+
assert.Equal(t, "user2/moveME", apiNL[0].Subject.Title)
350+
assert.True(t, apiNL[0].Unread)
351+
assert.False(t, apiNL[0].Pinned)
352+
})
353+
}

0 commit comments

Comments
 (0)