From e59f527f9236fa84108bceb4d3a4bc5ca4f8997d Mon Sep 17 00:00:00 2001 From: Emmy Ni Date: Fri, 30 Sep 2022 11:33:34 -0400 Subject: [PATCH 01/30] remove references to CloudBilling --- config/client.go | 1 - model/config.go | 5 ----- services/telemetry/telemetry.go | 1 - 3 files changed, 7 deletions(-) diff --git a/config/client.go b/config/client.go index 7315ab6898..3e0db4b588 100644 --- a/config/client.go +++ b/config/client.go @@ -54,7 +54,6 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li // This setting is only temporary, so keep using the old setting name for the mobile and web apps props["ExperimentalEnablePostMetadata"] = "true" - props["ExperimentalCloudBilling"] = strconv.FormatBool(*c.ExperimentalSettings.CloudBilling) props["EnableAppBar"] = strconv.FormatBool(*c.ExperimentalSettings.EnableAppBar) props["ExperimentalEnableAutomaticReplies"] = strconv.FormatBool(*c.TeamSettings.ExperimentalEnableAutomaticReplies) diff --git a/model/config.go b/model/config.go index 967cff1dc8..77a430dca4 100644 --- a/model/config.go +++ b/model/config.go @@ -955,7 +955,6 @@ type ExperimentalSettings struct { LinkMetadataTimeoutMilliseconds *int64 `access:"experimental_features,write_restrictable,cloud_restrictable"` RestrictSystemAdmin *bool `access:"experimental_features,write_restrictable"` UseNewSAMLLibrary *bool `access:"experimental_features,cloud_restrictable"` - CloudBilling *bool `access:"experimental_features,write_restrictable"` EnableSharedChannels *bool `access:"experimental_features"` EnableRemoteClusterService *bool `access:"experimental_features"` EnableAppBar *bool `access:"experimental_features"` @@ -978,10 +977,6 @@ func (s *ExperimentalSettings) SetDefaults() { s.RestrictSystemAdmin = NewBool(false) } - if s.CloudBilling == nil { - s.CloudBilling = NewBool(false) - } - if s.UseNewSAMLLibrary == nil { s.UseNewSAMLLibrary = NewBool(false) } diff --git a/services/telemetry/telemetry.go b/services/telemetry/telemetry.go index 3168f61e35..972912c2c0 100644 --- a/services/telemetry/telemetry.go +++ b/services/telemetry/telemetry.go @@ -727,7 +727,6 @@ func (ts *TelemetryService) trackConfig() { "link_metadata_timeout_milliseconds": *cfg.ExperimentalSettings.LinkMetadataTimeoutMilliseconds, "restrict_system_admin": *cfg.ExperimentalSettings.RestrictSystemAdmin, "use_new_saml_library": *cfg.ExperimentalSettings.UseNewSAMLLibrary, - "cloud_billing": *cfg.ExperimentalSettings.CloudBilling, "enable_shared_channels": *cfg.ExperimentalSettings.EnableSharedChannels, "enable_remote_cluster_service": *cfg.ExperimentalSettings.EnableRemoteClusterService && cfg.FeatureFlags.EnableRemoteClusterService, "enable_app_bar": *cfg.ExperimentalSettings.EnableAppBar, From 8edb8157f108b3c89b48e528e30f17780f00acb2 Mon Sep 17 00:00:00 2001 From: Cass C Date: Thu, 6 Oct 2022 15:51:15 -0400 Subject: [PATCH 02/30] prepackage Playbooks v1.32.4 (#21267) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2d830a9ba3..28d8832d6c 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.0.0 PLUGIN_PACKAGES += mattermost-plugin-custom-attributes-v1.3.0 PLUGIN_PACKAGES += mattermost-plugin-github-v2.0.1 PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.3.0 -PLUGIN_PACKAGES += mattermost-plugin-playbooks-v1.32.3 +PLUGIN_PACKAGES += mattermost-plugin-playbooks-v1.32.4 PLUGIN_PACKAGES += mattermost-plugin-jenkins-v1.1.0 PLUGIN_PACKAGES += mattermost-plugin-jira-v2.4.0 PLUGIN_PACKAGES += mattermost-plugin-nps-v1.2.0 From 18f0cee454ea84f33fc7ad9c6752fdfb29796482 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Thu, 6 Oct 2022 15:19:25 -0600 Subject: [PATCH 03/30] update prepackaged boards to v7.4.0 (#21206) Co-authored-by: Mattermod --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 28d8832d6c..56ea3db8bb 100644 --- a/Makefile +++ b/Makefile @@ -160,7 +160,7 @@ PLUGIN_PACKAGES += mattermost-plugin-jira-v2.4.0 PLUGIN_PACKAGES += mattermost-plugin-nps-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-zoom-v1.6.0 -PLUGIN_PACKAGES += focalboard-v7.2.1 +PLUGIN_PACKAGES += focalboard-v7.4.0 PLUGIN_PACKAGES += mattermost-plugin-apps-v1.1.0 # Prepares the enterprise build if exists. The IGNORE stuff is a hack to get the Makefile to execute the commands outside a target From 5a4cac43a8e2b732dc5985706daf66a16e59a2f0 Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Thu, 6 Oct 2022 18:23:30 -0400 Subject: [PATCH 04/30] MM-47037 - Prepackage Calls v0.9.0 (#21268) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 56ea3db8bb..032fe37f3b 100644 --- a/Makefile +++ b/Makefile @@ -149,7 +149,7 @@ TEMPLATES_DIR=templates PLUGIN_PACKAGES ?= mattermost-plugin-antivirus-v0.1.2 PLUGIN_PACKAGES += mattermost-plugin-autolink-v1.2.2 PLUGIN_PACKAGES += mattermost-plugin-aws-SNS-v1.2.0 -PLUGIN_PACKAGES += mattermost-plugin-calls-v0.8.1 +PLUGIN_PACKAGES += mattermost-plugin-calls-v0.9.0 PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.0.0 PLUGIN_PACKAGES += mattermost-plugin-custom-attributes-v1.3.0 PLUGIN_PACKAGES += mattermost-plugin-github-v2.0.1 From e99eba33ff265fd9513770ff2cab2aaf5ef0a893 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Fri, 7 Oct 2022 13:53:13 +0300 Subject: [PATCH 05/30] app/platform: move failover config initialization to platform (#21281) --- app/platform/config.go | 9 --------- app/platform/service.go | 19 +++++++++++++++---- app/server.go | 15 +-------------- 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/app/platform/config.go b/app/platform/config.go index 9fab59450e..5b0218dcd6 100644 --- a/app/platform/config.go +++ b/app/platform/config.go @@ -36,15 +36,6 @@ type ServiceConfig struct { Cluster einterfaces.ClusterInterface } -func (c *ServiceConfig) validate() error { - // Mandatory fields need to be checked here - if c.ConfigStore == nil { - return errors.New("ConfigStore is required") - } - - return nil -} - // ensure the config wrapper implements `product.ConfigService` var _ product.ConfigService = (*PlatformService)(nil) diff --git a/app/platform/service.go b/app/platform/service.go index 261297b2fc..b5a506ae50 100644 --- a/app/platform/service.go +++ b/app/platform/service.go @@ -97,10 +97,6 @@ type PlatformService struct { // New creates a new PlatformService. func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { - if err := sc.validate(); err != nil { - return nil, err - } - // Step 0: Create the PlatformService. // ConfigStore is and should be handled on a upper level. ps := &PlatformService{ @@ -138,6 +134,21 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { } } + // the config store is not set, we need to create a new one + if ps.configStore == nil { + innerStore, err := config.NewFileStore("config.json", true) + if err != nil { + return nil, fmt.Errorf("failed to load config from file: %w", err) + } + + configStore, err := config.NewStoreFromBacking(innerStore, nil, false) + if err != nil { + return nil, fmt.Errorf("failed to load config from file: %w", err) + } + + ps.configStore = configStore + } + // Step 2: Start logging. if err := ps.initLogging(); err != nil { return nil, fmt.Errorf("failed to initialize logging: %w", err) diff --git a/app/server.go b/app/server.go index 591b5751b1..a4688c8b43 100644 --- a/app/server.go +++ b/app/server.go @@ -201,20 +201,7 @@ func NewServer(options ...Option) (*Server, error) { // // Step 1: Platform. if s.platform == nil { - innerStore, err := config.NewFileStore("config.json", true) - if err != nil { - return nil, errors.Wrap(err, "failed to load config") - } - configStore, err := config.NewStoreFromBacking(innerStore, nil, false) - if err != nil { - return nil, errors.Wrap(err, "failed to load config") - } - - platformCfg := platform.ServiceConfig{ - ConfigStore: configStore, - } - - ps, sErr := platform.New(platformCfg, s.platformOptions...) + ps, sErr := platform.New(platform.ServiceConfig{}, s.platformOptions...) if sErr != nil { return nil, errors.Wrap(sErr, "failed to initialize platform") } From 56fd8bf5ccde47193c10bb35ae2132c620c052eb Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 7 Oct 2022 16:37:06 +0530 Subject: [PATCH 06/30] MM-47403: Populate the productHooks field (#21280) We need to do this to pass the hooked events to the products. https://mattermost.atlassian.net/browse/MM-47403 ```release-note NONE ``` --- plugin/interface_generator/main.go | 1 + plugin/product_hooks_generated.go | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/plugin/interface_generator/main.go b/plugin/interface_generator/main.go index 763868f52e..0a14a092b9 100644 --- a/plugin/interface_generator/main.go +++ b/plugin/interface_generator/main.go @@ -406,6 +406,7 @@ type hooksAdapter struct { func newAdapter(productHooks any) (*hooksAdapter, error) { a := &hooksAdapter{ implemented: make(map[int]struct{}), + productHooks: productHooks, } var tt reflect.Type ft := reflect.TypeOf(productHooks) diff --git a/plugin/product_hooks_generated.go b/plugin/product_hooks_generated.go index 2431113115..584f6bed95 100644 --- a/plugin/product_hooks_generated.go +++ b/plugin/product_hooks_generated.go @@ -121,7 +121,8 @@ type hooksAdapter struct { func newAdapter(productHooks any) (*hooksAdapter, error) { a := &hooksAdapter{ - implemented: make(map[int]struct{}), + implemented: make(map[int]struct{}), + productHooks: productHooks, } var tt reflect.Type ft := reflect.TypeOf(productHooks) From 6e9b808efd7f6ded8d4de20bac98e087d82e0252 Mon Sep 17 00:00:00 2001 From: Vishal Date: Fri, 7 Oct 2022 18:32:11 +0530 Subject: [PATCH 07/30] [MM-46644] Auto respond message (#20900) Automatic Merge --- model/utils.go | 25 +++++++++++++++++++++++++ store/sqlstore/user_store.go | 27 +++++++++++++++++++++++++++ store/storetest/user_store.go | 24 +++++++++++++++++++++++- 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/model/utils.go b/model/utils.go index 75e34c7d29..a8d82e3669 100644 --- a/model/utils.go +++ b/model/utils.go @@ -34,8 +34,11 @@ const ( SYMBOLS = " !\"\\#$%&'()*+,-./:;<=>?@[]^_`|~" BinaryParamKey = "MM_BINARY_PARAMETERS" NoTranslation = "" + maxPropSizeBytes = 1024 * 1024 ) +var ErrMaxPropSizeExceeded = fmt.Errorf("max prop size of %d exceeded", maxPropSizeBytes) + type StringInterface map[string]any type StringArray []string @@ -77,6 +80,14 @@ func (sa StringArray) Equals(input StringArray) bool { // Value converts StringArray to database value func (sa StringArray) Value() (driver.Value, error) { + sz := 0 + for i := range sa { + sz += len(sa[i]) + if sz > maxPropSizeBytes { + return nil, ErrMaxPropSizeExceeded + } + } + j, err := json.Marshal(sa) if err != nil { return nil, err @@ -127,6 +138,15 @@ func (m *StringMap) Scan(value any) error { func (m StringMap) Value() (driver.Value, error) { ok := m[BinaryParamKey] delete(m, BinaryParamKey) + + sz := 0 + for k := range m { + sz += len(k) + len(m[k]) + if sz > maxPropSizeBytes { + return nil, ErrMaxPropSizeExceeded + } + } + buf, err := json.Marshal(m) if err != nil { return nil, err @@ -182,6 +202,11 @@ func (si StringInterface) Value() (driver.Value, error) { if err != nil { return nil, err } + + if len(j) > maxPropSizeBytes { + return nil, ErrMaxPropSizeExceeded + } + // non utf8 characters are not supported https://mattermost.atlassian.net/browse/MM-41066 return string(j), err } diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 331061792a..0f724405d2 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -10,6 +10,7 @@ import ( "fmt" "sort" "strings" + "unicode/utf8" sq "github.com/mattermost/squirrel" "github.com/pkg/errors" @@ -17,6 +18,7 @@ import ( "github.com/mattermost/mattermost-server/v6/einterfaces" "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/shared/mlog" "github.com/mattermost/mattermost-server/v6/store" ) @@ -59,7 +61,24 @@ func newSqlUserStore(sqlStore *SqlStore, metrics einterfaces.MetricsInterface) s return us } +func (us SqlUserStore) validateAutoResponderMessageSize(notifyProps model.StringMap) error { + if notifyProps != nil { + maxPostSize := us.Post().GetMaxPostSize() + msg := notifyProps[model.AutoResponderMessageNotifyProp] + msgSize := utf8.RuneCountInString(msg) + if msgSize > maxPostSize { + mlog.Warn("auto_responder_message has size restrictions", mlog.Int("max_characters", maxPostSize), mlog.Int("received_size", msgSize)) + return errors.New("Auto responder message size can't be more than the allowed Post size") + } + } + return nil +} + func (us SqlUserStore) insert(user *model.User) (sql.Result, error) { + if err := us.validateAutoResponderMessageSize(user.NotifyProps); err != nil { + return nil, err + } + query := `INSERT INTO Users (Id, CreateAt, UpdateAt, DeleteAt, Username, Password, AuthData, AuthService, Email, EmailVerified, Nickname, FirstName, LastName, Position, Roles, AllowMarketing, @@ -150,6 +169,10 @@ func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model. return nil, err } + if err := us.validateAutoResponderMessageSize(user.NotifyProps); err != nil { + return nil, err + } + oldUser := model.User{} err := us.GetMasterX().Get(&oldUser, "SELECT * FROM Users WHERE Id=?", user.Id) if err != nil { @@ -228,6 +251,10 @@ func (us SqlUserStore) Update(user *model.User, trustedUpdateData bool) (*model. } func (us SqlUserStore) UpdateNotifyProps(userID string, props map[string]string) error { + if err := us.validateAutoResponderMessageSize(props); err != nil { + return err + } + buf, err := json.Marshal(props) if err != nil { return errors.Wrap(err, "failed marshalling session props") diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index db7ca1db78..98d6f2bc4f 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -130,7 +130,17 @@ func testUserStoreSave(t *testing.T, ss store.Store) { u2.Username = "" _, err = ss.User().Save(&u2) - require.Error(t, err, "should be unique username") + require.Error(t, err, "should be non-empty username") + + u3 := model.User{ + Email: MakeEmail(), + Username: model.NewId(), + NotifyProps: make(map[string]string, 1), + } + maxPostSize := ss.Post().GetMaxPostSize() + u3.NotifyProps[model.AutoResponderMessageNotifyProp] = strings.Repeat("a", maxPostSize+1) + _, err = ss.User().Save(&u3) + require.Error(t, err, "auto responder message size should not be greater than maxPostSize") for i := 0; i < 49; i++ { u := model.User{ @@ -234,6 +244,18 @@ func testUserStoreUpdate(t *testing.T, ss store.Store) { uNew, err := ss.User().Get(context.Background(), u1.Id) require.NoError(t, err) assert.Equal(t, props, uNew.NotifyProps) + + u4 := model.User{ + Email: MakeEmail(), + Username: model.NewId(), + NotifyProps: make(map[string]string, 1), + } + maxPostSize := ss.Post().GetMaxPostSize() + u4.NotifyProps[model.AutoResponderMessageNotifyProp] = strings.Repeat("a", maxPostSize+1) + _, err = ss.User().Update(&u4, false) + require.Error(t, err, "auto responder message size should not be greater than maxPostSize") + err = ss.User().UpdateNotifyProps(u4.Id, u4.NotifyProps) + require.Error(t, err, "auto responder message size should not be greater than maxPostSize") } func testUserStoreUpdateUpdateAt(t *testing.T, ss store.Store) { From 9a804a96fe293fea0c709bcb1569f2737785412b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20Andr=C3=A9s=20V=C3=A9lez=20Vidal?= Date: Fri, 7 Oct 2022 15:57:44 +0200 Subject: [PATCH 08/30] =?UTF-8?q?MM-47228=20-=20restrict=20guest=20invitat?= =?UTF-8?q?ion=20flow=20if=20subscription=20plan=20does=20n=E2=80=A6=20(#2?= =?UTF-8?q?1195)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MM-47228 - restrict guest invitation flow if subscription plan does not support it * fix i18n texts and add unit test * test the scenario where guest invites are blocked by subscription * cover the success scenarios for cloud free trial and paid subscription * fix go vet * use the cloud prefix for the sku * fix unit tests * check the licence value to determine if the guest accounts are enabled * remove unnecessary changes for getting the subscription information * restrict user demotion if guestAccounts is not available in license Co-authored-by: Pablo Velez Vidal --- api4/team.go | 11 +++++++++-- api4/team_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++++ api4/user.go | 7 +++++++ api4/user_test.go | 20 ++++++++++++++++++++ i18n/en.json | 16 ++++++++-------- model/license.go | 19 +++++++++++++++++++ 6 files changed, 109 insertions(+), 10 deletions(-) diff --git a/api4/team.go b/api4/team.go index 5ee8a0d3da..6ad6c9b52b 100644 --- a/api4/team.go +++ b/api4/team.go @@ -1462,12 +1462,12 @@ func inviteUsersToTeam(c *Context, w http.ResponseWriter, r *http.Request) { func inviteGuestsToChannels(c *Context, w http.ResponseWriter, r *http.Request) { graceful := r.URL.Query().Get("graceful") != "" if c.App.Channels().License() == nil { - c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invate_guests_to_channels.license.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invite_guests_to_channels.license.error", nil, "", http.StatusNotImplemented) return } if !*c.App.Config().GuestAccountsSettings.Enable { - c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invate_guests_to_channels.disabled.error", nil, "", http.StatusNotImplemented) + c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusNotImplemented) return } @@ -1485,6 +1485,13 @@ func inviteGuestsToChannels(c *Context, w http.ResponseWriter, r *http.Request) return } + guestEnabled := c.App.Channels().License() != nil && *c.App.Channels().License().Features.GuestAccounts + + if !guestEnabled { + c.Err = model.NewAppError("Api4.InviteGuestsToChannels", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) + return + } + var guestsInvite model.GuestsInvite if err := json.NewDecoder(r.Body).Decode(&guestsInvite); err != nil { c.Err = model.NewAppError("Api4.inviteGuestsToChannels", "api.team.invite_guests_to_channels.invalid_body.app_error", nil, "", http.StatusBadRequest).Wrap(err) diff --git a/api4/team_test.go b/api4/team_test.go index a476f17903..04cca44f71 100644 --- a/api4/team_test.go +++ b/api4/team_test.go @@ -3361,6 +3361,52 @@ func TestInviteGuestsToTeam(t *testing.T) { }) } +func TestInviteGuest(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + guest1 := th.GenerateTestEmail() + guest2 := th.GenerateTestEmail() + + emailList := []string{guest1, guest2} + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.Enable = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableEmailInvitations = true }) + + t.Run("Guest Account not available in license returns forbidden", func(t *testing.T) { + th.App.Srv().SetLicense(model.NewTestLicenseWithFalseDefaults("guest_accounts")) + + guestsInvite := model.GuestsInvite{ + Emails: emailList, + Channels: []string{th.BasicChannel.Id}, + Message: "test message", + } + buf, err := json.Marshal(guestsInvite) + require.NoError(t, err) + + res, err := th.SystemAdminClient.DoAPIPost("/teams/"+th.BasicTeam.Id+"/invite-guests/email", string(buf)) + + require.Equal(t, http.StatusForbidden, res.StatusCode) + require.True(t, strings.Contains(err.Error(), "Guest accounts are disabled")) + require.Error(t, err) + }) + + t.Run("Guest Account available in license returns OK", func(t *testing.T) { + th.App.Srv().SetLicense(model.NewTestLicense("guest_accounts")) + + guestsInvite := model.GuestsInvite{ + Emails: emailList, + Channels: []string{th.BasicChannel.Id}, + Message: "test message", + } + buf, err := json.Marshal(guestsInvite) + require.NoError(t, err) + + res, err := th.SystemAdminClient.DoAPIPost("/teams/"+th.BasicTeam.Id+"/invite-guests/email", string(buf)) + + require.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, err) + }) +} + func TestGetTeamInviteInfo(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() diff --git a/api4/user.go b/api4/user.go index 0cd58f56fb..423edc755c 100644 --- a/api4/user.go +++ b/api4/user.go @@ -2699,6 +2699,13 @@ func demoteUserToGuest(c *Context, w http.ResponseWriter, r *http.Request) { return } + guestEnabled := c.App.Channels().License() != nil && *c.App.Channels().License().Features.GuestAccounts + + if !guestEnabled { + c.Err = model.NewAppError("Api4.demoteUserToGuest", "api.team.invite_guests_to_channels.disabled.error", nil, "", http.StatusForbidden) + return + } + auditRec := c.MakeAuditRecord("demoteUserToGuest", audit.Fail) auditRec.AddEventParameter("user_id", c.Params.UserId) defer c.LogAuditRec(auditRec) diff --git a/api4/user_test.go b/api4/user_test.go index ba5eaa70a2..7f937fa7fc 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -5204,6 +5204,26 @@ func TestDemoteUserToGuest(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicense()) user := th.BasicUser + user2 := th.BasicUser2 + + t.Run("Guest Account not available in license returns forbidden", func(t *testing.T) { + th.App.Srv().SetLicense(model.NewTestLicenseWithFalseDefaults("guest_accounts")) + + res, err := th.SystemAdminClient.DoAPIPost("/users/"+user2.Id+"/demote", "") + + require.Equal(t, http.StatusForbidden, res.StatusCode) + require.True(t, strings.Contains(err.Error(), "Guest accounts are disabled")) + require.Error(t, err) + }) + + t.Run("Guest Account available in license returns OK", func(t *testing.T) { + th.App.Srv().SetLicense(model.NewTestLicense("guest_accounts")) + + res, err := th.SystemAdminClient.DoAPIPost("/users/"+user2.Id+"/demote", "") + + require.Equal(t, http.StatusOK, res.StatusCode) + require.NoError(t, err) + }) th.TestForSystemAdminAndLocal(t, func(t *testing.T, c *model.Client4) { _, _, err := c.GetUser(user.Id, "") diff --git a/i18n/en.json b/i18n/en.json index c2b52f87a3..539746b078 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2975,22 +2975,22 @@ "id": "api.team.invalidate_all_email_invites.app_error", "translation": "Error invalidating email invites." }, - { - "id": "api.team.invate_guests_to_channels.disabled.error", - "translation": "Guest accounts are disabled" - }, - { - "id": "api.team.invate_guests_to_channels.license.error", - "translation": "Your license does not support guest accounts" - }, { "id": "api.team.invite_guests.channel_in_invalid_team.app_error", "translation": "The channels of the invite must be part of the team of the invite." }, + { + "id": "api.team.invite_guests_to_channels.disabled.error", + "translation": "Guest accounts are disabled" + }, { "id": "api.team.invite_guests_to_channels.invalid_body.app_error", "translation": "Invalid or missing request body." }, + { + "id": "api.team.invite_guests_to_channels.license.error", + "translation": "Your license does not support guest accounts" + }, { "id": "api.team.invite_members.disabled.app_error", "translation": "Email invitations are disabled." diff --git a/model/license.go b/model/license.go index 5fc844e7af..846ce68558 100644 --- a/model/license.go +++ b/model/license.go @@ -326,6 +326,25 @@ func NewTestLicense(features ...string) *License { return ret } +// NewTestLicense returns a license that expires in the future and set as false the given features. +func NewTestLicenseWithFalseDefaults(features ...string) *License { + ret := &License{ + ExpiresAt: GetMillis() + 90*DayInMilliseconds, + Customer: &Customer{}, + Features: &Features{}, + } + ret.Features.SetDefaults() + + featureMap := map[string]bool{} + for _, feature := range features { + featureMap[feature] = false + } + featureJson, _ := json.Marshal(featureMap) + json.Unmarshal(featureJson, &ret.Features) + + return ret +} + func NewTestLicenseSKU(skuShortName string, features ...string) *License { lic := NewTestLicense(features...) lic.SkuShortName = skuShortName From a28183b07871d00527475951b9b6368814436f78 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Fri, 7 Oct 2022 10:48:50 -0600 Subject: [PATCH 09/30] update prepackaged to v7.4.1 (#21288) --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 032fe37f3b..80b0d047cd 100644 --- a/Makefile +++ b/Makefile @@ -160,7 +160,7 @@ PLUGIN_PACKAGES += mattermost-plugin-jira-v2.4.0 PLUGIN_PACKAGES += mattermost-plugin-nps-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-zoom-v1.6.0 -PLUGIN_PACKAGES += focalboard-v7.4.0 +PLUGIN_PACKAGES += focalboard-v7.4.1 PLUGIN_PACKAGES += mattermost-plugin-apps-v1.1.0 # Prepares the enterprise build if exists. The IGNORE stuff is a hack to get the Makefile to execute the commands outside a target From 9bf94bf6c1bcdc7d0dcb9c36b58bd98585b81585 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Sat, 8 Oct 2022 10:21:22 +0530 Subject: [PATCH 10/30] MM-47249: Blocklist focalboard plugin in product mode (#21243) We prevent the plugin from starting if in product mode, and also give pretty errors to make it clear to the users. https://mattermost.atlassian.net/browse/MM-47249 ```release-note NONE ``` Co-authored-by: Mattermod --- api4/config.go | 5 +++++ app/plugin.go | 21 +++++++++++++++++++++ i18n/en.json | 4 ++++ plugin/environment.go | 6 +++--- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/api4/config.go b/api4/config.go index 91e62862a4..a1f4bb3243 100644 --- a/api4/config.go +++ b/api4/config.go @@ -156,6 +156,11 @@ func updateConfig(c *Context, w http.ResponseWriter, r *http.Request) { *cfg.PluginSettings.MarketplaceURL = *appCfg.PluginSettings.MarketplaceURL } + if cfg.PluginSettings.PluginStates[model.PluginIdFocalboard].Enable && cfg.FeatureFlags.BoardsProduct { + c.Err = model.NewAppError("EnablePlugin", "app.plugin.product_mode.app_error", map[string]any{"Name": model.PluginIdFocalboard}, "", http.StatusInternalServerError) + return + } + if appErr := c.App.CheckFreemiumLimitsForConfigSave(appCfg, cfg); appErr != nil { c.Err = appErr return diff --git a/app/plugin.go b/app/plugin.go index 50b750f11c..b8c1898999 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -130,6 +130,23 @@ func (ch *Channels) syncPluginsActiveState() { } if pluginEnabled { + // Disable focalboard in product mode. + if pluginID == model.PluginIdFocalboard && ch.cfgSvc.Config().FeatureFlags.BoardsProduct { + msg := "Plugin cannot run in product mode. Disabling." + mlog.Warn(msg, mlog.String("plugin_id", model.PluginIdFocalboard)) + + // This is a mini-version of ch.disablePlugin. + // We don't call that directly, because that will recursively call + // this method. + ch.cfgSvc.UpdateConfig(func(cfg *model.Config) { + cfg.PluginSettings.PluginStates[pluginID] = &model.PluginState{Enable: false} + }) + pluginsEnvironment.SetPluginError(pluginID, msg) + ch.unregisterPluginCommands(pluginID) + disabledPlugins = append(disabledPlugins, plugin) + continue + } + enabledPlugins = append(enabledPlugins, plugin) } else { disabledPlugins = append(disabledPlugins, plugin) @@ -438,6 +455,10 @@ func (ch *Channels) enablePlugin(id string) *model.AppError { return model.NewAppError("EnablePlugin", "app.plugin.not_installed.app_error", nil, "", http.StatusNotFound) } + if id == model.PluginIdFocalboard && ch.cfgSvc.Config().FeatureFlags.BoardsProduct { + return model.NewAppError("EnablePlugin", "app.plugin.product_mode.app_error", map[string]any{"Name": model.PluginIdFocalboard}, "", http.StatusInternalServerError) + } + ch.cfgSvc.UpdateConfig(func(cfg *model.Config) { cfg.PluginSettings.PluginStates[id] = &model.PluginState{Enable: true} }) diff --git a/i18n/en.json b/i18n/en.json index 539746b078..5ebcc304a4 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5915,6 +5915,10 @@ "id": "app.plugin.not_installed.app_error", "translation": "Plugin is not installed." }, + { + "id": "app.plugin.product_mode.app_error", + "translation": "Plugin {{.Name}} cannot be enabled in product mode." + }, { "id": "app.plugin.remove.app_error", "translation": "Unable to delete plugin." diff --git a/plugin/environment.go b/plugin/environment.go index 9226a2e878..fa737f5ecf 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -137,7 +137,7 @@ func (env *Environment) IsActive(id string) bool { return env.GetPluginState(id) == model.PluginStateRunning } -func (env *Environment) setPluginError(id string, err string) { +func (env *Environment) SetPluginError(id string, err string) { if rp, ok := env.registeredPlugins.Load(id); ok { p := rp.(registeredPlugin) p.Error = err @@ -233,9 +233,9 @@ func (env *Environment) GetManifest(pluginId string) (*model.Manifest, error) { func (env *Environment) Activate(id string) (manifest *model.Manifest, activated bool, reterr error) { defer func() { if reterr != nil { - env.setPluginError(id, reterr.Error()) + env.SetPluginError(id, reterr.Error()) } else { - env.setPluginError(id, "") + env.SetPluginError(id, "") } }() From 542c36bd6804d074e789aa3c5d02a165a64bfd6f Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Sun, 11 Sep 2022 12:08:30 +0300 Subject: [PATCH 11/30] new ui for email verify change email address --- app/email/email.go | 4 + i18n/en.json | 2 +- templates/email_change_verify_body.html | 231 ++++++++++++++++++++---- 3 files changed, 199 insertions(+), 38 deletions(-) diff --git a/app/email/email.go b/app/email/email.go index b9057800fa..d9f3882c61 100644 --- a/app/email/email.go +++ b/app/email/email.go @@ -69,6 +69,10 @@ func (es *Service) SendEmailChangeVerifyEmail(newUserEmail, locale, siteURL, tok map[string]any{"TeamDisplayName": es.config().TeamSettings.SiteName}) data.Props["VerifyUrl"] = link data.Props["VerifyButton"] = T("api.templates.email_change_verify_body.button") + data.Props["QuestionTitle"] = T("api.templates.questions_footer.title") + data.Props["EmailInfo1"] = T("api.templates.email_us_anytime_at") + data.Props["SupportEmail"] = "feedback@mattermost.com" + data.Props["FooterV2"] = T("api.templates.email_footer_v2") body, err := es.templatesContainer.RenderToString("email_change_verify_body", data) if err != nil { diff --git a/i18n/en.json b/i18n/en.json index 5ebcc304a4..4a13acd640 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -3445,7 +3445,7 @@ }, { "id": "api.templates.email_change_verify_body.title", - "translation": "You updated your email" + "translation": "You successfully updated your email" }, { "id": "api.templates.email_change_verify_subject", diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index 0babf28737..7febc26063 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -1,46 +1,203 @@ {{define "email_change_verify_body"}} - - - - + + +
- - - - -
- + + + + + + + +
+
+
+ + + - -
+
+ + - - - - - - {{template "email_footer" . }} - +
- + +
+ + + + + + +
+ + + + + + +
+ +
+
+
- - - - - - {{template "email_info" . }} - -
-

{{.Props.Title}}

-

{{.Props.Info}}

-

- {{.Props.VerifyButton}} -

-
-
-
-
+ +
+ + + + + + +
+
+ + + + + + + + + + + + +
+

{{.Props.Title}}

+
+
{{.Props.Info}}
+
+ +
+
+
+
+
+ + + + + + +
+
+ + + + + + +
+ + + + + + +
+ +
+
+
+
+
+
+ + + + + + +
+
+ + + + + + +
+

{{.Props.QuestionTitle}}

+

{{.Props.EmailInfo1}}{{.Props.SupportEmail}}

+
+
+
+
+
+ + + + + + +
+
+ + + + + + +
+

+ {{.Props.FooterV2}} +

+
+
+
+
+
+ + {{end}} From 979359ade6830579e7e2645c92a9d02a65da44ee Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Mon, 3 Oct 2022 17:07:19 +0300 Subject: [PATCH 12/30] [MM-44790] fix the external link warning --- templates/email_change_verify_body.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index 7febc26063..d2b9d318ca 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -101,7 +101,7 @@ From affddb65dabfaced1dd1c4343b8678b22b6939e9 Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Mon, 3 Oct 2022 17:27:33 +0300 Subject: [PATCH 13/30] [MM-44790] add additional font family for consitency --- templates/email_change_verify_body.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index d2b9d318ca..a98286e8b0 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -157,8 +157,8 @@ From 6cc85b8350fc4c6900162ebc778e6865424cb21f Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Mon, 3 Oct 2022 18:02:46 +0300 Subject: [PATCH 14/30] [MM-44790] removing extra quotes around font-family --- templates/email_change_verify_body.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index a98286e8b0..0b171b98ca 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -157,8 +157,8 @@ From 9f6497fb0edccbe9d3719fa9cdd999fa1c9295a6 Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Tue, 11 Oct 2022 12:46:10 +0300 Subject: [PATCH 15/30] add metropolis font to email change verify email html --- templates/email_change_verify_body.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index 0b171b98ca..8f6918bfaf 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -2,6 +2,7 @@ + @@ -87,7 +88,7 @@ From f72540c68f78c96dd3892ac82c4c61ce2b6856c3 Mon Sep 17 00:00:00 2001 From: Amy Blais <29708087+amyblais@users.noreply.github.com> Date: Tue, 11 Oct 2022 09:02:11 -0400 Subject: [PATCH 16/30] Update supported versions (#21143) Automatic Merge --- i18n/en.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/i18n/en.json b/i18n/en.json index 5ebcc304a4..0d164e18b0 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -9473,11 +9473,11 @@ }, { "id": "web.error.unsupported_browser.min_browser_version.chrome", - "translation": "Version 100+" + "translation": "Version 102+" }, { "id": "web.error.unsupported_browser.min_browser_version.edge", - "translation": "Version 44+" + "translation": "Version 95+" }, { "id": "web.error.unsupported_browser.min_browser_version.firefox", From 79651874ea43d4ef73c278b7bb19c9cfb0d30002 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Tue, 11 Oct 2022 15:33:29 +0200 Subject: [PATCH 17/30] Do not send categories for teams deleted or not member of (#21038) * Do not send categories for teams deleted or not member of * Fix tests and add test for deleted teams and memberships --- store/sqlstore/channel_store_categories.go | 7 + store/storetest/channel_store_categories.go | 457 ++++++++++++-------- 2 files changed, 282 insertions(+), 182 deletions(-) diff --git a/store/sqlstore/channel_store_categories.go b/store/sqlstore/channel_store_categories.go index 2a1799b01a..27317fe3cf 100644 --- a/store/sqlstore/channel_store_categories.go +++ b/store/sqlstore/channel_store_categories.go @@ -535,6 +535,13 @@ func (s SqlChannelStore) getSidebarCategoriesT(db dbSelecter, userId string, opt Select("SidebarCategories.*", "SidebarChannels.ChannelId"). From("SidebarCategories"). LeftJoin("SidebarChannels ON SidebarChannels.CategoryId=Id"). + InnerJoin("Teams ON Teams.Id=SidebarCategories.TeamId"). + InnerJoin("TeamMembers ON TeamMembers.TeamId=SidebarCategories.TeamId"). + Where(sq.And{ + sq.Eq{"TeamMembers.UserId": userId}, + sq.Eq{"TeamMembers.DeleteAt": 0}, + sq.Eq{"Teams.DeleteAt": 0}, + }). Where(sq.And{ sq.Eq{"SidebarCategories.UserId": userId}, }). diff --git a/store/storetest/channel_store_categories.go b/store/storetest/channel_store_categories.go index faaa0dd75c..5b8e47bab2 100644 --- a/store/storetest/channel_store_categories.go +++ b/store/storetest/channel_store_categories.go @@ -28,13 +28,38 @@ func TestChannelStoreCategories(t *testing.T, ss store.Store, s SqlStore) { t.Run("SidebarCategoryDeadlock", func(t *testing.T) { testSidebarCategoryDeadlock(t, ss) }) } +func setupTeam(t *testing.T, ss store.Store, userIds ...string) *model.Team { + team, err := ss.Team().Save(&model.Team{ + DisplayName: "Name", + Name: NewTestId(), + Email: MakeEmail(), + Type: model.TeamOpen, + }) + assert.NoError(t, err) + + members := make([]*model.TeamMember, 0, len(userIds)) + for _, userId := range userIds { + members = append(members, &model.TeamMember{ + TeamId: team.Id, + UserId: userId, + }) + } + if len(members) > 0 { + _, err = ss.Team().SaveMultipleMembers(members, len(userIds)+1) + assert.NoError(t, err) + } + + return team +} + func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { t.Run("should create initial favorites/channels/DMs categories", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } @@ -45,25 +70,25 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, model.SidebarCategoryChannels, res.Categories[1].Type) assert.Equal(t, model.SidebarCategoryDirectMessages, res.Categories[2].Type) - res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) assert.NoError(t, err) assert.Equal(t, res, res2) }) t.Run("should create initial favorites/channels/DMs categories for multiple users", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + userId2 := model.NewId() + + team := setupTeam(t, ss, userId, userId2) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - userId2 := model.NewId() - res, nErr = ss.Channel().CreateInitialSidebarCategories(userId2, opts) assert.NoError(t, nErr) assert.Len(t, res.Categories, 3) @@ -71,26 +96,27 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, model.SidebarCategoryChannels, res.Categories[1].Type) assert.Equal(t, model.SidebarCategoryDirectMessages, res.Categories[2].Type) - res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId2, teamId) + res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId2, team.Id) assert.NoError(t, err) assert.Equal(t, res, res2) }) t.Run("should create initial favorites/channels/DMs categories on different teams", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) + team2 := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - teamId2 := model.NewId() opts = &store.SidebarCategorySearchOpts{ - TeamID: teamId2, + TeamID: team2.Id, ExcludeTeam: false, } res, nErr = ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -100,24 +126,25 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, model.SidebarCategoryChannels, res.Categories[1].Type) assert.Equal(t, model.SidebarCategoryDirectMessages, res.Categories[2].Type) - res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId2) + res2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team2.Id) assert.NoError(t, err) assert.Equal(t, res, res2) }) t.Run("shouldn't create additional categories when ones already exist", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, res, initialCategories) @@ -126,14 +153,15 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.NoError(t, nErr) assert.NotEmpty(t, res) - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) assert.NoError(t, err) assert.Equal(t, initialCategories.Categories, res.Categories) }) t.Run("shouldn't create additional categories when ones already exist even when ran simultaneously", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) var wg sync.WaitGroup @@ -144,7 +172,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { defer wg.Done() opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } _, _ = ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -153,18 +181,19 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { wg.Wait() - res, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) assert.NoError(t, err) assert.Len(t, res.Categories, 3) }) t.Run("should populate the Favorites category with regular channels", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) // Set up two channels, one favorited and one not channel1, nErr := ss.Channel().Save(&model.Channel{ - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, Name: "channel1", }, 1000) @@ -177,7 +206,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, err) channel2, nErr := ss.Channel().Save(&model.Channel{ - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, Name: "channel2", }, 1000) @@ -201,7 +230,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { // Create the categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } categories, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -213,18 +242,19 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, []string{channel2.Id}, categories.Categories[1].Channels) // Get and check the categories for channels - categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) require.Equal(t, categories, categories2) }) t.Run("should populate the Favorites category in alphabetical order", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) // Set up two channels channel1, nErr := ss.Channel().Save(&model.Channel{ - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, Name: "channel1", DisplayName: "zebra", @@ -238,7 +268,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, err) channel2, nErr := ss.Channel().Save(&model.Channel{ - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, Name: "channel2", DisplayName: "aardvark", @@ -269,7 +299,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { // Create the categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } categories, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -279,14 +309,15 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, []string{channel2.Id, channel1.Id}, categories.Categories[0].Channels) // Get and check the categories for channels - categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) require.Equal(t, categories, categories2) }) t.Run("should populate the Favorites category with DMs and GMs", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) otherUserId1 := model.NewId() otherUserId2 := model.NewId() @@ -336,7 +367,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { // Create the categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } categories, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -348,19 +379,20 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, []string{dmChannel2.Id}, categories.Categories[2].Channels) // Get and check the categories for channels - categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, categories, categories2) }) t.Run("should not populate the Favorites category with channels from other teams", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() - teamId2 := model.NewId() + + team := setupTeam(t, ss, userId) + team2 := setupTeam(t, ss, userId) // Set up a channel on another team and favorite it channel1, nErr := ss.Channel().Save(&model.Channel{ - TeamId: teamId2, + TeamId: team2.Id, Type: model.ChannelTypeOpen, Name: "channel1", }, 1000) @@ -384,7 +416,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { // Create the categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } categories, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -396,7 +428,7 @@ func testCreateInitialSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, []string{}, categories.Categories[1].Channels) // Get and check the categories for channels - categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories2, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) require.Equal(t, categories, categories2) }) @@ -465,10 +497,11 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { t.Run("should place the new category second if Favorites comes first", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -476,7 +509,7 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { require.NotEmpty(t, res) // Create the category - created, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + created, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ DisplayName: model.NewId(), }, @@ -484,7 +517,7 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { require.NoError(t, err) // Confirm that it comes second - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, res.Categories, 4) assert.Equal(t, model.SidebarCategoryFavorites, res.Categories[0].Type) @@ -494,10 +527,11 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { t.Run("should place the new category first if Favorites is not first", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -505,12 +539,12 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { require.NotEmpty(t, res) // Re-arrange the categories so that Favorites comes last - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, categories.Categories, 3) require.Equal(t, model.SidebarCategoryFavorites, categories.Categories[0].Type) - err = ss.Channel().UpdateSidebarCategoryOrder(userId, teamId, []string{ + err = ss.Channel().UpdateSidebarCategoryOrder(userId, team.Id, []string{ categories.Categories[1].Id, categories.Categories[2].Id, categories.Categories[0].Id, @@ -518,7 +552,7 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { require.NoError(t, err) // Create the category - created, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + created, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ DisplayName: model.NewId(), }, @@ -526,7 +560,7 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { require.NoError(t, err) // Confirm that it comes first - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, res.Categories, 4) assert.Equal(t, model.SidebarCategoryCustom, res.Categories[0].Type) @@ -535,10 +569,10 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { t.Run("should create the category with its channels", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -548,19 +582,19 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { // Create some channels channel1, err := ss.Channel().Save(&model.Channel{ Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, Name: model.NewId(), }, 100) require.NoError(t, err) channel2, err := ss.Channel().Save(&model.Channel{ Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, Name: model.NewId(), }, 100) require.NoError(t, err) // Create the category - created, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + created, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ DisplayName: model.NewId(), }, @@ -577,17 +611,17 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { t.Run("should remove any channels from their previous categories", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, categories.Categories, 3) @@ -599,13 +633,13 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { // Create some channels channel1, nErr := ss.Channel().Save(&model.Channel{ Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, Name: model.NewId(), }, 100) require.NoError(t, nErr) channel2, nErr := ss.Channel().Save(&model.Channel{ Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, Name: model.NewId(), }, 100) require.NoError(t, nErr) @@ -613,14 +647,14 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { // Assign them to categories favoritesCategory.Channels = []string{channel1.Id} channelsCategory.Channels = []string{channel2.Id} - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ favoritesCategory, channelsCategory, }) require.NoError(t, err) // Create the category - created, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + created, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ DisplayName: model.NewId(), }, @@ -643,14 +677,14 @@ func testCreateSidebarCategory(t *testing.T, ss store.Store) { func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("should return a custom category with its Channels field set", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) channelId1 := model.NewId() channelId2 := model.NewId() channelId3 := model.NewId() opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -658,10 +692,10 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { require.NotEmpty(t, res) // Create a category and assign some channels to it - created, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + created, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ UserId: userId, - TeamId: teamId, + TeamId: team.Id, DisplayName: model.NewId(), }, Channels: []string{channelId1, channelId2, channelId3}, @@ -680,18 +714,18 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("should return any orphaned channels with the Channels category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the channels category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) channelsCategory := categories.Categories[1] @@ -701,7 +735,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { channel1, nErr := ss.Channel().Save(&model.Channel{ Name: "channel1", DisplayName: "DEF", - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypePrivate, }, 10) require.NoError(t, nErr) @@ -715,7 +749,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { channel2, nErr := ss.Channel().Save(&model.Channel{ Name: "channel2", DisplayName: "ABC", - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, }, 10) require.NoError(t, nErr) @@ -748,18 +782,18 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("shouldn't return orphaned channels on another team with the Channels category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the channels category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, model.SidebarCategoryChannels, categories.Categories[1].Type) @@ -791,10 +825,10 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("shouldn't return non-orphaned channels with the Channels category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } // Create the initial categories and find the channels category @@ -802,7 +836,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := categories.Categories[0] @@ -814,7 +848,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { channel1, nErr := ss.Channel().Save(&model.Channel{ Name: "channel1", DisplayName: "DEF", - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypePrivate, }, 10) require.NoError(t, nErr) @@ -828,7 +862,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { channel2, nErr := ss.Channel().Save(&model.Channel{ Name: "channel2", DisplayName: "ABC", - TeamId: teamId, + TeamId: team.Id, Type: model.ChannelTypeOpen, }, 10) require.NoError(t, nErr) @@ -840,7 +874,7 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { require.NoError(t, nErr) // And assign one to another category - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{channel2.Id}, @@ -858,18 +892,18 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("should return any orphaned DM channels with the Direct Messages category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the DMs category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, model.SidebarCategoryDirectMessages, categories.Categories[2].Type) @@ -903,18 +937,18 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("should return any orphaned GM channels with the Direct Messages category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the DMs category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, model.SidebarCategoryDirectMessages, categories.Categories[2].Type) @@ -945,18 +979,18 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { t.Run("should return orphaned DM channels in the DMs category which are in a custom category on another team", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the DMs category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Equal(t, model.SidebarCategoryDirectMessages, categories.Categories[2].Type) @@ -981,19 +1015,19 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { require.NoError(t, nErr) // Create another team and assign the DM to a custom category on that team - otherTeamId := model.NewId() + otherTeam := setupTeam(t, ss, userId) opts = &store.SidebarCategorySearchOpts{ - TeamID: otherTeamId, + TeamID: otherTeam.Id, ExcludeTeam: false, } res, nErr = ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - _, err = ss.Channel().CreateSidebarCategory(userId, otherTeamId, &model.SidebarCategoryWithChannels{ + _, err = ss.Channel().CreateSidebarCategory(userId, otherTeam.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ UserId: userId, - TeamId: teamId, + TeamId: team.Id, }, Channels: []string{dmChannel.Id}, }) @@ -1011,10 +1045,10 @@ func testGetSidebarCategory(t *testing.T, ss store.Store, s SqlStore) { func testGetSidebarCategories(t *testing.T, ss store.Store) { t.Run("should return channels in the same order between different ways of getting categories", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -1027,7 +1061,7 @@ func testGetSidebarCategories(t *testing.T, ss store.Store) { model.NewId(), } - newCategory, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + newCategory, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ Channels: channelIds, }) require.NoError(t, err) @@ -1036,7 +1070,7 @@ func testGetSidebarCategories(t *testing.T, ss store.Store) { gotCategory, err := ss.Channel().GetSidebarCategory(newCategory.Id) require.NoError(t, err) - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, res.Categories, 4) @@ -1047,23 +1081,83 @@ func testGetSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, gotCategory.Channels, res.Categories[1].Channels) assert.Equal(t, channelIds, res.Categories[1].Channels) }) + t.Run("should not return categories for teams deleted, or no longer a member", func(t *testing.T) { + userId := model.NewId() + + teamMember1 := setupTeam(t, ss, userId) + teamMember2 := setupTeam(t, ss, userId) + teamDeleted := setupTeam(t, ss, userId) + teamDeleted.DeleteAt = model.GetMillis() + ss.Team().Update(teamDeleted) + teamNotMember := setupTeam(t, ss) + teamDeletedMember := setupTeam(t, ss, userId) + + members, err := ss.Team().GetMembersByIds(teamDeletedMember.Id, []string{userId}, nil) + require.NoError(t, err) + require.NotEmpty(t, members) + member := members[0] + member.DeleteAt = model.GetMillis() + ss.Team().UpdateMember(member) + + teamIds := []string{ + teamMember1.Id, + teamMember2.Id, + teamDeleted.Id, + teamNotMember.Id, + teamDeletedMember.Id, + } + + for _, id := range teamIds { + res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, &store.SidebarCategorySearchOpts{TeamID: id}) + require.NoError(t, nErr) + require.NotEmpty(t, res) + } + + opts := &store.SidebarCategorySearchOpts{ + TeamID: teamMember1.Id, + ExcludeTeam: false, + } + + // Team member and not exclude + res, err := ss.Channel().GetSidebarCategories(userId, opts) + require.NoError(t, err) + assert.Equal(t, 3, len(res.Categories)) + + // No team member and not exclude + opts.TeamID = teamDeleted.Id + res, err = ss.Channel().GetSidebarCategories(userId, opts) + require.NoError(t, err) + assert.Equal(t, 0, len(res.Categories)) + + // No team member and exclude + opts.ExcludeTeam = true + res, err = ss.Channel().GetSidebarCategories(userId, opts) + require.NoError(t, err) + assert.Equal(t, 6, len(res.Categories)) + + // Team member and exclude + opts.TeamID = teamMember1.Id + res, err = ss.Channel().GetSidebarCategories(userId, opts) + require.NoError(t, err) + assert.Equal(t, 3, len(res.Categories)) + }) } func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("ensure the query to update SidebarCategories hasn't been polluted by UpdateSidebarCategoryOrder", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, err := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, err) require.NotEmpty(t, res) - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := initialCategories.Categories[0] @@ -1071,7 +1165,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { dmsCategory := initialCategories.Categories[2] // And then update one of them - updated, _, err := ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updated, _, err := ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ channelsCategory, }) require.NoError(t, err) @@ -1079,7 +1173,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "Channels", updated[0].DisplayName) // And then reorder the categories - err = ss.Channel().UpdateSidebarCategoryOrder(userId, teamId, []string{dmsCategory.Id, favoritesCategory.Id, channelsCategory.Id}) + err = ss.Channel().UpdateSidebarCategoryOrder(userId, team.Id, []string{dmsCategory.Id, favoritesCategory.Id, channelsCategory.Id}) require.NoError(t, err) // Which somehow blanks out stuff because ??? @@ -1090,18 +1184,18 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("categories should be returned in their original order", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, err := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, err) require.NotEmpty(t, res) - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := initialCategories.Categories[0] @@ -1109,7 +1203,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { dmsCategory := initialCategories.Categories[2] // And then update them - updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ favoritesCategory, channelsCategory, dmsCategory, @@ -1122,24 +1216,24 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should silently fail to update read only fields", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := initialCategories.Categories[0] channelsCategory := initialCategories.Categories[1] dmsCategory := initialCategories.Categories[2] - customCategory, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{}) + customCategory, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{}) require.NoError(t, err) categoriesToUpdate := []*model.SidebarCategoryWithChannels{ @@ -1177,7 +1271,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { }, } - updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, teamId, categoriesToUpdate) + updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, team.Id, categoriesToUpdate) assert.NoError(t, err) assert.NotEqual(t, "Favorites", categoriesToUpdate[0].DisplayName) @@ -1192,18 +1286,18 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should add and remove favorites preferences based on the Favorites category", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the favorites category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := categories.Categories[0] @@ -1213,7 +1307,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { channel, nErr := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, }, 10) require.NoError(t, nErr) _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ @@ -1224,7 +1318,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, nErr) // Assign it to favorites - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{channel.Id}, @@ -1241,7 +1335,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { channelsCategory := categories.Categories[1] require.Equal(t, model.SidebarCategoryChannels, channelsCategory.Type) - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{channel.Id}, @@ -1257,18 +1351,18 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should add and remove favorites preferences for DMs", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create the initial categories and find the favorites category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := categories.Categories[0] @@ -1294,7 +1388,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.NoError(t, nErr) // Assign it to favorites - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{dmChannel.Id}, @@ -1311,7 +1405,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { dmsCategory := categories.Categories[2] require.Equal(t, model.SidebarCategoryDirectMessages, dmsCategory.Type) - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: dmsCategory.SidebarCategory, Channels: []string{dmChannel.Id}, @@ -1327,33 +1421,33 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should add and remove favorites preferences, even if the channel is already favorited in preferences", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() - teamId2 := model.NewId() + team := setupTeam(t, ss, userId) + team2 := setupTeam(t, ss, userId) // Create the initial categories and find the favorites categories in each team opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := categories.Categories[0] require.Equal(t, model.SidebarCategoryFavorites, favoritesCategory.Type) opts = &store.SidebarCategorySearchOpts{ - TeamID: teamId2, + TeamID: team2.Id, ExcludeTeam: false, } res, nErr = ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId2) + categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team2.Id) require.NoError(t, err) favoritesCategory2 := categories2.Categories[0] @@ -1379,7 +1473,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.NoError(t, nErr) // Assign it to favorites on the first team. The favorites preference gets set for all teams. - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{dmChannel.Id}, @@ -1393,7 +1487,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "true", res2.Value) // Assign it to favorites on the second team. The favorites preference is already set. - updated, _, err := ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updated, _, err := ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory2.SidebarCategory, Channels: []string{dmChannel.Id}, @@ -1408,7 +1502,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "true", res2.Value) // Remove it from favorites on the first team. This clears the favorites preference for all teams. - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{}, @@ -1421,7 +1515,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Nil(t, res2) // Remove it from favorites on the second team. The favorites preference was already deleted. - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId2, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory2.SidebarCategory, Channels: []string{}, @@ -1436,18 +1530,19 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should not affect other users' favorites preferences", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + userId2 := model.NewId() + team := setupTeam(t, ss, userId, userId2) // Create the initial categories and find the favorites category opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + categories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) favoritesCategory := categories.Categories[0] @@ -1456,13 +1551,11 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.Equal(t, model.SidebarCategoryChannels, channelsCategory.Type) // Create the other users' categories - userId2 := model.NewId() - res, nErr = ss.Channel().CreateInitialSidebarCategories(userId2, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId2, teamId) + categories2, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId2, team.Id) require.NoError(t, err) favoritesCategory2 := categories2.Categories[0] @@ -1474,7 +1567,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { channel, nErr := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, }, 10) require.NoError(t, nErr) _, nErr = ss.Channel().SaveMember(&model.ChannelMember{ @@ -1491,7 +1584,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, nErr) // Have user1 favorite it - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory.SidebarCategory, Channels: []string{channel.Id}, @@ -1513,7 +1606,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Nil(t, res2) // And user2 favorite it - _, _, err = ss.Channel().UpdateSidebarCategories(userId2, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId2, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: favoritesCategory2.SidebarCategory, Channels: []string{channel.Id}, @@ -1536,7 +1629,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "true", res2.Value) // And then user1 unfavorite it - _, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{channel.Id}, @@ -1558,7 +1651,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "true", res2.Value) // And finally user2 favorite it - _, _, err = ss.Channel().UpdateSidebarCategories(userId2, teamId, []*model.SidebarCategoryWithChannels{ + _, _, err = ss.Channel().UpdateSidebarCategories(userId2, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory2.SidebarCategory, Channels: []string{channel.Id}, @@ -1581,13 +1674,13 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("channels removed from Channels or DMs categories should be re-added", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Create some channels channel, nErr := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, }, 10) require.NoError(t, nErr) _, err := ss.Channel().SaveMember(&model.ChannelMember{ @@ -1615,7 +1708,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, nErr) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -1623,7 +1716,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NotEmpty(t, res) // And some categories - initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) channelsCategory := initialCategories.Categories[1] @@ -1644,7 +1737,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { }, } - updatedCategories, _, nErr := ss.Channel().UpdateSidebarCategories(userId, teamId, categoriesToUpdate) + updatedCategories, _, nErr := ss.Channel().UpdateSidebarCategories(userId, team.Id, categoriesToUpdate) assert.NoError(t, nErr) // The channels should still exist in the category because they would otherwise be orphaned @@ -1654,7 +1747,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should be able to move DMs into and out of custom categories", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) otherUserId := model.NewId() dmChannel, nErr := ss.Channel().SaveDirectChannel( @@ -1674,7 +1767,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, nErr) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) @@ -1682,14 +1775,14 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NotEmpty(t, res) // The DM should start in the DMs category - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) dmsCategory := initialCategories.Categories[2] require.Equal(t, []string{dmChannel.Id}, dmsCategory.Channels) // Now move the DM into a custom category - customCategory, err := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{}) + customCategory, err := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{}) require.NoError(t, err) categoriesToUpdate := []*model.SidebarCategoryWithChannels{ @@ -1703,7 +1796,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { }, } - updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, teamId, categoriesToUpdate) + updatedCategories, _, err := ss.Channel().UpdateSidebarCategories(userId, team.Id, categoriesToUpdate) assert.NoError(t, err) assert.Equal(t, dmsCategory.Id, updatedCategories[0].Id) assert.Equal(t, []string{}, updatedCategories[0].Channels) @@ -1730,7 +1823,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { }, } - updatedCategories, _, err = ss.Channel().UpdateSidebarCategories(userId, teamId, categoriesToUpdate) + updatedCategories, _, err = ss.Channel().UpdateSidebarCategories(userId, team.Id, categoriesToUpdate) assert.NoError(t, err) assert.Equal(t, dmsCategory.Id, updatedCategories[0].Id) assert.Equal(t, []string{dmChannel.Id}, updatedCategories[0].Channels) @@ -1748,13 +1841,13 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should successfully move channels between categories", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Join a channel channel, nErr := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, }, 10) require.NoError(t, nErr) _, err := ss.Channel().SaveMember(&model.ChannelMember{ @@ -1766,24 +1859,24 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { // And then create the initial categories so that it includes the channel opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) channelsCategory := initialCategories.Categories[1] require.Equal(t, []string{channel.Id}, channelsCategory.Channels) - customCategory, nErr := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{}) + customCategory, nErr := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{}) require.NoError(t, nErr) // Move the channel one way - updatedCategories, _, nErr := ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updatedCategories, _, nErr := ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{}, @@ -1799,7 +1892,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, []string{channel.Id}, updatedCategories[1].Channels) // And then the other - updatedCategories, _, nErr = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updatedCategories, _, nErr = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{channel.Id}, @@ -1816,13 +1909,13 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { t.Run("should correctly return the original categories that were modified", func(t *testing.T) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) // Join a channel channel, nErr := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamId, + TeamId: team.Id, }, 10) require.NoError(t, nErr) _, err := ss.Channel().SaveMember(&model.ChannelMember{ @@ -1834,20 +1927,20 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { // And then create the initial categories so that Channels includes the channel opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + initialCategories, nErr := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, nErr) channelsCategory := initialCategories.Categories[1] require.Equal(t, []string{channel.Id}, channelsCategory.Channels) - customCategory, nErr := ss.Channel().CreateSidebarCategory(userId, teamId, &model.SidebarCategoryWithChannels{ + customCategory, nErr := ss.Channel().CreateSidebarCategory(userId, team.Id, &model.SidebarCategoryWithChannels{ SidebarCategory: model.SidebarCategory{ DisplayName: "originalName", }, @@ -1855,7 +1948,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { require.NoError(t, nErr) // Rename the custom category - updatedCategories, originalCategories, nErr := ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updatedCategories, originalCategories, nErr := ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: model.SidebarCategory{ Id: customCategory.Id, @@ -1869,7 +1962,7 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { assert.Equal(t, "updatedName", updatedCategories[0].DisplayName) // Move a channel - updatedCategories, originalCategories, nErr = ss.Channel().UpdateSidebarCategories(userId, teamId, []*model.SidebarCategoryWithChannels{ + updatedCategories, originalCategories, nErr = ss.Channel().UpdateSidebarCategories(userId, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{}, @@ -1893,21 +1986,21 @@ func testUpdateSidebarCategories(t *testing.T, ss store.Store) { func setupInitialSidebarCategories(t *testing.T, ss store.Store) (string, string) { userId := model.NewId() - teamId := model.NewId() + team := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId, + TeamID: team.Id, ExcludeTeam: false, } res, nErr := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, nErr) require.NotEmpty(t, res) - res, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId) + res, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team.Id) require.NoError(t, err) require.Len(t, res.Categories, 3) - return userId, teamId + return userId, team.Id } func testClearSidebarOnTeamLeave(t *testing.T, ss store.Store, s SqlStore) { @@ -2020,17 +2113,17 @@ func testClearSidebarOnTeamLeave(t *testing.T, ss store.Store, s SqlStore) { } // Create a second team and set up the sidebar categories for it - teamId2 := model.NewId() + team2 := setupTeam(t, ss, userId) opts := &store.SidebarCategorySearchOpts{ - TeamID: teamId2, + TeamID: team2.Id, ExcludeTeam: false, } res, err := ss.Channel().CreateInitialSidebarCategories(userId, opts) require.NoError(t, err) require.NotEmpty(t, res) - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId2) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team2.Id) require.NoError(t, err) require.Len(t, res.Categories, 3) @@ -2053,12 +2146,12 @@ func testClearSidebarOnTeamLeave(t *testing.T, ss store.Store, s SqlStore) { // Do the same on the second team channel2, nErr := ss.Channel().Save(&model.Channel{ Name: model.NewId(), - TeamId: teamId2, + TeamId: team2.Id, Type: model.ChannelTypeOpen, }, 1000) require.NoError(t, nErr) - _, err = ss.Channel().CreateSidebarCategory(userId, teamId2, &model.SidebarCategoryWithChannels{ + _, err = ss.Channel().CreateSidebarCategory(userId, team2.Id, &model.SidebarCategoryWithChannels{ Channels: []string{channel2.Id, dmChannel1.Id}, }) require.NoError(t, err) @@ -2087,7 +2180,7 @@ func testClearSidebarOnTeamLeave(t *testing.T, ss store.Store, s SqlStore) { assert.Equal(t, int64(2), count) // Confirm that the categories on the second team are unchanged - res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, teamId2) + res, err = ss.Channel().GetSidebarCategoriesForTeamForUser(userId, team2.Id) require.NoError(t, err) assert.Len(t, res.Categories, 4) @@ -2267,13 +2360,13 @@ func testUpdateSidebarChannelsByPreferences(t *testing.T, ss store.Store) { // to catch a bug. func testSidebarCategoryDeadlock(t *testing.T, ss store.Store) { userID := model.NewId() - teamID := model.NewId() + team := setupTeam(t, ss, userID) // Join a channel channel, err := ss.Channel().Save(&model.Channel{ Name: "channel", Type: model.ChannelTypeOpen, - TeamId: teamID, + TeamId: team.Id, }, 10) require.NoError(t, err) _, err = ss.Channel().SaveMember(&model.ChannelMember{ @@ -2285,20 +2378,20 @@ func testSidebarCategoryDeadlock(t *testing.T, ss store.Store) { // And then create the initial categories so that it includes the channel opts := &store.SidebarCategorySearchOpts{ - TeamID: teamID, + TeamID: team.Id, ExcludeTeam: false, } res, err := ss.Channel().CreateInitialSidebarCategories(userID, opts) require.NoError(t, err) require.NotEmpty(t, res) - initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userID, teamID) + initialCategories, err := ss.Channel().GetSidebarCategoriesForTeamForUser(userID, team.Id) require.NoError(t, err) channelsCategory := initialCategories.Categories[1] require.Equal(t, []string{channel.Id}, channelsCategory.Channels) - customCategory, err := ss.Channel().CreateSidebarCategory(userID, teamID, &model.SidebarCategoryWithChannels{}) + customCategory, err := ss.Channel().CreateSidebarCategory(userID, team.Id, &model.SidebarCategoryWithChannels{}) require.NoError(t, err) var wg sync.WaitGroup @@ -2306,7 +2399,7 @@ func testSidebarCategoryDeadlock(t *testing.T, ss store.Store) { go func() { defer wg.Done() - _, _, err := ss.Channel().UpdateSidebarCategories(userID, teamID, []*model.SidebarCategoryWithChannels{ + _, _, err := ss.Channel().UpdateSidebarCategories(userID, team.Id, []*model.SidebarCategoryWithChannels{ { SidebarCategory: channelsCategory.SidebarCategory, Channels: []string{}, From 279c448da35b982500f57b0b93c971ad6f5acb3e Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 11 Oct 2022 19:04:09 +0530 Subject: [PATCH 18/30] MM-36295: Do not convert thumbnails/previews to jpeg for png images (#21230) We were applying a white background to transparent images and converting them to jpegs. This was to make text be legible behind a black preview background. However, this has led to a poor user experience, as users rarely download the full image but always click on previews. Therefore, we need the previews to remain as pngs. To fix this, we just re-encode them as pngs instead of jpgs. ```release-note NONE ``` --- api4/file_test.go | 16 +++-- app/file.go | 102 ++++++++++++++++------------ app/file_test.go | 2 +- app/slack.go | 8 +-- app/upload.go | 4 +- services/slackimport/slackimport.go | 12 ++-- tests/10000x1_expected_preview.jpeg | Bin 1076 -> 0 bytes tests/10000x1_expected_preview.png | Bin 0 -> 92 bytes tests/10000x1_expected_thumb.jpeg | Bin 628 -> 0 bytes tests/10000x1_expected_thumb.png | Bin 0 -> 76 bytes tests/1x10000_expected_preview.jpeg | Bin 3096 -> 0 bytes tests/1x10000_expected_preview.png | Bin 0 -> 121 bytes tests/1x10000_expected_thumb.jpeg | Bin 624 -> 0 bytes tests/1x10000_expected_thumb.png | Bin 0 -> 79 bytes 14 files changed, 83 insertions(+), 61 deletions(-) delete mode 100644 tests/10000x1_expected_preview.jpeg create mode 100644 tests/10000x1_expected_preview.png delete mode 100644 tests/10000x1_expected_thumb.jpeg create mode 100644 tests/10000x1_expected_thumb.png delete mode 100644 tests/1x10000_expected_preview.jpeg create mode 100644 tests/1x10000_expected_preview.png delete mode 100644 tests/1x10000_expected_thumb.jpeg create mode 100644 tests/1x10000_expected_thumb.png diff --git a/api4/file_test.go b/api4/file_test.go index 0184f972b7..85bd04ee21 100644 --- a/api4/file_test.go +++ b/api4/file_test.go @@ -397,8 +397,8 @@ func TestUploadFiles(t *testing.T) { { title: "Happy image thumbnail/preview 10", names: []string{"10000x1.png"}, - expectedImageThumbnailNames: []string{"10000x1_expected_thumb.jpeg"}, - expectedImagePreviewNames: []string{"10000x1_expected_preview.jpeg"}, + expectedImageThumbnailNames: []string{"10000x1_expected_thumb.png"}, + expectedImagePreviewNames: []string{"10000x1_expected_preview.png"}, expectImage: true, expectedImageWidths: []int{10000}, expectedImageHeights: []int{1}, @@ -409,8 +409,8 @@ func TestUploadFiles(t *testing.T) { { title: "Happy image thumbnail/preview 11", names: []string{"1x10000.png"}, - expectedImageThumbnailNames: []string{"1x10000_expected_thumb.jpeg"}, - expectedImagePreviewNames: []string{"1x10000_expected_preview.jpeg"}, + expectedImageThumbnailNames: []string{"1x10000_expected_thumb.png"}, + expectedImagePreviewNames: []string{"1x10000_expected_preview.png"}, expectImage: true, expectedImageWidths: []int{1}, expectedImageHeights: []int{10000}, @@ -678,8 +678,12 @@ func TestUploadFiles(t *testing.T) { fmt.Sprintf("File %v saved to:%q, expected:%q", dbInfo.Name, dbInfo.Path, expectedPath)) if tc.expectImage { - expectedThumbnailPath := fmt.Sprintf("%s/%s_thumb.jpg", expectedDir, name) - expectedPreviewPath := fmt.Sprintf("%s/%s_preview.jpg", expectedDir, name) + // We convert all other image types to jpeg, except pngs. + if ext != ".png" { + ext = ".jpg" + } + expectedThumbnailPath := fmt.Sprintf("%s/%s_thumb%s", expectedDir, name, ext) + expectedPreviewPath := fmt.Sprintf("%s/%s_preview%s", expectedDir, name, ext) assert.Equal(t, dbInfo.ThumbnailPath, expectedThumbnailPath, fmt.Sprintf("Thumbnail for %v saved to:%q, expected:%q", dbInfo.Name, dbInfo.ThumbnailPath, expectedThumbnailPath)) assert.Equal(t, dbInfo.PreviewPath, expectedPreviewPath, diff --git a/app/file.go b/app/file.go index b0e68f4921..5f899aff70 100644 --- a/app/file.go +++ b/app/file.go @@ -266,8 +266,8 @@ func (a *App) getInfoForFilename(post *model.Post, teamID, channelID, userID, ol if info.IsImage() && !info.IsSvg() { nameWithoutExtension := name[:strings.LastIndex(name, ".")] - info.PreviewPath = pathPrefix + nameWithoutExtension + "_preview.jpg" - info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg" + info.PreviewPath = pathPrefix + nameWithoutExtension + "_preview." + getFileExtFromMimeType(info.MimeType) + info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(info.MimeType) } return info @@ -724,8 +724,8 @@ func (t *UploadFileTask) preprocessImage() *model.AppError { t.fileinfo.HasPreviewImage = true nameWithoutExtension := t.Name[:strings.LastIndex(t.Name, ".")] - t.fileinfo.PreviewPath = t.pathPrefix() + nameWithoutExtension + "_preview.jpg" - t.fileinfo.ThumbnailPath = t.pathPrefix() + nameWithoutExtension + "_thumb.jpg" + t.fileinfo.PreviewPath = t.pathPrefix() + nameWithoutExtension + "_preview." + getFileExtFromMimeType(t.fileinfo.MimeType) + t.fileinfo.ThumbnailPath = t.pathPrefix() + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(t.fileinfo.MimeType) // check the image orientation with goexif; consume the bytes we // already have first, then keep Tee-ing from input. @@ -770,20 +770,22 @@ func (t *UploadFileTask) postprocessImage(file io.Reader) { defer release() } - // Fill in the background of a potentially-transparent png file as white - if imgType == "png" { - imaging.FillImageTransparency(decoded, image.White) - } - decoded = imaging.MakeImageUpright(decoded, t.imageOrientation) if decoded == nil { return } - writeJPEG := func(img image.Image, path string) { + writeImage := func(img image.Image, path string) { r, w := io.Pipe() go func() { - err := t.imgEncoder.EncodeJPEG(w, img, jpegEncQuality) + var err error + // It's okay to access imgType in a separate goroutine, + // because imgType is only written once and never written again. + if imgType == "png" { + err = t.imgEncoder.EncodePNG(w, img) + } else { + err = t.imgEncoder.EncodeJPEG(w, img, jpegEncQuality) + } if err != nil { mlog.Error("Unable to encode image as jpeg", mlog.String("path", path), mlog.Err(err)) w.CloseWithError(err) @@ -804,12 +806,12 @@ func (t *UploadFileTask) postprocessImage(file io.Reader) { // This is needed on mobile in case of animated GIFs. go func() { defer wg.Done() - writeJPEG(imaging.GenerateThumbnail(decoded, imageThumbnailWidth, imageThumbnailHeight), t.fileinfo.ThumbnailPath) + writeImage(imaging.GenerateThumbnail(decoded, imageThumbnailWidth, imageThumbnailHeight), t.fileinfo.ThumbnailPath) }() go func() { defer wg.Done() - writeJPEG(imaging.GeneratePreview(decoded, imagePreviewWidth), t.fileinfo.PreviewPath) + writeImage(imaging.GeneratePreview(decoded, imagePreviewWidth), t.fileinfo.PreviewPath) }() go func() { @@ -889,8 +891,8 @@ func (a *App) DoUploadFileExpectModification(c request.CTX, now time.Time, rawTe } nameWithoutExtension := filename[:strings.LastIndex(filename, ".")] - info.PreviewPath = pathPrefix + nameWithoutExtension + "_preview.jpg" - info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb.jpg" + info.PreviewPath = pathPrefix + nameWithoutExtension + "_preview." + getFileExtFromMimeType(info.MimeType) + info.ThumbnailPath = pathPrefix + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(info.MimeType) } if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { @@ -949,40 +951,33 @@ func (a *App) HandleImages(previewPathList []string, thumbnailPathList []string, wg := new(sync.WaitGroup) for i := range fileData { - img, release, err := prepareImage(a.ch.imgDecoder, bytes.NewReader(fileData[i])) + img, imgType, release, err := prepareImage(a.ch.imgDecoder, bytes.NewReader(fileData[i])) if err != nil { mlog.Debug("Failed to prepare image", mlog.Err(err)) continue } wg.Add(2) - go func(img image.Image, path string) { + go func(img image.Image, imgType, path string) { defer wg.Done() - a.generateThumbnailImage(img, path) - }(img, thumbnailPathList[i]) + a.generateThumbnailImage(img, imgType, path) + }(img, imgType, thumbnailPathList[i]) - go func(img image.Image, path string) { + go func(img image.Image, imgType, path string) { defer wg.Done() - a.generatePreviewImage(img, path) - }(img, previewPathList[i]) + a.generatePreviewImage(img, imgType, path) + }(img, imgType, previewPathList[i]) wg.Wait() release() } } -func prepareImage(imgDecoder *imaging.Decoder, imgData io.ReadSeeker) (img image.Image, release func(), err error) { +func prepareImage(imgDecoder *imaging.Decoder, imgData io.ReadSeeker) (img image.Image, imgType string, release func(), err error) { // Decode image bytes into Image object - var imgType string img, imgType, release, err = imgDecoder.DecodeMemBounded(imgData) if err != nil { - return nil, nil, fmt.Errorf("prepareImage: failed to decode image: %w", err) + return nil, "", nil, fmt.Errorf("prepareImage: failed to decode image: %w", err) } - - // Fill in the background of a potentially-transparent png file as white - if imgType == "png" { - imaging.FillImageTransparency(img, image.White) - } - imgData.Seek(0, io.SeekStart) // Flip the image to be upright @@ -992,14 +987,23 @@ func prepareImage(imgDecoder *imaging.Decoder, imgData io.ReadSeeker) (img image } img = imaging.MakeImageUpright(img, orientation) - return img, release, nil + return img, imgType, release, nil } -func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string) { +func (a *App) generateThumbnailImage(img image.Image, imgType, thumbnailPath string) { var buf bytes.Buffer - if err := a.ch.imgEncoder.EncodeJPEG(&buf, imaging.GenerateThumbnail(img, imageThumbnailWidth, imageThumbnailHeight), jpegEncQuality); err != nil { - mlog.Error("Unable to encode image as jpeg", mlog.String("path", thumbnailPath), mlog.Err(err)) - return + + thumb := imaging.GenerateThumbnail(img, imageThumbnailWidth, imageThumbnailHeight) + if imgType == "png" { + if err := a.ch.imgEncoder.EncodePNG(&buf, thumb); err != nil { + mlog.Error("Unable to encode image as png", mlog.String("path", thumbnailPath), mlog.Err(err)) + return + } + } else { + if err := a.ch.imgEncoder.EncodeJPEG(&buf, thumb, jpegEncQuality); err != nil { + mlog.Error("Unable to encode image as jpeg", mlog.String("path", thumbnailPath), mlog.Err(err)) + return + } } if _, err := a.WriteFile(&buf, thumbnailPath); err != nil { @@ -1008,13 +1012,20 @@ func (a *App) generateThumbnailImage(img image.Image, thumbnailPath string) { } } -func (a *App) generatePreviewImage(img image.Image, previewPath string) { +func (a *App) generatePreviewImage(img image.Image, imgType, previewPath string) { var buf bytes.Buffer - preview := imaging.GeneratePreview(img, imagePreviewWidth) - if err := a.ch.imgEncoder.EncodeJPEG(&buf, preview, jpegEncQuality); err != nil { - mlog.Error("Unable to encode image as preview jpg", mlog.Err(err), mlog.String("path", previewPath)) - return + preview := imaging.GeneratePreview(img, imagePreviewWidth) + if imgType == "png" { + if err := a.ch.imgEncoder.EncodePNG(&buf, preview); err != nil { + mlog.Error("Unable to encode image as preview png", mlog.Err(err), mlog.String("path", previewPath)) + return + } + } else { + if err := a.ch.imgEncoder.EncodeJPEG(&buf, preview, jpegEncQuality); err != nil { + mlog.Error("Unable to encode image as preview jpg", mlog.Err(err), mlog.String("path", previewPath)) + return + } } if _, err := a.WriteFile(&buf, previewPath); err != nil { @@ -1033,7 +1044,7 @@ func (a *App) generateMiniPreview(fi *model.FileInfo) { return } defer file.Close() - img, release, err := prepareImage(a.ch.imgDecoder, file) + img, _, release, err := prepareImage(a.ch.imgDecoder, file) if err != nil { mlog.Debug("generateMiniPreview: prepareImage failed", mlog.Err(err), mlog.String("fileinfo_id", fi.Id), mlog.String("channel_id", fi.ChannelId), @@ -1409,3 +1420,10 @@ func (a *App) getCloudFilesSizeLimit() (int64, *model.AppError) { return int64(math.Ceil(float64(*limits.Files.TotalStorage) / 8)), nil } + +func getFileExtFromMimeType(mimeType string) string { + if mimeType == "image/png" { + return "png" + } + return "jpg" +} diff --git a/app/file_test.go b/app/file_test.go index 599c4c6da1..09c03e640e 100644 --- a/app/file_test.go +++ b/app/file_test.go @@ -338,7 +338,7 @@ func TestGenerateThumbnailImage(t *testing.T) { thumbnailPath := filepath.Join(dataPath, thumbnailName) // when - th.App.generateThumbnailImage(img, thumbnailName) + th.App.generateThumbnailImage(img, "jpg", thumbnailName) defer os.Remove(thumbnailPath) // then diff --git a/app/slack.go b/app/slack.go index f7baeb53bc..b108b42980 100644 --- a/app/slack.go +++ b/app/slack.go @@ -40,12 +40,12 @@ func (a *App) SlackImport(c *request.Context, fileData multipart.File, fileSize GeneratePreviewImage: a.generatePreviewImage, InvalidateAllCaches: func() { a.ch.srv.InvalidateAllCaches() }, MaxPostSize: func() int { return a.ch.srv.platform.MaxPostSize() }, - PrepareImage: func(fileData []byte) (image.Image, func(), error) { - img, release, err := prepareImage(a.ch.imgDecoder, bytes.NewReader(fileData)) + PrepareImage: func(fileData []byte) (image.Image, string, func(), error) { + img, imgType, release, err := prepareImage(a.ch.imgDecoder, bytes.NewReader(fileData)) if err != nil { - return nil, nil, err + return nil, "", nil, err } - return img, release, err + return img, imgType, release, err }, } diff --git a/app/upload.go b/app/upload.go index 15ccfba51a..3909f5e096 100644 --- a/app/upload.go +++ b/app/upload.go @@ -298,8 +298,8 @@ func (a *App) UploadData(c *request.Context, us *model.UploadSession, rd io.Read } nameWithoutExtension := info.Name[:strings.LastIndex(info.Name, ".")] - info.PreviewPath = filepath.Dir(info.Path) + "/" + nameWithoutExtension + "_preview.jpg" - info.ThumbnailPath = filepath.Dir(info.Path) + "/" + nameWithoutExtension + "_thumb.jpg" + info.PreviewPath = filepath.Dir(info.Path) + "/" + nameWithoutExtension + "_preview." + getFileExtFromMimeType(info.MimeType) + info.ThumbnailPath = filepath.Dir(info.Path) + "/" + nameWithoutExtension + "_thumb." + getFileExtFromMimeType(info.MimeType) imgData, fileErr := a.ReadFile(uploadPath) if fileErr != nil { return nil, fileErr diff --git a/services/slackimport/slackimport.go b/services/slackimport/slackimport.go index 03863b202a..5b3b15df9f 100644 --- a/services/slackimport/slackimport.go +++ b/services/slackimport/slackimport.go @@ -91,11 +91,11 @@ type Actions struct { CreateGroupChannel func(request.CTX, []string) (*model.Channel, *model.AppError) CreateChannel func(*model.Channel, bool) (*model.Channel, *model.AppError) DoUploadFile func(time.Time, string, string, string, string, []byte) (*model.FileInfo, *model.AppError) - GenerateThumbnailImage func(image.Image, string) - GeneratePreviewImage func(image.Image, string) + GenerateThumbnailImage func(image.Image, string, string) + GeneratePreviewImage func(image.Image, string, string) InvalidateAllCaches func() MaxPostSize func() int - PrepareImage func(fileData []byte) (image.Image, func(), error) + PrepareImage func(fileData []byte) (image.Image, string, func(), error) } // SlackImporter is a service that allows to import slack dumps into mattermost @@ -793,13 +793,13 @@ func (si *SlackImporter) oldImportFile(timestamp time.Time, file io.Reader, team } if fileInfo.IsImage() && !fileInfo.IsSvg() { - img, release, err := si.actions.PrepareImage(data) + img, imgType, release, err := si.actions.PrepareImage(data) if err != nil { return nil, err } defer release() - si.actions.GenerateThumbnailImage(img, fileInfo.ThumbnailPath) - si.actions.GeneratePreviewImage(img, fileInfo.PreviewPath) + si.actions.GenerateThumbnailImage(img, imgType, fileInfo.ThumbnailPath) + si.actions.GeneratePreviewImage(img, imgType, fileInfo.PreviewPath) } return fileInfo, nil diff --git a/tests/10000x1_expected_preview.jpeg b/tests/10000x1_expected_preview.jpeg deleted file mode 100644 index c547e21eefc9f60da050b4f7d8a8e0f352931f70..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 1076 zcmex=nP38T>!MAjrYM$lk!rsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T T{9k=E7IiU<;^7$v|8D{SFo9Ub diff --git a/tests/10000x1_expected_preview.png b/tests/10000x1_expected_preview.png new file mode 100644 index 0000000000000000000000000000000000000000..bf2ca9cc218dc3f05531b743122fc2ae61df969e GIT binary patch literal 92 zcmeAS@N?(olHy`uVBq!ia0y~yU~d4j89A7MEaktG3U(zMMee&9;OZ5 o-`De~PPqLXD8;~#upTJ*|Nm`&Mi!n!HXA@fp00i_>zopr0H~W6vj6}9 literal 0 HcmV?d00001 diff --git a/tests/10000x1_expected_thumb.jpeg b/tests/10000x1_expected_thumb.jpeg deleted file mode 100644 index 90f2925324ff095484083398f9111f8bf51a8324..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 628 zcmex=nP38T>!MAjrYM$WXz|sKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T Q{9k=E7IiV;!vAjq0Em{u=Kufz diff --git a/tests/10000x1_expected_thumb.png b/tests/10000x1_expected_thumb.png new file mode 100644 index 0000000000000000000000000000000000000000..a354c410473a0afaaffa29debb380348e798873c GIT binary patch literal 76 zcmeAS@N?(olHy`uVBq!ia0vp^6+q0$!2~2{Y5(m3Qo^1tjv*Cul75~y;CjKxz{tSx Z|9|clMwS|xYbSux44$rjF6*2UngDQH6OsS` literal 0 HcmV?d00001 diff --git a/tests/1x10000_expected_preview.jpeg b/tests/1x10000_expected_preview.jpeg deleted file mode 100644 index e2511cb26fca3274a9d8617f7c2aab5eccc3adec..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 3096 zcmex=nP38T>!MAjqLEz`)4NsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T o{9k=E7IiU<;?Xb|O#`E8U^ESkrh(BkFq#HN)4nP38T>!MAjrXx!obMPsKme|$jB_n z`2PswA_fLVRz@&jfC5G)p!?X^IXJnv1sIqZnVFebm_e=us;mXdF|Y`-3Mm>ovIz$! zvMUve7&T5@$f4}C@t|nX#SbdRNkvVZTw>x9l2WQ_>Kd9_CZ=ZQ7M51dF0O9w9-dyo zA)#U65s^{JDXD4c8JStdC8cHM6_r)ZEv;?s9i3g1CQq3GGAU*RJ2VdF$b$$4{OPfBE|D`;VW$ z7#Wx$-T{&j4Gc)#w^HS&+zk7m&W?T Q{9k=E7IiUT!~bsr09t{ Date: Tue, 11 Oct 2022 15:02:13 -0400 Subject: [PATCH 19/30] Update en.json (#21373) Automatic Merge --- i18n/en.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/i18n/en.json b/i18n/en.json index 0d164e18b0..43d7a804fb 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -9473,7 +9473,7 @@ }, { "id": "web.error.unsupported_browser.min_browser_version.chrome", - "translation": "Version 102+" + "translation": "Version 106+" }, { "id": "web.error.unsupported_browser.min_browser_version.edge", From 66b7c45e697d661c342fb6719aad5ebfc67ab13f Mon Sep 17 00:00:00 2001 From: Rob Bierbooms Date: Wed, 12 Oct 2022 15:19:00 +0200 Subject: [PATCH 20/30] [MM-47367] Make sure mini preview is saved to database when updating database (#21304) --- store/sqlstore/file_info_store.go | 1 + store/storetest/file_info_store.go | 34 ++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/store/sqlstore/file_info_store.go b/store/sqlstore/file_info_store.go index dbf47020e2..47d847b9d1 100644 --- a/store/sqlstore/file_info_store.go +++ b/store/sqlstore/file_info_store.go @@ -181,6 +181,7 @@ func (fs SqlFileInfoStore) Upsert(info *model.FileInfo) (*model.FileInfo, error) "Width": info.Width, "Height": info.Height, "HasPreviewImage": info.HasPreviewImage, + "MiniPreview": info.MiniPreview, "Content": info.Content, "RemoteId": info.RemoteId, }). diff --git a/store/storetest/file_info_store.go b/store/storetest/file_info_store.go index 1dc72b0536..4df67ae24a 100644 --- a/store/storetest/file_info_store.go +++ b/store/storetest/file_info_store.go @@ -28,6 +28,7 @@ func TestFileInfoStore(t *testing.T, ss store.Store) { t.Run("FileInfoPermanentDelete", func(t *testing.T) { testFileInfoPermanentDelete(t, ss) }) t.Run("FileInfoPermanentDeleteBatch", func(t *testing.T) { testFileInfoPermanentDeleteBatch(t, ss) }) t.Run("FileInfoPermanentDeleteByUser", func(t *testing.T) { testFileInfoPermanentDeleteByUser(t, ss) }) + t.Run("FileInfoUpdateMinipreview", func(t *testing.T) { testFileInfoUpdateMinipreview(t, ss) }) t.Run("GetFilesBatchForIndexing", func(t *testing.T) { testFileInfoStoreGetFilesBatchForIndexing(t, ss) }) t.Run("CountAll", func(t *testing.T) { testFileInfoStoreCountAll(t, ss) }) t.Run("GetStorageUsage", func(t *testing.T) { testFileInfoGetStorageUsage(t, ss) }) @@ -608,6 +609,39 @@ func testFileInfoPermanentDeleteByUser(t *testing.T, ss store.Store) { require.NoError(t, err) } +func testFileInfoUpdateMinipreview(t *testing.T, ss store.Store) { + info := &model.FileInfo{ + CreatorId: model.NewId(), + Path: "image.png", + } + + info, err := ss.FileInfo().Save(info) + require.NoError(t, err) + require.NotEqual(t, len(info.Id), 0) + + defer func() { + ss.FileInfo().PermanentDelete(info.Id) + }() + + rinfo, err := ss.FileInfo().Get(info.Id) + require.NoError(t, err) + require.Equal(t, info.Id, rinfo.Id) + require.Nil(t, rinfo.MiniPreview) + + miniPreview := []byte{0x0, 0x1, 0x2} + + rinfo.MiniPreview = &miniPreview + + rinfo, err = ss.FileInfo().Upsert(rinfo) + require.NoError(t, err) + require.Equal(t, info.Id, rinfo.Id) + + tinfo, err := ss.FileInfo().Get(info.Id) + require.NoError(t, err) + require.Equal(t, info.Id, tinfo.Id) + require.Equal(t, *tinfo.MiniPreview, miniPreview) +} + func testFileInfoStoreGetFilesBatchForIndexing(t *testing.T, ss store.Store) { c1 := &model.Channel{} c1.TeamId = model.NewId() From 71d1b7df533ecafc4807f7481dafdb13de97a14f Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Wed, 12 Oct 2022 16:31:16 +0300 Subject: [PATCH 21/30] [MM-47468] Fix store metrics initialization (#21374) * initialize metrics before store * add tests * reflect review comments --- app/platform/cluster.go | 4 +-- app/platform/config.go | 1 - app/platform/enterprise.go | 4 +-- app/platform/helper_test.go | 17 +++++---- app/platform/log.go | 4 +-- app/platform/metrics.go | 30 +++++++--------- app/platform/options.go | 2 +- app/platform/service.go | 32 +++++++++-------- app/platform/service_test.go | 70 ++++++++++++++++++++++++++++++++++++ app/platform/session.go | 6 ++-- app/platform/web_conn.go | 8 ++--- app/platform/web_hub.go | 8 ++--- 12 files changed, 128 insertions(+), 58 deletions(-) diff --git a/app/platform/cluster.go b/app/platform/cluster.go index 47722ba007..6575ab7daa 100644 --- a/app/platform/cluster.go +++ b/app/platform/cluster.go @@ -180,8 +180,8 @@ func (ps *PlatformService) InvokeClusterLeaderChangedListeners() { } func (ps *PlatformService) Publish(message *model.WebSocketEvent) { - if ps.metricsImpl() != nil { - ps.metricsImpl().IncrementWebsocketEvent(message.EventType()) + if ps.metricsIFace != nil { + ps.metricsIFace.IncrementWebsocketEvent(message.EventType()) } ps.PublishSkipClusterSend(message) diff --git a/app/platform/config.go b/app/platform/config.go index 5b0218dcd6..fd5bb05f76 100644 --- a/app/platform/config.go +++ b/app/platform/config.go @@ -32,7 +32,6 @@ type ServiceConfig struct { ConfigStore *config.Store Store store.Store // Optional fields - Metrics einterfaces.MetricsInterface Cluster einterfaces.ClusterInterface } diff --git a/app/platform/enterprise.go b/app/platform/enterprise.go index 37849fb3f3..cf7cdf0399 100644 --- a/app/platform/enterprise.go +++ b/app/platform/enterprise.go @@ -26,8 +26,8 @@ func RegisterLicenseInterface(f func(*PlatformService) einterfaces.LicenseInterf licenseInterface = f } -var metricsInterface func(*PlatformService, string, string) einterfaces.MetricsInterface +var metricsInterfaceFn func(*PlatformService, string, string) einterfaces.MetricsInterface func RegisterMetricsInterface(f func(*PlatformService, string, string) einterfaces.MetricsInterface) { - metricsInterface = f + metricsInterfaceFn = f } diff --git a/app/platform/helper_test.go b/app/platform/helper_test.go index 17e40627a3..b8d2da4594 100644 --- a/app/platform/helper_test.go +++ b/app/platform/helper_test.go @@ -55,7 +55,7 @@ func (ms *mockSuite) UserCanSeeOtherUser(userID string, otherUserId string) (boo return true, nil } -func Setup(tb testing.TB) *TestHelper { +func Setup(tb testing.TB, options ...Option) *TestHelper { if testing.Short() { tb.SkipNow() } @@ -64,7 +64,7 @@ func Setup(tb testing.TB) *TestHelper { dbStore.MarkSystemRanUnitTests() mainHelper.PreloadMigrations() - return setupTestHelper(dbStore, false, true, tb) + return setupTestHelper(dbStore, false, true, tb, options...) } func (th *TestHelper) InitBasic() *TestHelper { @@ -96,9 +96,9 @@ func (th *TestHelper) InitBasic() *TestHelper { return th } -func SetupWithStoreMock(tb testing.TB) *TestHelper { +func SetupWithStoreMock(tb testing.TB, options ...Option) *TestHelper { mockStore := testlib.GetMockStoreForSetupFunctions() - th := setupTestHelper(mockStore, false, false, tb) + th := setupTestHelper(mockStore, false, false, tb, options...) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) @@ -126,7 +126,7 @@ func SetupWithCluster(tb testing.TB, cluster einterfaces.ClusterInterface) *Test return th } -func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, tb testing.TB) *TestHelper { +func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer bool, tb testing.TB, options ...Option) *TestHelper { tempWorkspace, err := ioutil.TempDir("", "apptest") if err != nil { panic(err) @@ -149,7 +149,7 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo ps, err := New(ServiceConfig{ ConfigStore: configStore, Store: dbStore, - }) + }, options...) if err != nil { panic(err) } @@ -181,7 +181,10 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo th.Service.SetLicense(nil) } - th.Service.Start(th.Suite) + err = th.Service.Start(th.Suite) + if err != nil { + panic(err) + } return th } diff --git a/app/platform/log.go b/app/platform/log.go index 1d2cc8cf33..42ad85b54e 100644 --- a/app/platform/log.go +++ b/app/platform/log.go @@ -89,11 +89,11 @@ func (ps *PlatformService) NotificationsLogger() *mlog.Logger { } func (ps *PlatformService) EnableLoggingMetrics() { - if ps.metrics == nil || ps.metricsImpl() == nil { + if ps.metrics == nil || ps.metricsIFace == nil { return } - ps.logger.SetMetricsCollector(ps.metricsImpl().GetLoggerMetricsCollector(), mlog.DefaultMetricsUpdateFreqMillis) + ps.logger.SetMetricsCollector(ps.metricsIFace.GetLoggerMetricsCollector(), mlog.DefaultMetricsUpdateFreqMillis) // logging config needs to be reloaded when metrics collector is added or changed. if err := ps.initLogging(); err != nil { diff --git a/app/platform/metrics.go b/app/platform/metrics.go index f486f85cbc..2253594011 100644 --- a/app/platform/metrics.go +++ b/app/platform/metrics.go @@ -32,20 +32,13 @@ type platformMetrics struct { metricsImpl einterfaces.MetricsInterface - cfgFn func() *model.Config -} - -func (ps *PlatformService) metricsImpl() einterfaces.MetricsInterface { - if ps.metrics == nil { - return nil - } - - return ps.metrics.metricsImpl + cfgFn func() *model.Config + listenAddr string } // resetMetrics resets the metrics server. Clears the metrics if the metrics are disabled by the config. -func (ps *PlatformService) resetMetrics(metricsImpl einterfaces.MetricsInterface, cfgFn func() *model.Config) error { - if !*cfgFn().MetricsSettings.Enable { +func (ps *PlatformService) resetMetrics() error { + if !*ps.Config().MetricsSettings.Enable { if ps.metrics != nil { return ps.metrics.stopMetricsServer() } @@ -59,8 +52,8 @@ func (ps *PlatformService) resetMetrics(metricsImpl einterfaces.MetricsInterface } ps.metrics = &platformMetrics{ - cfgFn: cfgFn, - metricsImpl: metricsImpl, + cfgFn: ps.Config, + metricsImpl: ps.metricsIFace, logger: ps.logger, } @@ -68,8 +61,8 @@ func (ps *PlatformService) resetMetrics(metricsImpl einterfaces.MetricsInterface return err } - if metricsImpl != nil { - metricsImpl.Register() + if ps.metricsIFace != nil { + ps.metricsIFace.Register() } return ps.metrics.startMetricsServer() @@ -122,7 +115,8 @@ func (pm *platformMetrics) startMetricsServer() error { } }() - pm.logger.Info("Metrics and profiling server is started", mlog.String("address", l.Addr().String())) + pm.listenAddr = l.Addr().String() + pm.logger.Info("Metrics and profiling server is started", mlog.String("address", pm.listenAddr)) return nil } @@ -181,7 +175,7 @@ func (ps *PlatformService) HandleMetrics(route string, h http.Handler) { } func (ps *PlatformService) RestartMetrics() error { - return ps.resetMetrics(ps.serviceConfig.Metrics, ps.configStore.Get) + return ps.resetMetrics() } func (ps *PlatformService) Metrics() einterfaces.MetricsInterface { @@ -189,5 +183,5 @@ func (ps *PlatformService) Metrics() einterfaces.MetricsInterface { return nil } - return ps.metricsImpl() + return ps.metricsIFace } diff --git a/app/platform/options.go b/app/platform/options.go index 99d63d8f26..072914e104 100644 --- a/app/platform/options.go +++ b/app/platform/options.go @@ -46,7 +46,7 @@ func StoreOverride(override any) Option { func StoreOverrideWithCache(override store.Store) Option { return func(ps *PlatformService) error { ps.newStore = func() (store.Store, error) { - lcl, err := localcachelayer.NewLocalCacheLayer(override, ps.metricsImpl(), ps.clusterIFace, ps.cacheProvider) + lcl, err := localcachelayer.NewLocalCacheLayer(override, ps.metricsIFace, ps.clusterIFace, ps.cacheProvider) if err != nil { return nil, err } diff --git a/app/platform/service.go b/app/platform/service.go index b5a506ae50..d2104fb2bb 100644 --- a/app/platform/service.go +++ b/app/platform/service.go @@ -39,8 +39,7 @@ type PlatformService struct { WebSocketRouter *WebSocketRouter - serviceConfig *ServiceConfig - configStore *config.Store + configStore *config.Store cacheProvider cache.Provider statusCache cache.Cache @@ -57,6 +56,7 @@ type PlatformService struct { startMetrics bool metrics *platformMetrics + metricsIFace einterfaces.MetricsInterface featureFlagSynchronizerMutex sync.Mutex featureFlagSynchronizer *featureflag.Synchronizer @@ -100,7 +100,6 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { // Step 0: Create the PlatformService. // ConfigStore is and should be handled on a upper level. ps := &PlatformService{ - serviceConfig: &sc, Store: sc.Store, configStore: sc.ConfigStore, clusterIFace: sc.Cluster, @@ -170,15 +169,20 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { // Depends on step 3 (s.SearchEngine must be non-nil) ps.initEnterprise() - // Step 5: Store. - // Depends on Step 1 (config), 4 (metrics, cluster) and 5 (cacheProvider). + // Step 5: Init Metrics + if metricsInterfaceFn != nil { + ps.metricsIFace = metricsInterfaceFn(ps, *ps.configStore.Get().SqlSettings.DriverName, *ps.configStore.Get().SqlSettings.DataSource) + } + + // Step 6: Store. + // Depends on Step 0 (config), 1 (cacheProvider), 3 (search engine), 5 (metrics) and cluster. if ps.newStore == nil { ps.newStore = func() (store.Store, error) { - ps.sqlStore = sqlstore.New(ps.Config().SqlSettings, ps.Metrics()) + ps.sqlStore = sqlstore.New(ps.Config().SqlSettings, ps.metricsIFace) lcl, err2 := localcachelayer.NewLocalCacheLayer( retrylayer.New(ps.sqlStore), - ps.Metrics(), + ps.metricsIFace, ps.clusterIFace, ps.cacheProvider, ) @@ -204,7 +208,7 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { return timerlayer.New( searchStore, - ps.Metrics(), + ps.metricsIFace, ), nil } } @@ -234,20 +238,19 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { return nil, fmt.Errorf("could not create session cache: %w", err) } + // Step 7: Init License if model.BuildEnterpriseReady == "true" { ps.LoadLicense() } - if metricsInterface != nil { - sc.Metrics = metricsInterface(ps, *ps.configStore.Get().SqlSettings.DriverName, *ps.configStore.Get().SqlSettings.DataSource) - } - + // Step 8: Init Metrics Server depends on step 6 (store) and 7 (license) if ps.startMetrics { - if err = ps.resetMetrics(sc.Metrics, ps.configStore.Get); err != nil { - return nil, err + if mErr := ps.resetMetrics(); mErr != nil { + return nil, mErr } } + // Step 9: Init AsymmetricSigningKey depends on step 6 (store) if err = ps.EnsureAsymmetricSigningKey(); err != nil { return nil, fmt.Errorf("unable to ensure asymmetric signing key: %w", err) } @@ -299,6 +302,7 @@ func (ps *PlatformService) Start(suite SuiteIFace) error { return } }) + ps.licenseListenerId = ps.AddLicenseListener(func(oldLicense, newLicense *model.License) { ps.regenerateClientConfig() diff --git a/app/platform/service_test.go b/app/platform/service_test.go index e53afaae98..b2b7fa1e89 100644 --- a/app/platform/service_test.go +++ b/app/platform/service_test.go @@ -4,12 +4,16 @@ package platform import ( + "net/http" "os" + "strings" "testing" "github.com/mattermost/mattermost-server/v6/config" + "github.com/mattermost/mattermost-server/v6/einterfaces/mocks" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/store/storetest" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -83,3 +87,69 @@ func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { require.Len(t, ps.Config().SqlSettings.DataSourceSearchReplicas, 1) }) } + +func TestMetrics(t *testing.T) { + t.Run("ensure the metrics server is not started by default", func(t *testing.T) { + th := Setup(t) + defer th.TearDown() + + require.Nil(t, th.Service.metrics) + }) + + t.Run("ensure the metrics server is started", func(t *testing.T) { + th := Setup(t, StartMetrics()) + defer th.TearDown() + + // there is no config listener for the metrics + // we handle it on config save step + th.Service.UpdateConfig(func(c *model.Config) { + c.MetricsSettings.Enable = model.NewBool(true) + }) + th.Service.SaveConfig(th.Service.Config(), false) + + require.NotNil(t, th.Service.metrics) + metricsAddr := strings.Replace(th.Service.metrics.listenAddr, "[::]", "http://localhost", 1) + + resp, err := http.Get(metricsAddr) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + + th.Service.UpdateConfig(func(c *model.Config) { + c.MetricsSettings.Enable = model.NewBool(false) + }) + th.Service.SaveConfig(th.Service.Config(), false) + + _, err = http.Get(metricsAddr) + require.Error(t, err) + }) + + t.Run("ensure the metrics server is started with advanced metrics", func(t *testing.T) { + th := Setup(t, StartMetrics()) + defer th.TearDown() + + mockMetricsImpl := &mocks.MetricsInterface{} + mockMetricsImpl.On("Register").Return() + + th.Service.metricsIFace = mockMetricsImpl + err := th.Service.resetMetrics() + require.NoError(t, err) + + mockMetricsImpl.AssertExpectations(t) + }) + + t.Run("ensure advanced metrics have database metrics", func(t *testing.T) { + mockMetricsImpl := &mocks.MetricsInterface{} + mockMetricsImpl.On("Register").Return() + mockMetricsImpl.On("ObserveStoreMethodDuration", mock.Anything, mock.Anything, mock.Anything).Return() + + th := Setup(t, StartMetrics(), func(ps *PlatformService) error { + ps.metricsIFace = mockMetricsImpl + return nil + }) + defer th.TearDown() + + _ = th.CreateUserOrGuest(false) + + mockMetricsImpl.AssertExpectations(t) + }) +} diff --git a/app/platform/session.go b/app/platform/session.go index 1b085f7dd1..f6da9aec77 100644 --- a/app/platform/session.go +++ b/app/platform/session.go @@ -59,7 +59,7 @@ func (ps *PlatformService) ClearUserSessionCacheLocal(userID string) { if err := ps.sessionCache.Get(key, &session); err == nil { if session.UserId == userID { ps.sessionCache.Remove(key) - if m := ps.metricsImpl(); m != nil { + if m := ps.metricsIFace; m != nil { m.IncrementMemCacheInvalidationCounterSession() } } @@ -100,11 +100,11 @@ func (ps *PlatformService) ClearAllUsersSessionCache() { func (ps *PlatformService) GetSession(token string) (*model.Session, error) { var session = ps.sessionPool.Get().(*model.Session) if err := ps.sessionCache.Get(token, session); err == nil { - if m := ps.metricsImpl(); m != nil { + if m := ps.metricsIFace; m != nil { m.IncrementMemCacheHitCounterSession() } } else { - if m := ps.metricsImpl(); m != nil { + if m := ps.metricsIFace; m != nil { m.IncrementMemCacheMissCounterSession() } } diff --git a/app/platform/web_conn.go b/app/platform/web_conn.go index 2107333428..4e4fc75062 100644 --- a/app/platform/web_conn.go +++ b/app/platform/web_conn.go @@ -409,7 +409,7 @@ func (wc *WebConn) writePump() { wc.logSocketErr("websocket.drainDeadQueue", err) return } - if m := wc.Platform.metricsImpl(); m != nil { + if m := wc.Platform.metricsIFace; m != nil { m.IncrementWebsocketReconnectEvent(reconnectFound) } } else if wc.hasMsgLoss() { @@ -427,11 +427,11 @@ func (wc *WebConn) writePump() { wc.logSocketErr("websocket.sendHello", err) return } - if m := wc.Platform.metricsImpl(); m != nil { + if m := wc.Platform.metricsIFace; m != nil { m.IncrementWebsocketReconnectEvent(reconnectNotFound) } } else { - if m := wc.Platform.metricsImpl(); m != nil { + if m := wc.Platform.metricsIFace; m != nil { m.IncrementWebsocketReconnectEvent(reconnectLossless) } } @@ -488,7 +488,7 @@ func (wc *WebConn) writePump() { return } - if m := wc.Platform.metricsImpl(); m != nil { + if m := wc.Platform.metricsIFace; m != nil { m.IncrementWebSocketBroadcast(msg.EventType()) } case <-ticker.C: diff --git a/app/platform/web_hub.go b/app/platform/web_hub.go index 3a0d5fd6bb..dc83d574ce 100644 --- a/app/platform/web_hub.go +++ b/app/platform/web_hub.go @@ -142,7 +142,7 @@ func (ps *PlatformService) GetHubForUserId(userID string) *Hub { func (ps *PlatformService) HubRegister(webConn *WebConn) { hub := ps.GetHubForUserId(webConn.UserId) if hub != nil { - if metrics := ps.metricsImpl(); metrics != nil { + if metrics := ps.metricsIFace; metrics != nil { metrics.IncrementWebSocketBroadcastUsersRegistered(strconv.Itoa(hub.connectionIndex), 1) } hub.Register(webConn) @@ -153,7 +153,7 @@ func (ps *PlatformService) HubRegister(webConn *WebConn) { func (ps *PlatformService) HubUnregister(webConn *WebConn) { hub := ps.GetHubForUserId(webConn.UserId) if hub != nil { - if metrics := ps.metricsImpl(); metrics != nil { + if metrics := ps.metricsIFace; metrics != nil { metrics.DecrementWebSocketBroadcastUsersRegistered(strconv.Itoa(hub.connectionIndex), 1) } hub.Unregister(webConn) @@ -317,7 +317,7 @@ func (h *Hub) Broadcast(message *model.WebSocketEvent) { // And possibly, we can look into doing the hub initialization inside // NewServer itself. if h != nil && message != nil { - if metrics := h.platform.metricsImpl(); metrics != nil { + if metrics := h.platform.metricsIFace; metrics != nil { metrics.IncrementWebSocketBroadcastBufferSize(strconv.Itoa(h.connectionIndex), 1) } select { @@ -483,7 +483,7 @@ func (h *Hub) Start(suite SuiteIFace) { connIndex.Remove(directMsg.conn) } case msg := <-h.broadcast: - if metrics := h.platform.metricsImpl(); metrics != nil { + if metrics := h.platform.metricsIFace; metrics != nil { metrics.DecrementWebSocketBroadcastBufferSize(strconv.Itoa(h.connectionIndex), 1) } msg = msg.PrecomputeJSON() From 11665d28ea2cf88e2bfb909eab788478c7cdece5 Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Wed, 12 Oct 2022 12:53:38 -0300 Subject: [PATCH 22/30] Pre-package Playbooks v1.32.5 (#21376) * Pre-package Playbooks v1.32.5 * Pre-package Playbooks v1.32.6 --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 80b0d047cd..789655bb01 100644 --- a/Makefile +++ b/Makefile @@ -154,7 +154,7 @@ PLUGIN_PACKAGES += mattermost-plugin-channel-export-v1.0.0 PLUGIN_PACKAGES += mattermost-plugin-custom-attributes-v1.3.0 PLUGIN_PACKAGES += mattermost-plugin-github-v2.0.1 PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.3.0 -PLUGIN_PACKAGES += mattermost-plugin-playbooks-v1.32.4 +PLUGIN_PACKAGES += mattermost-plugin-playbooks-v1.32.6 PLUGIN_PACKAGES += mattermost-plugin-jenkins-v1.1.0 PLUGIN_PACKAGES += mattermost-plugin-jira-v2.4.0 PLUGIN_PACKAGES += mattermost-plugin-nps-v1.2.0 From a6a44b5824e7bcf0249836b3a0b6d1d423ab24f4 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Wed, 12 Oct 2022 11:32:14 -0600 Subject: [PATCH 23/30] prepackage boards v7.4.2 (#21382) Automatic Merge --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 789655bb01..43ccf306a5 100644 --- a/Makefile +++ b/Makefile @@ -160,7 +160,7 @@ PLUGIN_PACKAGES += mattermost-plugin-jira-v2.4.0 PLUGIN_PACKAGES += mattermost-plugin-nps-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.2.0 PLUGIN_PACKAGES += mattermost-plugin-zoom-v1.6.0 -PLUGIN_PACKAGES += focalboard-v7.4.1 +PLUGIN_PACKAGES += focalboard-v7.4.2 PLUGIN_PACKAGES += mattermost-plugin-apps-v1.1.0 # Prepares the enterprise build if exists. The IGNORE stuff is a hack to get the Makefile to execute the commands outside a target From 1d9fdce9167e46de14fb11983bb263b0bda9870b Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Thu, 13 Oct 2022 20:34:48 +0300 Subject: [PATCH 24/30] Revert "add metropolis font to email change verify email html" This reverts commit 9f6497fb0edccbe9d3719fa9cdd999fa1c9295a6. --- templates/email_change_verify_body.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index 8f6918bfaf..0b171b98ca 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -2,7 +2,6 @@ - @@ -88,7 +87,7 @@ From 8bd6ec8f994fe5cad8789f220a2aad8a33d0c38a Mon Sep 17 00:00:00 2001 From: Muhammad Salman Date: Thu, 13 Oct 2022 21:00:44 +0300 Subject: [PATCH 25/30] [MM-44790] removing extra margin bottom from the verify email change email heading --- templates/email_change_verify_body.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/email_change_verify_body.html b/templates/email_change_verify_body.html index 0b171b98ca..22a1cec84d 100644 --- a/templates/email_change_verify_body.html +++ b/templates/email_change_verify_body.html @@ -87,7 +87,7 @@ From 3747d998067c50670c2518652efec6ec44b55122 Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Thu, 13 Oct 2022 14:23:22 -0700 Subject: [PATCH 26/30] [MM-46694] A/B Test: welcome post (#20926) Co-authored-by: Mattermod --- app/app_iface.go | 1 + app/channel.go | 30 +++++++++++++ app/opentracing/opentracing_layer.go | 17 ++++++++ app/user.go | 13 ++++++ app/user_test.go | 51 ++++++++++++++++++++++ model/feature_flags.go | 4 ++ model/post.go | 2 + store/opentracinglayer/opentracinglayer.go | 18 ++++++++ store/retrylayer/retrylayer.go | 21 +++++++++ store/sqlstore/user_store.go | 10 +++++ store/store.go | 1 + store/storetest/mocks/UserStore.go | 21 +++++++++ store/storetest/user_store.go | 25 +++++++++++ store/timerlayer/timerlayer.go | 16 +++++++ 14 files changed, 230 insertions(+) diff --git a/app/app_iface.go b/app/app_iface.go index ef6b3b4733..aed132ba5f 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -871,6 +871,7 @@ type AppIface interface { InviteNewUsersToTeam(emailList []string, teamID, senderId string) *model.AppError InviteNewUsersToTeamGracefully(memberInvite *model.MemberInvite, teamID, senderId string, reminderInterval string) ([]*model.EmailInviteWithError, *model.AppError) IsCRTEnabledForUser(c request.CTX, userID string) bool + IsFirstAdmin(user *model.User) bool IsFirstUserAccount() bool IsLeader() bool IsPasswordValid(password string) *model.AppError diff --git a/app/channel.go b/app/channel.go index 8da9b638f9..504c7265d6 100644 --- a/app/channel.go +++ b/app/channel.go @@ -12,6 +12,7 @@ import ( "strings" "time" + "github.com/mattermost/logr/v2" "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/plugin" @@ -130,6 +131,35 @@ func (a *App) JoinDefaultChannels(c request.CTX, teamID string, user *model.User message.Add("user_id", user.Id) message.Add("team_id", channel.TeamId) a.Publish(message) + + // A/B Test on the welcome post + if a.Config().FeatureFlags.SendWelcomePost && channelName == model.DefaultChannelName { + nbTeams, err := a.Srv().Store().Team().AnalyticsTeamCount(&model.TeamSearch{ + IncludeDeleted: model.NewBool(true), + }) + if err != nil { + c.Logger().Warn("unable to get number of teams", logr.Err(err)) + return nil + } + + if nbTeams == 1 && a.IsFirstAdmin(user) { + // Post the welcome message + if _, err := a.CreatePost(c, &model.Post{ + ChannelId: channel.Id, + Type: model.PostTypeWelcomePost, + UserId: user.Id, + }, channel, false, false); err != nil { + c.Logger().Warn("unable to post welcome message", logr.Err(err)) + return nil + } + ts := a.Srv().GetTelemetryService() + if ts != nil { + ts.SendTelemetry("welcome-message-sent", map[string]any{ + "category": "growth", + }) + } + } + } } if nErr != nil { diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 6dd849aefb..97f929c77d 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -11626,6 +11626,23 @@ func (a *OpenTracingAppLayer) IsCRTEnabledForUser(c request.CTX, userID string) return resultVar0 } +func (a *OpenTracingAppLayer) IsFirstAdmin(user *model.User) bool { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.IsFirstAdmin") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.IsFirstAdmin(user) + + return resultVar0 +} + func (a *OpenTracingAppLayer) IsFirstUserAccount() bool { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.IsFirstUserAccount") diff --git a/app/user.go b/app/user.go index bdb9fc8841..c0a9773e61 100644 --- a/app/user.go +++ b/app/user.go @@ -209,6 +209,19 @@ func (a *App) IsFirstUserAccount() bool { return a.ch.srv.platform.IsFirstUserAccount() } +func (a *App) IsFirstAdmin(user *model.User) bool { + if !user.IsSystemAdmin() { + return false + } + + adminID, err := a.Srv().Store().User().GetFirstSystemAdminID() + if err != nil { + return false + } + + return adminID == user.Id +} + // CreateUser creates a user and sets several fields of the returned User struct to // their zero values. func (a *App) CreateUser(c request.CTX, user *model.User) (*model.User, *model.AppError) { diff --git a/app/user_test.go b/app/user_test.go index 174c193deb..139cc1c1e5 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -1801,3 +1801,54 @@ func TestCreateUserWithInitialPreferences(t *testing.T) { assert.Equal(t, "false", recommendedNextStepsPref[0].Value) }) } + +func TestIsFirstAdmin(t *testing.T) { + t.Run("should return false if user is not sysadmin", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + Id := model.NewId() + isFirstAdmin := th.App.IsFirstAdmin(&model.User{ + Id: Id, + Roles: model.SystemUserRoleId, + }) + require.False(t, isFirstAdmin) + }) + + t.Run("should return false if user is sysadmin but not the first one", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + Id := model.NewId() + + mockUserStore := storemocks.UserStore{} + mockUserStore.On("GetFirstSystemAdminID").Return(model.NewId(), nil) + + mockStore := th.App.Srv().Store().(*storemocks.Store) + mockStore.On("User").Return(&mockUserStore) + + isFirstAdmin := th.App.IsFirstAdmin(&model.User{ + Id: Id, + Roles: model.SystemAdminRoleId, + }) + require.False(t, isFirstAdmin) + }) + + t.Run("should return true if user is sysadmin and the first one", func(t *testing.T) { + th := SetupWithStoreMock(t) + defer th.TearDown() + + Id := model.NewId() + + mockStore := th.App.Srv().Store().(*storemocks.Store) + mockUserStore := storemocks.UserStore{} + mockUserStore.On("GetFirstSystemAdminID").Return(Id, nil) + mockStore.On("User").Return(&mockUserStore) + + isFirstAdmin := th.App.IsFirstAdmin(&model.User{ + Id: Id, + Roles: model.SystemAdminRoleId, + }) + require.True(t, isFirstAdmin) + }) +} diff --git a/model/feature_flags.go b/model/feature_flags.go index 04b2eab18d..9a111d216b 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -72,6 +72,9 @@ type FeatureFlags struct { PlanUpgradeButtonText string + // A/B Test on posting a welcome message + SendWelcomePost bool + PostPriority bool } @@ -99,6 +102,7 @@ func (f *FeatureFlags) SetDefaults() { f.CallsEnabled = true f.BoardsProduct = false f.PlanUpgradeButtonText = "upgrade" + f.SendWelcomePost = true f.PostPriority = false } diff --git a/model/post.go b/model/post.go index 4916779b52..3f9c5a630c 100644 --- a/model/post.go +++ b/model/post.go @@ -44,6 +44,7 @@ const ( PostTypeChannelRestored = "system_channel_restored" PostTypeEphemeral = "system_ephemeral" PostTypeChangeChannelPrivacy = "system_change_chan_privacy" + PostTypeWelcomePost = "system_welcome_post" PostTypeAddBotTeamsChannels = "add_bot_teams_channels" PostTypeSystemWarnMetricStatus = "warn_metric_status" PostTypeMe = "me" @@ -387,6 +388,7 @@ func (o *Post) IsValid(maxPostSize int) *AppError { PostTypeChangeChannelPrivacy, PostTypeAddBotTeamsChannels, PostTypeSystemWarnMetricStatus, + PostTypeWelcomePost, PostTypeMe: default: if !strings.HasPrefix(o.Type, PostCustomTypePrefix) { diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index eb65b1f798..44f7d09e86 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -10801,6 +10801,24 @@ func (s *OpenTracingLayerUserStore) GetEtagForProfilesNotInTeam(teamID string) s return result } +func (s *OpenTracingLayerUserStore) GetFirstSystemAdminID() (string, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetFirstSystemAdminID") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.UserStore.GetFirstSystemAdminID() + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerUserStore) GetForLogin(loginID string, allowSignInWithUsername bool, allowSignInWithEmail bool) (*model.User, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "UserStore.GetForLogin") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 4efa7172ba..292924476a 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -12324,6 +12324,27 @@ func (s *RetryLayerUserStore) GetEtagForProfilesNotInTeam(teamID string) string } +func (s *RetryLayerUserStore) GetFirstSystemAdminID() (string, error) { + + tries := 0 + for { + result, err := s.UserStore.GetFirstSystemAdminID() + 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 *RetryLayerUserStore) GetForLogin(loginID string, allowSignInWithUsername bool, allowSignInWithEmail bool) (*model.User, error) { tries := 0 diff --git a/store/sqlstore/user_store.go b/store/sqlstore/user_store.go index 0f724405d2..bd89779355 100644 --- a/store/sqlstore/user_store.go +++ b/store/sqlstore/user_store.go @@ -1748,6 +1748,16 @@ func (us SqlUserStore) InferSystemInstallDate() (int64, error) { return createAt, nil } +func (us SqlUserStore) GetFirstSystemAdminID() (string, error) { + var id string + err := us.GetReplicaX().Get(&id, "SELECT Id FROM Users WHERE Roles LIKE ? ORDER BY CreateAt ASC LIMIT 1", "%system_admin%") + if err != nil { + return "", errors.Wrap(err, "failed to get first system admin") + } + + return id, nil +} + func (us SqlUserStore) GetUsersBatchForIndexing(startTime int64, startFileID string, limit int) ([]*model.UserForIndexing, error) { users := []*model.User{} usersQuery, args, err := us.usersQuery. diff --git a/store/store.go b/store/store.go index b917adbede..f7c33a938e 100644 --- a/store/store.go +++ b/store/store.go @@ -481,6 +481,7 @@ type UserStore interface { IsEmpty(excludeBots bool) (bool, error) GetUsersWithInvalidEmails(page int, perPage int, restrictedDomains string) ([]*model.User, error) InsertUsers(users []*model.User) error + GetFirstSystemAdminID() (string, error) } type BotStore interface { diff --git a/store/storetest/mocks/UserStore.go b/store/storetest/mocks/UserStore.go index ea0fdcfcc0..290b37dae7 100644 --- a/store/storetest/mocks/UserStore.go +++ b/store/storetest/mocks/UserStore.go @@ -569,6 +569,27 @@ func (_m *UserStore) GetEtagForProfilesNotInTeam(teamID string) string { return r0 } +// GetFirstSystemAdminID provides a mock function with given fields: +func (_m *UserStore) GetFirstSystemAdminID() (string, error) { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + var r1 error + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetForLogin provides a mock function with given fields: loginID, allowSignInWithUsername, allowSignInWithEmail func (_m *UserStore) GetForLogin(loginID string, allowSignInWithUsername bool, allowSignInWithEmail bool) (*model.User, error) { ret := _m.Called(loginID, allowSignInWithUsername, allowSignInWithEmail) diff --git a/store/storetest/user_store.go b/store/storetest/user_store.go index 98d6f2bc4f..6d704854b4 100644 --- a/store/storetest/user_store.go +++ b/store/storetest/user_store.go @@ -94,6 +94,7 @@ func TestUserStore(t *testing.T, ss store.Store, s SqlStore) { t.Run("ResetLastPictureUpdate", func(t *testing.T) { testUserStoreResetLastPictureUpdate(t, ss) }) t.Run("GetKnownUsers", func(t *testing.T) { testGetKnownUsers(t, ss) }) t.Run("GetUsersWithInvalidEmails", func(t *testing.T) { testGetUsersWithInvalidEmails(t, ss) }) + t.Run("GetFirstSystemAdminID", func(t *testing.T) { testUserStoreGetFirstSystemAdminID(t, ss) }) } func testUserStoreSave(t *testing.T, ss store.Store) { @@ -4163,6 +4164,30 @@ func testCount(t *testing.T, ss store.Store) { } } +func testUserStoreGetFirstSystemAdminID(t *testing.T, ss store.Store) { + sysAdmin := &model.User{} + sysAdmin.Email = MakeEmail() + sysAdmin.Roles = model.SystemAdminRoleId + " " + model.SystemUserRoleId + sysAdmin, err := ss.User().Save(sysAdmin) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(sysAdmin.Id)) }() + + // We need the second system admin to be created after the first one + // our granulirity is ms + time.Sleep(1 * time.Millisecond) + + sysAdmin2 := &model.User{} + sysAdmin2.Email = MakeEmail() + sysAdmin2.Roles = model.SystemAdminRoleId + " " + model.SystemUserRoleId + sysAdmin2, err = ss.User().Save(sysAdmin2) + require.NoError(t, err) + defer func() { require.NoError(t, ss.User().PermanentDelete(sysAdmin2.Id)) }() + + returnedId, err := ss.User().GetFirstSystemAdminID() + require.NoError(t, err) + require.Equal(t, sysAdmin.Id, returnedId) +} + func testUserStoreAnalyticsActiveCount(t *testing.T, ss store.Store, s SqlStore) { cleanupStatusStore(t, s) diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index 0d7b338767..0558fa66c1 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -9727,6 +9727,22 @@ func (s *TimerLayerUserStore) GetEtagForProfilesNotInTeam(teamID string) string return result } +func (s *TimerLayerUserStore) GetFirstSystemAdminID() (string, error) { + start := time.Now() + + result, err := s.UserStore.GetFirstSystemAdminID() + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("UserStore.GetFirstSystemAdminID", success, elapsed) + } + return result, err +} + func (s *TimerLayerUserStore) GetForLogin(loginID string, allowSignInWithUsername bool, allowSignInWithEmail bool) (*model.User, error) { start := time.Now() From d96516acd92585435ad65f0bbe968b551405b12d Mon Sep 17 00:00:00 2001 From: Mattermod Date: Fri, 14 Oct 2022 02:32:14 +0300 Subject: [PATCH 27/30] Update minor version to 7.5.0 (#21405) Automatic Merge --- model/version.go | 1 + 1 file changed, 1 insertion(+) diff --git a/model/version.go b/model/version.go index be63d87971..4bfc461767 100644 --- a/model/version.go +++ b/model/version.go @@ -13,6 +13,7 @@ import ( // It should be maintained in chronological order with most current // release at the front of the list. var versions = []string{ + "7.5.0", "7.4.0", "7.3.0", "7.2.0", From c57706e6f4d4a932ee64f3fdd6b952ff6c818702 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 14 Oct 2022 14:17:43 +0530 Subject: [PATCH 28/30] GraphQL: Remove channel stats from the API (#21377) Channel Stats isn't optimized with dataloaders. And it's a heavy call, therefore clients can mistakenly call this with multiple channels and bring down the server. Since there is no advantage compared to a REST API, let's just remove it and keep things safe. ```release-note NONE ``` --- api4/resolver_channel.go | 30 --------------------------- api4/resolver_channel_test.go | 39 ----------------------------------- api4/schema.graphqls | 8 ------- 3 files changed, 77 deletions(-) diff --git a/api4/resolver_channel.go b/api4/resolver_channel.go index e8f10a8a98..2ad2eca523 100644 --- a/api4/resolver_channel.go +++ b/api4/resolver_channel.go @@ -29,36 +29,6 @@ func (ch *channel) Team(ctx context.Context) (*model.Team, error) { return getGraphQLTeam(ctx, ch.TeamId) } -// match with api4.getChannelStats -func (ch *channel) Stats(ctx context.Context) (*model.ChannelStats, error) { - c, err := getCtx(ctx) - if err != nil { - return nil, err - } - - if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), ch.Id, model.PermissionReadChannel) { - c.SetPermissionError(model.PermissionReadChannel) - return nil, c.Err - } - - memberCount, appErr := c.App.GetChannelMemberCount(c.AppContext, ch.Id) - if appErr != nil { - return nil, appErr - } - - guestCount, appErr := c.App.GetChannelGuestCount(c.AppContext, ch.Id) - if appErr != nil { - return nil, appErr - } - - pinnedPostCount, appErr := c.App.GetChannelPinnedPostCount(c.AppContext, ch.Id) - if appErr != nil { - return nil, appErr - } - - return &model.ChannelStats{ChannelId: ch.Id, MemberCount: memberCount, GuestCount: guestCount, PinnedPostCount: pinnedPostCount}, nil -} - func (ch *channel) Cursor() *string { cursor := string(channelCursorPrefix) + "-" + ch.Id encoded := base64.StdEncoding.EncodeToString([]byte(cursor)) diff --git a/api4/resolver_channel_test.go b/api4/resolver_channel_test.go index 36c7989ec1..d5a55dee16 100644 --- a/api4/resolver_channel_test.go +++ b/api4/resolver_channel_test.go @@ -48,12 +48,6 @@ func TestGraphQLChannels(t *testing.T) { ID string `json:"id"` DisplayName string `json:"displayName"` } `json:"team"` - Stats struct { - ChannelId string `json:"channelId"` - MemberCount float64 `json:"memberCount"` - GuestCount float64 `json:"guestCount"` - PinnedPostCount float64 `json:"pinnedpostCount"` - } `json:"stats"` } `json:"channels"` } @@ -388,39 +382,6 @@ func TestGraphQLChannels(t *testing.T) { require.NoError(t, json.Unmarshal(resp.Data, &q)) assert.Len(t, q.Channels, 5) }) - - t.Run("stats", func(t *testing.T) { - query := `query channels($teamId: String, $first: Int) { - channels(userId: "me", teamId: $teamId, first: $first) { - id - stats { - channelId - memberCount - } - } - } - ` - input := graphQLInput{ - OperationName: "channels", - Query: query, - Variables: map[string]any{ - "first": 10, - "teamId": myTeam.Id, - }, - } - - resp, err := th.MakeGraphQLRequest(&input) - require.NoError(t, err) - require.Len(t, resp.Errors, 0) - require.NoError(t, json.Unmarshal(resp.Data, &q)) - require.Len(t, q.Channels, 3) - for _, ch := range q.Channels { - require.Equal(t, ch.ID, ch.Stats.ChannelId) - count, appErr := th.App.GetChannelMemberCount(th.Context, ch.Stats.ChannelId) - require.Nil(t, appErr) - require.Equal(t, float64(count), ch.Stats.MemberCount) - } - }) } func TestGetPrettyDNForUsers(t *testing.T) { diff --git a/api4/schema.graphqls b/api4/schema.graphqls index a976ead790..5af4d4d87c 100644 --- a/api4/schema.graphqls +++ b/api4/schema.graphqls @@ -62,7 +62,6 @@ type Channel { totalMsgCount: Float! totalMsgCountRoot: Float! lastRootPostAt: Float! - stats: ChannelStats extraUpdateAt: Float! props: StringInterface! policyId: String @@ -220,10 +219,3 @@ type Session { props: StringMap! local: Boolean! } - -type ChannelStats { - channelId: String! - memberCount: Float! - guestCount: Float! - pinnedPostCount: Float! -} From 21bf5b5dd5914b585dc545da1d5c09e7cab1599c Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Sat, 15 Oct 2022 01:41:45 +0530 Subject: [PATCH 29/30] MM-47465: Skip flaky TestDeleteChannel (#21392) https://mattermost.atlassian.net/browse/MM-47465 ```release-note NONE ``` --- api4/channel_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/api4/channel_test.go b/api4/channel_test.go index 16e432bf0d..1e1ea4b2e1 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -1779,6 +1779,7 @@ func TestSearchGroupChannels(t *testing.T) { } func TestDeleteChannel(t *testing.T) { + t.Skip("MM-47465") th := Setup(t).InitBasic() defer th.TearDown() c := th.Client From 9a857b47b43e73db4e44d574fe81f3c35eea1d95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A1bor=20Boros?= Date: Sat, 15 Oct 2022 06:32:14 +0200 Subject: [PATCH 30/30] feat: nested config value setting using env vars (#21049) Automatic Merge --- config/environment.go | 6 +++ config/environment_test.go | 76 +++++++++++++++++++++++++++++++++++++- 2 files changed, 81 insertions(+), 1 deletion(-) diff --git a/config/environment.go b/config/environment.go index 12e6727484..4ded8d7399 100644 --- a/config/environment.go +++ b/config/environment.go @@ -4,6 +4,7 @@ package config import ( + "encoding/json" "os" "reflect" "strconv" @@ -73,6 +74,11 @@ func applyEnvKey(key, value string, rValueSubject reflect.Value) { } case reflect.SliceOf(reflect.TypeOf("")).Kind(): rFieldValue.Set(reflect.ValueOf(strings.Split(value, " "))) + case reflect.Map: + target := reflect.New(rFieldValue.Type()).Interface() + if err := json.Unmarshal([]byte(value), target); err == nil { + rFieldValue.Set(reflect.ValueOf(target).Elem()) + } } } diff --git a/config/environment_test.go b/config/environment_test.go index fb4b3261ed..e86bcc36e9 100644 --- a/config/environment_test.go +++ b/config/environment_test.go @@ -31,15 +31,54 @@ func TestRemoveEnvOverrides(t *testing.T) { expectedConfig *model.Config }{ { - name: "basic override", + name: "config override", inputConfig: modifiedDefault(func(in *model.Config) { *in.ServiceSettings.TLSMinVer = "1.4" + in.PluginSettings.PluginStates = map[string]*model.PluginState{ + "plugin1": { + Enable: false, + }, + } + in.PluginSettings.Plugins = map[string]map[string]interface{}{ + "com.mattermost.plugin-1": { + "key1": "value1", + }, + "com_mattermost_plugin-2": { + "key2": "value2", + }, + } }), env: map[string]string{ "MM_SERVICESETTINGS_TLSMINVER": "1.5", + "MM_PLUGINSETTINGS_PLUGINSTATES": `{ + "plugin1": { + "Enable": true + } + }`, + "MM_PLUGINSETTINGS_PLUGINS": `{ + "com.mattermost.plugin-1": { + "key1": "other-value" + }, + "com_mattermost_plugin-2": { + "key2": "other-value" + } + }`, }, expectedConfig: modifiedDefault(func(in *model.Config) { *in.ServiceSettings.TLSMinVer = "1.5" + in.PluginSettings.PluginStates = map[string]*model.PluginState{ + "plugin1": { + Enable: true, + }, + } + in.PluginSettings.Plugins = map[string]map[string]interface{}{ + "com.mattermost.plugin-1": { + "key1": "other-value", + }, + "com_mattermost_plugin-2": { + "key2": "other-value", + }, + } }), }, { @@ -102,6 +141,41 @@ func TestRemoveEnvOverrides(t *testing.T) { in.SqlSettings.DataSourceReplicas = []string{"otherthing", "alsothis"} }), }, + { + name: "complex env settings", + inputConfig: modifiedDefault(func(in *model.Config) { + }), + env: map[string]string{ + "MM_PLUGINSETTINGS_PLUGINSTATES": `{ + "com.mattermost.plugin-1": { + "enable": true + } + }`, + "MM_PLUGINSETTINGS_PLUGINS": `{ + "com.mattermost.plugin-1": { + "key": { + "key": "(?PKEY)-(?P\\d{1,6})(?P[,;]*)", + "value": "[$key-$id](https://example.com/?$project-$id)$comma" + } + } + }`, + }, + expectedConfig: modifiedDefault(func(in *model.Config) { + in.PluginSettings.PluginStates = map[string]*model.PluginState{ + "com.mattermost.plugin-1": { + Enable: true, + }, + } + in.PluginSettings.Plugins = map[string]map[string]interface{}{ + "com.mattermost.plugin-1": { + "key": map[string]interface{}{ + "key": "(?PKEY)-(?P\\d{1,6})(?P[,;]*)", + "value": "[$key-$id](https://example.com/?$project-$id)$comma", + }, + }, + } + }), + }, { name: "bad env", inputConfig: modifiedDefault(func(in *model.Config) {
- + {{.Props.VerifyButton}}
-

{{.Props.QuestionTitle}}

-

{{.Props.EmailInfo1}}{{.Props.SupportEmail}}

+

{{.Props.QuestionTitle}}

+

{{.Props.EmailInfo1}}{{.Props.SupportEmail}}
-

{{.Props.QuestionTitle}}

-

{{.Props.EmailInfo1}}{{.Props.SupportEmail}}

+

{{.Props.QuestionTitle}}

+

{{.Props.EmailInfo1}}{{.Props.SupportEmail}}
-

{{.Props.Title}}

+

{{.Props.Title}}
-

{{.Props.Title}}

+

{{.Props.Title}}
-

{{.Props.Title}}

+

{{.Props.Title}}