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

[$2000] Back from user details opens group chat instead of group details on android chrome #23935

Closed
1 of 6 tasks
kavimuru opened this issue Jul 31, 2023 · 27 comments
Closed
1 of 6 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

Comments

@kavimuru
Copy link

kavimuru commented Jul 31, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open the app in android chrome
  2. Open any group report
  3. Click on header to open details page
  4. Click on any user
  5. Click on "message %user%"
  6. Click back button in app and observe that it open group chat instead of group details page as we do on web chrome

Expected Result:

App should open group details page on back from user report if we open user report from group details page

Actual Result:

App opens group report instead of group details page on back from user report if we open user report from group details page on android chrome

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.47-3
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation

android.chrome.wrong.back.user.details.mp4
az_recorder_20230731_122300.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690437966940519

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d267fead81e68c17
  • Upwork Job ID: 1687163869748379648
  • Last Price Increase: 2023-08-15
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 31, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

Triggered auto assignment to @michaelhaxhiu (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Jul 31, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@kavimuru kavimuru changed the title Inconsistency bug: back from user details opens group chat instead of group details on android chrome Back from user details opens group chat instead of group details on android chrome Jul 31, 2023
@ygshbht
Copy link
Contributor

ygshbht commented Jul 31, 2023

Proposal

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

The issue at hand involves incorrect navigation behavior in Android Chrome: the application returns to the group chat instead of the group details page after messaging a user from the details page.

What is the root cause of that problem?

The root cause of this problem lies in the navigateToAndOpenReportWithAccountIDs function. When this function is called upon clicking the 'message' button, it dismisses the current Right Hand Navigation (RHN) screen, causing it to be wiped from the navigation history. As a result, when the back button is pressed, the application doesn't navigate back to the group details page (as it's no longer in the history stack), and instead opens the group chat.

function navigateToAndOpenReportWithAccountIDs(participantAccountIDs) {
let newChat = {};
const chat = ReportUtils.getChatByParticipants(participantAccountIDs);
if (!chat) {
newChat = ReportUtils.buildOptimisticChatReport(participantAccountIDs);
}
const reportID = chat ? chat.reportID : newChat.reportID;
// We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
openReport(reportID, [], newChat, '0', false, participantAccountIDs);
Navigation.dismissModal(reportID);
}

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

To fix this issue, we should modify the navigateToAndOpenReportWithAccountIDs function to navigate to the new report route instead of dismissing it. This way, the RHN screen (group details page in this case) will be kept in the navigation history and we can navigate back to it when the back button is pressed. Here's the proposed code modification:

function navigateToAndOpenReportWithAccountIDs(participantAccountIDs) {
    let newChat = {};
    const chat = ReportUtils.getChatByParticipants(participantAccountIDs);
    if (!chat) {
        newChat = ReportUtils.buildOptimisticChatReport(participantAccountIDs);
    }
    const reportID = chat ? chat.reportID : newChat.reportID;

    // We want to pass newChat here because if anything is passed in that param (even an existing chat), we will try to create a chat on the server
    openReport(reportID, [], newChat, '0', false, participantAccountIDs);
    Navigation.navigate(ROUTES.getReportRoute(reportID));
}

This fix, however, will highlight another issue: the inability to select the Left Hand Navigation (LHN) after the RHN is closed, due to an improperly positioned 'nodropzone' element. This will require a separate fix. The root cause of this issue should be investigated, and the position or behavior of the 'nodropzone' element should be adjusted accordingly to allow proper interaction with the LHN after the RHN is closed.
#23885
#23881

@jeet-dhandha
Copy link
Contributor

jeet-dhandha commented Jul 31, 2023

Proposal

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

  • On back from second report screen the profile page is not opened instead the first chat screen is opened.

What is the root cause of that problem?

  • The modal is dismissed and then we are redirected to the second chat screen thus updating the root state to:
    • Home
    • CentralPanNavigator - (first chat)
    • CentralPanNavigator - (secon chat)

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

  1. We should add two params in navigateToAndOpenReportWithAccountIDs function

/**
* This will find an existing chat, or create a new one if none exists, for the given accountID or set of accountIDs. It will then navigate to this chat.
*
* @param {Array} participantAccountIDs of user logins to start a chat report with.
*/
function navigateToAndOpenReportWithAccountIDs(participantAccountIDs) {

Params:

  • shouldDismissModal - to dismiss the modal
  • pushToNavigationState - to push the navigation state to the root state
  1. Then we update the navigation code as based on above params.
  2. We add code to PUSH report to the navigation state if pushToNavigationState is true. (Refer below code if needed).
Code Changes
function navigateToAndOpenReportWithAccountIDs(participantAccountIDs, shouldDismissModal = true, pushToNavigationState = false) {
    ...rest code
    if (shouldDismissModal) {
        Navigation.dismissModal(reportID);
    }

    if (pushToNavigationState) {
        Navigation.navigate(ROUTES.getReportRoute(reportID), 'PUSH');
    }
}
  1. Now we create a isSmallDevice constant which is basically this conditions Browser.isMobile() || isSmallScreenWidth and use it to decide whether to push the report to navigation state or not.

  2. Finally we update Report.navigateToAndOpenReportWithAccountIDs([accountID]) to Report.navigateToAndOpenReportWithAccountIDs([accountID], !isSmallDevice, isSmallDevice) in ProfilePage.js File.

What alternative solutions did you explore? (Optional)

  • N/A

@ygshbht
Copy link
Contributor

ygshbht commented Jul 31, 2023

This fix, however, will highlight another issue: the inability to select the Left Hand Navigation (LHN) after the RHN is closed, due to an improperly positioned 'nodropzone' element. This will require a separate fix. The root cause of this issue should be investigated, and the position or behavior of the 'nodropzone' element should be adjusted accordingly to allow proper interaction with the LHN after the RHN is closed.
#23885
#23881

The issue mentioned at the bottom might be fixed by this PR

@melvin-bot melvin-bot bot added the Overdue label Aug 2, 2023
@michaelhaxhiu
Copy link
Contributor

Hm I can't reproduce this one on my android (pixel / android / google chrome / staging).

@melvin-bot melvin-bot bot removed the Overdue label Aug 3, 2023
@michaelhaxhiu
Copy link
Contributor

screen-20230803-134952.mp4

Here's a recording. Am I missing a step here @dhanashree-sawant?

@michaelhaxhiu michaelhaxhiu added the Needs Reproduction Reproducible steps needed label Aug 3, 2023
@michaelhaxhiu
Copy link
Contributor

I'm leaning toward closing this one as-is until we get more clarity around the steps to take.

@ygshbht
Copy link
Contributor

ygshbht commented Aug 3, 2023

@michaelhaxhiu Yeah, you forgot to click the "Message user" button
Step 4

  1. Open the app in android chrome
  2. Open any group report
  3. Click on header to open details page
  4. Click on any user and click on message
  5. Click back button in app and observe that it open group chat instead of group details page as we do on web chrome

@dhanashree-sawant
Copy link

Hi @michaelhaxhiu, yes you forgot to click on 'Message user' after opening user details

@michaelhaxhiu michaelhaxhiu added External Added to denote the issue can be worked on by a contributor and removed Needs Reproduction Reproducible steps needed labels Aug 3, 2023
@melvin-bot melvin-bot bot changed the title Back from user details opens group chat instead of group details on android chrome [$1000] Back from user details opens group chat instead of group details on android chrome Aug 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 3, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

Current assignee @michaelhaxhiu is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

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

@michaelhaxhiu
Copy link
Contributor

Just tested again thanks for clarifying, I missed that as it was like 2 steps in 1. I just updated the reproduction steps for clarity 👍

@michaelhaxhiu michaelhaxhiu removed the Bug Something is broken. Auto assigns a BugZero manager. label Aug 4, 2023
@michaelhaxhiu michaelhaxhiu removed their assignment Aug 4, 2023
@michaelhaxhiu michaelhaxhiu added the Bug Something is broken. Auto assigns a BugZero manager. label Aug 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 4, 2023

Triggered auto assignment to @jliexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@Expensify Expensify deleted a comment from melvin-bot bot Aug 4, 2023
@michaelhaxhiu michaelhaxhiu self-assigned this Aug 4, 2023
@michaelhaxhiu
Copy link
Contributor

Note: I'm preparing to go OOO for ~2 weeks and need a BZ buddy to watch over this in the meantime. 🙏

Next step: Please ensure that we push this forward and double price if no winning proposal in the next week (by 8/11).

Thanks in advance @jliexpensify, I can take this back when I get back if it's not complete before then.

@melvin-bot melvin-bot bot added the Overdue label Aug 7, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 7, 2023

@michaelhaxhiu, @jliexpensify, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

@jliexpensify
Copy link
Contributor

Not overdue!

@melvin-bot melvin-bot bot removed the Overdue label Aug 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 10, 2023

📣 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 Aug 10, 2023
@jliexpensify
Copy link
Contributor

Not overdue, waiting on proposals

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Aug 11, 2023
@jliexpensify
Copy link
Contributor

No proposals yet, going to double this.

@melvin-bot melvin-bot bot removed the Overdue label Aug 15, 2023
@jliexpensify jliexpensify changed the title [$1000] Back from user details opens group chat instead of group details on android chrome [$2000] Back from user details opens group chat instead of group details on android chrome Aug 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 15, 2023

Upwork job price has been updated to $2000

@himanshuragi456
Copy link
Contributor

@jliexpensify I see no comments/reply is given to the current 2 proposals, are you sure those are not working/not acceptable?
note: asking because I gave good efforts on an issue once but then an already existing proposal(that was posted before price doubling) was accepted.

@aimane-chnaif
Copy link
Contributor

I think this should have been discussed while navigation refactor.
@mountiny can you please confirm that it's intentional to close all RHP modals (on mobile) when open DM chat by clicking "Message xxx" on profile page?

@aimane-chnaif
Copy link
Contributor

aimane-chnaif commented Aug 15, 2023

So the issue here is inconsistency - browser (or android device) back button goes to previous page (even RHP modal) correctly, while app back button goes to old screen skipping recent RHP modal pages.

@mountiny
Copy link
Contributor

@mountiny can you please confirm that it's intentional to close all RHP modals (on mobile) when open DM chat by clicking "Message xxx" on profile page?

I think this is fine and intentional to make sure that the LHN is closer in the stack

I am a little bit confused by this issue and honestly it feels so minor we could close this out. The mWeb / native discrepancies are hard

@jliexpensify
Copy link
Contributor

@himanshuragi456 Hi there - it's the C+ that will consider proposals, I'm just here to push the issue along and issue payments!

Ok cool, going to close this one then - based of Vit's comment:

I am a little bit confused by this issue and honestly it feels so minor we could close this out. The mWeb / native discrepancies are hard

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
Projects
None yet
Development

No branches or pull requests

9 participants