From 9730b46bca553d07e65350f2d60cf20685cae9ed Mon Sep 17 00:00:00 2001 From: George Goldberg Date: Wed, 28 Nov 2018 15:28:51 +0000 Subject: [PATCH] MM-13190: Fix NotifyProps in Bulk Import. (#9900) If some, but not all, notify props are specified for a user in the bulk import data, and that is a newly created user, we must explicitly initialise all the other notify props to their default values to avoid breaking client assumptions. --- app/import_functions.go | 1 + app/import_functions_test.go | 43 +++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/app/import_functions.go b/app/import_functions.go index 6afa58acd7..88a4c1d189 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -279,6 +279,7 @@ func (a *App) ImportUser(data *UserImportData, dryRun bool) *model.AppError { } else { user = &model.User{} user.MakeNonNil() + user.SetDefaultNotifications() hasUserChanged = true } diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 5e950d7d5f..09c053ef95 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -8,10 +8,12 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/store" "github.com/mattermost/mattermost-server/utils" - "github.com/stretchr/testify/assert" ) func TestImportImportScheme(t *testing.T) { @@ -1341,6 +1343,45 @@ func TestImportImportUser(t *testing.T) { } +func TestImportUserDefaultNotifyProps(t *testing.T) { + th := Setup() + defer th.TearDown() + + // Create a valid new user with some, but not all, notify props populated. + username := model.NewId() + data := UserImportData{ + Username: &username, + Email: ptrStr(model.NewId() + "@example.com"), + NotifyProps: &UserNotifyPropsImportData{ + Email: ptrStr("false"), + }, + } + + require.Nil(t, th.App.ImportUser(&data, false)) + + user, err := th.App.GetUserByUsername(username) + require.Nil(t, err) + + // Check the value of the notify prop we specified explicitly in the import data. + val, ok := user.NotifyProps[model.EMAIL_NOTIFY_PROP] + assert.True(t, ok) + assert.Equal(t, "false", val) + + // Check all the other notify props are set to their default values. + comparisonUser := model.User{} + comparisonUser.SetDefaultNotifications() + + for key, expectedValue := range comparisonUser.NotifyProps { + if key == model.EMAIL_NOTIFY_PROP { + continue + } + + val, ok := user.NotifyProps[key] + assert.True(t, ok) + assert.Equal(t, expectedValue, val) + } +} + func TestImportImportPost(t *testing.T) { th := Setup() defer th.TearDown()