[$500] [ANDROID APP] Notifications are displayed in the wrong order

[lanitochka17]

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 New Expensify app
2. Log in with account A
3. Minimize the application under account A
4. From another device under account B, send 10 messages (for example, numbers from 1 to 10) to the device with account A.
5. Check the order in which notifications are displayed on the device with account A

Expected Result:

Notifications are received and displayed in the correct order

Actual Result:

Notifications are displayed in the wrong order

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

Bug6189649_Record_Notifications_wrong_order.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Interanal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01e28fb1f0dbce38ca
• Upwork Job ID: 1699314253921251328
• Last Price Increase: 2023-09-13

[melvin-bot]

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

[jliexpensify]

Wow, yes can definitely reproduce!

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[spectusAkashIOS]

Proposal

Contributor details
Akash Patel ([email protected])
Upwork Profile Link: https://www.upwork.com/freelancers/~01481520e615cc06af

Checked the Video Recording attached with the issue.

First possible thing, when the notification is triggered on message sent, it is sending to the Server and then received on the target user's end.

Here, the possible issue can be, all the requests sending to the server in very short time, this will be handled asynchronously and then it will send the notifications to the target user in random order. [NOT IN THE ORDER SENDER SENT THE MESSAGES ORIGINALLY]

A fix can be, sending the notification to the server from Android app with a minor delay and try to handle the part of sending message in a synchronous way to avoid the re-ordering of tasks/notifications.

[melvin-bot]

📣 @spectusAkashIOS! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[saifali069]

Proposal

Problem Statement
The issue at hand pertains to the incorrect order of notifications in the Expensify app. When messages are sent from one user to another, the notifications are not being received in the expected chronological order. This disrupts the natural flow of communication within the app.

Root Cause Analysis
The root cause of this problem lies in the current notification handling process. Currently, all notification requests are sent to the server in rapid succession. As a result, the server processes these requests asynchronously, leading to notifications being delivered to the recipient in a seemingly random order. This behavior deviates from the sender's original message order.

Proposed Solution
To address this issue effectively, we recommend the following changes:

Introduce Delay: Modify the Android app to incorporate a minor delay when sending notifications to the server. By introducing this delay, we aim to mitigate the asynchronous handling of requests, ensuring that notifications are processed and delivered in the order they were originally sent by the sender.
Alternative Solutions (Optional)
While investigating this issue, we considered alternative solutions. However, after careful analysis, it was determined that introducing a delay in the notification sending process is the most practical and effective approach to resolving the problem. Other alternatives explored included modifying server-side processing and message queuing mechanisms, but these were deemed less efficient and more complex to implement.

[imranaalam]

---

Problem Statement:

Notifications from the Expensify app are not always displayed in the intended order on the client device. This can lead to a confusing user experience, as more recent notifications might appear before earlier ones.

---

Root Cause:

1. Lack of Sequential Identifiers: The current push notification handling mechanism does not consider the sequence of notifications. Notifications are processed as they arrive, and if there's any delay in the network or the processing of earlier notifications, later notifications might be displayed first.
2. Server-Side Ordering: It's uncertain if the server sends notifications in a strict sequence. Variabilities in how the server sends notifications or how they're routed through push services can affect the order in which they're received on the client side.

---

Proposed Solution:

Leverage a promise-based mechanism to ensure notifications are processed and displayed in the correct order:

1. Buffered Notifications: Store incoming notifications in a buffer.
2. Promise Queue: Queue notifications as tasks instead of processing them immediately. Each task will be a promise that processes a notification.
3. Processing Chain: Chain the promises to execute sequentially, ensuring the order of processing.
4. Error Handling: To prevent any error from disrupting the entire promise chain, integrate proper error handling using async/await with try/catch.

Code Implementation:

let bufferedNotifications = [];
let promiseChain = Promise.resolve();  // Start with a resolved promise as the base of the chain

function pushNotificationEventCallback(notificationEvent) {
    const { pushPayload } = notificationEvent;
    const timestampOrSeqNum = pushPayload.timestamp || pushPayload.sequenceNumber;

    // Check for duplicates
    if (bufferedNotifications.some(notif => notif.notification.id === pushPayload.id)) {
        return;  // Skip processing this notification
    }

    // Add the notification processing as a new link in the promise chain
    promiseChain = promiseChain.then(async () => {
        try {
            // Store the notification in the buffer
            bufferedNotifications.push({
                timestampOrSeqNum: timestampOrSeqNum,
                notification: pushPayload
            });

            // Sort the buffered notifications
            bufferedNotifications.sort((a, b) => a.timestampOrSeqNum - b.timestampOrSeqNum);

            // If the buffer gets too large, process and display the notifications
            if (bufferedNotifications.length > BUFFER_SIZE_LIMIT) {
                displayNotifications();
            }
        } catch (error) {
            console.error("Error processing the notification:", error);
        }
    });
}

function displayNotifications() {
    // Display notifications from the buffer
    console.log(bufferedNotifications);
    
    // Clear the buffer after display
    bufferedNotifications = [];
}

---

Alternative Solution:

1. Direct Handling with Timestamps/Sequence Numbers: Instead of buffering notifications, process them immediately as they arrive. Use their timestamps or sequence numbers to determine if they should be displayed immediately or held back waiting for any missing notifications. This approach minimizes memory usage but might introduce complexities in handling out-of-order arrivals.
2. Server-Side Adjustments: Collaborate with the backend team to ensure notifications are sent in a strict sequence. Implement server-side mechanisms to confirm delivery or resend missing notifications, potentially reducing the likelihood of out-of-order arrivals.
3. Custom Notification UI: Implement a custom in-app notification UI to have greater control over the order and display of notifications. Handle any out-of-order scenarios before presenting them to the user.

---

[melvin-bot]

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

[Ollyws]

Will review asap.

[melvin-bot]

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

[jliexpensify]

Bump @Ollyws for reviews!

[jliexpensify]

Bumping @Ollyws for reviews. If you're unable to review, let me know and I can reassign another C+.

[melvin-bot]

@Ollyws @jliexpensify 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!

[Ollyws]

@jliexpensify I think this one should probably be internal as it may require backend changes and also contributors or C+ can't test push notifications on iOS as far as I know (which would be required even though this is an android issue).

[melvin-bot]

Current assignee @Ollyws is eligible for the Internal assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to @cead22 (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[jliexpensify]

Hi @cead22 - hoping you could confirm that this is an Internal issue to be worked on? Thanks!

[cead22]

I think we should close this issue. Push notifications aren't guaranteed to be delivered in the order they're sent to apple/google. Moreover, this is an edge case that is unlikely to be experienced while using the app normally because usually there is some time from the moment you send a chat to the next.

Give that, this is more a feature request than a bug imo.

As an aside, it would be good if the video showed the full reproduction steps (cc @lanitochka17), and if we're just sending 1, 2, 3, ... in full speed, then it reinforces that this is an edge case that isn't reflective of the normal use of the app.

Please reopen if you disagree, including the video showing the full reproduction steps

[mvtglobally]

Issue not reproducible during KI retests. (First week)