From 63591016b3ecd79ab1172cd98e2e830a09d6f515 Mon Sep 17 00:00:00 2001 From: zeripath Date: Sat, 11 Jul 2020 22:46:01 +0100 Subject: [PATCH] Extend Notifications API and return pinned notifications by default (#12164) * Extend notifications API and return pinned notifications in notifications list Signed-off-by: Andrew Thornton * fix swagger Signed-off-by: Andrew Thornton * Fix swagger again Signed-off-by: Andrew Thornton * fix test Signed-off-by: Andrew Thornton * remove spurious debugs * as per @6543 Signed-off-by: Andrew Thornton * Update models/notification.go * as per @6543 Signed-off-by: Andrew Thornton Co-authored-by: techknowlogick --- 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 3050cca1e5..49b493aa4f 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 649eaa8a37..9c3f9b1472 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 9290bfe079..795e179cb9 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": { @@ -6382,6 +6421,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", @@ -6425,7 +6474,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": [ { @@ -6442,6 +6491,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",