Immediately remove messages flagged as assault or harassment #20223

neil-marcellini · 2023-06-05T23:12:23Z

Details

Now when you flag a message as assault or harassment it will be immediately removed from view. The message won't show in the chat, LHN, or on the search page.

Fixed Issues

$ https://github.com/Expensify/Expensify/issues/289225
PROPOSAL: N/A

Tests

First check out the corresponding Auth PR https://github.com/Expensify/Auth/pull/8058, build Auth and restart it.

Sign in with any account A
On another device sign in with account B
Send a message from B to A, saying "flag my next message"
Send another message
As account A, flag the message as assault or harassment
Verify that the message is immediately removed from the chat and is no longer visible
Verify that you see a whisper message from concierge saying Thanks for flagging that comment for the Expensify team to review. You will no longer see that message and the Expensify team will take the necessary steps to resolve the situation.
Verify that you do not see the flagged message in the LHN section for account B and see "flag my next message" instead
Open the search page and search for account B
Verify that you do not see the flagged message on the search page and see "flag my next message" instead
Refresh the page and verify that the flagged message is still removed and the whisper is still visible.
Verify the LHN and search page as before
As account B, verify that the flagged message is removed from the chat
Verify that you see a whisper message from concierge saying One of your messages has been flagged. I’ve hidden the message as a potential violation of Rule #2 (Don’t Ruin It For Everyone Else), and it will be reviewed. I will contact you when I have completed my review.
Verify that you do not see the flagged message in the LHN section for account A and see "flag my next message" instead
Open the search page and search for account A
Verify that you do not see the flagged message on the search page and see "flag my next message" instead

Verify that no errors appear in the JS console

Offline tests

Same as the tests above, but go offline, flag the comment, then go back online.
Note: This doesn't match the normal offline behavior where the pending "delete" has a strikethrough style, but I think it's best to make an exception for this case because it's not exactly a delete, and more importantly if someone says something awful you want it gone right away, even if you're offline.

QA Steps

Same as tests.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

I'm only testing on web because these changes are platform independent.

Web

Online

online.mov

Offline

offline.mov

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

neil-marcellini · 2023-06-15T04:37:46Z

I did notice a small issue which should probably be fixed. Although I'm successfully hiding the message in the chat, it's still showing up in the LHN from the report.lastMessageText, as you can see in the offline test video. It would also show up on the search or new chat page.

I'll leave it up to you guys whether we need to fix that before merging this. I could write some hacky code here in App to use the previous action message if the latest action is pendingRemove, or I could fix it in Web-Expensify. I'm not sure what's best but I would like to do it in a follow up so we can get the majority of this feature done in time.

melvin-bot · 2023-06-15T04:38:21Z

@abdulrahuman5196 @roryabraham One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

abdulrahuman5196 · 2023-06-15T17:57:15Z

@neil-marcellini I don't have access to auth PR mentioned in the tests, Do we need to wait for it to merge for testing this PR?

src/libs/ReportActionsUtils.js

thienlnam · 2023-06-15T18:10:49Z

@neil-marcellini I don't have access to auth PR mentioned in the tests, Do we need to wait for it to merge for testing this PR?

@abdulrahuman5196 Yeah, you won't be able to do the full tests until the auth PR is deployed (happening later today), but this should work locally/offline. Like if you flag a message as assault it should be hidden

neil-marcellini · 2023-06-15T20:57:04Z

I just realized we don't need to hold this on the Auth deploy. It won't break anything without the Auth changes, but it won't work until the Auth changes are deployed.

neil-marcellini · 2023-06-15T23:03:23Z

Ok @thienlnam @dangrous @roryabraham this is ready for review now. The Auth PR is merged but isn't on production quite yet. It should be soon and I'll try to comment here once it's deployed.

thienlnam · 2023-06-15T23:17:24Z

src/libs/OptionsListUtils.js

-const allReportActions = {};
+const allSortedReportActions = {};
 Onyx.connect({
    key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
    callback: (actions, key) => {
        if (!key || !actions) {
            return;
        }
-        allReportActions[key] = actions;
+        const sortedReportActions = ReportActionUtils.getSortedReportActions(_.toArray(actions), true);
        const reportID = CollectionUtils.extractCollectionItemID(key);
-        lastReportActions[reportID] = _.last(_.toArray(actions));
+        allSortedReportActions[reportID] = sortedReportActions;
+        lastReportActions[reportID] = _.first(sortedReportActions);
    },


I'm surprised this doesn't impact any other methods here, there isn't any place that uses allReportActions???

Also how come the change to lastReportActions? It doesn't seem like it is used in any of your changes

Yeah a couple things were surprising when I started looking at this, as you mentioned.

allReportActions was unused (idk why lint didn't catch that)

lastReportActions were not actually the "last" report actions for each report. They don't come pre-sorted in the object, they need to be sorted as we do on the ReportScreen with getSortedReportActions. I figured I should fix that while I'm making changes.

thienlnam · 2023-06-15T23:39:58Z

src/libs/OptionsListUtils.js

+
+        // Yeah this is a bit ugly. If the latest report action that is not a whisper has been moderated as pending remove, then set the last message text to the text of the latest visible action that is not a whisper.
+        const lastNonWhisper =
+            _.find(allSortedReportActions[report.reportID], (action) => {
+                const isWhisper = (action.whisperedTo || []).length > 0;
+                return !isWhisper;
+            }) || {};
+        if (lodashGet(lastNonWhisper, 'message[0].moderationDecisions[0].decision') === CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE) {
+            const latestVisibleAction =
+                _.find(allSortedReportActions[report.reportID], (action) => {
+                    const isWhisper = (action.whisperedTo || []).length > 0;
+                    return ReportActionUtils.shouldReportActionBeVisible(action, action.reportActionID) && !isWhisper;
+                }) || {};
+            lastMessageTextFromReport = lodashGet(latestVisibleAction, 'message[0].text', '');
+        }


Right now, we're doing this for every report.
Since this flow is not the main flow can we update the logic so that by default, we don't look for the last non whisper unless we have to?

So basically

Check the last action had a moderation decision

If not, proceed as normal
Otherwise, find the last visible reportAction

We need to check the last action that is not a whisper, because otherwise we're looking to see if the whisper is moderated which will always be false.

_.find will stop as soon as it finds a non whisper action, so it's only going to look through n actions where n is the number of whispers in a row from the latest action moving back in time. I think that's ok.

Hmm yeah, I guess there isn't a great way to avoid this

thienlnam

Just one last comment, but rest looks good

thienlnam · 2023-06-16T00:31:20Z

src/libs/OptionsListUtils.js

+
+        // Yeah this is a bit ugly. If the latest report action that is not a whisper has been moderated as pending remove, then set the last message text to the text of the latest visible action that is not a whisper.
+        const lastNonWhisper =
+            _.find(allSortedReportActions[report.reportID], (action) => {
+                const isWhisper = (action.whisperedTo || []).length > 0;
+                return !isWhisper;
+            }) || {};
+        if (lodashGet(lastNonWhisper, 'message[0].moderationDecisions[0].decision') === CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE) {
+            const latestVisibleAction =
+                _.find(allSortedReportActions[report.reportID], (action) => {
+                    const isWhisper = (action.whisperedTo || []).length > 0;
+                    return ReportActionUtils.shouldReportActionBeVisible(action, action.reportActionID) && !isWhisper;
+                }) || {};
+            lastMessageTextFromReport = lodashGet(latestVisibleAction, 'message[0].text', '');
+        }


Hmm yeah, I guess there isn't a great way to avoid this

src/libs/OptionsListUtils.js

thienlnam · 2023-06-16T00:52:31Z

src/pages/home/report/ReportActionItem.js

-    const whisperedTo = lodashGet(props.action, 'whisperedTo', []);
+    const whisperedTo = props.action.whisperedTo || [];


Why this change?

Lodash get is unnecessary here and this is faster and just as safe. Tim taught me that.

melvin-bot · 2023-06-16T16:32:51Z

@thienlnam looks like this was merged without a test passing. Please add a note explaining why this was done and remove the Emergency label if this is not an emergency.

thienlnam · 2023-06-16T16:34:07Z

My bad, forgot the checklist as I was on mobile but I did test this PR locally. @neil-marcellini could you please remove the label as I can't on mobile 🥲

neil-marcellini · 2023-06-16T16:41:52Z

I'm un-assigning @abdulrahuman5196 because we had to get this done quickly and so he wasn't able to test or review.

abdulrahuman5196 · 2023-06-16T17:24:15Z

Thank you for notifying. Good to have priority changed merged sooner. I was actually under the impression auth PR was going to be merged yesterday and I planned to test this PR today. Apologies if any delay from my end.

OSBotify · 2023-06-16T17:35:23Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-06-16T18:50:16Z

🚀 Deployed to staging by https://github.com/thienlnam in version: 1.3.29-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

aldo-expensify · 2023-06-16T21:14:21Z

@neil-marcellini I think this may be causing this deploy blocker: #20940
I think this fixes it: #20949

aldo-expensify · 2023-06-16T22:58:13Z

@neil-marcellini @dangrous

Do you think we should implement a similar filter for the last report action in Auth: https://github.com/Expensify/Auth/blob/58d8c605f8d820bf6283fb98101798e05287aa26/auth/lib/Report.cpp#L4385-L4416

Right now it doesn't ignore the report action if it has been reported.

dangrous · 2023-06-16T23:05:57Z

Oh yeah, I think that's correct - we should mirror the change there. Let me know if you agree @neil-marcellini

neil-marcellini · 2023-06-19T16:04:41Z

Yeah I think that would be better. @thienlnam discussed something like that previously but we decided to do the quick and dirty version on the front end. What problem is solved by doing the filtering on the backend?

aldo-expensify · 2023-06-19T17:34:52Z

What problem is solved by doing the filtering on the backend?

I'm not 100% sure, but it feels like the backend now calculates an incorrect version of the "last message" of a report which we use for LHN, and the front end has to correct it with the code front end code added. It just sounds dirtier to me, and harder to maintain because the last message in the report coming from the backend is not reliable anymore.

neil-marcellini · 2023-06-19T17:39:34Z

Great explanation, I agree!

aldo-expensify · 2023-06-19T18:55:40Z

I think this #20223 (comment) may be also causing that the report.lastActorAccountID is wrong, see this:

#20949 (comment)

I'll try to fix it in the backend.

OSBotify · 2023-06-21T02:17:46Z

🚀 Deployed to production by https://github.com/luacmartins in version: 1.3.29-11 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

s77rt · 2023-07-04T10:18:42Z

src/libs/ReportActionsUtils.js

+    if (lodashGet(reportAction, 'message[0].moderationDecisions[0].decision') === CONST.MODERATION.MODERATOR_DECISION_PENDING_REMOVE) {
+        return false;
+    }


Here we are hiding report actions that are pending remove. But didn't handle the same for threads. A report action can be threaded and if that report action is hidden so should the thread report. (Coming from #21497)

ah nice catch!

aldo-expensify · 2023-11-24T02:34:01Z

src/libs/OptionsListUtils.js

        const reportID = CollectionUtils.extractCollectionItemID(key);
-        lastReportActions[reportID] = _.last(_.toArray(actions));
+        allSortedReportActions[reportID] = sortedReportActions;


We never clean up allSortedReportActions so it keeps gathering garbage between sessions. This confused me a lot since I was able to find reportActions that were not in Onyx. This is also a memory leak.

@neil-marcellini 🙏 fix

Mark assault and harassment pending removal

eefe346

neil-marcellini self-assigned this Jun 5, 2023

dangrous self-requested a review June 7, 2023 17:36

neil-marcellini added 2 commits June 14, 2023 14:08

Merge branch 'main' into neil-moderation-pending-remove

cce3c21

Actions moderated pending remove are not visible

40d51d0

neil-marcellini changed the title ~~[HOLD Auth] Mark assault and harassment pending removal~~ [HOLD Auth 8058] Mark assault and harassment pending removal Jun 14, 2023

neil-marcellini changed the title ~~[HOLD Auth 8058] Mark assault and harassment pending removal~~ [HOLD Auth 8058] Immediately remove messages flagged as assault or harassment Jun 14, 2023

Fix style

291e2a2

neil-marcellini marked this pull request as ready for review June 15, 2023 04:37

neil-marcellini requested a review from a team as a code owner June 15, 2023 04:37

melvin-bot bot removed the request for review from a team June 15, 2023 04:38

melvin-bot bot requested review from abdulrahuman5196 and roryabraham June 15, 2023 04:38

thienlnam requested changes Jun 15, 2023

View reviewed changes

src/libs/ReportActionsUtils.js Show resolved Hide resolved

neil-marcellini added 3 commits June 15, 2023 13:03

Update report last message text when pendingRemove

bfbf8f5

Sort actions in Onyx.connect, fix last action

ef88762

Exclude whispers for last message text

5c4657a

neil-marcellini changed the title ~~[HOLD Auth 8058] Immediately remove messages flagged as assault or harassment~~ Immediately remove messages flagged as assault or harassment Jun 15, 2023

neil-marcellini added 3 commits June 15, 2023 14:10

Get last message text for report w/ special cases

0fc2f2f

Merge branch 'main' into neil-moderation-pending-remove

1766704

Fix style

1261fbe

thienlnam requested changes Jun 15, 2023

View reviewed changes

thienlnam requested changes Jun 16, 2023

View reviewed changes

DRY with isWhisper util

f6b1eb0

neil-marcellini requested a review from thienlnam June 16, 2023 00:46

thienlnam reviewed Jun 16, 2023

View reviewed changes

thienlnam approved these changes Jun 16, 2023

View reviewed changes

thienlnam merged commit 469596a into main Jun 16, 2023

thienlnam deleted the neil-moderation-pending-remove branch June 16, 2023 16:32

melvin-bot bot added the Emergency label Jun 16, 2023

neil-marcellini removed the Emergency label Jun 16, 2023

neil-marcellini removed the request for review from abdulrahuman5196 June 16, 2023 16:41

OSBotify mentioned this pull request Jun 16, 2023

Deploy Checklist: New Expensify 2023-06-16 #20928

Closed

aldo-expensify mentioned this pull request Jun 16, 2023

[HOLD] Don't use CREATED action as last message for LHN #20949

Closed

57 tasks

s77rt reviewed Jul 4, 2023

View reviewed changes

s77rt mentioned this pull request Jul 4, 2023

[HOLD for payment 2023-07-10] [$1000] A threaded message that is flagged with harassment or assault is not removed from LHN #21497

Closed

6 tasks

aldo-expensify reviewed Nov 24, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Immediately remove messages flagged as assault or harassment #20223

Immediately remove messages flagged as assault or harassment #20223

neil-marcellini commented Jun 5, 2023 •

edited

Loading

neil-marcellini commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

abdulrahuman5196 commented Jun 15, 2023

thienlnam commented Jun 15, 2023

neil-marcellini commented Jun 15, 2023

neil-marcellini commented Jun 15, 2023

thienlnam Jun 15, 2023

thienlnam Jun 15, 2023

neil-marcellini Jun 16, 2023 •

edited

Loading

thienlnam Jun 15, 2023

neil-marcellini Jun 16, 2023

thienlnam Jun 16, 2023

thienlnam left a comment

thienlnam Jun 16, 2023

thienlnam Jun 16, 2023

neil-marcellini Jun 16, 2023

melvin-bot bot commented Jun 16, 2023

thienlnam commented Jun 16, 2023

neil-marcellini commented Jun 16, 2023

abdulrahuman5196 commented Jun 16, 2023

OSBotify commented Jun 16, 2023

OSBotify commented Jun 16, 2023

aldo-expensify commented Jun 16, 2023 •

edited

Loading

aldo-expensify commented Jun 16, 2023

dangrous commented Jun 16, 2023

neil-marcellini commented Jun 19, 2023

aldo-expensify commented Jun 19, 2023

neil-marcellini commented Jun 19, 2023

aldo-expensify commented Jun 19, 2023

OSBotify commented Jun 21, 2023

s77rt Jul 4, 2023

dangrous Jul 5, 2023

aldo-expensify Nov 24, 2023

		const whisperedTo = lodashGet(props.action, 'whisperedTo', []);
		const whisperedTo = props.action.whisperedTo \|\| [];

Immediately remove messages flagged as assault or harassment #20223

Immediately remove messages flagged as assault or harassment #20223

Conversation

neil-marcellini commented Jun 5, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

neil-marcellini commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

abdulrahuman5196 commented Jun 15, 2023

thienlnam commented Jun 15, 2023

neil-marcellini commented Jun 15, 2023

neil-marcellini commented Jun 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-marcellini Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thienlnam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melvin-bot bot commented Jun 16, 2023

thienlnam commented Jun 16, 2023

neil-marcellini commented Jun 16, 2023

abdulrahuman5196 commented Jun 16, 2023

OSBotify commented Jun 16, 2023

OSBotify commented Jun 16, 2023

aldo-expensify commented Jun 16, 2023 • edited Loading

aldo-expensify commented Jun 16, 2023

dangrous commented Jun 16, 2023

neil-marcellini commented Jun 19, 2023

aldo-expensify commented Jun 19, 2023

neil-marcellini commented Jun 19, 2023

aldo-expensify commented Jun 19, 2023

OSBotify commented Jun 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

neil-marcellini commented Jun 5, 2023 •

edited

Loading

neil-marcellini Jun 16, 2023 •

edited

Loading

aldo-expensify commented Jun 16, 2023 •

edited

Loading