-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(users): user feedback messages improvements #1045
Conversation
0514162
to
c39cd9e
Compare
db/users/update.go
Outdated
@@ -47,6 +47,17 @@ func UpdateUser(ctx context.Context, app, addonUUID, username string) error { | |||
if !userExists { | |||
return errors.New(ctx, fmt.Sprintf("User \"%s\" does not exist", username)) | |||
} | |||
// Check if the user is protected |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c39cd9e
to
8187212
Compare
db/users/update.go
Outdated
var userExists bool | ||
var userProtected bool | ||
for _, user := range l { | ||
if user.Name == username { | ||
userExists = true | ||
|
||
if user.Protected { | ||
userProtected = true | ||
} | ||
break | ||
} | ||
} | ||
if !userExists { | ||
return errors.New(ctx, fmt.Sprintf("User \"%s\" does not exist", username)) | ||
} | ||
|
||
if userProtected { | ||
return errors.New(ctx, fmt.Sprintf("User \"%s\" is protected and cannot be updated", username)) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For better readability I suggest:
var userExists bool | |
var userProtected bool | |
for _, user := range l { | |
if user.Name == username { | |
userExists = true | |
if user.Protected { | |
userProtected = true | |
} | |
break | |
} | |
} | |
if !userExists { | |
return errors.New(ctx, fmt.Sprintf("User \"%s\" does not exist", username)) | |
} | |
if userProtected { | |
return errors.New(ctx, fmt.Sprintf("User \"%s\" is protected and cannot be updated", username)) | |
} | |
var user | |
for _, u := range l { | |
if u.Name == username { | |
user = u | |
break | |
} | |
} | |
if user == nil { | |
return errors.New(ctx, fmt.Sprintf("User \"%s\" does not exist", username)) | |
} | |
if user.Protected { | |
return errors.New(ctx, fmt.Sprintf("User \"%s\" is protected", username)) | |
} |
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pushed a suggestion, WDYT?
8187212
to
8f92e0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
- [ ] Add a changelog entry in the section "To Be Released" of CHANGELOG.md