-
Notifications
You must be signed in to change notification settings - Fork 0
OF-690-Added a check id in state #416
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
Conversation
| // Run reloadUserState when the screen is mounted | ||
| useEffect(() => { | ||
| reloadUserState(); | ||
| if (state.id) { |
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.
this should never be undefined.
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.
state.id is defined when a user is logged in. And this screen should only be visible when the user is logged in.
wsdt
left a comment
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.
Is this case actually happening? If so do we know why? Did the user clear the app data and not re-open the app (only case I can think of rn). Or our own logic resets the user state if JWT invalid.
This does not happen to me. I tried clearing the app data, tried to switch from "Be Approached" to "Approach Someone" for different logins, and tried setting up a call. But, this does not happen. To prevent the error, I added a check. |
it does not happen but the error is occurring? So you mean it is actually happening and the user id is not set in local state? Do we know why? Because this would need to be fixed instead of the symptom. |
No, the error is not occurring for me. The user id is set in the local state once logged in. It remains when we try to switch the option in the profile setting and save. |
ok awesome! But why do we need the fix/PR then? |
I added a check before calling the method to ensure the value is present, which may prevent issues in the future. |
ok but there was no Sentry ticket or issue at all? So it was "just in case"? Please let's focus on the Sentry errors which are actually occurring right now. Appreciate your proactiveness though! |
No description provided.