From 47b267e2c561d5546ba47ef1d201f64a032d37ec Mon Sep 17 00:00:00 2001 From: Caleb Guanzon Date: Tue, 21 Jun 2022 11:02:42 -0600 Subject: [PATCH] reuse the same mutation hook for read state flag=react_inbox refs VICE-2801 note: this commit also deletes CanvasInboxFullPage tests since they will be difficult to maintain now that helper methods are no longer built-in to sub-components.these deleted tests are also already existent in selenium this commit is a simple refactor moves the mark as read unread mutation hook to canvas inbox so it can be reused in all 3 places test plan: - no regressions found when marking as read unread via the bubble, the top bar, and auto marker when viewing a message Change-Id: If9edf3d1eb142ce2725fd00d75a5b04c35ac6bf1 Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/294404 Tested-by: Service Cloud Jenkins Reviewed-by: Drake Harper Product-Review: Drake Harper QA-Review: Jason Gillett --- .../ConversationListHolder.js | 32 ++----------- .../ConversationListItem.js | 12 ++--- .../__tests__/ConversationListItem.test.js | 15 +++--- .../inbox/react/containers/CanvasInbox.js | 33 +++++++++++++ .../containers/ConversationListContainer.js | 21 +++++++- .../MessageDetailContainer.js | 26 ++-------- .../__tests__/MessageDetailContainer.test.js | 13 +++-- .../containers/MessageListActionContainer.js | 41 ++-------------- .../__tests__/CanvasInboxFullPage.test.js | 48 ------------------- 9 files changed, 86 insertions(+), 155 deletions(-) diff --git a/ui/features/inbox/react/components/ConversationListHolder/ConversationListHolder.js b/ui/features/inbox/react/components/ConversationListHolder/ConversationListHolder.js index c7b582f60c5..93187009d85 100644 --- a/ui/features/inbox/react/components/ConversationListHolder/ConversationListHolder.js +++ b/ui/features/inbox/react/components/ConversationListHolder/ConversationListHolder.js @@ -22,14 +22,12 @@ import {Spinner} from '@instructure/ui-spinner' import {useScope as useI18nScope} from '@canvas/i18n' import PropTypes from 'prop-types' import React, {useEffect, useState, useContext, useCallback, useMemo} from 'react' -import {useMutation} from 'react-apollo' import InboxEmpty from '../../../svg/inbox-empty.svg' import {ConversationContext} from '../../../util/constants' import {Text} from '@instructure/ui-text' import {View} from '@instructure/ui-view' import {ConversationListItem} from './ConversationListItem' -import {UPDATE_CONVERSATION_PARTICIPANTS} from '../../../graphql/Mutations' const I18n = useI18nScope('conversations_2') @@ -42,7 +40,7 @@ export const ConversationListHolder = ({ }) => { const [selectedMessages, setSelectedMessages] = useState([]) const [rangeClickStart, setRangeClickStart] = useState() - const {setOnFailure, setOnSuccess} = useContext(AlertManagerContext) + const {setOnFailure} = useContext(AlertManagerContext) const {setMultiselect, isSubmissionCommentsType} = useContext(ConversationContext) const [menuData, setMenuData] = useState([...props.conversations]) @@ -186,27 +184,6 @@ export const ConversationListHolder = ({ setSelectedMessages([...updatedSelectedMessage]) } - const [readStateChangeConversationParticipants] = useMutation(UPDATE_CONVERSATION_PARTICIPANTS, { - onCompleted(data) { - if (data.updateConversationParticipants.errors) { - setOnFailure(I18n.t('Read state change operation failed')) - } else { - setOnSuccess( - I18n.t( - { - one: 'Read state Changed!', - other: 'Read states Changed!' - }, - {count: '1000'} - ) - ) - } - }, - onError() { - setOnFailure(I18n.t('Read state change failed')) - } - }) - // Render no results found item const renderNoResultsFound = () => { return ( @@ -248,9 +225,8 @@ export const ConversationListHolder = ({ onSelect={handleItemSelection} onStar={props.onStar} key={conversation._id} - readStateChangeConversationParticipants={ - isSubmissionCommentsType ? () => {} : readStateChangeConversationParticipants - } + onMarkAsRead={isSubmissionCommentsType ? () => {} : props.onMarkAsRead} + onMarkAsUnread={isSubmissionCommentsType ? () => {} : props.onMarkAsUnread} textSize={props.textSize} /> @@ -320,6 +296,8 @@ ConversationListHolder.propTypes = { id: PropTypes.string, onSelect: PropTypes.func, onStar: PropTypes.func, + onMarkAsRead: PropTypes.func, + onMarkAsUnread: PropTypes.func, textSize: PropTypes.string, datatestid: PropTypes.string, /** diff --git a/ui/features/inbox/react/components/ConversationListHolder/ConversationListItem.js b/ui/features/inbox/react/components/ConversationListHolder/ConversationListItem.js index 4f61616b6db..f47cb40a2eb 100644 --- a/ui/features/inbox/react/components/ConversationListHolder/ConversationListItem.js +++ b/ui/features/inbox/react/components/ConversationListHolder/ConversationListItem.js @@ -161,12 +161,9 @@ export const ConversationListItem = ({...props}) => { margin="x-small" onClick={e => { e.stopPropagation() - props.readStateChangeConversationParticipants({ - variables: { - conversationIds: [props.conversation._id], - workflowState: props.isUnread ? 'read' : 'unread' - } - }) + props.isUnread + ? props.onMarkAsRead(props.conversation._id) + : props.onMarkAsUnread(props.conversation._id) }} screenReaderLabel={props.isUnread ? I18n.t('Unread') : I18n.t('Read')} size="small" @@ -276,6 +273,7 @@ ConversationListItem.propTypes = { isUnread: PropTypes.bool, onSelect: PropTypes.func, onStar: PropTypes.func, - readStateChangeConversationParticipants: PropTypes.func, + onMarkAsRead: PropTypes.func, + onMarkAsUnread: PropTypes.func, textSize: PropTypes.string } diff --git a/ui/features/inbox/react/components/ConversationListHolder/__tests__/ConversationListItem.test.js b/ui/features/inbox/react/components/ConversationListHolder/__tests__/ConversationListItem.test.js index d1f4d33c3a5..c08befea32a 100644 --- a/ui/features/inbox/react/components/ConversationListHolder/__tests__/ConversationListItem.test.js +++ b/ui/features/inbox/react/components/ConversationListHolder/__tests__/ConversationListItem.test.js @@ -86,6 +86,8 @@ describe('ConversationListItem', () => { onSelect: jest.fn(), onOpen: jest.fn(), onStar: jest.fn(), + onMarkAsRead: jest.fn(), + onMarkAsUnRead: jest.fn(), readStateChangeConversationParticipants: jest.fn(), ...overrides } @@ -175,21 +177,18 @@ describe('ConversationListItem', () => { }) it('update read state called with correct parameters', () => { - const changeReadState = jest.fn() + const onMarkAsUnread = jest.fn() - const props = createProps({readStateChangeConversationParticipants: changeReadState}) + const props = createProps({ + onMarkAsUnread + }) const container = render() const unreadBadge = container.queryByTestId('read-badge') fireEvent.click(unreadBadge) - expect(changeReadState).toHaveBeenCalledWith({ - variables: { - conversationIds: ['1'], - workflowState: 'unread' - } - }) + expect(onMarkAsUnread).toHaveBeenCalledWith('1') }) }) diff --git a/ui/features/inbox/react/containers/CanvasInbox.js b/ui/features/inbox/react/containers/CanvasInbox.js index c390339c505..c6b85fc8619 100644 --- a/ui/features/inbox/react/containers/CanvasInbox.js +++ b/ui/features/inbox/react/containers/CanvasInbox.js @@ -409,6 +409,36 @@ const CanvasInbox = () => { }) } + const [readStateChangeConversationParticipants] = useMutation(UPDATE_CONVERSATION_PARTICIPANTS, { + onCompleted(data) { + if (data.updateConversationParticipants.errors) { + setOnFailure(I18n.t('Read state change operation failed')) + } else { + setOnSuccess( + I18n.t( + { + one: 'Read state Changed!', + other: 'Read states Changed!' + }, + {count: '1000'} + ) + ) + } + }, + onError() { + setOnFailure(I18n.t('Read state change failed')) + } + }) + + const handleReadState = (markAsRead, conversationIds = null) => { + readStateChangeConversationParticipants({ + variables: { + conversationIds: conversationIds || selectedConversations.map(convo => convo._id), + workflowState: markAsRead + } + }) + } + const onReply = ({conversationMessage = null, replyAll = false} = {}) => { conversationMessage = isSubmissionCommentsType ? {} : conversationMessage setSelectedConversationMessage(conversationMessage) @@ -495,6 +525,7 @@ const CanvasInbox = () => { onStar={handleStar} firstConversationIsStarred={firstConversationIsStarred} onDelete={handleDelete} + onReadStateChange={handleReadState} canReply={canReply} /> @@ -510,6 +541,7 @@ const CanvasInbox = () => { userFilter={userFilter} scope={scope} onSelectConversation={updateSelectedConversations} + onReadStateChange={handleReadState} /> )} @@ -572,6 +604,7 @@ const CanvasInbox = () => { } : null } + onReadStateChange={handleReadState} scope={scope} /> diff --git a/ui/features/inbox/react/containers/ConversationListContainer.js b/ui/features/inbox/react/containers/ConversationListContainer.js index 99e89c8b138..d5157e0b192 100644 --- a/ui/features/inbox/react/containers/ConversationListContainer.js +++ b/ui/features/inbox/react/containers/ConversationListContainer.js @@ -33,7 +33,13 @@ import {Responsive} from '@instructure/ui-responsive' const I18n = useI18nScope('conversations_2') -const ConversationListContainer = ({course, scope, onSelectConversation, userFilter}) => { +const ConversationListContainer = ({ + course, + scope, + onSelectConversation, + onReadStateChange, + userFilter +}) => { const {setOnFailure, setOnSuccess} = useContext(AlertManagerContext) const {isSubmissionCommentsType} = useContext(ConversationContext) const [isLoadingMoreData, setIsLoadingMoreData] = useState(false) @@ -69,6 +75,14 @@ const ConversationListContainer = ({course, scope, onSelectConversation, userFil }) } + const handleMarkAsUnread = conversationId => { + onReadStateChange('unread', [conversationId]) + } + + const handleMarkAsRead = conversationId => { + onReadStateChange('read', [conversationId]) + } + const conversationsQuery = useQuery(CONVERSATIONS_QUERY, { variables: {userID, scope, filter: [userFilter, course]}, fetchPolicy: 'cache-and-network', @@ -209,6 +223,8 @@ const ConversationListContainer = ({course, scope, onSelectConversation, userFil conversations={inboxItemData} onSelect={onSelectConversation} onStar={handleStar} + onMarkAsRead={handleMarkAsRead} + onMarkAsUnread={handleMarkAsUnread} textSize={responsiveProps.textSize} datatestid={responsiveProps.datatestid} hasMoreMenuData={ @@ -233,7 +249,8 @@ ConversationListContainer.propTypes = { course: PropTypes.string, userFilter: PropTypes.number, scope: PropTypes.string, - onSelectConversation: PropTypes.func + onSelectConversation: PropTypes.func, + onReadStateChange: PropTypes.func } ConversationListContainer.defaultProps = { diff --git a/ui/features/inbox/react/containers/MessageDetailContainer/MessageDetailContainer.js b/ui/features/inbox/react/containers/MessageDetailContainer/MessageDetailContainer.js index 896488b1b9d..fb50f95e9bd 100644 --- a/ui/features/inbox/react/containers/MessageDetailContainer/MessageDetailContainer.js +++ b/ui/features/inbox/react/containers/MessageDetailContainer/MessageDetailContainer.js @@ -20,10 +20,7 @@ import {AlertManagerContext} from '@canvas/alerts/react/AlertManager' import {Conversation} from '../../../graphql/Conversation' import {ConversationContext} from '../../../util/constants' import {CONVERSATION_MESSAGES_QUERY, SUBMISSION_COMMENTS_QUERY} from '../../../graphql/Queries' -import { - DELETE_CONVERSATION_MESSAGES, - UPDATE_CONVERSATION_PARTICIPANTS -} from '../../../graphql/Mutations' +import {DELETE_CONVERSATION_MESSAGES} from '../../../graphql/Mutations' import {useScope as useI18nScope} from '@canvas/i18n' import {MessageDetailHeader} from '../../components/MessageDetailHeader/MessageDetailHeader' import {MessageDetailItem} from '../../components/MessageDetailItem/MessageDetailItem' @@ -52,19 +49,6 @@ export const MessageDetailContainer = props => { setLastMessageItem(refCurrent) }, []) - const [readStateChangeConversationParticipants] = useMutation(UPDATE_CONVERSATION_PARTICIPANTS, { - onCompleted(data) { - if (data.updateConversationParticipants.errors) { - setOnFailure(I18n.t('Read state change operation failed')) - } else { - setOnSuccess(I18n.t('Read state Changed!')) - } - }, - onError() { - setOnFailure(I18n.t('Read state change failed')) - } - }) - const removeConversationMessagesFromCache = (cache, result) => { const options = { query: CONVERSATION_MESSAGES_QUERY, @@ -127,12 +111,7 @@ export const MessageDetailContainer = props => { conversationMessagesQuery.data?.legacyNode && props.conversation.workflowState === 'unread' ) { - readStateChangeConversationParticipants({ - variables: { - conversationIds: [props.conversation._id], - workflowState: 'read' - } - }) + props.onReadStateChange('read', props.conversation._id) } // eslint-disable-next-line react-hooks/exhaustive-deps }, [conversationMessagesQuery.data]) @@ -366,6 +345,7 @@ MessageDetailContainer.propTypes = { onForward: PropTypes.func, onStar: PropTypes.func, onUnstar: PropTypes.func, + onReadStateChange: PropTypes.func, setCanReply: PropTypes.func, scope: PropTypes.string } diff --git a/ui/features/inbox/react/containers/MessageDetailContainer/__tests__/MessageDetailContainer.test.js b/ui/features/inbox/react/containers/MessageDetailContainer/__tests__/MessageDetailContainer.test.js index b1864879d9c..06d7ad84821 100644 --- a/ui/features/inbox/react/containers/MessageDetailContainer/__tests__/MessageDetailContainer.test.js +++ b/ui/features/inbox/react/containers/MessageDetailContainer/__tests__/MessageDetailContainer.test.js @@ -75,6 +75,7 @@ describe('MessageDetailContainer', () => { onReplyAll = jest.fn(), onDelete = jest.fn(), onForward = jest.fn(), + onReadStateChange = jest.fn(), setOnSuccess = jest.fn(), setCanReply = jest.fn(), overrideProps = {} @@ -89,6 +90,7 @@ describe('MessageDetailContainer', () => { onReplyAll={onReplyAll} onDelete={onDelete} onForward={onForward} + onReadStateChange={onReadStateChange} setCanReply={setCanReply} {...overrideProps} /> @@ -180,16 +182,19 @@ describe('MessageDetailContainer', () => { }) it('should mark loaded conversation as read', async () => { - const mockSetOnSuccess = jest.fn() + const mockReadStateChange = jest.fn() const container = setup({ - setOnSuccess: mockSetOnSuccess, - conversation: {...Conversation.mock(), workflowState: 'unread'} + conversation: { + ...Conversation.mock(), + workflowState: 'unread' + }, + onReadStateChange: mockReadStateChange }) // wait for query to load await container.findAllByTestId('message-more-options') await waitForApolloLoading() - expect(mockSetOnSuccess).toHaveBeenCalled() + expect(mockReadStateChange).toHaveBeenCalled() }) }) }) diff --git a/ui/features/inbox/react/containers/MessageListActionContainer.js b/ui/features/inbox/react/containers/MessageListActionContainer.js index 670d8a2b8f0..10ee0a7e178 100644 --- a/ui/features/inbox/react/containers/MessageListActionContainer.js +++ b/ui/features/inbox/react/containers/MessageListActionContainer.js @@ -18,14 +18,13 @@ import {AlertManagerContext} from '@canvas/alerts/react/AlertManager' import {COURSES_QUERY} from '../../graphql/Queries' -import {UPDATE_CONVERSATION_PARTICIPANTS} from '../../graphql/Mutations' import {CourseSelect, ALL_COURSES_ID} from '../components/CourseSelect/CourseSelect' import {Flex} from '@instructure/ui-flex' import {useScope as useI18nScope} from '@canvas/i18n' import {MailboxSelectionDropdown} from '../components/MailboxSelectionDropdown/MailboxSelectionDropdown' import {MessageActionButtons} from '../components/MessageActionButtons/MessageActionButtons' import PropTypes from 'prop-types' -import {useQuery, useMutation} from 'react-apollo' +import {useQuery} from 'react-apollo' import React, {useContext, useEffect} from 'react' import {reduceDuplicateCourses} from '../../util/courses_helper' import {View} from '@instructure/ui-view' @@ -37,7 +36,7 @@ const I18n = useI18nScope('conversations_2') const MessageListActionContainer = props => { const LIMIT_TAG_COUNT = 1 - const {setOnFailure, setOnSuccess} = useContext(AlertManagerContext) + const {setOnFailure} = useContext(AlertManagerContext) const userID = ENV.current_user_id?.toString() const selectedReadStates = () => { @@ -58,27 +57,6 @@ const MessageListActionContainer = props => { const hasSelectedConversations = () => props.selectedConversations.length > 0 - const [readStateChangeConversationParticipants] = useMutation(UPDATE_CONVERSATION_PARTICIPANTS, { - onCompleted(data) { - if (data.updateConversationParticipants.errors) { - setOnFailure(I18n.t('Read state change operation failed')) - } else { - setOnSuccess( - I18n.t( - { - one: 'Read state Changed!', - other: 'Read states Changed!' - }, - {count: props.selectedConversations.length} - ) - ) - } - }, - onError() { - setOnFailure(I18n.t('Read state change failed')) - } - }) - const {loading, error, data} = useQuery(COURSES_QUERY, { variables: {userID} }) @@ -128,21 +106,11 @@ const MessageListActionContainer = props => { } const handleMarkAsUnread = () => { - readStateChangeConversationParticipants({ - variables: { - conversationIds: props.selectedConversations.map(convo => convo._id), - workflowState: 'unread' - } - }) + props.onReadStateChange('unread') } const handleMarkAsRead = () => { - readStateChangeConversationParticipants({ - variables: { - conversationIds: props.selectedConversations.map(convo => convo._id), - workflowState: 'read' - } - }) + props.onReadStateChange('read') } return ( @@ -288,6 +256,7 @@ MessageListActionContainer.propTypes = { firstConversationIsStarred: PropTypes.bool, onStar: PropTypes.func, onDelete: PropTypes.func, + onReadStateChange: PropTypes.func, activeCourseFilter: PropTypes.string, canReply: PropTypes.bool } diff --git a/ui/features/inbox/react/containers/__tests__/CanvasInboxFullPage.test.js b/ui/features/inbox/react/containers/__tests__/CanvasInboxFullPage.test.js index 1133743e27c..6e1b166d19c 100644 --- a/ui/features/inbox/react/containers/__tests__/CanvasInboxFullPage.test.js +++ b/ui/features/inbox/react/containers/__tests__/CanvasInboxFullPage.test.js @@ -120,44 +120,6 @@ describe('CanvasInbox Full Page', () => { expect(sentConversationNodes[1]).toHaveTextContent('this is the second reply message') }) - it('renders the conversation messages', async () => { - const container = setup() - await waitForApolloLoading() - - const conversation = await container.findByTestId('conversationListItem-Checkbox') - fireEvent.click(conversation) - await waitForApolloLoading() - - expect(await container.findByText('this is the first reply message')).toBeInTheDocument() - expect(await container.findByText('this is a reply all')).toBeInTheDocument() - expect(await container.findByText('testing 123')).toBeInTheDocument() - }) - - it('should check then uncheck a checkbox', async () => { - const container = setup() - - const checkbox = await container.findByTestId('conversationListItem-Checkbox') - expect(checkbox.checked).toBeFalsy() - fireEvent.click(checkbox) - expect(checkbox.checked).toBeTruthy() - fireEvent.click(checkbox) - expect(checkbox.checked).toBeFalsy() - }) - - it('should trigger confirm when deleting from message kebab menu', async () => { - window.confirm = jest.fn(() => true) - const container = setup() - - const conversation = await container.findByTestId('conversationListItem-Checkbox') - fireEvent.click(conversation) - - const moreOptionsButtons = await container.findAllByTestId('message-more-options') - fireEvent.click(moreOptionsButtons[1]) - const deleteOption = await container.findByTestId('message-delete') - fireEvent.click(deleteOption) - expect(window.confirm).toHaveBeenCalled() - }) - it('should find desktop message list container', () => { const container = setup() @@ -174,16 +136,6 @@ describe('CanvasInbox Full Page', () => { expect(mailboxDropdown.getAttribute('value')).toBe('Inbox') }) - it('should respect the initial loading url hash', async () => { - window.location.hash = '#filter=type=sent' - const container = setup() - await waitForApolloLoading() - expect(window.location.hash).toBe('#filter=type=sent') - - const mailboxDropdown = await container.findByLabelText('Mailbox Selection') - expect(mailboxDropdown.getAttribute('value')).toBe('Sent') - }) - describe('scope select', () => { it('should update filter if url filter value is updated', async () => { const container = setup()