From ecf8a741ac1f26b77071606026274f967355ddb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Guillermo=20Vay=C3=A1?= Date: Wed, 6 May 2026 15:14:18 +0200 Subject: [PATCH] Add unread badge to Recaps sidebar link (#36246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add unread badge to Recaps sidebar link Shows the count of unread finished recaps (completed or failed) on the LHS Recaps link. Pending and processing recaps are excluded so the badge only reflects work the user can actually read. When any unread recap has failed, the badge is colored as an error to surface the failure. The badge updates live through the existing recap_updated WebSocket event, which refreshes the recap in the Redux store. Co-Authored-By: Claude Opus 4.7 (1M context) * Fix Recaps failed-badge color losing to active sidebar rule The failed-badge modifier selector had the same specificity (0,4,0) as `.channel-view .sidebar--left .active .badge` in _badge.scss, so when the Recaps link was the active route the global mention background color won on cascade order. Scope the rule with `#SidebarContainer` so it wins on specificity (1 id + 4 classes) regardless of active state. Co-Authored-By: Claude Opus 4.7 (1M context) * Fix Recaps badge selector memoization getUnreadFinishedRecapsBadge was keyed off getAllRecaps, which is not memoized and returns a new array on every call. That broke reselect's reference-equality input check, so the selector recomputed and returned a fresh {count, hasFailed} object on every store dispatch — forcing RecapsLink (always mounted when the feature flag is on) to re-render on every action. Key the selector off state.entities.recaps directly and iterate ids in the result function so memoization holds when the recaps slice is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) * Address PR feedback on Recaps sidebar badge - Pass shallowEqual to the useSelector consuming getUnreadFinishedRecapsBadge. The selector returns a plain {count, hasFailed} object, so recap updates that change a recap but leave the badge values the same (e.g. marking a read recap) would otherwise force RecapsLink to re-render. - Scope the "no badge" negative assertion to the render container so it only asserts on the badge element, not any '1' or '.badge' elsewhere in the DOM. Co-Authored-By: Claude Opus 4.7 (1M context) * Address UX feedback on Recaps sidebar badge - Add `unread` class to the sidebar item and `unread-title` to the link when there are unread recaps so the label goes bold and the icon goes full-opacity, matching how channels and the threads link indicate unread state. - Keep the badge (and the new failed icon) visible on hover so it doesn't disappear under the cursor -- same override the threads link uses. - Replace the red failed-badge modifier with an amber alert icon rendered in place of the count badge when any unread recap has failed. Red mention badges are reserved for urgent priority messages and caused confusion here. Co-Authored-By: Claude Opus 4.7 (1M context) * Keep Recaps badge in place on hover The global sidebar hover rule shrinks padding-right from 16px to 5px to make room for the per-channel menu button, which shifted the badge right since it stays visible. Restore padding-right: 16px on hover for the Recaps link, matching what the threads link already does. Co-Authored-By: Claude Opus 4.7 (1M context) * Align Recaps failed-icon aria-label with tooltip The aria-label on the .RecapsFailedIcon span was a hardcoded English string ("Recap failed") that differed from the tooltip shown to sighted users ("One or more recaps failed"). Derive the aria-label from the same intl message used by the tooltip so screen readers and sighted users get the same wording and the label is localized. Co-Authored-By: Claude Opus 4.7 (1M context) * Stop Recaps link from overriding global unread label styling The combined `.active .SidebarLink, .SidebarLink.unread-title` rule pushed font-weight: 400 onto .SidebarChannelLinkLabel with specificity (0,4,0), overriding the global `.SidebarChannel.unread` rule that sets font-weight: 600 and --sidebar-unread-text at (0,3,0). As a result the Recaps label rendered at normal weight when unread, inconsistent with channels and the threads link. Split the rules: keep the active-state overrides as they were, and limit the unread-title rule to the icon-specific styling Recaps actually needs, letting the global unread styling apply to the label. Co-Authored-By: Claude Opus 4.7 (1M context) * Add i18n entry for Recaps failed-tooltip Co-Authored-By: Claude Opus 4.7 (1M context) * change size of alert icon * fix the right icon * Add ViewedAt to recaps and POST /recaps/mark_viewed endpoint Introduce a new ViewedAt field on Recap, separate from ReadAt, that tracks whether the user has at least seen a finished recap on the recaps page. ReadAt keeps its existing per-recap "Mark read" semantics. - New Postgres migration 000172 adds the ViewedAt column (default 0) and an idx_recaps_user_id_viewed_at index mirroring the existing ReadAt index. - New store method MarkRecapsAsViewed(userId, statuses) does a single UPDATE ... WHERE ViewedAt = 0 AND Status IN (...) RETURNING Id so the app layer can fan out one WS event per affected recap. - New App.MarkRecapsAsViewed(rctx) marks the user's not-yet-viewed completed/failed recaps and broadcasts WebsocketEventRecapUpdated per affected id. - New POST /recaps/mark_viewed handler. Registered before the {recap_id} regex routes so mark_viewed isn't captured as an id. - RegenerateRecap now resets ViewedAt = 0 so a regenerated recap is surfaced again in the badge once it completes. As a related fix, UpdateRecap now persists ReadAt and ViewedAt -- previously it silently dropped the ReadAt = 0 reset that RegenerateRecap was setting in memory. Co-Authored-By: Claude Opus 4.7 (1M context) * Mark recaps as viewed when the recaps page mounts Wire the new server endpoint into the webapp: - Recap type now includes viewed_at: number. - Client4.markRecapsAsViewed posts to /recaps/mark_viewed. - New markRecapsAsViewed redux action, fired alongside getRecaps and getAgents in the recaps page mount effect. The server broadcasts recap_updated per affected recap so other tabs/devices receive the update through the existing handleRecapUpdated WS handler -- no new client-side handler needed. - getUnreadFinishedRecapsBadge now filters on viewed_at === 0 instead of read_at === 0, so the sidebar badge clears on page open instead of requiring per-recap "Mark read" clicks. Selector tests updated to match. Co-Authored-By: Claude Opus 4.7 (1M context) * Address review feedback on Recaps viewed_at change - Defer markRecapsAsViewed until after getRecaps resolves on the recaps page mount. Previously they ran in parallel, so getRecaps could land last and overwrite the viewed_at: timestamps the WS-driven refresh had just written, briefly re-showing the badge. - Switch the markRecapsAsViewed audit log to LevelContent and record the affected ids as result state, matching the pattern of every other mutating recap handler (markRecapAsRead, deleteRecap, etc). recap_count meta is now recorded unconditionally. - Add an app-layer test that asserts MarkRecapsAsViewed publishes a recap_updated websocket event for each affected recap. The fan-out is the entire reason this lives in the app layer, so a regression removing the publish loop should fail loudly. - Add a store-layer regression test that UpdateRecap actually persists ReadAt = 0 / ViewedAt = 0 resets, guarding the regenerate flow against a future change that drops those columns from the update map. Co-Authored-By: Claude Opus 4.7 (1M context) * Update migrations.list for 000172_add_recaps_viewed_at Regenerated via `make migrations-extract` so the autogenerated sequence list includes the new recaps ViewedAt migration files. Co-Authored-By: Claude Opus 4.7 (1M context) * Use AddMeta for Recaps mark_viewed audit ids AddEventResultState takes a model.Auditable, not a plain map[string]any, so the previous attempt to record the affected ids did not compile. Record them as audit metadata instead, matching the pattern used by getRecaps which similarly returns a slice and uses AddMeta only. Co-Authored-By: Claude Opus 4.7 (1M context) * Split Recaps ViewedAt index into a CONCURRENTLY migration The lint check rejects bare CREATE/DROP INDEX in migrations because they take an ACCESS EXCLUSIVE lock and block DML. Split the index off into 000173 with CONCURRENTLY + the morph:nontransactional directive, matching the pattern used by 000168/000169 (LinkedFieldID column + its index). 000172 keeps just the ALTER TABLE ADD COLUMN, which can stay transactional. Co-Authored-By: Claude Opus 4.7 (1M context) * Add viewed_at to existing Recap test fixtures The Recap type now requires viewed_at, so the fixtures in recap_item.test.tsx, recap_processing.test.tsx, and recaps_list.test.tsx need it too. CI was rejecting them with TS2741. Co-Authored-By: Claude Opus 4.7 (1M context) * Mock markRecapsAsViewed in recaps.test.tsx The mount effect now also dispatches markRecapsAsViewed, but the manual jest.mock for 'mattermost-redux/actions/recaps' only exposed getRecaps, so the runtime call resolved to undefined and crashed with "markRecapsAsViewed is not a function". Add the missing entry to the mock. Co-Authored-By: Claude Opus 4.7 (1M context) * Add /recaps/mark_viewed and Recap.viewed_at to OpenAPI spec The recap-spec validator rejected the new POST /api/v4/recaps/mark_viewed handler because it had no documented operation. Add the path with its MarkRecapsAsViewed operationId, response shape, and behavior, and add the new viewed_at timestamp field to the Recap schema in definitions. Co-Authored-By: Claude Opus 4.7 (1M context) * Fill in app.recap.mark_viewed.app_error translation The new MarkRecapsAsViewed app method references this i18n key but the en.json entry was added with an empty translation. Co-Authored-By: Claude Opus 4.7 (1M context) * Skip markRecapsAsViewed when getRecaps fails Marking recaps as viewed implies the user just looked at them. If getRecaps fails the user is staring at an error/empty state, so we shouldn't ack them on the server. Gate the dispatch on the thunk's result.error -- the codebase's bindClientFunc swallows errors and returns {error}, so the conventional try/catch pattern doesn't apply here. Update the recaps.test.tsx dispatch mock to return a resolved promise so the new awaited result has the expected shape. Co-Authored-By: Claude Opus 4.7 (1M context) * Clear and assert markRecapsAsViewed mock in recaps.test.tsx Reset the new mock in beforeEach so it doesn't carry state across tests, and assert that the mount effect dispatches markRecapsAsViewed after getRecaps resolves. Awaiting via waitFor since the mark fires inside an async fetchData chain. Co-Authored-By: Claude Opus 4.7 (1M context) --------- Co-authored-by: Claude Opus 4.7 (1M context) --- api/v4/source/definitions.yaml | 4 + api/v4/source/recaps.yaml | 40 ++++++++ server/channels/api4/recap.go | 26 +++++ server/channels/app/recap.go | 25 ++++- server/channels/app/recap_test.go | 81 ++++++++++++++++ server/channels/db/migrations/migrations.list | 4 + .../000172_add_recaps_viewed_at.down.sql | 1 + .../000172_add_recaps_viewed_at.up.sql | 1 + ...te_recaps_user_id_viewed_at_index.down.sql | 2 + ...eate_recaps_user_id_viewed_at_index.up.sql | 2 + .../channels/store/retrylayer/retrylayer.go | 21 ++++ server/channels/store/sqlstore/recap_store.go | 32 +++++++ .../store/sqlstore/recap_store_test.go | 95 +++++++++++++++++++ server/channels/store/store.go | 1 + .../store/storetest/mocks/RecapStore.go | 30 ++++++ .../channels/store/timerlayer/timerlayer.go | 16 ++++ server/i18n/en.json | 4 + server/public/model/audit_events.go | 13 +-- server/public/model/recap.go | 2 + .../src/components/recaps/recap_item.test.tsx | 1 + .../recaps/recap_processing.test.tsx | 1 + .../src/components/recaps/recaps.test.tsx | 13 ++- .../channels/src/components/recaps/recaps.tsx | 12 ++- .../components/recaps/recaps_list.test.tsx | 2 + .../components/recaps_link/recaps_link.scss | 21 ++++ .../recaps_link/recaps_link.test.tsx | 84 ++++++++++++++++ .../components/recaps_link/recaps_link.tsx | 36 ++++++- webapp/channels/src/i18n/en.json | 1 + .../mattermost-redux/src/actions/recaps.ts | 13 +++ .../src/selectors/entities/recaps.test.ts | 81 ++++++++++++++++ .../src/selectors/entities/recaps.ts | 24 +++++ webapp/platform/client/src/client4.ts | 7 ++ webapp/platform/types/src/recaps.ts | 1 + 33 files changed, 680 insertions(+), 17 deletions(-) create mode 100644 server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.down.sql create mode 100644 server/channels/db/migrations/postgres/000172_add_recaps_viewed_at.up.sql create mode 100644 server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.down.sql create mode 100644 server/channels/db/migrations/postgres/000173_create_recaps_user_id_viewed_at_index.up.sql create mode 100644 webapp/channels/src/components/recaps_link/recaps_link.test.tsx create mode 100644 webapp/channels/src/packages/mattermost-redux/src/selectors/entities/recaps.test.ts 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 (
  • @@ -42,7 +55,9 @@ const RecapsLink = () => { to={`${url}/recaps`} id='sidebarItem_recaps' draggable='false' - className='SidebarLink sidebar-item' + className={classNames('SidebarLink sidebar-item', { + 'unread-title': hasUnread, + })} tabIndex={0} > @@ -56,6 +71,18 @@ const RecapsLink = () => { /> + {hasUnread && (hasFailed ? ( + + + + + + ) : ( + + ))}
@@ -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;