[MM-44954] Regenerate default avatar (#22871)

* Regenerate default profile picture if username has changed

- Only actions is profile picture has not been changed
- Adjusts ResetLastPictureUpdate store function to store
-curTime instead of 0
    - This is to support updating the default picture while still
    retaining the ability to discern a default image from a set one.
- Changes SetDefaultProfileImage to leverage UpdateDefaultProfileImage
- Test updates around updating user default profile pictures

* App interface updates

* Only display picture update date if non-negative

- Ensures we don't display negative timestamps (default images)
- Change ported for mono-repo changes

* Remove duplicate test assertion

---------

Co-authored-by: Nathan Geist <ngeist@spiria.com>
Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Nathan 2023-05-04 08:14:26 -06:00 committed by GitHub
parent 349e5d4573
commit 670b0e4c9f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 105 additions and 18 deletions

View File

@ -3664,9 +3664,16 @@ func TestSetDefaultProfileImage(t *testing.T) {
defer th.TearDown() defer th.TearDown()
user := th.BasicUser user := th.BasicUser
startTime := model.GetMillis()
time.Sleep(time.Millisecond)
_, err := th.Client.SetDefaultProfileImage(user.Id) _, err := th.Client.SetDefaultProfileImage(user.Id)
require.NoError(t, err) require.NoError(t, err)
iuser, getUserErr := th.App.GetUser(user.Id)
require.Nil(t, getUserErr)
assert.Less(t, iuser.LastPictureUpdate, -startTime, "LastPictureUpdate should be set to -(current time in milliseconds)")
resp, err := th.Client.SetDefaultProfileImage(model.NewId()) resp, err := th.Client.SetDefaultProfileImage(model.NewId())
require.Error(t, err) require.Error(t, err)
CheckForbiddenStatus(t, resp) CheckForbiddenStatus(t, resp)
@ -3684,12 +3691,14 @@ func TestSetDefaultProfileImage(t *testing.T) {
require.Fail(t, "Should have failed either forbidden or unauthorized") require.Fail(t, "Should have failed either forbidden or unauthorized")
} }
time.Sleep(time.Millisecond)
_, err = th.SystemAdminClient.SetDefaultProfileImage(user.Id) _, err = th.SystemAdminClient.SetDefaultProfileImage(user.Id)
require.NoError(t, err) require.NoError(t, err)
ruser, appErr := th.App.GetUser(user.Id) ruser, appErr := th.App.GetUser(user.Id)
require.Nil(t, appErr) require.Nil(t, appErr)
assert.Equal(t, int64(0), ruser.LastPictureUpdate, "Picture should have reset to default") assert.Less(t, ruser.LastPictureUpdate, iuser.LastPictureUpdate, "LastPictureUpdate should be updated to a lower negative number")
info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"} info := &model.FileInfo{Path: "users/" + user.Id + "/profile.png"}
err = th.cleanupTestFile(info) err = th.cleanupTestFile(info)

View File

@ -1120,6 +1120,7 @@ type AppIface interface {
UpdateChannelPrivacy(c request.CTX, oldChannel *model.Channel, user *model.User) (*model.Channel, *model.AppError) UpdateChannelPrivacy(c request.CTX, oldChannel *model.Channel, user *model.User) (*model.Channel, *model.AppError)
UpdateCommand(oldCmd, updatedCmd *model.Command) (*model.Command, *model.AppError) UpdateCommand(oldCmd, updatedCmd *model.Command) (*model.Command, *model.AppError)
UpdateConfig(f func(*model.Config)) UpdateConfig(f func(*model.Config))
UpdateDefaultProfileImage(c request.CTX, user *model.User) *model.AppError
UpdateEphemeralPost(c request.CTX, userID string, post *model.Post) *model.Post UpdateEphemeralPost(c request.CTX, userID string, post *model.Post) *model.Post
UpdateExpiredDNDStatuses() ([]*model.Status, error) UpdateExpiredDNDStatuses() ([]*model.Status, error)
UpdateGroup(group *model.Group) (*model.Group, *model.AppError) UpdateGroup(group *model.Group) (*model.Group, *model.AppError)

View File

@ -17376,6 +17376,28 @@ func (a *OpenTracingAppLayer) UpdateDNDStatusOfUsers() {
a.app.UpdateDNDStatusOfUsers() a.app.UpdateDNDStatusOfUsers()
} }
func (a *OpenTracingAppLayer) UpdateDefaultProfileImage(c request.CTX, user *model.User) *model.AppError {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpdateDefaultProfileImage")
a.ctx = newCtx
a.app.Srv().Store().SetContext(newCtx)
defer func() {
a.app.Srv().Store().SetContext(origCtx)
a.ctx = origCtx
}()
defer span.Finish()
resultVar0 := a.app.UpdateDefaultProfileImage(c, user)
if resultVar0 != nil {
span.LogFields(spanlog.Error(resultVar0))
ext.Error.Set(span, true)
}
return resultVar0
}
func (a *OpenTracingAppLayer) UpdateEphemeralPost(c request.CTX, userID string, post *model.Post) *model.Post { func (a *OpenTracingAppLayer) UpdateEphemeralPost(c request.CTX, userID string, post *model.Post) *model.Post {
origCtx := a.ctx origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpdateEphemeralPost") span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.UpdateEphemeralPost")

View File

@ -792,7 +792,7 @@ func (a *App) GetDefaultProfileImage(user *model.User) ([]byte, *model.AppError)
return a.ch.srv.GetDefaultProfileImage(user) return a.ch.srv.GetDefaultProfileImage(user)
} }
func (a *App) SetDefaultProfileImage(c request.CTX, user *model.User) *model.AppError { func (a *App) UpdateDefaultProfileImage(c request.CTX, user *model.User) *model.AppError {
img, appErr := a.GetDefaultProfileImage(user) img, appErr := a.GetDefaultProfileImage(user)
if appErr != nil { if appErr != nil {
return appErr return appErr
@ -809,6 +809,16 @@ func (a *App) SetDefaultProfileImage(c request.CTX, user *model.User) *model.App
a.InvalidateCacheForUser(user.Id) a.InvalidateCacheForUser(user.Id)
return nil
}
func (a *App) SetDefaultProfileImage(c request.CTX, user *model.User) *model.AppError {
if err := a.UpdateDefaultProfileImage(c, user); err != nil {
c.Logger().Error("Failed to update default profile image for user", mlog.String("user_id", user.Id), mlog.Err(err))
return err
}
updatedUser, appErr := a.GetUser(user.Id) updatedUser, appErr := a.GetUser(user.Id)
if appErr != nil { if appErr != nil {
c.Logger().Warn("Error in getting users profile forcing logout", mlog.String("user_id", user.Id), mlog.Err(appErr)) c.Logger().Warn("Error in getting users profile forcing logout", mlog.String("user_id", user.Id), mlog.Err(appErr))
@ -1228,37 +1238,53 @@ func (a *App) UpdateUser(c request.CTX, user *model.User, sendNotifications bool
} }
} }
newUser := userUpdate.New
if (newUser.Username != userUpdate.Old.Username) && (newUser.LastPictureUpdate <= 0) {
// When a username is updated and the profile is still using a default profile picture, generate a new one based on their username
if err := a.UpdateDefaultProfileImage(c, newUser); err != nil {
c.Logger().Warn("Error with updating default profile image", mlog.Err(err))
}
tempUser, getUserErr := a.GetUser(user.Id)
if getUserErr != nil {
c.Logger().Warn("Error when retrieving user after profile picture update, avatar may fail to update automatically on client applications.", mlog.Err(getUserErr))
} else {
newUser = tempUser
}
}
if sendNotifications { if sendNotifications {
if userUpdate.New.Email != userUpdate.Old.Email || newEmail != "" { if newUser.Email != userUpdate.Old.Email || newEmail != "" {
if *a.Config().EmailSettings.RequireEmailVerification { if *a.Config().EmailSettings.RequireEmailVerification {
a.Srv().Go(func() { a.Srv().Go(func() {
if err := a.SendEmailVerification(userUpdate.New, newEmail, ""); err != nil { if err := a.SendEmailVerification(newUser, newEmail, ""); err != nil {
c.Logger().Error("Failed to send email verification", mlog.Err(err)) c.Logger().Error("Failed to send email verification", mlog.Err(err))
} }
}) })
} else { } else {
a.Srv().Go(func() { a.Srv().Go(func() {
if err := a.Srv().EmailService.SendEmailChangeEmail(userUpdate.Old.Email, userUpdate.New.Email, userUpdate.New.Locale, a.GetSiteURL()); err != nil { if err := a.Srv().EmailService.SendEmailChangeEmail(userUpdate.Old.Email, newUser.Email, newUser.Locale, a.GetSiteURL()); err != nil {
c.Logger().Error("Failed to send email change email", mlog.Err(err)) c.Logger().Error("Failed to send email change email", mlog.Err(err))
} }
}) })
} }
} }
if userUpdate.New.Username != userUpdate.Old.Username { if newUser.Username != userUpdate.Old.Username {
a.Srv().Go(func() { a.Srv().Go(func() {
if err := a.Srv().EmailService.SendChangeUsernameEmail(userUpdate.New.Username, userUpdate.New.Email, userUpdate.New.Locale, a.GetSiteURL()); err != nil { if err := a.Srv().EmailService.SendChangeUsernameEmail(newUser.Username, newUser.Email, newUser.Locale, a.GetSiteURL()); err != nil {
c.Logger().Error("Failed to send change username email", mlog.Err(err)) c.Logger().Error("Failed to send change username email", mlog.Err(err))
} }
}) })
} }
a.sendUpdatedUserEvent(*userUpdate.New) a.sendUpdatedUserEvent(*newUser)
} }
a.InvalidateCacheForUser(user.Id) a.InvalidateCacheForUser(user.Id)
a.onUserProfileChange(user.Id) a.onUserProfileChange(user.Id)
return userUpdate.New, nil return newUser, nil
} }
func (a *App) UpdateUserActive(c request.CTX, userID string, active bool) *model.AppError { func (a *App) UpdateUserActive(c request.CTX, userID string, active bool) *model.AppError {

View File

@ -85,9 +85,11 @@ func TestCreateOAuthUser(t *testing.T) {
}) })
} }
func TestSetDefaultProfileImage(t *testing.T) { func TestUpdateDefaultProfileImage(t *testing.T) {
th := Setup(t).InitBasic() th := Setup(t).InitBasic()
defer th.TearDown() defer th.TearDown()
startTime := model.GetMillis()
time.Sleep(time.Millisecond)
err := th.App.SetDefaultProfileImage(th.Context, &model.User{ err := th.App.SetDefaultProfileImage(th.Context, &model.User{
Id: model.NewId(), Id: model.NewId(),
@ -98,11 +100,11 @@ func TestSetDefaultProfileImage(t *testing.T) {
user := th.BasicUser user := th.BasicUser
err = th.App.SetDefaultProfileImage(th.Context, user) err = th.App.UpdateDefaultProfileImage(th.Context, user)
require.Nil(t, err) require.Nil(t, err)
user = getUserFromDB(th.App, user.Id, t) user = getUserFromDB(th.App, user.Id, t)
assert.Equal(t, int64(0), user.LastPictureUpdate) assert.Less(t, user.LastPictureUpdate, -startTime, "LastPictureUpdate should be set to -(current time in milliseconds)")
} }
func TestAdjustProfileImage(t *testing.T) { func TestAdjustProfileImage(t *testing.T) {
@ -194,6 +196,33 @@ func TestUpdateUser(t *testing.T) {
require.NotNil(t, err) require.NotNil(t, err)
require.Nil(t, u) require.Nil(t, u)
}) })
t.Run("fails if default profile picture is not updated when user has default profile picture and username is changed", func(t *testing.T) {
user.Username = "updatedUsername"
iLastPictureUpdate := user.LastPictureUpdate
require.Equal(t, iLastPictureUpdate, int64(0))
u, err := th.App.UpdateUser(th.Context, user, false)
require.Nil(t, err)
require.NotNil(t, u)
require.Less(t, u.LastPictureUpdate, iLastPictureUpdate)
})
t.Run("fails if profile picture is updated when user has custom profile picture and username is changed", func(t *testing.T) {
// Give the user a LastPictureUpdate to mimic having a custom profile picture
err := th.App.Srv().Store().User().UpdateLastPictureUpdate(user.Id)
require.NoError(t, err)
iUser, errGetUser := th.App.GetUser(user.Id)
require.Nil(t, errGetUser)
iUser.Username = "updatedUsername"
iLastPictureUpdate := iUser.LastPictureUpdate
require.Greater(t, iLastPictureUpdate, int64(0))
// Attempt the update, ensure the LastPictureUpdate has not changed
updatedUser, errUpdateUser := th.App.UpdateUser(th.Context, iUser, false)
require.Nil(t, errUpdateUser)
require.NotNil(t, updatedUser)
require.Equal(t, updatedUser.LastPictureUpdate, iLastPictureUpdate)
})
} }
func TestUpdateUserMissingFields(t *testing.T) { func TestUpdateUserMissingFields(t *testing.T) {

View File

@ -285,7 +285,7 @@ func (us SqlUserStore) UpdateLastPictureUpdate(userId string) error {
func (us SqlUserStore) ResetLastPictureUpdate(userId string) error { func (us SqlUserStore) ResetLastPictureUpdate(userId string) error {
curTime := model.GetMillis() curTime := model.GetMillis()
if _, err := us.GetMasterX().Exec("UPDATE Users SET LastPictureUpdate = ?, UpdateAt = ? WHERE Id = ?", 0, curTime, userId); err != nil { if _, err := us.GetMasterX().Exec("UPDATE Users SET LastPictureUpdate = ?, UpdateAt = ? WHERE Id = ?", -curTime, curTime, userId); err != nil {
return errors.Wrapf(err, "failed to update User with userId=%s", userId) return errors.Wrapf(err, "failed to update User with userId=%s", userId)
} }

View File

@ -5895,6 +5895,7 @@ func testDeactivateGuests(t *testing.T, ss store.Store) {
} }
func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) { func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) {
startTime := model.GetMillis()
u1 := &model.User{} u1 := &model.User{}
u1.Email = MakeEmail() u1.Email = MakeEmail()
_, err := ss.User().Save(u1) _, err := ss.User().Save(u1)
@ -5909,8 +5910,7 @@ func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) {
user, err := ss.User().Get(context.Background(), u1.Id) user, err := ss.User().Get(context.Background(), u1.Id)
require.NoError(t, err) require.NoError(t, err)
assert.NotZero(t, user.LastPictureUpdate) assert.GreaterOrEqual(t, user.LastPictureUpdate, startTime)
assert.NotZero(t, user.UpdateAt)
// Ensure update at timestamp changes // Ensure update at timestamp changes
time.Sleep(time.Millisecond) time.Sleep(time.Millisecond)
@ -5923,8 +5923,8 @@ func testUserStoreResetLastPictureUpdate(t *testing.T, ss store.Store) {
user2, err := ss.User().Get(context.Background(), u1.Id) user2, err := ss.User().Get(context.Background(), u1.Id)
require.NoError(t, err) require.NoError(t, err)
assert.True(t, user2.UpdateAt > user.UpdateAt) assert.Greater(t, user2.UpdateAt, user.UpdateAt)
assert.Zero(t, user2.LastPictureUpdate) assert.Less(t, user2.LastPictureUpdate, -startTime)
} }
func testGetKnownUsers(t *testing.T, ss store.Store) { func testGetKnownUsers(t *testing.T, ss store.Store) {

View File

@ -1335,7 +1335,7 @@ export class UserSettingsGeneralTab extends React.Component<Props, State> {
if (Utils.isMobile()) { if (Utils.isMobile()) {
minMessage = formatMessage(holders.uploadImageMobile); minMessage = formatMessage(holders.uploadImageMobile);
} }
if (user.last_picture_update) { if (user.last_picture_update > 0) {
minMessage = ( minMessage = (
<FormattedMessage <FormattedMessage
id='user.settings.general.imageUpdated' id='user.settings.general.imageUpdated'