From c6e6299dd9061f825dd54ccb5b9f3792c155c212 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Fri, 5 Nov 2021 21:57:06 +0530 Subject: [PATCH 1/7] remove deleted messages from the unread message count --- src/libs/actions/Report.js | 6 +++--- src/libs/actions/ReportActions.js | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 378196515301..9d383d81a394 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -23,7 +23,7 @@ import { isConciergeChatReport, isDefaultRoom, isReportMessageAttachment, sortReportsByLastVisited, isArchivedRoom, } from '../reportUtils'; import Timers from '../Timers'; -import {dangerouslyGetReportActionsMaxSequenceNumber, isReportMissingActions} from './ReportActions'; +import {dangerouslyGetReportActionsMaxSequenceNumber, getdeletedCommentsCount, isReportMissingActions} from './ReportActions'; import Growl from '../Growl'; import {translateLocal} from '../translate'; @@ -104,7 +104,7 @@ function getUnreadActionCount(report) { // 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; + const unreadActionCount = report.reportActionCount - lastReadSequenceNumber - getdeletedCommentsCount(report.reportID, lastReadSequenceNumber); return Math.max(0, unreadActionCount); } @@ -414,7 +414,7 @@ function setLocalLastRead(reportID, lastReadSequenceNumber) { // 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 - const unreadActionCount = Math.max(reportMaxSequenceNumber - lastReadSequenceNumber, 0); + const unreadActionCount = Math.max(reportMaxSequenceNumber - lastReadSequenceNumber - getdeletedCommentsCount(reportID, lastReadSequenceNumber), 0); // Update the report optimistically. Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index 74fbb2dfb83d..dfa17d04197c 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -1,7 +1,9 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; +import lodashGet from 'lodash/get'; import ONYXKEYS from '../../ONYXKEYS'; import * as CollectionUtils from '../CollectionUtils'; +import CONST from '../../CONST'; /** * Map of the most recent non-loading sequenceNumber for a reportActions_* key in Onyx by reportID. @@ -17,6 +19,8 @@ import * as CollectionUtils from '../CollectionUtils'; * referenced and not the locally stored reportAction's max sequenceNumber. */ const reportActionsMaxSequenceNumbers = {}; +const reportActions = {}; + Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT_ACTIONS, callback: (actions, key) => { @@ -26,6 +30,7 @@ Onyx.connect({ const reportID = CollectionUtils.extractCollectionItemID(key); const actionsArray = _.toArray(actions); + reportActions[reportID] = actionsArray; const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.loading); const mostRecentAction = actionsArray[mostRecentNonLoadingActionIndex]; if (!mostRecentAction || _.isUndefined(mostRecentAction.sequenceNumber)) { @@ -67,7 +72,23 @@ function isReportMissingActions(reportID, maxKnownSequenceNumber) { || reportActionsMaxSequenceNumbers[reportID] < maxKnownSequenceNumber; } +function getdeletedCommentsCount(reportID, lastSequenceNumber) { + return _.reduce(reportActions[reportID], (deletedMessages, action) => { + if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber < lastSequenceNumber) { + return deletedMessages; + } + + // Empty ADDCOMMENT actions typically mean they have been deleted + const message = _.first(lodashGet(action, 'message', null)); + const html = lodashGet(message, 'html', ''); + if (_.isEmpty(html)) { + return deletedMessages + 1; + } + }, 0); +} + export { isReportMissingActions, dangerouslyGetReportActionsMaxSequenceNumber, + getdeletedCommentsCount, }; From 4a59ea64de9c2017b846e0e61fd37498d67a0903 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 6 Nov 2021 15:12:36 +0530 Subject: [PATCH 2/7] Fix logic to get the deleted messages count for a report --- src/libs/actions/ReportActions.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index dfa17d04197c..d8f7ec1bcf57 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -73,6 +73,10 @@ function isReportMissingActions(reportID, maxKnownSequenceNumber) { } function getdeletedCommentsCount(reportID, lastSequenceNumber) { + if (!reportActions[reportID]) { + return 0; + } + return _.reduce(reportActions[reportID], (deletedMessages, action) => { if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber < lastSequenceNumber) { return deletedMessages; @@ -81,9 +85,7 @@ function getdeletedCommentsCount(reportID, lastSequenceNumber) { // Empty ADDCOMMENT actions typically mean they have been deleted const message = _.first(lodashGet(action, 'message', null)); const html = lodashGet(message, 'html', ''); - if (_.isEmpty(html)) { - return deletedMessages + 1; - } + return _.isEmpty(html) ? deletedMessages + 1 : deletedMessages; }, 0); } From 16592c74623d92a7653c3a96461865776b8ff039 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 6 Nov 2021 15:28:45 +0530 Subject: [PATCH 3/7] refactor code --- src/libs/actions/Report.js | 10 +++++----- src/libs/actions/ReportActions.js | 10 ++++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 9d383d81a394..0c6f12dd8dfb 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -23,7 +23,7 @@ import { isConciergeChatReport, isDefaultRoom, isReportMessageAttachment, sortReportsByLastVisited, isArchivedRoom, } from '../reportUtils'; import Timers from '../Timers'; -import {dangerouslyGetReportActionsMaxSequenceNumber, getdeletedCommentsCount, isReportMissingActions} from './ReportActions'; +import * as ReportActions from './ReportActions'; import Growl from '../Growl'; import {translateLocal} from '../translate'; @@ -104,7 +104,7 @@ function getUnreadActionCount(report) { // 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 - getdeletedCommentsCount(report.reportID, lastReadSequenceNumber); + const unreadActionCount = report.reportActionCount - lastReadSequenceNumber - ReportActions.getDeletedCommentsCount(report.reportID, lastReadSequenceNumber); return Math.max(0, unreadActionCount); } @@ -414,7 +414,7 @@ function setLocalLastRead(reportID, lastReadSequenceNumber) { // 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 - const unreadActionCount = Math.max(reportMaxSequenceNumber - lastReadSequenceNumber - getdeletedCommentsCount(reportID, lastReadSequenceNumber), 0); + const unreadActionCount = Math.max(reportMaxSequenceNumber - lastReadSequenceNumber - ReportActions.getDeletedCommentsCount(reportID, lastReadSequenceNumber), 0); // Update the report optimistically. Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { @@ -974,7 +974,7 @@ function fetchAllReports( // data processing by Onyx. const reportIDsWithMissingActions = _.chain(returnedReports) .map(report => report.reportID) - .filter(reportID => isReportMissingActions(reportID, reportMaxSequenceNumbers[reportID])) + .filter(reportID => ReportActions.isReportMissingActions(reportID, reportMaxSequenceNumbers[reportID])) .value(); // Once we have the reports that are missing actions we will find the intersection between the most @@ -996,7 +996,7 @@ function fetchAllReports( reportIDs: reportIDsToFetchActions, }); _.each(reportIDsToFetchActions, (reportID) => { - const offset = dangerouslyGetReportActionsMaxSequenceNumber(reportID, false); + const offset = ReportActions.dangerouslyGetReportActionsMaxSequenceNumber(reportID, false); fetchActions(reportID, offset); }); diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index d8f7ec1bcf57..de604fbf5492 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -72,7 +72,13 @@ function isReportMissingActions(reportID, maxKnownSequenceNumber) { || reportActionsMaxSequenceNumbers[reportID] < maxKnownSequenceNumber; } -function getdeletedCommentsCount(reportID, lastSequenceNumber) { +/** + * Get the count of deleted messages upto a sequence number of a report + * @param {Number|String} reportID + * @param {Number} lastSequenceNumber + * @return {Number} + */ +function getDeletedCommentsCount(reportID, lastSequenceNumber) { if (!reportActions[reportID]) { return 0; } @@ -92,5 +98,5 @@ function getdeletedCommentsCount(reportID, lastSequenceNumber) { export { isReportMissingActions, dangerouslyGetReportActionsMaxSequenceNumber, - getdeletedCommentsCount, + getDeletedCommentsCount, }; From 2828b51b6b1791d7db7ce4466253e1e46c4310f7 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 14 Nov 2021 21:47:16 +0530 Subject: [PATCH 4/7] exclude the last sequence number from deleted messages we are getting deleted messages upto a sequence number but that does not include the message at that sequence number --- src/libs/actions/ReportActions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index de604fbf5492..cb19e6fdd44d 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -84,7 +84,7 @@ function getDeletedCommentsCount(reportID, lastSequenceNumber) { } return _.reduce(reportActions[reportID], (deletedMessages, action) => { - if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber < lastSequenceNumber) { + if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber <= lastSequenceNumber) { return deletedMessages; } From 519ab171c3296855bf7bc2cd3e2c6d825bd7b5a2 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sun, 14 Nov 2021 21:47:48 +0530 Subject: [PATCH 5/7] fix: new marker position --- src/libs/actions/Report.js | 10 ++++++++++ src/pages/home/report/ReportActionsView.js | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index f9631d87b3c2..6afa36dfdbc1 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1444,6 +1444,15 @@ function createPolicyRoom(policyID, reportName, visibility) { }); } +/** + * Get the last read sequence number for a report + * @param {String|Number} reportID + * @return {Number} + */ +function getLastReadSequenceNumber(reportID) { + return lastReadSequenceNumbers[reportID]; +} + export { fetchAllReports, fetchActions, @@ -1473,4 +1482,5 @@ export { setReportWithDraft, fetchActionsWithLoadingState, createPolicyRoom, + getLastReadSequenceNumber, }; diff --git a/src/pages/home/report/ReportActionsView.js b/src/pages/home/report/ReportActionsView.js index 84ee7d94afbf..0ad877de81de 100755 --- a/src/pages/home/report/ReportActionsView.js +++ b/src/pages/home/report/ReportActionsView.js @@ -372,7 +372,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 : (this.props.report.maxSequenceNumber - unreadActionCount) + 1; + const oldestUnreadSequenceNumber = unreadActionCount === 0 ? 0 : Report.getLastReadSequenceNumber(this.props.report.reportID) + 1; Report.setNewMarkerPosition(this.props.reportID, oldestUnreadSequenceNumber); } From f4b8be1cc12d09114937677ed05274bd4077466c Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Thu, 18 Nov 2021 23:58:50 +0530 Subject: [PATCH 6/7] update unread message count when user deletes a message --- src/libs/actions/Report.js | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 6afa36dfdbc1..2704ceef99ff 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1117,6 +1117,15 @@ function addAction(reportID, text, 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 * @@ -1139,7 +1148,9 @@ function deleteReportComment(reportID, reportAction) { ], }; - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge).then(() => { + setLocalLastRead(reportID, getLastReadSequenceNumber(reportID)); + }); // Try to delete the comment by calling the API API.Report_EditComment({ @@ -1158,8 +1169,9 @@ function deleteReportComment(reportID, reportAction) { ...reportAction, message: oldMessage, }; - - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge); + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge).then(() => { + setLocalLastRead(reportID, getLastReadSequenceNumber(reportID)); + }); }); } @@ -1444,15 +1456,6 @@ function createPolicyRoom(policyID, reportName, visibility) { }); } -/** - * Get the last read sequence number for a report - * @param {String|Number} reportID - * @return {Number} - */ -function getLastReadSequenceNumber(reportID) { - return lastReadSequenceNumbers[reportID]; -} - export { fetchAllReports, fetchActions, From a9531ace7ba4a6411a016043662ed66f4878d8f8 Mon Sep 17 00:00:00 2001 From: Rajat Parashar Date: Sat, 20 Nov 2021 12:19:53 +0530 Subject: [PATCH 7/7] Refactor based on comments --- src/libs/actions/ReportActions.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index cb19e6fdd44d..e80bbad61c56 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -73,18 +73,18 @@ function isReportMissingActions(reportID, maxKnownSequenceNumber) { } /** - * Get the count of deleted messages upto a sequence number of a report + * Get the count of deleted messages after a sequence number of a report * @param {Number|String} reportID - * @param {Number} lastSequenceNumber + * @param {Number} sequenceNumber * @return {Number} */ -function getDeletedCommentsCount(reportID, lastSequenceNumber) { +function getDeletedCommentsCount(reportID, sequenceNumber) { if (!reportActions[reportID]) { return 0; } return _.reduce(reportActions[reportID], (deletedMessages, action) => { - if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber <= lastSequenceNumber) { + if (action.actionName !== CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT || action.sequenceNumber <= sequenceNumber) { return deletedMessages; }