From 668518f0f49e3ef9fb9297b505a1a083596f1671 Mon Sep 17 00:00:00 2001 From: Muhammad S <841955+mhd-sln@users.noreply.github.com> Date: Fri, 16 Dec 2022 14:29:56 +0200 Subject: [PATCH 01/13] [MM-48759] track event when redirect from cws signup with utm campaign --- api4/user.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/api4/user.go b/api4/user.go index a35d1d6597..0d09f8f7eb 100644 --- a/api4/user.go +++ b/api4/user.go @@ -2017,9 +2017,15 @@ func loginCWS(c *Context, w http.ResponseWriter, r *http.Request) { redirectURL := *c.App.Config().ServiceSettings.SiteURL if campaign != "" { if url, ok := campaignToURL[campaign]; ok { + properties := map[string]any{ + "category": "acquisition", + "redirect_to": strings.TrimSuffix(url, "/"), + } + c.App.Srv().GetTelemetryService().SendTelemetry("product_start_redirect", properties) redirectURL += url } } + http.Redirect(w, r, redirectURL, http.StatusFound) } From 93f17980a113f3f476a0614be367a52b17f49f89 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 29 Dec 2022 12:21:25 +0530 Subject: [PATCH 02/13] Remove unnecessary debug log (#21959) This log is continuously spamming our community server and I see it all the time in my dev environment. Let's remove this to keep the logs clean. ```release-note NONE ``` --- api4/drafts.go | 1 - 1 file changed, 1 deletion(-) diff --git a/api4/drafts.go b/api4/drafts.go index 102c164b8f..c3640f7a84 100644 --- a/api4/drafts.go +++ b/api4/drafts.go @@ -124,7 +124,6 @@ func deleteDraft(c *Context, w http.ResponseWriter, r *http.Request) { switch { case err.StatusCode == http.StatusNotFound: // If the draft doesn't exist in the server, we don't need to delete. - mlog.Debug("Unable to find the draft", mlog.Err(err)) ReturnStatusOK(w) default: c.Err = err From babe01ee2ed4440499a699519b6407ad5de515d1 Mon Sep 17 00:00:00 2001 From: Vishal Date: Fri, 30 Dec 2022 14:41:31 +0530 Subject: [PATCH 03/13] remove crt feature flag (#21788) Co-authored-by: Mattermod --- api4/channel_test.go | 3 --- api4/post_test.go | 3 --- api4/user_test.go | 20 ++++---------------- app/channel_test.go | 9 ++------- app/notification.go | 2 +- app/notification_test.go | 4 +--- app/post_test.go | 16 ---------------- app/user_test.go | 3 --- model/feature_flags.go | 4 ---- 9 files changed, 8 insertions(+), 56 deletions(-) diff --git a/api4/channel_test.go b/api4/channel_test.go index 16e432bf0d..6f8f5aeabe 100644 --- a/api4/channel_test.go +++ b/api4/channel_test.go @@ -8,7 +8,6 @@ import ( "encoding/json" "fmt" "net/http" - "os" "sort" "strings" "sync" @@ -4584,8 +4583,6 @@ func TestViewChannelWithoutCollapsedThreads(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn diff --git a/api4/post_test.go b/api4/post_test.go index 8594fae136..7f3ec6c442 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -12,7 +12,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "os" "reflect" "sort" "strings" @@ -2954,8 +2953,6 @@ func TestSetChannelUnread(t *testing.T) { } func TestSetPostUnreadWithoutCollapsedThreads(t *testing.T) { - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th := Setup(t).InitBasic() defer th.TearDown() th.App.UpdateConfig(func(cfg *model.Config) { diff --git a/api4/user_test.go b/api4/user_test.go index affd768fd7..a20ae682a1 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -5794,8 +5794,6 @@ func TestUpdatePassword(t *testing.T) { func TestGetThreadsForUser(t *testing.T) { os.Setenv("MM_FEATUREFLAGS_POSTPRIORITY", "true") defer os.Unsetenv("MM_FEATUREFLAGS_POSTPRIORITY") - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th := Setup(t).InitBasic() defer th.TearDown() @@ -6151,8 +6149,6 @@ func TestGetThreadsForUser(t *testing.T) { func TestThreadSocketEvents(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.ConfigStore.SetReadOnlyFF(false) defer th.ConfigStore.SetReadOnlyFF(true) @@ -6533,8 +6529,6 @@ func TestMaintainUnreadRepliesInThread(t *testing.T) { defer th.UnlinkUserFromTeam(th.SystemAdminUser, th.BasicTeam) th.AddUserToChannel(th.SystemAdminUser, th.BasicChannel) defer th.RemoveUserFromChannel(th.SystemAdminUser, th.BasicChannel) - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn @@ -6590,8 +6584,7 @@ func TestMaintainUnreadRepliesInThread(t *testing.T) { func TestThreadCounts(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn @@ -6633,8 +6626,6 @@ func TestThreadCounts(t *testing.T) { func TestSingleThreadGet(t *testing.T) { os.Setenv("MM_FEATUREFLAGS_POSTPRIORITY", "true") defer os.Unsetenv("MM_FEATUREFLAGS_POSTPRIORITY") - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th := Setup(t).InitBasic() defer th.TearDown() @@ -6704,8 +6695,7 @@ func TestMaintainUnreadMentionsInThread(t *testing.T) { th.AddUserToChannel(th.SystemAdminUser, th.BasicChannel) defer th.RemoveUserFromChannel(th.SystemAdminUser, th.BasicChannel) client := th.Client - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn @@ -6768,8 +6758,7 @@ func TestMaintainUnreadMentionsInThread(t *testing.T) { func TestReadThreads(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn @@ -6872,8 +6861,7 @@ func TestReadThreads(t *testing.T) { func TestMarkThreadUnreadMentionCount(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn diff --git a/app/channel_test.go b/app/channel_test.go index bf21274763..af162e45b9 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net/http" - "os" "sort" "strings" "sync" @@ -2127,8 +2126,7 @@ func TestViewChannelCollapsedThreadsTurnedOff(t *testing.T) { th.AddUserToChannel(u2, c1) // Enable CRT - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn @@ -2198,8 +2196,6 @@ func TestViewChannelCollapsedThreadsTurnedOff(t *testing.T) { func TestMarkChannelAsUnreadFromPostCollapsedThreadsTurnedOff(t *testing.T) { // Enable CRT - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th := Setup(t).InitBasic() defer th.TearDown() @@ -2286,8 +2282,7 @@ func TestMarkChannelAsUnreadFromPostCollapsedThreadsTurnedOff(t *testing.T) { } func TestMarkUnreadCRTOffUpdatesThreads(t *testing.T) { - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th := Setup(t).InitBasic() defer th.TearDown() th.App.UpdateConfig(func(cfg *model.Config) { diff --git a/app/notification.go b/app/notification.go index cd275134d4..215945405c 100644 --- a/app/notification.go +++ b/app/notification.go @@ -43,7 +43,7 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea return []string{}, nil } - isCRTAllowed := a.Config().FeatureFlags.CollapsedThreads && *a.Config().ServiceSettings.CollapsedThreads != model.CollapsedThreadsDisabled + isCRTAllowed := *a.Config().ServiceSettings.CollapsedThreads != model.CollapsedThreadsDisabled pchan := make(chan store.StoreResult, 1) go func() { diff --git a/app/notification_test.go b/app/notification_test.go index 1563be191d..f00c038dac 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -5,7 +5,6 @@ package app import ( "fmt" - "os" "testing" "github.com/stretchr/testify/assert" @@ -2734,8 +2733,7 @@ func TestReplyPostNotificationsWithCRT(t *testing.T) { }() // Enable CRT - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true *cfg.ServiceSettings.CollapsedThreads = model.CollapsedThreadsDefaultOn diff --git a/app/post_test.go b/app/post_test.go index 0b9c37a3f8..d3073fe302 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -2357,8 +2357,6 @@ func TestThreadMembership(t *testing.T) { func TestFollowThreadSkipsParticipants(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true @@ -2415,8 +2413,6 @@ func TestFollowThreadSkipsParticipants(t *testing.T) { func TestAutofollowBasedOnRootPost(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true @@ -2445,8 +2441,6 @@ func TestAutofollowBasedOnRootPost(t *testing.T) { func TestViewChannelShouldNotUpdateThreads(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true @@ -2528,12 +2522,6 @@ func TestCollapsedThreadFetch(t *testing.T) { }) t.Run("Should not panic on unexpected db error", func(t *testing.T) { - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") - th.App.UpdateConfig(func(cfg *model.Config) { - cfg.FeatureFlags.CollapsedThreads = true - }) - channel := th.CreateChannel(th.Context, th.BasicTeam) th.AddUserToChannel(user2, channel) defer th.App.DeleteChannel(th.Context, channel, user1.Id) @@ -2765,8 +2753,6 @@ func TestSharedChannelSyncForPostActions(t *testing.T) { func TestAutofollowOnPostingAfterUnfollow(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true @@ -2834,8 +2820,6 @@ func TestGetPostIfAuthorized(t *testing.T) { func TestShouldNotRefollowOnOthersReply(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ThreadAutoFollow = true diff --git a/app/user_test.go b/app/user_test.go index 788ae11275..8a71680a20 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "errors" - "os" "path/filepath" "strings" "testing" @@ -1673,8 +1672,6 @@ func TestPatchUser(t *testing.T) { } func TestUpdateThreadReadForUser(t *testing.T) { - os.Setenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS", "true") - defer os.Unsetenv("MM_FEATUREFLAGS_COLLAPSEDTHREADS") t.Run("Ensure thread membership is created and followed", func(t *testing.T) { th := Setup(t).InitBasic() diff --git a/model/feature_flags.go b/model/feature_flags.go index 27bab4830e..47dd10378d 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -16,9 +16,6 @@ type FeatureFlags struct { // all other values as false. TestBoolFeature bool - // Toggle on and off support for Collapsed Threads - CollapsedThreads bool - // Enable the remote cluster service for shared channels. EnableRemoteClusterService bool @@ -81,7 +78,6 @@ type FeatureFlags struct { func (f *FeatureFlags) SetDefaults() { f.TestFeature = "off" f.TestBoolFeature = false - f.CollapsedThreads = true f.EnableRemoteClusterService = false f.AppsEnabled = true f.PluginApps = "" From f1e96cb83076ba5f6ad6d3f9a73264e1baaa33f7 Mon Sep 17 00:00:00 2001 From: Luis Suarez Date: Mon, 2 Jan 2023 18:07:38 -0500 Subject: [PATCH 04/13] MM-48988 Log plugins version on install and on startup (#21907) --- app/plugin_install.go | 2 ++ plugin/environment.go | 2 ++ 2 files changed, 4 insertions(+) diff --git a/app/plugin_install.go b/app/plugin_install.go index 431c3abebe..b35982b82c 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -402,6 +402,8 @@ func (ch *Channels) installExtractedPlugin(manifest *model.Manifest, fromPluginD manifest = updatedManifest } + mlog.Debug("Installing plugin", mlog.String("plugin_id", manifest.Id), mlog.String("version", manifest.Version)) + return manifest, nil } diff --git a/plugin/environment.go b/plugin/environment.go index 4ccbbdf023..cf7553579d 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -330,6 +330,8 @@ func (env *Environment) Activate(id string) (manifest *model.Manifest, activated return nil, false, fmt.Errorf("unable to start plugin: must at least have a web app or server component") } + mlog.Debug("Plugin activated", mlog.String("plugin_id", pluginInfo.Manifest.Id), mlog.String("version", pluginInfo.Manifest.Version)) + return pluginInfo.Manifest, true, nil } From 1353ebc3abf34c8767bf776e618361b870145bca Mon Sep 17 00:00:00 2001 From: Nathaniel Allred Date: Tue, 3 Jan 2023 08:36:23 -0600 Subject: [PATCH 05/13] Mm 49258 - Inform admins trying to self-hosted purchase who the email listed on the JWT claim is (#21964) * add email of JWT claim to bootstrap signup response --- model/hosted_customer.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/model/hosted_customer.go b/model/hosted_customer.go index 0e1373a2bc..4f1917bdaf 100644 --- a/model/hosted_customer.go +++ b/model/hosted_customer.go @@ -10,6 +10,8 @@ type BootstrapSelfHostedSignupRequest struct { type BootstrapSelfHostedSignupResponse struct { Progress string `json:"progress"` + // email listed on the JWT claim + Email string `json:"email"` } type BootstrapSelfHostedSignupResponseInternal struct { From 6ad18615352fd9b7f36fb00072a25968e1abc00a Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 3 Jan 2023 20:14:16 +0530 Subject: [PATCH 06/13] MM-48345: Fix Critical level logging (#21952) Critical wasn't added in the StdLevels slice and due to that mlog.Critical wasn't printing anything. This was a complete blindspot that was missed. We have removed all references to mlog.Critical in the codebase and pointed Critical to be Fatal in the library. In v8, we will remove Critical altogether. ```release-note Servers with an encrypted key will throw an error during startup now. ``` --- app/migrations.go | 74 ++++++++++++++--------------- app/platform/metrics.go | 2 +- app/server.go | 6 +-- services/upgrader/upgrader_linux.go | 6 +-- shared/mlog/global.go | 9 ++-- shared/mlog/global_test.go | 14 ++---- 6 files changed, 51 insertions(+), 60 deletions(-) diff --git a/app/migrations.go b/app/migrations.go index 21295aa840..70c11b75fc 100644 --- a/app/migrations.go +++ b/app/migrations.go @@ -46,7 +46,7 @@ func (s *Server) doAdvancedPermissionsMigration() { // If this failed for reasons other than the role already existing, don't mark the migration as done. fetchedRole, err := s.Store().Role().GetByName(context.Background(), role.Name) if err != nil { - mlog.Critical("Failed to migrate role to database.", mlog.Err(err)) + mlog.Fatal("Failed to migrate role to database.", mlog.Err(err)) allSucceeded = false continue } @@ -59,7 +59,7 @@ func (s *Server) doAdvancedPermissionsMigration() { role.Id = fetchedRole.Id if _, err = s.Store().Role().Save(role); err != nil { // Role is not the same, but failed to update. - mlog.Critical("Failed to migrate role to database.", mlog.Err(err)) + mlog.Fatal("Failed to migrate role to database.", mlog.Err(err)) allSucceeded = false } } @@ -81,7 +81,7 @@ func (s *Server) doAdvancedPermissionsMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark advanced permissions migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark advanced permissions migration as completed.", mlog.Err(err)) } } @@ -114,21 +114,21 @@ func (s *Server) doEmojisPermissionsMigration() { // Emoji creation is set to all by default role, err = s.GetRoleByName(context.Background(), model.SystemUserRoleId) if err != nil { - mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) + mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) return } if role != nil { role.Permissions = append(role.Permissions, model.PermissionCreateEmojis.Id, model.PermissionDeleteEmojis.Id) if _, nErr := s.Store().Role().Save(role); nErr != nil { - mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr)) + mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(nErr)) return } } systemAdminRole, err = s.GetRoleByName(context.Background(), model.SystemAdminRoleId) if err != nil { - mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) + mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) return } @@ -138,7 +138,7 @@ func (s *Server) doEmojisPermissionsMigration() { model.PermissionDeleteOthersEmojis.Id, ) if _, err := s.Store().Role().Save(systemAdminRole); err != nil { - mlog.Critical("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) + mlog.Fatal("Failed to migrate emojis creation permissions from mattermost config.", mlog.Err(err)) return } @@ -148,7 +148,7 @@ func (s *Server) doEmojisPermissionsMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark emojis permissions migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark emojis permissions migration as completed.", mlog.Err(err)) } } @@ -167,26 +167,26 @@ func (s *Server) doGuestRolesCreationMigration() { allSucceeded := true if _, err := s.Store().Role().GetByName(context.Background(), model.ChannelGuestRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.ChannelGuestRoleId]); err != nil { - mlog.Critical("Failed to create new guest role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.TeamGuestRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.TeamGuestRoleId]); err != nil { - mlog.Critical("Failed to create new guest role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.SystemGuestRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.SystemGuestRoleId]); err != nil { - mlog.Critical("Failed to create new guest role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new guest role to database.", mlog.Err(err)) allSucceeded = false } } schemes, err := s.Store().Scheme().GetAllPage("", 0, 1000000) if err != nil { - mlog.Critical("Failed to get all schemes.", mlog.Err(err)) + mlog.Fatal("Failed to get all schemes.", mlog.Err(err)) allSucceeded = false } for _, scheme := range schemes { @@ -201,7 +201,7 @@ func (s *Server) doGuestRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(teamGuestRole); err != nil { - mlog.Critical("Failed to create new guest role for custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new guest role for custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultTeamGuestRole = savedRole.Name @@ -217,7 +217,7 @@ func (s *Server) doGuestRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(channelGuestRole); err != nil { - mlog.Critical("Failed to create new guest role for custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new guest role for custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultChannelGuestRole = savedRole.Name @@ -225,7 +225,7 @@ func (s *Server) doGuestRolesCreationMigration() { _, err := s.Store().Scheme().Save(scheme) if err != nil { - mlog.Critical("Failed to update custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to update custom scheme.", mlog.Err(err)) allSucceeded = false } } @@ -241,7 +241,7 @@ func (s *Server) doGuestRolesCreationMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark guest roles creation migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark guest roles creation migration as completed.", mlog.Err(err)) } } @@ -260,19 +260,19 @@ func (s *Server) doSystemConsoleRolesCreationMigration() { allSucceeded := true if _, err := s.Store().Role().GetByName(context.Background(), model.SystemManagerRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.SystemManagerRoleId]); err != nil { - mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemManagerRoleId)) + mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemManagerRoleId)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.SystemReadOnlyAdminRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.SystemReadOnlyAdminRoleId]); err != nil { - mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemReadOnlyAdminRoleId)) + mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemReadOnlyAdminRoleId)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.SystemUserManagerRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.SystemUserManagerRoleId]); err != nil { - mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemUserManagerRoleId)) + mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemUserManagerRoleId)) allSucceeded = false } } @@ -287,7 +287,7 @@ func (s *Server) doSystemConsoleRolesCreationMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark system console roles creation migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark system console roles creation migration as completed.", mlog.Err(err)) } } @@ -302,7 +302,7 @@ func (s *Server) doCustomGroupAdminRoleCreationMigration() { allSucceeded := true if _, err := s.Store().Role().GetByName(context.Background(), model.SystemCustomGroupAdminRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.SystemCustomGroupAdminRoleId]); err != nil { - mlog.Critical("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemCustomGroupAdminRoleId)) + mlog.Fatal("Failed to create new role.", mlog.Err(err), mlog.String("role", model.SystemCustomGroupAdminRoleId)) allSucceeded = false } } @@ -317,7 +317,7 @@ func (s *Server) doCustomGroupAdminRoleCreationMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark custom group admin role creation migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark custom group admin role creation migration as completed.", mlog.Err(err)) } } @@ -337,7 +337,7 @@ func (s *Server) doContentExtractionConfigDefaultTrueMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark content extraction config migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark content extraction config migration as completed.", mlog.Err(err)) } } @@ -352,31 +352,31 @@ func (s *Server) doPlaybooksRolesCreationMigration() { allSucceeded := true if _, err := s.Store().Role().GetByName(context.Background(), model.PlaybookAdminRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.PlaybookAdminRoleId]); err != nil { - mlog.Critical("Failed to create new playbook admin role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new playbook admin role to database.", mlog.Err(err)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.PlaybookMemberRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.PlaybookMemberRoleId]); err != nil { - mlog.Critical("Failed to create new playbook member role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new playbook member role to database.", mlog.Err(err)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.RunAdminRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.RunAdminRoleId]); err != nil { - mlog.Critical("Failed to create new run admin role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new run admin role to database.", mlog.Err(err)) allSucceeded = false } } if _, err := s.Store().Role().GetByName(context.Background(), model.RunMemberRoleId); err != nil { if _, err := s.Store().Role().Save(roles[model.RunMemberRoleId]); err != nil { - mlog.Critical("Failed to create new run member role to database.", mlog.Err(err)) + mlog.Fatal("Failed to create new run member role to database.", mlog.Err(err)) allSucceeded = false } } schemes, err := s.Store().Scheme().GetAllPage(model.SchemeScopeTeam, 0, 1000000) if err != nil { - mlog.Critical("Failed to get all schemes.", mlog.Err(err)) + mlog.Fatal("Failed to get all schemes.", mlog.Err(err)) allSucceeded = false } @@ -391,7 +391,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(playbookAdminRole); err != nil { - mlog.Critical("Failed to create new playbook admin role for existing custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new playbook admin role for existing custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultPlaybookAdminRole = savedRole.Name @@ -406,7 +406,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(playbookMember); err != nil { - mlog.Critical("Failed to create new playbook member role for existing custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new playbook member role for existing custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultPlaybookMemberRole = savedRole.Name @@ -422,7 +422,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(runAdminRole); err != nil { - mlog.Critical("Failed to create new run admin role for existing custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new run admin role for existing custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultRunAdminRole = savedRole.Name @@ -438,7 +438,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } if savedRole, err := s.Store().Role().Save(runMemberRole); err != nil { - mlog.Critical("Failed to create new run member role for existing custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to create new run member role for existing custom scheme.", mlog.Err(err)) allSucceeded = false } else { scheme.DefaultRunMemberRole = savedRole.Name @@ -446,7 +446,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } _, err := s.Store().Scheme().Save(scheme) if err != nil { - mlog.Critical("Failed to update custom scheme.", mlog.Err(err)) + mlog.Fatal("Failed to update custom scheme.", mlog.Err(err)) allSucceeded = false } } @@ -462,7 +462,7 @@ func (s *Server) doPlaybooksRolesCreationMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark playbook roles creation migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark playbook roles creation migration as completed.", mlog.Err(err)) } } @@ -507,7 +507,7 @@ func (s *Server) doFirstAdminSetupCompleteMigration() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark first admin setup migration as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark first admin setup migration as completed.", mlog.Err(err)) } } @@ -534,7 +534,7 @@ func (s *Server) doRemainingSchemaMigrations() { } if err := s.Store().System().Save(&system); err != nil { - mlog.Critical("Failed to mark the remaining schema migrations as completed.", mlog.Err(err)) + mlog.Fatal("Failed to mark the remaining schema migrations as completed.", mlog.Err(err)) } } @@ -552,7 +552,7 @@ func (s *Server) doAppMigrations() { // migrations. For example, it needs the guest roles migration. err := s.doPermissionsMigrations() if err != nil { - mlog.Critical("(app.App).DoPermissionsMigrations failed", mlog.Err(err)) + mlog.Fatal("(app.App).DoPermissionsMigrations failed", mlog.Err(err)) } s.doContentExtractionConfigDefaultTrueMigration() s.doPlaybooksRolesCreationMigration() diff --git a/app/platform/metrics.go b/app/platform/metrics.go index 2253594011..f30edb5093 100644 --- a/app/platform/metrics.go +++ b/app/platform/metrics.go @@ -111,7 +111,7 @@ func (pm *platformMetrics) startMetricsServer() error { go func() { close(notify) if err := pm.server.Serve(l); err != nil && err != http.ErrServerClosed { - pm.logger.Critical(err.Error()) + pm.logger.Fatal(err.Error()) } }() diff --git a/app/server.go b/app/server.go index fbd433a6db..dc66353ee4 100644 --- a/app/server.go +++ b/app/server.go @@ -1059,7 +1059,7 @@ func (s *Server) Start() error { } if err != nil && err != http.ErrServerClosed { - mlog.Critical("Error starting server", mlog.Err(err)) + mlog.Fatal("Error starting server", mlog.Err(err)) time.Sleep(time.Second) } @@ -1068,7 +1068,7 @@ func (s *Server) Start() error { if *s.platform.Config().ServiceSettings.EnableLocalMode { if err := s.startLocalModeServer(); err != nil { - mlog.Critical(err.Error()) + mlog.Fatal(err.Error()) } } @@ -1100,7 +1100,7 @@ func (s *Server) startLocalModeServer() error { go func() { err = s.localModeServer.Serve(unixListener) if err != nil && err != http.ErrServerClosed { - mlog.Critical("Error starting unix socket server", mlog.Err(err)) + mlog.Fatal("Error starting unix socket server", mlog.Err(err)) } }() return nil diff --git a/services/upgrader/upgrader_linux.go b/services/upgrader/upgrader_linux.go index 1f895480ea..04ad1bcf82 100644 --- a/services/upgrader/upgrader_linux.go +++ b/services/upgrader/upgrader_linux.go @@ -333,7 +333,7 @@ func extractBinary(executablePath string, filename string) error { if err != nil { err2 := os.Rename(tmpFileName, executablePath) if err2 != nil { - mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.") + mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.") return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually") } return err @@ -342,13 +342,13 @@ func extractBinary(executablePath string, filename string) error { if _, err = io.Copy(outFile, tarReader); err != nil { err2 := os.Remove(executablePath) if err2 != nil { - mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.") + mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.") return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually") } err2 = os.Rename(tmpFileName, executablePath) if err2 != nil { - mlog.Critical("Unable to restore the backup of the executable file. Restore the executable file manually.") + mlog.Fatal("Unable to restore the backup of the executable file. Restore the executable file manually.") return errors.Wrap(err2, "critical error: unable to upgrade the binary or restore the old binary version. Please restore it manually") } return err diff --git a/shared/mlog/global.go b/shared/mlog/global.go index de346123aa..71d7430a42 100644 --- a/shared/mlog/global.go +++ b/shared/mlog/global.go @@ -113,13 +113,10 @@ func Error(msg string, fields ...Field) { // Convenience method equivalent to calling `Log` with the `Critical` level. // DEPRECATED: Either use Error or Fatal. +// Critical level isn't added in mlog/levels.go:StdAll so calling this doesn't +// really work. For now we just call Fatal to atleast print something. func Critical(msg string, fields ...Field) { - logger := getGlobalLogger() - if logger == nil { - defaultLog(LvlCritical, msg, fields...) - return - } - logger.Critical(msg, fields...) + Fatal(msg, fields...) } func Fatal(msg string, fields ...Field) { diff --git a/shared/mlog/global_test.go b/shared/mlog/global_test.go index d97b3b15a3..486387e5f6 100644 --- a/shared/mlog/global_test.go +++ b/shared/mlog/global_test.go @@ -24,7 +24,6 @@ func TestLoggingBeforeInitialized(t *testing.T) { mlog.Debug("debug log") mlog.Warn("warning log") mlog.Error("error log") - mlog.Critical("critical log") }) } @@ -40,14 +39,13 @@ func TestLoggingAfterInitialized(t *testing.T) { Type: "file", Format: "json", FormatOptions: json.RawMessage(`{"enable_caller":true}`), - Levels: []mlog.Level{mlog.LvlCritical, mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug}, + Levels: []mlog.Level{mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug}, }, []string{ `{"timestamp":0,"level":"debug","msg":"real debug log","caller":"mlog/global_test.go:0"}`, `{"timestamp":0,"level":"info","msg":"real info log","caller":"mlog/global_test.go:0"}`, `{"timestamp":0,"level":"warn","msg":"real warning log","caller":"mlog/global_test.go:0"}`, `{"timestamp":0,"level":"error","msg":"real error log","caller":"mlog/global_test.go:0"}`, - `{"timestamp":0,"level":"critical","msg":"real critical log","caller":"mlog/global_test.go:0"}`, }, }, { @@ -56,11 +54,10 @@ func TestLoggingAfterInitialized(t *testing.T) { Type: "file", Format: "json", FormatOptions: json.RawMessage(`{"enable_caller":true}`), - Levels: []mlog.Level{mlog.LvlCritical, mlog.LvlError}, + Levels: []mlog.Level{mlog.LvlError}, }, []string{ `{"timestamp":0,"level":"error","msg":"real error log","caller":"mlog/global_test.go:0"}`, - `{"timestamp":0,"level":"critical","msg":"real critical log","caller":"mlog/global_test.go:0"}`, }, }, { @@ -69,14 +66,13 @@ func TestLoggingAfterInitialized(t *testing.T) { Type: "file", Format: "plain", FormatOptions: json.RawMessage(`{"delim":" | ", "enable_caller":true}`), - Levels: []mlog.Level{mlog.LvlCritical, mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug}, + Levels: []mlog.Level{mlog.LvlError, mlog.LvlWarn, mlog.LvlInfo, mlog.LvlDebug}, }, []string{ `debug | TIME | real debug log | caller="mlog/global_test.go:0"`, `info | TIME | real info log | caller="mlog/global_test.go:0"`, `warn | TIME | real warning log | caller="mlog/global_test.go:0"`, `error | TIME | real error log | caller="mlog/global_test.go:0"`, - `critical | TIME | real critical log | caller="mlog/global_test.go:0"`, }, }, { @@ -85,11 +81,10 @@ func TestLoggingAfterInitialized(t *testing.T) { Type: "file", Format: "plain", FormatOptions: json.RawMessage(`{"delim":" | ", "enable_caller":true}`), - Levels: []mlog.Level{mlog.LvlCritical, mlog.LvlError}, + Levels: []mlog.Level{mlog.LvlError}, }, []string{ `error | TIME | real error log | caller="mlog/global_test.go:0"`, - `critical | TIME | real critical log | caller="mlog/global_test.go:0"`, }, }, } @@ -116,7 +111,6 @@ func TestLoggingAfterInitialized(t *testing.T) { mlog.Info("real info log") mlog.Warn("real warning log") mlog.Error("real error log") - mlog.Critical("real critical log") logger.Shutdown() From 356188de2884ae51f8c0747ecfe85793f171ae65 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 3 Jan 2023 20:14:45 +0530 Subject: [PATCH 07/13] Clean untranslated logs (#21956) Remove the word "" from appearing in the logs. ```release-note NONE ``` --- model/utils.go | 8 ++++++-- model/utils_test.go | 5 +++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/model/utils.go b/model/utils.go index a8d82e3669..4ed85e2b32 100644 --- a/model/utils.go +++ b/model/utils.go @@ -256,11 +256,15 @@ func (er *AppError) Error() string { // render the error information sb.WriteString(er.Where) sb.WriteString(": ") - sb.WriteString(er.Message) + if er.Message != NoTranslation { + sb.WriteString(er.Message) + } // only render the detailed error when it's present if er.DetailedError != "" { - sb.WriteString(", ") + if er.Message != NoTranslation { + sb.WriteString(", ") + } sb.WriteString(er.DetailedError) } diff --git a/model/utils_test.go b/model/utils_test.go index 1b96f44516..606477d750 100644 --- a/model/utils_test.go +++ b/model/utils_test.go @@ -81,6 +81,11 @@ func TestAppError(t *testing.T) { t.Log(appErr.Error()) } +func TestAppErrorNoTranslation(t *testing.T) { + appErr := NewAppError("TestAppError", NoTranslation, nil, "test error", http.StatusBadRequest) + require.Equal(t, "TestAppError: test error", appErr.Error()) +} + func TestAppErrorJunk(t *testing.T) { rerr := AppErrorFromJSON(strings.NewReader("This is a broken test")) require.Equal(t, "body: This is a broken test", rerr.DetailedError) From e4b050693db226304939fe2b171d33f19487766e Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 3 Jan 2023 21:30:51 +0530 Subject: [PATCH 08/13] Return 404 when thread membership not found (#21960) We were incorrectly returning by checking for `err != nil` when a 404 error was included in that condition. ```release-note NONE ``` --- app/post_test.go | 5 +++++ app/user.go | 31 +++++++++++++++++++------------ app/user_test.go | 5 +++++ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/app/post_test.go b/app/post_test.go index d3073fe302..4630eb75af 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -2408,6 +2408,11 @@ func TestFollowThreadSkipsParticipants(t *testing.T) { for _, p := range thread.Participants { require.True(t, p.Id == sysadmin.Id || p.Id == user.Id) } + + threadMembership.PostId = "notfound" + _, err = th.App.GetThreadForUser(threadMembership, false) + require.NotNil(t, err) + assert.Equal(t, http.StatusNotFound, err.StatusCode) } func TestAutofollowBasedOnRootPost(t *testing.T) { diff --git a/app/user.go b/app/user.go index d259439859..991457f7ba 100644 --- a/app/user.go +++ b/app/user.go @@ -2484,24 +2484,31 @@ func (a *App) GetThreadsForUser(userID, teamID string, options model.GetUserThre } func (a *App) GetThreadMembershipForUser(userId, threadId string) (*model.ThreadMembership, *model.AppError) { - threadMembership, err := a.Srv().Store().Thread().GetMembershipForUser(userId, threadId) - if err != nil { - return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - if threadMembership == nil { - return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.not_found", nil, "thread membership not found/followed", http.StatusNotFound) + threadMembership, nErr := a.Srv().Store().Thread().GetMembershipForUser(userId, threadId) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.not_found", nil, "", http.StatusNotFound).Wrap(nErr) + default: + return nil, model.NewAppError("GetThreadMembershipForUser", "app.user.get_thread_membership_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr) + } } return threadMembership, nil } func (a *App) GetThreadForUser(threadMembership *model.ThreadMembership, extended bool) (*model.ThreadResponse, *model.AppError) { - thread, err := a.Srv().Store().Thread().GetThreadForUser(threadMembership, extended, a.isPostPriorityEnabled()) - if err != nil { - return nil, model.NewAppError("GetThreadForUser", "app.user.get_threads_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(err) - } - if thread == nil { - return nil, model.NewAppError("GetThreadForUser", "app.user.get_threads_for_user.not_found", nil, "thread not found/followed", http.StatusNotFound) + thread, nErr := a.Srv().Store().Thread().GetThreadForUser(threadMembership, extended, a.isPostPriorityEnabled()) + if nErr != nil { + var nfErr *store.ErrNotFound + switch { + case errors.As(nErr, &nfErr): + return nil, model.NewAppError("GetThreadForUser", "app.user.get_threads_for_user.not_found", nil, "thread not found/followed", http.StatusNotFound) + default: + return nil, model.NewAppError("GetThreadForUser", "app.user.get_threads_for_user.app_error", nil, "", http.StatusInternalServerError).Wrap(nErr) + } } + a.sanitizeProfiles(thread.Participants, false) thread.Post.SanitizeProps() return thread, nil diff --git a/app/user_test.go b/app/user_test.go index 8a71680a20..086e71e25f 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -8,6 +8,7 @@ import ( "context" "encoding/json" "errors" + "net/http" "path/filepath" "strings" "testing" @@ -1700,6 +1701,10 @@ func TestUpdateThreadReadForUser(t *testing.T) { require.Nil(t, appErr) require.NotNil(t, threadMembership) assert.True(t, threadMembership.Following) + + _, appErr = th.App.GetThreadMembershipForUser(th.BasicUser.Id, "notfound") + require.NotNil(t, appErr) + assert.Equal(t, http.StatusNotFound, appErr.StatusCode) }) t.Run("Ensure no panic on error", func(t *testing.T) { From ed23df6a2e48a4b229e8fb37c28104304b31b91d Mon Sep 17 00:00:00 2001 From: Mylon Suren <23694620+mylonsuren@users.noreply.github.com> Date: Tue, 3 Jan 2023 16:30:30 -0500 Subject: [PATCH 09/13] [MM-47489] Move Shared Channels (Experimental) to Professional (#21882) * Move shared channels to professional license * Remove references to SharedChannels from license and use license SKU instead * add tests * Refactor shared channels license check and add tests * Re-add removed negation on license check Co-authored-by: Mattermod --- app/server.go | 2 +- config/client.go | 2 +- config/client_test.go | 72 +++++++++++++++++++++++++++++++++++++++++++ model/license.go | 10 ++++++ model/license_test.go | 50 ++++++++++++++++++++++++++++++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/app/server.go b/app/server.go index dc66353ee4..f7a9e9b912 100644 --- a/app/server.go +++ b/app/server.go @@ -595,7 +595,7 @@ func (s *Server) startInterClusterServices(license *model.License) error { // Shared Channels service // License check - if !*license.Features.SharedChannels { + if !license.HasSharedChannels() { mlog.Debug("License does not have shared channels enabled") return nil } diff --git a/config/client.go b/config/client.go index e26c2bea40..6ce5433dc1 100644 --- a/config/client.go +++ b/config/client.go @@ -196,7 +196,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li props["DataRetentionBoardsRetentionDays"] = strconv.FormatInt(int64(*c.DataRetentionSettings.BoardsRetentionDays), 10) } - if *license.Features.SharedChannels { + if license.HasSharedChannels() { props["ExperimentalSharedChannels"] = strconv.FormatBool(*c.ExperimentalSettings.EnableSharedChannels) props["ExperimentalRemoteClusterService"] = strconv.FormatBool(c.FeatureFlags.EnableRemoteClusterService && *c.ExperimentalSettings.EnableRemoteClusterService) } diff --git a/config/client_test.go b/config/client_test.go index a13b9cea77..9c92cfe329 100644 --- a/config/client_test.go +++ b/config/client_test.go @@ -254,6 +254,78 @@ func TestGetClientConfig(t *testing.T) { "EnableCustomGroups": "false", }, }, + { + "Shared channels other license", + &model.Config{ + ExperimentalSettings: model.ExperimentalSettings{ + EnableSharedChannels: model.NewBool(true), + }, + }, + "", + &model.License{ + Features: &model.Features{ + SharedChannels: model.NewBool(false), + }, + SkuShortName: "other", + }, + map[string]string{ + "ExperimentalSharedChannels": "false", + }, + }, + { + "licensed for shared channels", + &model.Config{ + ExperimentalSettings: model.ExperimentalSettings{ + EnableSharedChannels: model.NewBool(true), + }, + }, + "", + &model.License{ + Features: &model.Features{ + SharedChannels: model.NewBool(true), + }, + SkuShortName: "other", + }, + map[string]string{ + "ExperimentalSharedChannels": "true", + }, + }, + { + "Shared channels professional license", + &model.Config{ + ExperimentalSettings: model.ExperimentalSettings{ + EnableSharedChannels: model.NewBool(true), + }, + }, + "", + &model.License{ + Features: &model.Features{ + SharedChannels: model.NewBool(false), + }, + SkuShortName: model.LicenseShortSkuProfessional, + }, + map[string]string{ + "ExperimentalSharedChannels": "true", + }, + }, + { + "Shared channels enterprise license", + &model.Config{ + ExperimentalSettings: model.ExperimentalSettings{ + EnableSharedChannels: model.NewBool(true), + }, + }, + "", + &model.License{ + Features: &model.Features{ + SharedChannels: model.NewBool(false), + }, + SkuShortName: model.LicenseShortSkuEnterprise, + }, + map[string]string{ + "ExperimentalSharedChannels": "true", + }, + }, } for _, testCase := range testCases { diff --git a/model/license.go b/model/license.go index 94f0b81da4..cf5c30a258 100644 --- a/model/license.go +++ b/model/license.go @@ -312,6 +312,16 @@ func (l *License) HasEnterpriseMarketplacePlugins() bool { l.SkuShortName == LicenseShortSkuEnterprise } +func (l *License) HasSharedChannels() bool { + if l == nil { + return false + } + + return (l.Features != nil && l.Features.SharedChannels != nil && *l.Features.SharedChannels) || + l.SkuShortName == LicenseShortSkuProfessional || + l.SkuShortName == LicenseShortSkuEnterprise +} + // NewTestLicense returns a license that expires in the future and has the given features. func NewTestLicense(features ...string) *License { ret := &License{ diff --git a/model/license_test.go b/model/license_test.go index 6319ccc8e9..1d1a5f1acf 100644 --- a/model/license_test.go +++ b/model/license_test.go @@ -343,3 +343,53 @@ func TestLicense_IsSanctionedTrial(t *testing.T) { assert.True(t, license.IsSanctionedTrial()) }) } + +func TestLicenseHasSharedChannels(t *testing.T) { + + testCases := []struct { + description string + license License + expectedValue bool + }{ + { + "licensed for shared channels", + License{ + Features: &Features{ + SharedChannels: NewBool(true), + }, + SkuShortName: "other", + }, + true, + }, + { + "not licensed for shared channels", + License{ + Features: &Features{}, + SkuShortName: "other", + }, + false, + }, + { + "professional license for shared channels", + License{ + Features: &Features{}, + SkuShortName: LicenseShortSkuProfessional, + }, + true, + }, + { + "enterprise license for shared channels", + License{ + Features: &Features{}, + SkuShortName: LicenseShortSkuEnterprise, + }, + true, + }, + } + + for _, testCase := range testCases { + t.Run(testCase.description, func(t *testing.T) { + assert.Equal(t, testCase.expectedValue, testCase.license.HasSharedChannels()) + }) + } +} From 254bc4f3a36771f9a1381356cfb1595aa8a00c85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Csaba=20T=C3=B3th=20//=20BDSC=20Business=20Digitalisation?= =?UTF-8?q?=20Kft?= Date: Tue, 3 Jan 2023 00:07:47 +0100 Subject: [PATCH 10/13] Translated using Weblate (Hungarian) Currently translated at 96.0% (2333 of 2428 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/hu/ --- i18n/hu.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/hu.json b/i18n/hu.json index c010bb1704..dc185e734b 100644 --- a/i18n/hu.json +++ b/i18n/hu.json @@ -9362,5 +9362,9 @@ { "id": "model.group.name.reserved_name.app_error", "translation": "csoport név létezik mint lefoglalt név" + }, + { + "id": "api.acknowledgement.delete.archived_channel.app_error", + "translation": "Nem lehet eltávolítani egy archivált csatornában lévő jóváhagyást." } ] From b61545804f9de03b6f7e41962d3c77e910f0fe7d Mon Sep 17 00:00:00 2001 From: Ji-Hyeon Gim Date: Tue, 3 Jan 2023 00:07:47 +0100 Subject: [PATCH 11/13] Translated using Weblate (Korean) Currently translated at 82.7% (2008 of 2428 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ko/ --- i18n/ko.json | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/i18n/ko.json b/i18n/ko.json index ad4733a1aa..69ea350a1b 100644 --- a/i18n/ko.json +++ b/i18n/ko.json @@ -1369,11 +1369,11 @@ }, { "id": "api.post.send_notification_and_forget.push_comment_on_post", - "translation": " 당신의 게시글에 답글이 달렸습니다." + "translation": " 당신의 게시글에 댓글이 달렸습니다." }, { "id": "api.post.send_notification_and_forget.push_comment_on_thread", - "translation": " 당신이 참여한 글타래에 답글이 달렸습니다." + "translation": " 당신이 참여한 글타래에 댓글이 달렸습니다." }, { "id": "api.post.send_notifications_and_forget.push_explicit_mention", @@ -4565,7 +4565,7 @@ }, { "id": "oauth.gitlab.tos.error", - "translation": "GitLab's Terms of Service have updated. Please go to gitlab.com to accept them and then try logging into Mattermost again." + "translation": "GitLab 서비스 약관이 업데이트되었습니다. {{.URL}}으로 이동하여 수락한 다음 Mattermost에 다시 로그인해주세요." }, { "id": "plugin.api.update_user_status.bad_status", @@ -4741,7 +4741,7 @@ }, { "id": "web.error.unsupported_browser.min_os_version.mac", - "translation": "macOS 10.14 이상" + "translation": "macOS 11 버전 이상" }, { "id": "web.error.unsupported_browser.min_browser_version.safari", @@ -4753,11 +4753,11 @@ }, { "id": "web.error.unsupported_browser.min_browser_version.edge", - "translation": "44버전 이상" + "translation": "95 버전 이상" }, { "id": "web.error.unsupported_browser.min_browser_version.chrome", - "translation": "100 버전 이상" + "translation": "106 버전 이상" }, { "id": "web.error.unsupported_browser.learn_more", @@ -7241,11 +7241,11 @@ }, { "id": "api.post.send_notification_and_forget.push_comment_on_crt_thread", - "translation": " 지켜보는 중인 글타래에 답글을 남겼습니다." + "translation": " 지켜보는 글타래에 댓글을 남겼습니다." }, { "id": "api.post.send_notification_and_forget.push_comment_on_crt_thread_dm", - "translation": " 글타래에 답글을 남겼습니다." + "translation": " 글타래에 댓글을 남겼습니다." }, { "id": "api.command_remote.missing_command", @@ -8034,5 +8034,9 @@ { "id": "app.user.update_threads_read_for_user.app_error", "translation": "모든 사용자 글타래들을 읽음 상태로 변경할 수 없습니다" + }, + { + "id": "api.admin.syncables_error", + "translation": "구성원을 그룹-팀 및 그룹-채널에 추가하지 못했습니다" } ] From d4de6e6120bebe57c4b8805d3a53d156d7a6df0a Mon Sep 17 00:00:00 2001 From: Konstantinos Pittas Date: Wed, 4 Jan 2023 21:49:15 +0200 Subject: [PATCH 12/13] [MM-23837] Support multiple users/channels in invite slash command (#21726) * accept multiple users and channels * remove logs * add translation for multiple * refactor * fix lint issues * rollback translations and use multiple messages * fix test * fix spacing * extract permission checking * improve check * improve error messages * rewrite tests for better clarity This way the environment is being rebuild so it starts (almost) fresh without interfering with each other * make errors non-blocking * simplify responses collector Co-authored-by: Mattermod --- app/slashcommands/command_invite.go | 247 +++++++++-------- app/slashcommands/command_invite_test.go | 326 +++++++++++++---------- i18n/en.json | 8 +- 3 files changed, 327 insertions(+), 254 deletions(-) diff --git a/app/slashcommands/command_invite.go b/app/slashcommands/command_invite.go index 3bf4931324..d4abda9054 100644 --- a/app/slashcommands/command_invite.go +++ b/app/slashcommands/command_invite.go @@ -10,7 +10,6 @@ import ( "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/shared/i18n" - "github.com/mattermost/mattermost-server/v6/shared/mlog" ) type InviteProvider struct { @@ -38,137 +37,177 @@ func (*InviteProvider) GetCommand(a *app.App, T i18n.TranslateFunc) *model.Comma } } -func (*InviteProvider) DoCommand(a *app.App, c request.CTX, args *model.CommandArgs, message string) *model.CommandResponse { +func (i *InviteProvider) DoCommand(a *app.App, c request.CTX, args *model.CommandArgs, message string) *model.CommandResponse { + return &model.CommandResponse{ + Text: i.doCommand(a, c, args, message), + ResponseType: model.CommandResponseTypeEphemeral, + } +} + +func (i *InviteProvider) doCommand(a *app.App, c request.CTX, args *model.CommandArgs, message string) string { if message == "" { - return &model.CommandResponse{ - Text: args.T("api.command_invite.missing_message.app_error"), - ResponseType: model.CommandResponseTypeEphemeral, + return args.T("api.command_invite.missing_message.app_error") + } + + resps := &[]string{} + + targetUsers, targetChannels, resp := i.parseMessage(a, c, args, resps, message) + if resp != "" { + return resp + } + + // Verify that the inviter has permissions to invite users to the every channel. + targetChannels = i.checkPermissions(a, c, args, resps, targetUsers[0], targetChannels) + + for _, targetUser := range targetUsers { + for _, targetChannel := range targetChannels { + if resp = i.addUserToChannel(a, c, args, targetUser, targetChannel); resp != "" { + *resps = append(*resps, resp) + continue + } + if args.ChannelId != targetChannel.Id { + *resps = append(*resps, args.T("api.command_invite.success", map[string]any{ + "User": targetUser.Username, + "Channel": targetChannel.Name, + })) + } } } - splitMessage := strings.SplitN(message, " ", 2) - targetUsername := splitMessage[0] - targetUsername = strings.TrimPrefix(targetUsername, "@") + if len(*resps) > 0 { + return strings.Join(*resps, "\n") + } - userProfile, nErr := a.Srv().Store().User().GetByUsername(targetUsername) - if nErr != nil { - mlog.Error(nErr.Error()) - return &model.CommandResponse{ - Text: args.T("api.command_invite.missing_user.app_error"), - ResponseType: model.CommandResponseTypeEphemeral, + return "" +} + +func (i *InviteProvider) parseMessage(a *app.App, c request.CTX, args *model.CommandArgs, resps *[]string, message string) ([]*model.User, []*model.Channel, string) { + splitMessage := strings.Split(message, " ") + + targetUsers := make([]*model.User, 0, 1) + targetChannels := make([]*model.Channel, 0) + + for j, msg := range splitMessage { + if msg == "" { + continue } + + if msg[0] == '@' || (msg[0] != '~' && j == 0) { + targetUsername := strings.TrimPrefix(msg, "@") + userProfile := i.getUserProfile(a, targetUsername) + if userProfile == nil { + *resps = append(*resps, args.T("api.command_invite.missing_user.app_error", map[string]any{ + "User": targetUsername, + })) + continue + } + targetUsers = append(targetUsers, userProfile) + } else { + targetChannelName := strings.TrimPrefix(msg, "~") + channelToJoin, err := a.GetChannelByName(c, targetChannelName, args.TeamId, false) + if err != nil { + *resps = append(*resps, args.T("api.command_invite.channel.error", map[string]any{ + "Channel": targetChannelName, + })) + continue + } + targetChannels = append(targetChannels, channelToJoin) + } + } + + if len(targetUsers) == 0 { + if len(*resps) != 0 { + return nil, nil, strings.Join(*resps, "\n") + } + return nil, nil, args.T("api.command_invite.missing_message.app_error") + } + + if len(targetChannels) == 0 { + if len(*resps) != 0 { + return nil, nil, strings.Join(*resps, "\n") + } + + channelToJoin, err := a.GetChannel(c, args.ChannelId) + if err != nil { + return nil, nil, args.T("api.command_invite.channel.app_error") + } + targetChannels = append(targetChannels, channelToJoin) + } + + return targetUsers, targetChannels, "" +} + +func (i *InviteProvider) getUserProfile(a *app.App, username string) *model.User { + userProfile, nErr := a.Srv().Store().User().GetByUsername(username) + if nErr != nil { + return nil } if userProfile.DeleteAt != 0 { - return &model.CommandResponse{ - Text: args.T("api.command_invite.missing_user.app_error"), - ResponseType: model.CommandResponseTypeEphemeral, - } + return nil } - var channelToJoin *model.Channel + return userProfile +} + +func (i *InviteProvider) checkPermissions(a *app.App, c request.CTX, args *model.CommandArgs, resps *[]string, targetUser *model.User, targetChannels []*model.Channel) []*model.Channel { var err *model.AppError - // User set a channel to add the invited user - if len(splitMessage) > 1 && splitMessage[1] != "" { - targetChannelName := strings.TrimPrefix(strings.TrimSpace(splitMessage[1]), "~") - - if channelToJoin, err = a.GetChannelByName(c, targetChannelName, args.TeamId, false); err != nil { - return &model.CommandResponse{ - Text: args.T("api.command_invite.channel.error", map[string]any{ - "Channel": targetChannelName, - }), - ResponseType: model.CommandResponseTypeEphemeral, + validChannels := make([]*model.Channel, 0, len(targetChannels)) + for _, targetChannel := range targetChannels { + switch targetChannel.Type { + case model.ChannelTypeOpen: + if !a.HasPermissionToChannel(c, args.UserId, targetChannel.Id, model.PermissionManagePublicChannelMembers) { + *resps = append(*resps, args.T("api.command_invite.permission.app_error", map[string]any{ + "User": targetUser.Username, + "Channel": targetChannel.Name, + })) + continue } - } - } else { - channelToJoin, err = a.GetChannel(c, args.ChannelId) - if err != nil { - return &model.CommandResponse{ - Text: args.T("api.command_invite.channel.app_error"), - ResponseType: model.CommandResponseTypeEphemeral, - } - } - } - - // Permissions Check - switch channelToJoin.Type { - case model.ChannelTypeOpen: - if !a.HasPermissionToChannel(c, args.UserId, channelToJoin.Id, model.PermissionManagePublicChannelMembers) { - return &model.CommandResponse{ - Text: args.T("api.command_invite.permission.app_error", map[string]any{ - "User": userProfile.Username, - "Channel": channelToJoin.Name, - }), - ResponseType: model.CommandResponseTypeEphemeral, - } - } - case model.ChannelTypePrivate: - if !a.HasPermissionToChannel(c, args.UserId, channelToJoin.Id, model.PermissionManagePrivateChannelMembers) { - if _, err = a.GetChannelMember(c, channelToJoin.Id, args.UserId); err == nil { - // User doing the inviting is a member of the channel. - return &model.CommandResponse{ - Text: args.T("api.command_invite.permission.app_error", map[string]any{ - "User": userProfile.Username, - "Channel": channelToJoin.Name, - }), - ResponseType: model.CommandResponseTypeEphemeral, + case model.ChannelTypePrivate: + if !a.HasPermissionToChannel(c, args.UserId, targetChannel.Id, model.PermissionManagePrivateChannelMembers) { + if _, err = a.GetChannelMember(c, targetChannel.Id, args.UserId); err == nil { + // User doing the inviting is a member of the channel. + *resps = append(*resps, args.T("api.command_invite.permission.app_error", map[string]any{ + "User": targetUser.Username, + "Channel": targetChannel.Name, + })) + continue } + // User doing the inviting is *not* a member of the channel. + *resps = append(*resps, args.T("api.command_invite.private_channel.app_error", map[string]any{ + "Channel": targetChannel.Name, + })) + continue } - // User doing the inviting is *not* a member of the channel. - return &model.CommandResponse{ - Text: args.T("api.command_invite.private_channel.app_error", map[string]any{ - "Channel": channelToJoin.Name, - }), - ResponseType: model.CommandResponseTypeEphemeral, - } - } - default: - return &model.CommandResponse{ - Text: args.T("api.command_invite.directchannel.app_error"), - ResponseType: model.CommandResponseTypeEphemeral, + default: + *resps = append(*resps, args.T("api.command_invite.directchannel.app_error")) + continue } + validChannels = append(validChannels, targetChannel) } + return validChannels +} +func (i *InviteProvider) addUserToChannel(a *app.App, c request.CTX, args *model.CommandArgs, userProfile *model.User, channelToJoin *model.Channel) string { // Check if user is already in the channel - _, err = a.GetChannelMember(c, channelToJoin.Id, userProfile.Id) + _, err := a.GetChannelMember(c, channelToJoin.Id, userProfile.Id) if err == nil { - return &model.CommandResponse{ - Text: args.T("api.command_invite.user_already_in_channel.app_error", map[string]any{ - "User": userProfile.Username, - }), - ResponseType: model.CommandResponseTypeEphemeral, - } + return args.T("api.command_invite.user_already_in_channel.app_error", map[string]any{ + "User": userProfile.Username, + }) } - if _, err := a.AddChannelMember(c, userProfile.Id, channelToJoin, app.ChannelMemberOpts{ - UserRequestorID: args.UserId, - }); err != nil { - var text string + if _, err = a.AddChannelMember(c, userProfile.Id, channelToJoin, app.ChannelMemberOpts{UserRequestorID: args.UserId}); err != nil { if err.Id == "api.channel.add_members.user_denied" { - text = args.T("api.command_invite.group_constrained_user_denied") + return args.T("api.command_invite.group_constrained_user_denied") } else if err.Id == "app.team.get_member.missing.app_error" || err.Id == "api.channel.add_user.to.channel.failed.deleted.app_error" { - text = args.T("api.command_invite.user_not_in_team.app_error", map[string]any{ + return args.T("api.command_invite.user_not_in_team.app_error", map[string]any{ "Username": userProfile.Username, }) - } else { - text = args.T("api.command_invite.fail.app_error") - } - return &model.CommandResponse{ - Text: text, - ResponseType: model.CommandResponseTypeEphemeral, } + return args.T("api.command_invite.fail.app_error") } - if args.ChannelId != channelToJoin.Id { - return &model.CommandResponse{ - Text: args.T("api.command_invite.success", map[string]any{ - "User": userProfile.Username, - "Channel": channelToJoin.Name, - }), - ResponseType: model.CommandResponseTypeEphemeral, - } - } - - return &model.CommandResponse{} + return "" } diff --git a/app/slashcommands/command_invite_test.go b/app/slashcommands/command_invite_test.go index 5de53899c0..5be71aaadf 100644 --- a/app/slashcommands/command_invite_test.go +++ b/app/slashcommands/command_invite_test.go @@ -17,46 +17,7 @@ func TestInviteProvider(t *testing.T) { th := setup(t).initBasic() defer th.tearDown() - channel := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) - privateChannel := th.createChannel(th.BasicTeam, model.ChannelTypePrivate) - dmChannel := th.createDmChannel(th.BasicUser2) - privateChannel2 := th.createChannelWithAnotherUser(th.BasicTeam, model.ChannelTypePrivate, th.BasicUser2.Id) - - basicUser3 := th.createUser() - th.linkUserToTeam(basicUser3, th.BasicTeam) - basicUser4 := th.createUser() - deactivatedUser := th.createUser() - th.App.UpdateActive(th.Context, deactivatedUser, false) - - var err *model.AppError - _, err = th.App.CreateBot(th.Context, &model.Bot{ - Username: "bot1", - OwnerId: basicUser3.Id, - Description: "a test bot", - }) - require.Nil(t, err) - - bot2, err := th.App.CreateBot(th.Context, &model.Bot{ - Username: "bot2", - OwnerId: basicUser3.Id, - Description: "a test bot", - }) - require.Nil(t, err) - _, _, err = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, bot2.UserId, basicUser3.Id) - require.Nil(t, err) - - bot3, err := th.App.CreateBot(th.Context, &model.Bot{ - Username: "bot3", - OwnerId: basicUser3.Id, - Description: "a test bot", - }) - require.Nil(t, err) - _, _, err = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, bot3.UserId, basicUser3.Id) - require.Nil(t, err) - err = th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, bot3.UserId, basicUser3.Id) - require.Nil(t, err) - - InviteP := InviteProvider{} + inviteProvider := InviteProvider{} args := &model.CommandArgs{ T: func(s string, args ...any) string { return s }, ChannelId: th.BasicChannel.Id, @@ -64,115 +25,188 @@ func TestInviteProvider(t *testing.T) { UserId: th.BasicUser.Id, } - userAndWrongChannel := "@" + th.BasicUser2.Username + " wrongchannel1" - userAndChannel := "@" + th.BasicUser2.Username + " ~" + channel.Name + " " - userAndDisplayChannel := "@" + th.BasicUser2.Username + " ~" + channel.DisplayName + " " - userAndPrivateChannel := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name - userAndDMChannel := "@" + basicUser3.Username + " ~" + dmChannel.Name - userAndInvalidPrivate := "@" + basicUser3.Username + " ~" + privateChannel2.Name - deactivatedUserPublicChannel := "@" + deactivatedUser.Username + " ~" + channel.Name - - groupChannel := th.createChannel(th.BasicTeam, model.ChannelTypePrivate) - _, err = th.App.AddChannelMember(th.Context, th.BasicUser.Id, groupChannel, app.ChannelMemberOpts{}) - require.Nil(t, err) - groupChannel.GroupConstrained = model.NewBool(true) - groupChannel, _ = th.App.UpdateChannel(th.Context, groupChannel) - - groupChannelNonUser := "@" + th.BasicUser2.Username + " ~" + groupChannel.Name - - tests := []struct { - desc string - expected string - msg string - }{ - { - desc: "Missing user and channel in the command", - expected: "api.command_invite.missing_message.app_error", - msg: "", - }, - { - desc: "User added in the current channel", - expected: "", - msg: th.BasicUser2.Username, - }, - { - desc: "Add user to another channel not the current", - expected: "api.command_invite.success", - msg: userAndChannel, - }, - { - desc: "try to add a user to a direct channel", - expected: "api.command_invite.directchannel.app_error", - msg: userAndDMChannel, - }, - { - desc: "Try to add a user to a invalid channel", - expected: "api.command_invite.channel.error", - msg: userAndWrongChannel, - }, - { - desc: "Try to add a user to an private channel", - expected: "api.command_invite.success", - msg: userAndPrivateChannel, - }, - { - desc: "Using display channel name which is different form Channel name", - expected: "api.command_invite.channel.error", - msg: userAndDisplayChannel, - }, - { - desc: "Invalid user to current channel", - expected: "api.command_invite.missing_user.app_error", - msg: "@invalidUser123", - }, - { - desc: "Invalid user to current channel without @", - expected: "api.command_invite.missing_user.app_error", - msg: "invalidUser321", - }, - { - desc: "try to add a user which is not part of the team", - expected: "api.command_invite.user_not_in_team.app_error", - msg: basicUser4.Username, - }, - { - desc: "try to add a user not part of the group to a group channel", - expected: "api.command_invite.group_constrained_user_denied", - msg: groupChannelNonUser, - }, - { - desc: "try to add a user to a private channel with no permission", - expected: "api.command_invite.private_channel.app_error", - msg: userAndInvalidPrivate, - }, - { - desc: "try to add a deleted user to a public channel", - expected: "api.command_invite.missing_user.app_error", - msg: deactivatedUserPublicChannel, - }, - { - desc: "try to add bot to a public channel", - expected: "api.command_invite.user_not_in_team.app_error", - msg: "@bot1", - }, - { - desc: "add bot to a public channel", - expected: "", - msg: "@bot2", - }, - { - desc: "try to add bot removed from a team to a public channel", - expected: "api.command_invite.user_not_in_team.app_error", - msg: "@bot3", - }, + runCmd := func(msg string, expected string) { + actual := inviteProvider.DoCommand(th.App, th.Context, args, msg).Text + assert.Equal(t, expected, actual) } - for _, test := range tests { - t.Run(test.desc, func(t *testing.T) { - actual := InviteP.DoCommand(th.App, th.Context, args, test.msg).Text - assert.Equal(t, test.expected, actual) - }) + checkIsMember := func(channelID, userID string) { + _, channelMemberErr := th.App.GetChannelMember(th.Context, channelID, userID) + require.Nil(t, channelMemberErr, "Failed to add user to channel") } + + checkIsNotMember := func(channelID, userID string) { + _, channelMemberErr := th.App.GetChannelMember(th.Context, channelID, userID) + require.NotNil(t, channelMemberErr, "Failed to add user to channel") + } + + t.Run("try to add missing user and channel in the command", func(t *testing.T) { + msg := "" + runCmd(msg, "api.command_invite.missing_message.app_error") + }) + + t.Run("user added in the current channel", func(t *testing.T) { + msg := th.BasicUser2.Username + runCmd(msg, "") + checkIsMember(th.BasicChannel.Id, th.BasicUser2.Id) + }) + + t.Run("add user to another channel not the current", func(t *testing.T) { + channel := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + + msg := "@" + th.BasicUser2.Username + " ~" + channel.Name + " " + runCmd(msg, "api.command_invite.success") + checkIsMember(channel.Id, th.BasicUser2.Id) + }) + + t.Run("add a user to a private channel", func(t *testing.T) { + privateChannel := th.createChannel(th.BasicTeam, model.ChannelTypePrivate) + + msg := "@" + th.BasicUser2.Username + " ~" + privateChannel.Name + runCmd(msg, "api.command_invite.success") + checkIsMember(privateChannel.Id, th.BasicUser2.Id) + }) + + t.Run("add multiple users to multiple channels", func(t *testing.T) { + anotherUser := th.createUser() + th.linkUserToTeam(anotherUser, th.BasicTeam) + channel1 := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + channel2 := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + + msg := "@" + th.BasicUser2.Username + " @" + anotherUser.Username + " ~" + channel1.Name + " ~" + channel2.Name + expected := "api.command_invite.success\napi.command_invite.success\napi.command_invite.success\napi.command_invite.success" + runCmd(msg, expected) + checkIsMember(channel1.Id, th.BasicUser2.Id) + checkIsMember(channel2.Id, th.BasicUser2.Id) + checkIsMember(channel1.Id, anotherUser.Id) + checkIsMember(channel2.Id, anotherUser.Id) + }) + + t.Run("adds multiple users even when some are invalid or already members", func(t *testing.T) { + channel := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + userAlreadyInChannel := th.createUser() + th.linkUserToTeam(userAlreadyInChannel, th.BasicTeam) + th.addUserToChannel(userAlreadyInChannel, channel) + userInTeam := th.createUser() + th.linkUserToTeam(userInTeam, th.BasicTeam) + userNotInTeam := th.createUser() + + msg := "@invalidUser123 @" + userAlreadyInChannel.Username + " @" + userInTeam.Username + " @" + userNotInTeam.Username + " ~" + channel.Name + expected := "api.command_invite.missing_user.app_error\n" + expected += "api.command_invite.user_already_in_channel.app_error\n" + expected += "api.command_invite.success\n" + expected += "api.command_invite.user_not_in_team.app_error" + runCmd(msg, expected) + checkIsMember(channel.Id, userInTeam.Id) + }) + + t.Run("try to add a user to a direct channel", func(t *testing.T) { + anotherUser := th.createUser() + th.linkUserToTeam(anotherUser, th.BasicTeam) + directChannel := th.createDmChannel(th.BasicUser2) + + msg := "@" + anotherUser.Username + " ~" + directChannel.Name + runCmd(msg, "api.command_invite.directchannel.app_error") + checkIsNotMember(directChannel.Id, anotherUser.Id) + }) + + t.Run("try to add a user to an invalid channel", func(t *testing.T) { + msg := "@" + th.BasicUser2.Username + " wrongchannel1" + runCmd(msg, "api.command_invite.channel.error") + }) + + t.Run("try to add a user using channel's display name", func(t *testing.T) { + channel := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + + msg := "@" + th.BasicUser2.Username + " ~" + channel.DisplayName + runCmd(msg, "api.command_invite.channel.error") + checkIsNotMember(channel.Id, th.BasicUser2.Id) + }) + + t.Run("try add invalid user to current channel", func(t *testing.T) { + msg := "@invalidUser123" + runCmd(msg, "api.command_invite.missing_user.app_error") + }) + + t.Run("invalid user to current channel without @", func(t *testing.T) { + msg := "invalidUser123" + runCmd(msg, "api.command_invite.missing_user.app_error") + }) + + t.Run("try to add a user which is not part of the team", func(t *testing.T) { + anotherUser := th.createUser() + // Do not add user to the team + + msg := anotherUser.Username + runCmd(msg, "api.command_invite.user_not_in_team.app_error") + }) + + t.Run("try to add a user not part of the group to a group channel", func(t *testing.T) { + groupChannel := th.createChannel(th.BasicTeam, model.ChannelTypePrivate) + _, err := th.App.AddChannelMember(th.Context, th.BasicUser.Id, groupChannel, app.ChannelMemberOpts{}) + require.Nil(t, err) + groupChannel.GroupConstrained = model.NewBool(true) + groupChannel, _ = th.App.UpdateChannel(th.Context, groupChannel) + + msg := "@" + th.BasicUser2.Username + " ~" + groupChannel.Name + runCmd(msg, "api.command_invite.group_constrained_user_denied") + checkIsNotMember(groupChannel.Id, th.BasicUser2.Id) + }) + + t.Run("try to add a user to a private channel with no permission", func(t *testing.T) { + anotherUser := th.createUser() + th.linkUserToTeam(anotherUser, th.BasicTeam) + privateChannel := th.createChannelWithAnotherUser(th.BasicTeam, model.ChannelTypePrivate, th.BasicUser2.Id) + + msg := "@" + anotherUser.Username + " ~" + privateChannel.Name + runCmd(msg, "api.command_invite.private_channel.app_error") + checkIsNotMember(privateChannel.Id, anotherUser.Id) + }) + + t.Run("try to add a deleted user to a public channel", func(t *testing.T) { + channel := th.createChannel(th.BasicTeam, model.ChannelTypeOpen) + deactivatedUser := th.createUser() + _, appErr := th.App.UpdateActive(th.Context, deactivatedUser, false) + require.Nil(t, appErr) + + msg := "@" + deactivatedUser.Username + " ~" + channel.Name + runCmd(msg, "api.command_invite.missing_user.app_error") + checkIsNotMember(channel.Id, deactivatedUser.Id) + }) + + t.Run("add bot to a public channel", func(t *testing.T) { + bot, appErr := th.App.CreateBot(th.Context, &model.Bot{Username: "bot_" + model.NewId(), OwnerId: th.BasicUser2.Id}) + require.Nil(t, appErr) + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, bot.UserId, th.BasicUser2.Id) + require.Nil(t, appErr) + + msg := "@" + bot.Username + runCmd(msg, "") + checkIsMember(th.BasicChannel.Id, bot.UserId) + }) + + t.Run("try to add bot to a public channel without being a member", func(t *testing.T) { + bot, appErr := th.App.CreateBot(th.Context, &model.Bot{Username: "bot_" + model.NewId(), OwnerId: th.BasicUser2.Id}) + require.Nil(t, appErr) + // Do not add to the team + + msg := "@" + bot.Username + runCmd(msg, "api.command_invite.user_not_in_team.app_error") + checkIsNotMember(th.BasicChannel.Id, bot.UserId) + }) + + t.Run("try to add bot removed from a team to a public channel", func(t *testing.T) { + bot, appErr := th.App.CreateBot(th.Context, &model.Bot{Username: "bot_" + model.NewId(), OwnerId: th.BasicUser2.Id}) + require.Nil(t, appErr) + _, _, appErr = th.App.AddUserToTeam(th.Context, th.BasicTeam.Id, bot.UserId, th.BasicUser2.Id) + require.Nil(t, appErr) + appErr = th.App.RemoveUserFromTeam(th.Context, th.BasicTeam.Id, bot.UserId, th.BasicUser2.Id) + require.Nil(t, appErr) + + msg := "@" + bot.Username + runCmd(msg, "api.command_invite.user_not_in_team.app_error") + checkIsNotMember(th.BasicChannel.Id, bot.UserId) + }) } func TestInviteGroup(t *testing.T) { diff --git a/i18n/en.json b/i18n/en.json index 4cb1c9c634..0fac9e2df3 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -900,15 +900,15 @@ }, { "id": "api.command_invite.hint", - "translation": "@[username] ~[channel]" + "translation": "@[username]... ~[channel]..." }, { "id": "api.command_invite.missing_message.app_error", - "translation": "Missing Username and Channel." + "translation": "Missing Username and/or Channel." }, { "id": "api.command_invite.missing_user.app_error", - "translation": "We couldn't find the user. They may have been deactivated by the System Administrator." + "translation": "We couldn't find the user {{.User}}. They may have been deactivated by the System Administrator." }, { "id": "api.command_invite.name", @@ -920,7 +920,7 @@ }, { "id": "api.command_invite.private_channel.app_error", - "translation": "Could not find the channel {{.Channel}}. Please use the channel handle to identify channels." + "translation": "Could not find the channel {{.Channel}}. Please use the [channel handle](https://docs.mattermost.com/messaging/managing-channels.html#naming-a-channel) to identify channels." }, { "id": "api.command_invite.success", From 6b41f914cc21d6fed6f201ea327457f2b77572e7 Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Thu, 5 Jan 2023 10:00:28 +0300 Subject: [PATCH 13/13] cicleci: add ability use target branch for pulling focalboard (#21990) --- .circleci/config.yml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e452a6a2f4..e10fa93097 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -21,6 +21,10 @@ executors: jobs: setup-multi-product-repositories: + parameters: + target-branch: + type: string + default: '$(curl -s -H "Authorization: token ${GITHUB_TOKEN}" $(echo https://api.github.com/repos/${CIRCLE_PULL_REQUEST:19} | sed "s/\/pull\//\/pulls\//") | jq ".base.ref" | tr -d "\042" )' working_directory: /mnt/ramdisk/mattermost-server docker: - image: cimg/go:1.18 @@ -31,7 +35,7 @@ jobs: cd .. git clone --depth=1 --no-single-branch https://github.com/mattermost/focalboard.git cd focalboard - git checkout $CIRCLE_BRANCH || git checkout rolling-stable + git checkout $CIRCLE_BRANCH || git checkout <> || git checkout rolling-stable echo $(git rev-parse HEAD) cd ../mattermost-server make setup-go-work @@ -487,6 +491,8 @@ workflows: untagged-build: jobs: - setup-multi-product-repositories: + context: + - matterbuild-github-token filters: branches: ignore: @@ -652,6 +658,8 @@ workflows: release-build: jobs: - setup-multi-product-repositories: + context: + - matterbuild-github-token filters: branches: only: