Skip to content

new user update endpoint - #162

Merged
melvinsoft merged 2 commits into
appsembler/eucalyptus/developfrom
maxi/new-user-update-endpoint
Sep 20, 2017
Merged

new user update endpoint#162
melvinsoft merged 2 commits into
appsembler/eucalyptus/developfrom
maxi/new-user-update-endpoint

Conversation

@melvinsoft

Copy link
Copy Markdown

New endpoint to update user attributes, please check the docs added first.

@melvinsoft

Copy link
Copy Markdown
Author

@melvinsoft
melvinsoft force-pushed the maxi/new-user-update-endpoint branch from 4bc92a6 to bb303d0 Compare September 9, 2017 17:46
fix in case custom form doesn't exists

docs for the new endpoint

remove prints
@melvinsoft
melvinsoft force-pushed the maxi/new-user-update-endpoint branch from bb303d0 to 95c7097 Compare September 9, 2017 17:53

@tkeemon tkeemon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Just one minor change.

# update email
if 'email' in data:
user.email = data['email']
user.save()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to watch out for an IntegrityError here. If a request attempts to change an email address to one already in use, then this will error out.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tkeemon Great catch, change addressed, but to my surprise, I can have two users with the same email, the IntegrityError isn't triggered. So I added the same check that we've in other endpoints.

@tkeemon tkeemon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good to me.

👍

@amirtds amirtds left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@melvinsoft , looks great

@melvinsoft
melvinsoft merged commit d0a06d4 into appsembler/eucalyptus/develop Sep 20, 2017
@melvinsoft
melvinsoft deleted the maxi/new-user-update-endpoint branch September 20, 2017 14:03
@melvinsoft
melvinsoft restored the maxi/new-user-update-endpoint branch September 20, 2017 14:04
@melvinsoft
melvinsoft deleted the maxi/new-user-update-endpoint branch September 20, 2017 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants