fix: address reviewer feedbacks

status-im · Jan 19, 2024 · a954176 · a954176
1 parent d1f6c04
commit a954176
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 12 deletions.
diff --git a/multiaccounts/database.go b/multiaccounts/database.go
@@ -344,8 +344,12 @@ func (db *Database) UpdateAccountTimestamp(keyUID string, loginTimestamp int64)
 	return err
 }
 
-func (db *Database) UpdateAccountCustomizationColor(keyUID string, color string, clock uint64) (sql.Result, error) {
-	return db.db.Exec("UPDATE accounts SET customizationColor = ?, customizationColorClock = ? WHERE keyUid = ? AND customizationColorClock < ?", color, clock, keyUID, clock)
+func (db *Database) UpdateAccountCustomizationColor(keyUID string, color string, clock uint64) (int64, error) {
+	result, err := db.db.Exec("UPDATE accounts SET customizationColor = ?, customizationColorClock = ? WHERE keyUid = ? AND customizationColorClock < ?", color, clock, keyUID, clock)
+	if err != nil {
+		return 0, err
+	}
+	return result.RowsAffected()
 }
 
 func (db *Database) DeleteAccount(keyUID string) error {

diff --git a/protocol/messenger_handler.go b/protocol/messenger_handler.go
@@ -2415,15 +2415,11 @@ func (m *Messenger) HandleSyncSetting(messageState *ReceivedMessageState, messag
 }
 
 func (m *Messenger) HandleSyncAccountCustomizationColor(state *ReceivedMessageState, message *protobuf.SyncAccountCustomizationColor, statusMessage *v1protocol.StatusMessage) error {
-	result, err := m.multiAccounts.UpdateAccountCustomizationColor(message.GetKeyUid(), message.GetCustomizationColor(), message.GetUpdatedAt())
+	affected, err := m.multiAccounts.UpdateAccountCustomizationColor(message.GetKeyUid(), message.GetCustomizationColor(), message.GetUpdatedAt())
 	if err != nil {
 		return err
 	}
 
-	affected, err := result.RowsAffected()
-	if err != nil {
-		return err
-	}
 	if affected > 0 {
 		state.Response.CustomizationColor = message.GetCustomizationColor()
 	}

diff --git a/protocol/messenger_settings.go b/protocol/messenger_settings.go
@@ -24,6 +24,9 @@ func (m *Messenger) SetCustomNodes(request *requests.SetCustomNodes) error {
 }
 
 func (m *Messenger) SetCustomizationColor(ctx context.Context, request *requests.SetCustomizationColor) error {
+	if err := request.Validate(); err != nil {
+		return err
+	}
 
 	acc, err := m.multiAccounts.GetAccount(request.KeyUID)
 	if err != nil {
@@ -32,20 +35,24 @@ func (m *Messenger) SetCustomizationColor(ctx context.Context, request *requests
 
 	acc.CustomizationColor = request.CustomizationColor
 
+	//Use a combination of wall clock + lamport timestamp, just like Chat#NextClockAndTimestamp
 	tNow := timesource.GetCurrentTimeInMillis()
 	if acc.CustomizationColorClock >= tNow {
 		acc.CustomizationColorClock++
 	} else {
 		acc.CustomizationColorClock = tNow
 	}
 
-	_, err = m.multiAccounts.UpdateAccountCustomizationColor(request.KeyUID, string(acc.CustomizationColor), acc.CustomizationColorClock)
+	affected, err := m.multiAccounts.UpdateAccountCustomizationColor(request.KeyUID, string(acc.CustomizationColor), acc.CustomizationColorClock)
 	if err != nil {
 		return err
 	}
-	err = m.syncAccountCustomizationColor(ctx, acc)
-	if err != nil {
-		return err
+
+	if affected > 0 {
+		err = m.syncAccountCustomizationColor(ctx, acc)
+		if err != nil {
+			return err
+		}
 	}
 	return nil
 }
diff --git a/protocol/messenger_settings_test.go b/protocol/messenger_settings_test.go
@@ -96,5 +96,6 @@ func (s *MessengerSettingsSuite) TestSetCustomizationColor() {
 	acc, err = s.alice2.multiAccounts.GetAccount(s.alice.account.KeyUID)
 	acc2, err = s.alice2.multiAccounts.GetAccount(s.alice2.account.KeyUID)
 	s.Require().NoError(err)
+	s.Require().Equal(common.CustomizationColorBlue, acc.CustomizationColor)
 	s.Require().Equal(acc.CustomizationColor, acc2.CustomizationColor)
 }
diff --git a/protocol/requests/set_customization_color.go b/protocol/requests/set_customization_color.go
@@ -1,8 +1,26 @@
 package requests
 
-import "github.com/status-im/status-go/multiaccounts/common"
+import (
+	"errors"
+	"github.com/status-im/status-go/multiaccounts/common"
+)
+
+var ErrSetCustomizationColorInvalidColor = errors.New("customizationColor: invalid color")
+var ErrSetCustomizationColorInvalidKeyUID = errors.New("keyUid: invalid id")
 
 type SetCustomizationColor struct {
 	CustomizationColor common.CustomizationColor `json:"customizationColor"`
 	KeyUID             string                    `json:"keyUid"`
 }
+
+func (a *SetCustomizationColor) Validate() error {
+	if len(a.CustomizationColor) == 0 {
+		return ErrSetCustomizationColorInvalidColor
+	}
+
+	if len(a.KeyUID) == 0 {
+		return ErrSetCustomizationColorInvalidKeyUID
+	}
+
+	return nil
+}