-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[No QA] Handle Moderated Messages in UI #19476
Changes from 14 commits
783b4b6
535c054
f831821
26b7345
70c4c18
5359e45
d42b150
801b9d0
a381211
f49a93e
b5083ee
b104d9b
5e9a8c8
e97ac5d
097ad8a
9bc5cd8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -10,6 +10,7 @@ import reportActionPropTypes from './reportActionPropTypes'; | |||||
import * as StyleUtils from '../../../styles/StyleUtils'; | ||||||
import PressableWithSecondaryInteraction from '../../../components/PressableWithSecondaryInteraction'; | ||||||
import Hoverable from '../../../components/Hoverable'; | ||||||
import Button from '../../../components/Button'; | ||||||
import ReportActionItemSingle from './ReportActionItemSingle'; | ||||||
import ReportActionItemGrouped from './ReportActionItemGrouped'; | ||||||
import MoneyRequestAction from '../../../components/ReportActionItem/MoneyRequestAction'; | ||||||
|
@@ -105,6 +106,8 @@ const defaultProps = { | |||||
|
||||||
function ReportActionItem(props) { | ||||||
const [isContextMenuActive, setIsContextMenuActive] = useState(ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)); | ||||||
const [isHidden, setIsHidden] = useState(false); | ||||||
const [moderationDecision, setModerationDecision] = useState(CONST.MODERATION.MODERATOR_DECISION_APPROVED); | ||||||
const textInputRef = useRef(); | ||||||
const popoverAnchorRef = useRef(); | ||||||
|
||||||
|
@@ -117,6 +120,21 @@ function ReportActionItem(props) { | |||||
focusTextInputAfterAnimation(textInputRef.current, 100); | ||||||
}, [isDraftEmpty]); | ||||||
|
||||||
// Hide the message if it is being moderated for a higher offense, or is hidden by a moderator | ||||||
// Removed messages should not be shown anyway and should not need this flow | ||||||
useEffect(() => { | ||||||
if (_.isEmpty(props.action.moderationDecisions)) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's first check that it's an ADDCOMMENT report action and then update to check the message object like we talked about |
||||||
return; | ||||||
} | ||||||
|
||||||
// Right now we are only sending the latest moderationDecision to the frontend even though it is an array | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this because any previous moderation decisions won't really be used in the frontend? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep! We originally were just going to send them all because it was simpler for the backend, but this made a little more sense for JS. |
||||||
const latestDecision = props.action.moderationDecisions[0]; | ||||||
if (latestDecision.decision === CONST.MODERATION.MODERATOR_DECISION_PENDING_HIDE || latestDecision.decision === CONST.MODERATION.MODERATOR_DECISION_HIDDEN) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
NAB |
||||||
setIsHidden(true); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We fail to set a hidden false case here which causes a regression #22024 , For more details refer this proposal |
||||||
} | ||||||
setModerationDecision(latestDecision.decision); | ||||||
}, [props.action, moderationDecision]); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we check props.action.message so this doesn't re-render as much? Also it doesn't seem like we need to add a case for moderationDecision. It only changes when the prop.action.message changes, and then it would cause another run of this since the moderationDecision changed There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah i agree on moderationDecision... but the linter did not haha. I'll see if switching over to the right format for the message object fixes that. |
||||||
|
||||||
const toggleContextMenuFromActiveReportAction = useCallback(() => { | ||||||
setIsContextMenuActive(ReportActionContextMenu.isActiveReportAction(props.action.reportActionID)); | ||||||
}, [props.action.reportActionID]); | ||||||
|
@@ -227,6 +245,7 @@ function ReportActionItem(props) { | |||||
); | ||||||
} else { | ||||||
const message = _.last(lodashGet(props.action, 'message', [{}])); | ||||||
const hasBeenFlagged = moderationDecision !== CONST.MODERATION.MODERATOR_DECISION_APPROVED && moderationDecision !== CONST.MODERATION.MODERATOR_DECISION_PENDING; | ||||||
MariaHCD marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
const isAttachment = _.has(props.action, 'isAttachment') ? props.action.isAttachment : ReportUtils.isReportMessageAttachment(message); | ||||||
children = ( | ||||||
<ShowContextMenuContext.Provider | ||||||
|
@@ -238,13 +257,32 @@ function ReportActionItem(props) { | |||||
}} | ||||||
> | ||||||
{!props.draftMessage ? ( | ||||||
<ReportActionItemMessage | ||||||
action={props.action} | ||||||
style={[ | ||||||
!props.displayAsGroup && isAttachment ? styles.mt2 : undefined, | ||||||
_.contains([..._.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG), CONST.REPORT.ACTIONS.TYPE.IOU], props.action.actionName) ? styles.colorMuted : undefined, | ||||||
]} | ||||||
/> | ||||||
<View style={props.displayAsGroup && hasBeenFlagged ? styles.blockquote : {}}> | ||||||
<ReportActionItemMessage | ||||||
action={props.action} | ||||||
isHidden={isHidden} | ||||||
style={[ | ||||||
!props.displayAsGroup && isAttachment ? styles.mt2 : undefined, | ||||||
_.contains([..._.values(CONST.REPORT.ACTIONS.TYPE.POLICYCHANGELOG), CONST.REPORT.ACTIONS.TYPE.IOU], props.action.actionName) | ||||||
? styles.colorMuted | ||||||
: undefined, | ||||||
]} | ||||||
/> | ||||||
{props.displayAsGroup && hasBeenFlagged && ( | ||||||
<Button | ||||||
small | ||||||
style={[styles.mt2, styles.alignSelfStart]} | ||||||
onPress={() => setIsHidden(!isHidden)} | ||||||
> | ||||||
<Text | ||||||
style={styles.buttonSmallText} | ||||||
selectable={false} | ||||||
> | ||||||
{isHidden ? props.translate('moderation.revealMessage') : props.translate('moderation.hideMessage')} | ||||||
</Text> | ||||||
</Button> | ||||||
)} | ||||||
</View> | ||||||
) : ( | ||||||
<ReportActionItemMessageEdit | ||||||
action={props.action} | ||||||
|
@@ -260,6 +298,20 @@ function ReportActionItem(props) { | |||||
} | ||||||
/> | ||||||
)} | ||||||
{!props.displayAsGroup && hasBeenFlagged && ( | ||||||
<Button | ||||||
small | ||||||
style={[styles.mt2, styles.alignSelfStart]} | ||||||
onPress={() => setIsHidden(!isHidden)} | ||||||
> | ||||||
<Text | ||||||
style={styles.buttonSmallText} | ||||||
selectable={false} | ||||||
> | ||||||
{isHidden ? 'Reveal message' : 'Hide message'} | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You forgot to translate this which caused a regression #20090 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, saw that one when it happened... I changed it in one of the places it appears but not the other one. Thanks! |
||||||
</Text> | ||||||
</Button> | ||||||
)} | ||||||
</ShowContextMenuContext.Provider> | ||||||
); | ||||||
} | ||||||
|
@@ -319,6 +371,7 @@ function ReportActionItem(props) { | |||||
wrapperStyles={[styles.chatItem, isWhisper ? styles.pt1 : {}]} | ||||||
shouldShowSubscriptAvatar={props.shouldShowSubscriptAvatar} | ||||||
report={props.report} | ||||||
hasBeenFlagged={moderationDecision !== CONST.MODERATION.MODERATOR_DECISION_APPROVED && moderationDecision !== CONST.MODERATION.MODERATOR_DECISION_PENDING} | ||||||
> | ||||||
{content} | ||||||
</ReportActionItemSingle> | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, we missed a small offline edge case in this PR - #20946
On deleting a hidden message, the reveal message button should have been removed too.
More details here #20946 (comment)