Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.33k stars 2.76k forks source link

[$250] Android - Room - When 2nd user suggestions whisper triggered, 1st whisper disappeared #47255

Open lanitochka17 opened 1 month ago

lanitochka17 commented 1 month ago

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: 9.0.19 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch app
  2. Tap on a room chat
  3. Enter @jaihanumanblog@gmail.com & send the message
  4. Note whisper is shown
  5. Enter @vincenzoddragon@gmail.com & send the message
  6. Note whisper for step 5 user shown but step 4 whisper disappeared.
  7. Navigate to troubleshoot-- clear cache
  8. Revisit the room chat
  9. Now note 2 whisper message displayed

Expected Result:

When 2nd user suggestions whisper triggered, 1st whisper must not be disappeared

Actual Result:

When 2nd user suggestions whisper triggered, 1st whisper disappeared

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/6496f93c-4c6e-4472-a32d-7462cd02da97

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01510cc10b7d91bb22
  • Upwork Job ID: 1823376178599453123
  • Last Price Increase: 2024-08-27
  • Automatic offers:
    • akinwale | Reviewer | 103711687
melvin-bot[bot] commented 1 month ago

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

lanitochka17 commented 1 month ago

@sonialiap FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

sonialiap commented 1 month ago

Sounds like a bug to me

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~01510cc10b7d91bb22

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

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

sonialiap commented 1 month ago

@stephanieelliott I'm OOO Aug 19-30, adding leave buddy. Status: waiting for proposals

melvin-bot[bot] commented 3 weeks ago

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

akinwale commented 3 weeks ago

Still waiting for proposals.

melvin-bot[bot] commented 3 weeks ago

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

burczu commented 3 weeks ago

Hi, I’m Bartek from Callstack and I would like to work on this issue.

melvin-bot[bot] commented 3 weeks ago

@akinwale, @sonialiap, @stephanieelliott Whoops! This issue is 2 days overdue. Let's get this updated quick!

VincentCorleone commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-08-25 05:22:31 UTC.

Proposal

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

After mentioning a user who are not in the cureent room the second time, previous ACTIONABLEMENTIONWHISPER message disappear.

What is the root cause of that problem?

Short Answer

After mentioning a user who is not in the current room the second time, the browser requests the addComment api to the backend, then the backend gives back an inappropriate response that contains a merge command that will delete the previous ACTIONABLEMENTIONWHISPER message.

Long Answer

Let's observe three JSON segments first.

Onyx data after update from the previous response(Unrelevant fields are omitted):

{
    "reportActions_3703223916129889": {
        "297903893454568668": {
            "actionName": "ACTIONABLEMENTIONWHISPER",
            "message": [
                {
                    "deleted": "",
                    "html": "Heads up, <mention-user accountID=15075387></mention-user> isn't a member of this room.",
                    "isDeletedParentAction": false,
                    "isEdited": false,
                    "text": "Heads up,  isn't a member of this room.",
                    "type": "COMMENT",
                    "whisperedTo": [
                        17159591
                    ]
                }
            ],
        }
    }
}

Onyx data after update from the previous response(Unrelevant fields are omitted):

{
    "reportActions_3703223916129889": {
        "297903893454568668": {
            "actionName": "ACTIONABLEMENTIONWHISPER",
            "message": [
                {
                    "deleted": "2024-08-23 17:15:21.188",
                    "type": "TEXT"
                }
            ],
        }
    }
}

Onyx data after clearing cache and refresh(Unrelevant fields are omitted):

{
    "reportActions_3703223916129889": {
        "297903893454568668": {
            "message": [
                {
                    "type": "COMMENT",
                    "html": "Heads up, <mention-user accountID=15075387></mention-user> isn't a member of this room.",
                    "text": "Heads up,  isn't a member of this room.",
                    "isEdited": false,
                    "whisperedTo": [
                        17159591
                    ],
                    "isDeletedParentAction": false,
                    "deleted": ""
                }
            ],
            "actionName": "ACTIONABLEMENTIONWHISPER"
        }
    }
}

We can see in Onyx data after updating from the previous response, reportActions_3703223916129889.297903893454568668.message.deleted is modified to a specific timestamp from an empty string and reportActions_3703223916129889.297903893454568668.message lost detail message to show in the UI layer. After my investigation, I got that ReportActionsUtils.isDeletedAction(reportAction) equals false when deleted is set to an empty string and ReportActionsUtils.isDeletedAction(reportAction) equals true when deleted is set to a specific time stamp. That ReportActionsUtils.isDeletedAction(reportAction) equals false will result in this report action not showing in the UI layer.

What makes the modification? It's the second response of the addComment api. Let's compare the first and the second response of the addComment api. The difference is here. We can see onyxData[0] makes the modification.

So the root cause is the inappropriate response of addComment api from the backend.

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

Ask the backend developer to remove the logic that generates onyxData[0] in the second response of the addComment api.

What alternative solutions did you explore? (Optional)

The best solution is to modify the backend. A worse solution is to block all the merge commands that will delete the previous ACTIONABLEMENTIONWHISPER message in the addComment API response. But the worse solution can prove my answer is corrent at least.

The worse solution is here:

Between Line15 and 16: https://github.com/Expensify/App/blob/42bed64c6a6d36bca317812d2a570a850f5ecc86/src/libs/Middleware/SaveResponseInOnyx.ts#L15-L23 Add the codes below:

        if(request.command == "AddComment"){
            const toRemove = response?.onyxData?.find((item)=> {
                if(item.key.startsWith("reportActions_")){
                    const tmpObj: { [key: string]: any } = item.value as Object;
                    return Object.keys(tmpObj).find((key)=> tmpObj[key].message[0]?.deleted!=='');
                }else{
                    return false;
                }

            });
            if(toRemove){
                const index = response?.onyxData?.indexOf(toRemove);
                if (index !== -1) {
                    response?.onyxData?.splice(Number(index), 1);
                }
            }
        }

I have tested that this modification will solve this issue.

melvin-bot[bot] commented 2 weeks ago

@akinwale @sonialiap @stephanieelliott 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!

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

@akinwale, @sonialiap, @stephanieelliott Still overdue 6 days?! Let's take care of this!

stephanieelliott commented 2 weeks ago

Sounds good @burczu, will assign you!

melvin-bot[bot] commented 2 weeks ago

📣 @akinwale 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

burczu commented 2 weeks ago

@akinwale Please take a look at the proposal from @VincentCorleone - I've requested to be assigned to this issue as a contributor from the agency, but if the proposal is legit, I think it's better to assign its author.

stephanieelliott commented 2 weeks ago

Hey @akinwale bump on the above ^^, can you review the proposal for us?

akinwale commented 2 weeks ago

After taking a look, the proposal suggests that this ideally should be fixed in the BE. The alternative approach is more of a workaround which isn't suitable, so we're looking for a better solution on the frontend, if viable.

cc @stephanieelliott @burczu

burczu commented 1 week ago

@akinwale I've done my research and I agree with the @VincentCorleone proposal, that this is the backend issue and we should ask someone from the backend team to take a look at this.

This part of the AddComment API response removes the previous whisper indeed:

Screenshot 2024-09-02 at 13 41 17

melvin-bot[bot] commented 1 week ago

@akinwale, @burczu, @sonialiap, @stephanieelliott Whoops! This issue is 2 days overdue. Let's get this updated quick!

sonialiap commented 1 week ago

Sounds like both @akinwale and @burczu agree that this should be an internal issue 👍 ? If yes, I'll add the internal label

melvin-bot[bot] commented 1 week ago

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

akinwale commented 1 week ago

Sounds like both @akinwale and @burczu agree that this should be an internal issue 👍 ? If yes, I'll add the internal label

Yes, this can be made internal.

melvin-bot[bot] commented 6 days ago

@akinwale @burczu @sonialiap this issue is now 4 weeks old, please consider:

Thanks!

sonialiap commented 5 days ago

Moved to internal

melvin-bot[bot] commented 4 days ago

@akinwale, @burczu, @sonialiap Whoops! This issue is 2 days overdue. Let's get this updated quick!

sonialiap commented 4 days ago

VSB is on hold so internal issues aren't being picked up unless it's an important bug. I don't think this issue is critical but would be good to fix, so keeping it open but moving to weekly