-
Notifications
You must be signed in to change notification settings - Fork 21
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
237 edit profile picture and name #462
Conversation
note that the user data cannot be updated until the Safe is deployed and signed up to the Hub |
…ia into 237-edit-profile-picture-and-name
Thanks for reminding. That case is covered by redirection - if safe is not deployed user will end up on /welcome, /validation pages so I guess we are fine here |
I was testing in local dev env, and just after registering (in validation view), The username wasn't there, and instead the safeAddress. Then after I refreshed, I was able to see my username in the validation view. |
Hey @llunaCreixent yes we have this registered as an issue here: #421 |
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.
Looks good!! But I added com comments
- ✅ Tests: PASS
- ✅ Lint: didn't pass but I added a commit
If my username was "miko" and I wanted to edit to "Miko", I should be able to do it as long as the username with other case variations is not taken by other account. Right now it says that the username is taken, but actually it's not used by other account, so it should be allowed (unless ther's other user with username "MIKO" or similar) |
…sUBI/circles-myxogastria into 237-edit-profile-picture-and-name
That is what I check in the API side: https://github.com/CirclesUBI/circles-api/blob/c68f5b483fd46101394a5fef60e959ad29341659/src/controllers/users.js#L92 But maybe it's ok if it's not in the frontend. MAybe ask @triaslucia ?? |
Shall we add in the Description of this PR, "Closes #421" ?? |
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.
approved now for real, functionality works!
I created issue for that: |
no let's leave it I need to test it and probably add a little additional code in one place.. |
Closes #421