-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Feature flags env variable gating #9481
Changes from all commits
fbd24b7
3f56143
669f876
7d4c99c
fab2a1b
1289954
e4f1e63
d3b5ad4
31f9344
7ff8d65
7261b7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,22 +27,17 @@ export const VerifyEffect = () => { | |
); | ||
|
||
useEffect(() => { | ||
const getTokens = async () => { | ||
if (isDefined(errorMessage)) { | ||
enqueueSnackBar(errorMessage, { | ||
variant: SnackBarVariant.Error, | ||
}); | ||
} | ||
if (!loginToken) { | ||
navigate(AppPath.SignInUp); | ||
} else { | ||
setIsAppWaitingForFreshObjectMetadata(true); | ||
await verify(loginToken); | ||
} | ||
}; | ||
|
||
if (!isLogged) { | ||
getTokens(); | ||
if (isDefined(errorMessage)) { | ||
enqueueSnackBar(errorMessage, { | ||
variant: SnackBarVariant.Error, | ||
}); | ||
} | ||
|
||
if (isDefined(loginToken)) { | ||
setIsAppWaitingForFreshObjectMetadata(true); | ||
verify(loginToken); | ||
} else if (!isLogged) { | ||
Comment on lines
+36
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: verify() returns a Promise but it's not being awaited. This could lead to race conditions if the verification fails and the component unmounts before completion. |
||
navigate(AppPath.SignInUp); | ||
} | ||
// Verify only needs to run once at mount | ||
// eslint-disable-next-line react-hooks/exhaustive-deps | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -268,6 +268,8 @@ export const useAuth = () => { | |
|
||
const handleVerify = useCallback( | ||
async (loginToken: string) => { | ||
setIsVerifyPendingState(true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: setIsVerifyPendingState(true) should be wrapped in try/catch to ensure it gets set back to false if verification fails |
||
|
||
const verifyResult = await verify({ | ||
variables: { loginToken }, | ||
}); | ||
|
@@ -282,16 +284,11 @@ export const useAuth = () => { | |
|
||
setTokenPair(verifyResult.data?.verify.tokens); | ||
|
||
const { user, workspaceMember, workspace } = await loadCurrentUser(); | ||
await loadCurrentUser(); | ||
|
||
return { | ||
user, | ||
workspaceMember, | ||
workspace, | ||
tokens: verifyResult.data?.verify.tokens, | ||
}; | ||
setIsVerifyPendingState(false); | ||
}, | ||
[verify, setTokenPair, loadCurrentUser], | ||
[setIsVerifyPendingState, verify, setTokenPair, loadCurrentUser], | ||
); | ||
|
||
const handleCrendentialsSignIn = useCallback( | ||
|
@@ -301,21 +298,9 @@ export const useAuth = () => { | |
password, | ||
captchaToken, | ||
); | ||
setIsVerifyPendingState(true); | ||
|
||
const { user, workspaceMember, workspace } = await handleVerify( | ||
loginToken.token, | ||
); | ||
|
||
setIsVerifyPendingState(false); | ||
|
||
return { | ||
user, | ||
workspaceMember, | ||
workspace, | ||
}; | ||
await handleVerify(loginToken.token); | ||
}, | ||
[handleChallenge, handleVerify, setIsVerifyPendingState], | ||
[handleChallenge, handleVerify], | ||
); | ||
|
||
const handleSignOut = useCallback(async () => { | ||
|
@@ -360,13 +345,7 @@ export const useAuth = () => { | |
); | ||
} | ||
|
||
const { user, workspace, workspaceMember } = await handleVerify( | ||
signUpResult.data?.signUp.loginToken.token, | ||
); | ||
|
||
setIsVerifyPendingState(false); | ||
|
||
return { user, workspaceMember, workspace }; | ||
await handleVerify(signUpResult.data?.signUp.loginToken.token); | ||
}, | ||
[ | ||
setIsVerifyPendingState, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { createState } from 'twenty-ui'; | ||
|
||
export const canManageFeatureFlagsState = createState<boolean>({ | ||
key: 'canManageFeatureFlagsState', | ||
defaultValue: false, | ||
}); |
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.
It would be good to give a bit more context in your PR description when you introduce something like that as it doesn't directly relate to the original issue / I'm not sure exactly what problem we're solving here (race condition / dual execution?). Seems similar to topics discussed here: #9288
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.
cc @AMoreaux
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.
Two commits come from this PR #9451
It allows impersonation on the same workspace.