diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index 09e3bac9c84..e2abece907c 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -4927,6 +4927,10 @@ components: type: integer format: int64 description: The time in milliseconds the recap was marked as read + viewed_at: + type: integer + format: int64 + description: The time in milliseconds the recap was marked as viewed (set in bulk when the recaps page is opened) total_message_count: type: integer description: Total number of messages summarized across all channels diff --git a/api/v4/source/recaps.yaml b/api/v4/source/recaps.yaml index 85419629a26..a42450c59e9 100644 --- a/api/v4/source/recaps.yaml +++ b/api/v4/source/recaps.yaml @@ -93,6 +93,46 @@ $ref: "#/components/responses/BadRequest" "401": $ref: "#/components/responses/Unauthorized" + "/api/v4/recaps/mark_viewed": + post: + tags: + - recaps + - ai + summary: Mark all of the authenticated user's finished recaps as viewed + description: > + Mark every not-yet-viewed completed or failed recap belonging to the + authenticated user as viewed at the current time. Pending and + processing recaps are not affected. Returns the IDs of the recaps + that were updated. The server broadcasts a `recap_updated` WebSocket + event for each affected recap. + + Typically called once when the recaps page is opened so the sidebar + unread badge can be cleared in bulk. + + ##### Permissions + + Must be authenticated. Operates only on the authenticated user's + own recaps. + + __Minimum server version__: 11.2 + operationId: MarkRecapsAsViewed + responses: + "200": + description: Recaps marked as viewed successfully + content: + application/json: + schema: + type: object + properties: + recap_ids: + type: array + items: + type: string + description: IDs of the recaps that were updated + "401": + $ref: "#/components/responses/Unauthorized" + "501": + description: AI Recaps feature flag is disabled "/api/v4/recaps/{recap_id}": get: tags: diff --git a/server/channels/api4/recap.go b/server/channels/api4/recap.go index 7f17e04ad2c..dda803bf8e3 100644 --- a/server/channels/api4/recap.go +++ b/server/channels/api4/recap.go @@ -15,6 +15,7 @@ import ( func (api *API) InitRecap() { api.BaseRoutes.Recaps.Handle("", api.APISessionRequired(createRecap)).Methods(http.MethodPost) api.BaseRoutes.Recaps.Handle("", api.APISessionRequired(getRecaps)).Methods(http.MethodGet) + api.BaseRoutes.Recaps.Handle("/mark_viewed", api.APISessionRequired(markRecapsAsViewed)).Methods(http.MethodPost) api.BaseRoutes.Recaps.Handle("/{recap_id:[A-Za-z0-9]+}", api.APISessionRequired(getRecap)).Methods(http.MethodGet) api.BaseRoutes.Recaps.Handle("/{recap_id:[A-Za-z0-9]+}/read", api.APISessionRequired(markRecapAsRead)).Methods(http.MethodPost) api.BaseRoutes.Recaps.Handle("/{recap_id:[A-Za-z0-9]+}/regenerate", api.APISessionRequired(regenerateRecap)).Methods(http.MethodPost) @@ -199,6 +200,31 @@ func markRecapAsRead(c *Context, w http.ResponseWriter, r *http.Request) { } } +func markRecapsAsViewed(c *Context, w http.ResponseWriter, r *http.Request) { + requireRecapsEnabled(c) + if c.Err != nil { + return + } + + auditRec := c.MakeAuditRecord(model.AuditEventMarkRecapsAsViewed, model.AuditStatusFail) + defer c.LogAuditRecWithLevel(auditRec, app.LevelContent) + auditRec.AddEventObjectType("recap") + + ids, err := c.App.MarkRecapsAsViewed(c.AppContext) + if err != nil { + c.Err = err + return + } + + auditRec.Success() + auditRec.AddMeta("recap_count", len(ids)) + auditRec.AddMeta("recap_ids", ids) + + if err := json.NewEncoder(w).Encode(map[string]any{"recap_ids": ids}); err != nil { + c.Logger.Warn("Error encoding response", mlog.Err(err)) + } +} + func regenerateRecap(c *Context, w http.ResponseWriter, r *http.Request) { requireRecapsEnabled(c) if c.Err != nil { diff --git a/server/channels/app/recap.go b/server/channels/app/recap.go index 31db75d15e4..6277add027d 100644 --- a/server/channels/app/recap.go +++ b/server/channels/app/recap.go @@ -113,6 +113,27 @@ func (a *App) MarkRecapAsRead(rctx request.CTX, recap *model.Recap) (*model.Reca return recap, nil } +// MarkRecapsAsViewed marks all of the user's not-yet-viewed completed/failed +// recaps as viewed at the current timestamp and broadcasts a recap_updated +// WebSocket event for each affected recap so other clients can refresh. +func (a *App) MarkRecapsAsViewed(rctx request.CTX) ([]string, *model.AppError) { + userID := rctx.Session().UserId + statuses := []string{model.RecapStatusCompleted, model.RecapStatusFailed} + + ids, err := a.Srv().Store().Recap().MarkRecapsAsViewed(userID, statuses) + if err != nil { + return nil, model.NewAppError("MarkRecapsAsViewed", "app.recap.mark_viewed.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + for _, id := range ids { + message := model.NewWebSocketEvent(model.WebsocketEventRecapUpdated, "", "", userID, nil, "") + message.Add("recap_id", id) + a.Publish(message) + } + + return ids, nil +} + // RegenerateRecap regenerates an existing recap func (a *App) RegenerateRecap(rctx request.CTX, userID string, recap *model.Recap) (*model.Recap, *model.AppError) { recapID := recap.Id @@ -134,9 +155,11 @@ func (a *App) RegenerateRecap(rctx request.CTX, userID string, recap *model.Reca return nil, model.NewAppError("RegenerateRecap", "app.recap.delete_channels.app_error", nil, "", http.StatusInternalServerError).Wrap(deleteErr) } - // Update recap status to pending and reset read status + // Update recap status to pending and reset read/viewed status so the recap + // reappears in the badge once it completes again. recap.Status = model.RecapStatusPending recap.ReadAt = 0 + recap.ViewedAt = 0 recap.UpdateAt = model.GetMillis() recap.TotalMessageCount = 0 diff --git a/server/channels/app/recap_test.go b/server/channels/app/recap_test.go index 574a6ac441c..390be1cf682 100644 --- a/server/channels/app/recap_test.go +++ b/server/channels/app/recap_test.go @@ -5,6 +5,7 @@ package app import ( "testing" + "time" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/request" @@ -244,6 +245,86 @@ func TestMarkRecapAsRead(t *testing.T) { }) } +func TestMarkRecapsAsViewed(t *testing.T) { + th := Setup(t).InitBasic(t) + + th.App.UpdateConfig(func(cfg *model.Config) { cfg.FeatureFlags.EnableAIRecaps = true }) + + save := func(userID, status string, viewedAt int64) string { + r := &model.Recap{ + Id: model.NewId(), + UserId: userID, + Title: "T", + CreateAt: model.GetMillis(), + UpdateAt: model.GetMillis(), + Status: status, + ViewedAt: viewedAt, + TotalMessageCount: 1, + } + _, err := th.App.Srv().Store().Recap().SaveRecap(r) + require.NoError(t, err) + return r.Id + } + + t.Run("marks completed and failed and ignores in-flight statuses", func(t *testing.T) { + userID := model.NewId() + completed := save(userID, model.RecapStatusCompleted, 0) + failed := save(userID, model.RecapStatusFailed, 0) + pending := save(userID, model.RecapStatusPending, 0) + processing := save(userID, model.RecapStatusProcessing, 0) + + ctx := th.Context.WithSession(&model.Session{UserId: userID}) + ids, appErr := th.App.MarkRecapsAsViewed(ctx) + require.Nil(t, appErr) + assert.ElementsMatch(t, []string{completed, failed}, ids) + + r1, err := th.App.Srv().Store().Recap().GetRecap(pending) + require.NoError(t, err) + assert.Zero(t, r1.ViewedAt) + r2, err := th.App.Srv().Store().Recap().GetRecap(processing) + require.NoError(t, err) + assert.Zero(t, r2.ViewedAt) + }) + + t.Run("returns empty list when nothing to mark", func(t *testing.T) { + ctx := th.Context.WithSession(&model.Session{UserId: model.NewId()}) + ids, appErr := th.App.MarkRecapsAsViewed(ctx) + require.Nil(t, appErr) + assert.Empty(t, ids) + }) + + t.Run("publishes a recap_updated websocket event per affected recap", func(t *testing.T) { + userID := th.BasicUser.Id + + // Two completed recaps that need to be marked as viewed. + a := save(userID, model.RecapStatusCompleted, 0) + b := save(userID, model.RecapStatusCompleted, 0) + + messages, closeWS := connectFakeWebSocket(t, th, userID, "", []model.WebsocketEventType{model.WebsocketEventRecapUpdated}) + defer closeWS() + + ctx := th.Context.WithSession(&model.Session{UserId: userID}) + ids, appErr := th.App.MarkRecapsAsViewed(ctx) + require.Nil(t, appErr) + assert.ElementsMatch(t, []string{a, b}, ids) + + seen := make(map[string]bool) + deadline := time.After(5 * time.Second) + for len(seen) < 2 { + select { + case msg := <-messages: + recapID, ok := msg.GetData()["recap_id"].(string) + require.True(t, ok, "recap_updated event missing recap_id") + seen[recapID] = true + case <-deadline: + require.Failf(t, "timed out waiting for recap_updated events", "received %d/2", len(seen)) + } + } + assert.True(t, seen[a]) + assert.True(t, seen[b]) + }) +} + func TestProcessRecapChannel(t *testing.T) { t.Run("process empty channel", func(t *testing.T) { th := Setup(t).InitBasic(t) diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 4cf308dc78c..46008e2090a 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -339,3 +339,7 @@ channels/db/migrations/postgres/000170_add_property_groups_version.down.sql channels/db/migrations/postgres/000170_add_property_groups_version.up.sql channels/db/migrations/postgres/000171_drop_property_fields_protected_index.down.sql channels/db/migrations/postgres/000171_drop_property_fields_protected_index.up.sql +channels/db/migrations/postgres/000172_add_recaps_viewed_at.down.sql +channels/db/migrations/postgres/000172_add_recaps_viewed_at.up.sql +channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.down.sql +channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.up.sql diff --git a/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.down.sql b/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.down.sql new file mode 100644 index 00000000000..40717b8b72c --- /dev/null +++ b/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE Recaps DROP COLUMN IF EXISTS ViewedAt; diff --git a/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.up.sql b/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.up.sql new file mode 100644 index 00000000000..8447cd653ab --- /dev/null +++ b/server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.up.sql @@ -0,0 +1 @@ +ALTER TABLE Recaps ADD COLUMN IF NOT EXISTS ViewedAt BIGINT NOT NULL DEFAULT 0; diff --git a/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.down.sql b/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.down.sql new file mode 100644 index 00000000000..40542f65204 --- /dev/null +++ b/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.down.sql @@ -0,0 +1,2 @@ +-- morph:nontransactional +DROP INDEX CONCURRENTLY IF EXISTS idx_recaps_user_id_viewed_at; diff --git a/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.up.sql b/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.up.sql new file mode 100644 index 00000000000..232139a2df5 --- /dev/null +++ b/server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.up.sql @@ -0,0 +1,2 @@ +-- morph:nontransactional +CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_recaps_user_id_viewed_at ON Recaps(UserId, ViewedAt); diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index e1c61c43535..755d759792c 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -10947,6 +10947,27 @@ func (s *RetryLayerRecapStore) MarkRecapAsRead(id string) error { } +func (s *RetryLayerRecapStore) MarkRecapsAsViewed(userId string, statuses []string) ([]string, error) { + + tries := 0 + for { + result, err := s.RecapStore.MarkRecapsAsViewed(userId, statuses) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerRecapStore) SaveRecap(recap *model.Recap) (*model.Recap, error) { tries := 0 diff --git a/server/channels/store/sqlstore/recap_store.go b/server/channels/store/sqlstore/recap_store.go index 5d2327556eb..ad8c2b59f74 100644 --- a/server/channels/store/sqlstore/recap_store.go +++ b/server/channels/store/sqlstore/recap_store.go @@ -23,6 +23,7 @@ var ( "UpdateAt", "DeleteAt", "ReadAt", + "ViewedAt", "TotalMessageCount", "Status", "BotID", @@ -72,6 +73,7 @@ func (s *SqlRecapStore) recapToMap(recap *model.Recap) map[string]any { "UpdateAt": recap.UpdateAt, "DeleteAt": recap.DeleteAt, "ReadAt": recap.ReadAt, + "ViewedAt": recap.ViewedAt, "TotalMessageCount": recap.TotalMessageCount, "Status": recap.Status, "BotID": recap.BotID, @@ -157,6 +159,8 @@ func (s *SqlRecapStore) UpdateRecap(recap *model.Recap) (*model.Recap, error) { "UpdateAt": recap.UpdateAt, "TotalMessageCount": recap.TotalMessageCount, "Status": recap.Status, + "ReadAt": recap.ReadAt, + "ViewedAt": recap.ViewedAt, }). Where(sq.Eq{"Id": recap.Id}) @@ -203,6 +207,34 @@ func (s *SqlRecapStore) MarkRecapAsRead(id string) error { return nil } +func (s *SqlRecapStore) MarkRecapsAsViewed(userId string, statuses []string) ([]string, error) { + if len(statuses) == 0 { + return nil, nil + } + + now := model.GetMillis() + + query, args, err := s.getQueryBuilder(). + Update("Recaps"). + SetMap(map[string]any{ + "ViewedAt": now, + "UpdateAt": now, + }). + Where(sq.Eq{"UserId": userId, "ViewedAt": 0, "DeleteAt": 0, "Status": statuses}). + Suffix("RETURNING Id"). + ToSql() + if err != nil { + return nil, errors.Wrap(err, "failed to build MarkRecapsAsViewed query") + } + + var ids []string + if err := s.GetMaster().Select(&ids, query, args...); err != nil { + return nil, errors.Wrapf(err, "failed to mark recaps as viewed for userId=%s", userId) + } + + return ids, nil +} + func (s *SqlRecapStore) DeleteRecap(id string) error { deleteAt := model.GetMillis() diff --git a/server/channels/store/sqlstore/recap_store_test.go b/server/channels/store/sqlstore/recap_store_test.go index 4f4778221a9..1dae88a7563 100644 --- a/server/channels/store/sqlstore/recap_store_test.go +++ b/server/channels/store/sqlstore/recap_store_test.go @@ -166,6 +166,101 @@ func TestRecapStore(t *testing.T) { } }) + t.Run("MarkRecapsAsViewed", func(t *testing.T) { + userId := model.NewId() + otherUserId := model.NewId() + + save := func(userID, status string, viewedAt int64) string { + r := &model.Recap{ + Id: model.NewId(), + UserId: userID, + Title: "T", + CreateAt: model.GetMillis(), + UpdateAt: model.GetMillis(), + Status: status, + ViewedAt: viewedAt, + BotID: "bot", + TotalMessageCount: 1, + } + _, err := ss.Recap().SaveRecap(r) + require.NoError(t, err) + return r.Id + } + + completed := save(userId, model.RecapStatusCompleted, 0) + failed := save(userId, model.RecapStatusFailed, 0) + pending := save(userId, model.RecapStatusPending, 0) + processing := save(userId, model.RecapStatusProcessing, 0) + alreadyViewed := save(userId, model.RecapStatusCompleted, 1234) + otherUser := save(otherUserId, model.RecapStatusCompleted, 0) + + ids, err := ss.Recap().MarkRecapsAsViewed(userId, []string{model.RecapStatusCompleted, model.RecapStatusFailed}) + require.NoError(t, err) + assert.ElementsMatch(t, []string{completed, failed}, ids) + + // completed and failed are now viewed + r1, err := ss.Recap().GetRecap(completed) + require.NoError(t, err) + assert.NotZero(t, r1.ViewedAt) + r2, err := ss.Recap().GetRecap(failed) + require.NoError(t, err) + assert.NotZero(t, r2.ViewedAt) + + // pending/processing untouched + r3, err := ss.Recap().GetRecap(pending) + require.NoError(t, err) + assert.Zero(t, r3.ViewedAt) + r4, err := ss.Recap().GetRecap(processing) + require.NoError(t, err) + assert.Zero(t, r4.ViewedAt) + + // already-viewed unchanged + r5, err := ss.Recap().GetRecap(alreadyViewed) + require.NoError(t, err) + assert.Equal(t, int64(1234), r5.ViewedAt) + + // other user untouched + r6, err := ss.Recap().GetRecap(otherUser) + require.NoError(t, err) + assert.Zero(t, r6.ViewedAt) + + // idempotent: second call returns no ids + ids2, err := ss.Recap().MarkRecapsAsViewed(userId, []string{model.RecapStatusCompleted, model.RecapStatusFailed}) + require.NoError(t, err) + assert.Empty(t, ids2) + }) + + t.Run("UpdateRecap persists ReadAt and ViewedAt resets", func(t *testing.T) { + recap := &model.Recap{ + Id: model.NewId(), + UserId: model.NewId(), + Title: "T", + CreateAt: model.GetMillis(), + UpdateAt: model.GetMillis(), + ReadAt: 500, + ViewedAt: 600, + Status: model.RecapStatusCompleted, + TotalMessageCount: 1, + BotID: "bot", + } + _, err := ss.Recap().SaveRecap(recap) + require.NoError(t, err) + + // RegenerateRecap-style reset: clear both timestamps and revert status. + recap.ReadAt = 0 + recap.ViewedAt = 0 + recap.Status = model.RecapStatusPending + recap.UpdateAt = model.GetMillis() + _, err = ss.Recap().UpdateRecap(recap) + require.NoError(t, err) + + fresh, err := ss.Recap().GetRecap(recap.Id) + require.NoError(t, err) + assert.Zero(t, fresh.ReadAt, "ReadAt should be reset by UpdateRecap") + assert.Zero(t, fresh.ViewedAt, "ViewedAt should be reset by UpdateRecap") + assert.Equal(t, model.RecapStatusPending, fresh.Status) + }) + t.Run("DeleteRecap", func(t *testing.T) { recap := &model.Recap{ Id: model.NewId(), diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 9b0d0d09f29..3d090500a5f 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1332,6 +1332,7 @@ type RecapStore interface { GetRecapsForUser(userId string, page, perPage int) ([]*model.Recap, error) UpdateRecapStatus(id, status string) error MarkRecapAsRead(id string) error + MarkRecapsAsViewed(userId string, statuses []string) ([]string, error) DeleteRecap(id string) error DeleteRecapChannels(recapId string) error SaveRecapChannel(recapChannel *model.RecapChannel) error diff --git a/server/channels/store/storetest/mocks/RecapStore.go b/server/channels/store/storetest/mocks/RecapStore.go index f74c42b972b..de356d2f917 100644 --- a/server/channels/store/storetest/mocks/RecapStore.go +++ b/server/channels/store/storetest/mocks/RecapStore.go @@ -158,6 +158,36 @@ func (_m *RecapStore) MarkRecapAsRead(id string) error { return r0 } +// MarkRecapsAsViewed provides a mock function with given fields: userId, statuses +func (_m *RecapStore) MarkRecapsAsViewed(userId string, statuses []string) ([]string, error) { + ret := _m.Called(userId, statuses) + + if len(ret) == 0 { + panic("no return value specified for MarkRecapsAsViewed") + } + + var r0 []string + var r1 error + if rf, ok := ret.Get(0).(func(string, []string) ([]string, error)); ok { + return rf(userId, statuses) + } + if rf, ok := ret.Get(0).(func(string, []string) []string); ok { + r0 = rf(userId, statuses) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]string) + } + } + + if rf, ok := ret.Get(1).(func(string, []string) error); ok { + r1 = rf(userId, statuses) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // SaveRecap provides a mock function with given fields: recap func (_m *RecapStore) SaveRecap(recap *model.Recap) (*model.Recap, error) { ret := _m.Called(recap) diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index 048c7a3005a..75fba604060 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -8715,6 +8715,22 @@ func (s *TimerLayerRecapStore) MarkRecapAsRead(id string) error { return err } +func (s *TimerLayerRecapStore) MarkRecapsAsViewed(userId string, statuses []string) ([]string, error) { + start := time.Now() + + result, err := s.RecapStore.MarkRecapsAsViewed(userId, statuses) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("RecapStore.MarkRecapsAsViewed", success, elapsed) + } + return result, err +} + func (s *TimerLayerRecapStore) SaveRecap(recap *model.Recap) (*model.Recap, error) { start := time.Now() diff --git a/server/i18n/en.json b/server/i18n/en.json index ba99ee89cdd..f9307ba6f63 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -8292,6 +8292,10 @@ "id": "app.recap.mark_read.app_error", "translation": "Failed to mark recap as read." }, + { + "id": "app.recap.mark_viewed.app_error", + "translation": "Failed to mark recaps as viewed." + }, { "id": "app.recap.permission_denied", "translation": "No permission for recap." diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index 563c84d8b12..54f2b42fa79 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -311,12 +311,13 @@ const ( // Recaps const ( - AuditEventCreateRecap = "createRecap" // create recap summarizing channel content - AuditEventGetRecap = "getRecap" // view a single recap - AuditEventGetRecaps = "getRecaps" // list user's recaps - AuditEventMarkRecapAsRead = "markRecapAsRead" // mark recap as read - AuditEventRegenerateRecap = "regenerateRecap" // regenerate recap with updated channel content - AuditEventDeleteRecap = "deleteRecap" // delete recap + AuditEventCreateRecap = "createRecap" // create recap summarizing channel content + AuditEventGetRecap = "getRecap" // view a single recap + AuditEventGetRecaps = "getRecaps" // list user's recaps + AuditEventMarkRecapAsRead = "markRecapAsRead" // mark recap as read + AuditEventMarkRecapsAsViewed = "markRecapsAsViewed" // bulk mark user's finished recaps as viewed + AuditEventRegenerateRecap = "regenerateRecap" // regenerate recap with updated channel content + AuditEventDeleteRecap = "deleteRecap" // delete recap ) // Preferences diff --git a/server/public/model/recap.go b/server/public/model/recap.go index d3f02fc4c57..fc7276ec4d4 100644 --- a/server/public/model/recap.go +++ b/server/public/model/recap.go @@ -11,6 +11,7 @@ type Recap struct { UpdateAt int64 `json:"update_at"` DeleteAt int64 `json:"delete_at"` ReadAt int64 `json:"read_at"` + ViewedAt int64 `json:"viewed_at"` TotalMessageCount int `json:"total_message_count"` Status string `json:"status"` BotID string `json:"bot_id"` @@ -71,5 +72,6 @@ func (r *Recap) Auditable() map[string]any { "create_at": r.CreateAt, "update_at": r.UpdateAt, "read_at": r.ReadAt, + "viewed_at": r.ViewedAt, } } diff --git a/webapp/channels/src/components/recaps/recap_item.test.tsx b/webapp/channels/src/components/recaps/recap_item.test.tsx index 3c9c0fcec96..1a78ae276b1 100644 --- a/webapp/channels/src/components/recaps/recap_item.test.tsx +++ b/webapp/channels/src/components/recaps/recap_item.test.tsx @@ -87,6 +87,7 @@ describe('RecapItem', () => { update_at: 1000, delete_at: 0, read_at: 0, + viewed_at: 0, channels: [ { id: 'recap_channel1', diff --git a/webapp/channels/src/components/recaps/recap_processing.test.tsx b/webapp/channels/src/components/recaps/recap_processing.test.tsx index cdbbd801358..fbaa816d6c0 100644 --- a/webapp/channels/src/components/recaps/recap_processing.test.tsx +++ b/webapp/channels/src/components/recaps/recap_processing.test.tsx @@ -21,6 +21,7 @@ describe('RecapProcessing', () => { update_at: 1000, delete_at: 0, read_at: 0, + viewed_at: 0, channels: [], total_message_count: 0, }; diff --git a/webapp/channels/src/components/recaps/recaps.test.tsx b/webapp/channels/src/components/recaps/recaps.test.tsx index 2da6ff0803c..e8fe2940efc 100644 --- a/webapp/channels/src/components/recaps/recaps.test.tsx +++ b/webapp/channels/src/components/recaps/recaps.test.tsx @@ -4,15 +4,16 @@ import React from 'react'; import {MemoryRouter} from 'react-router-dom'; -import {renderWithContext} from 'tests/react_testing_utils'; +import {renderWithContext, waitFor} from 'tests/react_testing_utils'; import {LhsItemType, LhsPage} from 'types/store/lhs'; import Recaps from './recaps'; -const mockDispatch = jest.fn(); +const mockDispatch = jest.fn(() => Promise.resolve({data: []})); const mockGetAgents = jest.fn(() => ({type: 'GET_AGENTS'})); const mockGetRecaps = jest.fn((page: number, perPage: number) => ({type: 'GET_RECAPS', meta: {page, perPage}})); +const mockMarkRecapsAsViewed = jest.fn(() => ({type: 'MARK_RECAPS_VIEWED'})); const mockSelectLhsItem = jest.fn((type: string, id?: string) => { return {type: 'SELECT_LHS_ITEM', meta: {lhsType: type, id}}; }); @@ -29,6 +30,7 @@ jest.mock('mattermost-redux/actions/agents', () => ({ jest.mock('mattermost-redux/actions/recaps', () => ({ getRecaps: (page: number, perPage: number) => mockGetRecaps(page, perPage), + markRecapsAsViewed: () => mockMarkRecapsAsViewed(), })); jest.mock('mattermost-redux/selectors/entities/recaps', () => ({ @@ -55,10 +57,11 @@ describe('components/recaps/Recaps', () => { mockDispatch.mockClear(); mockGetAgents.mockClear(); mockGetRecaps.mockClear(); + mockMarkRecapsAsViewed.mockClear(); mockSelectLhsItem.mockClear(); }); - test('selects Recaps in the LHS on mount', () => { + test('selects Recaps in the LHS on mount', async () => { renderWithContext( @@ -71,5 +74,9 @@ describe('components/recaps/Recaps', () => { expect(mockDispatch).toHaveBeenCalledWith(expect.objectContaining({type: 'SELECT_LHS_ITEM'})); expect(mockDispatch).toHaveBeenCalledWith(expect.objectContaining({type: 'GET_RECAPS'})); expect(mockDispatch).toHaveBeenCalledWith({type: 'GET_AGENTS'}); + + // markRecapsAsViewed runs asynchronously after getRecaps resolves. + await waitFor(() => expect(mockMarkRecapsAsViewed).toHaveBeenCalled()); + expect(mockDispatch).toHaveBeenCalledWith({type: 'MARK_RECAPS_VIEWED'}); }); }); diff --git a/webapp/channels/src/components/recaps/recaps.tsx b/webapp/channels/src/components/recaps/recaps.tsx index 6683468493c..4c37e660846 100644 --- a/webapp/channels/src/components/recaps/recaps.tsx +++ b/webapp/channels/src/components/recaps/recaps.tsx @@ -9,7 +9,7 @@ import {Redirect} from 'react-router-dom'; import {PlusIcon} from '@mattermost/compass-icons/components'; import {getAgents} from 'mattermost-redux/actions/agents'; -import {getRecaps} from 'mattermost-redux/actions/recaps'; +import {getRecaps, markRecapsAsViewed} from 'mattermost-redux/actions/recaps'; import {getAllRecaps, getUnreadRecaps, getReadRecaps} from 'mattermost-redux/selectors/entities/recaps'; import {selectLhsItem} from 'actions/views/lhs'; @@ -45,8 +45,16 @@ const Recaps = () => { useEffect(() => { dispatch(selectLhsItem(LhsItemType.Page, LhsPage.Recaps)); const fetchData = async () => { - await dispatch(getRecaps(0, 60)); + const result = await dispatch(getRecaps(0, 60)); setIsLoading(false); + + // Only mark viewed when getRecaps succeeded. Marking after the + // fetch (rather than in parallel) also prevents getRecaps's + // response from overwriting the viewed_at timestamps the + // WS-driven refresh is about to set. + if (!result.error) { + dispatch(markRecapsAsViewed()); + } }; fetchData(); dispatch(getAgents()); diff --git a/webapp/channels/src/components/recaps/recaps_list.test.tsx b/webapp/channels/src/components/recaps/recaps_list.test.tsx index b66cd51e9e6..dad5a41c4cb 100644 --- a/webapp/channels/src/components/recaps/recaps_list.test.tsx +++ b/webapp/channels/src/components/recaps/recaps_list.test.tsx @@ -26,6 +26,7 @@ describe('RecapsList', () => { update_at: 1000, delete_at: 0, read_at: 0, + viewed_at: 0, channels: [], total_message_count: 5, }, @@ -39,6 +40,7 @@ describe('RecapsList', () => { update_at: 2000, delete_at: 0, read_at: 0, + viewed_at: 0, channels: [], total_message_count: 10, }, diff --git a/webapp/channels/src/components/recaps_link/recaps_link.scss b/webapp/channels/src/components/recaps_link/recaps_link.scss index b13bbb6017e..986bd5e5ffc 100644 --- a/webapp/channels/src/components/recaps_link/recaps_link.scss +++ b/webapp/channels/src/components/recaps_link/recaps_link.scss @@ -77,5 +77,26 @@ font-weight: 400; } } + + .SidebarLink.unread-title .icon { + color: var(--sidebar-text); + opacity: 1; + } + + .RecapsFailedIcon { + display: flex; + align-items: center; + color: var(--away-indicator); + } +} + +#SidebarContainer .SidebarRecaps .SidebarLink:hover { + padding-right: 16px; + + .badge, + .RecapsFailedIcon { + position: initial; + visibility: visible; + } } diff --git a/webapp/channels/src/components/recaps_link/recaps_link.test.tsx b/webapp/channels/src/components/recaps_link/recaps_link.test.tsx new file mode 100644 index 00000000000..88dd7740fff --- /dev/null +++ b/webapp/channels/src/components/recaps_link/recaps_link.test.tsx @@ -0,0 +1,84 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import React from 'react'; +import {MemoryRouter, Route} from 'react-router-dom'; + +import {renderWithContext, screen} from 'tests/react_testing_utils'; + +import RecapsLink from './recaps_link'; + +const mockUseFeatureFlag = jest.fn(() => 'true'); +jest.mock('components/common/hooks/useGetFeatureFlagValue', () => ({ + __esModule: true, + default: (...args: unknown[]) => mockUseFeatureFlag(...args as []), +})); + +const mockGetBadge = jest.fn(() => ({count: 0, hasFailed: false})); +jest.mock('mattermost-redux/selectors/entities/recaps', () => ({ + getUnreadFinishedRecapsBadge: () => mockGetBadge(), +})); + +const defaultState = { + entities: { + teams: { + currentTeamId: 'team1', + teams: {team1: {id: 'team1', name: 'team1'}}, + }, + users: { + currentUserId: 'user1', + profiles: {user1: {id: 'user1'}}, + }, + }, +}; + +function renderLink() { + return renderWithContext( + + + + + , + defaultState, + ); +} + +describe('components/recaps_link/RecapsLink', () => { + beforeEach(() => { + mockUseFeatureFlag.mockReturnValue('true'); + mockGetBadge.mockReturnValue({count: 0, hasFailed: false}); + }); + + test('renders nothing when the feature flag is disabled', () => { + mockUseFeatureFlag.mockReturnValue('false'); + mockGetBadge.mockReturnValue({count: 5, hasFailed: true}); + const {container} = renderLink(); + expect(container).toBeEmptyDOMElement(); + }); + + test('does not render a badge when there are no unread recaps', () => { + const {container} = renderLink(); + expect(screen.getByText('Recaps')).toBeInTheDocument(); + expect(container.querySelector('.badge')).not.toBeInTheDocument(); + expect(container.querySelector('.RecapsFailedIcon')).not.toBeInTheDocument(); + expect(container.querySelector('.SidebarChannel')).not.toHaveClass('unread'); + }); + + test('renders the count badge and marks the link unread when there are unread recaps', () => { + mockGetBadge.mockReturnValue({count: 3, hasFailed: false}); + const {container} = renderLink(); + expect(screen.getByText('3')).toBeInTheDocument(); + expect(container.querySelector('.SidebarChannel')).toHaveClass('unread'); + expect(container.querySelector('.SidebarLink')).toHaveClass('unread-title'); + expect(container.querySelector('.RecapsFailedIcon')).not.toBeInTheDocument(); + }); + + test('renders an alert icon instead of the count when a failed unread recap is present', () => { + mockGetBadge.mockReturnValue({count: 2, hasFailed: true}); + const {container} = renderLink(); + expect(container.querySelector('.RecapsFailedIcon')).toBeInTheDocument(); + expect(container.querySelector('.badge')).not.toBeInTheDocument(); + expect(screen.queryByText('2')).not.toBeInTheDocument(); + expect(container.querySelector('.SidebarChannel')).toHaveClass('unread'); + }); +}); diff --git a/webapp/channels/src/components/recaps_link/recaps_link.tsx b/webapp/channels/src/components/recaps_link/recaps_link.tsx index 75aa934a73b..3250b254aa0 100644 --- a/webapp/channels/src/components/recaps_link/recaps_link.tsx +++ b/webapp/channels/src/components/recaps_link/recaps_link.tsx @@ -3,24 +3,34 @@ import classNames from 'classnames'; import React from 'react'; -import {FormattedMessage} from 'react-intl'; -import {useSelector} from 'react-redux'; +import {defineMessage, FormattedMessage, useIntl} from 'react-intl'; +import {shallowEqual, useSelector} from 'react-redux'; import {Link, useLocation, matchPath, useRouteMatch} from 'react-router-dom'; -import {CreationOutlineIcon} from '@mattermost/compass-icons/components'; +import {AlertOutlineIcon, CreationOutlineIcon} from '@mattermost/compass-icons/components'; +import {getUnreadFinishedRecapsBadge} from 'mattermost-redux/selectors/entities/recaps'; import {getCurrentTeamId} from 'mattermost-redux/selectors/entities/teams'; import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; import useGetFeatureFlagValue from 'components/common/hooks/useGetFeatureFlagValue'; +import ChannelMentionBadge from 'components/sidebar/sidebar_channel/channel_mention_badge'; +import WithTooltip from 'components/with_tooltip'; import './recaps_link.scss'; +const failedTooltip = defineMessage({ + id: 'recaps.sidebarLink.failedTooltip', + defaultMessage: 'One or more recaps failed', +}); + const RecapsLink = () => { + const {formatMessage} = useIntl(); const {url} = useRouteMatch(); const {pathname} = useLocation(); const currentTeamId = useSelector(getCurrentTeamId); const currentUserId = useSelector(getCurrentUserId); + const {count: unreadCount, hasFailed} = useSelector(getUnreadFinishedRecapsBadge, shallowEqual); const enableAIRecaps = useGetFeatureFlagValue('EnableAIRecaps'); const inRecaps = matchPath(pathname, {path: '/:team/recaps/:recapId?'}) != null; @@ -29,12 +39,15 @@ const RecapsLink = () => { return null; } + const hasUnread = unreadCount > 0; + return ( @@ -63,4 +90,3 @@ const RecapsLink = () => { }; export default RecapsLink; - diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 0b719fcdae8..6720ce8f873 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -5968,6 +5968,7 @@ "recaps.processing.subtitle": "Recap created. You'll receive a summary shortly", "recaps.readTab": "Read", "recaps.sidebarLink": "Recaps", + "recaps.sidebarLink.failedTooltip": "One or more recaps failed", "recaps.status.failed": "Failed", "recaps.title": "Recaps", "recaps.unreadTab": "Unread", diff --git a/webapp/channels/src/packages/mattermost-redux/src/actions/recaps.ts b/webapp/channels/src/packages/mattermost-redux/src/actions/recaps.ts index a923f69c780..2f2d94440a2 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/actions/recaps.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/actions/recaps.ts @@ -46,6 +46,19 @@ export function markRecapAsRead(recapId: string): ActionFuncAsync { }); } +export function markRecapsAsViewed(): ActionFuncAsync<{recap_ids: string[]}> { + return async (dispatch, getState) => { + try { + const data = await Client4.markRecapsAsViewed(); + return {data}; + } catch (error) { + dispatch(logError(error)); + forceLogoutIfNecessary(error, dispatch, getState); + return {error}; + } + }; +} + export function regenerateRecap(recapId: string): ActionFuncAsync { return bindClientFunc({ clientFunc: () => Client4.regenerateRecap(recapId), diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.test.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.test.ts new file mode 100644 index 00000000000..2ad7a8e243e --- /dev/null +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.test.ts @@ -0,0 +1,81 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import type {Recap} from '@mattermost/types/recaps'; +import {RecapStatus} from '@mattermost/types/recaps'; +import type {GlobalState} from '@mattermost/types/store'; + +import {getUnreadFinishedRecapsBadge} from './recaps'; + +function makeRecap(id: string, status: RecapStatus, viewedAt: number): Recap { + return { + id, + user_id: 'user1', + title: `Recap ${id}`, + create_at: 1, + update_at: 1, + delete_at: 0, + read_at: 0, + viewed_at: viewedAt, + total_message_count: 0, + status, + bot_id: 'bot1', + } as Recap; +} + +function makeState(recaps: Recap[]): GlobalState { + const byId: Record = {}; + const allIds: string[] = []; + for (const r of recaps) { + byId[r.id] = r; + allIds.push(r.id); + } + return { + entities: { + recaps: {byId, allIds}, + }, + } as unknown as GlobalState; +} + +describe('selectors/entities/recaps', () => { + describe('getUnreadFinishedRecapsBadge', () => { + test('returns zero count when there are no recaps', () => { + const state = makeState([]); + expect(getUnreadFinishedRecapsBadge(state)).toEqual({count: 0, hasFailed: false}); + }); + + test('counts not-yet-viewed completed recaps', () => { + const state = makeState([ + makeRecap('a', RecapStatus.COMPLETED, 0), + makeRecap('b', RecapStatus.COMPLETED, 0), + makeRecap('c', RecapStatus.COMPLETED, 123), + ]); + expect(getUnreadFinishedRecapsBadge(state)).toEqual({count: 2, hasFailed: false}); + }); + + test('excludes pending and processing recaps from the count', () => { + const state = makeState([ + makeRecap('a', RecapStatus.PENDING, 0), + makeRecap('b', RecapStatus.PROCESSING, 0), + makeRecap('c', RecapStatus.COMPLETED, 0), + ]); + expect(getUnreadFinishedRecapsBadge(state)).toEqual({count: 1, hasFailed: false}); + }); + + test('includes not-yet-viewed failed recaps and flags hasFailed', () => { + const state = makeState([ + makeRecap('a', RecapStatus.COMPLETED, 0), + makeRecap('b', RecapStatus.FAILED, 0), + ]); + expect(getUnreadFinishedRecapsBadge(state)).toEqual({count: 2, hasFailed: true}); + }); + + test('does not set hasFailed when failed recap has been viewed', () => { + const state = makeState([ + makeRecap('a', RecapStatus.COMPLETED, 0), + makeRecap('b', RecapStatus.FAILED, 500), + ]); + expect(getUnreadFinishedRecapsBadge(state)).toEqual({count: 1, hasFailed: false}); + }); + }); +}); diff --git a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.ts b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.ts index a9f02b021f6..4965e07d43b 100644 --- a/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.ts +++ b/webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.ts @@ -65,3 +65,27 @@ export const getReadRecaps = createSelector( }, ); +const getRecapsSlice = (state: GlobalState) => state.entities.recaps; + +export const getUnreadFinishedRecapsBadge = createSelector( + 'getUnreadFinishedRecapsBadge', + getRecapsSlice, + ({byId, allIds}) => { + let count = 0; + let hasFailed = false; + for (const id of allIds) { + const recap = byId[id]; + if (!recap || recap.viewed_at !== 0) { + continue; + } + if (recap.status === RecapStatus.COMPLETED) { + count++; + } else if (recap.status === RecapStatus.FAILED) { + count++; + hasFailed = true; + } + } + return {count, hasFailed}; + }, +); + diff --git a/webapp/platform/client/src/client4.ts b/webapp/platform/client/src/client4.ts index 1b08ac035ec..65aaa8f02a0 100644 --- a/webapp/platform/client/src/client4.ts +++ b/webapp/platform/client/src/client4.ts @@ -3475,6 +3475,13 @@ export default class Client4 { ); }; + markRecapsAsViewed = () => { + return this.doFetch<{recap_ids: string[]}>( + `${this.getRecapsRoute()}/mark_viewed`, + {method: 'post'}, + ); + }; + regenerateRecap = (recapId: string) => { return this.doFetch( `${this.getRecapsRoute()}/${recapId}/regenerate`, diff --git a/webapp/platform/types/src/recaps.ts b/webapp/platform/types/src/recaps.ts index 9a3da4ccc98..4a162a4b06e 100644 --- a/webapp/platform/types/src/recaps.ts +++ b/webapp/platform/types/src/recaps.ts @@ -9,6 +9,7 @@ export type Recap = { update_at: number; delete_at: number; read_at: number; + viewed_at: number; total_message_count: number; status: RecapStatus; bot_id: string;