Skip to content

Commit e57e113

Browse files
marcaaronmarcaaron
authored
Merge pull request #10317 from Expensify/marcaaron-addCommentPatternB
Show comment API errors with `OfflineWithFeedback`
2 parents bd75a5f + 0858736 commit e57e113

File tree

12 files changed

+98
-84
lines changed

12 files changed

+98
-84
lines changed

‎src/CONST.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,9 @@ const CONST = {
777777
DELETE: 'delete',
778778
UPDATE: 'update',
779779
},
780+
BRICK_ROAD_INDICATOR_STATUS: {
781+
ERROR: 'error',
782+
},
780783
};
781784

782785
export default CONST;

‎src/components/MenuItem.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ const MenuItem = props => (
133133
</View>
134134
)}
135135
{props.brickRoadIndicator && (
136-
<View style={[[styles.alignItemsCenter, styles.justifyContentCenter]]}>
136+
<View style={[styles.alignItemsCenter, styles.justifyContentCenter]}>
137137
<Icon
138138
src={Expensicons.DotIndicator}
139139
fill={props.brickRoadIndicator === 'error' ? colors.red : colors.green}

‎src/components/OfflineWithFeedback.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,18 @@ const propTypes = {
4343
PropTypes.arrayOf(PropTypes.object),
4444
PropTypes.object,
4545
]),
46+
47+
/** Additional style object for the error row */
48+
errorRowStyles: PropTypes.arrayOf(PropTypes.object),
49+
4650
...withLocalizePropTypes,
4751
};
4852

4953
const defaultProps = {
5054
pendingAction: null,
5155
errors: null,
5256
style: [],
57+
errorRowStyles: [],
5358
};
5459

5560
/**
@@ -97,7 +102,7 @@ const OfflineWithFeedback = (props) => {
97102
</View>
98103
)}
99104
{hasErrors && (
100-
<View style={styles.offlineFeedback.error}>
105+
<View style={[styles.offlineFeedback.error, ...props.errorRowStyles]}>
101106
<View style={styles.offlineFeedback.errorDot}>
102107
<Icon src={Expensicons.DotIndicator} fill={colors.red} height={variables.iconSizeSmall} width={variables.iconSizeSmall} />
103108
</View>

‎src/components/OptionRow.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import SelectCircle from './SelectCircle';
2323
import SubscriptAvatar from './SubscriptAvatar';
2424
import CONST from '../CONST';
2525
import * as ReportUtils from '../libs/ReportUtils';
26+
import variables from '../styles/variables';
2627

2728
const propTypes = {
2829
/** Background Color of the Option Row */
@@ -205,6 +206,16 @@ const OptionRow = (props) => {
205206
</Text>
206207
</View>
207208
) : null}
209+
{props.option.brickRoadIndicator === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR && (
210+
<View style={[styles.alignItemsCenter, styles.justifyContentCenter]}>
211+
<Icon
212+
src={Expensicons.DotIndicator}
213+
fill={colors.red}
214+
height={variables.iconSizeSmall}
215+
width={variables.iconSizeSmall}
216+
/>
217+
</View>
218+
)}
208219
{props.showSelectedState && <SelectCircle isChecked={props.isSelected} />}
209220
</View>
210221
</View>
@@ -298,5 +309,9 @@ export default withLocalize(memo(OptionRow, (prevProps, nextProps) => {
298309
return false;
299310
}
300311

312+
if (prevProps.option.brickRoadIndicator !== nextProps.option.brickRoadIndicator) {
313+
return false;
314+
}
315+
301316
return true;
302317
}));

‎src/components/optionPropTypes.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import PropTypes from 'prop-types';
2+
import CONST from '../CONST';
23
import participantPropTypes from './participantPropTypes';
34

45
export default PropTypes.shape({
@@ -49,4 +50,7 @@ export default PropTypes.shape({
4950

5051
// Text to show for tooltip
5152
tooltipText: PropTypes.string,
53+
54+
/** If we need to show a brick road indicator or not */
55+
brickRoadIndicator: PropTypes.oneOf([CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR, '']),
5256
});

‎src/libs/OptionsListUtils.js

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,19 +190,35 @@ function hasReportDraftComment(report, reportsWithDraft = {}) {
190190
&& lodashGet(reportsWithDraft, `${ONYXKEYS.COLLECTION.REPORTS_WITH_DRAFT}${report.reportID}`, false);
191191
}
192192

193+
/**
194+
* @param {Object} report
195+
* @param {Object} reportActions
196+
* @returns {String}
197+
*/
198+
function getBrickRoadIndicatorStatusForReport(report, reportActions) {
199+
const reportID = lodashGet(report, 'reportID');
200+
const reportsActions = lodashGet(reportActions, `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {});
201+
if (_.isEmpty(reportsActions)) {
202+
return '';
203+
}
204+
205+
return _.find(reportsActions, action => !_.isEmpty(action.errors)) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
206+
}
207+
193208
/**
194209
* Creates a report list option
195210
*
196211
* @param {Array<String>} logins
197212
* @param {Object} personalDetails
198213
* @param {Object} report
199214
* @param {Object} reportsWithDraft
215+
* @param {Object} reportActions
200216
* @param {Object} options
201217
* @param {Boolean} [options.showChatPreviewLine]
202218
* @param {Boolean} [options.forcePolicyNamePreview]
203219
* @returns {Object}
204220
*/
205-
function createOption(logins, personalDetails, report, reportsWithDraft, {
221+
function createOption(logins, personalDetails, report, reportsWithDraft, reportActions = {}, {
206222
showChatPreviewLine = false,
207223
forcePolicyNamePreview = false,
208224
}) {
@@ -252,6 +268,7 @@ function createOption(logins, personalDetails, report, reportsWithDraft, {
252268
return {
253269
text: reportName,
254270
alternateText,
271+
brickRoadIndicator: getBrickRoadIndicatorStatusForReport(report, reportActions),
255272
icons: ReportUtils.getIcons(report, personalDetails, policies, lodashGet(personalDetail, ['avatar'])),
256273
tooltipText,
257274
ownerEmail: lodashGet(report, ['ownerEmail']),
@@ -345,6 +362,7 @@ function isCurrentUser(userDetails) {
345362
*/
346363
function getOptions(reports, personalDetails, activeReportID, {
347364
reportsWithDraft = {},
365+
reportActions = {},
348366
betas = [],
349367
selectedOptions = [],
350368
maxRecentReportsToShow = 0,
@@ -449,7 +467,7 @@ function getOptions(reports, personalDetails, activeReportID, {
449467
reportMapForLogins[logins[0]] = report;
450468
}
451469
const isSearchingSomeonesPolicyExpenseChat = !report.isOwnPolicyExpenseChat && searchValue !== '';
452-
allReportOptions.push(createOption(logins, personalDetails, report, reportsWithDraft, {
470+
allReportOptions.push(createOption(logins, personalDetails, report, reportsWithDraft, reportActions, {
453471
showChatPreviewLine,
454472
forcePolicyNamePreview: isPolicyExpenseChat ? isSearchingSomeonesPolicyExpenseChat : forcePolicyNamePreview,
455473
}));
@@ -460,6 +478,7 @@ function getOptions(reports, personalDetails, activeReportID, {
460478
personalDetails,
461479
reportMapForLogins[personalDetail.login],
462480
reportsWithDraft,
481+
reportActions,
463482
{
464483
showChatPreviewLine,
465484
forcePolicyNamePreview,
@@ -581,7 +600,7 @@ function getOptions(reports, personalDetails, activeReportID, {
581600
const login = (Str.isValidPhone(searchValue) && !searchValue.includes('+'))
582601
? `+${countryCodeByIP}${searchValue}`
583602
: searchValue;
584-
userToInvite = createOption([login], personalDetails, null, reportsWithDraft, {
603+
userToInvite = createOption([login], personalDetails, null, reportsWithDraft, reportActions, {
585604
showChatPreviewLine,
586605
});
587606
userToInvite.icons = [ReportUtils.getDefaultAvatar(login)];
@@ -744,9 +763,10 @@ function getMemberInviteOptions(
744763
* @param {String} priorityMode
745764
* @param {Array<String>} betas
746765
* @param {Object} reportsWithDraft
766+
* @param {Object} reportActions
747767
* @returns {Object}
748768
*/
749-
function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportsWithDraft) {
769+
function calculateSidebarOptions(reports, personalDetails, activeReportID, priorityMode, betas, reportsWithDraft, reportActions) {
750770
let sideBarOptions = {
751771
prioritizeIOUDebts: true,
752772
prioritizeReportsWithDraftComments: true,
@@ -767,6 +787,7 @@ function calculateSidebarOptions(reports, personalDetails, activeReportID, prior
767787
prioritizePinnedReports: true,
768788
...sideBarOptions,
769789
reportsWithDraft,
790+
reportActions,
770791
});
771792
}
772793

‎src/libs/actions/Report.js

Lines changed: 2 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,6 @@ const allReports = {};
5959
let conciergeChatReportID;
6060
const typingWatchTimers = {};
6161

62-
// Map of optimistic report action IDs. These should be cleared when replaced by a recent fetch of report history
63-
// since we will then be up to date and any optimistic actions that are still waiting to be replaced can be removed.
64-
const optimisticReportActionIDs = {};
65-
6662
/**
6763
* @param {Number} reportID
6864
* @param {Number} lastReadSequenceNumber
@@ -398,26 +394,6 @@ function setLocalIOUReportData(iouReportObject) {
398394
Onyx.merge(iouReportKey, iouReportObject);
399395
}
400396

401-
/**
402-
* Remove all optimistic actions from report actions and reset the optimisticReportActionsIDs array. We do this
403-
* to clear any stuck optimistic actions that have not be updated for whatever reason.
404-
*
405-
* @param {Number} reportID
406-
*/
407-
function removeOptimisticActions(reportID) {
408-
const actionIDs = optimisticReportActionIDs[reportID] || [];
409-
const actionsToRemove = _.reduce(actionIDs, (actions, actionID) => ({
410-
...actions,
411-
[actionID]: null,
412-
}), {});
413-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, actionsToRemove);
414-
415-
// Reset the optimistic report action IDs to an empty array
416-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
417-
optimisticReportActionIDs: [],
418-
});
419-
}
420-
421397
/**
422398
* Fetch the iouReport and persist the data to Onyx.
423399
*
@@ -663,10 +639,6 @@ function fetchActions(reportID) {
663639
reportActionsLimit: CONST.REPORT.ACTIONS.LIMIT,
664640
})
665641
.then((data) => {
666-
// We must remove all optimistic actions so there will not be any stuck comments. At this point, we should
667-
// be caught up and no longer need any optimistic comments.
668-
removeOptimisticActions(reportID);
669-
670642
const indexedData = _.indexBy(data.history, 'sequenceNumber');
671643
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, indexedData);
672644
});
@@ -820,7 +792,7 @@ function buildOptimisticReportAction(reportID, text, file) {
820792
isFirstItem: false,
821793
isAttachment,
822794
attachmentInfo,
823-
isLoading: true,
795+
pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
824796
shouldShow: true,
825797
},
826798
};
@@ -875,19 +847,6 @@ function addActions(reportID, text = '', file) {
875847
lastReadSequenceNumber: newSequenceNumber,
876848
};
877849

878-
// Store the optimistic action ID on the report the comment was added to. It will be removed later when refetching
879-
// report actions in order to clear out any stuck actions (i.e. actions where the client never received a Pusher
880-
// event, for whatever reason, from the server with the new action data
881-
optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || [])];
882-
883-
if (text) {
884-
optimisticReport.optimisticReportActionIDs.push(reportCommentAction.clientID);
885-
}
886-
887-
if (file) {
888-
optimisticReport.optimisticReportActionIDs.push(attachmentAction.clientID);
889-
}
890-
891850
// Optimistically add the new actions to the store before waiting to save them to the server
892851
const optimisticReportActions = {};
893852
if (text) {
@@ -930,26 +889,8 @@ function addActions(reportID, text = '', file) {
930889
DateUtils.setTimezoneUpdated();
931890
}
932891

933-
const failureDataReportActions = {};
934-
const defaultLoadingState = {
935-
isLoading: false,
936-
};
937-
938-
if (text) {
939-
failureDataReportActions[reportCommentAction.clientID] = defaultLoadingState;
940-
}
941-
942-
if (file) {
943-
failureDataReportActions[attachmentAction.clientID] = defaultLoadingState;
944-
}
945-
946892
API.write(commandName, parameters, {
947893
optimisticData,
948-
failureData: [{
949-
onyxMethod: CONST.ONYX.METHOD.MERGE,
950-
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
951-
value: failureDataReportActions,
952-
}],
953894
});
954895
}
955896

@@ -1246,9 +1187,6 @@ function handleReportChanged(report) {
12461187
if (report.reportID && report.reportName === undefined) {
12471188
fetchChatReportsByIDs([report.reportID]);
12481189
}
1249-
1250-
// Store optimistic actions IDs for each report
1251-
optimisticReportActionIDs[report.reportID] = report.optimisticReportActionIDs;
12521190
}
12531191

12541192
/**
@@ -1489,7 +1427,7 @@ function viewNewReportAction(reportID, action) {
14891427
if (isFromCurrentUser) {
14901428
updatedReportObject.unreadActionCount = 0;
14911429
updatedReportObject.lastVisitedTimestamp = Date.now();
1492-
updatedReportObject.lastReadSequenceNumber = action.sequenceNumber;
1430+
updatedReportObject.lastReadSequenceNumber = action.pendingAction ? lastReadSequenceNumber : action.sequenceNumber;
14931431
} else if (incomingSequenceNumber > lastReadSequenceNumber) {
14941432
updatedReportObject.unreadActionCount = getUnreadActionCount(reportID) + 1;
14951433
}

‎src/libs/actions/ReportActions.js

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,10 +130,21 @@ function isFromCurrentUser(reportID, sequenceNumber, currentUserAccountID, actio
130130
return action.actorAccountID === currentUserAccountID;
131131
}
132132

133+
/**
134+
* @param {Number} reportID
135+
* @param {String} clientID
136+
*/
137+
function deleteClientAction(reportID, clientID) {
138+
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
139+
[clientID]: null,
140+
});
141+
}
142+
133143
export {
134144
isReportMissingActions,
135145
dangerouslyGetReportActionsMaxSequenceNumber,
136146
getDeletedCommentsCount,
137147
getLastVisibleMessageText,
138148
isFromCurrentUser,
149+
deleteClientAction,
139150
};

‎src/pages/home/report/ReportActionItem.js

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ import styles from '../../../styles/styles';
3131
import SelectionScraper from '../../../libs/SelectionScraper';
3232
import * as User from '../../../libs/actions/User';
3333
import * as ReportUtils from '../../../libs/ReportUtils';
34+
import OfflineWithFeedback from '../../../components/OfflineWithFeedback';
35+
import * as ReportActions from '../../../libs/actions/ReportActions';
3436

3537
const propTypes = {
3638
/** The ID of the report this action is on. */
@@ -185,17 +187,24 @@ class ReportActionItem extends Component {
185187
(this.props.network.isOffline && this.props.action.isLoading) || this.props.action.error,
186188
)}
187189
>
188-
{!this.props.displayAsGroup
189-
? (
190-
<ReportActionItemSingle action={this.props.action} showHeader={!this.props.draftMessage}>
191-
{children}
192-
</ReportActionItemSingle>
193-
)
194-
: (
195-
<ReportActionItemGrouped>
196-
{children}
197-
</ReportActionItemGrouped>
198-
)}
190+
<OfflineWithFeedback
191+
onClose={() => ReportActions.deleteClientAction(this.props.report.reportID, this.props.action.clientID)}
192+
pendingAction={this.props.action.pendingAction}
193+
errors={this.props.action.errors}
194+
errorRowStyles={[styles.ml10, styles.mr2]}
195+
>
196+
{!this.props.displayAsGroup
197+
? (
198+
<ReportActionItemSingle action={this.props.action} showHeader={!this.props.draftMessage}>
199+
{children}
200+
</ReportActionItemSingle>
201+
)
202+
: (
203+
<ReportActionItemGrouped>
204+
{children}
205+
</ReportActionItemGrouped>
206+
)}
207+
</OfflineWithFeedback>
199208
</View>
200209
<MiniReportActionContextMenu
201210
reportID={this.props.reportID}

0 commit comments

Comments
 (0)