[$500] Opens report instead of LHN on Native apps for new users when invited to a workspace #29207

m-natarajan · 2023-10-10T16:59:04Z

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.3.80-3
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @c3024
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696954351917789

Action Performed:

Create a workspace from User A
Invite a User B who is not an Expensify member yet to this workspace
Login with User B (the new user who does not have an Expensify account yet) from iOS or Android app

Expected Result:

LHN should open on the app

Actual Result:

The workspace chat opens on the app

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Android: Native

AndroidUnavailableWorkspace.mp4

Android: mWeb Chrome

AndroidUnavailableWorkspace.1.mp4

iOS: Native

rpreplay-final1696953027_Rdb3DKBX.mp4

iOS: mWeb Safari

iossafari_tlxmIu3D.mp4

MacOS: Chrome / Safari

web_NrW4jzHh.mp4

new.user.mp4

MacOS: Desktop

desktop.1.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0174b94be9389d33ad
Upwork Job ID: 1711788460798902272
Last Price Increase: 2024-03-13
Automatic offers:

dukenv0307 | Contributor | 0

melvin-bot · 2023-10-10T16:59:09Z

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

melvin-bot · 2023-10-10T16:59:16Z

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

deomaius · 2023-10-12T07:51:09Z

Proposal

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

The onboarding process of a newly invited user - specificially to a workspace - is misaligned to the intended logic.

What is the root cause of that problem?

There is pre-coded logic to in Welcome.js to enforce this behaviour, it is not a result of a bug.

App/src/libs/actions/Welcome.js

Line 137 in ae9f83f

    
           Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(workspaceChatReport.reportID));

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

The conditional parameter shouldNavigateToWorkspaceChat and if statement to enforce the redirect should be redacted.

App/src/libs/actions/Welcome.js

Lines 136 to 146 in ae9f83f

    
           if (shouldNavigateToWorkspaceChat && workspaceChatReport) { 
        
               Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(workspaceChatReport.reportID)); 
        
               // If showPopoverMenu exists and returns true then it opened the Popover Menu successfully, and we can update isFirstTimeNewExpensifyUser 
        
               // so the Welcome logic doesn't run again 
        
               if (showPopoverMenu()) { 
        
                   isFirstTimeNewExpensifyUser = false; 
        
               } 
        
               return; 
        
           }

melvin-bot · 2023-10-16T11:55:58Z

@0xmiroslav Eep! 4 days overdue now. Issues have feelings too...

melvin-bot · 2023-10-17T16:00:19Z

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

melvin-bot · 2023-10-18T12:22:00Z

@0xmiroslav Still overdue 6 days?! Let's take care of this!

0xmiros · 2023-10-18T12:38:06Z

This is tagged as native only issue. If that's true, the root cause should explain why issue doesn't happen on mWeb.

Awaiting more proposals

deomaius · 2023-10-18T15:33:18Z

This is tagged as native only issue. If that's true, the root cause should explain why issue doesn't happen on web.

It occurs on web also would you know, so what are the next steps? Eager to submit a PR and waiting on your words.

deomaius · 2023-10-23T14:46:46Z

@miljakljajic can you provide some assistance here, been waiting for a sufficient review for two of my proposals for over 2 weeks yet your reviewer - who is assigned to both issues - has failed to follow through.

I am heavily contemplating not contributing to this organisation again because of this situation, I am eager to contribute but not if there is unprofessional treatment for external contributors present.

melvin-bot · 2023-10-24T16:01:27Z

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

deomaius · 2023-10-24T16:35:06Z

Pinging @m-natarajan as maybe they could give a response since being the author of this specific issue.

melvin-bot · 2023-10-25T12:54:03Z

@0xmiroslav 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

melvin-bot · 2023-10-27T12:53:41Z

@0xmiroslav 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

melvin-bot · 2023-10-31T13:21:20Z

@0xmiroslav 12 days overdue. Walking. Toward. The. Light...

melvin-bot · 2023-10-31T16:01:35Z

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

melvin-bot · 2023-11-03T13:21:40Z

This issue has not been updated in over 14 days. @0xmiroslav eroding to Weekly issue.

deomaius · 2023-11-04T02:35:23Z

I will not be recommending contributing to expensify to any engineers from my expierence upon three individual proposals; #29212, #29110 and this issue #29207.

I have been mislead consitently par in part because of negilgent reviewers such as @0xmiroslav and poor originisational discourse on the viability of a issue - preimposed to issuing it and a contributor submitting work (@miljakljajic). I feel wasted of my time and my skills and treated horribly in this situation. I wish you the best working with air, if this proposal goes through so be it - but my opinion will not change about the organisational management of this entity.

Goodday.

melvin-bot · 2023-11-07T16:01:03Z

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

melvin-bot · 2024-03-18T11:41:53Z

Triggered auto assignment to @iwiznia, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

zanyrenney · 2024-03-18T11:49:48Z

@iwiznia would you mind reviewing and choose which of the proposals to go with, please? @dukenv0307 has said either @DylanDylann or @ikevin127 proposals seem most suitable. Thanks!

ikevin127 · 2024-03-18T12:56:56Z

IMO we don't need to show the modal in this case since it can cause the inconsistency.

@dukenv0307 This does not make sense, what inconsistency are you referring to ?

Since if we don't redirect the user to the workspace report, they are just going to land on the LHN on their first login.

Currently for all other cases where no navigation is called (user lands on LHN) on first login, we always show the welcome modal.

I think we should do the same here, otherwise should we change BE and not send the Welcome email for this specific case ? 👀

dukenv0307 · 2024-03-18T13:46:10Z

This does not make sense, what inconsistency are you referring to ?

I mean the inconsistency between desktop and mobile devices. Currently, if the welcome modal is shown on large screens, it will be shown on small screens too. If we add the condition to check isNarrowLayout here the welcome modal will be shown on small screen but not large one.

Let's dig deeper into the reason why we shouldn't navigate to WS chat in this case. It's because on large screens we can show both LHN and chat, but on small screens, we need to choose one (in this case it's LHN), so I just wonder if we should show the welcome modal just because it's on small screens.

Anyway, we need to hear the final expectation from @iwiznia

ikevin127 · 2024-03-18T14:25:11Z

Thanks for the explanation, indeed it makes sense to be aligned and have the Welcome modal (RHP on large screens) pop up regardless - if that's what we want to do here.

This means that on large screens we see LHN + the report opened on the right side + the Welcome modal (RHP).

For this we would have to remove this check: Object.keys(allPolicies ?? {}).length === 1 (checks if we have more than one reports (Concierge) on first login).

While for narrow layout devices we have 2 options:

navigate and open workspace report on login + show welcome modal
don't navigate to workspace report, landing on LHN upon first login + show welcome modal (my proposal)

iwiznia · 2024-03-18T16:01:48Z

ok let me see if I get this correctly.
In large screens, all works great because we:

Open the chat in the main pane
Open the welcome page in the RHP

But on small screens, we can't do that, there's no RHP, so we need to chose to either:

Show the workspace chat
Show the welcome page

Is that correct?

iwiznia · 2024-03-18T16:04:29Z

Not 100% sure about this, but in small screens I think we could/should do:

Open the welcome page
When you close it, behind it is the workspace chat page

This both ensures you see the welcome page and that you see the workspace chat and it's the closest thing to what large screens do

ikevin127 · 2024-03-18T16:09:02Z

What is currently happening:

Large screens:

user part of a workspace logins first time
workspace report opens on right side of LHN
no welcome message (RHP not opening - by design)

Small screens exactly the same thing as for large screens (except you don't see the LHN on small screens, you see only the report you're navigated to).

Also on small screens there's no RHP so the modal pops up from the bottom of the screen.

This issue was created because the behaviour on small screens is apparently not desired.

My suggestion is we should show welcome message regardless of screen size.

My proposal achieved the expected result of the issue + mentioned the welcome modal consideration.

iwiznia · 2024-03-18T16:13:00Z

workspace report opens on right side of LHN

You mean it opens in the RHP? If so, what do we show in the main pane?

My suggestion is we should show welcome message regardless of screen size.

I think I agree with this

ikevin127 · 2024-03-18T16:41:52Z

You mean it opens in the RHP? If so, what do we show in the main pane?

@iwiznia No, right side of LHN = main pane, not RHP.

iwiznia · 2024-03-18T16:44:52Z

ok cool, we agree. To summarize what we want:
Large screens:

Open welcome page in RHP
Open worskapce chat in main pane

Small screens:

Open welcome page in RHP that takes the whole screen
When closing RHP, behind it should be the workspace chat open

ikevin127 · 2024-03-18T17:18:20Z

Updated proposal

updated RCA and solution to align w/ latest expectations as per #29207 (comment)

cc @iwiznia

DylanDylann · 2024-03-19T09:52:20Z

@iwiznia I think there is conflict between the new expected behavior and this one. What do you think?

iwiznia · 2024-03-19T15:29:03Z

Why you say that? From a quick look seems like that is already implemented what I said above?

DylanDylann · 2024-03-19T15:36:17Z

In this comment, we only open welcome banner if there is only personal policy (user is not invited to any workspace, allPolicy.length === 1).
In your comment: When user is invited to any workspace, the welcome banner still appears (allPolicy.length > 1)

iwiznia · 2024-03-19T15:48:10Z

Ohhhh 🤔 you are correct and I was wrong... in this flow we are already invited to a policy, so we don't need to show the welcome card.

DylanDylann · 2024-03-19T15:52:52Z

@iwiznia Based on this C+ comment, my proposal will fix the original issue

ikevin127 · 2024-03-19T16:06:15Z

Reverted proposal

reverted to previous solution according to latest decision from #29207 (comment)

Note

The other proposal is not using the correct methods for implementing the proposed solution.

I think a 50/50 split is fair if @DylanDylann will handle the issue using the correct logic mentioned in my proposal's solution.

cc @iwiznia

DylanDylann · 2024-03-19T16:23:27Z

@ikevin127 Why did you say: "The other proposal is not using the correct methods for implementing the proposed solution."?

ikevin127 · 2024-03-19T16:27:44Z

Because isSmallScreenWidth (suggested in your solution) comes from the useWindowDimensions hook which we cannot use within action files like @libs/actions/Welcome.ts.

DylanDylann · 2024-03-19T16:35:19Z

@ikevin127 We can create a 3rd param in show function, named isSmallScreenWidth and then when calling show in BottomTabBar component, we can use useWindowDimensions to get the isSmallScreenWidth. I think it is very straightforward

ikevin127 · 2024-03-19T16:55:04Z

These details are missing from your proposal, but sure - no problem if you're gonna do it like that.
I'll keep an eye on the PR and will make sure to ask for that split if my solution's implementation will be used ✌

dukenv0307 · 2024-03-20T02:47:02Z

Based on this comment. proposal from @DylanDylann looks good to me cc @iwiznia

iwiznia · 2024-03-20T16:05:22Z

I am confused, isn't that proposal making it so that we do not navigate to the report?

ikevin127 · 2024-03-20T18:32:07Z

@iwiznia Yes it is, as per this issue's expected result.

To recap, the options here are either of the two:

Don't navigate to workapace report on first login on narrow layout devices.
Do navigate (current behaviour).

If we want to keep option 2, then this issue can be closed.

m-natarajan added the External Added to denote the issue can be worked on by a contributor label Oct 10, 2023

melvin-bot bot changed the title ~~Opens report instead of LHN on Native apps for new users when invited to a workspace~~ [$500] Opens report instead of LHN on Native apps for new users when invited to a workspace Oct 10, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 10, 2023

melvin-bot bot assigned 0xmiros Oct 10, 2023

melvin-bot bot added the Daily KSv2 label Oct 10, 2023

melvin-bot bot added the Overdue label Oct 16, 2023

melvin-bot bot removed the Overdue label Oct 18, 2023

melvin-bot bot added the Overdue label Oct 23, 2023

deomaius mentioned this issue Oct 25, 2023

[$500] Unavailable workspace is shown in LHN and header for a new user #29212

Closed

6 tasks

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Nov 3, 2023

melvin-bot bot removed the Overdue label Nov 3, 2023

melvin-bot bot removed the Overdue label Mar 18, 2024

melvin-bot bot assigned iwiznia Mar 18, 2024

zanyrenney added this to [#whatsnext] #wave-collect Mar 19, 2024

iwiznia closed this as completed Mar 20, 2024

github-project-automation bot moved this to Done in [#whatsnext] #wave-collect Mar 20, 2024

[$500] Opens report instead of LHN on Native apps for new users when invited to a workspace #29207

[$500] Opens report instead of LHN on Native apps for new users when invited to a workspace #29207

Comments

m-natarajan commented Oct 10, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 10, 2023

melvin-bot bot commented Oct 10, 2023

deomaius commented Oct 12, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

melvin-bot bot commented Oct 16, 2023

melvin-bot bot commented Oct 17, 2023

melvin-bot bot commented Oct 18, 2023

0xmiros commented Oct 18, 2023 • edited Loading

deomaius commented Oct 18, 2023 • edited Loading

deomaius commented Oct 23, 2023

melvin-bot bot commented Oct 24, 2023

deomaius commented Oct 24, 2023

melvin-bot bot commented Oct 25, 2023

melvin-bot bot commented Oct 27, 2023

melvin-bot bot commented Oct 31, 2023

melvin-bot bot commented Oct 31, 2023

melvin-bot bot commented Nov 3, 2023

deomaius commented Nov 4, 2023

melvin-bot bot commented Nov 7, 2023

melvin-bot bot commented Mar 18, 2024

zanyrenney commented Mar 18, 2024 • edited Loading

ikevin127 commented Mar 18, 2024

dukenv0307 commented Mar 18, 2024

ikevin127 commented Mar 18, 2024

iwiznia commented Mar 18, 2024

iwiznia commented Mar 18, 2024

ikevin127 commented Mar 18, 2024 • edited Loading

iwiznia commented Mar 18, 2024

ikevin127 commented Mar 18, 2024

iwiznia commented Mar 18, 2024

ikevin127 commented Mar 18, 2024

Updated proposal

DylanDylann commented Mar 19, 2024

iwiznia commented Mar 19, 2024

DylanDylann commented Mar 19, 2024 • edited Loading

iwiznia commented Mar 19, 2024

DylanDylann commented Mar 19, 2024

ikevin127 commented Mar 19, 2024

Reverted proposal

DylanDylann commented Mar 19, 2024

ikevin127 commented Mar 19, 2024

DylanDylann commented Mar 19, 2024

ikevin127 commented Mar 19, 2024

dukenv0307 commented Mar 20, 2024

iwiznia commented Mar 20, 2024

ikevin127 commented Mar 20, 2024

m-natarajan commented Oct 10, 2023 •

edited by melvin-bot bot

Loading

deomaius commented Oct 12, 2023 •

edited

Loading

0xmiros commented Oct 18, 2023 •

edited

Loading

deomaius commented Oct 18, 2023 •

edited

Loading

zanyrenney commented Mar 18, 2024 •

edited

Loading

ikevin127 commented Mar 18, 2024 •

edited

Loading

DylanDylann commented Mar 19, 2024 •

edited

Loading