Skip to content
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] Workspace - No pending workspace join is shown via share link #52394

Open
5 of 8 tasks
IuliiaHerets opened this issue Nov 12, 2024 · 40 comments
Open
5 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Overdue

Comments

@IuliiaHerets
Copy link

IuliiaHerets commented Nov 12, 2024

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: 9.0.60-0
Reproducible in staging?: Y
Reproducible in production?: Y
Issue was found when executing this PR: #51631
Email or phone of affected tester (no customers): applausetester+bp2510@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

Precondition: user A with a workspace.

  1. User A: go to Settings > Workspaces > workspace > Profile
  2. Click on Share > Copy URL
  3. Send URL to user B, that is not a member of this ws
  4. User B (logged in ND in mWeb): open URL in mobile browser

Expected Result:

List of workspaces opened.
Workspace user B was sent a link has a label 'Requested'.

Actual Result:

Settings tab is opened. Sometimes, tap on Workspaces has no response. When open link again, user B is joined ws automatically.

Workaround:

Unknown

Platforms:

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6662227_1731398893074.Screenrecorder-2024-11-11-22-14-11-219.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021856489377948710082
  • Upwork Job ID: 1856489377948710082
  • Last Price Increase: 2024-12-11
Issue OwnerCurrent Issue Owner: @hoangzinh
@IuliiaHerets IuliiaHerets added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Nov 12, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

Triggered auto assignment to @kadiealexander (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

Copy link

melvin-bot bot commented Nov 13, 2024

Job added to Upwork: https://www.upwork.com/jobs/~021856489377948710082

@melvin-bot melvin-bot bot changed the title Workspace - No pending workspace join is shown via share link [$250] Workspace - No pending workspace join is shown via share link Nov 13, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 13, 2024
Copy link

melvin-bot bot commented Nov 13, 2024

Triggered auto assignment to Contributor-plus team member for initial proposal review - @hoangzinh (External)

@ikevin127
Copy link
Contributor

This is a tricky one as it seems to be Android (mWeb only ?) specific, possibly something navigation related because the logic below works as expected on all other platforms:

function WorkspaceJoinUserPage({route, policy}: WorkspaceJoinUserPageProps) {
const styles = useThemeStyles();
const policyID = route?.params?.policyID;
const inviterEmail = route?.params?.email;
const isUnmounted = useRef(false);
useEffect(() => {
if (!isJoinLinkUsed) {
return;
}
navigateAfterJoinRequest();
}, []);
useEffect(() => {
if (isUnmounted.current || isJoinLinkUsed) {
return;
}
if (!isEmptyObject(policy) && !policy?.isJoinRequestPending && !PolicyUtils.isPendingDeletePolicy(policy)) {
Navigation.isNavigationReady().then(() => {
Navigation.goBack(undefined, false, true);
Navigation.navigate(ROUTES.WORKSPACE_INITIAL.getRoute(policyID ?? '-1'));
});
return;
}
MemberAction.inviteMemberToWorkspace(policyID, inviterEmail);
isJoinLinkUsed = true;
Navigation.isNavigationReady().then(() => {
if (isUnmounted.current) {
return;
}
navigateAfterJoinRequest();
});
}, [policy, policyID, inviterEmail]);
useEffect(
() => () => {
isUnmounted.current = true;
},
[],
);
return (
<ScreenWrapper testID={WorkspaceJoinUserPage.displayName}>
<FullScreenLoadingIndicator style={[styles.flex1, styles.pRelative]} />
</ScreenWrapper>
);
}

@Shahidullah-Muffakir
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Requested badge is not showing in mobile devices

What is the root cause of that problem?

we are removing the requested badge in mobile devices here:

<View style={[styles.flexRow, !shouldUseNarrowLayout && styles.workspaceThreeDotMenu]}>

What changes do you think we should make in order to solve the problem?

if we want to show it in mobile devices remove the !shouldUseNarrowLayout

@Shahidullah-Muffakir
Copy link
Contributor

mweb

@hoangzinh
Copy link
Contributor

Hi @Shahidullah-Muffakir please note that it's not only "Requested badge" issue but also navigation issue in small screen devices. Can you retake a look?

@FitseTLT
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

Workspace - No pending workspace join is shown via share link

What is the root cause of that problem?

Here we have two consecutive navigation to settings and workspace settings pages for small screen web after goBack

Navigation.goBack(undefined, false, true);
if (getIsSmallScreenWidth()) {
Navigation.navigate(ROUTES.SETTINGS);
}
Navigation.navigate(ROUTES.SETTINGS_WORKSPACES);

we do that to put settings page below workspace settings page in the stack but for only android mWeb this is making the app stuck.

What changes do you think we should make in order to solve the problem?

We can fix it if we join the goBack and the navigation to settings page together to achieve the same result

navigateAfterJoinRequest = () => {
    const isSmallScreen = getIsSmallScreenWidth();
    Navigation.goBack(isSmallScreen ? ROUTES.SETTINGS : undefined, isSmallScreen, true);

    Navigation.navigate(ROUTES.SETTINGS_WORKSPACES);

POC:

2024-11-15.19-51-17.mp4

What alternative solutions did you explore? (Optional)

Copy link

melvin-bot bot commented Nov 18, 2024

@hoangzinh, @kadiealexander Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Nov 18, 2024
@jjcoffee
Copy link
Contributor

Proposal

Please re-state the problem that we are trying to solve in this issue.

On Android mWeb only, opening the workspace join link URL results in an incorrect navigation state where the settings page is showing with the Workspaces menu item selected (as if it were in the desktop view), but the Workspaces screen is not showing.

What is the root cause of that problem?

We perform multiple navigations to get to the final settings/workspaces route, which is currently triggered after isNavigationReady here:

Navigation.isNavigationReady().then(() => {
if (isUnmounted.current) {
return;
}
navigateAfterJoinRequest();
});

When we navigate directly via URL to the join link, it seems that there's some sort of race condition in the way Chrome handles navigation transitions (interestingly I can only reproduce on Android Chrome and Linux Chrome, but not MacOS Chrome). Most likely the navigation transitions are being queued differently in some subtle way.

What changes do you think we should make in order to solve the problem?

To ensure that we wait until navigation transitions complete first, we can wrap the navigation with runAfterInteractions to ensure the interaction queue is idle, which reduces the chance of intermediate navigation steps being skipped or misordered.

InteractionManager.runAfterInteractions(() => {
	navigateAfterJoinRequest();
});

What alternative solutions did you explore? (Optional)

Whilst digging into if there was some general navigation issue, I noticed that this call to updateLastVisitedPath seems to be playing a part in the issue too:

if (focusedRoute && !CONST.EXCLUDE_FROM_LAST_VISITED_PATH.includes(focusedRoute?.name)) {
updateLastVisitedPath(currentPath);
if (currentPath.startsWith(`/${ROUTES.ONBOARDING_ROOT.route}`)) {
updateOnboardingLastVisitedPath(currentPath);
}
}

Therefore adding WORKSPACE_JOIN_USER to EXCLUDE_FROM_LAST_VISITED_PATH also works as a fix and I don't see an obvious downside as we wouldn't want/need to return to the join link. This effectively bypasses the issue, but it's 50/50 on whether this counts as a workaround or not 😄

@muttmuure muttmuure moved this to Bugs and Follow Up Issues in [#whatsnext] #expense Nov 19, 2024
@hoangzinh
Copy link
Contributor

Hi, @FitseTLT @jjcoffee Do you still reproduce this issue in the latest main branch? I tried a few times, but it seems everything works for me

Screen.Recording.2024-11-19.at.22.11.25.mov

@melvin-bot melvin-bot bot removed the Overdue label Nov 19, 2024
@jjcoffee
Copy link
Contributor

@hoangzinh I had slightly inconsistent behaviour, sometimes the first load works fine, but all subsequent loads wouldn't.

@FitseTLT
Copy link
Contributor

@hoangzinh Yes I can consistently reproduce it

2024-11-19.20-00-49.mp4

@hoangzinh
Copy link
Contributor

hoangzinh commented Nov 20, 2024

I'm unable to reproduce this issue. @jjcoffee if you can reproduce this issue, would you like to be a C+ on this issue?

Screen.Recording.2024-11-20.at.21.49.25.mov

Copy link

melvin-bot bot commented Nov 20, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@hoangzinh
Copy link
Contributor

hoangzinh commented Nov 22, 2024

When we navigate directly via URL to the join link, it seems that there's some sort of race condition in the way Chrome handles navigation transitions

Hi @jjcoffee, Can you find evidence to support your assumption? It's hard for me to go to a solution if we're still unsure about root cause.

@jjcoffee
Copy link
Contributor

Can you find evidence to support your assumption?

@hoangzinh Unfortunately not, other than that this issue only occurs specifically on Chrome. It would require significant extra time to dig further into the root cause.

I agree that we should avoid those consecutive navigations.

Just to note that the consecutive navigation isn't what causes this issue (you can test this either by making multiple navigations to other routes, or by navigating directly to the workspace list).

The issue occurs with the settings screen for any "sub" route, so you'll see the same issue for navigation to settings/subscription (the Subscription menu item will be highlighted but the page won't show).

@FitseTLT
Copy link
Contributor

Just to note that the consecutive navigation isn't what causes this issue (you can test this either by making multiple navigations to other routes, or by navigating directly to the workspace list).

The issue occurs with the settings screen for any "sub" route, so you'll see the same issue for navigation to settings/subscription (the Subscription menu item will be highlighted but the page won't show).

But still the separate navigation calls are causing the problem though it is specifically happening for sub routes and fortunately in this case we can avoid the bug if we avoid one of the navigations via the goBack.

Hi @FitseTLT, I agree that we should avoid those consecutive navigations. But can you elaborate a bit on why it makes the app stuck (on android mWeb)? Thank you.

Honestly speaking it is difficult to give a deeper RCA @hoangzinh the bug is Android mWeb specific bug related with consecutive navigations into subroutes fortunately we can avoid the bug if we join the navigation calls without losing anything.

@jjcoffee
Copy link
Contributor

@FitseTLT Unfortunately, your solution doesn't work consistently for me, so I don't think the consecutive navigation calls are the cause:

issue-52394-2024-11-22_12.40.40.mp4

@hoangzinh
Copy link
Contributor

@jjcoffee your solution works, but I'm unsure if we reached root cause or not.

Copy link

melvin-bot bot commented Nov 26, 2024

@hoangzinh @kadiealexander this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Nov 26, 2024
Copy link

melvin-bot bot commented Nov 26, 2024

@hoangzinh, @kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

@hoangzinh
Copy link
Contributor

not overdue, still waiting for a proposal that helps us understand the root cause.

@melvin-bot melvin-bot bot removed the Overdue label Nov 26, 2024
Copy link

melvin-bot bot commented Nov 27, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Dec 2, 2024

@hoangzinh, @kadiealexander Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Dec 2, 2024
@hoangzinh
Copy link
Contributor

Same as above #52394 (comment)

@melvin-bot melvin-bot bot removed the Overdue label Dec 2, 2024
Copy link

melvin-bot bot commented Dec 4, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Dec 4, 2024
@hoangzinh
Copy link
Contributor

@kadiealexander should we adjust bounty to get more eyes on this issue?

@melvin-bot melvin-bot bot removed the Overdue label Dec 5, 2024
@mvtglobally
Copy link

Issue not reproducible during KI retests. (First week)

Copy link

melvin-bot bot commented Dec 9, 2024

@hoangzinh, @kadiealexander Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Dec 9, 2024
Copy link

melvin-bot bot commented Dec 10, 2024

@hoangzinh @kadiealexander this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

Copy link

melvin-bot bot commented Dec 11, 2024

@hoangzinh, @kadiealexander Huh... This is 4 days overdue. Who can take care of this?

@hoangzinh
Copy link
Contributor

Issue not reproducible during KI retests. (First week)

@kadiealexander can you request to test this issue again? It's tricky for me to reproduce this issue so I'm hard to verify whether it's reproducible or not.

@melvin-bot melvin-bot bot removed the Overdue label Dec 11, 2024
Copy link

melvin-bot bot commented Dec 11, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@mvtglobally
Copy link

Issue not reproducible during KI retests. (Second week)

@melvin-bot melvin-bot bot added the Overdue label Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Overdue
Projects
Status: Bugs and Follow Up Issues
Development

No branches or pull requests

8 participants