diff --git a/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts b/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts index 1b33914aacfd..d4e534a14e68 100644 --- a/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts +++ b/e2e-tests/playwright/specs/functional/channels/post_list/post_height.spec.ts @@ -6,8 +6,9 @@ import type {ServerChannel} from '@mattermost/types/channels'; import type {Team} from '@mattermost/types/teams'; import type {UserProfile} from '@mattermost/types/users'; import type {Disposable, Locator, Page} from '@playwright/test'; +import type {Post} from '@mattermost/types/posts'; -import {expect, setupFileServer, test, watchElementSize} from '@mattermost/playwright-lib'; +import {expect, setupFileServer, test, testConfig, watchElementSize} from '@mattermost/playwright-lib'; import type {ChannelsPage, ChannelsPost, PlaywrightClient4} from '@mattermost/playwright-lib'; test.describe('Post height', () => { @@ -54,10 +55,8 @@ test.describe('Post height', () => { type PostHeightTestCase = { name: string; - /** Static seed options for posts that don't depend on the file server URL. */ - seedOptions?: SeedOptions; - /** Seed options builder for posts that reference the file server (e.g. Markdown images). */ - getSeedOptions?: (baseUrl: string) => SeedOptions; + /** Returns the post to be measured and does any other prep work needed to set up the post. */ + makePost: (options: {fileServerUrl: string; siteUrl: string}) => Promise; /** Extra assertions to run once the post has loaded. */ additionalCheck?: (args: {postComponent: ChannelsPost}) => Promise; /** Playwright project names for which this test case should be skipped. */ @@ -67,16 +66,18 @@ test.describe('Post height', () => { const testCases: PostHeightTestCase[] = [ { name: 'text only post', - seedOptions: { - message: 'text only post', - }, + makePost: () => + seedPost({ + message: 'text only post', + }), }, { name: 'post with replies', - seedOptions: { - message: 'post with replies', - replyCount: 3, - }, + makePost: () => + seedPost({ + message: 'post with replies', + replyCount: 3, + }), additionalCheck: async ({postComponent}) => { // * Verify that the thread footer has rendered const image = postComponent.container.locator('.ThreadFooter'); @@ -85,10 +86,11 @@ test.describe('Post height', () => { }, { name: 'post with reactions', - seedOptions: { - message: 'post with reactions', - reactions: ['thumbsup', 'heart', 'tada'], - }, + makePost: () => + seedPost({ + message: 'post with reactions', + reactions: ['thumbsup', 'heart', 'tada'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the reactions have rendered const image = postComponent.container.locator('.Reaction'); @@ -97,10 +99,11 @@ test.describe('Post height', () => { }, { name: 'post with a single image', - seedOptions: { - message: 'post with a single image', - files: ['mattermost.png'], - }, + makePost: () => + seedPost({ + message: 'post with a single image', + files: ['mattermost.png'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -109,10 +112,11 @@ test.describe('Post height', () => { }, { name: 'post with a single small image', - seedOptions: { - message: 'post with a single small image', - files: ['small-image.png'], - }, + makePost: () => + seedPost({ + message: 'post with a single small image', + files: ['small-image.png'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the image has rendered const image = postComponent.container.locator('.small-image__container'); @@ -123,10 +127,11 @@ test.describe('Post height', () => { name: 'post with a single large image', // MM-69979 Skip this on iPad because images that are too wide but above the minimum height cause layout shift skipProjects: ['ipad'], - seedOptions: { - message: 'post with a single large image', - files: ['huge-image.jpg'], - }, + makePost: () => + seedPost({ + message: 'post with a single large image', + files: ['huge-image.jpg'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -135,10 +140,11 @@ test.describe('Post height', () => { }, { name: 'post with a single wide image', - seedOptions: { - message: 'post with a single wide image', - files: ['image-400x40.jpg'], - }, + makePost: () => + seedPost({ + message: 'post with a single wide image', + files: ['image-400x40.jpg'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the image has rendered const image = postComponent.container.locator('.small-image__container img'); @@ -147,10 +153,11 @@ test.describe('Post height', () => { }, { name: 'post with a single tall image', - seedOptions: { - message: 'post with a single tall image', - files: ['image-40x400.jpg'], - }, + makePost: () => + seedPost({ + message: 'post with a single tall image', + files: ['image-40x400.jpg'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the image has rendered const image = postComponent.container.locator('.small-image__container img'); @@ -159,10 +166,11 @@ test.describe('Post height', () => { }, { name: 'post with a non-image file attachment', - seedOptions: { - message: 'post with a non-image file attachment', - files: ['sample_text_file.txt'], - }, + makePost: () => + seedPost({ + message: 'post with a non-image file attachment', + files: ['sample_text_file.txt'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the attachment has rendered const image = postComponent.container.locator('.post-image__columns'); @@ -171,10 +179,11 @@ test.describe('Post height', () => { }, { name: 'post with multiple images', - seedOptions: { - message: 'post with multiple images', - files: ['mattermost.png', 'mattermost-icon_128x128.png', 'mattermost.png'], - }, + makePost: () => + seedPost({ + message: 'post with multiple images', + files: ['mattermost.png', 'mattermost-icon_128x128.png', 'mattermost.png'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the images have rendered const image = postComponent.container.locator('.MediaGallery__tile img'); @@ -183,38 +192,42 @@ test.describe('Post height', () => { }, { name: 'post with a code block without a language', - seedOptions: { - message: '```\nconst foo = 1;\nconst bar = 2;\n```', - }, + makePost: () => + seedPost({ + message: '```\nconst foo = 1;\nconst bar = 2;\n```', + }), }, { name: 'post with a syntax-highlighted code block', - seedOptions: { - message: '```javascript\nconst foo = 1;\nconst bar = 2;\n```', - }, + makePost: () => + seedPost({ + message: '```javascript\nconst foo = 1;\nconst bar = 2;\n```', + }), }, { name: 'post with a message attachment', - seedOptions: { - message: 'post with a message attachment', - props: { - attachments: [ - { - author_name: 'Author', - title: 'Message attachment title', - title_link: 'https://example.com', - text: 'Message attachment body text', - }, - ], - }, - }, + makePost: () => + seedPost({ + message: 'post with a message attachment', + props: { + attachments: [ + { + author_name: 'Author', + title: 'Message attachment title', + title_link: 'https://example.com', + text: 'Message attachment body text', + }, + ], + }, + }), }, { name: 'post with a single SVG attachment', - seedOptions: { - message: 'post with a single SVG attachment', - files: ['icon.svg'], - }, + makePost: () => + seedPost({ + message: 'post with a single SVG attachment', + files: ['icon.svg'], + }), additionalCheck: async ({postComponent}) => { // * Verify that the SVG has rendered as an image const image = postComponent.container.locator('.image-loaded-container'); @@ -223,9 +236,10 @@ test.describe('Post height', () => { }, { name: 'post with a Markdown image', - getSeedOptions: (baseUrl) => ({ - message: `![mattermost](${baseUrl}/mattermost.png)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![mattermost](${fileServerUrl}/mattermost.png)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -234,9 +248,10 @@ test.describe('Post height', () => { }, { name: 'post with a small Markdown image', - getSeedOptions: (baseUrl) => ({ - message: `![small image](${baseUrl}/small-image.png)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![small image](${fileServerUrl}/small-image.png)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -247,9 +262,10 @@ test.describe('Post height', () => { name: 'post with a large Markdown image', // MM-69979 Images that are too wide but above the minimum height cause layout shift skipProjects: ['chrome', 'firefox', 'ipad'], - getSeedOptions: (baseUrl) => ({ - message: `![large image](${baseUrl}/huge-image.jpg)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![large image](${fileServerUrl}/huge-image.jpg)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -258,9 +274,10 @@ test.describe('Post height', () => { }, { name: 'post with a wide Markdown image', - getSeedOptions: (baseUrl) => ({ - message: `![wide image](${baseUrl}/image-400x40.jpg)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![wide image](${fileServerUrl}/image-400x40.jpg)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -269,9 +286,10 @@ test.describe('Post height', () => { }, { name: 'post with a tall Markdown image', - getSeedOptions: (baseUrl) => ({ - message: `![tall image](${baseUrl}/image-40x400.jpg)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![tall image](${fileServerUrl}/image-40x400.jpg)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -282,9 +300,10 @@ test.describe('Post height', () => { name: 'post with an SVG Markdown image', // Either Chrome preloads the SVG's dimensions early or Firefox doesn't allocate the height properly skipProjects: ['firefox'], - getSeedOptions: (baseUrl) => ({ - message: `![icon](${baseUrl}/icon.svg)`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `![icon](${fileServerUrl}/icon.svg)`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the Markdown image has rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -293,9 +312,10 @@ test.describe('Post height', () => { }, { name: 'post with an image preview', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/mattermost.png`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/mattermost.png`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the image is rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -304,9 +324,10 @@ test.describe('Post height', () => { }, { name: 'post with a small image preview', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/small-image.png`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/small-image.png`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the image is rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -317,9 +338,10 @@ test.describe('Post height', () => { name: 'post with a large image preview', // MM-69979 Images that are too wide but above the minimum height cause layout shift skipProjects: ['chrome', 'firefox', 'ipad'], - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/huge-image.jpg`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/huge-image.jpg`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the image is rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -328,9 +350,10 @@ test.describe('Post height', () => { }, { name: 'post with a wide image preview', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/image-400x40.jpg`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/image-400x40.jpg`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the image is rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -339,9 +362,10 @@ test.describe('Post height', () => { }, { name: 'post with a tall image preview', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/image-40x400.jpg`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/image-40x400.jpg`, + }), additionalCheck: async ({postComponent}) => { // * Verify that the image is rendered const image = postComponent.container.locator('.image-loaded-container'); @@ -350,9 +374,10 @@ test.describe('Post height', () => { }, { name: 'post with an OpenGraph preview', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/opengraph.html`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/opengraph.html`, + }), additionalCheck: async ({postComponent}) => { // * Verify that an OpenGraph preview was rendered const preview = postComponent.container.locator('.PostAttachmentOpenGraph'); @@ -367,9 +392,10 @@ test.describe('Post height', () => { }, { name: 'post with an OpenGraph preview with a larger image', - getSeedOptions: (baseUrl) => ({ - message: `${baseUrl}/opengraph-huge.html`, - }), + makePost: ({fileServerUrl}) => + seedPost({ + message: `${fileServerUrl}/opengraph-huge.html`, + }), additionalCheck: async ({postComponent}) => { const preview = postComponent.container.locator('.PostAttachmentOpenGraph'); await expect(preview).toBeVisible(); @@ -379,6 +405,36 @@ test.describe('Post height', () => { await expect(preview.locator('.PostAttachmentOpenGraph__image img')).toBeVisible(); }, }, + { + name: 'post with a post preview', + makePost: async ({siteUrl}) => { + const linkedPost = await seedPost({ + message: 'This is a post to be previewed.', + }); + + return seedPost({ + message: `${siteUrl}/${team.name}/pl/${linkedPost.id}`, + }); + }, + }, + { + name: 'post with a long post preview', + makePost: async ({siteUrl}) => { + const linkedPost = await seedPost({ + message: new Array(50).fill('This is a multi-line post to be previewed.').join('\n'), + }); + + return seedPost({ + message: `${siteUrl}/${team.name}/pl/${linkedPost.id}`, + }); + }, + additionalCheck: async ({postComponent}) => { + // * Verify that the preview is faded out and has the "Show more" link visible + const showMoreButton = postComponent.container.locator('.post-preview-collapse__show-more-button'); + await expect(showMoreButton).toBeVisible(); + await expect(postComponent.container.locator('.post-message-preview--overflow')).toBeVisible(); + }, + }, ]; for (const testCase of testCases) { @@ -391,10 +447,10 @@ test.describe('Post height', () => { `Not supported on ${testInfo.project.name}`, ); - const seedOptions = testCase.getSeedOptions - ? testCase.getSeedOptions(fileServerUrl) - : testCase.seedOptions!; - const post = await seedPost(seedOptions); + const post = await testCase.makePost({ + fileServerUrl, + siteUrl: testConfig.internalBaseURL, + }); const {sizeWatcher, postComponent} = await openChannelAndGetPost(post.id); diff --git a/server/channels/api4/apitestlib.go b/server/channels/api4/apitestlib.go index 845812288fd7..cf6ce88db350 100644 --- a/server/channels/api4/apitestlib.go +++ b/server/channels/api4/apitestlib.go @@ -370,7 +370,7 @@ func SetupConfigWithStoreMock(tb testing.TB, updateConfig func(cfg *model.Config } func SetupWithStoreMock(tb testing.TB) *TestHelper { - th := setupTestHelper(tb, testlib.GetMockStoreForSetupFunctions(), nil, nil, false, false, nil, nil) + th := setupTestHelper(tb, testlib.GetMockStoreForSetupFunctions(), nil, nil, false, false, useCustomPushNotificationServer, nil) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) @@ -383,10 +383,15 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { return th } +func useCustomPushNotificationServer(config *model.Config) { + *config.EmailSettings.PushNotificationServer = "https://push.example.com" +} + func SetupEnterpriseWithStoreMock(tb testing.TB, options ...app.Option) *TestHelper { removeSpuriousErrors := func(config *model.Config) { // If not set, you will receive an unactionable error in the console *config.ServiceSettings.SiteURL = "http://localhost:8065" + useCustomPushNotificationServer(config) } th := setupTestHelper(tb, testlib.GetMockStoreForSetupFunctions(), nil, nil, true, false, removeSpuriousErrors, options) diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 2be8d2ff74b1..b95a4ddda16d 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -6878,6 +6878,11 @@ func TestGetChannelModerations(t *testing.T) { scheme := th.SetupTeamScheme(t) scheme.DefaultChannelGuestRole = "" + // Restore the real store so helper cleanup (cache invalidation, license reload) + // doesn't run against the partial mock. + originalStore := th.App.Srv().Store() + t.Cleanup(func() { th.App.Srv().SetStore(originalStore) }) + mockStore := mocks.Store{} // Playbooks DB job requires a plugin mock @@ -7034,6 +7039,11 @@ func TestPatchChannelModerations(t *testing.T) { scheme := th.SetupTeamScheme(t) scheme.DefaultChannelGuestRole = "" + // Restore the real store so helper cleanup (cache invalidation, license reload) + // doesn't run against the partial mock. + originalStore := th.App.Srv().Store() + t.Cleanup(func() { th.App.Srv().SetStore(originalStore) }) + mockStore := mocks.Store{} // Playbooks DB job requires a plugin mock diff --git a/server/channels/app/helper_test.go b/server/channels/app/helper_test.go index 7e53b889d8a6..611b806bdc02 100644 --- a/server/channels/app/helper_test.go +++ b/server/channels/app/helper_test.go @@ -261,9 +261,13 @@ func SetupWithoutPreloadMigrations(tb testing.TB) *TestHelper { return setupTestHelper(dbStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), false, true, nil, nil, tb) } +func useCustomPushNotificationServer(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = "https://push.example.com" +} + func SetupWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), false, false, nil, nil, tb) + th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), false, false, useCustomPushNotificationServer, nil, tb) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) @@ -284,7 +288,7 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), true, false, nil, nil, tb) + th := setupTestHelper(mockStore, mainHelper.GetSQLStore(), mainHelper.GetSQLSettings(), mainHelper.GetSearchEngine(), true, false, useCustomPushNotificationServer, nil, tb) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 4efe985718d4..06562479396b 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -30,15 +30,7 @@ func (a *App) canSendPushNotifications() bool { } pushServer := *a.Config().EmailSettings.PushNotificationServer - // Check for MHPNS servers (both current and legacy DNS aliases) - isMHPNSServer := pushServer == model.MHPNS || - pushServer == model.MHPNSLegacyUS || - pushServer == model.MHPNSLegacyDE || - pushServer == model.MHPNSGlobal || - pushServer == model.MHPNSUS || - pushServer == model.MHPNSEU || - pushServer == model.MHPNSAP - if license := a.Srv().License(); isMHPNSServer && (license == nil || !*license.Features.MHPNS) { + if model.IsMHPNSEndpoint(pushServer) && !a.Srv().License().HasMHPNS() { a.Log().LogM(mlog.MlvlNotificationWarn, "Push notifications are disabled - license missing", mlog.String("status", model.NotificationStatusNotSent), mlog.String("reason", "push_disabled_license"), diff --git a/server/channels/app/push_notification_server.go b/server/channels/app/push_notification_server.go new file mode 100644 index 000000000000..8ef58c80f4af --- /dev/null +++ b/server/channels/app/push_notification_server.go @@ -0,0 +1,79 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" +) + +// syncPushNotificationServerWithLicense switches EmailSettings.PushNotificationServer to the +// hosted push notification service (MHPNS) endpoint when the license grants MHPNS access, and +// back to the test (TPNS) endpoint when it no longer does. It runs on license changes, on +// server start, and when this node becomes the cluster leader. +// +// The contract: promote only from exact TPNS to the Global endpoint; on entitlement loss, +// revert any Mattermost-hosted production endpoint (global, regional, or legacy) to TPNS, so +// a lapsed license never leaves push pointing at an endpoint that refuses to send. Custom +// endpoints and env-managed values are never touched. The sync stays stateless because both +// directions derive entirely from the current config value and the license. +func (s *Server) syncPushNotificationServerWithLicense() { + if !s.IsLeader() { + return + } + + license := s.License() + // Cloud config is centrally managed; never rewrite it here. + if license.IsCloud() { + return + } + + if s.platform.IsConfigReadOnly() { + return + } + + // Respect an environment-variable override on the setting. The config store re-applies env + // overrides on save anyway, so without this guard a save would be futile and only produce + // spurious audit records, logs, and cluster config traffic on every license event. + if emailOverrides, ok := s.platform.GetEnvironmentOverrides()["EmailSettings"].(map[string]any); ok { + if _, overridden := emailOverrides["PushNotificationServer"]; overridden { + return + } + } + + entitled := license.HasMHPNS() + + // Decide and mutate on the same snapshot so a concurrent config write between the + // decision and the save can't be stomped with a stale value. The residual race between + // Clone and Set is inherent to every SaveConfig caller. + cfg := s.platform.Config().Clone() + current := *cfg.EmailSettings.PushNotificationServer + + var target string + switch { + case entitled && current == model.GenericNotificationServer: + target = model.MHPNSGlobal + case !entitled && model.IsMHPNSEndpoint(current): + target = model.GenericNotificationServer + default: + return + } + + cfg.EmailSettings.PushNotificationServer = model.NewPointer(target) + if _, _, appErr := s.platform.SaveConfig(cfg, true); appErr != nil { + mlog.Warn("Failed to switch push notification server for license entitlement", + mlog.String("old", current), mlog.String("new", target), mlog.Err(appErr)) + return + } + mlog.Info("Automatically switched push notification server based on license entitlement", + mlog.String("old", current), mlog.String("new", target)) + + rctx := request.EmptyContext(s.Log()) + appInstance := New(ServerConnector(s.Channels())) + rec := appInstance.MakeAuditRecord(rctx, model.AuditEventAutoSelectPushNotificationServer, model.AuditStatusSuccess) + model.AddEventParameterToAuditRec(rec, "old_push_notification_server", current) + model.AddEventParameterToAuditRec(rec, "new_push_notification_server", target) + appInstance.LogAuditRec(rctx, rec, nil) +} diff --git a/server/channels/app/push_notification_server_test.go b/server/channels/app/push_notification_server_test.go new file mode 100644 index 000000000000..357d54305765 --- /dev/null +++ b/server/channels/app/push_notification_server_test.go @@ -0,0 +1,192 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + + "github.com/mattermost/mattermost/server/public/model" + emailmocks "github.com/mattermost/mattermost/server/v8/channels/app/email/mocks" + clustermocks "github.com/mattermost/mattermost/server/v8/einterfaces/mocks" +) + +func TestSyncPushNotificationServerWithLicense(t *testing.T) { + // Not parallel: subtests mutate the license, shared config, and environment. + th := Setup(t) + + licenseWithMHPNS := model.NewTestLicense("mhpns") + licenseWithoutMHPNS := model.NewTestLicense() + licenseWithoutMHPNS.Features.MHPNS = model.NewPointer(false) + + tests := []struct { + name string + license *model.License + initialServer string + expectedServer string + }{ + { + name: "entitled license switches TPNS to Global", + license: licenseWithMHPNS, + initialServer: model.GenericNotificationServer, + expectedServer: model.MHPNSGlobal, + }, + { + name: "entitlement removed reverts Global to TPNS", + license: licenseWithoutMHPNS, + initialServer: model.MHPNSGlobal, + expectedServer: model.GenericNotificationServer, + }, + { + name: "license removed reverts Global to TPNS", + license: nil, + initialServer: model.MHPNSGlobal, + expectedServer: model.GenericNotificationServer, + }, + { + name: "entitled license leaves custom endpoint untouched", + license: licenseWithMHPNS, + initialServer: "https://push.example.com", + expectedServer: "https://push.example.com", + }, + { + name: "entitled license leaves regional endpoint untouched", + license: licenseWithMHPNS, + initialServer: model.MHPNSEU, + expectedServer: model.MHPNSEU, + }, + { + name: "entitled license leaves Global untouched", + license: licenseWithMHPNS, + initialServer: model.MHPNSGlobal, + expectedServer: model.MHPNSGlobal, + }, + { + name: "unentitled license reverts regional endpoint (MHPNSUS) to TPNS", + license: licenseWithoutMHPNS, + initialServer: model.MHPNSUS, + expectedServer: model.GenericNotificationServer, + }, + { + name: "unentitled license reverts legacy endpoint (MHPNSLegacyDE) to TPNS", + license: licenseWithoutMHPNS, + initialServer: model.MHPNSLegacyDE, + expectedServer: model.GenericNotificationServer, + }, + { + name: "unentitled license leaves TPNS untouched", + license: licenseWithoutMHPNS, + initialServer: model.GenericNotificationServer, + expectedServer: model.GenericNotificationServer, + }, + { + name: "unentitled license leaves custom endpoint untouched", + license: licenseWithoutMHPNS, + initialServer: "https://push.example.com", + expectedServer: "https://push.example.com", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + th.App.Srv().SetLicense(nil) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = tc.initialServer + *cfg.EmailSettings.SendPushNotifications = true + }) + + // Setting the license fires the listener registered in NewServer, + // which runs syncPushNotificationServerWithLicense. + th.App.Srv().SetLicense(tc.license) + + cfg := th.App.Config() + assert.Equal(t, tc.expectedServer, *cfg.EmailSettings.PushNotificationServer) + assert.True(t, *cfg.EmailSettings.SendPushNotifications, "SendPushNotifications must never be modified") + }) + } + + t.Run("direct call switches TPNS to Global on the startup path", func(t *testing.T) { + th.App.Srv().SetLicense(licenseWithMHPNS) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = model.GenericNotificationServer + }) + + th.Server.syncPushNotificationServerWithLicense() + + assert.Equal(t, model.MHPNSGlobal, *th.App.Config().EmailSettings.PushNotificationServer) + }) + + t.Run("license with nil MHPNS feature is unentitled and reverts Global to TPNS", func(t *testing.T) { + license := model.NewTestLicense() + th.App.Srv().SetLicense(license) + // SetLicense normalizes feature defaults, back-filling any nil pointer, so a license + // with a nil MHPNS can only reach the sync through the direct path. Clear the field + // on the stored license to prove the entitlement check is nil-safe and treats the + // license as unentitled. Restore it afterwards: license logging during teardown + // dereferences every feature pointer via Features.ToMap. + license.Features.MHPNS = nil + t.Cleanup(func() { license.Features.MHPNS = model.NewPointer(false) }) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = model.MHPNSGlobal + }) + + th.Server.syncPushNotificationServerWithLicense() + + assert.Equal(t, model.GenericNotificationServer, *th.App.Config().EmailSettings.PushNotificationServer) + }) + + t.Run("environment override leaves setting untouched", func(t *testing.T) { + t.Setenv("MM_EMAILSETTINGS_PUSHNOTIFICATIONSERVER", model.GenericNotificationServer) + + // The config value alone can't prove the env-override guard fired: a save of this + // config would be a no-op anyway, because the store re-applies env overrides and + // skips config listeners when the effective config is unchanged. The one side + // effect a futile save cannot avoid is cluster propagation — SaveConfig calls + // ConfigChanged on the cluster interface unconditionally — so probe that to prove + // the guard returned before saving. + clusterMock := &clustermocks.ClusterInterface{} + clusterMock.On("IsLeader").Return(true).Maybe() + clusterMock.On("GetClusterId").Return("").Maybe() + clusterMock.On("SendClusterMessage", mock.Anything).Return().Maybe() + clusterMock.On("RegisterClusterMessageHandler", mock.Anything, mock.Anything).Return().Maybe() + clusterMock.On("StopInterNodeCommunication").Return().Maybe() + clusterMock.On("Shutdown").Return().Maybe() + clusterMock.On("ConfigChanged", mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() + + // The subtest needs its own harness so the mock is installed before the platform + // starts; swapping the cluster interface mid-test races with platform goroutines + // that read it. Setup also isolates the env var set above. + envTh := SetupWithClusterMock(t, clusterMock) + + envTh.App.Srv().SetLicense(licenseWithMHPNS) + envTh.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = model.GenericNotificationServer + }) + + envTh.Server.syncPushNotificationServerWithLicense() + + clusterMock.AssertNotCalled(t, "ConfigChanged", mock.Anything, mock.Anything, mock.Anything) + assert.Equal(t, model.GenericNotificationServer, *envTh.App.Config().EmailSettings.PushNotificationServer) + }) + + t.Run("reverting hosted endpoint does not re-init email batching", func(t *testing.T) { + originalEmailService := th.App.Srv().EmailService + t.Cleanup(func() { + th.App.Srv().EmailService = originalEmailService + }) + + emailServiceMock := emailmocks.ServiceInterface{} + th.App.Srv().EmailService = &emailServiceMock + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.EmailSettings.PushNotificationServer = model.MHPNSGlobal + }) + th.App.Srv().SetLicense(nil) + + emailServiceMock.AssertNotCalled(t, "InitEmailBatching") + assert.Equal(t, model.GenericNotificationServer, *th.App.Config().EmailSettings.PushNotificationServer) + }) +} diff --git a/server/channels/app/server.go b/server/channels/app/server.go index defab167a4d6..5f35380c73fd 100644 --- a/server/channels/app/server.go +++ b/server/channels/app/server.go @@ -128,6 +128,9 @@ type Server struct { clusterLeaderListenerId string loggerLicenseListenerId string + pushNotificationServerLicenseListenerId string + pushNotificationServerClusterLeaderListenerId string + platform *platform.PlatformService platformOptions []platform.Option telemetryService *telemetry.TelemetryService @@ -506,9 +509,11 @@ func NewServer(options ...Option) (*Server, error) { } } - // Start email batching because it's not like the other jobs - s.platform.AddConfigListener(func(_, _ *model.Config) { - s.EmailService.InitEmailBatching() + // Re-init email batching only when its enable flag or interval changes. + s.platform.AddConfigListener(func(oldCfg, newCfg *model.Config) { + if emailBatchingSettingChanged(oldCfg, newCfg) { + s.EmailService.InitEmailBatching() + } }) pwd, _ := os.Getwd() @@ -540,6 +545,15 @@ func NewServer(options ...Option) (*Server, error) { s.platform.EnableLoggingMetrics() }) + // Keep the push notification server in sync with the license's HPNS entitlement, and let a + // newly-elected cluster leader repair any transition missed while another node was leader. + s.pushNotificationServerLicenseListenerId = s.AddLicenseListener(func(oldLicense, newLicense *model.License) { + s.syncPushNotificationServerWithLicense() + }) + s.pushNotificationServerClusterLeaderListenerId = s.AddClusterLeaderChangedListener(func() { + s.syncPushNotificationServerWithLicense() + }) + // if enabled - perform initial product notices fetch if *s.platform.Config().AnnouncementSettings.AdminNoticesEnabled || *s.platform.Config().AnnouncementSettings.UserNoticesEnabled { s.platform.Go(func() { @@ -767,6 +781,8 @@ func (s *Server) Shutdown() { s.RemoveLicenseListener(s.loggerLicenseListenerId) s.RemoveClusterLeaderChangedListener(s.clusterLeaderListenerId) + s.RemoveLicenseListener(s.pushNotificationServerLicenseListenerId) + s.RemoveClusterLeaderChangedListener(s.pushNotificationServerClusterLeaderListenerId) var err error s.serviceMux.RLock() @@ -1008,6 +1024,8 @@ func (s *Server) Start() error { } } + s.syncPushNotificationServerWithLicense() + s.checkPushNotificationServerURL() if err = s.platform.ReloadConfig(); err != nil { @@ -2045,3 +2063,13 @@ func (s *Server) Platform() *platform.PlatformService { func (s *Server) Log() *mlog.Logger { return s.platform.Logger() } + +func emailBatchingSettingChanged(oldCfg, newCfg *model.Config) bool { + if oldCfg == nil || newCfg == nil { + return true + } + return model.SafeDereference(oldCfg.EmailSettings.EnableEmailBatching) != + model.SafeDereference(newCfg.EmailSettings.EnableEmailBatching) || + model.SafeDereference(oldCfg.EmailSettings.EmailBatchingInterval) != + model.SafeDereference(newCfg.EmailSettings.EmailBatchingInterval) +} diff --git a/server/channels/app/server_test.go b/server/channels/app/server_test.go index ddc3f4909a03..310252d72f9d 100644 --- a/server/channels/app/server_test.go +++ b/server/channels/app/server_test.go @@ -566,3 +566,50 @@ func TestOriginChecker(t *testing.T) { require.Equalf(t, tc.Pass, res, "Test case (%d)", i) } } + +func TestEmailBatchingSettingChanged(t *testing.T) { + t.Parallel() + + cfg := func(enabled bool, interval int) *model.Config { + c := &model.Config{} + c.EmailSettings.EnableEmailBatching = model.NewPointer(enabled) + c.EmailSettings.EmailBatchingInterval = model.NewPointer(interval) + return c + } + + tests := []struct { + name string + oldCfg *model.Config + newCfg *model.Config + expected bool + }{ + {name: "nil old config", oldCfg: nil, newCfg: cfg(true, 30), expected: true}, + {name: "nil new config", oldCfg: cfg(true, 30), newCfg: nil, expected: true}, + {name: "unchanged disabled", oldCfg: cfg(false, 30), newCfg: cfg(false, 30), expected: false}, + {name: "unchanged enabled", oldCfg: cfg(true, 30), newCfg: cfg(true, 30), expected: false}, + {name: "enabled", oldCfg: cfg(false, 30), newCfg: cfg(true, 30), expected: true}, + {name: "disabled", oldCfg: cfg(true, 30), newCfg: cfg(false, 30), expected: true}, + {name: "interval changed", oldCfg: cfg(true, 30), newCfg: cfg(true, 300), expected: true}, + { + name: "push notification server change is ignored", + oldCfg: func() *model.Config { + c := cfg(false, 30) + c.EmailSettings.PushNotificationServer = model.NewPointer(model.MHPNSGlobal) + return c + }(), + newCfg: func() *model.Config { + c := cfg(false, 30) + c.EmailSettings.PushNotificationServer = model.NewPointer(model.GenericNotificationServer) + return c + }(), + expected: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.expected, emailBatchingSettingChanged(tc.oldCfg, tc.newCfg)) + }) + } +} diff --git a/server/channels/app/team_test.go b/server/channels/app/team_test.go index 4df60b6ceda5..5e10ee7750c5 100644 --- a/server/channels/app/team_test.go +++ b/server/channels/app/team_test.go @@ -2219,7 +2219,9 @@ func TestInviteNewUsersToTeamGracefully(t *testing.T) { mainHelper.Parallel(t) th := Setup(t).InitBasic(t) - th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) + license := model.NewTestLicenseWithFalseDefaults("mhpns") + license.SkuShortName = model.LicenseShortSkuEnterprise + th.App.Srv().SetLicense(license) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true *cfg.TeamSettings.LockProfileFieldsForEmailUsers = model.TeamSettingsLockProfileFieldsNameAndUsername @@ -2368,6 +2370,8 @@ func TestInviteNewUsersToTeamGracefully(t *testing.T) { t.Run("it returns error for deactivated user without sending email", func(t *testing.T) { emailServiceMock := emailmocks.ServiceInterface{} emailServiceMock.On("Stop").Once().Return() + // The teardown license reset saves a config change, whose listener re-inits email batching. + emailServiceMock.On("InitEmailBatching").Return().Maybe() th.App.Srv().EmailService = &emailServiceMock _, appErr := th.App.UpdateActive(th.Context, th.BasicUser2, false) diff --git a/server/public/model/audit_events.go b/server/public/model/audit_events.go index 8d047029c58e..ff916bd7d24c 100644 --- a/server/public/model/audit_events.go +++ b/server/public/model/audit_events.go @@ -142,15 +142,16 @@ const ( // Configuration const ( - AuditEventConfigReload = "configReload" // reload server configuration - AuditEventGetConfig = "getConfig" // get current server configuration - AuditEventLocalGetClientConfig = "localGetClientConfig" // get client configuration locally - AuditEventLocalGetConfig = "localGetConfig" // get server configuration locally - AuditEventLocalPatchConfig = "localPatchConfig" // update server configuration locally - AuditEventLocalUpdateConfig = "localUpdateConfig" // update server configuration locally - AuditEventMigrateConfig = "migrateConfig" // migrate configs with file values from one store to another - AuditEventPatchConfig = "patchConfig" // update server configuration - AuditEventUpdateConfig = "updateConfig" // update server configuration + AuditEventAutoSelectPushNotificationServer = "autoSelectPushNotificationServer" // automatically switch push notification server based on license entitlement + AuditEventConfigReload = "configReload" // reload server configuration + AuditEventGetConfig = "getConfig" // get current server configuration + AuditEventLocalGetClientConfig = "localGetClientConfig" // get client configuration locally + AuditEventLocalGetConfig = "localGetConfig" // get server configuration locally + AuditEventLocalPatchConfig = "localPatchConfig" // update server configuration locally + AuditEventLocalUpdateConfig = "localUpdateConfig" // update server configuration locally + AuditEventMigrateConfig = "migrateConfig" // migrate configs with file values from one store to another + AuditEventPatchConfig = "patchConfig" // update server configuration + AuditEventUpdateConfig = "updateConfig" // update server configuration ) // Custom Profile Attributes diff --git a/server/public/model/license.go b/server/public/model/license.go index 97f91c843760..40417c40919e 100644 --- a/server/public/model/license.go +++ b/server/public/model/license.go @@ -430,6 +430,11 @@ func (l *License) HasSharedChannels() bool { MinimumProfessionalLicense(l) } +// HasMHPNS reports whether the license grants access to the Mattermost hosted push notification service. +func (l *License) HasMHPNS() bool { + return l != nil && l.Features != nil && l.Features.MHPNS != nil && *l.Features.MHPNS +} + // NewTestLicense returns a license that expires in the future and has the given features. func NewTestLicense(features ...string) *License { ret := &License{ diff --git a/server/public/model/license_test.go b/server/public/model/license_test.go index 3a0431c12cbc..64b4ba568fc9 100644 --- a/server/public/model/license_test.go +++ b/server/public/model/license_test.go @@ -486,6 +486,56 @@ func TestLicenseHasSharedChannels(t *testing.T) { } } +func TestLicenseHasMHPNS(t *testing.T) { + testCases := []struct { + description string + license *License + expectedValue bool + }{ + { + "nil license", + nil, + false, + }, + { + "nil features", + &License{}, + false, + }, + { + "nil MHPNS feature", + &License{ + Features: &Features{}, + }, + false, + }, + { + "MHPNS feature disabled", + &License{ + Features: &Features{ + MHPNS: new(false), + }, + }, + false, + }, + { + "MHPNS feature enabled", + &License{ + Features: &Features{ + MHPNS: new(true), + }, + }, + true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + assert.Equal(t, testCase.expectedValue, testCase.license.HasMHPNS()) + }) + } +} + func TestMinimumProfessionalLicense(t *testing.T) { testCases := []struct { description string diff --git a/server/public/model/push_notification.go b/server/public/model/push_notification.go index fc7688c77011..84c6c0b7afdd 100644 --- a/server/public/model/push_notification.go +++ b/server/public/model/push_notification.go @@ -4,6 +4,7 @@ package model import ( + "slices" "strings" ) @@ -43,6 +44,19 @@ const ( PushReceived = "Received by device" ) +// IsMHPNSEndpoint reports whether the given push notification server URL is one of the +// Mattermost-hosted (HPNS) production endpoints. +func IsMHPNSEndpoint(url string) bool { + return slices.Contains([]string{ + MHPNSLegacyUS, + MHPNSLegacyDE, + MHPNSGlobal, + MHPNSUS, + MHPNSEU, + MHPNSAP, + }, url) +} + // PushSubType allows for passing additional message type information // to mobile clients in a backwards-compatible way type PushSubType string diff --git a/server/public/model/push_notification_test.go b/server/public/model/push_notification_test.go index 3d59e74017dc..9f28c7f91dfa 100644 --- a/server/public/model/push_notification_test.go +++ b/server/public/model/push_notification_test.go @@ -54,3 +54,29 @@ func TestPushNotificationDeviceId(t *testing.T) { msg.Platform = "" msg.DeviceId = "" } + +func TestIsMHPNSEndpoint(t *testing.T) { + tests := []struct { + name string + url string + expected bool + }{ + {name: "legacy US endpoint", url: MHPNSLegacyUS, expected: true}, + {name: "legacy DE endpoint", url: MHPNSLegacyDE, expected: true}, + {name: "global endpoint", url: MHPNSGlobal, expected: true}, + {name: "US endpoint", url: MHPNSUS, expected: true}, + {name: "EU endpoint", url: MHPNSEU, expected: true}, + {name: "AP endpoint", url: MHPNSAP, expected: true}, + {name: "legacy MHPNS alias", url: MHPNS, expected: true}, + {name: "test endpoint", url: GenericNotificationServer, expected: false}, + {name: "custom endpoint", url: "https://push.example.com", expected: false}, + {name: "empty string", url: "", expected: false}, + {name: "case variant", url: "https://GLOBAL.push.mattermost.com", expected: false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.expected, IsMHPNSEndpoint(tc.url)) + }) + } +} diff --git a/webapp/channels/src/components/user_settings/display/__snapshots__/user_settings_display.test.tsx.snap b/webapp/channels/src/components/user_settings/display/__snapshots__/user_settings_display.test.tsx.snap index 358f493e1120..49e9d9ac9531 100644 --- a/webapp/channels/src/components/user_settings/display/__snapshots__/user_settings_display.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/display/__snapshots__/user_settings_display.test.tsx.snap @@ -25,14 +25,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -624,14 +624,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -1223,14 +1223,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -1822,14 +1822,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -2421,14 +2421,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -2936,14 +2936,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -3445,14 +3445,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -4044,14 +4044,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -4655,14 +4655,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -5201,14 +5201,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -5747,14 +5747,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -6293,14 +6293,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -6847,14 +6847,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Display Settings

@@ -7393,14 +7393,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should not show la

-
-
+ Display Settings

diff --git a/webapp/channels/src/components/user_settings/headers/setting_mobile_header.tsx b/webapp/channels/src/components/user_settings/headers/setting_mobile_header.tsx index d892c3517753..50d0eca4679f 100644 --- a/webapp/channels/src/components/user_settings/headers/setting_mobile_header.tsx +++ b/webapp/channels/src/components/user_settings/headers/setting_mobile_header.tsx @@ -29,7 +29,10 @@ const SettingMobileHeader = ({

-
+
+ {text}

diff --git a/webapp/channels/src/components/user_settings/modal/user_settings_modal.test.tsx b/webapp/channels/src/components/user_settings/modal/user_settings_modal.test.tsx index f8229626c0fd..a3b35f44906e 100644 --- a/webapp/channels/src/components/user_settings/modal/user_settings_modal.test.tsx +++ b/webapp/channels/src/components/user_settings/modal/user_settings_modal.test.tsx @@ -8,7 +8,7 @@ import React from 'react'; import type {DeepPartial} from '@mattermost/types/utilities'; import mergeObjects from 'packages/mattermost-redux/test/merge_objects'; -import {renderWithContext} from 'tests/react_testing_utils'; +import {renderWithContext, userEvent} from 'tests/react_testing_utils'; import {TestHelper} from 'utils/test_helper'; import type {GlobalState} from 'types/store'; @@ -41,6 +41,7 @@ jest.mock('@mattermost/client', () => ({ })); jest.mock('utils/url', () => ({ + ...jest.requireActual('utils/url'), isValidUrl: jest.fn((url = '') => (/^https?:\/\//i).test(url)), })); @@ -48,29 +49,6 @@ jest.mock('utils/utils', () => ({ getDisplayName: jest.fn(() => 'Test User'), })); -jest.mock('components/user_settings', () => { - return function MockUserSettings({ - activeTab, - pluginSettings, - }: { - activeTab?: string; - pluginSettings: Record; - }) { - const activePluginSettings = activeTab ? pluginSettings[activeTab] : undefined; - - return ( -
- {activePluginSettings && ( - <> -
{`${activePluginSettings.uiName} Settings`}
-
{`${activePluginSettings.uiName} Settings`}
- - )} -
- ); - }; -}); - describe('do first render to avoid other testing issues', () => { // For some reason, the first time we render, the modal does not // completly renders. This makes it so further tests go properly @@ -210,6 +188,28 @@ describe('tabs are properly rendered', () => { }); }); +describe('collapsing the settings pane on mobile', () => { + it('hides the settings pane and clears the active tab', async () => { + renderWithContext(, baseState); + + const modalDialog = document.querySelector('.settings-modal'); + expect(modalDialog).toBeInTheDocument(); + expect(modalDialog).not.toHaveClass('display--content'); + + // Selecting a tab shows the settings pane over the tab list on mobile + await userEvent.click(screen.getByRole('tab', {name: 'display'})); + + expect(modalDialog).toHaveClass('display--content'); + expect(screen.getByRole('tab', {name: 'display'})).toHaveAttribute('aria-selected', 'true'); + + // Pressing back collapses the settings pane to show the tab list again + await userEvent.click(screen.getByRole('button', {name: 'Collapse Icon'})); + + expect(modalDialog).not.toHaveClass('display--content'); + expect(screen.getByRole('tab', {name: 'display'})).toHaveAttribute('aria-selected', 'false'); + }); +}); + describe('plugin tabs use the correct icon', () => { it('use power plug when no icon', () => { const uiName = 'plugin_a'; diff --git a/webapp/channels/src/components/user_settings/modal/user_settings_modal.tsx b/webapp/channels/src/components/user_settings/modal/user_settings_modal.tsx index 7d3c97ab1e15..81efe162d5dd 100644 --- a/webapp/channels/src/components/user_settings/modal/user_settings_modal.tsx +++ b/webapp/channels/src/components/user_settings/modal/user_settings_modal.tsx @@ -2,7 +2,6 @@ // See LICENSE.txt for license information. import React from 'react'; -import ReactDOM from 'react-dom'; import {FormattedMessage, injectIntl} from 'react-intl'; import type {IntlShape} from 'react-intl'; @@ -173,8 +172,8 @@ class UserSettingsModal extends React.PureComponent { // Called to hide the settings pane when on mobile handleCollapse = () => { - const el = ReactDOM.findDOMNode(this.modalBodyRef.current) as HTMLDivElement; - el.closest('.modal-dialog')!.classList.remove('display--content'); + const el = this.modalBodyRef.current; + el?.closest('.modal-dialog')!.classList.remove('display--content'); this.setState({ active_tab: '', diff --git a/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap b/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap index 15a77af8004d..e9a3c21fc3bc 100644 --- a/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/notifications/__snapshots__/user_settings_notifications.test.tsx.snap @@ -28,14 +28,14 @@ Object {

-
-
+ Notification settings

@@ -381,14 +381,14 @@ Object {

-
-
+ Notification settings

@@ -800,14 +800,14 @@ Object {

-
-
+ Notification settings

@@ -1155,14 +1155,14 @@ Object {

-
-
+ Notification settings

@@ -1576,14 +1576,14 @@ Object {

-
-
+ Notification settings

@@ -1892,14 +1892,14 @@ Object {

-
-
+ Notification settings

diff --git a/webapp/channels/src/components/user_settings/plugin/__snapshots__/index.test.tsx.snap b/webapp/channels/src/components/user_settings/plugin/__snapshots__/index.test.tsx.snap index b02e3407106f..9e1658ea6235 100644 --- a/webapp/channels/src/components/user_settings/plugin/__snapshots__/index.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/plugin/__snapshots__/index.test.tsx.snap @@ -25,14 +25,14 @@ exports[`plugin tab all props are properly passed to the children 1`] = `

-
-
+ plugin A Settings

diff --git a/webapp/channels/src/components/user_settings/security/__snapshots__/user_settings_security.test.tsx.snap b/webapp/channels/src/components/user_settings/security/__snapshots__/user_settings_security.test.tsx.snap index cf3ee003d0da..fa4c1c14f2be 100644 --- a/webapp/channels/src/components/user_settings/security/__snapshots__/user_settings_security.test.tsx.snap +++ b/webapp/channels/src/components/user_settings/security/__snapshots__/user_settings_security.test.tsx.snap @@ -25,14 +25,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Security Settings

@@ -196,14 +196,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Security Settings

@@ -367,14 +367,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Security Settings

@@ -538,14 +538,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Security Settings

@@ -709,14 +709,14 @@ exports[`components/user_settings/display/UserSettingsDisplay should match snaps

-
-
+ Security Settings

diff --git a/webapp/channels/src/sass/components/_post.scss b/webapp/channels/src/sass/components/_post.scss index 02e47161e669..9034f697ced7 100644 --- a/webapp/channels/src/sass/components/_post.scss +++ b/webapp/channels/src/sass/components/_post.scss @@ -1965,25 +1965,14 @@ // with a bottom fade so truncated content reads as "more below". .post-message--collapsed.post-message-preview--overflow { .post-message__text-container { - position: relative; + -webkit-mask-image: linear-gradient(to bottom, black calc(100% - 82px), transparent calc(100% - 24px)); + mask-image: linear-gradient(to bottom, black calc(100% - 82px), transparent calc(100% - 24px)); + } - &::after { - position: absolute; - z-index: 1; - right: 0; - bottom: 0; - left: 0; - height: 40%; - min-height: 28px; - max-height: 64px; - background: linear-gradient( - to bottom, - rgba(var(--center-channel-bg-rgb), 0) 0%, - var(--center-channel-bg) 100% - ); - content: ''; - pointer-events: none; - } + .post-preview-collapse__show-more-button { + position: absolute; + bottom: 0; + left: 0; } } diff --git a/webapp/channels/src/utils/test_helper.ts b/webapp/channels/src/utils/test_helper.ts index bacc253566f2..81aa2d6bcd2f 100644 --- a/webapp/channels/src/utils/test_helper.ts +++ b/webapp/channels/src/utils/test_helper.ts @@ -52,7 +52,7 @@ export class TestHelper { email: '', first_name: '', last_name: '', - locale: '', + locale: 'en', nickname: '', position: '', terms_of_service_create_at: 0,