From 58542577391477f9cf169a4a84fee4c3847b9fce Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Sat, 18 Apr 2026 01:12:09 +0530 Subject: [PATCH] Manual CP of 35755 for v10.11 (#36135) Automatic Merge --- server/channels/app/channel.go | 12 +- .../app/post_persistent_notification.go | 27 +++- .../app/post_persistent_notification_test.go | 128 ++++++++++++++++++ 3 files changed, 160 insertions(+), 7 deletions(-) diff --git a/server/channels/app/channel.go b/server/channels/app/channel.go index 3e4a673aac..4d72f81b8b 100644 --- a/server/channels/app/channel.go +++ b/server/channels/app/channel.go @@ -1547,6 +1547,12 @@ func (a *App) DeleteChannel(c request.CTX, channel *model.Channel, userID string return err } + deleteAt := model.GetMillis() + + if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { + return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + if user != nil { T := i18n.GetUserTranslations(user.Locale) @@ -1602,12 +1608,6 @@ func (a *App) DeleteChannel(c request.CTX, channel *model.Channel, userID string return model.NewAppError("DeleteChannel", "app.post_persistent_notification.delete_by_channel.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - deleteAt := model.GetMillis() - - if err := a.Srv().Store().Channel().Delete(channel.Id, deleteAt); err != nil { - return model.NewAppError("DeleteChannel", "app.channel.delete.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - a.Srv().Platform().InvalidateCacheForChannel(channel) var message *model.WebSocketEvent diff --git a/server/channels/app/post_persistent_notification.go b/server/channels/app/post_persistent_notification.go index a784cc790c..7174c06d5e 100644 --- a/server/channels/app/post_persistent_notification.go +++ b/server/channels/app/post_persistent_notification.go @@ -162,13 +162,25 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos return err } + var postsForPersistentNotificationCleanup []*model.Post + for _, post := range posts { channel := channelsMap[post.ChannelId] + if channel == nil { + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue + } + team := teamsMap[channel.TeamId] // GMs and DMs don't belong to any team if channel.IsGroupOrDirect() { team = &model.Team{} + } else if team == nil { + // cleanup persistent notification for posts with missing teams when they are not DM or GM + postsForPersistentNotificationCleanup = append(postsForPersistentNotificationCleanup, post) + continue } + profileMap := channelProfileMap[channel.Id] // Ensure the sender is always in the profile map: for example, system admins can post @@ -209,6 +221,14 @@ func (a *App) forEachPersistentNotificationPost(posts []*model.Post, fn func(pos } } + if len(postsForPersistentNotificationCleanup) > 0 { + for _, post := range postsForPersistentNotificationCleanup { + if appErr := a.DeletePersistentNotification(request.EmptyContext(a.Log()), post); appErr != nil { + a.Log().Warn("Failed to delete persistent notification for post", mlog.String("post_id", post.Id), mlog.String("channel_id", post.ChannelId), mlog.Err(appErr)) + } + } + } + return nil } @@ -218,9 +238,14 @@ func (a *App) persistentNotificationsAuxiliaryData(channelsMap map[string]*model channelKeywords := make(map[string]MentionKeywords, len(channelsMap)) channelNotifyProps := make(map[string]map[string]model.StringMap, len(channelsMap)) for _, c := range channelsMap { + team := teamsMap[c.TeamId] + if team == nil && !c.IsGroupOrDirect() { + continue + } + // In DM, notifications can't be send to any 3rd person. if c.Type != model.ChannelTypeDirect { - groups, err := a.getGroupsAllowedForReferenceInChannel(c, teamsMap[c.TeamId]) + groups, err := a.getGroupsAllowedForReferenceInChannel(c, team) if err != nil { return nil, nil, nil, nil, errors.Wrapf(err, "failed to get profiles for channel %s", c.Id) } diff --git a/server/channels/app/post_persistent_notification_test.go b/server/channels/app/post_persistent_notification_test.go index 996f9423a5..9d07307ad2 100644 --- a/server/channels/app/post_persistent_notification_test.go +++ b/server/channels/app/post_persistent_notification_test.go @@ -198,6 +198,132 @@ func TestDeletePersistentNotification(t *testing.T) { }) } +func TestForEachPersistentNotificationPost(t *testing.T) { + mainHelper.Parallel(t) + + th := SetupWithStoreMock(t) + defer th.TearDown() + + // Common data + user1 := &model.User{Id: "uid1", Username: "user-1"} + user2 := &model.User{Id: "uid2", Username: "user-2"} + user3 := &model.User{Id: "uid3", Username: "user-3"} + team := &model.Team{Id: "tid"} + channel := &model.Channel{Id: "chid", TeamId: team.Id, Type: model.ChannelTypeOpen} + + th.App.Srv().SetLicense(getLicWithSkuShortName(model.LicenseShortSkuProfessional)) + cfg := th.App.Config() + *cfg.ServiceSettings.PostPriority = true + *cfg.ServiceSettings.AllowPersistentNotifications = true + + // setupMocks creates a fresh mock store with the given data and sets it on the server. + // Returns the PostPersistentNotificationStore mock for test-specific expectations. + setupMocks := func(channels []*model.Channel, teams []*model.Team, profileMap map[string]*model.User) *storemocks.PostPersistentNotificationStore { + localStore := storemocks.Store{} + localStore.On("Close").Return(nil) + + mockChannel := storemocks.ChannelStore{} + localStore.On("Channel").Return(&mockChannel) + mockChannel.On("GetChannelsByIds", mock.Anything, mock.Anything).Return(channels, nil) + mockChannel.On("GetAllChannelMembersNotifyPropsForChannel", mock.Anything, mock.Anything).Return(map[string]model.StringMap{}, nil) + + mockTeam := storemocks.TeamStore{} + localStore.On("Team").Return(&mockTeam) + mockTeam.On("GetMany", mock.Anything).Return(teams, nil) + + mockUser := storemocks.UserStore{} + localStore.On("User").Return(&mockUser) + mockUser.On("GetAllProfilesInChannel", mock.Anything, mock.Anything, mock.Anything).Return(profileMap, nil) + + mockGroup := storemocks.GroupStore{} + localStore.On("Group").Return(&mockGroup) + mockGroup.On("GetGroups", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return([]*model.Group{}, nil) + + mockPPN := storemocks.PostPersistentNotificationStore{} + localStore.On("PostPersistentNotification").Return(&mockPPN) + + th.App.Srv().SetStore(&localStore) + return &mockPPN + } + + t.Run("should cleanup posts whose channel no longer exists", func(t *testing.T) { + profileMap := map[string]*model.User{user1.Id: user1} + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user1.Id} + post2 := &model.Post{Id: "pid2", ChannelId: "deleted-channel-id", Message: "hello", UserId: user1.Id} + + mockPPN := setupMocks([]*model.Channel{channel}, []*model.Team{team}, profileMap) + mockPPN.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPPN.On("Delete", []string{post2.Id}).Return(nil) + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + assert.Equal(t, []string{"pid1"}, fnCalled) + mockPPN.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should cleanup posts whose team no longer exists", func(t *testing.T) { + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + channelWithMissingTeam := &model.Channel{Id: "chid2", TeamId: "deleted-team-id", Type: model.ChannelTypeOpen} + post1 := &model.Post{Id: "pid1", ChannelId: channel.Id, Message: "hello @user-1", UserId: user2.Id} + post2 := &model.Post{Id: "pid2", ChannelId: channelWithMissingTeam.Id, Message: "hello @user-1", UserId: user2.Id} + + mockPPN := setupMocks([]*model.Channel{channel, channelWithMissingTeam}, []*model.Team{team}, profileMap) + mockPPN.On("GetSingle", post2.Id).Return(&model.PostPersistentNotifications{PostId: post2.Id}, nil) + mockPPN.On("Delete", []string{post2.Id}).Return(nil) + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1, post2}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + assert.Equal(t, []string{"pid1"}, fnCalled) + mockPPN.AssertCalled(t, "Delete", []string{post2.Id}) + }) + + t.Run("should not cleanup DM posts that have no team", func(t *testing.T) { + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2} + dmChannel := &model.Channel{Id: "dm-chid", TeamId: "", Type: model.ChannelTypeDirect, Name: model.GetDMNameFromIds(user1.Id, user2.Id)} + post1 := &model.Post{Id: "pid1", ChannelId: dmChannel.Id, Message: "hello", UserId: user1.Id} + + mockPPN := setupMocks([]*model.Channel{dmChannel}, []*model.Team{}, profileMap) + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + assert.Equal(t, []string{"pid1"}, fnCalled) + mockPPN.AssertNotCalled(t, "Delete", mock.Anything) + }) + + t.Run("should not cleanup GM posts that have no team", func(t *testing.T) { + profileMap := map[string]*model.User{user1.Id: user1, user2.Id: user2, user3.Id: user3} + gmChannel := &model.Channel{Id: "gm-chid", TeamId: "", Type: model.ChannelTypeGroup} + post1 := &model.Post{Id: "pid1", ChannelId: gmChannel.Id, Message: "hello @user-2", UserId: user1.Id} + + mockPPN := setupMocks([]*model.Channel{gmChannel}, []*model.Team{}, profileMap) + + fnCalled := []string{} + err := th.App.forEachPersistentNotificationPost([]*model.Post{post1}, func(post *model.Post, _ *model.Channel, _ *model.Team, _ *MentionResults, _ model.UserMap, _ map[string]map[string]model.StringMap) error { + fnCalled = append(fnCalled, post.Id) + return nil + }) + require.NoError(t, err) + + assert.Equal(t, []string{"pid1"}, fnCalled) + mockPPN.AssertNotCalled(t, "Delete", mock.Anything) + }) +} + func TestSendPersistentNotifications(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic() @@ -230,6 +356,7 @@ func TestSendPersistentNotificationsBotSender(t *testing.T) { mainHelper.Parallel(t) t.Run("should send notification when bot is sender", func(t *testing.T) { th := Setup(t).InitBasic() + defer th.TearDown() bot, appErr := th.App.CreateBot(th.Context, &model.Bot{ Username: "testbot", @@ -282,6 +409,7 @@ func TestSendPersistentNotificationsBotSenderNotInChannel(t *testing.T) { mainHelper.Parallel(t) t.Run("should send notification when bot sender is not a channel member", func(t *testing.T) { th := Setup(t).InitBasic() + defer th.TearDown() bot, appErr := th.App.CreateBot(th.Context, &model.Bot{ Username: "testbot",