Hybrid app - Getting duplicate notifications for the Expensify statement

[m-natarajan]

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

---

Version Number: Reproducible in staging?: Needs Reproduction Reproducible in production?: Needs Reproduction If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: 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: @AndrewGable Slack conversation (hyperlinked to channel name): #expensify-bugs

Action Performed:

1. Login to the HybridApp.
2. Set up and enable Expensify statement notifications.

Expected Result:

One notification from Concierge regarding statement

Actual Result:

Duplicate notifications received for same month

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

[melvin-bot[bot]]

Triggered auto assignment to @jliexpensify (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.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[melvin-bot[bot]]

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

[jliexpensify]

Sorry, the weekend then was OOO so I missed this. I don't have an iOS device so going to post in the #quality (?) channel. I'm thinking it might be Internal.

[jliexpensify]

Posted in #quality

[jliexpensify]

Not overdue, just added labels

[melvin-bot[bot]]

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

[jliexpensify]

Waiting on an internal engineer

[melvin-bot[bot]]

@jliexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[jliexpensify]

Still waiting for someone to pick this up

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[jliexpensify]

Waiting for an internal engineer.

[muttmuure]

Does this happen when you're logged into NewApp and HybridApp at the same time? Or just HybridApp?

[AndrewGable]

HybridApp only

[jliexpensify]

Internal, waiting on an engineer

[AndrewGable]

Still happening BTW

[arosiclair]

I'll look into this when I get some cycles

[arosiclair]

I spent some time on this yesterday. I should be able to reproduce this in dev by just running the ExpensifyWalletRegulatoryEmails:

CreateJob
name: www-prod/ExpensifyWalletRegulatoryEmails
data: {"debugEmail": "[email protected]"}

I wasn't able to reproduce with NewDot so I may need to test with the HybridApp specifically.

[arosiclair]

Looks like this is affecting most old push notifications:

[AndrewGable]

Agree - I think I saw it on most of them

[arosiclair]

No progress on this. HybridApp and push notification dev aren't in a good spot right now so this probably has to wait until that gets better.

[arosiclair]

Alright I was able to reproduce this issue with unit tests. The root problem is in the logic for sendReportCommentNotifications. For concierge chats, both $shouldSendToNewDot and $shouldSendToOldDot are true so we end up sending push notifications twice.

With the HybridApp in play, I don't think choosing the destination/payload based on the report type makes sense anymore so instead this should probably be completely controlled by the tryNewDot NVP. So I think I'll update sendPushNotification to take both the old and new payloads as params and then it can choose which to use accordingly.

[arosiclair]

Posted the PR

[arosiclair]

Duplicate Concierge push notifications are now fixed but this is still happening since those aren't Concierge messages. I'll look into it soon.

[arosiclair]

I wasn't able to reproduce the duplicate transaction notifications using unit tests so it looks unrelated to MobilePushNotifications logic. Instead, I think there's probably an issue with TransactionNotifications::sendNotification. We might be accidentally calling that twice in response to Marqeta webhook notifications. cc @madmax330 in case you have any ideas.

[arosiclair]

Since this is a separate issue, I reported it in #quality here so we can open a new GH and get it re-prioritized. Closing this out