fix-issue-7011 #7125
fix-issue-7011 #7125
Conversation
|
Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗 |
|
vsnegi039 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 97e2de6 in 1 minute and 29 seconds
More details
- Looked at
42lines of code in1files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/pages/SignUp/SignUp.tsx:113
- Draft comment:
Consider handling redirection and notification inside useEffect dependencies to ensure predictable side-effects. Also, review inline style usages in notifications if possible, using design tokens/constants. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/pages/SignUp/SignUp.tsx:63
- Draft comment:
Avoid performing side-effects (like history.push) directly in the render body. Consider moving this token check and redirection into a useEffect hook to prevent unexpected behavior. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_2nnvhYgXr9dJulss
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -59,6 +60,13 @@ function SignUp({ version }: SignUpProps): JSX.Element { | |||
| const { search } = useLocation(); | |||
| const params = new URLSearchParams(search); | |||
| const token = params.get('token'); | |||
| if (token === '') { | |||
There was a problem hiding this comment.
Avoid side-effects during render: redirecting (history.push) and triggering notifications should be done in useEffect rather than in the component body.
| @@ -59,6 +60,13 @@ function SignUp({ version }: SignUpProps): JSX.Element { | |||
| const { search } = useLocation(); | |||
| const params = new URLSearchParams(search); | |||
| const token = params.get('token'); | |||
| if (token === '') { | |||
There was a problem hiding this comment.
The condition 'if (token === "")' only catches empty strings. Consider using 'if (!token)' to also handle the null case, ensuring missing tokens are correctly redirected.
| if (token === '') { | |
| if (!token) { |
Summary
Redirect users to login if invitation token is invalid or missing
Related Issues / PR's
#7011
Screenshots
Affected Areas and Manually Tested Areas
Important
Redirects users to login in
SignUp.tsxif invitation token is invalid or missing, with error notifications.tokenis invalid or missing inSignUp.tsx.t('token_required')iftokenis empty.getInviteDetailsResponse.data?.erroris present.notifications.errorcall inSignUp()to handle missingtoken.history.push(ROUTES.LOGIN)for redirection inSignUp().This description was created by
for 97e2de6. It will automatically update as commits are pushed.