-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[$250] Log in – Welcome modal is missing when login as a new user from public room login modal #41500
Comments
Triggered auto assignment to @dylanexpensify ( |
@dylanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors |
Job added to Upwork: https://www.upwork.com/jobs/~017616f5f829d51bb6 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @rojiphil ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Welcome modal does not open for a new account when logged in from right hand sign modal for a public room What is the root cause of that problem?The App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx Lines 53 to 55 in cca1952
for showing the onboarding modal triggers only when isLoadingApp changes and isLoadingApp changes only when openApp is called.Lines 216 to 225 in 387ab0e
so, when we visit a public room openApp is called and after logging in from RHP sign in modal, it is not called again and isLoadingApp value does not change and the useEffect does not trigger.
What changes do you think we should make in order to solve the problem?We can have an Onyx value to check if the RHP SignIn modal closed after signing in and set it to App/src/pages/signin/SignInModal.tsx Lines 29 to 30 in 387ab0e
like Navigation.isNavigationReady().then(() => Navigation.dismissModal());
+++Onyx.set(ONYXKEYS.SIGNIN_MODAL_CLOSED, true);
} after the dismissing the RHP. What alternative solutions did you explore? (Optional) |
ProposalPlease re-state the problem that we are trying to solve in this issue.The welcome modal is missing when login as a new user from a public room What is the root cause of that problem?The BottomTabBar file(Inside Report page) will invoke the Welcome onboarding functionality after the user logs in as a new user from the Login Page, but If I try to log in as a new user from the Public room the BottomTabBar won't be invoked, I think because the BottomTabBar is being cached on the Report Page as I've already opened it so it won't call the Welcome onboarding functionality. I've tried to hard refresh to remove the cache and the Welcome onboarding is showing up but after a while when I tried to refresh the page again and again it won't show up. For the Guest input, the issue comes from the backend, and when a user logs in as a new user from the public room we do not remove the Guest input value. What changes do you think we should make in order to solve the problem?We can add the Welcome onboarding functionality inside the LoginForm directly so that when the user logs in as a new account, the Welcome onboarding will be triggered automatically. App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx Line 53 in 2a6d63d
And for the Guest input issue, we can fix it inside LoginForm and Report Utils (for rendering the participant name) directly by checking if the user has logged in as a new user and has not yet completed the Welcome onboarding and the Account Display name as a guest, we can set it the user display name to empty which will show the user email. I've fixed the issue and it is working successfully, below is the video demo(updated): 2024-05-12.16-28-17.mp4What alternative solutions did you explore? (Optional)N/A |
ProposalPlease re-state the problem that we are trying to solve in this issue.Welcome modal doesn't show up when user sign in in rigth panel if the user visit public room. What is the root cause of that problem?Welcome step is executed by this function: App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx Lines 53 to 55 in 550e83b
But it will only executed right after App is finished loading. the use effect depend on isLoadingApp which is based on onyxkey: IS_LOADING_APP. When user visit public room the app is finished loading and the useEffect function is executed but won't reach welcome function because the user is anonymous user. What changes do you think we should make in order to solve the problem?We can change or add the useEffect dependency using other onyx value for example export default withOnyx<PurposeForUsingExpensifyModalProps, PurposeForUsingExpensifyModalOnyxProps>({
isLoadingApp: {
key: ONYXKEYS.IS_LOADING_APP,
},
lastVisitedPath: {
key: ONYXKEYS.LAST_VISITED_PATH,
},
})(BottomTabBar); the the dependency of useEffect will be: + }, [lastVisitedPath, isLoadingApp]); and to prevent the welcome modal shown two times right after user completed welcome steps and app send nvp onboarding to server then the app navigate to welcome video. We could add: + let isOnboardingInProgress : boolean = false;
...
...
function isOnboardingFlowCompleted({onCompleted, onNotCompleted}: HasCompletedOnboardingFlowProps) {
isOnboardingFlowStatusKnownPromise.then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
+ isOnboardingInProgress = false;
onCompleted?.();
} else {
+ if (isOnboardingInProgress) {
+ return;
+ }
+ isOnboardingInProgress = true;
onNotCompleted?.();
}
});
} The solution is not fixed to certain onyxkey but can use other onyxkey based on other consideration. What alternative solutions did you explore?Adding dependency on useEffect will cause unnecessary re-trigger of function startOnBoarding(){
listenToSessionChange();
isUserLoggedInPromise
.then(() => isServerDataReadyPromise)
.then(() => {
listenToNvpOnboardingChange();
return isOnboardingFlowStatusKnownPromise;
}).then(() => {
listenToPathChange();
return isRouteReadyPromise;
}).then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
onCompleted?.();
} else {
Navigation.navigate(ROUTES.ONBOARDING_ROOT)
//onNotCompleted?.();
}
return;
});
}
startOnBoarding(); and in callback of LAST_ViSITED_PATH onyx: const rootRoute = navigationRef.getRootState()?.routes;
const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
return;
}
resolveRouteIsReady();
Onyx.disconnect(lastVistedConnectID); Complete rough code:let resolveIsReadyPromise: (value?: Promise<void>) => void | undefined;
let isServerDataReadyPromise = new Promise<void>((resolve) => {
resolveIsReadyPromise = resolve;
});
let resolveOnboardingFlowStatus: (value?: Promise<void>) => void | undefined;
let isOnboardingFlowStatusKnownPromise = new Promise<void>((resolve) => {
resolveOnboardingFlowStatus = resolve;
});
let resolveRouteIsReady: (value?: Promise<void>) => void | undefined;
let isRouteReadyPromise = new Promise<void>((resolve) => {
resolveRouteIsReady = resolve;
});
let resolveUserIsLoggedIn: (value?: Promise<void>) => void | undefined;
let isUserLoggedInPromise = new Promise<void>((resolve) => {
resolveUserIsLoggedIn = resolve;
});
function onServerDataReady(): Promise<void> {
return isServerDataReadyPromise;
}
const listenToSessionChange = function() {
const sessionConeectionId = Onyx.connect({
key: ONYXKEYS.SESSION,
initWithStoredValues: false,
callback: (session) => {
if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) {
return;
}
resolveUserIsLoggedIn?.();
Onyx.disconnect(sessionConeectionId);
},
});
}
const listenToPathChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.LAST_VISITED_PATH,
initWithStoredValues: true,
callback: (cc) => {
const rootRoute = navigationRef.getRootState()?.routes;
const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
return;
}
resolveRouteIsReady();
Onyx.disconnect(lastVistedConnectID);
},
});
}
const listenToNvpOnboardingChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.NVP_ONBOARDING,
initWithStoredValues: true,
callback: (value) => {
if (value === null || value === undefined) {
return;
}
onboarding = value;
resolveOnboardingFlowStatus();
Onyx.disconnect(connectionId);
},
});
}
function startOnBoarding(){
listenToSessionChange();
isUserLoggedInPromise
.then(() => isServerDataReadyPromise)
.then(() => {
listenToNvpOnboardingChange();
return isOnboardingFlowStatusKnownPromise;
}).then(() => {
listenToPathChange();
return isRouteReadyPromise;
}).then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
onCompleted?.();
} else {
Navigation.navigate(ROUTES.ONBOARDING_ROOT)
//onNotCompleted?.();
}
return;
});
}
startOnBoarding(); |
Proposal |
@rojiphil to review updated proposal |
Reviewing today |
@NJ-2020 Can you please add relevant code to your proposal so we know how you fixed it? |
Proposal |
@rojiphil Done |
Thanks for your proposals. |
Ok, I will look into it. Thanks |
Logic to show the onboarding modal or not is within the App/src/libs/actions/Welcome.ts Line 38 in 5730ba4
For users who already completed the onboarding, the backend returns the The proposal extends this logic to trigger it similarly when a user logs in from RHP. So, with the suggested solution About the If we want the display name to be login for new sign-ins from RHP as well similar to new logins in normal flow, that needs to be fixed from backend because Let me know if I am missing something or something is unclear in my comments. |
Proposalupdated on alternative solution
@rojiphil yes adding dependency on useEffect will cause unnecessary re-trigger of Complete rough code:let resolveIsReadyPromise: (value?: Promise<void>) => void | undefined;
let isServerDataReadyPromise = new Promise<void>((resolve) => {
resolveIsReadyPromise = resolve;
});
let resolveOnboardingFlowStatus: (value?: Promise<void>) => void | undefined;
let isOnboardingFlowStatusKnownPromise = new Promise<void>((resolve) => {
resolveOnboardingFlowStatus = resolve;
});
let resolveRouteIsReady: (value?: Promise<void>) => void | undefined;
let isRouteReadyPromise = new Promise<void>((resolve) => {
resolveRouteIsReady = resolve;
});
let resolveUserIsLoggedIn: (value?: Promise<void>) => void | undefined;
let isUserLoggedInPromise = new Promise<void>((resolve) => {
resolveUserIsLoggedIn = resolve;
});
function onServerDataReady(): Promise<void> {
return isServerDataReadyPromise;
}
const listenToSessionChange = function() {
const sessionConeectionId = Onyx.connect({
key: ONYXKEYS.SESSION,
initWithStoredValues: false,
callback: (session) => {
if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) {
return;
}
resolveUserIsLoggedIn?.();
Onyx.disconnect(sessionConeectionId);
},
});
}
const listenToPathChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.LAST_VISITED_PATH,
initWithStoredValues: true,
callback: (cc) => {
const rootRoute = navigationRef.getRootState()?.routes;
const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
return;
}
resolveRouteIsReady();
Onyx.disconnect(connectionId);
},
});
}
const listenToNvpOnboardingChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.NVP_ONBOARDING,
initWithStoredValues: true,
callback: (value) => {
if (value === null || value === undefined) {
return;
}
onboarding = value;
resolveOnboardingFlowStatus();
Onyx.disconnect(connectionId);
},
});
}
function startOnBoarding(){
listenToSessionChange();
isUserLoggedInPromise
.then(() => isServerDataReadyPromise)
.then(() => {
listenToNvpOnboardingChange();
return isOnboardingFlowStatusKnownPromise;
}).then(() => {
listenToPathChange();
return isRouteReadyPromise;
}).then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
onCompleted?.();
} else {
Navigation.navigate(ROUTES.ONBOARDING_ROOT)
//onNotCompleted?.();
}
return;
});
}
startOnBoarding(); function startOnBoarding(){
listenToSessionChange();
isUserLoggedInPromise
.then(() => isServerDataReadyPromise)
.then(() => {
listenToNvpOnboardingChange();
return isOnboardingFlowStatusKnownPromise;
}).then(() => {
listenToPathChange();
return isRouteReadyPromise;
}).then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
onCompleted?.();
} else {
Navigation.navigate(ROUTES.ONBOARDING_ROOT)
//onNotCompleted?.();
}
return;
});
}
startOnBoarding(); and in callback of LAST_ViSITED_PATH onyx: const rootRoute = navigationRef.getRootState()?.routes;
const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
return;
}
resolveRouteIsReady();
Onyx.disconnect(lastVistedConnectID); Complete rough code:let resolveIsReadyPromise: (value?: Promise<void>) => void | undefined;
let isServerDataReadyPromise = new Promise<void>((resolve) => {
resolveIsReadyPromise = resolve;
});
let resolveOnboardingFlowStatus: (value?: Promise<void>) => void | undefined;
let isOnboardingFlowStatusKnownPromise = new Promise<void>((resolve) => {
resolveOnboardingFlowStatus = resolve;
});
let resolveRouteIsReady: (value?: Promise<void>) => void | undefined;
let isRouteReadyPromise = new Promise<void>((resolve) => {
resolveRouteIsReady = resolve;
});
let resolveUserIsLoggedIn: (value?: Promise<void>) => void | undefined;
let isUserLoggedInPromise = new Promise<void>((resolve) => {
resolveUserIsLoggedIn = resolve;
});
function onServerDataReady(): Promise<void> {
return isServerDataReadyPromise;
}
const listenToSessionChange = function() {
const sessionConeectionId = Onyx.connect({
key: ONYXKEYS.SESSION,
initWithStoredValues: false,
callback: (session) => {
if (session?.accountID === undefined || session?.authTokenType === CONST.AUTH_TOKEN_TYPES.ANONYMOUS) {
return;
}
resolveUserIsLoggedIn?.();
Onyx.disconnect(sessionConeectionId);
},
});
}
const listenToPathChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.LAST_VISITED_PATH,
initWithStoredValues: true,
callback: (cc) => {
const rootRoute = navigationRef.getRootState()?.routes;
const currentRouteName = rootRoute?.[rootRoute.length - 1]?.name;
if (Boolean(currentRouteName && currentRouteName !== NAVIGATORS.BOTTOM_TAB_NAVIGATOR && currentRouteName !== NAVIGATORS.CENTRAL_PANE_NAVIGATOR)) {
return;
}
resolveRouteIsReady();
Onyx.disconnect(lastVistedConnectID);
},
});
}
const listenToNvpOnboardingChange = function() {
const connectionId = Onyx.connect({
key: ONYXKEYS.NVP_ONBOARDING,
initWithStoredValues: true,
callback: (value) => {
if (value === null || value === undefined) {
return;
}
onboarding = value;
resolveOnboardingFlowStatus();
Onyx.disconnect(connectionId);
},
});
}
function startOnBoarding(){
listenToSessionChange();
isUserLoggedInPromise
.then(() => isServerDataReadyPromise)
.then(() => {
listenToNvpOnboardingChange();
return isOnboardingFlowStatusKnownPromise;
}).then(() => {
listenToPathChange();
return isRouteReadyPromise;
}).then(() => {
if (Array.isArray(onboarding) || onboarding?.hasCompletedGuidedSetupFlow === undefined) {
return;
}
if (onboarding?.hasCompletedGuidedSetupFlow) {
onCompleted?.();
} else {
Navigation.navigate(ROUTES.ONBOARDING_ROOT)
//onNotCompleted?.();
}
return;
});
}
startOnBoarding();
For the guest user name, The guest name will change when user complete the |
Proposal
|
@rojiphil Done. I've updated my proposal Looking forward to hearing from you |
Proposal |
Triggered auto assignment to @lschurr ( |
Reassigning as I head to parental leave next week! Thanks Lauren! |
Was there actually a PR linked to this GH for fixing the bug or is that being handled elsewhere? |
@lschurr As I understand, we implemented and merged the PR for this issue only to realize that it did not get fixed completely as mentioned here due to an additional case added via some parallel changes elsewhere. The fixes as mentioned here address the additional case introduced through parallel changes. It has been quite a roller coaster ride for this issue but we finally made it as mentioned here. We can process the payment for this issue and close this. |
@rojiphil can you help with the breakdown... again? I'm trying to figure out how this slots in with our processes. Is it
If yes, this is what we have in this SO
|
@mallenexpensify Steps (1) and (2) are correct. In step (3), PR is not reverted but there were a few parallel changes (i.e new feature of onboarding a new customer via |
Thanks @rojiphil can you and @c3024 please accept the job and reply here once you have? |
Accepted offer. Thanks |
Accepted the offer. Thanks! |
Thanks for jumping in @mallenexpensify - I finished up the payments! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 1.4.69-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4531005
Email or phone of affected tester (no customers): ponikarchuks+131524@gmail.com
Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
Welcome modal appears and user should enter the name
Actual Result:
Welcome modal is missing when login as a new user from public room login modal, the user's name remains Guest
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
Screenshots/Videos
Add any screenshot/video evidence
Bug6468653_1714637134436.Guest.mp4
View all open jobs on GitHub
Issue Owner
Current Issue Owner: @c3024Issue Owner
Current Issue Owner: @rojiphilThe text was updated successfully, but these errors were encountered: