diff --git a/src/CONST.js b/src/CONST.js index 22a5ea8a3c82..e5ffb21bbe37 100755 --- a/src/CONST.js +++ b/src/CONST.js @@ -777,6 +777,9 @@ const CONST = { DELETE: 'delete', UPDATE: 'update', }, + BRICK_ROAD_INDICATOR_STATUS: { + ERROR: 'error', + }, }; export default CONST; diff --git a/src/components/MenuItem.js b/src/components/MenuItem.js index 08b3085c0be3..e4cfd785dac3 100644 --- a/src/components/MenuItem.js +++ b/src/components/MenuItem.js @@ -133,7 +133,7 @@ const MenuItem = props => ( )} {props.brickRoadIndicator && ( - + { )} {hasErrors && ( - + diff --git a/src/components/OptionRow.js b/src/components/OptionRow.js index a6a75bceb48a..cadae24e801e 100644 --- a/src/components/OptionRow.js +++ b/src/components/OptionRow.js @@ -23,6 +23,7 @@ import SelectCircle from './SelectCircle'; import SubscriptAvatar from './SubscriptAvatar'; import CONST from '../CONST'; import * as ReportUtils from '../libs/ReportUtils'; +import variables from '../styles/variables'; const propTypes = { /** Background Color of the Option Row */ @@ -205,6 +206,16 @@ const OptionRow = (props) => { ) : null} + {props.option.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR && ( + + + + )} {props.showSelectedState && } @@ -298,5 +309,9 @@ export default withLocalize(memo(OptionRow, (prevProps, nextProps) => { return false; } + if (prevProps.option.brickRoadIndicator !== nextProps.option.brickRoadIndicator) { + return false; + } + return true; })); diff --git a/src/components/optionPropTypes.js b/src/components/optionPropTypes.js index 47af58d240cb..78dbd19b5339 100644 --- a/src/components/optionPropTypes.js +++ b/src/components/optionPropTypes.js @@ -1,4 +1,5 @@ import PropTypes from 'prop-types'; +import CONST from '../CONST'; import participantPropTypes from './participantPropTypes'; export default PropTypes.shape({ @@ -49,4 +50,7 @@ export default PropTypes.shape({ // Text to show for tooltip tooltipText: PropTypes.string, + + /** If we need to show a brick road indicator or not */ + brickRoadIndicator: PropTypes.oneOf([CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR, '']), }); diff --git a/src/libs/OptionsListUtils.js b/src/libs/OptionsListUtils.js index 61e05b4a5a9f..d793bfaaef2d 100644 --- a/src/libs/OptionsListUtils.js +++ b/src/libs/OptionsListUtils.js @@ -190,6 +190,21 @@ function hasReportDraftComment(report, reportsWithDraft = {}) { && lodashGet(reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${report.reportID}`, false); } +/** + * @param {Object} report + * @param {Object} reportActions + * @returns {String} + */ +function getBrickRoadIndicatorStatusForReport(report, reportActions) { + const reportID = lodashGet(report, 'reportID'); + const reportsActions = lodashGet(reportActions, `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {}); + if (_.isEmpty(reportsActions)) { + return ''; + } + + return _.find(reportsActions, action => !_.isEmpty(action.errors)) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''; +} + /** * Creates a report list option * @@ -197,12 +212,13 @@ function hasReportDraftComment(report, reportsWithDraft = {}) { * @param {Object} personalDetails * @param {Object} report * @param {Object} reportsWithDraft + * @param {Object} reportActions * @param {Object} options * @param {Boolean} [options.showChatPreviewLine] * @param {Boolean} [options.forcePolicyNamePreview] * @returns {Object} */ -function createOption(logins, personalDetails, report, reportsWithDraft, { +function createOption(logins, personalDetails, report, reportsWithDraft, reportActions = {}, { showChatPreviewLine = false, forcePolicyNamePreview = false, }) { @@ -252,6 +268,7 @@ function createOption(logins, personalDetails, report, reportsWithDraft, { return { text: reportName, alternateText, + brickRoadIndicator: getBrickRoadIndicatorStatusForReport(report, reportActions), icons: ReportUtils.getIcons(report, personalDetails, policies, lodashGet(personalDetail, ['avatar'])), tooltipText, ownerEmail: lodashGet(report, ['ownerEmail']), @@ -345,6 +362,7 @@ function isCurrentUser(userDetails) { */ function getOptions(reports, personalDetails, activeReportID, { reportsWithDraft = {}, + reportActions = {}, betas = [], selectedOptions = [], maxRecentReportsToShow = 0, @@ -449,7 +467,7 @@ function getOptions(reports, personalDetails, activeReportID, { reportMapForLogins[logins[0]] = report; } const isSearchingSomeonesPolicyExpenseChat = !report.isOwnPolicyExpenseChat && searchValue !== ''; - allReportOptions.push(createOption(logins, personalDetails, report, reportsWithDraft, { + allReportOptions.push(createOption(logins, personalDetails, report, reportsWithDraft, reportActions, { showChatPreviewLine, forcePolicyNamePreview: isPolicyExpenseChat ? isSearchingSomeonesPolicyExpenseChat : forcePolicyNamePreview, })); @@ -460,6 +478,7 @@ function getOptions(reports, personalDetails, activeReportID, { personalDetails, reportMapForLogins[personalDetail.login], reportsWithDraft, + reportActions, { showChatPreviewLine, forcePolicyNamePreview, @@ -581,7 +600,7 @@ function getOptions(reports, personalDetails, activeReportID, { const login = (Str.isValidPhone(searchValue) && !searchValue.includes('+')) ? `+${countryCodeByIP}${searchValue}` : searchValue; - userToInvite = createOption([login], personalDetails, null, reportsWithDraft, { + userToInvite = createOption([login], personalDetails, null, reportsWithDraft, reportActions, { showChatPreviewLine, }); userToInvite.icons = [ReportUtils.getDefaultAvatar(login)]; @@ -744,9 +763,10 @@ function getMemberInviteOptions( * @param {String} priorityMode * @param {Array} betas * @param {Object} reportsWithDraft + * @param {Object} reportActions * @returns {Object} */ -function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportsWithDraft) { +function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportsWithDraft, reportActions) { let sideBarOptions = { prioritizeIOUDebts: true, prioritizeReportsWithDraftComments: true, @@ -767,6 +787,7 @@ function calculateSidebarOptions(reports, personalDetails, activeReportID, prior prioritizePinnedReports: true, ...sideBarOptions, reportsWithDraft, + reportActions, }); } diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 31384254d8a6..4abdb48a9fd7 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -59,10 +59,6 @@ const allReports = {}; let conciergeChatReportID; const typingWatchTimers = {}; -// 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 = {}; - /** * @param {Number} reportID * @param {Number} lastReadSequenceNumber @@ -398,26 +394,6 @@ function setLocalIOUReportData(iouReportObject) { Onyx.merge(iouReportKey, iouReportObject); } -/** - * 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. - * - * @param {Number} reportID - */ -function removeOptimisticActions(reportID) { - const actionIDs = optimisticReportActionIDs[reportID] || []; - const actionsToRemove = _.reduce(actionIDs, (actions, actionID) => ({ - ...actions, - [actionID]: null, - }), {}); - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionsToRemove); - - // Reset the optimistic report action IDs to an empty array - Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, { - optimisticReportActionIDs: [], - }); -} - /** * Fetch the iouReport and persist the data to Onyx. * @@ -663,10 +639,6 @@ function fetchActions(reportID) { reportActionsLimit: CONST.REPORT.ACTIONS.LIMIT, }) .then((data) => { - // We must remove all optimistic actions so there will not be any stuck comments. At this point, we should - // be caught up and no longer need any optimistic comments. - removeOptimisticActions(reportID); - const indexedData = _.indexBy(data.history, 'sequenceNumber'); Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, indexedData); }); @@ -821,7 +793,7 @@ function buildOptimisticReportAction(reportID, text, file) { isFirstItem: false, isAttachment, attachmentInfo, - isLoading: true, + pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD, shouldShow: true, }, }; @@ -876,19 +848,6 @@ function addActions(reportID, text = '', file) { lastReadSequenceNumber: newSequenceNumber, }; - // Store the optimistic action ID on the report the comment was added to. It will be removed later when refetching - // report actions in order to clear out any stuck actions (i.e. actions where the client never received a Pusher - // event, for whatever reason, from the server with the new action data - optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || [])]; - - if (text) { - optimisticReport.optimisticReportActionIDs.push(reportCommentAction.clientID); - } - - if (file) { - optimisticReport.optimisticReportActionIDs.push(attachmentAction.clientID); - } - // Optimistically add the new actions to the store before waiting to save them to the server const optimisticReportActions = {}; if (text) { @@ -931,26 +890,8 @@ function addActions(reportID, text = '', file) { DateUtils.setTimezoneUpdated(); } - const failureDataReportActions = {}; - const defaultLoadingState = { - isLoading: false, - }; - - if (text) { - failureDataReportActions[reportCommentAction.clientID] = defaultLoadingState; - } - - if (file) { - failureDataReportActions[attachmentAction.clientID] = defaultLoadingState; - } - API.write(commandName, parameters, { optimisticData, - failureData: [{ - onyxMethod: CONST.ONYX.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, - value: failureDataReportActions, - }], }); } @@ -1247,9 +1188,6 @@ function handleReportChanged(report) { if (report.reportID && report.reportName === undefined) { fetchChatReportsByIDs([report.reportID]); } - - // Store optimistic actions IDs for each report - optimisticReportActionIDs[report.reportID] = report.optimisticReportActionIDs; } /** @@ -1490,7 +1428,7 @@ function viewNewReportAction(reportID, action) { if (isFromCurrentUser) { updatedReportObject.unreadActionCount = 0; updatedReportObject.lastVisitedTimestamp = Date.now(); - updatedReportObject.lastReadSequenceNumber = action.sequenceNumber; + updatedReportObject.lastReadSequenceNumber = action.pendingAction ? lastReadSequenceNumber : action.sequenceNumber; } else if (incomingSequenceNumber > lastReadSequenceNumber) { updatedReportObject.unreadActionCount = getUnreadActionCount(reportID) + 1; } diff --git a/src/libs/actions/ReportActions.js b/src/libs/actions/ReportActions.js index 8b87ccb8480f..caed131681db 100644 --- a/src/libs/actions/ReportActions.js +++ b/src/libs/actions/ReportActions.js @@ -130,10 +130,21 @@ function isFromCurrentUser(reportID, sequenceNumber, currentUserAccountID, actio return action.actorAccountID === currentUserAccountID; } +/** + * @param {Number} reportID + * @param {String} clientID + */ +function deleteClientAction(reportID, clientID) { + Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, { + [clientID]: null, + }); +} + export { isReportMissingActions, dangerouslyGetReportActionsMaxSequenceNumber, getDeletedCommentsCount, getLastVisibleMessageText, isFromCurrentUser, + deleteClientAction, }; diff --git a/src/pages/home/report/ReportActionItem.js b/src/pages/home/report/ReportActionItem.js index 948cac72c2e2..9d5926987242 100644 --- a/src/pages/home/report/ReportActionItem.js +++ b/src/pages/home/report/ReportActionItem.js @@ -31,6 +31,8 @@ import styles from '../../../styles/styles'; import SelectionScraper from '../../../libs/SelectionScraper'; import * as User from '../../../libs/actions/User'; import * as ReportUtils from '../../../libs/ReportUtils'; +import OfflineWithFeedback from '../../../components/OfflineWithFeedback'; +import * as ReportActions from '../../../libs/actions/ReportActions'; const propTypes = { /** The ID of the report this action is on. */ @@ -185,17 +187,24 @@ class ReportActionItem extends Component { (this.props.network.isOffline && this.props.action.isLoading) || this.props.action.error, )} > - {!this.props.displayAsGroup - ? ( - - {children} - - ) - : ( - - {children} - - )} + ReportActions.deleteClientAction(this.props.report.reportID, this.props.action.clientID)} + pendingAction={this.props.action.pendingAction} + errors={this.props.action.errors} + errorRowStyles={[styles.ml10, styles.mr2]} + > + {!this.props.displayAsGroup + ? ( + + {children} + + ) + : ( + + {children} + + )} + { clientID = resultAction.sequenceNumber; expect(resultAction.message).toEqual(REPORT_ACTION.message); expect(resultAction.person).toEqual(REPORT_ACTION.person); - expect(resultAction.isLoading).toEqual(true); + expect(resultAction.pendingAction).toEqual(CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD); // 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; + delete actionWithoutLoading.pendingAction; channel.emit(Pusher.TYPE.ONYX_API_UPDATE, [ { onyxMethod: CONST.ONYX.METHOD.MERGE, @@ -137,7 +137,7 @@ describe('actions/Report', () => { const resultAction = reportActions[ACTION_ID]; // Verify that our action is no longer in the loading state - expect(resultAction.isLoading).not.toBeDefined(); + expect(resultAction.pendingAction).not.toBeDefined(); }); });