diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index d9735a94dc80..05924321d313 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -27,6 +27,7 @@ import Growl from '../Growl'; import * as Localize from '../Localize'; import PusherUtils from '../PusherUtils'; import DateUtils from '../DateUtils'; +import * as ReportActionsUtils from '../ReportActionsUtils'; let currentUserEmail; let currentUserAccountID; @@ -59,78 +60,55 @@ const allReports = {}; let conciergeChatReportID; const typingWatchTimers = {}; -/** - * Map of the most recent sequenceNumber for a reports_* key in Onyx by reportID. - * - * There are several sources that can set the most recent reportAction's sequenceNumber for a report: - * - * - Fetching the report object - * - Fetching the report history - * - Optimistically creating a report action - * - Handling a report action via Pusher - * - * Those values are stored in reportMaxSequenceNumbers and treated as the main source of truth for each report's max - * sequenceNumber. - */ -const reportMaxSequenceNumbers = {}; - -// Keeps track of the last read for each report -const lastReadSequenceNumbers = {}; - // Map of optimistic report action IDs. These should be cleared when replaced by a recent fetch of report history // since we will then be up to date and any optimistic actions that are still waiting to be replaced can be removed. const optimisticReportActionIDs = {}; -// Boolean to indicate if report data is loading from the API or not. -let isReportDataLoading = true; -Onyx.connect({ - key: ONYXKEYS.IS_LOADING_REPORT_DATA, - callback: val => isReportDataLoading = val, -}); - /** - * Checks the report to see if there are any unread action items - * - * @param {Object} report + * @param {Number} reportID + * @param {Number} lastReadSequenceNumber + * @param {Number} maxSequenceNumber * @returns {Boolean} */ -function getUnreadActionCount(report) { - const lastReadSequenceNumber = lodashGet(report, [ - 'reportNameValuePairs', - `lastRead_${currentUserAccountID}`, - 'sequenceNumber', - ]); - - // Save the lastReadActionID locally so we can access this later - lastReadSequenceNumbers[report.reportID] = lastReadSequenceNumber; - - if (report.reportActionCount === 0) { +function calculateUnreadActionCount( + reportID, + lastReadSequenceNumber, + maxSequenceNumber, +) { + if (maxSequenceNumber === 0) { return 0; } if (!lastReadSequenceNumber) { - return report.reportActionCount; + return maxSequenceNumber; } - // There are unread items if the last one the user has read is less - // than the highest sequence number we have - const unreadActionCount = report.reportActionCount - lastReadSequenceNumber - ReportActions.getDeletedCommentsCount(report.reportID, lastReadSequenceNumber); + const unreadActionCount = maxSequenceNumber - lastReadSequenceNumber - ReportActions.getDeletedCommentsCount(reportID, lastReadSequenceNumber); return Math.max(0, unreadActionCount); } /** - * Returns the number of unread actions for a reportID - * * @param {Number} reportID - * @param {Number} sequenceNumber * @returns {Number} */ -function getUnreadActionCountFromSequenceNumber(reportID, sequenceNumber) { - const reportMaxSequenceNumber = reportMaxSequenceNumbers[reportID]; +function getUnreadActionCount(reportID) { + return lodashGet(allReports, [reportID, 'unreadActionCount'], 0); +} + +/** + * @param {Number} reportID + * @returns {Number} + */ +function getLastReadSequenceNumber(reportID) { + return lodashGet(allReports, [reportID, 'lastReadSequenceNumber'], 0); +} - // Determine the number of unread actions by deducting the last read sequence from the total. If, for some reason, - // the last read sequence is higher than the actual last sequence, let's just assume all actions are read - return Math.max(reportMaxSequenceNumber - sequenceNumber - ReportActions.getDeletedCommentsCount(reportID, sequenceNumber), 0); +/** + * @param {Number} reportID + * @returns {Number} + */ +function getMaxSequenceNumber(reportID) { + return lodashGet(allReports, [reportID, 'maxSequenceNumber'], 0); } /** @@ -186,6 +164,11 @@ function getSimplifiedReportObject(report) { // Used for User Created Policy Rooms, will denote how access to a chat room is given among workspace members const visibility = lodashGet(report, ['reportNameValuePairs', 'visibility']); + const lastReadSequenceNumber = lodashGet(report, [ + 'reportNameValuePairs', + `lastRead_${currentUserAccountID}`, + 'sequenceNumber', + ]); return { reportID: report.reportID, @@ -193,7 +176,7 @@ function getSimplifiedReportObject(report) { chatType, ownerEmail: LoginUtils.getEmailWithoutMergedAccountPrefix(lodashGet(report, ['ownerEmail'], '')), policyID: lodashGet(report, ['reportNameValuePairs', 'expensify_policyID'], ''), - unreadActionCount: getUnreadActionCount(report), + unreadActionCount: calculateUnreadActionCount(report.reportID, lastReadSequenceNumber, report.reportActionCount), maxSequenceNumber: lodashGet(report, 'reportActionCount', 0), participants: getParticipantEmailsFromReport(report), isPinned: report.isPinned, @@ -202,6 +185,7 @@ function getSimplifiedReportObject(report) { `lastRead_${currentUserAccountID}`, 'timestamp', ], 0), + lastReadSequenceNumber, lastMessageTimestamp, lastMessageText: isLastMessageAttachment ? '[Attachment]' : lastMessageText, lastActorEmail, @@ -416,22 +400,6 @@ function setLocalIOUReportData(iouReportObject) { Onyx.merge(iouReportKey, iouReportObject); } -/** - * Update the lastRead actionID and timestamp in local memory and Onyx - * - * @param {Number} reportID - * @param {Number} lastReadSequenceNumber - */ -function setLocalLastRead(reportID, lastReadSequenceNumber) { - lastReadSequenceNumbers[reportID] = lastReadSequenceNumber; - - // Update the report optimistically. - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - unreadActionCount: getUnreadActionCountFromSequenceNumber(reportID, lastReadSequenceNumber), - lastVisitedTimestamp: Date.now(), - }); -} - /** * Remove all optimistic actions from report actions and reset the optimisticReportActionsIDs array. We do this * to clear any stuck optimistic actions that have not be updated for whatever reason. @@ -520,28 +488,6 @@ function setNewMarkerPosition(reportID, sequenceNumber) { }); } -/** - * Updates a report action's message to be a new value. - * - * @param {Number} reportID - * @param {Number} sequenceNumber - * @param {Object} message - */ -function updateReportActionMessage(reportID, sequenceNumber, message) { - const actionToMerge = {}; - actionToMerge[sequenceNumber] = {message: [message]}; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionToMerge).then(() => { - // If the message is deleted, update the last read message and the unread counter - if (!message.html) { - setLocalLastRead(reportID, lastReadSequenceNumbers[reportID]); - } - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - lastMessageText: ReportActions.getLastVisibleMessageText(reportID), - }); - }); -} - /** * Get the private pusher channel name for a Report. * @@ -569,7 +515,29 @@ function subscribeToUserEvents() { // Live-update a report's actions when an 'edit comment' event is received. PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.REPORT_COMMENT_EDIT, currentUserAccountID, - pushJSON => updateReportActionMessage(pushJSON.reportID, pushJSON.sequenceNumber, pushJSON.message)); + ({reportID, sequenceNumber, message}) => { + // We only want the active client to process these events once otherwise multiple tabs would decrement the 'unreadActionCount' + if (!ActiveClientManager.isClientTheLeader()) { + return; + } + + const actionsToMerge = {}; + actionsToMerge[sequenceNumber] = {message: [message]}; + + // If someone besides the current user deleted an action and the sequenceNumber is greater than our last read we will decrement the unread count + // we skip this for the current user because we should already have decremented the count optimistically when they deleted the comment. + const isFromCurrentUser = ReportActions.isFromCurrentUser(reportID, sequenceNumber, currentUserAccountID, actionsToMerge); + if (!message.html && !isFromCurrentUser && sequenceNumber > getLastReadSequenceNumber(reportID)) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + unreadActionCount: Math.max(getUnreadActionCount(reportID) - 1, 0), + }); + } + + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + lastMessageText: ReportActions.getLastVisibleMessageText(reportID, actionsToMerge), + }); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionsToMerge); + }); } /** @@ -819,7 +787,7 @@ function fetchAllReports( // data processing by Onyx. const reportIDsWithMissingActions = _.chain(returnedReports) .map(report => report.reportID) - .filter(reportID => ReportActions.isReportMissingActions(reportID, reportMaxSequenceNumbers[reportID])) + .filter(reportID => ReportActions.isReportMissingActions(reportID, lodashGet(allReports, [reportID, 'maxSequenceNumber']))) .value(); // Once we have the reports that are missing actions we will find the intersection between the most @@ -868,7 +836,7 @@ function addComment(reportID, text, file) { const attachmentInfo = isAttachment ? file : {}; // The new sequence number will be one higher than the highest - const highestSequenceNumber = reportMaxSequenceNumbers[reportID] || 0; + const highestSequenceNumber = getMaxSequenceNumber(reportID); const newSequenceNumber = highestSequenceNumber + 1; const htmlForNewComment = isAttachment ? 'Uploading Attachment...' : commentText; @@ -882,6 +850,8 @@ function addComment(reportID, text, file) { lastMessageTimestamp: moment().unix(), lastMessageText: ReportUtils.formatReportLastMessageText(textForNewComment), lastActorEmail: currentUserEmail, + unreadActionCount: 0, + lastReadSequenceNumber: newSequenceNumber, }; // Generate a clientID so we can save the optimistic action to storage with the clientID as key. Later, we will @@ -996,15 +966,6 @@ function addAttachment(reportID, file) { addComment(reportID, '', file); } -/** - * Get the last read sequence number for a report - * @param {String|Number} reportID - * @return {Number} - */ -function getLastReadSequenceNumber(reportID) { - return lastReadSequenceNumbers[reportID]; -} - /** * Deletes a comment from the report, basically sets it as empty string * @@ -1027,8 +988,17 @@ function deleteReportComment(reportID, reportAction) { ], }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge).then(() => { - setLocalLastRead(reportID, getLastReadSequenceNumber(reportID)); + // If the comment we are deleting is more recent than our last read comment we will update the unread count + if (sequenceNumber > getLastReadSequenceNumber(reportID)) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + unreadActionCount: Math.max(getUnreadActionCount(reportID) - 1, 0), + }); + } + + // Optimistically update the report and reportActions + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + lastMessageText: ReportActions.getLastVisibleMessageText(reportID, reportActionsToMerge), }); // Try to delete the comment by calling the API @@ -1048,50 +1018,19 @@ function deleteReportComment(reportID, reportAction) { ...reportAction, message: oldMessage, }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge).then(() => { - setLocalLastRead(reportID, getLastReadSequenceNumber(reportID)); - }); - }); -} -/** - * Updates the last read action ID on the report. It optimistically makes the change to the store, and then let's the - * network layer handle the delayed write. - * - * @param {Number} reportID - * @param {Number} [sequenceNumber] This can be used to set the last read actionID to a specific - * @param {Boolean} [manuallyMarked] If the user manually marked this as unread, we need to tell the API - * spot (eg. mark-as-unread). Otherwise, when this param is omitted, the highest sequence number becomes the one that - * is last read (meaning that the entire report history has been read) - */ -function updateLastReadActionID(reportID, sequenceNumber, manuallyMarked = false) { - // If report data is loading, we can't update the last read sequence number because it is obsolete - if (isReportDataLoading) { - return; - } - - // If we aren't specifying a sequenceNumber and have no valid maxSequenceNumber for this report then we should not - // update the last read. Most likely, we have just created the report and it has no comments. But we should err on - // the side of caution and do nothing in this case. - if (_.isUndefined(sequenceNumber) - && (!reportMaxSequenceNumbers[reportID] && reportMaxSequenceNumbers[reportID] !== 0)) { - return; - } - - // Need to subtract 1 from sequenceNumber so that the "New" marker appears in the right spot (the last read - // action). If 1 isn't subtracted then the "New" marker appears one row below the action (the first unread action) - // Note: sequenceNumber can be 1 for the first message, so we can't use - // (sequenceNumber - 1) || reportMaxSequenceNumbers[reportID] because the first expression results in 0 which is falsy. - const lastReadSequenceNumber = _.isNumber(sequenceNumber) ? (sequenceNumber - 1) : reportMaxSequenceNumbers[reportID]; + if (sequenceNumber > getLastReadSequenceNumber(reportID)) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + unreadActionCount: getUnreadActionCount(reportID) + 1, + }); + } - setLocalLastRead(reportID, lastReadSequenceNumber); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { + lastMessageText: ReportActions.getLastVisibleMessageText(reportID, reportActionsToMerge), + }); - // Mark the report as not having any unread items - DeprecatedAPI.Report_UpdateLastRead({ - reportID, - sequenceNumber: lastReadSequenceNumber, - markAsUnread: manuallyMarked, - }); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); + }); } /** @@ -1100,8 +1039,7 @@ function updateLastReadActionID(reportID, sequenceNumber, manuallyMarked = false * @param {Number} reportID */ function openReport(reportID) { - const sequenceNumber = reportMaxSequenceNumbers[reportID]; - lastReadSequenceNumbers[reportID] = sequenceNumber; + const sequenceNumber = getMaxSequenceNumber(reportID); API.write('OpenReport', { reportID, @@ -1109,11 +1047,12 @@ function openReport(reportID) { }, { optimisticData: [{ - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { + lastReadSequenceNumber: sequenceNumber, lastVisitedTimestamp: Date.now(), - unreadActionCount: getUnreadActionCountFromSequenceNumber(reportID, sequenceNumber), + unreadActionCount: 0, }, }], }); @@ -1125,8 +1064,7 @@ function openReport(reportID) { * @param {Number} reportID */ function readNewestAction(reportID) { - const sequenceNumber = reportMaxSequenceNumbers[reportID]; - lastReadSequenceNumbers[reportID] = sequenceNumber; + const sequenceNumber = getMaxSequenceNumber(reportID); API.write('ReadNewestAction', { reportID, @@ -1134,11 +1072,12 @@ function readNewestAction(reportID) { }, { optimisticData: [{ - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { + lastReadSequenceNumber: sequenceNumber, lastVisitedTimestamp: Date.now(), - unreadActionCount: getUnreadActionCountFromSequenceNumber(reportID, sequenceNumber), + unreadActionCount: 0, }, }], }); @@ -1151,19 +1090,22 @@ function readNewestAction(reportID) { * @param {Number} sequenceNumber */ function markCommentAsUnread(reportID, sequenceNumber) { - lastReadSequenceNumbers[reportID] = sequenceNumber; - API.write('MarkCommentAsUnread', + const maxSequenceNumber = getMaxSequenceNumber(reportID); + const newLastReadSequenceNumber = sequenceNumber - 1; + API.write('MarkAsUnread', { reportID, sequenceNumber, }, { optimisticData: [{ - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`, value: { - lastVisitedTimestamp: Math.round(Date.now() / 1000), - unreadActionCount: getUnreadActionCountFromSequenceNumber(reportID, sequenceNumber), + newMarkerSequenceNumber: sequenceNumber, + lastReadSequenceNumber: newLastReadSequenceNumber, + lastVisitedTimestamp: Date.now(), + unreadActionCount: calculateUnreadActionCount(reportID, newLastReadSequenceNumber, maxSequenceNumber), }, }], }); @@ -1180,7 +1122,7 @@ function togglePinnedState(report) { // Optimistically pin/unpin the report before we send out the command const optimisticData = [ { - onyxMethod: 'merge', + onyxMethod: CONST.ONYX.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, value: {isPinned: pinnedValue}, }, @@ -1251,9 +1193,6 @@ function handleReportChanged(report) { fetchChatReportsByIDs([report.reportID]); } - // Store the max sequence number for each report - reportMaxSequenceNumbers[report.reportID] = report.maxSequenceNumber; - // Store optimistic actions IDs for each report optimisticReportActionIDs[report.reportID] = report.optimisticReportActionIDs; } @@ -1486,22 +1425,22 @@ function setIsComposerFullSize(reportID, isComposerFullSize) { * @param {Object} action */ function viewNewReportAction(reportID, action) { - const newMaxSequenceNumber = action.sequenceNumber; + const incomingSequenceNumber = action.sequenceNumber; const isFromCurrentUser = action.actorAccountID === currentUserAccountID; + const lastReadSequenceNumber = getLastReadSequenceNumber(reportID); + const updatedReportObject = {}; - // When handling an action from the current users we can assume that their - // last read actionID has been updated in the server but not necessarily reflected - // locally so we must first update it and then calculate the unread (which should be 0) + // When handling an action from the current user we can assume that their last read actionID has been updated in the server, but not necessarily reflected + // locally (e.g. could be from a different session) so we set their unreadActionCount to zero. If the action is from another user we will only update the + // unreadActionCount if the incoming sequenceNumber is higher than the last read for the user. if (isFromCurrentUser) { - setLocalLastRead(reportID, newMaxSequenceNumber); + updatedReportObject.unreadActionCount = 0; + updatedReportObject.lastVisitedTimestamp = Date.now(); + updatedReportObject.lastReadSequenceNumber = action.sequenceNumber; + } else if (incomingSequenceNumber > lastReadSequenceNumber) { + updatedReportObject.unreadActionCount = getUnreadActionCount(reportID) + 1; } - const updatedReportObject = { - // Use updated lastReadSequenceNumber, value may have been modified by setLocalLastRead - // Here deletedComments count does not include the new action being added. We can safely assume that newly received action is not deleted. - unreadActionCount: newMaxSequenceNumber - (lastReadSequenceNumbers[reportID] || 0) - ReportActions.getDeletedCommentsCount(reportID, lastReadSequenceNumbers[reportID] || 0), - }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject); // If chat report receives an action with IOU and we have an IOUReportID, update IOU object @@ -1548,8 +1487,7 @@ function viewNewReportAction(reportID, action) { // When a new message comes in, if the New marker is not already set (newMarkerSequenceNumber === 0), set the marker above the incoming message. const report = lodashGet(allReports, 'reportID', {}); if (lodashGet(report, 'newMarkerSequenceNumber', 0) === 0 && report.unreadActionCount > 0) { - const oldestUnreadSeq = (report.maxSequenceNumber - report.unreadActionCount) + 1; - setNewMarkerPosition(reportID, oldestUnreadSeq); + setNewMarkerPosition(reportID, report.lastReadSequenceNumber + 1); } Log.info('[LOCAL_NOTIFICATION] Creating notification'); @@ -1562,6 +1500,8 @@ function viewNewReportAction(reportID, action) { }); } +// We are using this map to ensure actions are only handled once +const handledReportActions = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, initWithStoredValues: false, @@ -1573,6 +1513,18 @@ Onyx.connect({ } _.each(actions, (action) => { + if (lodashGet(handledReportActions, [reportID, action.sequenceNumber])) { + return; + } + + if (ReportActionsUtils.isDeletedAction(action)) { + return; + } + + if (action.isLoading) { + return; + } + if (!action.timestamp) { return; } @@ -1583,6 +1535,8 @@ Onyx.connect({ } viewNewReportAction(reportID, action); + handledReportActions[reportID] = handledReportActions[reportID] || {}; + handledReportActions[reportID][action.sequenceNumber] = true; }); }, }); @@ -1596,7 +1550,6 @@ export { fetchIOUReportByIDAndUpdateChatReport, addComment, addAttachment, - updateLastReadActionID, updateNotificationPreference, setNewMarkerPosition, subscribeToReportTypingEvents, @@ -1618,7 +1571,6 @@ export { fetchActionsWithLoadingState, createPolicyRoom, renameReport, - getLastReadSequenceNumber, setIsComposerFullSize, markCommentAsUnread, readNewestAction, diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index a069aa6b115c..8b87ccb8480f 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -1,6 +1,7 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; import lodashGet from 'lodash/get'; +import lodashMerge from 'lodash/merge'; import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import ONYXKEYS from '../../ONYXKEYS'; import * as CollectionUtils from '../CollectionUtils'; @@ -101,22 +102,38 @@ function getDeletedCommentsCount(reportID, sequenceNumber) { /** * Get the message text for the last action that was not deleted * @param {Number} reportID + * @param {Object} [actionsToMerge] * @return {String} */ -function getLastVisibleMessageText(reportID) { +function getLastVisibleMessageText(reportID, actionsToMerge = {}) { const parser = new ExpensiMark(); - const lastMessageIndex = _.findLastIndex(reportActions[reportID], action => ( + const existingReportActions = _.indexBy(reportActions[reportID], 'sequenceNumber'); + const actions = _.toArray(lodashMerge({}, existingReportActions, actionsToMerge)); + const lastMessageIndex = _.findLastIndex(actions, action => ( !ReportActionsUtils.isDeletedAction(action) )); - const htmlText = lodashGet(reportActions, [reportID, lastMessageIndex, 'message', 0, 'html'], ''); + const htmlText = lodashGet(actions, [lastMessageIndex, 'message', 0, 'html'], ''); const messageText = parser.htmlToText(htmlText); - return ReportUtils.formatReportLastMessageText(messageText); } +/** + * @param {Number} reportID + * @param {Number} sequenceNumber + * @param {Number} currentUserAccountID + * @param {Object} [actionsToMerge] + * @returns {Boolean} + */ +function isFromCurrentUser(reportID, sequenceNumber, currentUserAccountID, actionsToMerge = {}) { + const existingReportActions = _.indexBy(reportActions[reportID], 'sequenceNumber'); + const action = lodashMerge({}, existingReportActions, actionsToMerge)[sequenceNumber]; + return action.actorAccountID === currentUserAccountID; +} + export { isReportMissingActions, dangerouslyGetReportActionsMaxSequenceNumber, getDeletedCommentsCount, getLastVisibleMessageText, + isFromCurrentUser, }; diff --git a/src/libs/deprecatedAPI.js b/src/libs/deprecatedAPI.js index 24a9c3e9c73c..c08340bced61 100644 --- a/src/libs/deprecatedAPI.js +++ b/src/libs/deprecatedAPI.js @@ -356,18 +356,6 @@ function Report_EditComment(parameters) { return Network.post(commandName, parameters); } -/** - * @param {Object} parameters - * @param {Number} parameters.reportID - * @param {Number} parameters.sequenceNumber - * @returns {Promise} - */ -function Report_UpdateLastRead(parameters) { - const commandName = 'Report_UpdateLastRead'; - requireParameters(['reportID', 'sequenceNumber'], parameters, commandName); - return Network.post(commandName, parameters); -} - /** * @param {Object} parameters * @param {String} parameters.email @@ -942,7 +930,6 @@ export { Report_GetHistory, Report_TogglePinned, Report_EditComment, - Report_UpdateLastRead, ResendValidateCode, ResetPassword, SetNameValuePair, diff --git a/src/pages/home/report/ContextMenu/ContextMenuActions.js b/src/pages/home/report/ContextMenu/ContextMenuActions.js index 34cb41968eb1..575e2ff1686d 100644 --- a/src/pages/home/report/ContextMenu/ContextMenuActions.js +++ b/src/pages/home/report/ContextMenu/ContextMenuActions.js @@ -140,7 +140,6 @@ export default [ shouldShow: type => type === CONTEXT_MENU_TYPES.REPORT_ACTION, onPress: (closePopover, {reportAction, reportID}) => { Report.markCommentAsUnread(reportID, reportAction.sequenceNumber); - Report.setNewMarkerPosition(reportID, reportAction.sequenceNumber); if (closePopover) { hideContextMenu(true, ReportActionComposeFocusManager.focus); } diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 4f517bb3b902..2348489c9a8b 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -115,13 +115,13 @@ class ReportActionsView extends React.Component { this.updateMessageCounterCount = this.updateMessageCounterCount.bind(this); this.loadMoreChats = this.loadMoreChats.bind(this); this.recordTimeToMeasureItemLayout = this.recordTimeToMeasureItemLayout.bind(this); - this.scrollToBottomAndUpdateLastRead = this.scrollToBottomAndUpdateLastRead.bind(this); + this.scrollToBottomAndMarkReportAsRead = this.scrollToBottomAndMarkReportAsRead.bind(this); this.updateNewMarkerAndMarkReadOnce = _.once(this.updateNewMarkerAndMarkRead.bind(this)); } componentDidMount() { this.appStateChangeListener = AppState.addEventListener('change', () => { - if (!Visibility.isVisible() || this.props.isDrawerOpen) { + if (!this.getIsReportFullyVisible()) { return; } @@ -201,7 +201,9 @@ class ReportActionsView extends React.Component { componentDidUpdate(prevProps) { if (prevProps.network.isOffline && !this.props.network.isOffline) { - Report.openReport(this.props.reportID); + if (this.isReportFullyVisible()) { + Report.openReport(this.props.reportID); + } this.fetchData(); } @@ -216,9 +218,7 @@ class ReportActionsView extends React.Component { const currentLastSequenceNumber = lodashGet(CollectionUtils.lastItem(this.props.reportActions), 'sequenceNumber'); // Record the max action when window is visible and the sidebar is not covering the report view on a small screen - const isSidebarCoveringReportView = this.props.isSmallScreenWidth && this.props.isDrawerOpen; - const shouldRecordMaxAction = Visibility.isVisible() && !isSidebarCoveringReportView; - + const isReportFullyVisible = this.getIsReportFullyVisible(); const sidebarClosed = prevProps.isDrawerOpen && !this.props.isDrawerOpen; const screenSizeIncreased = prevProps.isSmallScreenWidth && !this.props.isSmallScreenWidth; const reportBecomeVisible = sidebarClosed || screenSizeIncreased; @@ -233,7 +233,7 @@ class ReportActionsView extends React.Component { // Only update the unread count when the floating message counter is visible // Otherwise counter will be shown on scrolling up from the bottom even if user have read those messages if (this.state.isFloatingMessageCounterVisible) { - this.updateMessageCounterCount(!shouldRecordMaxAction); + this.updateMessageCounterCount(!isReportFullyVisible); } // Show new floating message counter when there is a new message @@ -242,13 +242,13 @@ class ReportActionsView extends React.Component { // When the last action changes, record the max action // This will make the NEW marker line go away if you receive comments in the same chat you're looking at - if (shouldRecordMaxAction) { + if (isReportFullyVisible) { Report.readNewestAction(this.props.reportID); } } // Update the new marker position and last read action when we are closing the sidebar or moving from a small to large screen size - if (shouldRecordMaxAction && reportBecomeVisible) { + if (isReportFullyVisible && reportBecomeVisible) { this.updateNewMarkerPosition(this.props.report.unreadActionCount); Report.openReport(this.props.reportID); } @@ -266,6 +266,14 @@ class ReportActionsView extends React.Component { Report.unsubscribeFromReportChannel(this.props.reportID); } + /** + * @returns {Boolean} + */ + getIsReportFullyVisible() { + const isSidebarCoveringReportView = this.props.isSmallScreenWidth && this.props.isDrawerOpen; + return Visibility.isVisible() && !isSidebarCoveringReportView; + } + fetchData() { Report.fetchActions(this.props.reportID); } @@ -295,13 +303,10 @@ class ReportActionsView extends React.Component { Report.fetchActionsWithLoadingState(this.props.reportID, offset); } - /** - * This function is triggered from the ref callback for the scrollview. That way it can be scrolled once all the - * items have been rendered. If the number of actions has changed since it was last rendered, then - * scroll the list to the end. As a report can contain non-message actions, we should confirm that list data exists. - */ - scrollToBottomAndUpdateLastRead() { + scrollToBottomAndMarkReportAsRead() { ReportScrollManager.scrollToBottom(); + Report.readNewestAction(this.props.reportID); + Report.setNewMarkerPosition(this.props.reportID, 0); } /** @@ -312,7 +317,7 @@ class ReportActionsView extends React.Component { // Since we want the New marker to remain in place even if newer messages come in, we set it once on mount. // We determine the last read action by deducting the number of unread actions from the total number. // Then, we add 1 because we want the New marker displayed over the oldest unread sequence. - const oldestUnreadSequenceNumber = unreadActionCount === 0 ? 0 : Report.getLastReadSequenceNumber(this.props.report.reportID) + 1; + const oldestUnreadSequenceNumber = unreadActionCount === 0 ? 0 : this.props.report.lastReadSequenceNumber + 1; Report.setNewMarkerPosition(this.props.reportID, oldestUnreadSequenceNumber); } @@ -351,8 +356,8 @@ class ReportActionsView extends React.Component { updateNewMarkerAndMarkRead() { this.updateNewMarkerPosition(this.props.report.unreadActionCount); - // Only mark as read if the report is open - if (!this.props.isDrawerOpen) { + // Only mark as read if the report is fully visible + if (this.getIsReportFullyVisible()) { Report.openReport(this.props.reportID); } } @@ -417,7 +422,7 @@ class ReportActionsView extends React.Component { { Pusher.unsubscribe(`${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}1${CONFIG.PUSHER.SUFFIX}`); }); - it('should store a new report action in Onyx when reportComment event is handled via Pusher', () => { + it('should store a new report action in Onyx when onyxApiUpdate event is handled via Pusher', () => { global.fetch = TestHelper.getGlobalFetchMock(); const TEST_USER_ACCOUNT_ID = 1; @@ -106,6 +107,8 @@ describe('actions/Report', () => { // We subscribed to the Pusher channel above and now we need to simulate a reportComment action // Pusher event so we can verify that action was handled correctly and merged into the reportActions. const channel = Pusher.getChannel(`${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}1${CONFIG.PUSHER.SUFFIX}`); + const actionWithoutLoading = {...resultAction}; + delete actionWithoutLoading.isLoading; channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ { onyxMethod: 'merge', @@ -124,7 +127,7 @@ describe('actions/Report', () => { key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, value: { [clientID]: null, - [ACTION_ID]: _.without(resultAction, 'loading'), + [ACTION_ID]: actionWithoutLoading, }, }, ]); @@ -212,4 +215,213 @@ describe('actions/Report', () => { expect(addCommentCalls.length).toBe(1); }); }); + + it('should be updated correctly when new comments are added, deleted or marked as unread', () => { + const REPORT_ID = 1; + + let report; + Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, + callback: val => report = val, + }); + + let reportActions; + Onyx.connect({ + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + callback: val => reportActions = val, + }); + + const USER_1_LOGIN = 'user@test.com'; + const USER_1_ACCOUNT_ID = 1; + const USER_2_LOGIN = 'different-user@test.com'; + const USER_2_ACCOUNT_ID = 2; + const channel = Pusher.getChannel(`${CONST.PUSHER.PRIVATE_USER_CHANNEL_PREFIX}${USER_1_ACCOUNT_ID}${CONFIG.PUSHER.SUFFIX}`); + return Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, {reportName: 'Test', reportID: REPORT_ID}) + .then(() => TestHelper.signInWithTestUser(USER_1_ACCOUNT_ID, USER_1_LOGIN)) + .then(() => { + // Given a test user that is subscribed to Pusher events + User.subscribeToUserEvents(); + return waitForPromisesToResolve(); + }) + .then(() => TestHelper.fetchPersonalDetailsForTestUser(USER_1_ACCOUNT_ID, USER_1_LOGIN, { + [USER_1_LOGIN]: { + accountID: USER_1_ACCOUNT_ID, + email: USER_1_LOGIN, + firstName: 'Test', + lastName: 'User', + }, + })) + .then(() => { + // When a Pusher event is handled for a new report comment + channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, + value: { + reportID: REPORT_ID, + maxSequenceNumber: 1, + notificationPreference: 'always', + lastMessageTimestamp: 0, + lastMessageText: 'Comment 1', + lastActorEmail: USER_2_LOGIN, + newMarkerSequenceNumber: 0, + lastReadSequenceNumber: 0, + }, + }, + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + value: { + 1: { + actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, + actorAccountID: USER_2_ACCOUNT_ID, + actorEmail: USER_2_LOGIN, + automatic: false, + avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png', + message: [{type: 'COMMENT', html: 'Comment 1', text: 'Comment 1'}], + person: [{type: 'TEXT', style: 'strong', text: 'Test User'}], + sequenceNumber: 1, + shouldShow: true, + timestamp: moment().unix(), + }, + }, + }, + ]); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then the report will have an unreadActionCount + expect(report.unreadActionCount).toBe(1); + + // When the user visits the report + Report.openReport(REPORT_ID); + return waitForPromisesToResolve(); + }) + .then(() => { + // The unreadActionCount will return to 0 + expect(report.unreadActionCount).toBe(0); + + // When the user manually marks a message as "unread" + Report.markCommentAsUnread(REPORT_ID, 1); + return waitForPromisesToResolve(); + }) + .then(() => { + // The unreadActionCount will increase and the new marker will be set correctly + expect(report.unreadActionCount).toBe(1); + expect(report.newMarkerSequenceNumber).toBe(1); + + // When a new comment is added by the current user + Report.addComment(REPORT_ID, 'Current User Comment 1'); + return waitForPromisesToResolve(); + }) + .then(() => { + // The unreadActionCount should be 0 and the lastReadSequenceNumber incremented + expect(report.unreadActionCount).toBe(0); + expect(report.lastReadSequenceNumber).toBe(2); + expect(report.lastMessageText).toBe('Current User Comment 1'); + + // When another comment is added by the current user + Report.addComment(REPORT_ID, 'Current User Comment 2'); + return waitForPromisesToResolve(); + }) + .then(() => { + // The unreadActionCount should be 0 and the lastReadSequenceNumber incremented + expect(report.unreadActionCount).toBe(0); + expect(report.lastReadSequenceNumber).toBe(3); + expect(report.lastMessageText).toBe('Current User Comment 2'); + + // When another comment is added by the current user + Report.addComment(REPORT_ID, 'Current User Comment 3'); + return waitForPromisesToResolve(); + }) + .then(() => { + // The unreadActionCount should be 0 and the lastReadSequenceNumber incremented + expect(report.unreadActionCount).toBe(0); + expect(report.lastReadSequenceNumber).toBe(4); + expect(report.lastMessageText).toBe('Current User Comment 3'); + + const USER_1_BASE_ACTION = { + actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT, + actorAccountID: USER_1_ACCOUNT_ID, + actorEmail: USER_1_LOGIN, + automatic: false, + avatar: 'https://d2k5nsl2zxldvw.cloudfront.net/images/avatars/avatar_3.png', + person: [{type: 'TEXT', style: 'strong', text: 'Test User'}], + shouldShow: true, + timestamp: moment().unix(), + reportActionID: 'derp', + }; + + // When we emit the events for these pending created actions to update them to not pending + channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, + value: { + reportID: REPORT_ID, + maxSequenceNumber: 4, + notificationPreference: 'always', + lastMessageTimestamp: 0, + lastMessageText: 'Current User Comment 3', + lastActorEmail: 'test@test.com', + lastReadSequenceNumber: 4, + }, + }, + { + onyxMethod: CONST.ONYX.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, + value: { + [_.toArray(reportActions)[1].clientID]: null, + [_.toArray(reportActions)[2].clientID]: null, + [_.toArray(reportActions)[3].clientID]: null, + 2: { + ...USER_1_BASE_ACTION, + message: [{type: 'COMMENT', html: 'Current User Comment 1', text: 'Current User Comment 1'}], + sequenceNumber: 2, + }, + 3: { + ...USER_1_BASE_ACTION, + message: [{type: 'COMMENT', html: 'Current User Comment 2', text: 'Current User Comment 2'}], + sequenceNumber: 3, + }, + 4: { + ...USER_1_BASE_ACTION, + message: [{type: 'COMMENT', html: 'Current User Comment 3', text: 'Current User Comment 3'}], + sequenceNumber: 4, + }, + }, + }, + ]); + + return waitForPromisesToResolve(); + }) + .then(() => { + // If the user deletes a comment that is before the last read + Report.deleteReportComment(REPORT_ID, reportActions[2]); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then no change will occur + expect(report.lastReadSequenceNumber).toBe(4); + expect(report.unreadActionCount).toBe(0); + + // When the user manually marks a message as "unread" + Report.markCommentAsUnread(REPORT_ID, 3); + return waitForPromisesToResolve(); + }) + .then(() => { + // Then we should expect the unreadActionCount to be updated + expect(report.unreadActionCount).toBe(2); + expect(report.lastReadSequenceNumber).toBe(2); + expect(report.newMarkerSequenceNumber).toBe(3); + + // If the user deletes the last comment after the last read the unreadActionCount will decrease and the lastMessageText will reflect the new last comment + Report.deleteReportComment(REPORT_ID, reportActions[4]); + return waitForPromisesToResolve(); + }) + .then(() => { + expect(report.unreadActionCount).toBe(1); + expect(report.lastMessageText).toBe('Current User Comment 2'); + }); + }); });