feat: postgres store #4626
feat: postgres store #4626
Conversation
| if req.Name != "" { | ||
| user.Name = cases.Title(language.AmericanEnglish).String(strings.ToLower(req.Name)) | ||
| } | ||
|
|
||
| if req.Username != "" { | ||
| user.Username = strings.ToLower(req.Username) | ||
| } | ||
|
|
||
| if req.Email != "" { | ||
| user.Email = strings.ToLower(req.Email) | ||
| } | ||
|
|
||
| if req.RecoveryEmail != "" { | ||
| user.Preferences.SecurityEmail = strings.ToLower(req.RecoveryEmail) |
There was a problem hiding this comment.
I think here we can use the previous strategy, which is always to the lowercase function call. The checking seems to be not worth and it complicates the code without a real, probably, good performance gain.
| @@ -88,8 +79,9 @@ func (s *service) Setup(ctx context.Context, req requests.Setup) error { | |||
| }, | |||
| } | |||
|
|
|||
| // TODO: use a transaction here | |||
There was a problem hiding this comment.
I understand that you want to do the user creation and the namespace creation using a database transaction and avoid the need to create and delete the user just by logic. However, I think that this to-do is likely to be left for the future because it's going to demand some more complicated changes in the datastore layer. So my suggestion is for you to describe it better in the description so we can leave it for the next step of the work.
There was a problem hiding this comment.
I agree. The comment will be removed before the merge when I've taken notes about what I need to do afterward.
| @@ -65,21 +78,20 @@ func (s *service) UpdateUser(ctx context.Context, req *requests.UpdateUser) ([]s | |||
| // | |||
| // Deprecated, use [Service.UpdateUser] instead. | |||
| func (s *service) UpdatePasswordUser(ctx context.Context, id, currentPassword, newPassword string) error { | |||
| user, _, err := s.store.UserGetByID(ctx, id, false) | |||
| user, err := s.store.UserGet(ctx, store.UserIdentID, id) | |||
There was a problem hiding this comment.
I believe that this duplicated function could die before we do the change. So we have a restore reduction that needs to be supported for the next version.
No description provided.