[Ready for C+ payment via Newdot][$1000] Web - Chat - The flags list are still open even when the attachment was deleted from sender's side

[kbecciv]

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. Go to staging dot on web chrome and login with User A
2. Go to a chat with User B and send an attachment
3. Login with User B on another web chrome (Place the window side by side to make work easier)
4. Now from User B, click on offensive message icon and notice that a list of flags appears (do not close it)
5. From User A account, delete the attachment
6. On User B side notice that even when the attachment are already deleted from User A side , the flags menu is still opened, and if you click on bullying flag, it shows shows a warning message even for an empty attachment message.

Expected Result:

The flags list should not be open when the attachment was deleted from the sender's side

Actual Result:

The flags list are still open even when the attachment was deleted from sender's side

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

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

error-2023-06-08_11.27.23.mp4

Recording.3088.mp4

Expensify/Expensify Issue URL:

Issue reported by: @priya-zha

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686207956963489

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0127efb9c7f0ca6faa
• Upwork Job ID: 1668865351377502208
• Last Price Increase: 2023-06-21

[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

[hungvu193]

Proposal

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

Web - Chat - The flags list are still open even when the attachment was deleted from sender's side

What is the root cause of that problem?

We are not checking if the current reportAction is deleted then hide the FlagCommentPage.

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

So we can add a check inside FlagCommentPage to dismiss the modal when reportAction is deleted.
We also need to check if we are flagging the first message if thread because reportAction in that case will be undefined.

    React.useEffect(() => {
        let reportToCheck;
        if (reportAction === undefined) {
            reportToCheck = ReportActionsUtils.getParentReportAction(props.report);
        } else {
            reportToCheck = reportAction
        }
        if (ReportActionsUtils.isDeletedAction(reportToCheck)) {
            Navigation.dismissModal();
        }
    }, [reportAction, props.report]);

What alternative solutions did you explore? (Optional)

If we still want to keep the FlagCommentPage visible when comment is deleted, we should add a check when user flag that comment, if it's deleted, we should do nothing and dismiss the modal, so we won't see the flag message from Concierge, so we should update our flagComment function:

    const flagComment = (severity) => {
        let reportID = getReportID(props.route);

        // Handle threads if needed
        if (reportAction === undefined || reportAction.reportActionID === undefined) {
            reportID = ReportUtils.getParentReport(props.report).reportID;
            reportAction = ReportActionsUtils.getParentReportAction(props.report);
        }
        if (!ReportUtils.isDeletedAction(reportAction) {
        Report.flagComment(reportID, reportAction, severity);       
       }
        Navigation.dismissModal();
    };

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

[hungvu193]

Updated my proposal to use ReportActionUtils #20674 (comment)

[kadiealexander]

Reproduced:

2023-06-14_18-17-01.mp4

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0127efb9c7f0ca6faa

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @francoisl (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dukenv0307]

Proposal

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

The flags list still opens even when the message was deleted from sender's side

What is the root cause of that problem?

Actually we don't check if the report action is deleted, we will hide flag comment page

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

We could create a useEffect to check if reportAction is deleted, we will hide flag comment page. To check reportAction is deleted or not we can use ReportActionsUtils.isDeletedAction function.

With this check above, we should move this logic here to before useEffect to get correct reportAction before check it is deleted or not in useEffect because if we don't move this logic when we flag the message that is the first chat of thread, reportAction is undefined and then the modal will dismiss by the check in useEffect.

App/src/pages/FlagCommentPage.js

Lines 118 to 122 in 23978e4

	// Handle threads if needed
	if (reportAction === undefined) {
	reportID = ReportUtils.getParentReport(props.report).reportID;
	reportAction = ReportActionsUtils.getParentReportAction(props.report);
	}

let reportAction = props.reportActions[`${props.route.params.reportActionID.toString()}`];

// Handle threads if needed
if (reportAction === undefined) {
    reportAction = ReportActionsUtils.getParentReportAction(props.report);
}

React.useEffect(() => {
    if (ReportActionsUtils.isDeletedAction(reportAction)) {
         Navigation.dismissModal();
    }
 }, [reportAction]);

What alternative solutions did you explore? (Optional)

If we want to keep opening the flag page when the message is deleted, in flagComment function we can check if the message is deleted, then we do nothing and just dismiss modal.

let reportID = getReportID(props.route);

// Handle threads if needed
if (reportAction === undefined || reportAction.reportActionID === undefined) {
    reportID = ReportUtils.getParentReport(props.report).reportID;
    reportAction = ReportActionsUtils.getParentReportAction(props.report);
}
 if (!ReportActionsUtils.isDeletedAction(reportAction)) {
   Report.flagComment(reportID, reportAction, severity);
}
Navigation.dismissModal();

[spcheema]

Proposed solution

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

The flags list remains open when an attachment is deleted from sender side

What is the root cause of that problem?

There is no check to verify if a report is deleted or not

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

We need to add a check for deleted report inside the flagComment method and use useEffect to trigger close model in the case of report is deleted

 const flagComment = (severity) => {
        let reportID = getReportID(props.route);

        // Handle threads if needed
        if (reportAction === undefined) {
            reportID = ReportUtils.getParentReport(props.report).reportID;
            reportAction = ReportActionsUtils.getParentReportAction(props.report);
        }

        React.useEffect(() => {
            if (ReportActionsUtils.isDeletedAction(reportAction)) {
                Navigation.dismissModal();
            }
        }, [reportAction]);
        Report.flagComment(reportID, reportAction, severity);
        Navigation.dismissModal();
    };

What alternative solutions did you explore? (Optional)

NA

[francoisl]

I don't know if this is a bug - at least with how it works now, the potentially offensive message will still be moderated.

cc @dangrous if you have an opinion, since I know you worked on moderation recently.

[dangrous]

Huh, this is an interesting case. I think we wouldn't want the pane to automatically close because I think that would be confusing for the person who was using it.

My only question (I have not tested this), is there an error if the flagger chooses an option in this case, or does just nothing happen? If nothing happens, I think we can close this bug. If there's an error, we probably want to update it to notify the flagger that the comment no longer exists, and hide the error.

[dukenv0307]

@dangrous I've tested, the flagger chooses an option in this case, the message deleted will display with reveal button like this video and after click on this button it display an empty message. So I think we should fix this bug and if expected behavior is display not found page we can wrap this page by FullPageNotFound with shouldShow props is ReportActionsUtils.isDeletedAction(reportAction)

Screencast.from.17-06-2023.01.07.25.webm

[dangrous]

Ah got it. @francoisl do you think this is a backend issue? Once the message is fully deleted, shouldn't the report action be removed fully from the front end, and so this wouldn't occur?

[francoisl]

Once the message is fully deleted, shouldn't the report action be removed fully from the front end

Good point, I wasn't sure so I asked internally, and so basically we could make that change in the backend but it might not be super obvious as there would be other things to take into account like the Unread message marker for example.

I'm thinking for now, we can probably do one of the front-end solutions proposed initially, to hide the hide panel. That said, both proposals define the useEffect() inside the function flagComment, which AFAIK they shouldn't.

[dangrous]

Ah yeah, I guess I was combining two things. To clarify (mostly for me):

• The bug listed in this particular issue - I'm not sure we need to fix, as long as it doesn't break things elsewhere. I think it would be more confusing for the flagger for that pane to suddenly close if the author deleted their comment. I'd prefer it to stay open, allow the flagger to flag, and then just do nothing.
• This is a different bug, which is more related to my question here.

[melvin-bot]

📣 @dukenv0307 You have been assigned to this job by @francoisl!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[dukenv0307]

@Santhosh-Sellavel The PR is ready to review.

[dukenv0307]

@dangrous @francoisl I have a recent PR that display not found page if the reportAction cannot flag. But in this, we only test for the case reportAction of the current user and didn't pay attention to the case of this issue. And when @Santhosh-Sellavel ping me here #21658 (comment), I just found the cause in this comment #21658 (comment).

So we need to confirm what should we do now in this issue:

1. Close the PR and accept the expected behavior is not found page will display if reportAction is deleted.
2. Remove page not found view and move the logic that we use to display not found page here into flagComment function and check if it is true, do nothing and only dismiss the modal.

App/src/pages/FlagCommentPage.js

Line 158 in 0bbf3fc

<FullPageNotFoundView shouldShow={!shouldShowLoading && !ReportUtils.shouldShowFlagComment(reportAction, props.report)}>

[dangrous]

I'll defer to you @francoisl!

For me, I think we could potentially keep both, since they're a bit different - I'd still prefer the flag comment page doesn't disappear in the middle of a user looking at it, but would otherwise show not found if it was clicked AFTER something was deleted. I'd also be okay with one of @dukenv0307's suggestions, probably the first one, but I think that would be my second choice. This is all definitely personal opinion though.

[Santhosh-Sellavel]

I'm confused here, I'll just wait for @francoisl!

[francoisl]

Hmm yeah not ideal. @dukenv0307 can you think of a way to update your current PR so that it doesn't switch to the FullPageNotFoundView if the flag panel was already open before the comment got deleted, but without breaking your previous changes from #21771?

[dukenv0307]

@francoisl

• We want to just keep it for now and will display not found page if reload or always keep show the page for the deleted reportAction.

• If we want to keep it we can remove the check !ReportActionsUtils.isDeletedAction(reportAction) in canFlagComment function. I think it fine because the deleted action will be hidden and canFlagComment still work as well to display flag icon or not.

• And what should we do if we keep showing page and choose an option? That is still does nothing and only dismisses modal or other.

[francoisl]

We want to just keep it for now and will display not found page if reload

Yes ^

If we want to keep it we can remove the check !ReportActionsUtils.isDeletedAction(reportAction) in canFlagComment function. I think it fine because the deleted action will be hidden and canFlagComment still work as well to display flag icon or not.

That would sorta work but I think that solution has a disadvantage: if you reload the page while the flag page is open, we would still show the flag options for the deleted message.

I can't think of a solution that would basically act as follows:

• if the flag page is already open when the message gets deleted, then keep it open and do nothing when an option is chosen
• if the user reloads the page (or opens a /flag/{reportID}/{reportActionID} link) after the message got deleted, then show the PageNotFound

So taking all that into consideration, I'd say the next best option is to close this issue and the PR, and accept that the current behavior is expected.

[dukenv0307]

@Santhosh-Sellavel Base on @francoisl 's comment #20674 (comment). I think we can close the PR.

[francoisl]

Based on the 👍 reactions it seems like everyone agrees, so I'm going to close the issue and the PR. Thanks for the proposals and working with through together 🙌

[dukenv0307]

@francoisl I think this issue is eligible for compensation since contributors are already assigned and started on the PR. It's just due to other changes, the expectation changes and the solution becomes outdated.

Some other similar cases where compensation is made are below:
#18664 (comment)
#21419 (comment)

[kadiealexander]

Spoken about this privately and agreed to pay out contributor and C+ for this. I have sent Upwork contracts to both.

[dukenv0307]

@kadiealexander accepted, thank you!

[kadiealexander]

Payouts due:

• Issue Reporter: $250 @priya-zha
• Contributor: $1000 @dukenv0307
• Contributor+: $1000 @Santhosh-Sellavel (to be paid via Newdot)

Eligible for 50% #urgency bonus? No

Upwork job is here.

[Santhosh-Sellavel]

Requested $1k on ND

[JmillsExpensify]

Reviewed details for @Santhosh-Sellavel. These details are accurate based on summary from Business Reviewer and are now approved for payment in NewDot.