From 489e9162fc650c92c20c3c301845a1e772eca64e Mon Sep 17 00:00:00 2001 From: zeripath Date: Mon, 13 Jul 2020 21:52:05 +0100 Subject: [PATCH] Extend Notifications API and return pinned notifications by default (#12164) (#12232) Backport #12164 This PR extends the notifications API to allow specific notification statuses to be searched for and to allow setting of notifications to statuses other than read. By default unread and pinned statuses will be returned when querying for notifications - however pinned statuses will not be marked as read. Close #12152 Signed-off-by: Andrew Thornton art27@cantab.net --- integrations/api_notification_test.go | 6 +-- integrations/eventsource_test.go | 4 +- models/notification.go | 6 +-- routers/api/v1/notify/repo.go | 78 ++++++++++++++++++++++++--- routers/api/v1/notify/threads.go | 13 ++++- routers/api/v1/notify/user.go | 48 ++++++++++++++--- templates/swagger/v1_json.tmpl | 75 +++++++++++++++++++++++++- 7 files changed, 206 insertions(+), 24 deletions(-) diff --git a/integrations/api_notification_test.go b/integrations/api_notification_test.go index 3296604a08..4204173420 100644 --- a/integrations/api_notification_test.go +++ b/integrations/api_notification_test.go @@ -55,7 +55,7 @@ func TestAPINotification(t *testing.T) { assert.EqualValues(t, false, apiNL[2].Pinned) // -- GET /repos/{owner}/{repo}/notifications -- - req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/notifications?token=%s", user2.Name, repo1.Name, token)) + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/repos/%s/%s/notifications?status-types=unread&token=%s", user2.Name, repo1.Name, token)) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiNL) @@ -92,7 +92,7 @@ func TestAPINotification(t *testing.T) { assert.True(t, new.New > 0) // -- mark notifications as read -- - req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?token=%s", token)) + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?status-types=unread&token=%s", token)) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiNL) assert.Len(t, apiNL, 2) @@ -101,7 +101,7 @@ func TestAPINotification(t *testing.T) { req = NewRequest(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/notifications?last_read_at=%s&token=%s", user2.Name, repo1.Name, lastReadAt, token)) resp = session.MakeRequest(t, req, http.StatusResetContent) - req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?token=%s", token)) + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?status-types=unread&token=%s", token)) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiNL) assert.Len(t, apiNL, 1) diff --git a/integrations/eventsource_test.go b/integrations/eventsource_test.go index bc15453147..caaf8c2cef 100644 --- a/integrations/eventsource_test.go +++ b/integrations/eventsource_test.go @@ -59,7 +59,7 @@ func TestEventSourceManagerRun(t *testing.T) { var apiNL []api.NotificationThread // -- mark notifications as read -- - req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?token=%s", token)) + req := NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?status-types=unread&token=%s", token)) resp := session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiNL) @@ -69,7 +69,7 @@ func TestEventSourceManagerRun(t *testing.T) { req = NewRequest(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/%s/notifications?last_read_at=%s&token=%s", user2.Name, repo1.Name, lastReadAt, token)) resp = session.MakeRequest(t, req, http.StatusResetContent) - req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?token=%s", token)) + req = NewRequest(t, "GET", fmt.Sprintf("/api/v1/notifications?token=%s&status-types=unread", token)) resp = session.MakeRequest(t, req, http.StatusOK) DecodeJSON(t, resp, &apiNL) assert.Len(t, apiNL, 1) diff --git a/models/notification.go b/models/notification.go index 1c378a1350..9258b68f22 100644 --- a/models/notification.go +++ b/models/notification.go @@ -72,7 +72,7 @@ type FindNotificationOptions struct { UserID int64 RepoID int64 IssueID int64 - Status NotificationStatus + Status []NotificationStatus UpdatedAfterUnix int64 UpdatedBeforeUnix int64 } @@ -89,8 +89,8 @@ func (opts *FindNotificationOptions) ToCond() builder.Cond { if opts.IssueID != 0 { cond = cond.And(builder.Eq{"notification.issue_id": opts.IssueID}) } - if opts.Status != 0 { - cond = cond.And(builder.Eq{"notification.status": opts.Status}) + if len(opts.Status) > 0 { + cond = cond.And(builder.In("notification.status", opts.Status)) } if opts.UpdatedAfterUnix != 0 { cond = cond.And(builder.Gte{"notification.updated_unix": opts.UpdatedAfterUnix}) diff --git a/routers/api/v1/notify/repo.go b/routers/api/v1/notify/repo.go index 10c00c5467..ca9aea2565 100644 --- a/routers/api/v1/notify/repo.go +++ b/routers/api/v1/notify/repo.go @@ -11,9 +11,37 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/routers/api/v1/utils" ) +func statusStringToNotificationStatus(status string) models.NotificationStatus { + switch strings.ToLower(strings.TrimSpace(status)) { + case "unread": + return models.NotificationStatusUnread + case "read": + return models.NotificationStatusRead + case "pinned": + return models.NotificationStatusPinned + default: + return 0 + } +} + +func statusStringsToNotificationStatuses(statuses []string, defaultStatuses []string) []models.NotificationStatus { + if len(statuses) == 0 { + statuses = defaultStatuses + } + results := make([]models.NotificationStatus, 0, len(statuses)) + for _, status := range statuses { + notificationStatus := statusStringToNotificationStatus(status) + if notificationStatus > 0 { + results = append(results, notificationStatus) + } + } + return results +} + // ListRepoNotifications list users's notification threads on a specific repo func ListRepoNotifications(ctx *context.APIContext) { // swagger:operation GET /repos/{owner}/{repo}/notifications notification notifyGetRepoList @@ -39,6 +67,14 @@ func ListRepoNotifications(ctx *context.APIContext) { // description: If true, show notifications marked as read. Default value is false // type: string // required: false + // - name: status-types + // in: query + // description: "Show notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread & pinned" + // type: array + // collectionFormat: multi + // items: + // type: string + // required: false // - name: since // in: query // description: Only show notifications updated after the given time. This is a timestamp in RFC 3339 format @@ -75,9 +111,10 @@ func ListRepoNotifications(ctx *context.APIContext) { UpdatedBeforeUnix: before, UpdatedAfterUnix: since, } - qAll := strings.Trim(ctx.Query("all"), " ") - if qAll != "true" { - opts.Status = models.NotificationStatusUnread + + if !ctx.QueryBool("all") { + statuses := ctx.QueryStrings("status-types") + opts.Status = statusStringsToNotificationStatuses(statuses, []string{"unread", "pinned"}) } nl, err := models.GetNotifications(opts) if err != nil { @@ -97,7 +134,7 @@ func ListRepoNotifications(ctx *context.APIContext) { func ReadRepoNotifications(ctx *context.APIContext) { // swagger:operation PUT /repos/{owner}/{repo}/notifications notification notifyReadRepoList // --- - // summary: Mark notification threads as read on a specific repo + // summary: Mark notification threads as read, pinned or unread on a specific repo // consumes: // - application/json // produces: @@ -113,6 +150,24 @@ func ReadRepoNotifications(ctx *context.APIContext) { // description: name of the repo // type: string // required: true + // - name: all + // in: query + // description: If true, mark all notifications on this repo. Default value is false + // type: string + // required: false + // - name: status-types + // in: query + // description: "Mark notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread." + // type: array + // collectionFormat: multi + // items: + // type: string + // required: false + // - name: to-status + // in: query + // description: Status to mark notifications as. Defaults to read. + // type: string + // required: false // - name: last_read_at // in: query // description: Describes the last point that notifications were checked. Anything updated since this time will not be updated. @@ -135,11 +190,17 @@ func ReadRepoNotifications(ctx *context.APIContext) { lastRead = tmpLastRead.Unix() } } + opts := models.FindNotificationOptions{ UserID: ctx.User.ID, RepoID: ctx.Repo.Repository.ID, UpdatedBeforeUnix: lastRead, - Status: models.NotificationStatusUnread, + } + + if !ctx.QueryBool("all") { + statuses := ctx.QueryStrings("status-types") + opts.Status = statusStringsToNotificationStatuses(statuses, []string{"unread"}) + log.Error("%v", opts.Status) } nl, err := models.GetNotifications(opts) if err != nil { @@ -147,8 +208,13 @@ func ReadRepoNotifications(ctx *context.APIContext) { return } + targetStatus := statusStringToNotificationStatus(ctx.Query("to-status")) + if targetStatus == 0 { + targetStatus = models.NotificationStatusRead + } + for _, n := range nl { - err := models.SetNotificationStatus(n.ID, ctx.User, models.NotificationStatusRead) + err := models.SetNotificationStatus(n.ID, ctx.User, targetStatus) if err != nil { ctx.InternalServerError(err) return diff --git a/routers/api/v1/notify/threads.go b/routers/api/v1/notify/threads.go index d0119e9938..86ae2dca31 100644 --- a/routers/api/v1/notify/threads.go +++ b/routers/api/v1/notify/threads.go @@ -62,6 +62,12 @@ func ReadThread(ctx *context.APIContext) { // description: id of notification thread // type: string // required: true + // - name: to-status + // in: query + // description: Status to mark notifications as + // type: string + // default: read + // required: false // responses: // "205": // "$ref": "#/responses/empty" @@ -75,7 +81,12 @@ func ReadThread(ctx *context.APIContext) { return } - err := models.SetNotificationStatus(n.ID, ctx.User, models.NotificationStatusRead) + targetStatus := statusStringToNotificationStatus(ctx.Query("to-status")) + if targetStatus == 0 { + targetStatus = models.NotificationStatusRead + } + + err := models.SetNotificationStatus(n.ID, ctx.User, targetStatus) if err != nil { ctx.InternalServerError(err) return diff --git a/routers/api/v1/notify/user.go b/routers/api/v1/notify/user.go index 7f731e25d4..08a2f08415 100644 --- a/routers/api/v1/notify/user.go +++ b/routers/api/v1/notify/user.go @@ -29,6 +29,14 @@ func ListNotifications(ctx *context.APIContext) { // description: If true, show notifications marked as read. Default value is false // type: string // required: false + // - name: status-types + // in: query + // description: "Show notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread & pinned." + // type: array + // collectionFormat: multi + // items: + // type: string + // required: false // - name: since // in: query // description: Only show notifications updated after the given time. This is a timestamp in RFC 3339 format @@ -64,9 +72,9 @@ func ListNotifications(ctx *context.APIContext) { UpdatedBeforeUnix: before, UpdatedAfterUnix: since, } - qAll := strings.Trim(ctx.Query("all"), " ") - if qAll != "true" { - opts.Status = models.NotificationStatusUnread + if !ctx.QueryBool("all") { + statuses := ctx.QueryStrings("status-types") + opts.Status = statusStringsToNotificationStatuses(statuses, []string{"unread", "pinned"}) } nl, err := models.GetNotifications(opts) if err != nil { @@ -82,11 +90,11 @@ func ListNotifications(ctx *context.APIContext) { ctx.JSON(http.StatusOK, nl.APIFormat()) } -// ReadNotifications mark notification threads as read +// ReadNotifications mark notification threads as read, unread, or pinned func ReadNotifications(ctx *context.APIContext) { // swagger:operation PUT /notifications notification notifyReadList // --- - // summary: Mark notification threads as read + // summary: Mark notification threads as read, pinned or unread // consumes: // - application/json // produces: @@ -98,6 +106,24 @@ func ReadNotifications(ctx *context.APIContext) { // type: string // format: date-time // required: false + // - name: all + // in: query + // description: If true, mark all notifications on this repo. Default value is false + // type: string + // required: false + // - name: status-types + // in: query + // description: "Mark notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread." + // type: array + // collectionFormat: multi + // items: + // type: string + // required: false + // - name: to-status + // in: query + // description: Status to mark notifications as, Defaults to read. + // type: string + // required: false // responses: // "205": // "$ref": "#/responses/empty" @@ -117,7 +143,10 @@ func ReadNotifications(ctx *context.APIContext) { opts := models.FindNotificationOptions{ UserID: ctx.User.ID, UpdatedBeforeUnix: lastRead, - Status: models.NotificationStatusUnread, + } + if !ctx.QueryBool("all") { + statuses := ctx.QueryStrings("status-types") + opts.Status = statusStringsToNotificationStatuses(statuses, []string{"unread"}) } nl, err := models.GetNotifications(opts) if err != nil { @@ -125,8 +154,13 @@ func ReadNotifications(ctx *context.APIContext) { return } + targetStatus := statusStringToNotificationStatus(ctx.Query("to-status")) + if targetStatus == 0 { + targetStatus = models.NotificationStatusRead + } + for _, n := range nl { - err := models.SetNotificationStatus(n.ID, ctx.User, models.NotificationStatusRead) + err := models.SetNotificationStatus(n.ID, ctx.User, targetStatus) if err != nil { ctx.InternalServerError(err) return diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 5058166ab7..12c732a9a9 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -459,6 +459,16 @@ "name": "all", "in": "query" }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "description": "Show notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread \u0026 pinned.", + "name": "status-types", + "in": "query" + }, { "type": "string", "format": "date-time", @@ -502,7 +512,7 @@ "tags": [ "notification" ], - "summary": "Mark notification threads as read", + "summary": "Mark notification threads as read, pinned or unread", "operationId": "notifyReadList", "parameters": [ { @@ -511,6 +521,28 @@ "description": "Describes the last point that notifications were checked. Anything updated since this time will not be updated.", "name": "last_read_at", "in": "query" + }, + { + "type": "string", + "description": "If true, mark all notifications on this repo. Default value is false", + "name": "all", + "in": "query" + }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "description": "Mark notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread.", + "name": "status-types", + "in": "query" + }, + { + "type": "string", + "description": "Status to mark notifications as, Defaults to read.", + "name": "to-status", + "in": "query" } ], "responses": { @@ -587,6 +619,13 @@ "name": "id", "in": "path", "required": true + }, + { + "type": "string", + "default": "read", + "description": "Status to mark notifications as", + "name": "to-status", + "in": "query" } ], "responses": { @@ -6290,6 +6329,16 @@ "name": "all", "in": "query" }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "description": "Show notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread \u0026 pinned", + "name": "status-types", + "in": "query" + }, { "type": "string", "format": "date-time", @@ -6333,7 +6382,7 @@ "tags": [ "notification" ], - "summary": "Mark notification threads as read on a specific repo", + "summary": "Mark notification threads as read, pinned or unread on a specific repo", "operationId": "notifyReadRepoList", "parameters": [ { @@ -6350,6 +6399,28 @@ "in": "path", "required": true }, + { + "type": "string", + "description": "If true, mark all notifications on this repo. Default value is false", + "name": "all", + "in": "query" + }, + { + "type": "array", + "items": { + "type": "string" + }, + "collectionFormat": "multi", + "description": "Mark notifications with the provided status types. Options are: unread, read and/or pinned. Defaults to unread.", + "name": "status-types", + "in": "query" + }, + { + "type": "string", + "description": "Status to mark notifications as. Defaults to read.", + "name": "to-status", + "in": "query" + }, { "type": "string", "format": "date-time",