From cf71b776228aa39ac20a7a9ee9c4ed224c39a662 Mon Sep 17 00:00:00 2001 From: seotts Date: Mon, 15 Mar 2021 15:41:06 -0400 Subject: [PATCH 1/6] Work on adding messaging for previous mute --- src/views/preview/comment/compose-comment.jsx | 33 +++++++++++++++---- src/views/preview/l10n.json | 5 +++ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/views/preview/comment/compose-comment.jsx b/src/views/preview/comment/compose-comment.jsx index b66cc83a0..2d7218631 100644 --- a/src/views/preview/comment/compose-comment.jsx +++ b/src/views/preview/comment/compose-comment.jsx @@ -29,7 +29,8 @@ const ComposeStatus = keyMirror({ EDITING: null, SUBMITTING: null, REJECTED: null, - REJECTED_MUTE: null + REJECTED_MUTE: null, + COMPOSE_DISALLOWED: null }); class ComposeComment extends React.Component { @@ -48,7 +49,7 @@ class ComposeComment extends React.Component { this.props.muteStatus.muteExpiresAt * 1000 : 0; // convert to ms this.state = { message: '', - status: ComposeStatus.EDITING, + status: muteExpiresAtMs > 0 ? ComposeStatus.COMPOSE_DISALLOWED : ComposeStatus.EDITING, error: null, appealId: null, muteOpen: muteExpiresAtMs > Date.now() && this.props.isReply, @@ -100,10 +101,17 @@ class ComposeComment extends React.Component { let muteType = null; if (body.status && body.status.mute_status) { muteExpiresAtMs = body.status.mute_status.muteExpiresAt * 1000; // convert to ms - rejectedStatus = ComposeStatus.REJECTED_MUTE; + + if (body.rejected === 'isBad') { + rejectedStatus = ComposeStatus.REJECTED_MUTE; + } else { + rejectedStatus = ComposeStatus.COMPOSE_DISALLOWED; + } + if (this.shouldShowMuteModal(body.status.mute_status)) { muteOpen = true; } + showWarning = body.status.mute_status.showWarning; muteType = body.status.mute_status.currentMessageType; this.setupMuteExpirationTimeout(muteExpiresAtMs); @@ -176,6 +184,12 @@ class ComposeComment extends React.Component { return false; } + // If the user is already muted (for example, in a different tab), + // do not show modal because it would be confusing + if (this.state.status === ComposeStatus.COMPOSE_DISALLOWED) { + return false; + } + // If the backend tells us to show a warning about getting blocked, we should show the modal // regardless of what the offenses list looks like. if (muteStatus.showWarning) { @@ -210,30 +224,35 @@ class ComposeComment extends React.Component { pii: { name: 'pii', commentType: 'comment.type.pii', + commentTypePast: 'comment.type.pii.past', muteStepHeader: 'comment.pii.header', muteStepContent: ['comment.pii.content1', 'comment.pii.content2', 'comment.pii.content3'] }, unconstructive: { name: 'unconstructive', commentType: 'comment.type.unconstructive', + commentTypePast: 'comment.type.unconstructive.past', muteStepHeader: 'comment.unconstructive.header', muteStepContent: ['comment.unconstructive.content1', 'comment.unconstructive.content2'] }, vulgarity: { name: 'vulgarity', commentType: 'comment.type.vulgarity', + commentTypePast: 'comment.type.vulgarity.past', muteStepHeader: 'comment.vulgarity.header', muteStepContent: ['comment.vulgarity.content1', 'comment.vulgarity.content2'] }, spam: { name: 'spam', commentType: 'comment.type.spam', + commentTypePast: 'comment.type.spam.past', muteStepHeader: 'comment.spam.header', muteStepContent: ['comment.spam.content1', 'comment.spam.content2'] }, general: { name: 'general', commentType: 'comment.type.general', + commentTypePast: 'comment.type.general.past', muteStepHeader: 'comment.general.header', muteStepContent: ['comment.general.content1'] } @@ -261,7 +280,7 @@ class ComposeComment extends React.Component { {(this.isMuted() && !(this.props.isReply && this.state.status !== ComposeStatus.REJECTED_MUTE)) ? ( -

+

) : null } - {!this.isMuted() || (this.isMuted() && this.state.status === ComposeStatus.REJECTED_MUTE) ? ( + {this.state.status === ComposeStatus.COMPOSE_DISALLOWED ? null : (

- {this.state.error && this.state.status !== ComposeStatus.REJECTED_MUTE ? ( + {this.state.status === ComposeStatus.REJECTED ? (
- ) : null } + )} {this.state.muteOpen ? ( Date: Wed, 17 Mar 2021 16:07:30 -0400 Subject: [PATCH 2/6] Continued work --- src/views/preview/comment/compose-comment.jsx | 14 ++++++++------ test/unit/components/compose-comment.test.jsx | 9 ++++++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/views/preview/comment/compose-comment.jsx b/src/views/preview/comment/compose-comment.jsx index 2d7218631..66d900235 100644 --- a/src/views/preview/comment/compose-comment.jsx +++ b/src/views/preview/comment/compose-comment.jsx @@ -97,18 +97,20 @@ class ComposeComment extends React.Component { let muteOpen = false; let muteExpiresAtMs = 0; let rejectedStatus = ComposeStatus.REJECTED; + let justMuted = true; let showWarning = false; let muteType = null; if (body.status && body.status.mute_status) { muteExpiresAtMs = body.status.mute_status.muteExpiresAt * 1000; // convert to ms - if (body.rejected === 'isBad') { + if (body.rejected === JUST_MUTED_ERROR) { rejectedStatus = ComposeStatus.REJECTED_MUTE; } else { rejectedStatus = ComposeStatus.COMPOSE_DISALLOWED; + justMuted = false; } - - if (this.shouldShowMuteModal(body.status.mute_status)) { + + if (this.shouldShowMuteModal(body.status.mute_status, justMuted)) { muteOpen = true; } @@ -170,7 +172,7 @@ class ComposeComment extends React.Component { muteOpen: true }); } - shouldShowMuteModal (muteStatus) { + shouldShowMuteModal (muteStatus, justMuted) { // We should show the mute modal if the user is in danger of being blocked or // when the user is newly muted or hasn't seen it for a while. // We don't want to show it more than about once a week. @@ -186,7 +188,7 @@ class ComposeComment extends React.Component { // If the user is already muted (for example, in a different tab), // do not show modal because it would be confusing - if (this.state.status === ComposeStatus.COMPOSE_DISALLOWED) { + if (!justMuted) { return false; } @@ -390,7 +392,7 @@ class ComposeComment extends React.Component { muteModalMessages={this.getMuteMessageInfo()} shouldCloseOnOverlayClick={false} showFeedback={ - this.state.status === ComposeStatus.REJECTED_MUTE && this.state.error === JUST_MUTED_ERROR + this.state.status === ComposeStatus.REJECTED_MUTE } showWarning={this.state.showWarning} startStep={this.getMuteModalStartStep()} diff --git a/test/unit/components/compose-comment.test.jsx b/test/unit/components/compose-comment.test.jsx index 293abb3d7..d34f38acd 100644 --- a/test/unit/components/compose-comment.test.jsx +++ b/test/unit/components/compose-comment.test.jsx @@ -68,7 +68,10 @@ describe('Compose Comment test', () => { test('Error messages shows when comment rejected ', () => { const component = getComposeCommentWrapper({}); const commentInstance = component.instance(); - commentInstance.setState({error: 'isFlood'}); + commentInstance.setState({ + error: 'isFlood', + status: 'REJECTED' + }); component.update(); expect(component.find('FlexRow.compose-error-row').exists()).toEqual(true); // Buttons stay enabled when comment rejected for non-mute reasons @@ -76,12 +79,12 @@ describe('Compose Comment test', () => { expect(component.find('Button.compose-cancel').props().disabled).toBe(false); }); - test('No error message shows when comment rejected because user muted ', () => { + test('No error message shows when comment rejected because user is already muted ', () => { const component = getComposeCommentWrapper({}); const commentInstance = component.instance(); commentInstance.setState({ error: 'isMuted', - status: 'REJECTED_MUTE' + status: 'COMPOSE_DISALLOWED' }); component.update(); expect(component.find('FlexRow.compose-error-row').exists()).toEqual(false); From 94eb3dc6dee6a29058c45e495f777f7a7d2d9ff5 Mon Sep 17 00:00:00 2001 From: seotts Date: Thu, 18 Mar 2021 16:15:11 -0400 Subject: [PATCH 3/6] Continued work --- src/views/preview/comment/compose-comment.jsx | 21 ++++++++--- test/unit/components/compose-comment.test.jsx | 35 ++++++++++++++----- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/views/preview/comment/compose-comment.jsx b/src/views/preview/comment/compose-comment.jsx index 66d900235..58c322ca4 100644 --- a/src/views/preview/comment/compose-comment.jsx +++ b/src/views/preview/comment/compose-comment.jsx @@ -49,7 +49,7 @@ class ComposeComment extends React.Component { this.props.muteStatus.muteExpiresAt * 1000 : 0; // convert to ms this.state = { message: '', - status: muteExpiresAtMs > 0 ? ComposeStatus.COMPOSE_DISALLOWED : ComposeStatus.EDITING, + status: muteExpiresAtMs > Date.now() ? ComposeStatus.COMPOSE_DISALLOWED : ComposeStatus.EDITING, error: null, appealId: null, muteOpen: muteExpiresAtMs > Date.now() && this.props.isReply, @@ -162,7 +162,7 @@ class ComposeComment extends React.Component { // Cancel (i.e. complete) the reply action if the user clicked on the reply button while // alreay muted. This "closes" the reply. If they just got muted, we want to leave it open // so the blue CommentingStatus box shows. - if (this.props.isReply && this.state.status !== ComposeStatus.REJECTED_MUTE) { + if (this.props.isReply && this.state.status === ComposeStatus.COMPOSE_DISALLOWED) { this.handleCancel(); } } @@ -186,6 +186,11 @@ class ComposeComment extends React.Component { return false; } + // TODO: Check with Kathy, but we think you should always see the modal when you reply? + if (this.props.isReply) { + return true; + } + // If the user is already muted (for example, in a different tab), // do not show modal because it would be confusing if (!justMuted) { @@ -215,7 +220,7 @@ class ComposeComment extends React.Component { // Decides which step of the mute modal to start on. If this was a reply button click, // we show them the step that tells them how much time is left on their mute, otherwise // they start at the beginning of the progression. - return this.props.isReply && this.state.status !== ComposeStatus.REJECTED_MUTE ? + return this.props.isReply && this.state.status === ComposeStatus.COMPOSE_DISALLOWED ? MuteModal.steps.MUTE_INFO : MuteModal.steps.COMMENT_ISSUE; } @@ -282,7 +287,15 @@ class ComposeComment extends React.Component { {(this.isMuted() && !(this.props.isReply && this.state.status !== ComposeStatus.REJECTED_MUTE)) ? ( -

+

+ +

{ expect(component.find('FlexRow.compose-error-row').exists()).toEqual(false); }); - test('Comment Status shows but compose box does not when mute expiration in the future ', () => { + test('Comment Status shows but compose box does not when you load the page and you are already muted', () => { const realDateNow = Date.now.bind(global.Date); global.Date.now = () => 0; const component = getComposeCommentWrapper({}); const commentInstance = component.instance(); - commentInstance.setState({muteExpiresAtMs: 100}); + commentInstance.setState({muteExpiresAtMs: 100, status: 'COMPOSE_DISALLOWED'}); component.update(); + // Compose box should be hidden if muted unless they got muted due to a comment they just posted. expect(component.find('FlexRow.compose-comment').exists()).toEqual(false); expect(component.find('MuteModal').exists()).toEqual(false); @@ -236,7 +237,7 @@ describe('Compose Comment test', () => { const commentInstance = component.instance(); commentInstance.setState({ error: 'some error', - status: 'FLOOD' + status: 'REJECTED' }); component.update(); expect(component.find('FlexRow.compose-error-row').exists()).toEqual(true); @@ -338,7 +339,7 @@ describe('Compose Comment test', () => { expect(component.find('MuteModal').props().showFeedback).toBe(true); commentInstance.setState({ - status: 'REJECTED_MUTE', + status: 'COMPOSE_DISALLOWED', error: 'isMute', showWarning: true, muteOpen: true @@ -392,7 +393,7 @@ describe('Compose Comment test', () => { offenses: [offense] }; const commentInstance = getComposeCommentWrapper({}).instance(); - expect(commentInstance.shouldShowMuteModal(muteStatus)).toBe(true); + expect(commentInstance.shouldShowMuteModal(muteStatus, true)).toBe(true); global.Date.now = realDateNow; }); @@ -413,7 +414,7 @@ describe('Compose Comment test', () => { offenses: offenses }; const commentInstance = getComposeCommentWrapper({}).instance(); - expect(commentInstance.shouldShowMuteModal(muteStatus)).toBe(false); + expect(commentInstance.shouldShowMuteModal(muteStatus, true)).toBe(false); global.Date.now = realDateNow; }); @@ -435,7 +436,25 @@ describe('Compose Comment test', () => { showWarning: true }; const commentInstance = getComposeCommentWrapper({}).instance(); - expect(commentInstance.shouldShowMuteModal(muteStatus)).toBe(true); + expect(commentInstance.shouldShowMuteModal(muteStatus, true)).toBe(true); + global.Date.now = realDateNow; + }); + + test('shouldShowMuteModal is false when the user is already muted, even when only 1 recent offesnse ', () => { + const realDateNow = Date.now.bind(global.Date); + global.Date.now = () => 0; + // Since Date.now mocked to 0 above, we just need a small number to make + // it look like it was created < 2 minutes ago. + const offense = { + expiresAt: '1000', + createdAt: '-60' // ~1 ago min given shouldShowMuteModal's conversions, + }; + const muteStatus = { + offenses: [offense] + }; + const justMuted = false; + const commentInstance = getComposeCommentWrapper({}).instance(); + expect(commentInstance.shouldShowMuteModal(muteStatus, justMuted)).toBe(false); global.Date.now = realDateNow; }); @@ -455,7 +474,7 @@ describe('Compose Comment test', () => { test('getMuteModalStartStep: A reply click when already muted ', () => { const commentInstance = getComposeCommentWrapper({isReply: true}).instance(); commentInstance.setState({ - status: 'EDITING' + status: 'COMPOSE_DISALLOWED' }); expect(commentInstance.getMuteModalStartStep()).toBe(1); }); From 5d7d1cde3dbc47df59deb2974b0c0c68a45a4801 Mon Sep 17 00:00:00 2001 From: seotts Date: Fri, 19 Mar 2021 15:18:36 -0400 Subject: [PATCH 4/6] Add comments; fix reply logic --- src/views/preview/comment/compose-comment.jsx | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/views/preview/comment/compose-comment.jsx b/src/views/preview/comment/compose-comment.jsx index 58c322ca4..2627334ed 100644 --- a/src/views/preview/comment/compose-comment.jsx +++ b/src/views/preview/comment/compose-comment.jsx @@ -28,9 +28,9 @@ const JUST_MUTED_ERROR = 'isBad'; const ComposeStatus = keyMirror({ EDITING: null, SUBMITTING: null, - REJECTED: null, - REJECTED_MUTE: null, - COMPOSE_DISALLOWED: null + REJECTED: null, // comment rejected for a reason other than muting (such as commenting too quickly) + REJECTED_MUTE: null, // comment made in this ComposeComment was rejected and muted the user + COMPOSE_DISALLOWED: null // user is already muted due to past behavior }); class ComposeComment extends React.Component { @@ -186,14 +186,14 @@ class ComposeComment extends React.Component { return false; } - // TODO: Check with Kathy, but we think you should always see the modal when you reply? - if (this.props.isReply) { - return true; - } - // If the user is already muted (for example, in a different tab), - // do not show modal because it would be confusing + // do not show modal unless the comment is a reply. We always want to show + // the modal on replies when the user is already muted because the blue box + // may be out-of-sight for them. if (!justMuted) { + if (this.props.isReply) { + return true; + } return false; } @@ -284,11 +284,13 @@ class ComposeComment extends React.Component { render () { return ( - {(this.isMuted() && !(this.props.isReply && this.state.status !== ComposeStatus.REJECTED_MUTE)) ? ( + {/* If a user is muted, show the blue mute box, unless + the comment is a reply and the user was already muted before attempting to make it. */} + {(this.isMuted() && !(this.props.isReply && this.state.status === ComposeStatus.COMPOSE_DISALLOWED)) ? (

- Date: Fri, 19 Mar 2021 15:29:29 -0400 Subject: [PATCH 5/6] Add test of shoudShowMuteModal when isReply --- test/unit/components/compose-comment.test.jsx | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/test/unit/components/compose-comment.test.jsx b/test/unit/components/compose-comment.test.jsx index a454799f5..6caf12314 100644 --- a/test/unit/components/compose-comment.test.jsx +++ b/test/unit/components/compose-comment.test.jsx @@ -458,7 +458,25 @@ describe('Compose Comment test', () => { global.Date.now = realDateNow; }); - test('getMuteModalStartStep: not a reply ', () => { + test('shouldShowMuteModal is true when the user is already muted if the comment is a reply', () => { + const realDateNow = Date.now.bind(global.Date); + global.Date.now = () => 0; + // Since Date.now mocked to 0 above, we just need a small number to make + // it look like it was created < 2 minutes ago. + const offense = { + expiresAt: '1000', + createdAt: '-60' // ~1 ago min given shouldShowMuteModal's conversions, + }; + const muteStatus = { + offenses: [offense] + }; + const justMuted = false; + const commentInstance = getComposeCommentWrapper({isReply: true}).instance(); + expect(commentInstance.shouldShowMuteModal(muteStatus, justMuted)).toBe(true); + global.Date.now = realDateNow; + }); + + test('getMuteModalStartStep: not a reply', () => { const commentInstance = getComposeCommentWrapper({}).instance(); expect(commentInstance.getMuteModalStartStep()).toBe(0); }); From d8a802fca28ba3ece3ff940ad3b9a9b6b78c585b Mon Sep 17 00:00:00 2001 From: seotts Date: Fri, 19 Mar 2021 15:42:54 -0400 Subject: [PATCH 6/6] Test that status gets correctly set in constructor --- test/unit/components/compose-comment.test.jsx | 33 +++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/test/unit/components/compose-comment.test.jsx b/test/unit/components/compose-comment.test.jsx index 6caf12314..226d80062 100644 --- a/test/unit/components/compose-comment.test.jsx +++ b/test/unit/components/compose-comment.test.jsx @@ -51,6 +51,36 @@ describe('Compose Comment test', () => { return wrapper.dive(); // unwrap redux connect(injectIntl(ComposeComment)) }; + test('status is EDITING when props do not contain a muteStatus ', () => { + const commentInstance = getComposeCommentWrapper({}).instance(); + expect(commentInstance.state.status).toBe('EDITING'); + }); + + test('status is COMPOSE_DISALLOWED when props contain a future mute', () => { + jest.useFakeTimers(); + const realDateNow = Date.now.bind(global.Date); + global.Date.now = () => 0; + const mutedStore = mockStore({ + session: { + session: { + user: {}, + permissions: { + mute_status: { + muteExpiresAt: 5, + offenses: [], + showWarning: true + } + } + } + } + }); + const component = getComposeCommentWrapper({}, mutedStore); + const commentInstance = component.instance(); + + expect(commentInstance.state.status).toBe('COMPOSE_DISALLOWED'); + global.Date.now = realDateNow; + }); + test('Modal & Comment status do not show ', () => { const component = getComposeCommentWrapper({}); // Comment compsoe box is there @@ -176,7 +206,7 @@ describe('Compose Comment test', () => { expect(component.find('CommentingStatus').exists()).toEqual(true); global.Date.now = realDateNow; }); - + test('Comment Status shows when user just submitted a reply comment that got them muted', () => { const realDateNow = Date.now.bind(global.Date); global.Date.now = () => 0; @@ -360,7 +390,6 @@ describe('Compose Comment test', () => { expect(component.find('MuteModal').exists()).toEqual(true); expect(component.find('MuteModal').props().showFeedback).toBe(false); }); - test('shouldShowMuteModal is false when muteStatus is undefined ', () => { const commentInstance = getComposeCommentWrapper({}).instance(); expect(commentInstance.shouldShowMuteModal()).toBe(false);