[HOLD for payment 2023-07-06] Android- Chat- The user is not navigated to the chat screen after opening chat from notification

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Close the NewDot app
2. Wait for ~1 minute
3. Send a message from B to A. Push notification should appear
4. Double-tap the notification. It should navigate the user to the correct chat, and the new message should be available.

Expected Result:

The user is expecting that the app opens and then the user is redirected to the chat screen in one-on-one chat bar

Actual Result:

The user is been sent to the homepage of the app when the app is opened through the chat message received from the notification bar

Workaround:

Unknown

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.29.0

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

Notes/Photos/Videos: Any additional supporting documentation

Bug6095699_mobizen_20230616_150002.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team / @Victor-Nyagudi

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0169b8f4b5af31c852
• Upwork Job ID: 1671205349345284096
• Last Price Increase: 2023-06-20

[melvin-bot]

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

[melvin-bot]

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

[arosiclair]

This regression reached prod but should have been caught in regression tests. Giving Applause the benefit of the doubt, I think this is what happened:

Maybe they are clicking the notification with the chat already open. Right now, clicking the notification just opens the app and doesn’t navigate. So if they already had the chat open, it’d seem like it worked correctly

[arosiclair]

Seems that it works while the app is alive, but not while killed:

screen-20230620-131107_2.mp4

I'll look into this

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @mollfpr (Internal)

[arosiclair]

As suspected this was caused by the navigation reboot. Specifically, we're no longer waiting for the NavigationContainer to be ready. Additionally, we're still making a call to Navigation.setDidTapNotification() which does not exist and causes an error (source).

There's a few thing we need to do here:

• Fix the bug
• Add logging for notification click handling so we can better debug this in the future
• Update regression test steps to select a different chat before testing notification click through and also to test click through with the app killed

[melvin-bot]

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

[arosiclair]

Found a fix and posted a PR. Also pinged some knowledgeable navigation folks

[arosiclair]

First fix broke initial app startup (boot splash was never dismissed). Just found a better fix for an error when checking the active route during startup. Now out for review!

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix push notification click through #21257

If no regressions arise, payment will be issued on 2023-07-06. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
• [@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@mollfpr] Determine if we should create a regression test for this bug.
• [@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[arosiclair]

@mollfpr I think we should propose the regression test steps I wrote on the PR. We should update these QA steps specifically so that there's another section for testing click through after the app is killed. Thoughts?

[mollfpr]

Yup, sounds good to me. I guess we only need to add a new section to test on the killed app(?)

[arosiclair]

Yeah that and also lets add a step to navigate to the Concierge chat before clicking the notification in each section. Like how I wrote my steps. That will make sure the notification isn't only just re-opening the app with the chat already open.

[arosiclair]

@mollfpr can you complete the checklist? Getting those regression tests updated is a very important improvement for this

[mollfpr]

Sorry, I'm running out of time 🙏

I'll complete the checklist in the morning.

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
[@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

Thanks to @arosiclair explain here #21120 (comment)

[@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

The regression steps should be enough.

[@mollfpr] Determine if we should create a regression test for this bug.
[@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

As suggested by @arosiclair, we should add the new test case here https://expensify.testrail.io/index.php?/cases/view/1971003&group_by=cases:section_id&group_order=asc&display_deleted_cases=0

Notification click through (Android & iOS only)

1. Log into NewDot
2. Navigate to the Concierge chat
3. Send yourself a message
4. Verify a notification for the message is received
5. Click the notification
6. Verify the app navigates to the chat for the message

Notification click-through while the app is killed (Android & iOS only)

1. Navigate back to the Concierge chat
2. Kill the app by swiping it away in the recent apps view
3. Send yourself another message
4. Verify a notification for the message is received
5. Click the message
6. Verify the app opens and navigates to the chat for the message

[Victor-Nyagudi]

@mollfpr The payment date for this issue is July 6th, but I haven't received an offer yet for the reporting bonus.

Cc: @adelekennedy since you're also assigned here in case you're the one I should be asking.

[mollfpr]

@Victor-Nyagudi BZ team will take care of the payment, which in this issue is @adelekennedy

[Victor-Nyagudi]

Bump @adelekennedy .

I see @MelvinBot also added the Overdue label.

[adelekennedy]

was ooo Friday as I was sick and then the weekend.

@Victor-Nyagudi $250 reporting
@mollfpr 1k for PR review

will you apply here?

[Victor-Nyagudi]

Sorry to hear that, @adelekennedy. Hope you're feeling better now.

I've applied on Upwork.

[mollfpr]

Applied, thank you @adelekennedy