[HOLD for payment 2024-03-18] [$250] Room - Flag as offensive function is absent in the conversation, but present in the thread

[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: 1.4.38-0
Reproducible in staging?: y
Reproducible in production?: y
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: Applause internal team
Slack conversation:

Action Performed:

1. Open https://staging.new.expensify.com/

2. Log in under your HT account A

3. In incognito mode, open https://staging.new.expensify.com/

4. Log in with a different account B

5. Under user A, create a WS

6. Invite user B to the created WS

7. Under user A, send a message to the @announce room

8. In the room settings, enable Who can post - Admins only

9. Under user B, right-click on a message from user A

10. Note that the Flag as offensive function is not available

11. Create a thread with this message

12. In the thread, right-click on the parent message

Expected Result:

The Flag as offensive function must be present in the conversation and is present in the thread. Or absent in the conversation and thread, if so intended.

Actual Result:

Flag as offensive function is absent in the conversation, but present in the thread

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6370545_1707321136324.Recording__1302.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~015264f638251d5c13
• Upwork Job ID: 1755273971773313024
• Last Price Increase: 2024-02-28
• Automatic offers:
• getusha | Reviewer | 0
• dukenv0307 | Contributor | 0

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~015264f638251d5c13

[melvin-bot]

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

[melvin-bot]

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

[m-natarajan]

We think that this bug might be related #vip-vsb
CC @quinthar

[FitseTLT]

Proposal

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

Flag as offensive function is absent in the conversation, but present in the thread

What is the root cause of that problem?

The problem is on the room isAllowedToComment is false for the report action as the writeCapability is set to admin but when you create a thread with it isAllowedToComment is calculated according to the new thread as the report so now in the new thread it will be true and it will include flag as offensive menu item in the context menu

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

We need to check if it is the main message of a thread and ensure isAllowedToComment is calculated according to the parent report on which the report action originally belongs to not the thread

App/src/libs/ReportUtils.ts

Lines 3857 to 3865 in f6ba751

	return Boolean(
	!isCurrentUserAction &&
	reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT &&
	!ReportActionsUtils.isDeletedAction(reportAction) &&
	!ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
	!isEmptyObject(report) &&
	report &&
	isAllowedToComment(report),
	);

    const reportActionReport = reportAction?.childReportID && Number(reportAction?.childReportID) === Number(reportID) ? getReport(report?.parentReportID) : report;

    return Boolean(
        !isCurrentUserAction &&
            reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT &&
            !ReportActionsUtils.isDeletedAction(reportAction) &&
            !ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
            !isEmptyObject(report) &&
            report &&
            isAllowedToComment(reportActionReport),
    );

What alternative solutions did you explore? (Optional)

[dukenv0307]

Proposal

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

Flag as offensive function is absent in the conversation, but present in the thread

What is the root cause of that problem?

The report being used to check write capability here is not correct in case the report action being evaluated is the parent report action in a thread. In this case, we're currently checking the write capability of the child report rather than the parent report.

So let's say report action A belongs to #announce room, and a thread B is threaded from A, then when right click on the report action A inside thread B (as the first message/parent report action), the reportID and report here will be of the thread B, not #announce room

App/src/libs/ReportUtils.ts

Line 3841 in fe61eda

const report = getReport(reportID);

, user can post to the thread, but not to the #announce room, so they can flag the report action if it's displayed as parent report action of thread B, but cannot flag when it's displayed as a report action of #announce room.

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

In here, first check if the childReportID exists in reportAction and is equal to the reportID, if yes, this is the case of report action being evaluated is the parent report action in a thread, and we should get the parent report to evaluate instead.

let report = getReport(reportID);
if (reportAction?.childReportID && String(reportAction?.childReportID) === String(reportID)) {
    report = getReport(report?.parentReportID);
}

The correct parent report will then be used here and the right allowed to comment value will be retrieved.

What alternative solutions did you explore? (Optional)

NA

[dukenv0307]

Proposal updated to add example code change and expand on RCA.

[dukenv0307]

@FitseTLT I'd suggest you to refrain from copying parts (or whole) of later proposals and add to yours (and without the proposal updated comment)

Here's @FitseTLT's proposal at the time I posted mine, just so @getusha is not confused

[FitseTLT]

@dukenv0307 I am not finished with my proposal and when I am finished will notify

[FitseTLT]

Updated

[Tony-MK]

Proposal

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

Room - Flag as an offensive function is absent in the conversation but present in the thread

What is the root cause of that problem?

The isAllowedToComment determines whether to flag an offensive comment.

App/src/libs/ReportUtils.ts

Lines 3925 to 3933 in 198ca2c

	return Boolean(
	!isCurrentUserAction &&
	reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT &&
	!ReportActionsUtils.isDeletedAction(reportAction) &&
	!ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
	!isEmptyObject(report) &&
	report &&
	isAllowedToComment(report),
	);

Furthermore, the isAllowedToComment will return false because report?.writeCapability is present and the member is not a part of the admin.

App/src/libs/ReportUtils.ts

Lines 1068 to 1069 in 1b06012

	const policy = allPolicies[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`];
	return policy?.role === CONST.POLICY.ROLE.ADMIN;

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

We can refuse to display the flag button if the policy only allows admins to comment.

Instead of the report, use the root workspace report when using the isAllowedToComment in the canFlagReportAction function to the function to perform when the chats theard which are children in the policy but the parent report is still a thread and not the root workspace report.

App/src/libs/ReportUtils.ts

Line 3932 in 198ca2c

isAllowedToComment(report),

Therefore, we should replace the isAllowedToComment with the condition below.

isWorkspaceThread(report) ? isAllowedToComment(getRootParentReport(report)) :  isAllowedToComment(report)

What alternative solutions did you explore? (Optional)

On the other hand, we could remove the isAllowedToComment and allow everyone to flag the comment just like reacting, replying, and marking it unread.

Like the mentioned buttons, even if the user is unauthenticated, the user will log in after pressing the flag comment button.

[getusha]

@Tony-MK's proposal looks good to me.
🎀 👀 🎀 C+ Reviewed.

[melvin-bot]

Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[dukenv0307]

@getusha That proposal will lead to users unable to flag any report action inside the thread, which is not correct because the members in a admin-write-only workspace could still reply to threads and talk to each other on threads, this is expected business logic. Per design, if users can send comments in a thread/report, they should be able to flag messages in it (which is why isAllowedToComment is used).

Could you give some feedback on my proposal which will produce the correct behavior (only the report action that belongs to the main report, which can't be commented on, cannot be flagged, the rest of the report actions inside the thread could still be flagged normally)?

Thank you!

[Tony-MK]

Could we get a clarification on the expected result?

Should we show the offensive Flag in the room thread or not?

Also, should we apply the fix to the threads inside the room thread?

[JmillsExpensify]

Hmm, yeah I think this is by design right? The admin-only post is not flagged by design, but the thread is a distinct convo from the parent, so it can be. @dangrous Do you mind confirming?

[dukenv0307]

Hmm, yeah I think this is by design right? The admin-only post is not flagged by design, but the thread is a distinct convo from the parent, so it can be. @dangrous Do you mind confirming?

Just to be clear, there're 2 part of the threads:

1. The thread parent report action, which is a message from the parent report which should not be able to be flagged because the parent report is admin-only
2. The thread content, which is another convo by itself and should be flaggable since members can freely comment on it

@dangrous Could you check if this is right?

Thank you!

[dangrous]

Hi! So @JmillsExpensify and I discussed separately:

1. We're going to bring this to a broader discussion, we're not sure on the action here.
2. Yes, this should be able to be flagged.

[melvin-bot]

📣 @getusha 🎉 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

[melvin-bot]

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

Offer link
Upwork job
Please accept the offer 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 📖

[Tony-MK]

hmm..., @youssef-lr, I think you might have made a mistake.

I believe my proposal fixes the issue because the selected proposal only prevents non-admins from flagging the child thread but not deeper workspace chat threads from the child thread.

This was the main reason I posted my proposal and was selected by @getusha.

Kindly, could you re-evaluate my proposal?

Thank you

[youssef-lr]

@Tony-MK This is your suggested change:

Therefore, we should replace the isAllowedToComment with the condition below.

isWorkspaceThread(report) ? isAllowedToComment(getRootParentReport(report)) : isAllowedToComment(report)

As @dukenv0307 mentioned, I think isWorkspaceThread(report) will return true for every reply in a thread from an admins-only room. So canFlagReportAction will always return isAllowedToComment(getRootParentReport(report)) which should be false for non-admins in a thread, even though they're allowed to comment on the thread. Anything I'm missing here?

[Tony-MK]

@youssef-lr ,After checking back on my proposal, I remember I used the getRootParentReport function to get correct report to pass to isAllowToComment because the thread report can not be used to evaluate whether a user can flag.

That's why the parent admin report doesn't allow a user to flag a messge but can the message thread.

Therefore, it should return true for non admin rooms.

I tested it and worked but could I not be grasping something?

[dukenv0307]

@getusha PR is ready to 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.4.49-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix/36045: Flag as offensive is absent #37399

If no regressions arise, payment will be issued on 2024-03-18. 🎊

For reference, here are some details about the assignees on this issue:

• @getusha requires payment automatic offer (Reviewer)
• @dukenv0307 requires payment automatic offer (Contributor)

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

• [@getusha] The PR that introduced the bug has been identified. Link to the PR:
• [@getusha] 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:
• [@getusha] 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:
• [@getusha] Determine if we should create a regression test for this bug.
• [@getusha] 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.
• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[JmillsExpensify]

@getusha Do you mind filling out the BZ checklist above? In the meantime, I believe the payment summary is:

• Contributor: @dukenv0307 $250
• Contributor+: @getusha $250

[getusha]

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:

• [@getusha] The PR that introduced the bug has been identified. Link to the PR: Disallow flagging whispers #29994
• [@getusha] 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: https://github.com/Expensify/App/pull/29994/files#r1532291802
• [@getusha] 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: N/a
• [@getusha] Determine if we should create a regression test for this bug. No i think this is an edge case.
• [@getusha] 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.

[getusha]

Contributor: @dukenv0307 $500
Contributor+: @getusha $500

@JmillsExpensify it is $250 based on #36045 (comment)

[JmillsExpensify]

Thanks. I updated the payment summary above and paid all contracts in Upwork.