-
Notifications
You must be signed in to change notification settings - Fork 27
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
feat: add account-sharing page #1138
Conversation
3e47de5
to
fe5d2b4
Compare
Thank you for the initial pass here! A few requested 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.
lgtm!
- Click on the **Add Member** button. | ||
|
||
<img src="/img/account-sharing/add-member.png" width="60%"/> | ||
- Enter the email address of the user you wish to add. Please ensure that the user has an existing Momento account, meaning they must have signed into the Momento console at least once using that email address. |
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.
what happens if they type an e-mail address that has never logged in to the Momento console?
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.
The user will not be identified by the server and we throw an error on console saying "User could not be found."
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.
okay. i just tested it and I see what you mean. This PR is good then, but we should create a ticket for the console itself to augment that error message a bit, so that users will know what the remedy is if they get that error.
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.
just one question and a few minor nits
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 but we should hold off on merging it until we release the console changes.
393a12b
to
b39a920
Compare
PR Description:
Issue:
https://github.com/momentohq/momento-console/issues/853