diff --git a/Makefile b/Makefile index 7b3e766cab..d714aff153 100644 --- a/Makefile +++ b/Makefile @@ -90,7 +90,7 @@ PLUGIN_PACKAGES += mattermost-plugin-welcomebot-v1.1.1 PLUGIN_PACKAGES += mattermost-plugin-aws-SNS-v1.0.2 PLUGIN_PACKAGES += mattermost-plugin-antivirus-v0.1.1 PLUGIN_PACKAGES += mattermost-plugin-jira-v2.2.1 -PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.0.0 +PLUGIN_PACKAGES += mattermost-plugin-gitlab-v1.0.1 PLUGIN_PACKAGES += mattermost-plugin-jenkins-v1.0.0 # Prepares the enterprise build if exists. The IGNORE stuff is a hack to get the Makefile to execute the commands outside a target diff --git a/api4/plugin_test.go b/api4/plugin_test.go index 4dbed6ba4a..210ec1b722 100644 --- a/api4/plugin_test.go +++ b/api4/plugin_test.go @@ -305,11 +305,34 @@ func TestNotifyClusterPluginEvent(t *testing.T) { require.Equal(t, "testplugin", manifest.Id) // Successful remove + webSocketClient, err := th.CreateWebSocketSystemAdminClient() + require.Nil(t, err) + webSocketClient.Listen() + defer webSocketClient.Close() + done := make(chan bool) + go func() { + for { + select { + case resp := <-webSocketClient.EventChannel: + if resp.Event == model.WEBSOCKET_EVENT_PLUGIN_STATUSES_CHANGED && len(resp.Data["plugin_statuses"].([]interface{})) == 0 { + done <- true + return + } + case <-time.After(5 * time.Second): + done <- false + return + } + } + }() + testCluster.ClearMessages() ok, resp := th.SystemAdminClient.RemovePlugin(manifest.Id) CheckNoError(t, resp) require.True(t, ok) + result := <-done + require.True(t, result, "plugin_statuses_changed websocket event was not received") + messages = testCluster.GetMessages() expectedRemoveMessage := &model.ClusterMessage{ diff --git a/api4/post.go b/api4/post.go index ddf182638d..685ce9bf0e 100644 --- a/api4/post.go +++ b/api4/post.go @@ -385,8 +385,11 @@ func getPostThread(c *Context, w http.ResponseWriter, r *http.Request) { if c.Err != nil { return } - - list, err := c.App.GetPostThread(c.Params.PostId) + skipFetchThreads := false + if r.URL.Query().Get("fetchThreads") == "false" { + skipFetchThreads = true + } + list, err := c.App.GetPostThread(c.Params.PostId, skipFetchThreads) if err != nil { c.Err = err return diff --git a/api4/team.go b/api4/team.go index 06e130ae3e..7891f68074 100644 --- a/api4/team.go +++ b/api4/team.go @@ -499,14 +499,14 @@ func addUserToTeamFromInvite(c *Context, w http.ResponseWriter, r *http.Request) var member *model.TeamMember var err *model.AppError - if c.App.Session.Props[model.SESSION_PROP_IS_GUEST] == "true" { - c.Err = model.NewAppError("addUserToTeamFromInvite", "api.team.add_user_to_team_from_invite.guest.app_error", nil, "", http.StatusForbidden) - return - } - if len(tokenId) > 0 { member, err = c.App.AddTeamMemberByToken(c.App.Session.UserId, tokenId) } else if len(inviteId) > 0 { + if c.App.Session.Props[model.SESSION_PROP_IS_GUEST] == "true" { + c.Err = model.NewAppError("addUserToTeamFromInvite", "api.team.add_user_to_team_from_invite.guest.app_error", nil, "", http.StatusForbidden) + return + } + member, err = c.App.AddTeamMemberByInviteId(inviteId, c.App.Session.UserId) } else { err = model.NewAppError("addTeamMember", "api.team.add_user_to_team.missing_parameter.app_error", nil, "", http.StatusBadRequest) diff --git a/app/auto_posts.go b/app/auto_posts.go index b8837b4c9f..bded3f202e 100644 --- a/app/auto_posts.go +++ b/app/auto_posts.go @@ -92,8 +92,8 @@ func (cfg *AutoPostCreator) CreateRandomPostNested(parentId, rootId string) (*mo RootId: rootId, Message: postText, FileIds: fileIds} - rpost, err2 := cfg.client.CreatePost(post) - if err2 != nil { + rpost, resp := cfg.client.CreatePost(post) + if resp != nil && resp.Error != nil { return nil, false } return rpost, true diff --git a/app/channel.go b/app/channel.go index 01dd1366b4..868892a925 100644 --- a/app/channel.go +++ b/app/channel.go @@ -160,7 +160,7 @@ func (a *App) CreateChannelWithUser(channel *model.Channel, userId string) (*mod } // Get total number of channels on current team - count, err := a.GetNumberOfChannelsOnTeam(channel.TeamId, false) + count, err := a.GetNumberOfChannelsOnTeam(channel.TeamId) if err != nil { return nil, err } @@ -1724,23 +1724,12 @@ func (a *App) RemoveUserFromChannel(userIdToRemove string, removerUserId string, return nil } -func (a *App) GetNumberOfChannelsOnTeam(teamId string, includeDeleted bool) (int, *model.AppError) { +func (a *App) GetNumberOfChannelsOnTeam(teamId string) (int, *model.AppError) { // Get total number of channels on current team list, err := a.Srv.Store.Channel().GetTeamChannels(teamId) if err != nil { return 0, err } - - if !includeDeleted { - count := 0 - for _, channel := range *list { - if channel.DeleteAt == 0 { - count++ - } - } - return count, nil - } - return len(*list), nil } diff --git a/app/diagnostics.go b/app/diagnostics.go index bb7fd0fac5..863af16592 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -47,6 +47,7 @@ const ( TRACK_CONFIG_DATA_RETENTION = "config_data_retention" TRACK_CONFIG_MESSAGE_EXPORT = "config_message_export" TRACK_CONFIG_DISPLAY = "config_display" + TRACK_CONFIG_GUEST_ACCOUNTS = "config_guest_accounts" TRACK_CONFIG_IMAGE_PROXY = "config_image_proxy" TRACK_PERMISSIONS_GENERAL = "permissions_general" TRACK_PERMISSIONS_SYSTEM_SCHEME = "permissions_system_scheme" @@ -509,6 +510,7 @@ func (a *App) trackConfig() { "isdefault_scoping_idp_provider_id": isDefault(*cfg.SamlSettings.ScopingIDPProviderId, ""), "isdefault_scoping_idp_name": isDefault(*cfg.SamlSettings.ScopingIDPName, ""), "isdefault_id_attribute": isDefault(*cfg.SamlSettings.IdAttribute, model.SAML_SETTINGS_DEFAULT_ID_ATTRIBUTE), + "isdefault_guest_attribute": isDefault(*cfg.SamlSettings.GuestAttribute, model.SAML_SETTINGS_DEFAULT_GUEST_ATTRIBUTE), "isdefault_first_name_attribute": isDefault(*cfg.SamlSettings.FirstNameAttribute, model.SAML_SETTINGS_DEFAULT_FIRST_NAME_ATTRIBUTE), "isdefault_last_name_attribute": isDefault(*cfg.SamlSettings.LastNameAttribute, model.SAML_SETTINGS_DEFAULT_LAST_NAME_ATTRIBUTE), "isdefault_email_attribute": isDefault(*cfg.SamlSettings.EmailAttribute, model.SAML_SETTINGS_DEFAULT_EMAIL_ATTRIBUTE), @@ -630,6 +632,13 @@ func (a *App) trackConfig() { "isdefault_custom_url_schemes": len(cfg.DisplaySettings.CustomUrlSchemes) != 0, }) + a.SendDiagnostic(TRACK_CONFIG_GUEST_ACCOUNTS, map[string]interface{}{ + "enable": *cfg.GuestAccountsSettings.Enable, + "allow_email_accounts": *cfg.GuestAccountsSettings.AllowEmailAccounts, + "enforce_multifactor_authentication": *cfg.GuestAccountsSettings.EnforceMultifactorAuthentication, + "isdefault_restrict_creation_to_domains": isDefault(*cfg.GuestAccountsSettings.RestrictCreationToDomains, ""), + }) + a.SendDiagnostic(TRACK_CONFIG_IMAGE_PROXY, map[string]interface{}{ "enable": *cfg.ImageProxySettings.Enable, "image_proxy_type": *cfg.ImageProxySettings.ImageProxyType, diff --git a/app/import_functions_test.go b/app/import_functions_test.go index d01d837136..8712746ec1 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -61,180 +61,157 @@ func TestImportImportScheme(t *testing.T) { Description: ptrStr("description"), } - if err := th.App.ImportScheme(&data, true); err == nil { - t.Fatalf("Should have failed to import.") - } + err := th.App.ImportScheme(&data, true) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing a valid scheme in dryRun mode. data.DisplayName = ptrStr("display name") - if err := th.App.ImportScheme(&data, true); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportScheme(&data, true) + require.Nil(t, err, "Should have succeeded.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing an invalid scheme. data.DisplayName = nil - if err := th.App.ImportScheme(&data, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err = th.App.ImportScheme(&data, false) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing a valid scheme with all params set. data.DisplayName = ptrStr("display name") - if err := th.App.ImportScheme(&data, false); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportScheme(&data, false) + require.Nil(t, err, "Should have succeeded.") - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, *data.Scope, scheme.Scope) + scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, *data.Scope, scheme.Scope) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) // Try modifying all the fields and re-importing. data.DisplayName = ptrStr("new display name") data.Description = ptrStr("new description") - if err := th.App.ImportScheme(&data, false); err != nil { - t.Fatalf("Should have succeeded: %v", err) - } + err = th.App.ImportScheme(&data, false) + require.Nil(t, err, "Should have succeeded: %v", err) - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, *data.Scope, scheme.Scope) + scheme, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, *data.Scope, scheme.Scope) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) // Try changing the scope of the scheme and reimporting. data.Scope = ptrStr("channel") - if err := th.App.ImportScheme(&data, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err = th.App.ImportScheme(&data, false) + require.NotNil(t, err, "Should have failed to import.") + + scheme, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) + + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, "team", scheme.Scope) - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, "team", scheme.Scope) - } } func TestImportImportSchemeWithoutGuestRoles(t *testing.T) { @@ -271,180 +248,156 @@ func TestImportImportSchemeWithoutGuestRoles(t *testing.T) { Description: ptrStr("description"), } - if err := th.App.ImportScheme(&data, true); err == nil { - t.Fatalf("Should have failed to import.") - } + err := th.App.ImportScheme(&data, true) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing a valid scheme in dryRun mode. data.DisplayName = ptrStr("display name") - if err := th.App.ImportScheme(&data, true); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportScheme(&data, true) + require.Nil(t, err, "Should have succeeded.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing an invalid scheme. data.DisplayName = nil - if err := th.App.ImportScheme(&data, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err = th.App.ImportScheme(&data, false) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err == nil { - t.Fatalf("Scheme should not have imported.") - } + _, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.NotNil(t, err, "Scheme should not have imported.") // Try importing a valid scheme with all params set. data.DisplayName = ptrStr("display name") - if err := th.App.ImportScheme(&data, false); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportScheme(&data, false) + require.Nil(t, err, "Should have succeeded.") - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, *data.Scope, scheme.Scope) + scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, *data.Scope, scheme.Scope) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) // Try modifying all the fields and re-importing. data.DisplayName = ptrStr("new display name") data.Description = ptrStr("new description") - if err := th.App.ImportScheme(&data, false); err != nil { - t.Fatalf("Should have succeeded: %v", err) - } + err = th.App.ImportScheme(&data, false) + require.Nil(t, err, "Should have succeeded: %v", err) - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, *data.Scope, scheme.Scope) + scheme, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, *data.Scope, scheme.Scope) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamAdminRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamUserRole) + require.Nil(t, err, "Should have found the imported role.") - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + assert.Equal(t, *data.DefaultTeamUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) - if role, err := th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } - } + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultTeamGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultTeamGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelAdminRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelAdminRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelUserRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelUserRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) + + role, err = th.App.Srv.Store.Role().GetByName(scheme.DefaultChannelGuestRole) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.DefaultChannelGuestRole.DisplayName, role.DisplayName) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) // Try changing the scope of the scheme and reimporting. data.Scope = ptrStr("channel") - if err := th.App.ImportScheme(&data, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err = th.App.ImportScheme(&data, false) + require.NotNil(t, err, "Should have failed to import.") - if scheme, err := th.App.Srv.Store.Scheme().GetByName(*data.Name); err != nil { - t.Fatalf("Failed to import scheme: %v", err) - } else { - assert.Equal(t, *data.Name, scheme.Name) - assert.Equal(t, *data.DisplayName, scheme.DisplayName) - assert.Equal(t, *data.Description, scheme.Description) - assert.Equal(t, "team", scheme.Scope) - } + scheme, err = th.App.Srv.Store.Scheme().GetByName(*data.Name) + require.Nil(t, err, "Failed to import scheme: %v", err) + + assert.Equal(t, *data.Name, scheme.Name) + assert.Equal(t, *data.DisplayName, scheme.DisplayName) + assert.Equal(t, *data.Description, scheme.Description) + assert.Equal(t, "team", scheme.Scope) } func TestImportImportRole(t *testing.T) { @@ -457,75 +410,65 @@ func TestImportImportRole(t *testing.T) { Name: &rid1, } - if err := th.App.ImportRole(&data, true, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err := th.App.ImportRole(&data, true, false) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Role().GetByName(rid1); err == nil { - t.Fatalf("Role should not have imported.") - } + _, err = th.App.Srv.Store.Role().GetByName(rid1) + require.NotNil(t, err, "Should have failed to import.") // Try importing the valid role in dryRun mode. data.DisplayName = ptrStr("display name") - if err := th.App.ImportRole(&data, true, false); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportRole(&data, true, false) + require.Nil(t, err, "Should have succeeded.") - if _, err := th.App.Srv.Store.Role().GetByName(rid1); err == nil { - t.Fatalf("Role should not have imported as we are in dry run mode.") - } + _, err = th.App.Srv.Store.Role().GetByName(rid1) + require.NotNil(t, err, "Role should not have imported as we are in dry run mode.") // Try importing an invalid role. data.DisplayName = nil - if err := th.App.ImportRole(&data, false, false); err == nil { - t.Fatalf("Should have failed to import.") - } + err = th.App.ImportRole(&data, false, false) + require.NotNil(t, err, "Should have failed to import.") - if _, err := th.App.Srv.Store.Role().GetByName(rid1); err == nil { - t.Fatalf("Role should not have imported.") - } + _, err = th.App.Srv.Store.Role().GetByName(rid1) + require.NotNil(t, err, "Role should not have imported.") // Try importing a valid role with all params set. data.DisplayName = ptrStr("display name") data.Description = ptrStr("description") data.Permissions = &[]string{"invite_user", "add_user_to_team"} - if err := th.App.ImportRole(&data, false, false); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportRole(&data, false, false) + require.Nil(t, err, "Should have succeeded.") - if role, err := th.App.Srv.Store.Role().GetByName(rid1); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.Name, role.Name) - assert.Equal(t, *data.DisplayName, role.DisplayName) - assert.Equal(t, *data.Description, role.Description) - assert.Equal(t, *data.Permissions, role.Permissions) - assert.False(t, role.BuiltIn) - assert.False(t, role.SchemeManaged) - } + role, err := th.App.Srv.Store.Role().GetByName(rid1) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.Name, role.Name) + assert.Equal(t, *data.DisplayName, role.DisplayName) + assert.Equal(t, *data.Description, role.Description) + assert.Equal(t, *data.Permissions, role.Permissions) + assert.False(t, role.BuiltIn) + assert.False(t, role.SchemeManaged) // Try changing all the params and reimporting. data.DisplayName = ptrStr("new display name") data.Description = ptrStr("description") data.Permissions = &[]string{"use_slash_commands"} - if err := th.App.ImportRole(&data, false, true); err != nil { - t.Fatalf("Should have succeeded. %v", err) - } + err = th.App.ImportRole(&data, false, true) + require.Nil(t, err, "Should have succeeded. %v", err) - if role, err := th.App.Srv.Store.Role().GetByName(rid1); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data.Name, role.Name) - assert.Equal(t, *data.DisplayName, role.DisplayName) - assert.Equal(t, *data.Description, role.Description) - assert.Equal(t, *data.Permissions, role.Permissions) - assert.False(t, role.BuiltIn) - assert.True(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(rid1) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data.Name, role.Name) + assert.Equal(t, *data.DisplayName, role.DisplayName) + assert.Equal(t, *data.Description, role.Description) + assert.Equal(t, *data.Permissions, role.Permissions) + assert.False(t, role.BuiltIn) + assert.True(t, role.SchemeManaged) // Check that re-importing with only required fields doesn't update the others. data2 := RoleImportData{ @@ -533,20 +476,18 @@ func TestImportImportRole(t *testing.T) { DisplayName: ptrStr("new display name again"), } - if err := th.App.ImportRole(&data2, false, false); err != nil { - t.Fatalf("Should have succeeded.") - } + err = th.App.ImportRole(&data2, false, false) + require.Nil(t, err, "Should have succeeded.") - if role, err := th.App.Srv.Store.Role().GetByName(rid1); err != nil { - t.Fatalf("Should have found the imported role.") - } else { - assert.Equal(t, *data2.Name, role.Name) - assert.Equal(t, *data2.DisplayName, role.DisplayName) - assert.Equal(t, *data.Description, role.Description) - assert.Equal(t, *data.Permissions, role.Permissions) - assert.False(t, role.BuiltIn) - assert.False(t, role.SchemeManaged) - } + role, err = th.App.Srv.Store.Role().GetByName(rid1) + require.Nil(t, err, "Should have found the imported role.") + + assert.Equal(t, *data2.Name, role.Name) + assert.Equal(t, *data2.DisplayName, role.DisplayName) + assert.Equal(t, *data.Description, role.Description) + assert.Equal(t, *data.Permissions, role.Permissions) + assert.False(t, role.BuiltIn) + assert.False(t, role.SchemeManaged) } func TestImportImportTeam(t *testing.T) { @@ -565,9 +506,7 @@ func TestImportImportTeam(t *testing.T) { // Check how many teams are in the database. teamsCount, err := th.App.Srv.Store.Team().AnalyticsTeamCount() - if err != nil { - t.Fatalf("Failed to get team count.") - } + require.Nil(t, err, "Failed to get team count.") data := TeamImportData{ Name: ptrStr(model.NewId()), @@ -579,47 +518,42 @@ func TestImportImportTeam(t *testing.T) { } // Try importing an invalid team in dryRun mode. - if err := th.App.ImportTeam(&data, true); err == nil { - t.Fatalf("Should have received an error importing an invalid team.") - } + err = th.App.ImportTeam(&data, true) + require.NotNil(t, err, "Should have received an error importing an invalid team.") // Do a valid team in dry-run mode. data.Type = ptrStr("O") - if err := th.App.ImportTeam(&data, true); err != nil { - t.Fatalf("Received an error validating valid team.") - } + err = th.App.ImportTeam(&data, true) + require.Nil(t, err, "Received an error validating valid team.") // Check that no more teams are in the DB. th.CheckTeamCount(t, teamsCount) // Do an invalid team in apply mode, check db changes. data.Type = ptrStr("XYZ") - if err := th.App.ImportTeam(&data, false); err == nil { - t.Fatalf("Import should have failed on invalid team.") - } + err = th.App.ImportTeam(&data, false) + require.NotNil(t, err, "Import should have failed on invalid team.") // Check that no more teams are in the DB. th.CheckTeamCount(t, teamsCount) // Do a valid team in apply mode, check db changes. data.Type = ptrStr("O") - if err := th.App.ImportTeam(&data, false); err != nil { - t.Fatalf("Received an error importing valid team: %v", err) - } + err = th.App.ImportTeam(&data, false) + require.Nil(t, err, "Received an error importing valid team: %v", err) // Check that one more team is in the DB. th.CheckTeamCount(t, teamsCount+1) // Get the team and check that all the fields are correct. - if team, err := th.App.GetTeamByName(*data.Name); err != nil { - t.Fatalf("Failed to get team from database.") - } else { - assert.Equal(t, *data.DisplayName, team.DisplayName) - assert.Equal(t, *data.Type, team.Type) - assert.Equal(t, *data.Description, team.Description) - assert.Equal(t, *data.AllowOpenInvite, team.AllowOpenInvite) - assert.Equal(t, scheme1.Id, *team.SchemeId) - } + team, err := th.App.GetTeamByName(*data.Name) + require.Nil(t, err, "Failed to get team from database.") + + assert.Equal(t, *data.DisplayName, team.DisplayName) + assert.Equal(t, *data.Type, team.Type) + assert.Equal(t, *data.Description, team.Description) + assert.Equal(t, *data.AllowOpenInvite, team.AllowOpenInvite) + assert.Equal(t, scheme1.Id, *team.SchemeId) // Alter all the fields of that team (apart from unique identifier) and import again. data.DisplayName = ptrStr("Display Name 2") @@ -630,22 +564,20 @@ func TestImportImportTeam(t *testing.T) { // Check that the original number of teams are again in the DB (because this query doesn't include deleted). data.Type = ptrStr("O") - if err := th.App.ImportTeam(&data, false); err != nil { - t.Fatalf("Received an error importing updated valid team.") - } + err = th.App.ImportTeam(&data, false) + require.Nil(t, err, "Received an error importing updated valid team.") th.CheckTeamCount(t, teamsCount+1) // Get the team and check that all fields are correct. - if team, err := th.App.GetTeamByName(*data.Name); err != nil { - t.Fatalf("Failed to get team from database.") - } else { - assert.Equal(t, *data.DisplayName, team.DisplayName) - assert.Equal(t, *data.Type, team.Type) - assert.Equal(t, *data.Description, team.Description) - assert.Equal(t, *data.AllowOpenInvite, team.AllowOpenInvite) - assert.Equal(t, scheme2.Id, *team.SchemeId) - } + team, err = th.App.GetTeamByName(*data.Name) + require.Nil(t, err, "Failed to get team from database.") + + assert.Equal(t, *data.DisplayName, team.DisplayName) + assert.Equal(t, *data.Type, team.Type) + assert.Equal(t, *data.Description, team.Description) + assert.Equal(t, *data.AllowOpenInvite, team.AllowOpenInvite) + assert.Equal(t, scheme2.Id, *team.SchemeId) } func TestImportImportChannel(t *testing.T) { @@ -670,9 +602,7 @@ func TestImportImportChannel(t *testing.T) { Type: ptrStr("O"), }, false) team, err := th.App.GetTeamByName(teamName) - if err != nil { - t.Fatalf("Failed to get team from database.") - } + require.Nil(t, err, "Failed to get team from database.") // Check how many channels are in the database. channelCount, err := th.App.Srv.Store.Channel().AnalyticsTypeCount("", model.CHANNEL_OPEN) @@ -687,9 +617,8 @@ func TestImportImportChannel(t *testing.T) { Purpose: ptrStr("Channel Purpose"), Scheme: &scheme1.Name, } - if err := th.App.ImportChannel(&data, true); err == nil { - t.Fatalf("Expected error due to invalid name.") - } + err = th.App.ImportChannel(&data, true) + require.NotNil(t, err, "Expected error due to invalid name.") // Check that no more channels are in the DB. th.CheckChannelsCount(t, channelCount) @@ -697,27 +626,24 @@ func TestImportImportChannel(t *testing.T) { // Do a valid channel with a nonexistent team in dry-run mode. data.Name = ptrStr("channelname") data.Team = ptrStr(model.NewId()) - if err := th.App.ImportChannel(&data, true); err != nil { - t.Fatalf("Expected success as cannot validate channel name in dry run mode.") - } + err = th.App.ImportChannel(&data, true) + require.Nil(t, err, "Expected success as cannot validate channel name in dry run mode.") // Check that no more channels are in the DB. th.CheckChannelsCount(t, channelCount) // Do a valid channel in dry-run mode. data.Team = &teamName - if err := th.App.ImportChannel(&data, true); err != nil { - t.Fatalf("Expected success as valid team.") - } + err = th.App.ImportChannel(&data, true) + require.Nil(t, err, "Expected success as valid team.") // Check that no more channels are in the DB. th.CheckChannelsCount(t, channelCount) // Do an invalid channel in apply mode. data.Name = nil - if err := th.App.ImportChannel(&data, false); err == nil { - t.Fatalf("Expected error due to invalid name (apply mode).") - } + err = th.App.ImportChannel(&data, false) + require.NotNil(t, err, "Expected error due to invalid name (apply mode).") // Check that no more channels are in the DB. th.CheckChannelsCount(t, channelCount) @@ -725,33 +651,30 @@ func TestImportImportChannel(t *testing.T) { // Do a valid channel in apply mode with a non-existent team. data.Name = ptrStr("channelname") data.Team = ptrStr(model.NewId()) - if err := th.App.ImportChannel(&data, false); err == nil { - t.Fatalf("Expected error due to non-existent team (apply mode).") - } + err = th.App.ImportChannel(&data, false) + require.NotNil(t, err, "Expected error due to non-existent team (apply mode).") // Check that no more channels are in the DB. th.CheckChannelsCount(t, channelCount) // Do a valid channel in apply mode. data.Team = &teamName - if err := th.App.ImportChannel(&data, false); err != nil { - t.Fatalf("Expected success in apply mode: %v", err.Error()) - } + err = th.App.ImportChannel(&data, false) + require.Nil(t, err, "Expected success in apply mode") // Check that 1 more channel is in the DB. th.CheckChannelsCount(t, channelCount+1) // Get the Channel and check all the fields are correct. - if channel, err := th.App.GetChannelByName(*data.Name, team.Id, false); err != nil { - t.Fatalf("Failed to get channel from database.") - } else { - assert.Equal(t, *data.Name, channel.Name) - assert.Equal(t, *data.DisplayName, channel.DisplayName) - assert.Equal(t, *data.Type, channel.Type) - assert.Equal(t, *data.Header, channel.Header) - assert.Equal(t, *data.Purpose, channel.Purpose) - assert.Equal(t, scheme1.Id, *channel.SchemeId) - } + channel, err := th.App.GetChannelByName(*data.Name, team.Id, false) + require.Nil(t, err, "Failed to get channel from database.") + + assert.Equal(t, *data.Name, channel.Name) + assert.Equal(t, *data.DisplayName, channel.DisplayName) + assert.Equal(t, *data.Type, channel.Type) + assert.Equal(t, *data.Header, channel.Header) + assert.Equal(t, *data.Purpose, channel.Purpose) + assert.Equal(t, scheme1.Id, *channel.SchemeId) // Alter all the fields of that channel. data.DisplayName = ptrStr("Chaned Disp Name") @@ -759,25 +682,22 @@ func TestImportImportChannel(t *testing.T) { data.Header = ptrStr("New Header") data.Purpose = ptrStr("New Purpose") data.Scheme = &scheme2.Name - if err := th.App.ImportChannel(&data, false); err != nil { - t.Fatalf("Expected success in apply mode: %v", err.Error()) - } + err = th.App.ImportChannel(&data, false) + require.Nil(t, err, "Expected success in apply mode") // Check channel count the same. th.CheckChannelsCount(t, channelCount) // Get the Channel and check all the fields are correct. - if channel, err := th.App.GetChannelByName(*data.Name, team.Id, false); err != nil { - t.Fatalf("Failed to get channel from database.") - } else { - assert.Equal(t, *data.Name, channel.Name) - assert.Equal(t, *data.DisplayName, channel.DisplayName) - assert.Equal(t, *data.Type, channel.Type) - assert.Equal(t, *data.Header, channel.Header) - assert.Equal(t, *data.Purpose, channel.Purpose) - assert.Equal(t, scheme2.Id, *channel.SchemeId) - } + channel, err = th.App.GetChannelByName(*data.Name, team.Id, false) + require.Nil(t, err, "Failed to get channel from database.") + assert.Equal(t, *data.Name, channel.Name) + assert.Equal(t, *data.DisplayName, channel.DisplayName) + assert.Equal(t, *data.Type, channel.Type) + assert.Equal(t, *data.Header, channel.Header) + assert.Equal(t, *data.Purpose, channel.Purpose) + assert.Equal(t, scheme2.Id, *channel.SchemeId) } func TestImportImportUser(t *testing.T) { @@ -795,9 +715,8 @@ func TestImportImportUser(t *testing.T) { data := UserImportData{ Username: ptrStr(model.NewId()), } - if err = th.App.ImportUser(&data, true); err == nil { - t.Fatalf("Should have failed to import invalid user.") - } + err = th.App.ImportUser(&data, true) + require.NotNil(t, err, "Should have failed to import invalid user.") // Check that no more users are in the DB. userCount2, err := th.App.Srv.Store.User().Count(model.UserCountOptions{ @@ -812,9 +731,8 @@ func TestImportImportUser(t *testing.T) { Username: ptrStr(model.NewId()), Email: ptrStr(model.NewId() + "@example.com"), } - if err = th.App.ImportUser(&data, true); err != nil { - t.Fatalf("Should have succeeded to import valid user.") - } + err = th.App.ImportUser(&data, true) + require.Nil(t, err, "Should have succeeded to import valid user.") // Check that no more users are in the DB. userCount3, err := th.App.Srv.Store.User().Count(model.UserCountOptions{ @@ -828,9 +746,8 @@ func TestImportImportUser(t *testing.T) { data = UserImportData{ Username: ptrStr(model.NewId()), } - if err = th.App.ImportUser(&data, false); err == nil { - t.Fatalf("Should have failed to import invalid user.") - } + err = th.App.ImportUser(&data, false) + require.NotNil(t, err, "Should have failed to import invalid user.") // Check that no more users are in the DB. userCount4, err := th.App.Srv.Store.User().Count(model.UserCountOptions{ @@ -852,9 +769,8 @@ func TestImportImportUser(t *testing.T) { LastName: ptrStr(model.NewId()), Position: ptrStr(model.NewId()), } - if err = th.App.ImportUser(&data, false); err != nil { - t.Fatalf("Should have succeeded to import valid user.") - } + err = th.App.ImportUser(&data, false) + require.Nil(t, err, "Should have succeeded to import valid user.") // Check that one more user is in the DB. userCount5, err := th.App.Srv.Store.User().Count(model.UserCountOptions{ @@ -865,37 +781,24 @@ func TestImportImportUser(t *testing.T) { assert.Equal(t, userCount+1, userCount5, "Unexpected number of users") // Get the user and check all the fields are correct. - if user, err2 := th.App.GetUserByUsername(username); err2 != nil { - t.Fatalf("Failed to get user from database.") - } else { - if user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position { - t.Fatalf("User properties do not match Import Data.") - } - // Check calculated properties. - if user.AuthService != "" { - t.Fatalf("Expected Auth Service to be empty.") - } + user, err2 := th.App.GetUserByUsername(username) + require.Nil(t, err2, "Failed to get user from database.") - if !(user.AuthData == nil || *user.AuthData == "") { - t.Fatalf("Expected AuthData to be empty.") - } + userBool := user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position + require.False(t, userBool, "User properties do not match Import Data.") - if len(user.Password) == 0 { - t.Fatalf("Expected password to be set.") - } + // Check calculated properties. + require.Empty(t, user.AuthService, "Expected Auth Service to be empty.") - if !user.EmailVerified { - t.Fatalf("Expected EmailVerified to be true.") - } + require.Empty(t, user.AuthData, "Expected AuthData to be empty.") - if user.Locale != *th.App.Config().LocalizationSettings.DefaultClientLocale { - t.Fatalf("Expected Locale to be the default.") - } + require.NotEmpty(t, user.Password, "Expected password to be set.") - if user.Roles != "system_user" { - t.Fatalf("Expected roles to be system_user") - } - } + require.True(t, user.EmailVerified, "Expected EmailVerified to be true.") + + require.Equal(t, user.Locale, *th.App.Config().LocalizationSettings.DefaultClientLocale, "Expected Locale to be the default.") + + require.Equal(t, user.Roles, "system_user", "Expected roles to be system_user") // Alter all the fields of that user. data.Email = ptrStr(model.NewId() + "@example.com") @@ -908,9 +811,9 @@ func TestImportImportUser(t *testing.T) { data.Position = ptrStr(model.NewId()) data.Roles = ptrStr("system_admin system_user") data.Locale = ptrStr("zh_CN") - if err = th.App.ImportUser(&data, false); err != nil { - t.Fatalf("Should have succeeded to update valid user %v", err) - } + + err = th.App.ImportUser(&data, false) + require.Nil(t, err, "Should have succeeded to update valid user %v", err) // Check user count the same. userCount6, err := th.App.Srv.Store.User().Count(model.UserCountOptions{ @@ -921,58 +824,40 @@ func TestImportImportUser(t *testing.T) { assert.Equal(t, userCount+1, userCount6, "Unexpected number of users") // Get the user and check all the fields are correct. - if user, err2 := th.App.GetUserByUsername(username); err2 != nil { - t.Fatalf("Failed to get user from database.") - } else { - if user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position { - t.Fatalf("Updated User properties do not match Import Data.") - } - // Check calculated properties. - if user.AuthService != "ldap" { - t.Fatalf("Expected Auth Service to be ldap \"%v\"", user.AuthService) - } + user, err2 = th.App.GetUserByUsername(username) + require.Nil(t, err2, "Failed to get user from database.") - if !(user.AuthData == data.AuthData || *user.AuthData == *data.AuthData) { - t.Fatalf("Expected AuthData to be set.") - } + userBool = user.Email != *data.Email || user.Nickname != *data.Nickname || user.FirstName != *data.FirstName || user.LastName != *data.LastName || user.Position != *data.Position + require.False(t, userBool, "Updated User properties do not match Import Data.") - if len(user.Password) != 0 { - t.Fatalf("Expected password to be empty.") - } + require.Equal(t, "ldap", user.AuthService, "Expected Auth Service to be ldap \"%v\"", user.AuthService) - if !user.EmailVerified { - t.Fatalf("Expected EmailVerified to be true.") - } + require.Equal(t, user.AuthData, data.AuthData, "Expected AuthData to be set.") - if user.Locale != *data.Locale { - t.Fatalf("Expected Locale to be the set.") - } + require.Len(t, user.Password, 0, "Expected password to be empty.") - if user.Roles != *data.Roles { - t.Fatalf("Expected roles to be set: %v", user.Roles) - } - } + require.True(t, user.EmailVerified, "Expected EmailVerified to be true.") + + require.Equal(t, *data.Locale, user.Locale, "Expected Locale to be the set.") + + require.Equal(t, *data.Roles, user.Roles, "Expected roles to be set: %v", user.Roles) // Check Password and AuthData together. data.Password = ptrStr("PasswordTest") - if err = th.App.ImportUser(&data, false); err == nil { - t.Fatalf("Should have failed to import invalid user.") - } + err = th.App.ImportUser(&data, false) + require.NotNil(t, err, "Should have failed to import invalid user.") data.AuthData = nil - if err = th.App.ImportUser(&data, false); err != nil { - t.Fatalf("Should have succeeded to update valid user %v", err) - } + err = th.App.ImportUser(&data, false) + require.Nil(t, err, "Should have succeeded to update valid user %v", err) data.Password = ptrStr("") - if err = th.App.ImportUser(&data, false); err == nil { - t.Fatalf("Should have failed to import invalid user.") - } + err = th.App.ImportUser(&data, false) + require.NotNil(t, err, "Should have failed to import invalid user.") data.Password = ptrStr(strings.Repeat("0123456789", 10)) - if err = th.App.ImportUser(&data, false); err == nil { - t.Fatalf("Should have failed to import invalid user.") - } + err = th.App.ImportUser(&data, false) + require.NotNil(t, err, "Should have failed to import invalid user.") data.Password = ptrStr("TestPassword") @@ -984,9 +869,7 @@ func TestImportImportUser(t *testing.T) { Type: ptrStr("O"), }, false) team, err := th.App.GetTeamByName(teamName) - if err != nil { - t.Fatalf("Failed to get team from database.") - } + require.Nil(t, err, "Failed to get team from database.") channelName := model.NewId() th.App.ImportChannel(&ChannelImportData{ @@ -996,9 +879,7 @@ func TestImportImportUser(t *testing.T) { Type: ptrStr("O"), }, false) channel, err := th.App.GetChannelByName(channelName, team.Id, false) - if err != nil { - t.Fatalf("Failed to get channel from database.") - } + require.Nil(t, err, "Failed to get channel from database.") username = model.NewId() data = UserImportData{ @@ -1011,15 +892,11 @@ func TestImportImportUser(t *testing.T) { } teamMembers, err := th.App.GetTeamMembers(team.Id, 0, 1000, nil) - if err != nil { - t.Fatalf("Failed to get team member count") - } + require.Nil(t, err, "Failed to get team member count") teamMemberCount := len(teamMembers) channelMemberCount, err := th.App.GetChannelMemberCount(channel.Id) - if err != nil { - t.Fatalf("Failed to get channel member count") - } + require.Nil(t, err, "Failed to get channel member count") // Test with an invalid team & channel membership in dry-run mode. data.Teams = &[]UserTeamImportData{ @@ -1175,10 +1052,8 @@ func TestImportImportUser(t *testing.T) { require.Equal(t, channelMemberCount, cmc) // Check team member properties. - user, err := th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + user, err = th.App.GetUserByUsername(username) + require.Nil(t, err, "Failed to get user from database.") teamMember, err := th.App.GetTeamMember(team.Id, user.Id) require.Nil(t, err, "Failed to get team member from database.") @@ -1283,9 +1158,7 @@ func TestImportImportUser(t *testing.T) { // Check their values. user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_THEME, "", *data.Theme) checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_DISPLAY_SETTINGS, model.PREFERENCE_NAME_USE_MILITARY_TIME, *data.UseMilitaryTime) @@ -1336,9 +1209,7 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkNotifyProp(t, user, model.DESKTOP_NOTIFY_PROP, model.USER_NOTIFY_ALL) checkNotifyProp(t, user, model.DESKTOP_SOUND_NOTIFY_PROP, "true") @@ -1364,9 +1235,7 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkNotifyProp(t, user, model.DESKTOP_NOTIFY_PROP, model.USER_NOTIFY_ALL) checkNotifyProp(t, user, model.DESKTOP_SOUND_NOTIFY_PROP, "true") @@ -1392,9 +1261,7 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkNotifyProp(t, user, model.DESKTOP_NOTIFY_PROP, model.USER_NOTIFY_MENTION) checkNotifyProp(t, user, model.DESKTOP_SOUND_NOTIFY_PROP, "false") @@ -1419,9 +1286,7 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkNotifyProp(t, user, model.DESKTOP_NOTIFY_PROP, model.USER_NOTIFY_MENTION) checkNotifyProp(t, user, model.DESKTOP_SOUND_NOTIFY_PROP, "false") @@ -1453,9 +1318,7 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") checkNotifyProp(t, user, model.DESKTOP_NOTIFY_PROP, model.USER_NOTIFY_MENTION) checkNotifyProp(t, user, model.DESKTOP_SOUND_NOTIFY_PROP, "false") @@ -1525,9 +1388,7 @@ func TestImportImportUser(t *testing.T) { err = th.App.ImportTeam(teamData, false) assert.Nil(t, err) team, err = th.App.GetTeamByName(teamName) - if err != nil { - t.Fatalf("Failed to get team from database.") - } + require.Nil(t, err, "Failed to get team from database.") channelData := &ChannelImportData{ Team: &teamName, @@ -1540,9 +1401,7 @@ func TestImportImportUser(t *testing.T) { err = th.App.ImportChannel(channelData, false) assert.Nil(t, err) channel, err = th.App.GetChannelByName(*channelData.Name, team.Id, false) - if err != nil { - t.Fatalf("Failed to get channel from database: %v", err.Error()) - } + require.Nil(t, err, "Failed to get channel from database") // Test with a valid team & valid channel name in apply mode. userData := &UserImportData{ @@ -1565,23 +1424,19 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(*userData.Username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") teamMember, err = th.App.GetTeamMember(team.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the team member") - } + require.Nil(t, err, "Failed to get the team member") + assert.True(t, teamMember.SchemeAdmin) assert.True(t, teamMember.SchemeUser) assert.False(t, teamMember.SchemeGuest) assert.Equal(t, "", teamMember.ExplicitRoles) channelMember, err = th.App.GetChannelMember(channel.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the channel member") - } + require.Nil(t, err, "Failed to get the channel member") + assert.True(t, channelMember.SchemeAdmin) assert.True(t, channelMember.SchemeUser) assert.False(t, channelMember.SchemeGuest) @@ -1611,14 +1466,10 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(*deletedUserData.Username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") teamMember, err = th.App.GetTeamMember(team.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the team member") - } + require.Nil(t, err, "Failed to get the team member") assert.False(t, teamMember.SchemeAdmin) assert.True(t, teamMember.SchemeUser) @@ -1626,9 +1477,7 @@ func TestImportImportUser(t *testing.T) { assert.Equal(t, "", teamMember.ExplicitRoles) channelMember, err = th.App.GetChannelMember(channel.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the channel member") - } + require.Nil(t, err, "Failed to get the channel member") assert.False(t, teamMember.SchemeAdmin) assert.True(t, channelMember.SchemeUser) @@ -1659,14 +1508,10 @@ func TestImportImportUser(t *testing.T) { assert.Nil(t, err) user, err = th.App.GetUserByUsername(*deletedGuestData.Username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") teamMember, err = th.App.GetTeamMember(team.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the team member") - } + require.Nil(t, err, "Failed to get the team member") assert.False(t, teamMember.SchemeAdmin) assert.False(t, teamMember.SchemeUser) @@ -1674,9 +1519,7 @@ func TestImportImportUser(t *testing.T) { assert.Equal(t, "", teamMember.ExplicitRoles) channelMember, err = th.App.GetChannelMember(channel.Id, user.Id) - if err != nil { - t.Fatalf("Failed to get the channel member") - } + require.Nil(t, err, "Failed to get the channel member") assert.False(t, teamMember.SchemeAdmin) assert.False(t, channelMember.SchemeUser) @@ -1734,9 +1577,7 @@ func TestImportImportPost(t *testing.T) { Type: ptrStr("O"), }, false) team, err := th.App.GetTeamByName(teamName) - if err != nil { - t.Fatalf("Failed to get team from database.") - } + require.Nil(t, err, "Failed to get team from database.") // Create a Channel. channelName := model.NewId() @@ -1747,9 +1588,7 @@ func TestImportImportPost(t *testing.T) { Type: ptrStr("O"), }, false) channel, err := th.App.GetChannelByName(channelName, team.Id, false) - if err != nil { - t.Fatalf("Failed to get channel from database.") - } + require.Nil(t, err, "Failed to get channel from database.") // Create a user. username := model.NewId() @@ -1758,15 +1597,11 @@ func TestImportImportPost(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user, err := th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") // Count the number of posts in the testing team. initialPostCount, err := th.App.Srv.Store.Post().AnalyticsPostCount(team.Id, false, false) - if err != nil { - t.Fatal(err) - } + require.Nil(t, err) // Try adding an invalid post in dry run mode. data := &PostImportData{ @@ -1852,17 +1687,13 @@ func TestImportImportPost(t *testing.T) { // Check the post values. posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time) - if err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id { - t.Fatal("Post properties not as expected") - } - } + require.Nil(t, err) + + require.Len(t, posts, 1, "Unexpected number of posts found.") + + post := posts[0] + postBool := post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id + require.False(t, postBool, "Post properties not as expected") // Update the post. data = &PostImportData{ @@ -1878,17 +1709,13 @@ func TestImportImportPost(t *testing.T) { // Check the post values. posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, time) - if err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id { - t.Fatal("Post properties not as expected") - } - } + require.Nil(t, err) + + require.Len(t, posts, 1, "Unexpected number of posts found.") + + post = posts[0] + postBool = post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id + require.False(t, postBool, "Post properties not as expected") // Save the post with a different time. newTime := time + 1 @@ -1929,20 +1756,15 @@ func TestImportImportPost(t *testing.T) { AssertAllPostsCount(t, th.App, initialPostCount, 4, team.Id) posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, hashtagTime) - if err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id { - t.Fatal("Post properties not as expected") - } - if post.Hashtags != "#hashtagmashupcity" { - t.Fatalf("Hashtags not as expected: %s", post.Hashtags) - } - } + require.Nil(t, err) + + require.Len(t, posts, 1, "Unexpected number of posts found.") + + post = posts[0] + postBool = post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id + require.False(t, postBool, "Post properties not as expected") + + require.Equal(t, "#hashtagmashupcity", post.Hashtags, "Hashtags not as expected: %s", post.Hashtags) // Post with flags. username2 := model.NewId() @@ -1951,9 +1773,7 @@ func TestImportImportPost(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user2, err := th.App.GetUserByUsername(username2) - if err != nil { - t.Fatalf("Failed to get user from database.") - } + require.Nil(t, err, "Failed to get user from database.") flagsTime := hashtagTime + 1 data = &PostImportData{ @@ -1967,26 +1787,24 @@ func TestImportImportPost(t *testing.T) { username2, }, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 5, team.Id) // Check the post values. - if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, flagsTime); err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id { - t.Fatal("Post properties not as expected") - } + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, flagsTime) + require.Nil(t, err) - checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_FLAGGED_POST, post.Id, "true") - checkPreference(t, th.App, user2.Id, model.PREFERENCE_CATEGORY_FLAGGED_POST, post.Id, "true") - } + require.Len(t, posts, 1, "Unexpected number of posts found.") + + post = posts[0] + postBool = post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id + require.False(t, postBool, "Post properties not as expected") + + checkPreference(t, th.App, user.Id, model.PREFERENCE_CATEGORY_FLAGGED_POST, post.Id, "true") + checkPreference(t, th.App, user2.Id, model.PREFERENCE_CATEGORY_FLAGGED_POST, post.Id, "true") // Post with reaction. reactionPostTime := hashtagTime + 2 @@ -2003,29 +1821,25 @@ func TestImportImportPost(t *testing.T) { CreateAt: &reactionTime, }}, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 6, team.Id) // Check the post values. - if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, reactionPostTime); err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id || !post.HasReactions { - t.Fatal("Post properties not as expected") - } + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, reactionPostTime) + require.Nil(t, err) - if reactions, err := th.App.Srv.Store.Reaction().GetForPost(post.Id, false); err != nil { - t.Fatal("Can't get reaction") - } else if len(reactions) != 1 { - t.Fatal("Invalid number of reactions") - } - } + require.Len(t, posts, 1, "Unexpected number of posts found.") + + post = posts[0] + postBool = post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id || !post.HasReactions + require.False(t, postBool, "Post properties not as expected") + + reactions, err := th.App.Srv.Store.Reaction().GetForPost(post.Id, false) + require.Nil(t, err, "Can't get reaction") + + require.Len(t, reactions, 1, "Invalid number of reactions") // Post with reply. replyPostTime := hashtagTime + 4 @@ -2042,40 +1856,32 @@ func TestImportImportPost(t *testing.T) { CreateAt: &replyTime, }}, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 8, team.Id) // Check the post values. - if posts, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyPostTime); err != nil { - t.Fatal(err.Error()) - } else { - if len(posts) != 1 { - t.Fatal("Unexpected number of posts found.") - } - post := posts[0] - if post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id { - t.Fatal("Post properties not as expected") - } + posts, err = th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyPostTime) + require.Nil(t, err) - // Check the reply values. - if replies, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyTime); err != nil { - t.Fatal(err.Error()) - } else { - if len(replies) != 1 { - t.Fatal("Unexpected number of posts found.") - } - reply := replies[0] - if reply.Message != *(*data.Replies)[0].Message || reply.CreateAt != *(*data.Replies)[0].CreateAt || reply.UserId != user2.Id { - t.Fatal("Post properties not as expected") - } + require.Len(t, posts, 1, "Unexpected number of posts found.") - if reply.RootId != post.Id { - t.Fatal("Unexpected reply RootId") - } - } - } + post = posts[0] + postBool = post.Message != *data.Message || post.CreateAt != *data.CreateAt || post.UserId != user.Id + require.False(t, postBool, "Post properties not as expected") + + // Check the reply values. + replies, err := th.App.Srv.Store.Post().GetPostsCreatedAt(channel.Id, replyTime) + require.Nil(t, err) + + require.Len(t, replies, 1, "Unexpected number of posts found.") + + reply := replies[0] + replyBool := reply.Message != *(*data.Replies)[0].Message || reply.CreateAt != *(*data.Replies)[0].CreateAt || reply.UserId != user2.Id + require.False(t, replyBool, "Post properties not as expected") + + require.Equal(t, post.Id, reply.RootId, "Unexpected reply RootId") // Update post with replies. data = &PostImportData{ @@ -2090,9 +1896,9 @@ func TestImportImportPost(t *testing.T) { CreateAt: &replyTime, }}, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 8, team.Id) // Create new post with replies based on the previous one. @@ -2108,9 +1914,9 @@ func TestImportImportPost(t *testing.T) { CreateAt: &replyTime, }}, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 10, team.Id) // Create new reply for existing post with replies. @@ -2126,9 +1932,9 @@ func TestImportImportPost(t *testing.T) { CreateAt: &replyTime, }}, } - if err := th.App.ImportPost(data, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportPost(data, false) + require.Nil(t, err, "Expected success.") + AssertAllPostsCount(t, th.App, initialPostCount, 11, team.Id) } @@ -2700,7 +2506,6 @@ func TestImportAttachment(t *testing.T) { } func TestImportPostAndRepliesWithAttachments(t *testing.T) { - th := Setup(t) defer th.TearDown() @@ -2712,9 +2517,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { Type: ptrStr("O"), }, false) team, err := th.App.GetTeamByName(teamName) - if err != nil { - t.Fatalf("Failed to get team from database.") - } + require.Nil(t, err, "Failed to get team from database.") // Create a Channel. channelName := model.NewId() @@ -2725,9 +2528,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { Type: ptrStr("O"), }, false) _, err = th.App.GetChannelByName(channelName, team.Id, false) - if err != nil { - t.Fatalf("Failed to get channel from database.") - } + require.Nil(t, err, "Failed to get channel from database.") // Create a user3. username := model.NewId() @@ -2736,9 +2537,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user3, err := th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user3 from database.") - } + require.Nil(t, err, "Failed to get user3 from database.") username2 := model.NewId() th.App.ImportUser(&UserImportData{ @@ -2746,9 +2545,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user4, err := th.App.GetUserByUsername(username2) - if err != nil { - t.Fatalf("Failed to get user3 from database.") - } + require.Nil(t, err, "Failed to get user3 from database.") // Post with attachments. time := model.GetMillis() @@ -2796,9 +2593,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user3, err = th.App.GetUserByUsername(username3) - if err != nil { - t.Fatalf("Failed to get user3 from database.") - } + require.Nil(t, err, "Failed to get user3 from database.") username4 := model.NewId() th.App.ImportUser(&UserImportData{ @@ -2807,9 +2602,7 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { }, false) user4, err = th.App.GetUserByUsername(username4) - if err != nil { - t.Fatalf("Failed to get user3 from database.") - } + require.Nil(t, err, "Failed to get user3 from database.") directImportData := &DirectPostImportData{ ChannelMembers: &[]string{ @@ -2827,15 +2620,13 @@ func TestImportPostAndRepliesWithAttachments(t *testing.T) { }}, } - if err := th.App.ImportDirectPost(directImportData, false); err != nil { - t.Fatalf("Expected success.") - } + err = th.App.ImportDirectPost(directImportData, false) + require.Nil(t, err, "Expected success.") attachments = GetAttachments(user4.Id, th, t) assert.Equal(t, len(attachments), 1) assert.Contains(t, attachments[0].Path, "noteam") AssertFileIdsInPost(attachments, th, t) - } func TestImportDirectPostWithAttachments(t *testing.T) { @@ -2861,9 +2652,7 @@ func TestImportDirectPostWithAttachments(t *testing.T) { Email: ptrStr(model.NewId() + "@example.com"), }, false) user1, err := th.App.GetUserByUsername(username) - if err != nil { - t.Fatalf("Failed to get user1 from database.") - } + require.Nil(t, err, "Failed to get user1 from database.") username2 := model.NewId() th.App.ImportUser(&UserImportData{ @@ -2872,9 +2661,7 @@ func TestImportDirectPostWithAttachments(t *testing.T) { }, false) user2, err := th.App.GetUserByUsername(username2) - if err != nil { - t.Fatalf("Failed to get user2 from database.") - } + require.Nil(t, err, "Failed to get user2 from database.") directImportData := &DirectPostImportData{ ChannelMembers: &[]string{ @@ -2888,9 +2675,8 @@ func TestImportDirectPostWithAttachments(t *testing.T) { } t.Run("Regular import of attachment", func(t *testing.T) { - if err := th.App.ImportDirectPost(directImportData, false); err != nil { - t.Fatalf("Expected success.") - } + err := th.App.ImportDirectPost(directImportData, false) + require.Nil(t, err, "Expected success.") attachments := GetAttachments(user1.Id, th, t) assert.Equal(t, len(attachments), 1) @@ -2899,9 +2685,8 @@ func TestImportDirectPostWithAttachments(t *testing.T) { }) t.Run("Attempt to import again with same file entirely, should NOT add an attachment", func(t *testing.T) { - if err := th.App.ImportDirectPost(directImportData, false); err != nil { - t.Fatalf("Expected success.") - } + err := th.App.ImportDirectPost(directImportData, false) + require.Nil(t, err, "Expected success.") attachments := GetAttachments(user1.Id, th, t) assert.Equal(t, len(attachments), 1) @@ -2919,9 +2704,8 @@ func TestImportDirectPostWithAttachments(t *testing.T) { Attachments: &[]AttachmentImportData{{Path: &testImageFake}}, } - if err := th.App.ImportDirectPost(directImportDataFake, false); err != nil { - t.Fatalf("Expected success.") - } + err := th.App.ImportDirectPost(directImportDataFake, false) + require.Nil(t, err, "Expected success.") attachments := GetAttachments(user1.Id, th, t) assert.Equal(t, len(attachments), 2) @@ -2939,9 +2723,8 @@ func TestImportDirectPostWithAttachments(t *testing.T) { Attachments: &[]AttachmentImportData{{Path: &testImage2}}, } - if err := th.App.ImportDirectPost(directImportData2, false); err != nil { - t.Fatalf("Expected success.") - } + err := th.App.ImportDirectPost(directImportData2, false) + require.Nil(t, err, "Expected success.") attachments := GetAttachments(user1.Id, th, t) assert.Equal(t, len(attachments), 3) diff --git a/app/job_test.go b/app/job_test.go index 6a3b0f6357..d45a45a6e8 100644 --- a/app/job_test.go +++ b/app/job_test.go @@ -23,11 +23,9 @@ func TestGetJob(t *testing.T) { defer th.App.Srv.Store.Job().Delete(status.Id) - if received, err := th.App.GetJob(status.Id); err != nil { - t.Fatal(err) - } else if received.Id != status.Id || received.Status != status.Status { - t.Fatal("inccorrect job status received") - } + received, err := th.App.GetJob(status.Id) + require.Nil(t, err) + require.Equal(t, status, received, "incorrect job status received") } func TestGetJobByType(t *testing.T) { @@ -60,21 +58,14 @@ func TestGetJobByType(t *testing.T) { defer th.App.Srv.Store.Job().Delete(status.Id) } - if received, err := th.App.GetJobsByType(jobType, 0, 2); err != nil { - t.Fatal(err) - } else if len(received) != 2 { - t.Fatal("received wrong number of statuses") - } else if received[0].Id != statuses[2].Id { - t.Fatal("should've received newest job first") - } else if received[1].Id != statuses[0].Id { - t.Fatal("should've received second newest job second") - } + received, err := th.App.GetJobsByType(jobType, 0, 2) + require.Nil(t, err) + require.Len(t, received, 2, "received wrong number of statuses") + require.Equal(t, statuses[2], received[0], "should've received newest job first") + require.Equal(t, statuses[0], received[1], "should've received second newest job second") - if received, err := th.App.GetJobsByType(jobType, 2, 2); err != nil { - t.Fatal(err) - } else if len(received) != 1 { - t.Fatal("received wrong number of statuses") - } else if received[0].Id != statuses[1].Id { - t.Fatal("should've received oldest job last") - } + received, err = th.App.GetJobsByType(jobType, 2, 2) + require.Nil(t, err) + require.Len(t, received, 1, "received wrong number of statuses") + require.Equal(t, statuses[1], received[0], "should've received oldest job last") } diff --git a/app/plugin.go b/app/plugin.go index c1d67a44fc..e543a7de51 100644 --- a/app/plugin.go +++ b/app/plugin.go @@ -43,6 +43,7 @@ func (a *App) SetPluginsEnvironment(pluginsEnvironment *plugin.Environment) { } func (a *App) SyncPluginsActiveState() { + // Acquiring lock manually, as plugins might be disabled. See GetPluginsEnvironment. a.Srv.PluginsLock.RLock() pluginsEnvironment := a.Srv.PluginsEnvironment a.Srv.PluginsLock.RUnlock() @@ -124,6 +125,7 @@ func (a *App) NewPluginAPI(manifest *model.Manifest) plugin.API { } func (a *App) InitPlugins(pluginDir, webappPluginDir string) { + // Acquiring lock manually, as plugins might be disabled. See GetPluginsEnvironment. a.Srv.PluginsLock.RLock() pluginsEnvironment := a.Srv.PluginsEnvironment a.Srv.PluginsLock.RUnlock() @@ -257,9 +259,7 @@ func (a *App) SyncPlugins() *model.AppError { } func (a *App) ShutDownPlugins() { - a.Srv.PluginsLock.Lock() - pluginsEnvironment := a.Srv.PluginsEnvironment - defer a.Srv.PluginsLock.Unlock() + pluginsEnvironment := a.GetPluginsEnvironment() if pluginsEnvironment == nil { return } @@ -270,7 +270,15 @@ func (a *App) ShutDownPlugins() { a.RemoveConfigListener(a.Srv.PluginConfigListenerId) a.Srv.PluginConfigListenerId = "" - a.Srv.PluginsEnvironment = nil + + // Acquiring lock manually before cleaning up PluginsEnvironment. + a.Srv.PluginsLock.Lock() + defer a.Srv.PluginsLock.Unlock() + if a.Srv.PluginsEnvironment == pluginsEnvironment { + a.Srv.PluginsEnvironment = nil + } else { + mlog.Warn("Another PluginsEnvironment detected while shutting down plugins.") + } } func (a *App) GetActivePluginManifests() ([]*model.Manifest, *model.AppError) { diff --git a/app/plugin_api.go b/app/plugin_api.go index 867e2f587d..04da1e7ce1 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -466,7 +466,7 @@ func (api *PluginAPI) DeletePost(postId string) *model.AppError { } func (api *PluginAPI) GetPostThread(postId string) (*model.PostList, *model.AppError) { - return api.app.GetPostThread(postId) + return api.app.GetPostThread(postId, false) } func (api *PluginAPI) GetPost(postId string) (*model.Post, *model.AppError) { diff --git a/app/plugin_deadlock_test.go b/app/plugin_deadlock_test.go index ae777e34f9..0eb28c5e16 100644 --- a/app/plugin_deadlock_test.go +++ b/app/plugin_deadlock_test.go @@ -4,13 +4,14 @@ package app import ( - "github.com/stretchr/testify/require" "os" "strings" "testing" "text/template" "time" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/model" ) @@ -209,4 +210,112 @@ func TestPluginDeadlock(t *testing.T) { }() } }) + + t.Run("CreatePost on OnDeactivate Plugin", func(t *testing.T) { + th := Setup(t).InitBasic() + + pluginPostOnActivate := template.Must(template.New("pluginPostOnActivate").Parse(` + package main + + import ( + "github.com/mattermost/mattermost-server/plugin" + "github.com/mattermost/mattermost-server/model" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnDeactivate() error { + _, err := p.API.CreatePost(&model.Post{ + UserId: "{{.User.Id}}", + ChannelId: "{{.Channel.Id}}", + Message: "OnDeactivate", + }) + if err != nil { + panic(err.Error()) + } + + return nil + } + + func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) { + updatedPost := &model.Post{ + UserId: "{{.User.Id}}", + ChannelId: "{{.Channel.Id}}", + Message: "messageUpdated", + Props: map[string]interface{}{ + "from_plugin": true, + }, + } + + return updatedPost, "" + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } +`, + )) + + templateData := struct { + User *model.User + Channel *model.Channel + }{ + th.BasicUser, + th.BasicChannel, + } + + plugins := []string{} + pluginTemplates := []*template.Template{ + pluginPostOnActivate, + } + for _, pluginTemplate := range pluginTemplates { + b := &strings.Builder{} + pluginTemplate.Execute(b, templateData) + + plugins = append(plugins, b.String()) + } + + done := make(chan bool) + go func() { + posts, appErr := th.App.GetPosts(th.BasicChannel.Id, 0, 2) + require.Nil(t, appErr) + require.NotNil(t, posts) + + messageWillBePostedCalled := false + for _, p := range posts.Posts { + if p.Message == "messageUpdated" { + messageWillBePostedCalled = true + } + } + require.False(t, messageWillBePostedCalled, "MessageWillBePosted should not have been called") + + SetAppEnvironmentWithPlugins(t, plugins, th.App, th.App.NewPluginAPI) + th.TearDown() + + posts, appErr = th.App.GetPosts(th.BasicChannel.Id, 0, 2) + require.Nil(t, appErr) + require.NotNil(t, posts) + + messageWillBePostedCalled = false + for _, p := range posts.Posts { + if p.Message == "messageUpdated" { + messageWillBePostedCalled = true + } + } + require.True(t, messageWillBePostedCalled, "MessageWillBePosted was not called on deactivate") + close(done) + }() + + select { + case <-done: + case <-time.After(30 * time.Second): + require.Fail(t, "plugin failed to activate: likely deadlocked") + go func() { + time.Sleep(5 * time.Second) + os.Exit(1) + }() + } + }) } diff --git a/app/plugin_install.go b/app/plugin_install.go index 10fe455db1..26b768418e 100644 --- a/app/plugin_install.go +++ b/app/plugin_install.go @@ -251,6 +251,10 @@ func (a *App) removePlugin(id string) *model.AppError { }, ) + if err := a.notifyPluginStatusesChanged(); err != nil { + mlog.Error("Failed to notify plugin status changed", mlog.Err(err)) + } + return nil } diff --git a/app/post.go b/app/post.go index aa463e4b35..121d99bd46 100644 --- a/app/post.go +++ b/app/post.go @@ -167,7 +167,7 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo if len(post.RootId) > 0 { pchan = make(chan store.StoreResult, 1) go func() { - r, pErr := a.Srv.Store.Post().Get(post.RootId, true) + r, pErr := a.Srv.Store.Post().Get(post.RootId, false) pchan <- store.StoreResult{Data: r, Err: pErr} close(pchan) }() @@ -475,7 +475,7 @@ func (a *App) DeleteEphemeralPost(userId, postId string) { func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model.AppError) { post.SanitizeProps() - postLists, err := a.Srv.Store.Post().Get(post.Id, true) + postLists, err := a.Srv.Store.Post().Get(post.Id, false) if err != nil { return nil, err } @@ -634,8 +634,8 @@ func (a *App) GetSinglePost(postId string) (*model.Post, *model.AppError) { return a.Srv.Store.Post().GetSingle(postId) } -func (a *App) GetPostThread(postId string) (*model.PostList, *model.AppError) { - return a.Srv.Store.Post().Get(postId, false) +func (a *App) GetPostThread(postId string, skipFetchThreads bool) (*model.PostList, *model.AppError) { + return a.Srv.Store.Post().Get(postId, skipFetchThreads) } func (a *App) GetFlaggedPosts(userId string, offset int, limit int) (*model.PostList, *model.AppError) { @@ -789,7 +789,7 @@ func (a *App) GetPostsForChannelAroundLastUnread(channelId, userId string, limit return model.NewPostList(), nil } - postList, err := a.GetPostThread(lastUnreadPostId) + postList, err := a.GetPostThread(lastUnreadPostId, false) if err != nil { return nil, err } diff --git a/app/server_app_adapters.go b/app/server_app_adapters.go index f9c761647f..85bcdc11d2 100644 --- a/app/server_app_adapters.go +++ b/app/server_app_adapters.go @@ -4,7 +4,6 @@ package app import ( - "fmt" "net/http" "net/url" "path" @@ -67,7 +66,7 @@ func (s *Server) RunOldAppInitalization() error { } if htmlTemplateWatcher, err := utils.NewHTMLTemplateWatcher("templates"); err != nil { - mlog.Error(fmt.Sprintf("Failed to parse server templates %v", err)) + mlog.Error("Failed to parse server templates", mlog.Err(err)) } else { s.FakeApp().Srv.htmlTemplateWatcher = htmlTemplateWatcher } @@ -131,7 +130,7 @@ func (s *Server) RunOldAppInitalization() error { appErr = backend.TestConnection() } if appErr != nil { - mlog.Error("Problem with file storage settings: " + appErr.Error()) + mlog.Error("Problem with file storage settings", mlog.Err(appErr)) } if model.BuildEnterpriseReady == "true" { diff --git a/cmd/mattermost/commands/command_test.go b/cmd/mattermost/commands/command_test.go index d6594cf585..b3e304c4da 100644 --- a/cmd/mattermost/commands/command_test.go +++ b/cmd/mattermost/commands/command_test.go @@ -276,7 +276,7 @@ func TestModifyCommand(t *testing.T) { assert.Equal(t, cmd.IconURL, command.IconURL) }) - t.Run("mispelled flag", func(t *testing.T) { + t.Run("misspelled flag", func(t *testing.T) { args := []string{"command", "", command.Id, "--trigger-wor", "sometrigger"} output, _ := th.RunCommandWithOutput(t, args...) assert.Contains(t, string(output), "Error: unknown flag:") diff --git a/config/main_test.go b/config/main_test.go index 3d7192ff7f..65e932268f 100644 --- a/config/main_test.go +++ b/config/main_test.go @@ -47,7 +47,7 @@ func truncateTable(t *testing.T, table string) { require.NoError(t, err) default: - t.Fatalf("unsupported driver name: %s", *sqlSetting.DriverName) + require.Failf(t, "failed", "unsupported driver name: %s", *sqlSetting.DriverName) } } diff --git a/i18n/en.json b/i18n/en.json index bf4283b445..c8f717bca2 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4722,6 +4722,10 @@ "id": "model.config.is_valid.saml_email_attribute.app_error", "translation": "Invalid Email attribute. Must be set." }, + { + "id": "model.config.is_valid.saml_guest_attribute.app_error", + "translation": "Invalid Guest attribute. Must be in the form 'field=value'" + }, { "id": "model.config.is_valid.saml_idp_cert.app_error", "translation": "Identity Provider Public Certificate missing. Did you forget to upload it?" diff --git a/model/audit_test.go b/model/audit_test.go index 44e4cfb34b..7a9ce9395e 100644 --- a/model/audit_test.go +++ b/model/audit_test.go @@ -6,14 +6,13 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestAuditJson(t *testing.T) { audit := Audit{Id: NewId(), UserId: NewId(), CreateAt: GetMillis()} json := audit.ToJson() result := AuditFromJson(strings.NewReader(json)) - - if audit.Id != result.Id { - t.Fatal("Ids do not match") - } + require.Equal(t, audit.Id, result.Id, "Ids do not match") } diff --git a/model/audits_test.go b/model/audits_test.go index 3b22b26683..9c1fb9e575 100644 --- a/model/audits_test.go +++ b/model/audits_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestAuditsJson(t *testing.T) { @@ -13,9 +15,7 @@ func TestAuditsJson(t *testing.T) { json := audit.ToJson() result := AuditFromJson(strings.NewReader(json)) - if audit.Id != result.Id { - t.Fatal("Ids do not match") - } + require.Equal(t, audit.Id, result.Id, "Ids do not match") var audits Audits = make([]Audit, 1) audits[0] = audit @@ -23,7 +23,5 @@ func TestAuditsJson(t *testing.T) { ljson := audits.ToJson() results := AuditsFromJson(strings.NewReader(ljson)) - if audits[0].Id != results[0].Id { - t.Fatal("Ids do not match") - } + require.Equal(t, audits[0].Id, results[0].Id, "Ids do not match") } diff --git a/model/authorize_test.go b/model/authorize_test.go index 0775b06c1f..211d74def4 100644 --- a/model/authorize_test.go +++ b/model/authorize_test.go @@ -18,10 +18,7 @@ func TestAuthJson(t *testing.T) { json := a1.ToJson() ra1 := AuthDataFromJson(strings.NewReader(json)) - - if a1.Code != ra1.Code { - t.Fatal("codes didn't match") - } + require.Equal(t, a1.Code, ra1.Code, "codes didn't match") a2 := AuthorizeRequest{} a2.ClientId = NewId() @@ -30,9 +27,7 @@ func TestAuthJson(t *testing.T) { json = a2.ToJson() ra2 := AuthorizeRequestFromJson(strings.NewReader(json)) - if a2.ClientId != ra2.ClientId { - t.Fatal("client ids didn't match") - } + require.Equal(t, a2.ClientId, ra2.ClientId, "client ids didn't match") } func TestAuthPreSave(t *testing.T) { @@ -51,85 +46,59 @@ func TestAuthIsValid(t *testing.T) { require.NotNil(t, ad.IsValid()) ad.ClientId = NewRandomString(28) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed Client Id") - } + require.NotNil(t, ad.IsValid(), "Should have failed Client Id") ad.ClientId = NewId() require.NotNil(t, ad.IsValid()) ad.UserId = NewRandomString(28) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed User Id") - } + require.NotNil(t, ad.IsValid(), "Should have failed User Id") ad.UserId = NewId() require.NotNil(t, ad.IsValid()) ad.Code = NewRandomString(129) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed Code to long") - } + require.NotNil(t, ad.IsValid(), "Should have failed Code to long") ad.Code = "" - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed Code not set") - } + require.NotNil(t, ad.IsValid(), "Should have failed Code not set") ad.Code = NewId() require.NotNil(t, ad.IsValid()) ad.ExpiresIn = 0 - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed invalid ExpiresIn") - } + require.NotNil(t, ad.IsValid(), "Should have failed invalid ExpiresIn") ad.ExpiresIn = 1 require.NotNil(t, ad.IsValid()) ad.CreateAt = 0 - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed Invalid Create At") - } + require.NotNil(t, ad.IsValid(), "Should have failed Invalid Create At") ad.CreateAt = 1 require.NotNil(t, ad.IsValid()) ad.State = NewRandomString(129) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed invalid State") - } + require.NotNil(t, ad.IsValid(), "Should have failed invalid State") ad.State = NewRandomString(128) - if err := ad.IsValid(); err == nil { - t.Fatal(err) - } + require.NotNil(t, ad.IsValid()) ad.Scope = NewRandomString(1025) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed invalid Scope") - } + require.NotNil(t, ad.IsValid(), "Should have failed invalid Scope") ad.Scope = NewRandomString(128) require.NotNil(t, ad.IsValid()) ad.RedirectUri = "" - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed Redirect URI not set") - } + require.NotNil(t, ad.IsValid(), "Should have failed Redirect URI not set") ad.RedirectUri = NewRandomString(28) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed invalid URL") - } + require.NotNil(t, ad.IsValid(), "Should have failed invalid URL") ad.RedirectUri = NewRandomString(257) - if err := ad.IsValid(); err == nil { - t.Fatal("Should have failed invalid URL") - } + require.NotNil(t, ad.IsValid(), "Should have failed invalid URL") ad.RedirectUri = "http://example.com" - if err := ad.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, ad.IsValid()) } diff --git a/model/channel_search_test.go b/model/channel_search_test.go index 97c1b3ecdb..e53fe4f2ca 100644 --- a/model/channel_search_test.go +++ b/model/channel_search_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) func TestChannelSearchJson(t *testing.T) { @@ -13,7 +15,5 @@ func TestChannelSearchJson(t *testing.T) { json := channelSearch.ToJson() rchannelSearch := ChannelSearchFromJson(strings.NewReader(json)) - if channelSearch.Term != rchannelSearch.Term { - t.Fatal("Terms do not match") - } + assert.Equal(t, channelSearch.Term, rchannelSearch.Term) } diff --git a/model/cluster_discovery_test.go b/model/cluster_discovery_test.go index 0e4aaba4b0..28adfff2df 100644 --- a/model/cluster_discovery_test.go +++ b/model/cluster_discovery_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) func TestClusterDiscovery(t *testing.T) { @@ -18,9 +20,8 @@ func TestClusterDiscovery(t *testing.T) { json := o.ToJson() result1 := ClusterDiscoveryFromJson(strings.NewReader(json)) - if result1.ClusterName != "cluster_name" { - t.Fatal("should be set") - } + assert.NotNil(t, result1) + assert.Equal(t, "cluster_name", result1.ClusterName) result2 := ClusterDiscoveryFromJson(strings.NewReader(json)) result3 := ClusterDiscoveryFromJson(strings.NewReader(json)) @@ -31,9 +32,7 @@ func TestClusterDiscovery(t *testing.T) { result3.Id = "3" result3.Hostname = "something_diff" - if !o.IsEqual(result1) { - t.Fatal("Should be equal") - } + assert.True(t, o.IsEqual(result1)) list := make([]*ClusterDiscovery, 0) list = append(list, &o) @@ -45,9 +44,7 @@ func TestClusterDiscovery(t *testing.T) { return !o.IsEqual(in) }) - if len(rlist) != 1 { - t.Fatal("should only have 1 result") - } + assert.Len(t, rlist, 1) o.AutoFillHostname() o.Hostname = "" diff --git a/model/cluster_info_test.go b/model/cluster_info_test.go index ec6ba6c1fd..4f81d221cc 100644 --- a/model/cluster_info_test.go +++ b/model/cluster_info_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) func TestClusterInfoJson(t *testing.T) { @@ -13,9 +15,7 @@ func TestClusterInfoJson(t *testing.T) { json := cluster.ToJson() result := ClusterInfoFromJson(strings.NewReader(json)) - if cluster.IpAddress != result.IpAddress { - t.Fatal("Ids do not match") - } + assert.Equal(t, cluster.IpAddress, result.IpAddress, "Ids do not match") } func TestClusterInfosJson(t *testing.T) { @@ -25,7 +25,5 @@ func TestClusterInfosJson(t *testing.T) { json := ClusterInfosToJson(clusterInfos) result := ClusterInfosFromJson(strings.NewReader(json)) - if clusterInfos[0].IpAddress != result[0].IpAddress { - t.Fatal("Ids do not match") - } + assert.Equal(t, clusterInfos[0].IpAddress, result[0].IpAddress, "Ids do not match") } diff --git a/model/command_test.go b/model/command_test.go index 53c15f74ad..0c8136b57c 100644 --- a/model/command_test.go +++ b/model/command_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestCommandJson(t *testing.T) { @@ -13,9 +15,7 @@ func TestCommandJson(t *testing.T) { json := o.ToJson() ro := CommandFromJson(strings.NewReader(json)) - if o.Id != ro.Id { - t.Fatal("Ids do not match") - } + require.Equal(t, o.Id, ro.Id, "Ids do not match") } func TestCommandIsValid(t *testing.T) { @@ -33,134 +33,82 @@ func TestCommandIsValid(t *testing.T) { Description: "", } - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Id = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Id = NewId() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Token = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Token = NewId() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.CreateAt = 0 - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.CreateAt = GetMillis() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.UpdateAt = 0 - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.UpdateAt = GetMillis() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.CreatorId = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.CreatorId = NewId() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.TeamId = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.TeamId = NewId() - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Trigger = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Trigger = strings.Repeat("1", 129) - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Trigger = strings.Repeat("1", 128) - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.URL = "" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.URL = "1234" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.URL = "https://example.com" - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Method = "https://example.com" - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Method = COMMAND_METHOD_GET - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Method = COMMAND_METHOD_POST - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.DisplayName = strings.Repeat("1", 65) - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.DisplayName = strings.Repeat("1", 64) - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) o.Description = strings.Repeat("1", 129) - if err := o.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.NotNil(t, o.IsValid(), "should be invalid") o.Description = strings.Repeat("1", 128) - if err := o.IsValid(); err != nil { - t.Fatal(err) - } + require.Nil(t, o.IsValid()) } func TestCommandPreSave(t *testing.T) { diff --git a/model/command_webhook_test.go b/model/command_webhook_test.go index 629bbdaa70..141350b78a 100644 --- a/model/command_webhook_test.go +++ b/model/command_webhook_test.go @@ -5,17 +5,17 @@ package model import ( "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestCommandWebhookPreSave(t *testing.T) { h := CommandWebhook{} h.PreSave() - if len(h.Id) != 26 { - t.Fatal("Id should be generated") - } - if h.CreateAt == 0 { - t.Fatal("CreateAt should be set") - } + + require.Len(t, h.Id, 26, "Id should be generated") + require.NotEqual(t, 0, h.CreateAt, "CreateAt should be set") } func TestCommandWebhookIsValid(t *testing.T) { @@ -44,11 +44,14 @@ func TestCommandWebhookIsValid(t *testing.T) { tmp := h test.Transform() err := h.IsValid() - if test.ExpectedError == "" && err != nil { - t.Fatal("hook should be valid") - } else if test.ExpectedError != "" && test.ExpectedError != err.Id { - t.Fatal("expected " + test.ExpectedError + " error") + + if test.ExpectedError == "" { + assert.Error(t, err, "hook should be valid") + } else { + require.NotNil(t, err) + assert.Equal(t, test.ExpectedError, err.Id, "expected "+test.ExpectedError+" error") } + h = tmp } } diff --git a/model/compliance_test.go b/model/compliance_test.go index 32ce48db5c..2f22dc2b22 100644 --- a/model/compliance_test.go +++ b/model/compliance_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestCompliance(t *testing.T) { @@ -13,7 +15,5 @@ func TestCompliance(t *testing.T) { json := o.ToJson() result := ComplianceFromJson(strings.NewReader(json)) - if o.Desc != result.Desc { - t.Fatal("JobName do not match") - } + require.Equal(t, o.Desc, result.Desc, "JobName do not match") } diff --git a/model/config.go b/model/config.go index 81e594f03d..016477a3ab 100644 --- a/model/config.go +++ b/model/config.go @@ -130,6 +130,7 @@ const ( LDAP_SETTINGS_DEFAULT_GROUP_ID_ATTRIBUTE = "" SAML_SETTINGS_DEFAULT_ID_ATTRIBUTE = "" + SAML_SETTINGS_DEFAULT_GUEST_ATTRIBUTE = "" SAML_SETTINGS_DEFAULT_FIRST_NAME_ATTRIBUTE = "" SAML_SETTINGS_DEFAULT_LAST_NAME_ATTRIBUTE = "" SAML_SETTINGS_DEFAULT_EMAIL_ATTRIBUTE = "" @@ -1907,6 +1908,7 @@ type SamlSettings struct { // User Mapping IdAttribute *string + GuestAttribute *string FirstNameAttribute *string LastNameAttribute *string EmailAttribute *string @@ -1999,6 +2001,9 @@ func (s *SamlSettings) SetDefaults() { s.IdAttribute = NewString(SAML_SETTINGS_DEFAULT_ID_ATTRIBUTE) } + if s.GuestAttribute == nil { + s.GuestAttribute = NewString(SAML_SETTINGS_DEFAULT_GUEST_ATTRIBUTE) + } if s.FirstNameAttribute == nil { s.FirstNameAttribute = NewString(SAML_SETTINGS_DEFAULT_FIRST_NAME_ATTRIBUTE) } @@ -2835,6 +2840,15 @@ func (ss *SamlSettings) isValid() *AppError { if !(*ss.CanonicalAlgorithm == SAML_SETTINGS_CANONICAL_ALGORITHM_C14N || *ss.CanonicalAlgorithm == SAML_SETTINGS_CANONICAL_ALGORITHM_C14N11) { return NewAppError("Config.IsValid", "model.config.is_valid.saml_canonical_algorithm.app_error", nil, "", http.StatusBadRequest) } + + if len(*ss.GuestAttribute) > 0 { + if !(strings.Contains(*ss.GuestAttribute, "=")) { + return NewAppError("Config.IsValid", "model.config.is_valid.saml_guest_attribute.app_error", nil, "", http.StatusBadRequest) + } + if len(strings.Split(*ss.GuestAttribute, "=")) != 2 { + return NewAppError("Config.IsValid", "model.config.is_valid.saml_guest_attribute.app_error", nil, "", http.StatusBadRequest) + } + } } return nil diff --git a/model/config_test.go b/model/config_test.go index a3ed8bc7ff..743b6a58ba 100644 --- a/model/config_test.go +++ b/model/config_test.go @@ -195,6 +195,21 @@ func TestConfigIsValidFakeAlgorithm(t *testing.T) { require.Equal(t, "model.config.is_valid.saml_signature_algorithm.app_error", err.Message) } +func TestConfigOverwriteGuestSettings(t *testing.T) { + const attribute = "FakeAttributeName" + c1 := Config{ + SamlSettings: SamlSettings{ + GuestAttribute: NewString(attribute), + }, + } + + c1.SetDefaults() + + if *c1.SamlSettings.GuestAttribute != attribute { + t.Fatal("SamlSettings.GuestAttribute should be overwritten") + } +} + func TestConfigDefaultServiceSettingsExperimentalGroupUnreadChannels(t *testing.T) { c1 := Config{} c1.SetDefaults() diff --git a/model/emoji_search_test.go b/model/emoji_search_test.go index 6e3b012138..14055b8178 100644 --- a/model/emoji_search_test.go +++ b/model/emoji_search_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestEmojiSearchJson(t *testing.T) { @@ -13,7 +15,5 @@ func TestEmojiSearchJson(t *testing.T) { json := emojiSearch.ToJson() remojiSearch := EmojiSearchFromJson(strings.NewReader(json)) - if emojiSearch.Term != remojiSearch.Term { - t.Fatal("Terms do not match") - } + require.Equal(t, emojiSearch.Term, remojiSearch.Term, "Terms do not match") } diff --git a/model/initial_load_test.go b/model/initial_load_test.go index ecae7ac881..980dcea11f 100644 --- a/model/initial_load_test.go +++ b/model/initial_load_test.go @@ -4,6 +4,7 @@ package model import ( + "github.com/stretchr/testify/require" "strings" "testing" ) @@ -14,7 +15,5 @@ func TestInitialLoadJson(t *testing.T) { json := o.ToJson() ro := InitialLoadFromJson(strings.NewReader(json)) - if o.User.Id != ro.User.Id { - t.Fatal("Ids do not match") - } + require.Equal(t, o.User.Id, ro.User.Id, "Ids do not match") } diff --git a/model/license_test.go b/model/license_test.go index c0a206e6c9..9ddf710e7a 100644 --- a/model/license_test.go +++ b/model/license_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/assert" ) func TestLicenseFeaturesToMap(t *testing.T) { @@ -102,27 +104,20 @@ func TestLicenseFeaturesSetDefaults(t *testing.T) { func TestLicenseIsExpired(t *testing.T) { l1 := License{} l1.ExpiresAt = GetMillis() - 1000 - if !l1.IsExpired() { - t.Fatal("license should be expired") - } + assert.True(t, l1.IsExpired()) l1.ExpiresAt = GetMillis() + 10000 - if l1.IsExpired() { - t.Fatal("license should not be expired") - } + assert.False(t, l1.IsExpired()) } func TestLicenseIsStarted(t *testing.T) { l1 := License{} l1.StartsAt = GetMillis() - 1000 - if !l1.IsStarted() { - t.Fatal("license should be started") - } + + assert.True(t, l1.IsStarted()) l1.StartsAt = GetMillis() + 10000 - if l1.IsStarted() { - t.Fatal("license should not be started") - } + assert.False(t, l1.IsStarted()) } func TestLicenseToFromJson(t *testing.T) { @@ -147,9 +142,7 @@ func TestLicenseToFromJson(t *testing.T) { j := l.ToJson() l1 := LicenseFromJson(strings.NewReader(j)) - if l1 == nil { - t.Fatalf("Decoding failed but should have passed.") - } + assert.NotNil(t, l1) CheckString(t, l1.Id, l.Id) CheckInt64(t, l1.IssuedAt, l.IssuedAt) @@ -184,9 +177,7 @@ func TestLicenseToFromJson(t *testing.T) { invalid := `{"asdf` l2 := LicenseFromJson(strings.NewReader(invalid)) - if l2 != nil { - t.Fatalf("Should have failed but didn't") - } + assert.Nil(t, l2) } func TestLicenseRecordIsValid(t *testing.T) { @@ -195,38 +186,31 @@ func TestLicenseRecordIsValid(t *testing.T) { Bytes: "asdfghjkl;", } - if err := lr.IsValid(); err == nil { - t.Fatalf("Should have been invalid") - } + err := lr.IsValid() + assert.NotNil(t, err) lr.Id = NewId() lr.CreateAt = 0 - if err := lr.IsValid(); err == nil { - t.Fatalf("Should have been invalid") - } + err = lr.IsValid() + assert.NotNil(t, err) lr.CreateAt = GetMillis() lr.Bytes = "" - if err := lr.IsValid(); err == nil { - t.Fatalf("Should have been invalid") - } + err = lr.IsValid() + assert.NotNil(t, err) lr.Bytes = strings.Repeat("0123456789", 1001) - if err := lr.IsValid(); err == nil { - t.Fatalf("Should have been invalid") - } + err = lr.IsValid() + assert.NotNil(t, err) lr.Bytes = "ASDFGHJKL;" - if err := lr.IsValid(); err != nil { - t.Fatal(err) - } + err = lr.IsValid() + assert.Nil(t, err) } func TestLicenseRecordPreSave(t *testing.T) { lr := LicenseRecord{} lr.PreSave() - if lr.CreateAt == 0 { - t.Fatal("CreateAt should not be zero") - } + assert.NotZero(t, lr.CreateAt) } diff --git a/model/modeltestlib_test.go b/model/modeltestlib_test.go index 2734b3eade..02e597665f 100644 --- a/model/modeltestlib_test.go +++ b/model/modeltestlib_test.go @@ -4,48 +4,31 @@ package model import ( - "runtime/debug" "testing" + + "github.com/stretchr/testify/assert" ) func CheckInt(t *testing.T, got int, expected int) { - if got != expected { - debug.PrintStack() - t.Fatalf("Got: %v, Expected: %v", got, expected) - } + assert.Equal(t, got, expected) } func CheckInt64(t *testing.T, got int64, expected int64) { - if got != expected { - debug.PrintStack() - t.Fatalf("Got: %v, Expected: %v", got, expected) - } + assert.Equal(t, got, expected) } func CheckString(t *testing.T, got string, expected string) { - if got != expected { - debug.PrintStack() - t.Fatalf("Got: %v, Expected: %v", got, expected) - } + assert.Equal(t, got, expected) } func CheckTrue(t *testing.T, test bool) { - if !test { - debug.PrintStack() - t.Fatal("Expected true") - } + assert.True(t, test) } func CheckFalse(t *testing.T, test bool) { - if test { - debug.PrintStack() - t.Fatal("Expected true") - } + assert.False(t, test) } func CheckBool(t *testing.T, got bool, expected bool) { - if got != expected { - debug.PrintStack() - t.Fatalf("Got: %v, Expected: %v", got, expected) - } + assert.Equal(t, got, expected) } diff --git a/model/oauth_test.go b/model/oauth_test.go index cbed8a6330..fb6d44ddd3 100644 --- a/model/oauth_test.go +++ b/model/oauth_test.go @@ -22,9 +22,7 @@ func TestOAuthAppJson(t *testing.T) { json := a1.ToJson() ra1 := OAuthAppFromJson(strings.NewReader(json)) - if a1.Id != ra1.Id { - t.Fatal("ids did not match") - } + require.Equal(t, a1.Id, ra1.Id, "ids did not match") } func TestOAuthAppPreSave(t *testing.T) { diff --git a/model/preference_test.go b/model/preference_test.go index d4035125a2..a619950e2d 100644 --- a/model/preference_test.go +++ b/model/preference_test.go @@ -57,9 +57,7 @@ func TestPreferencePreUpdate(t *testing.T) { preference.PreUpdate() var props map[string]string - if err := json.NewDecoder(strings.NewReader(preference.Value)).Decode(&props); err != nil { - t.Fatal(err) - } + require.Nil(t, json.NewDecoder(strings.NewReader(preference.Value)).Decode(&props)) require.Equal(t, "#ff0000", props["color"], "shouldn't have changed valid props") require.Equal(t, "#faf", props["color2"], "shouldn't have changed valid props") diff --git a/model/saml_test.go b/model/saml_test.go index d7745bd6bc..6fd05b20d6 100644 --- a/model/saml_test.go +++ b/model/saml_test.go @@ -6,6 +6,8 @@ package model import ( "strings" "testing" + + "github.com/stretchr/testify/require" ) func TestSamlCertificateStatusJson(t *testing.T) { @@ -13,12 +15,8 @@ func TestSamlCertificateStatusJson(t *testing.T) { json := status.ToJson() rstatus := SamlCertificateStatusFromJson(strings.NewReader(json)) - if status.IdpCertificateFile != rstatus.IdpCertificateFile { - t.Fatal("IdpCertificateFile do not match") - } + require.Equal(t, status.IdpCertificateFile, rstatus.IdpCertificateFile, "IdpCertificateFile do not match") rstatus = SamlCertificateStatusFromJson(strings.NewReader("junk")) - if rstatus != nil { - t.Fatal("should be nil") - } + require.Nil(t, rstatus, "should be nil") } diff --git a/model/search_params_test.go b/model/search_params_test.go index df9386a39b..056a5c9b68 100644 --- a/model/search_params_test.go +++ b/model/search_params_test.go @@ -12,77 +12,103 @@ import ( ) func TestSplitWords(t *testing.T) { - if words := splitWords(""); len(words) != 0 { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words := splitWords("") + require.Equal(t, 0, len(words)) - if words := splitWords(" "); len(words) != 0 { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords(" ") + require.Equal(t, 0, len(words)) - if words := splitWords("word"); len(words) != 1 || words[0] != "word" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("word") + require.Equal(t, 1, len(words)) + require.Equal(t, "word", words[0]) - if words := splitWords("wo\"rd"); len(words) != 2 || words[0] != "wo" || words[1] != "\"rd" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("wo\"rd") + require.Equal(t, 2, len(words)) + require.Equal(t, "wo", words[0]) + require.Equal(t, "\"rd", words[1]) - if words := splitWords("wo\"rd\""); len(words) != 2 || words[0] != "wo" || words[1] != "\"rd\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("wo\"rd\"") + require.Equal(t, 2, len(words)) + require.Equal(t, "wo", words[0]) + require.Equal(t, "\"rd\"", words[1]) - if words := splitWords("wo-\"rd\""); len(words) != 2 || words[0] != "wo" || words[1] != "-\"rd\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("wo-\"rd\"") + require.Equal(t, 2, len(words)) + require.Equal(t, "wo", words[0]) + require.Equal(t, "-\"rd\"", words[1]) - if words := splitWords("word1 word2 word3"); len(words) != 3 || words[0] != "word1" || words[1] != "word2" || words[2] != "word3" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("word1 word2 word3") + require.Equal(t, 3, len(words)) + require.Equal(t, "word1", words[0]) + require.Equal(t, "word2", words[1]) + require.Equal(t, "word3", words[2]) - if words := splitWords("word1 \"word2 word3"); len(words) != 3 || words[0] != "word1" || words[1] != "\"word2" || words[2] != "word3" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("word1 \"word2 word3") + require.Equal(t, 3, len(words)) + require.Equal(t, "word1", words[0]) + require.Equal(t, "\"word2", words[1]) + require.Equal(t, "word3", words[2]) - if words := splitWords("\"word1 word2 word3"); len(words) != 3 || words[0] != "\"word1" || words[1] != "word2" || words[2] != "word3" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("\"word1 word2 word3") + require.Equal(t, 3, len(words)) + require.Equal(t, "\"word1", words[0]) + require.Equal(t, "word2", words[1]) + require.Equal(t, "word3", words[2]) - if words := splitWords("word1 word2 word3\""); len(words) != 4 || words[0] != "word1" || words[1] != "word2" || words[2] != "word3" || words[3] != "\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("word1 word2 word3\"") + require.Equal(t, 4, len(words)) + require.Equal(t, "word1", words[0]) + require.Equal(t, "word2", words[1]) + require.Equal(t, "word3", words[2]) + require.Equal(t, "\"", words[3]) - if words := splitWords("word1 #word2 ##word3"); len(words) != 3 || words[0] != "word1" || words[1] != "#word2" || words[2] != "##word3" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("word1 #word2 ##word3") + require.Equal(t, 3, len(words)) + require.Equal(t, "word1", words[0]) + require.Equal(t, "#word2", words[1]) + require.Equal(t, "##word3", words[2]) - if words := splitWords(" word1 word2 word3 "); len(words) != 3 || words[0] != "word1" || words[1] != "word2" || words[2] != "word3" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords(" word1 word2 word3 ") + require.Equal(t, 3, len(words)) + require.Equal(t, "word1", words[0]) + require.Equal(t, "word2", words[1]) + require.Equal(t, "word3", words[2]) - if words := splitWords("\"quoted\""); len(words) != 1 || words[0] != "\"quoted\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("\"quoted\"") + require.Equal(t, 1, len(words)) + require.Equal(t, "\"quoted\"", words[0]) - if words := splitWords("-\"quoted\""); len(words) != 1 || words[0] != "-\"quoted\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("-\"quoted\"") + require.Equal(t, 1, len(words)) + require.Equal(t, "-\"quoted\"", words[0]) - if words := splitWords("\"quoted multiple words\""); len(words) != 1 || words[0] != "\"quoted multiple words\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("\"quoted multiple words\"") + require.Equal(t, 1, len(words)) + require.Equal(t, "\"quoted multiple words\"", words[0]) - if words := splitWords("some stuff \"quoted multiple words\" more stuff"); len(words) != 5 || words[0] != "some" || words[1] != "stuff" || words[2] != "\"quoted multiple words\"" || words[3] != "more" || words[4] != "stuff" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("some stuff \"quoted multiple words\" more stuff") + require.Equal(t, 5, len(words)) + require.Equal(t, "some", words[0]) + require.Equal(t, "stuff", words[1]) + require.Equal(t, "\"quoted multiple words\"", words[2]) + require.Equal(t, "more", words[3]) + require.Equal(t, "stuff", words[4]) - if words := splitWords("some stuff -\"quoted multiple words\" more stuff"); len(words) != 5 || words[0] != "some" || words[1] != "stuff" || words[2] != "-\"quoted multiple words\"" || words[3] != "more" || words[4] != "stuff" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("some stuff -\"quoted multiple words\" more stuff") + require.Equal(t, 5, len(words)) + require.Equal(t, "some", words[0]) + require.Equal(t, "stuff", words[1]) + require.Equal(t, "-\"quoted multiple words\"", words[2]) + require.Equal(t, "more", words[3]) + require.Equal(t, "stuff", words[4]) - if words := splitWords("some \"stuff\" \"quoted multiple words\" #some \"more stuff\""); len(words) != 5 || words[0] != "some" || words[1] != "\"stuff\"" || words[2] != "\"quoted multiple words\"" || words[3] != "#some" || words[4] != "\"more stuff\"" { - t.Fatalf("Incorrect output splitWords: %v", words) - } + words = splitWords("some \"stuff\" \"quoted multiple words\" #some \"more stuff\"") + require.Equal(t, 5, len(words)) + require.Equal(t, "some", words[0]) + require.Equal(t, "\"stuff\"", words[1]) + require.Equal(t, "\"quoted multiple words\"", words[2]) + require.Equal(t, "#some", words[3]) + require.Equal(t, "\"more stuff\"", words[4]) } func TestParseSearchFlags(t *testing.T) { diff --git a/model/user_access_token_search_test.go b/model/user_access_token_search_test.go index 15a53f5368..0d1ea57535 100644 --- a/model/user_access_token_search_test.go +++ b/model/user_access_token_search_test.go @@ -4,6 +4,7 @@ package model import ( + "github.com/stretchr/testify/require" "strings" "testing" ) @@ -12,8 +13,5 @@ func TestUserAccessTokenSearchJson(t *testing.T) { userAccessTokenSearch := UserAccessTokenSearch{Term: NewId()} json := userAccessTokenSearch.ToJson() ruserAccessTokenSearch := UserAccessTokenSearchFromJson(strings.NewReader(json)) - - if userAccessTokenSearch.Term != ruserAccessTokenSearch.Term { - t.Fatal("Terms do not match") - } + require.Equal(t, userAccessTokenSearch.Term, ruserAccessTokenSearch.Term, "Terms do not match") } diff --git a/model/user_search_test.go b/model/user_search_test.go index 89c8b68f7f..dc0dfae2d6 100644 --- a/model/user_search_test.go +++ b/model/user_search_test.go @@ -6,6 +6,8 @@ package model import ( "bytes" "testing" + + "github.com/stretchr/testify/assert" ) func TestUserSearchJson(t *testing.T) { @@ -13,7 +15,5 @@ func TestUserSearchJson(t *testing.T) { json := userSearch.ToJson() ruserSearch := UserSearchFromJson(bytes.NewReader(json)) - if userSearch.Term != ruserSearch.Term { - t.Fatal("Terms do not match") - } + assert.Equal(t, userSearch.Term, ruserSearch.Term, "Terms do not match") } diff --git a/model/user_terms_of_Service_test.go b/model/user_terms_of_Service_test.go index 801888e971..242325a74c 100644 --- a/model/user_terms_of_Service_test.go +++ b/model/user_terms_of_Service_test.go @@ -8,29 +8,21 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestUserTermsOfServiceIsValid(t *testing.T) { s := UserTermsOfService{} - - if err := s.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.Error(t, s.IsValid(), "should be invalid") s.UserId = NewId() - if err := s.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.Error(t, s.IsValid(), "should be invalid") s.TermsOfServiceId = NewId() - if err := s.IsValid(); err == nil { - t.Fatal("should be invalid") - } + require.Error(t, s.IsValid(), "should be invalid") s.CreateAt = GetMillis() - if err := s.IsValid(); err != nil { - t.Fatal("should be valid") - } + require.Nil(t, s.IsValid(), "should be valid") } func TestUserTermsOfServiceJson(t *testing.T) { diff --git a/model/user_test.go b/model/user_test.go index 95cb03a493..d3d64da78b 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -16,13 +16,8 @@ import ( func TestPasswordHash(t *testing.T) { hash := HashPassword("Test") - if !ComparePassword(hash, "Test") { - t.Fatal("Passwords don't match") - } - - if ComparePassword(hash, "Test2") { - t.Fatal("Passwords should not have matched") - } + assert.True(t, ComparePassword(hash, "Test"), "Passwords don't match") + assert.False(t, ComparePassword(hash, "Test2"), "Passwords should not have matched") } func TestUserDeepCopy(t *testing.T) { @@ -60,22 +55,15 @@ func TestUserJson(t *testing.T) { json := user.ToJson() ruser := UserFromJson(strings.NewReader(json)) - if user.Id != ruser.Id { - t.Fatal("Ids do not match") - } + assert.Equal(t, user.Id, ruser.Id, "Ids do not match") } func TestUserPreSave(t *testing.T) { user := User{Password: "test"} user.PreSave() user.Etag(true, true) - if user.Timezone == nil { - t.Fatal("Timezone is nil") - } - - if user.Timezone["useAutomaticTimezone"] != "true" { - t.Fatal("Timezone is not set to default") - } + assert.NotNil(t, user.Timezone, "Timezone is nil") + assert.Equal(t, user.Timezone["useAutomaticTimezone"], "true", "Timezone is not set to default"); } func TestUserPreUpdate(t *testing.T) { @@ -86,39 +74,28 @@ func TestUserPreUpdate(t *testing.T) { func TestUserUpdateMentionKeysFromUsername(t *testing.T) { user := User{Username: "user"} user.SetDefaultNotifications() - - if user.NotifyProps["mention_keys"] != "user,@user" { - t.Fatalf("default mention keys are invalid: %v", user.NotifyProps["mention_keys"]) - } + assert.Equalf(t, user.NotifyProps["mention_keys"], "user,@user", "default mention keys are invalid: %v", user.NotifyProps["mention_keys"]) user.Username = "person" user.UpdateMentionKeysFromUsername("user") - if user.NotifyProps["mention_keys"] != "person,@person" { - t.Fatalf("mention keys are invalid after changing username: %v", user.NotifyProps["mention_keys"]) - } + assert.Equalf(t, user.NotifyProps["mention_keys"], "person,@person", "mention keys are invalid after changing username: %v", user.NotifyProps["mention_keys"]) user.NotifyProps["mention_keys"] += ",mention" user.UpdateMentionKeysFromUsername("person") - if user.NotifyProps["mention_keys"] != "person,@person,mention" { - t.Fatalf("mention keys are invalid after adding extra mention keyword: %v", user.NotifyProps["mention_keys"]) - } + assert.Equalf(t, user.NotifyProps["mention_keys"], "person,@person,mention", "mention keys are invalid after adding extra mention keyword: %v", user.NotifyProps["mention_keys"]) user.Username = "user" user.UpdateMentionKeysFromUsername("person") - if user.NotifyProps["mention_keys"] != "user,@user,mention" { - t.Fatalf("mention keys are invalid after changing username with extra mention keyword: %v", user.NotifyProps["mention_keys"]) - } + assert.Equalf(t, user.NotifyProps["mention_keys"], "user,@user,mention", "mention keys are invalid after changing username with extra mention keyword: %v", user.NotifyProps["mention_keys"]) } func TestUserIsValid(t *testing.T) { user := User{} - - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "id", "") { - t.Fatal(err) - } + err := user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "id", ""), "expected user is valid error: %s", err.Error()) user.Id = NewId() - err := user.IsValid() + err = user.IsValid() require.True(t, HasExpectedUserIsValidError(err, "create_at", user.Id), "expected user is valid error: %s", err.Error()) user.CreateAt = GetMillis() @@ -148,37 +125,28 @@ func TestUserIsValid(t *testing.T) { require.True(t, HasExpectedUserIsValidError(err, "nickname", user.Id), "expected user is valid error: %s", err.Error()) user.Nickname = strings.Repeat("a", 64) - if err := user.IsValid(); err != nil { - t.Fatal(err) - } + require.Error(t, user.IsValid()) user.FirstName = "" user.LastName = "" - if err := user.IsValid(); err != nil { - t.Fatal(err) - } + require.Error(t, user.IsValid()) user.FirstName = strings.Repeat("a", 65) - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "first_name", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "first_name", user.Id), "expected user is valid error: %s", err.Error()) user.FirstName = strings.Repeat("a", 64) user.LastName = strings.Repeat("a", 65) - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "last_name", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "last_name", user.Id), "expected user is valid error: %s", err.Error()) user.LastName = strings.Repeat("a", 64) user.Position = strings.Repeat("a", 128) - if err := user.IsValid(); err != nil { - t.Fatal(err) - } + require.Error(t, user.IsValid()) user.Position = strings.Repeat("a", 129) - if err := user.IsValid(); !HasExpectedUserIsValidError(err, "position", user.Id) { - t.Fatal(err) - } + err = user.IsValid() + require.True(t, HasExpectedUserIsValidError(err, "position", user.Id), "expected user is valid error: %s", err.Error()) } func HasExpectedUserIsValidError(err *AppError, fieldName string, userId string) bool { @@ -194,98 +162,53 @@ func HasExpectedUserIsValidError(err *AppError, fieldName string, userId string) func TestUserGetFullName(t *testing.T) { user := User{} - - if fullName := user.GetFullName(); fullName != "" { - t.Fatal("Full name should be blank") - } + assert.Equal(t, user.GetFullName(), "", "Full name should be blank") user.FirstName = "first" - if fullName := user.GetFullName(); fullName != "first" { - t.Fatal("Full name should be first name") - } + assert.Equal(t, user.GetFullName(), "first", "Full name should be first name") user.FirstName = "" user.LastName = "last" - if fullName := user.GetFullName(); fullName != "last" { - t.Fatal("Full name should be last name") - } + assert.Equal(t, user.GetFullName(), "last", "Full name should be last name") user.FirstName = "first" - if fullName := user.GetFullName(); fullName != "first last" { - t.Fatal("Full name should be first name and last name") - } + assert.Equal(t, user.GetFullName(), "first last", "Full name should be first name and last name") } func TestUserGetDisplayName(t *testing.T) { user := User{Username: "username"} - if displayName := user.GetDisplayName(SHOW_FULLNAME); displayName != "username" { - t.Fatal("Display name should be username") - } - - if displayName := user.GetDisplayName(SHOW_NICKNAME_FULLNAME); displayName != "username" { - t.Fatal("Display name should be username") - } - - if displayName := user.GetDisplayName(SHOW_USERNAME); displayName != "username" { - t.Fatal("Display name should be username") - } + assert.Equal(t, user.GetDisplayName(SHOW_FULLNAME), "username", "Display name should be username") + assert.Equal(t, user.GetDisplayName(SHOW_NICKNAME_FULLNAME), "username", "Display name should be username") + assert.Equal(t, user.GetDisplayName(SHOW_USERNAME), "username", "Display name should be username") user.FirstName = "first" user.LastName = "last" - if displayName := user.GetDisplayName(SHOW_FULLNAME); displayName != "first last" { - t.Fatal("Display name should be full name") - } - - if displayName := user.GetDisplayName(SHOW_NICKNAME_FULLNAME); displayName != "first last" { - t.Fatal("Display name should be full name since there is no nickname") - } - - if displayName := user.GetDisplayName(SHOW_USERNAME); displayName != "username" { - t.Fatal("Display name should be username") - } + assert.Equal(t, user.GetDisplayName(SHOW_FULLNAME), "first last", "Display name should be full name") + assert.Equal(t, user.GetDisplayName(SHOW_NICKNAME_FULLNAME), "first last", "Display name should be full name since there is no nickname") + assert.Equal(t, user.GetDisplayName(SHOW_USERNAME), "username", "Display name should be username") user.Nickname = "nickname" - if displayName := user.GetDisplayName(SHOW_NICKNAME_FULLNAME); displayName != "nickname" { - t.Fatal("Display name should be nickname") - } + assert.Equal(t, user.GetDisplayName(SHOW_NICKNAME_FULLNAME), "nickname", "Display name should be nickname") } func TestUserGetDisplayNameWithPrefix(t *testing.T) { user := User{Username: "username"} - if displayName := user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"); displayName != "@username" { - t.Fatal("Display name should be username") - } - - if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "@username" { - t.Fatal("Display name should be username") - } - - if displayName := user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"); displayName != "@username" { - t.Fatal("Display name should be username") - } + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"), "@username", "Display name should be username") + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"), "@username", "Display name should be username") + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"), "@username", "Display name should be username") user.FirstName = "first" user.LastName = "last" - if displayName := user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"); displayName != "first last" { - t.Fatal("Display name should be full name") - } - - if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "first last" { - t.Fatal("Display name should be full name since there is no nickname") - } - - if displayName := user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"); displayName != "@username" { - t.Fatal("Display name should be username") - } + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_FULLNAME, "@"), "first last", "Display name should be full name") + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"), "first last", "Display name should be full name since there is no nickname") + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_USERNAME, "@"), "@username", "Display name should be username") user.Nickname = "nickname" - if displayName := user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"); displayName != "nickname" { - t.Fatal("Display name should be nickname") - } + assert.Equal(t, user.GetDisplayNameWithPrefix(SHOW_NICKNAME_FULLNAME, "@"), "nickname", "Display name should be nickname") } var usernames = []struct { @@ -319,45 +242,23 @@ func TestValidUsername(t *testing.T) { } func TestNormalizeUsername(t *testing.T) { - if NormalizeUsername("Spin-punch") != "spin-punch" { - t.Fatal("didn't normalize username properly") - } - if NormalizeUsername("PUNCH") != "punch" { - t.Fatal("didn't normalize username properly") - } - if NormalizeUsername("spin") != "spin" { - t.Fatal("didn't normalize username properly") - } + assert.Equal(t, NormalizeUsername("Spin-punch"), "spin-punch", "didn't normalize username properly") + assert.Equal(t, NormalizeUsername("PUNCH"), "punch", "didn't normalize username properly") + assert.Equal(t, NormalizeUsername("spin"), "spin", "didn't normalize username properly") } func TestNormalizeEmail(t *testing.T) { - if NormalizeEmail("TEST@EXAMPLE.COM") != "test@example.com" { - t.Fatal("didn't normalize email properly") - } - if NormalizeEmail("TEST2@example.com") != "test2@example.com" { - t.Fatal("didn't normalize email properly") - } - if NormalizeEmail("test3@example.com") != "test3@example.com" { - t.Fatal("didn't normalize email properly") - } + assert.Equal(t, NormalizeEmail("TEST@EXAMPLE.COM"), "test@example.com", "didn't normalize email properly") + assert.Equal(t, NormalizeEmail("TEST2@example.com"), "test2@example.com", "didn't normalize email properly") + assert.Equal(t, NormalizeEmail("test3@example.com"), "test3@example.com", "didn't normalize email properly") } func TestCleanUsername(t *testing.T) { - if CleanUsername("Spin-punch") != "spin-punch" { - t.Fatal("didn't clean name properly") - } - if CleanUsername("PUNCH") != "punch" { - t.Fatal("didn't clean name properly") - } - if CleanUsername("spin'punch") != "spin-punch" { - t.Fatal("didn't clean name properly") - } - if CleanUsername("spin") != "spin" { - t.Fatal("didn't clean name properly") - } - if len(CleanUsername("all")) != 27 { - t.Fatal("didn't clean name properly") - } + assert.Equal(t, CleanUsername("Spin-punch"), "spin-punch", "didn't clean name properly") + assert.Equal(t, CleanUsername("PUNCH"), "punch", "didn't clean name properly") + assert.Equal(t, CleanUsername("spin'punch"), "spin-punch", "didn't clean name properly") + assert.Equal(t, CleanUsername("spin"), "spin", "didn't clean name properly") + assert.Equal(t, len(CleanUsername("all")), 27, "didn't clean name properly") } func TestRoles(t *testing.T) { diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index 4c8698cdbd..1f77730da4 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -282,7 +282,7 @@ func (s *SqlPostStore) Get(id string, skipFetchThreads bool) (*model.PostList, * var post model.Post var postFetchQuery string if skipFetchThreads { - postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.RootId) FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0" + postFetchQuery = "SELECT p.*, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = p.Id) as ReplyCount FROM Posts p WHERE p.Id = :Id AND p.DeleteAt = 0" } else { postFetchQuery = "SELECT * FROM Posts WHERE Id = :Id AND DeleteAt = 0" } @@ -541,20 +541,28 @@ func (s *SqlPostStore) GetPostsSince(options model.GetPostsSinceOptions, allowFr } var posts []*model.Post + + replyCountQuery1 := "" + replyCountQuery2 := "" + if options.SkipFetchThreads { + replyCountQuery1 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p1.RootId = '' AND Posts.RootId = p1.Id) as ReplyCount` + replyCountQuery2 = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE p2.RootId = '' AND Posts.RootId = p2.Id) as ReplyCount` + } + _, err := s.GetReplica().Select(&posts, `(SELECT - * + *`+replyCountQuery1+` FROM - Posts + Posts p1 WHERE (UpdateAt > :Time AND ChannelId = :ChannelId) LIMIT 1000) UNION (SELECT - * + *`+replyCountQuery2+` FROM - Posts + Posts p2 WHERE Id IN @@ -647,15 +655,18 @@ func (s *SqlPostStore) getPostsAround(before bool, options model.GetPostsOptions } } rootQuery := s.getQueryBuilder().Select("p.*") + idQuery := sq.Or{ + sq.Eq{"Id": rootIds}, + } if options.SkipFetchThreads { rootQuery = rootQuery.Column(sq.Alias(replyCountSubQuery, "ReplyCount")) + } else { + idQuery = append(idQuery, sq.Eq{"RootId": rootIds}) // preserve original behaviour } + rootQuery = rootQuery.From("Posts p"). Where(sq.And{ - sq.Or{ - sq.Eq{"RootId": rootIds}, - sq.Eq{"Id": rootIds}, - }, + idQuery, sq.Eq{"ChannelId": options.ChannelId}, sq.Eq{"DeleteAt": 0}, }). @@ -785,8 +796,11 @@ func (s *SqlPostStore) getRootPosts(channelId string, offset int, limit int, ski func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int, skipFetchThreads bool) ([]*model.Post, *model.AppError) { var posts []*model.Post replyCountQuery := "" + onStatement := "q1.RootId = q2.Id" if skipFetchThreads { replyCountQuery = ` ,(SELECT COUNT(Posts.Id) FROM Posts WHERE q2.RootId = '' AND Posts.RootId = q2.Id) as ReplyCount` + } else { + onStatement += " OR q1.RootId = q2.RootId" } _, err := s.GetReplica().Select(&posts, `SELECT q2.*`+replyCountQuery+` @@ -806,7 +820,7 @@ func (s *SqlPostStore) getParentsPosts(channelId string, offset int, limit int, ORDER BY CreateAt DESC LIMIT :Limit OFFSET :Offset) q3 WHERE q3.RootId != '') q1 - ON q1.RootId = q2.Id OR q1.RootId = q2.RootId + ON `+onStatement+` WHERE ChannelId = :ChannelId2 AND DeleteAt = 0 diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index ad18de2f53..21162536e6 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -1008,8 +1008,6 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) { post1.Id: post1, post2.Id: post2, post3.Id: post3, - post4.Id: post4, - post6.Id: post6, }, postList.Posts) }) @@ -1031,7 +1029,6 @@ func testPostStoreGetPostsBeforeAfter(t *testing.T, ss store.Store) { assert.Equal(t, []string{post6.Id, post5.Id}, postList.Order) assert.Equal(t, map[string]*model.Post{ post2.Id: post2, - post4.Id: post4, post5.Id: post5, post6.Id: post6, }, postList.Posts) diff --git a/store/storetest/system_store.go b/store/storetest/system_store.go index 02964fd046..1a18111c81 100644 --- a/store/storetest/system_store.go +++ b/store/storetest/system_store.go @@ -42,15 +42,13 @@ func testSystemStore(t *testing.T, ss store.Store) { func testSystemStoreSaveOrUpdate(t *testing.T, ss store.Store) { system := &model.System{Name: model.NewId(), Value: "value"} - if err := ss.System().SaveOrUpdate(system); err != nil { - t.Fatal(err) - } + err := ss.System().SaveOrUpdate(system) + require.Nil(t, err) system.Value = "value2" - if err := ss.System().SaveOrUpdate(system); err != nil { - t.Fatal(err) - } + err = ss.System().SaveOrUpdate(system) + require.Nil(t, err) } func testSystemStorePermanentDeleteByName(t *testing.T, ss store.Store) {