Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove deleted messages from the unread message count #6248

Merged
merged 9 commits into from
Nov 20, 2021
29 changes: 21 additions & 8 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
isConciergeChatReport, isDefaultRoom, isReportMessageAttachment, sortReportsByLastVisited, isArchivedRoom,
} from '../reportUtils';
import Timers from '../Timers';
import {dangerouslyGetReportActionsMaxSequenceNumber, isReportMissingActions} from './ReportActions';
import * as ReportActions from './ReportActions';
import Growl from '../Growl';
import {translateLocal} from '../translate';

Expand Down Expand Up @@ -106,7 +106,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 - ReportActions.getDeletedCommentsCount(report.reportID, lastReadSequenceNumber);
return Math.max(0, unreadActionCount);
}

Expand Down Expand Up @@ -418,7 +418,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 - ReportActions.getDeletedCommentsCount(reportID, lastReadSequenceNumber), 0);

// Update the report optimistically.
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
Expand Down Expand Up @@ -978,7 +978,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
Expand All @@ -1000,7 +1000,7 @@ function fetchAllReports(
reportIDs: reportIDsToFetchActions,
});
_.each(reportIDsToFetchActions, (reportID) => {
const offset = dangerouslyGetReportActionsMaxSequenceNumber(reportID, false);
const offset = ReportActions.dangerouslyGetReportActionsMaxSequenceNumber(reportID, false);
fetchActions(reportID, offset);
});

Expand Down Expand Up @@ -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
*
Expand All @@ -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({
Expand All @@ -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));
});
});
}

Expand Down Expand Up @@ -1473,4 +1485,5 @@ export {
setReportWithDraft,
fetchActionsWithLoadingState,
createPolicyRoom,
getLastReadSequenceNumber,
};
29 changes: 29 additions & 0 deletions src/libs/actions/ReportActions.js
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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) => {
Expand All @@ -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)) {
Expand Down Expand Up @@ -67,7 +72,31 @@ function isReportMissingActions(reportID, maxKnownSequenceNumber) {
|| reportActionsMaxSequenceNumbers[reportID] < maxKnownSequenceNumber;
}

/**
* Get the count of deleted messages upto a sequence number of a report
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
* @param {Number|String} reportID
* @param {Number} lastSequenceNumber
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
* @return {Number}
*/
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;
}

// Empty ADDCOMMENT actions typically mean they have been deleted
const message = _.first(lodashGet(action, 'message', null));
const html = lodashGet(message, 'html', '');
return _.isEmpty(html) ? deletedMessages + 1 : deletedMessages;
}, 0);
}

export {
isReportMissingActions,
dangerouslyGetReportActionsMaxSequenceNumber,
getDeletedCommentsCount,
};
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionsView.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down