diff --git a/app/import_functions.go b/app/import_functions.go index ee684980f4..cf1925330e 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1813,8 +1813,13 @@ func (a *App) importMultipleDirectPostLines(c *request.Context, lines []LineImpo } func (a *App) importEmoji(data *EmojiImportData, dryRun bool) *model.AppError { - if err := validateEmojiImportData(data); err != nil { - return err + aerr := validateEmojiImportData(data) + if aerr != nil { + if aerr.Id == "model.emoji.system_emoji_name.app_error" { + mlog.Warn("Skipping emoji import due to name conflict with system emoji", mlog.String("emoji_name", *data.Name)) + return nil + } + return aerr } // If this is a Dry Run, do not continue any further. diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 948fe84e83..e8503d0eb9 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -4075,6 +4075,10 @@ func TestImportImportEmoji(t *testing.T) { err = th.App.importEmoji(&data, false) assert.Nil(t, err, "Second run should have succeeded apply mode") + + data = EmojiImportData{Name: ptrStr("smiley"), Image: ptrStr(testImage)} + err = th.App.importEmoji(&data, false) + assert.Nil(t, err, "System emoji should not fail") } func TestImportAttachment(t *testing.T) { diff --git a/app/import_validators.go b/app/import_validators.go index 9cc37cf265..927776eac5 100644 --- a/app/import_validators.go +++ b/app/import_validators.go @@ -560,6 +560,8 @@ func validateDirectPostImportData(data *DirectPostImportData, maxPostSize int) * return nil } +// validateEmojiImportData validates emoji data and returns if the import name +// conflicts with a system emoji. func validateEmojiImportData(data *EmojiImportData) *model.AppError { if data == nil { return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.empty.error", nil, "", http.StatusBadRequest) @@ -569,14 +571,14 @@ func validateEmojiImportData(data *EmojiImportData) *model.AppError { return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.name_missing.error", nil, "", http.StatusBadRequest) } - if err := model.IsValidEmojiName(*data.Name); err != nil { - return err - } - if data.Image == nil || *data.Image == "" { return model.NewAppError("BulkImport", "app.import.validate_emoji_import_data.image_missing.error", nil, "", http.StatusBadRequest) } + if err := model.IsValidEmojiName(*data.Name); err != nil { + return err + } + return nil } diff --git a/app/import_validators_test.go b/app/import_validators_test.go index e2340fd66f..52f32d73e1 100644 --- a/app/import_validators_test.go +++ b/app/import_validators_test.go @@ -1343,34 +1343,37 @@ func TestImportValidateDirectPostImportData(t *testing.T) { } func TestImportValidateEmojiImportData(t *testing.T) { - data := EmojiImportData{ - Name: ptrStr("parrot2"), - Image: ptrStr("/path/to/image"), + var testCases = []struct { + testName string + name *string + image *string + expectError bool + expectSystemEmoji bool + }{ + {"success", ptrStr("parrot2"), ptrStr("/path/to/image"), false, false}, + {"system emoji", ptrStr("smiley"), ptrStr("/path/to/image"), true, true}, + {"empty name", ptrStr(""), ptrStr("/path/to/image"), true, false}, + {"empty image", ptrStr("parrot2"), ptrStr(""), true, false}, + {"empty name and image", ptrStr(""), ptrStr(""), true, false}, + {"nil name", nil, ptrStr("/path/to/image"), true, false}, + {"nil image", ptrStr("parrot2"), nil, true, false}, + {"nil name and image", nil, nil, true, false}, } - err := validateEmojiImportData(&data) - assert.Nil(t, err, "Validation should succeed") + for _, tc := range testCases { + t.Run(tc.testName, func(t *testing.T) { + data := EmojiImportData{ + Name: tc.name, + Image: tc.image, + } - *data.Name = "smiley" - err = validateEmojiImportData(&data) - assert.NotNil(t, err) - - *data.Name = "" - err = validateEmojiImportData(&data) - assert.NotNil(t, err) - - *data.Name = "" - *data.Image = "" - err = validateEmojiImportData(&data) - assert.NotNil(t, err) - - *data.Image = "/path/to/image" - data.Name = nil - err = validateEmojiImportData(&data) - assert.NotNil(t, err) - - data.Name = ptrStr("parrot") - data.Image = nil - err = validateEmojiImportData(&data) - assert.NotNil(t, err) + err := validateEmojiImportData(&data) + if tc.expectError { + require.NotNil(t, err) + assert.Equal(t, tc.expectSystemEmoji, err.Id == "model.emoji.system_emoji_name.app_error") + } else { + assert.Nil(t, err) + } + }) + } } diff --git a/i18n/en.json b/i18n/en.json index f30e7a727e..e8641fc4cc 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -8363,6 +8363,10 @@ "id": "model.emoji.name.app_error", "translation": "Name must be 1 to 64 lowercase alphanumeric characters." }, + { + "id": "model.emoji.system_emoji_name.app_error", + "translation": "Name conflicts with existing system emoji name." + }, { "id": "model.emoji.update_at.app_error", "translation": "Update at must be a valid time." diff --git a/model/emoji.go b/model/emoji.go index fd0e8ab369..4b30ee4364 100644 --- a/model/emoji.go +++ b/model/emoji.go @@ -78,9 +78,12 @@ func (emoji *Emoji) IsValid() *AppError { } func IsValidEmojiName(name string) *AppError { - if name == "" || len(name) > EmojiNameMaxLength || !IsValidAlphaNumHyphenUnderscorePlus(name) || inSystemEmoji(name) { + if name == "" || len(name) > EmojiNameMaxLength || !IsValidAlphaNumHyphenUnderscorePlus(name) { return NewAppError("Emoji.IsValid", "model.emoji.name.app_error", nil, "", http.StatusBadRequest) } + if inSystemEmoji(name) { + return NewAppError("Emoji.IsValid", "model.emoji.system_emoji_name.app_error", nil, "", http.StatusBadRequest) + } return nil }