diff --git a/server/channels/app/scheduled_post_job.go b/server/channels/app/scheduled_post_job.go index 0ab2e80ad0..ebf4dbc441 100644 --- a/server/channels/app/scheduled_post_job.go +++ b/server/channels/app/scheduled_post_job.go @@ -6,6 +6,7 @@ package app import ( "fmt" "net/http" + "strings" "time" "github.com/mattermost/mattermost/server/v8/platform/services/telemetry" @@ -418,13 +419,60 @@ func (a *App) notifyUser(rctx request.CTX, userId string, userFailedMessages []* } T := i18n.GetUserTranslations(user.Locale) - messageContent := T("app.scheduled_post.failed_messages", map[string]interface{}{ - "Count": len(userFailedMessages), + + type channelErrorKey struct { + ChannelId string + ErrorCode string + } + + channelErrorCounts := make(map[channelErrorKey]int) + channelIdsSet := make(map[string]struct{}) + for _, msg := range userFailedMessages { + key := channelErrorKey{ChannelId: msg.ChannelId, ErrorCode: msg.ErrorCode} + channelErrorCounts[key]++ + channelIdsSet[msg.ChannelId] = struct{}{} + } + + channelNames := make(map[string]string) + for channelId := range channelIdsSet { + ch, err := a.GetChannel(rctx, channelId) + if err != nil { + rctx.Logger().Error("Failed to get channel", mlog.String("channel_id", channelId), mlog.Err(err)) + channelNames[channelId] = T("app.scheduled_post.unknown_channel") + continue + } + if ch.Type != model.ChannelTypePrivate { + channelNames[channelId] = ch.DisplayName + } else { + channelNames[channelId] = T("app.scheduled_post.private_channel") + } + } + + var messageBuilder strings.Builder + + totalFailedMessages := len(userFailedMessages) + messageHeader := T("app.scheduled_post.failed_messages", map[string]interface{}{ + "Count": totalFailedMessages, }) + messageBuilder.WriteString(messageHeader) + messageBuilder.WriteString("\n") + + for key, count := range channelErrorCounts { + channelName := channelNames[key.ChannelId] + errorReason := getErrorReason(T, key.ErrorCode) + + detailedMessage := T("app.scheduled_post.failed_message_detail", map[string]interface{}{ + "Count": count, + "ChannelName": channelName, + "ErrorReason": errorReason, + }) + messageBuilder.WriteString(detailedMessage) + messageBuilder.WriteString("\n") + } post := &model.Post{ ChannelId: channel.Id, - Message: messageContent, + Message: messageBuilder.String(), Type: model.PostTypeDefault, UserId: systemBot.UserId, } @@ -433,3 +481,32 @@ func (a *App) notifyUser(rctx request.CTX, userId string, userFailedMessages []* rctx.Logger().Error("Failed to post notification about failed scheduled messages", mlog.Err(err)) } } + +func getErrorReason(T i18n.TranslateFunc, errorCode string) string { + var reason string + switch errorCode { + case "unknown": + reason = T("app.scheduled_post.error_reason.unknown") + case "channel_archived": + reason = T("app.scheduled_post.error_reason.channel_archived") + case "channel_not_found": + reason = T("app.scheduled_post.error_reason.channel_not_found") + case "user_missing": + reason = T("app.scheduled_post.error_reason.user_missing") + case "user_deleted": + reason = T("app.scheduled_post.error_reason.user_deleted") + case "no_channel_permission": + reason = T("app.scheduled_post.error_reason.no_channel_permission") + case "no_channel_member": + reason = T("app.scheduled_post.error_reason.no_channel_member") + case "thread_deleted": + reason = T("app.scheduled_post.error_reason.thread_deleted") + case "unable_to_send": + reason = T("app.scheduled_post.error_reason.unable_to_send") + case "invalid_post": + reason = T("app.scheduled_post.error_reason.invalid_post") + default: + reason = errorCode + } + return reason +} diff --git a/server/channels/app/scheduled_post_job_test.go b/server/channels/app/scheduled_post_job_test.go index 22a36f0798..b9cb403608 100644 --- a/server/channels/app/scheduled_post_job_test.go +++ b/server/channels/app/scheduled_post_job_test.go @@ -4,6 +4,7 @@ package app import ( + "strings" "testing" "time" @@ -281,6 +282,9 @@ func TestHandleFailedScheduledPosts(t *testing.T) { user1 := th.BasicUser user2 := th.BasicUser2 + channel1 := th.BasicChannel + channel2 := th.CreateChannel(th.Context, th.BasicTeam) + // Create failed scheduled posts: 1 for user1 and 2 for user2 failedScheduledPosts := []*model.ScheduledPost{ { @@ -288,7 +292,7 @@ func TestHandleFailedScheduledPosts(t *testing.T) { Draft: model.Draft{ CreateAt: model.GetMillis(), UserId: user1.Id, - ChannelId: th.BasicChannel.Id, + ChannelId: channel1.Id, Message: "Failed scheduled post for user 1", }, ErrorCode: model.ScheduledPostErrorUnknownError, @@ -298,7 +302,7 @@ func TestHandleFailedScheduledPosts(t *testing.T) { Draft: model.Draft{ CreateAt: model.GetMillis(), UserId: user2.Id, - ChannelId: th.BasicChannel.Id, + ChannelId: channel1.Id, Message: "Failed scheduled post 1 for user 2", }, ErrorCode: model.ScheduledPostErrorCodeNoChannelPermission, @@ -308,7 +312,7 @@ func TestHandleFailedScheduledPosts(t *testing.T) { Draft: model.Draft{ CreateAt: model.GetMillis(), UserId: user2.Id, - ChannelId: th.BasicChannel.Id, + ChannelId: channel2.Id, Message: "Failed scheduled post 2 for user 2", }, ErrorCode: model.ScheduledPostErrorNoChannelMember, @@ -342,22 +346,22 @@ func TestHandleFailedScheduledPosts(t *testing.T) { if received.GetBroadcast().UserId == user2.Id { assert.Equal(t, model.WebsocketScheduledPostUpdated, received.EventType()) } - case <-time.After(1 * time.Second): + case <-time.After(3 * time.Second): t.Errorf("Timeout while waiting for a WebSocket event for scheduled post %d", i+1) } } // Helper function to check notifications for a specific user - checkUserNotification := func(user *model.User, expectedCount int) { - // Wait time for notifications to be sent (adding 2 secs because it is run in a separate rountine) - var timeout = 2 * time.Second + checkUserNotification := func(user *model.User) { + // Wait time for notifications to be sent (adding 5 secs because it is run in a separate goroutine) + var timeout = 5 * time.Second begin := time.Now() channel, appErr := th.App.GetOrCreateDirectChannel(rctx, user.Id, systemBot.UserId) assert.True(t, appErr == nil) var posts *model.PostList // wait for the notification to be sent into the channel. - // idea is to get the channel and try to find posts, if not, wait 100ms and try again until timout or there is posts lengh + // idea is to get the channel and try to find posts, if not, wait 100ms and try again until timeout or there is posts length for { if time.Since(begin) > timeout { break @@ -371,24 +375,85 @@ func TestHandleFailedScheduledPosts(t *testing.T) { } assert.NotEmpty(t, posts.Posts, "Expected notification for user %s to have been sent", user.Id) - // Validate the actual content of the notification posted (to include count verification) - T := i18n.GetUserTranslations(user.Locale) - messageContent := T("app.scheduled_post.failed_messages", map[string]interface{}{ - "Count": expectedCount, - }) + // Collect failed messages for users + var userFailedMessages []*model.ScheduledPost + for _, sp := range failedScheduledPosts { + if sp.UserId == user.Id { + userFailedMessages = append(userFailedMessages, sp) + } + } + T := i18n.GetUserTranslations(user.Locale) + + // Aggregate failed messages by channel and error code + type channelErrorKey struct { + ChannelId string + ErrorCode string + } + + channelErrorCounts := make(map[channelErrorKey]int) + channelIdsSet := make(map[string]struct{}) + for _, msg := range userFailedMessages { + key := channelErrorKey{ChannelId: msg.ChannelId, ErrorCode: msg.ErrorCode} + channelErrorCounts[key]++ + channelIdsSet[msg.ChannelId] = struct{}{} + } + + // Get the channel names + channelNames := make(map[string]string) + for channelId := range channelIdsSet { + ch, err := th.App.GetChannel(rctx, channelId) + assert.Nil(t, err) + channelNames[channelId] = ch.DisplayName + } + + // Helper function to get error reason + getErrorReason := func(T i18n.TranslateFunc, errorCode string) string { + key := "app.scheduled_post.error_reason." + errorCode + reason := T(key) + if reason == key { + return errorCode + } + return reason + } + + // Build the expected message content + var messageBuilder strings.Builder + + messageHeader := T("app.scheduled_post.failed_messages", map[string]interface{}{ + "Count": len(userFailedMessages), + }) + messageBuilder.WriteString(messageHeader) + messageBuilder.WriteString("\n") + + for key, count := range channelErrorCounts { + channelName := channelNames[key.ChannelId] + errorReason := getErrorReason(T, key.ErrorCode) + + detailMessage := T("app.scheduled_post.failed_message_detail", map[string]interface{}{ + "Count": count, + "ChannelName": channelName, + "ErrorReason": errorReason, + }) + messageBuilder.WriteString(detailMessage) + messageBuilder.WriteString("\n") + } + + expectedMessageContent := messageBuilder.String() + + // Validate the actual content of the notification posted found := false for _, post := range posts.Posts { - if post.UserId == systemBot.UserId && post.Message == messageContent { + if post.UserId == systemBot.UserId && post.Message == expectedMessageContent { found = true break } } - assert.True(t, found, "Notification post not found for user %s with expected count %d", user.Id, expectedCount) + assert.True(t, found, "\nNotification post not found for user %s with expected message. \n Expected: %s \n", user.Id, expectedMessageContent) } // Check notifications sent for failed messages for both users - checkUserNotification(user1, 1) - checkUserNotification(user2, 2) + checkUserNotification(user1) + checkUserNotification(user2) }) } diff --git a/server/i18n/en.json b/server/i18n/en.json index e490429c07..67d8c0c08c 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -6546,6 +6546,53 @@ "id": "app.save_scheduled_post.save.app_error", "translation": "Error occurred saving the scheduled post." }, + { + "id": "app.scheduled_post.error_reason.channel_archived", + "translation": "Channel is archived" + }, + { + "id": "app.scheduled_post.error_reason.channel_not_found", + "translation": "Channel not found" + }, + { + "id": "app.scheduled_post.error_reason.invalid_post", + "translation": "Invalid post content" + }, + { + "id": "app.scheduled_post.error_reason.no_channel_member", + "translation": "Not a member of the channel" + }, + { + "id": "app.scheduled_post.error_reason.no_channel_permission", + "translation": "No permission to post in channel" + }, + { + "id": "app.scheduled_post.error_reason.thread_deleted", + "translation": "Thread has been deleted" + }, + { + "id": "app.scheduled_post.error_reason.unable_to_send", + "translation": "Unable to send the message" + }, + { + "id": "app.scheduled_post.error_reason.unknown", + "translation": "Unknown Error" + }, + { + "id": "app.scheduled_post.error_reason.user_deleted", + "translation": "User account is deleted" + }, + { + "id": "app.scheduled_post.error_reason.user_missing", + "translation": "User does not exist" + }, + { + "id": "app.scheduled_post.failed_message_detail", + "translation": { + "one": "- {{.Count}} in channel {{.ChannelName}}. Reason: {{.ErrorReason}}", + "other": "- {{.Count}} in channel {{.ChannelName}}. Reason: {{.ErrorReason}}" + } + }, { "id": "app.scheduled_post.failed_messages", "translation": { @@ -6557,6 +6604,14 @@ "id": "app.scheduled_post.permanent_delete_by_user.app_error", "translation": "Unable to delete scheduled posts for user." }, + { + "id": "app.scheduled_post.private_channel", + "translation": "Private channel" + }, + { + "id": "app.scheduled_post.unknown_channel", + "translation": "Unknown Channel" + }, { "id": "app.scheme.delete.app_error", "translation": "Unable to delete this scheme."