-
Notifications
You must be signed in to change notification settings - Fork 1
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
Helpscout - Allow users to accept invites #1133
Conversation
Bundle sizes [mpdx-react]Compared against e1c3140
|
pages/account_lists/[accountListId]/accept_invite/[inviteId].page.tsx
Outdated
Show resolved
Hide resolved
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.
Looking great! I left a few comments, but that is it. Great work. I don't want to block you so I will approve this.
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.
Great work! I think the most important thing from my comments is removing useAccountListId()
because unless I'm mistaken it will either always return undefined or might even be returning the inviter's account list id. It might be working because of the default account list redirect logic you implemented. But if we're relying on that, let's make it explicit and redirect the user to /accountLists/_/settings/preferences
(with an underscore as an intentionally invalid account list id or some other clearly invalid id) and add a code comment to document what we're doing.
pages/account_lists/[accountListId]/accept_invite/[inviteId].page.tsx
Outdated
Show resolved
Hide resolved
pages/account_lists/[accountListId]/accept_invite/[inviteId].page.test.tsx
Outdated
Show resolved
Hide resolved
Also, old organization invite links don't redirect to the new URLs but that might be OK because they might not be as common. |
Description
https://secure.helpscout.net/conversation/2731091026/1241412?folderId=7669074
Checklist: