-
Notifications
You must be signed in to change notification settings - Fork 15
fix(change_logs):add change logs entries for user update/create #140
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
base: main
Are you sure you want to change the base?
Conversation
8e164a6
to
e001138
Compare
c0e40b2
to
75bc23b
Compare
pkg/auth/auth.go
Outdated
updateActive := false | ||
updatedUser.Active = &updateActive | ||
|
||
if err := db.DB.Updates(&updatedUser).Error; err != nil { |
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.
Use tx instead of db.DB
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.
done!
75bc23b
to
9e9e454
Compare
pkg/utils/util.go
Outdated
AddChangelog("DisplayName", oldUser.DisplayName, newUser.DisplayName, &changes) | ||
AddChangelog("UserEmail", oldUser.UserEmail, newUser.UserEmail, &changes) | ||
AddChangelog("UserLevel", oldUser.UserLevel, newUser.UserLevel, &changes) | ||
AddChangelog("UserPassword", oldUser.UserPassword, newUser.UserPassword, &changes) |
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.
Do not add password changes to changelogs. It will be a security issue.
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.
So should I totally remove the password field or add mask '****'. Else for password change logs will not create
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.
Totally remove it. Changing password should be a personal thing and logs shouldn't be generated for it.
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.
ok, i changed that and working as expected ...
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.
1dab160
to
91f42ff
Compare
Found error on testing. Internal server error on passing empty object to PATCH request: curl -X 'PATCH' }' |
91f42ff
to
12fb4a3
Compare
yes, done! |
pkg/auth/auth.go
Outdated
} | ||
updatedUser.Id = olduser.Id | ||
// update the user in the database | ||
if err := tx.Clauses(clause.Returning{}).Updates(&updatedUser).Error; err != nil { |
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.
Clauses(clause.Returning{}) should be removed as we are fetching the updated user below anyhow?
12fb4a3
to
8eb10ba
Compare
add changelogs for user updates and creation