From 1ca5367f27c5c2a1eec4ba9957776fef54f4521d Mon Sep 17 00:00:00 2001 From: kolaente Date: Sat, 6 Jun 2026 23:29:59 +0200 Subject: [PATCH] feat(api/v2): add notifications list/mark-read + mark-all on /api/v2 Ports the v1 DatabaseNotifications routes to the Huma /api/v2 API: - GET /notifications lists the caller's own notifications (paginated) - PUT /notifications/{notificationid} marks one (un-)read - POST /notifications is a custom action marking all as read; the link-share guard, session and commit live in the handler since there is no CRUDable Do* for a bulk mark. Adds fixture rows and a webtest matrix mirroring the v1 model behaviour (own-only visibility, mark-(un)read, link-share refusal on every route). --- pkg/db/fixtures/notifications.yml | 22 +++- pkg/routes/api/v2/notifications.go | 139 ++++++++++++++++++++++ pkg/webtests/huma_notification_test.go | 156 +++++++++++++++++++++++++ 3 files changed, 316 insertions(+), 1 deletion(-) create mode 100644 pkg/routes/api/v2/notifications.go create mode 100644 pkg/webtests/huma_notification_test.go diff --git a/pkg/db/fixtures/notifications.yml b/pkg/db/fixtures/notifications.yml index fe51488c7..bb4f90062 100644 --- a/pkg/db/fixtures/notifications.yml +++ b/pkg/db/fixtures/notifications.yml @@ -1 +1,21 @@ -[] +- id: 1 + notifiable_id: 1 + notification: '{"test":"notification one"}' + name: test.notification + subject_id: 1 + read_at: null + created: 2022-01-01 00:00:00 +- id: 2 + notifiable_id: 1 + notification: '{"test":"notification two"}' + name: test.notification + subject_id: 2 + read_at: null + created: 2022-01-02 00:00:00 +- id: 3 + notifiable_id: 2 + notification: '{"test":"other user"}' + name: test.notification + subject_id: 3 + read_at: null + created: 2022-01-03 00:00:00 diff --git a/pkg/routes/api/v2/notifications.go b/pkg/routes/api/v2/notifications.go new file mode 100644 index 000000000..2ea3a8112 --- /dev/null +++ b/pkg/routes/api/v2/notifications.go @@ -0,0 +1,139 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package apiv2 + +import ( + "context" + "fmt" + "net/http" + + "code.vikunja.io/api/pkg/db" + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/notifications" + "code.vikunja.io/api/pkg/web/handler" + + "github.com/danielgtaylor/huma/v2" +) + +// Element type is the foreign notifications.DatabaseNotification because that's +// what models.DatabaseNotifications.ReadAll returns directly, not a models wrapper. +type notificationListBody struct { + Body Paginated[*notifications.DatabaseNotification] +} + +// markAllReadBody is the mark-all-as-read confirmation. The action has no +// resource to return, so it carries a short status message rather than reusing +// emptyBody (204) — mirrors v1's {"message":"success"} response. +type markAllReadBody struct { + Body struct { + Message string `json:"message" readOnly:"true" doc:"A confirmation message."` + } +} + +// RegisterNotificationRoutes wires notification list / mark-read / mark-all onto the Huma API. +func RegisterNotificationRoutes(api huma.API) { + tags := []string{"notifications"} + + Register(api, huma.Operation{ + OperationID: "notifications-list", + Summary: "List notifications", + Description: "Returns the authenticated user's own notifications, newest first. Link shares have no notifications and are refused.", + Method: http.MethodGet, + Path: "/notifications", + Tags: tags, + }, notificationsList) + + Register(api, huma.Operation{ + OperationID: "notifications-mark-read", + Summary: "Mark a notification as (un-)read", + Description: "Marks one of the authenticated user's notifications as read or unread. A user can only mark their own notifications.", + Method: http.MethodPut, + Path: "/notifications/{notificationid}", + Tags: tags, + }, notificationsMarkRead) + + Register(api, huma.Operation{ + OperationID: "notifications-mark-all-read", + Summary: "Mark all notifications as read", + Description: "Marks every notification of the authenticated user as read. Link shares have no notifications and are refused.", + Method: http.MethodPost, + Path: "/notifications", + Tags: tags, + }, notificationsMarkAllRead) +} + +func init() { AddRouteRegistrar(RegisterNotificationRoutes) } + +func notificationsList(ctx context.Context, in *ListParams) (*notificationListBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + result, _, total, err := handler.DoReadAll(ctx, &models.DatabaseNotifications{}, a, in.Q, in.Page, in.PerPage) + if err != nil { + return nil, translateDomainError(err) + } + items, ok := result.([]*notifications.DatabaseNotification) + if !ok { + return nil, fmt.Errorf("notifications.ReadAll returned unexpected type %T (expected []*notifications.DatabaseNotification)", result) + } + return ¬ificationListBody{Body: NewPaginated(items, total, in.Page, in.PerPage)}, nil +} + +func notificationsMarkRead(ctx context.Context, in *struct { + ID int64 `path:"notificationid"` + Body models.DatabaseNotifications +}) (*singleBody[models.DatabaseNotifications], error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + n := &in.Body + n.ID = in.ID // URL wins over body + if err := handler.DoUpdate(ctx, n, a); err != nil { + return nil, translateDomainError(err) + } + return &singleBody[models.DatabaseNotifications]{Body: n}, nil +} + +// notificationsMarkAllRead is a custom action: there is no CRUDable Do* for a +// bulk mark, so the handler owns the link-share guard, the session and the +// commit. Mirrors apiv1.MarkAllNotificationsAsRead. +func notificationsMarkAllRead(ctx context.Context, _ *struct{}) (*markAllReadBody, error) { + a, err := authFromCtx(ctx) + if err != nil { + return nil, err + } + if _, is := a.(*models.LinkSharing); is { + return nil, huma.Error403Forbidden("link shares cannot have notifications") + } + + s := db.NewSession() + defer s.Close() + + if err := notifications.MarkAllNotificationsAsRead(s, a.GetID()); err != nil { + _ = s.Rollback() + return nil, translateDomainError(err) + } + if err := s.Commit(); err != nil { + return nil, translateDomainError(err) + } + + out := &markAllReadBody{} + out.Body.Message = "success" + return out, nil +} diff --git a/pkg/webtests/huma_notification_test.go b/pkg/webtests/huma_notification_test.go new file mode 100644 index 000000000..ac7c54a06 --- /dev/null +++ b/pkg/webtests/huma_notification_test.go @@ -0,0 +1,156 @@ +// Vikunja is a to-do list application to facilitate your life. +// Copyright 2018-present Vikunja and contributors. All rights reserved. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package webtests + +import ( + "encoding/json" + "net/http" + "testing" + + "code.vikunja.io/api/pkg/models" + "code.vikunja.io/api/pkg/modules/auth" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestHumaNotification mirrors v1's notification routes (notifications.yml has +// no v1 webtest, so this is ported 1:1 from the v1 handler/model behaviour). +// Link-share guards and mark-all live in separate top-level funcs below because +// they need a dedicated echo.Echo — interleaving setupTestEnv() inside the +// webHandlerTestV2 matrix rotates the JWT key out from under its cached env. +// +// Fixture topology (see pkg/db/fixtures/notifications.yml): +// - #1, #2: belong to user1, both unread. +// - #3: belongs to user2, unread — must stay invisible/untouched for user1. +func TestHumaNotification(t *testing.T) { + testHandler := webHandlerTestV2{ + user: &testuser1, + basePath: "/api/v2/notifications", + idParam: "notificationid", + t: t, + } + + t.Run("ReadAll", func(t *testing.T) { + t.Run("Own notifications only", func(t *testing.T) { + rec, err := testHandler.testReadAllWithUser(nil, nil) + require.NoError(t, err) + + ids := notificationIDsFromReadAll(t, rec.Body.Bytes()) + // Exact set: user1 sees only their own notifications, not user2's #3. + assert.ElementsMatch(t, []int64{1, 2}, ids, + "ReadAll must return exactly {1,2}; body: %s", rec.Body.String()) + assert.NotContains(t, ids, int64(3), "user2's notification #3 must be hidden") + }) + }) + + t.Run("MarkAsRead", func(t *testing.T) { + t.Run("Normal - mark own as read", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"notificationid": "1"}, `{"read":true}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"id":1`) + // A read notification carries a non-zero read_at timestamp. + assert.NotContains(t, rec.Body.String(), `"read_at":"0001-01-01T00:00:00Z"`, + "read_at must be set after marking read; body: %s", rec.Body.String()) + }) + t.Run("Mark own as unread", func(t *testing.T) { + rec, err := testHandler.testUpdateWithUser(nil, map[string]string{"notificationid": "2"}, `{"read":false}`) + require.NoError(t, err) + assert.Contains(t, rec.Body.String(), `"read_at":"0001-01-01T00:00:00Z"`, + "read_at must be zeroed when marking unread; body: %s", rec.Body.String()) + }) + t.Run("Forbidden - other user's notification (#3)", func(t *testing.T) { + // CanUpdate scopes by notifiable_id; #3 belongs to user2. + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"notificationid": "3"}, `{"read":true}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + t.Run("Forbidden - nonexistent", func(t *testing.T) { + _, err := testHandler.testUpdateWithUser(nil, map[string]string{"notificationid": "9999"}, `{"read":true}`) + require.Error(t, err) + assert.Equal(t, http.StatusForbidden, getHTTPErrorCode(err)) + }) + }) +} + +// TestHumaNotification_MarkAllAsRead covers the custom bulk action: it marks +// every notification of the caller as read and leaves other users untouched. +func TestHumaNotification_MarkAllAsRead(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token := humaTokenFor(t, &testuser1) + + rec := humaRequest(t, e, http.MethodPost, "/api/v2/notifications", "", token, "") + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + assert.Contains(t, rec.Body.String(), `"message":"success"`) + + // Re-list and confirm all of user1's notifications are now read. + list := humaRequest(t, e, http.MethodGet, "/api/v2/notifications", "", token, "") + require.Equal(t, http.StatusOK, list.Code, "body: %s", list.Body.String()) + assert.NotContains(t, list.Body.String(), `"read_at":"0001-01-01T00:00:00Z"`, + "every notification must be read after mark-all; body: %s", list.Body.String()) + + // user2's notification #3 must remain untouched (unread). + otherList := humaRequest(t, e, http.MethodGet, "/api/v2/notifications", "", humaTokenFor(t, &testuser2), "") + require.Equal(t, http.StatusOK, otherList.Code, "body: %s", otherList.Body.String()) + assert.Contains(t, otherList.Body.String(), `"read_at":"0001-01-01T00:00:00Z"`, + "another user's notifications must stay unread; body: %s", otherList.Body.String()) +} + +// TestHumaNotification_LinkShareForbidden ports v1's guard: a link-share auth +// has no notifications, so list / mark-read / mark-all all refuse it (403). +func TestHumaNotification_LinkShareForbidden(t *testing.T) { + e, err := setupTestEnv() + require.NoError(t, err) + token, err := auth.NewLinkShareJWTAuthtoken(&models.LinkSharing{ + ID: 1, + Hash: "test", + ProjectID: 1, + Permission: models.PermissionRead, + SharingType: models.SharingTypeWithoutPassword, + SharedByID: 1, + }) + require.NoError(t, err) + + t.Run("list", func(t *testing.T) { + rec := humaRequest(t, e, http.MethodGet, "/api/v2/notifications", "", token, "") + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("mark read", func(t *testing.T) { + rec := humaRequest(t, e, http.MethodPut, "/api/v2/notifications/1", `{"read":true}`, token, "") + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) + t.Run("mark all read", func(t *testing.T) { + rec := humaRequest(t, e, http.MethodPost, "/api/v2/notifications", "", token, "") + assert.Equal(t, http.StatusForbidden, rec.Code, "body: %s", rec.Body.String()) + }) +} + +func notificationIDsFromReadAll(t *testing.T, body []byte) []int64 { + t.Helper() + var resp struct { + Items []struct { + ID int64 `json:"id"` + } `json:"items"` + } + require.NoError(t, json.Unmarshal(body, &resp), "ReadAll body must be a paginated envelope: %s", string(body)) + ids := make([]int64, 0, len(resp.Items)) + for _, it := range resp.Items { + ids = append(ids, it.ID) + } + return ids +}