Skip to content

Commit 258c268

Browse files
Merge pull request #9350 from Expensify/marcaaron-updateReportWithNewAction2
Replace Report_AddComment with AddComment (Part 2)
2 parents 2005ab5 + 8761aa6 commit 258c268

15 files changed

+161
-135
lines changed

src/CONST.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,12 @@ const CONST = {
730730

731731
// There's a limit of 60k characters in Auth - https://github.com/Expensify/Auth/blob/198d59547f71fdee8121325e8bc9241fc9c3236a/auth/lib/Request.h#L28
732732
MAX_COMMENT_LENGTH: 60000,
733+
734+
ONYX: {
735+
METHOD: {
736+
MERGE: 'merge',
737+
},
738+
},
733739
};
734740

735741
export default CONST;

src/components/InlineSystemMessage.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@ import Icon from './Icon';
99

1010
const propTypes = {
1111
/** Error to display */
12-
message: PropTypes.string.isRequired,
12+
message: PropTypes.string,
13+
};
14+
15+
const defaultProps = {
16+
message: '',
1317
};
1418

1519
const InlineSystemMessage = (props) => {
@@ -25,5 +29,6 @@ const InlineSystemMessage = (props) => {
2529
};
2630

2731
InlineSystemMessage.propTypes = propTypes;
32+
InlineSystemMessage.defaultProps = defaultProps;
2833
InlineSystemMessage.displayName = 'InlineSystemMessage';
2934
export default InlineSystemMessage;

src/libs/DateUtils.js

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,21 +115,38 @@ function startCurrentDateUpdater() {
115115
});
116116
}
117117

118-
/*
119-
* Updates user's timezone, if their timezone is set to automatic and
120-
* is different from current timezone
118+
/**
119+
* @returns {Object}
121120
*/
122-
function updateTimezone() {
121+
function getCurrentTimezone() {
123122
const currentTimezone = moment.tz.guess(true);
124123
if (timezone.automatic && timezone.selected !== currentTimezone) {
125-
PersonalDetails.setPersonalDetails({timezone: {...timezone, selected: currentTimezone}});
124+
return {...timezone, selected: currentTimezone};
126125
}
126+
return timezone;
127127
}
128128

129129
/*
130-
* Returns a version of updateTimezone function throttled by 5 minutes
130+
* Updates user's timezone, if their timezone is set to automatic and
131+
* is different from current timezone
132+
*/
133+
function updateTimezone() {
134+
PersonalDetails.setPersonalDetails({timezone: getCurrentTimezone()});
135+
}
136+
137+
// Used to throttle updates to the timezone when necessary
138+
let lastUpdatedTimezoneTime = moment();
139+
140+
/**
141+
* @returns {Boolean}
131142
*/
132-
const throttledUpdateTimezone = _.throttle(() => updateTimezone(), 1000 * 60 * 5);
143+
function canUpdateTimezone() {
144+
return lastUpdatedTimezoneTime.isBefore(moment().subtract(5, 'minutes'));
145+
}
146+
147+
function setTimezoneUpdated() {
148+
lastUpdatedTimezoneTime = moment();
149+
}
133150

134151
/**
135152
* @namespace DateUtils
@@ -139,8 +156,10 @@ const DateUtils = {
139156
timestampToDateTime,
140157
startCurrentDateUpdater,
141158
updateTimezone,
142-
throttledUpdateTimezone,
143159
getLocalMomentFromTimestamp,
160+
getCurrentTimezone,
161+
canUpdateTimezone,
162+
setTimezoneUpdated,
144163
};
145164

146165
export default DateUtils;

src/libs/actions/Report.js

Lines changed: 64 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import * as Pusher from '../Pusher/pusher';
99
import LocalNotification from '../Notification/LocalNotification';
1010
import PushNotification from '../Notification/PushNotification';
1111
import * as PersonalDetails from './PersonalDetails';
12-
import * as User from './User';
1312
import Navigation from '../Navigation/Navigation';
1413
import * as ActiveClientManager from '../ActiveClientManager';
1514
import Visibility from '../Visibility';
@@ -27,6 +26,7 @@ import * as ReportActions from './ReportActions';
2726
import Growl from '../Growl';
2827
import * as Localize from '../Localize';
2928
import PusherUtils from '../PusherUtils';
29+
import DateUtils from '../DateUtils';
3030

3131
let currentUserEmail;
3232
let currentUserAccountID;
@@ -532,54 +532,6 @@ function updateReportActionMessage(reportID, sequenceNumber, message) {
532532
});
533533
}
534534

535-
/**
536-
* Updates a report in the store with a new report action
537-
*
538-
* @param {Number} reportID
539-
* @param {Object} reportAction
540-
* @param {String} [notificationPreference] On what cadence the user would like to be notified
541-
*/
542-
function updateReportWithNewAction(
543-
reportID,
544-
reportAction,
545-
notificationPreference = CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS,
546-
) {
547-
const messageHtml = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.RENAMED
548-
? lodashGet(reportAction, 'originalMessage.html', '')
549-
: lodashGet(reportAction, ['message', 0, 'html'], '');
550-
551-
const parser = new ExpensiMark();
552-
const messageText = parser.htmlToText(messageHtml);
553-
554-
const updatedReportObject = {
555-
// Always merge the reportID into Onyx. If the report doesn't exist in Onyx yet, then all the rest of the data will be filled out by handleReportChanged
556-
reportID,
557-
maxSequenceNumber: reportAction.sequenceNumber,
558-
notificationPreference,
559-
lastMessageTimestamp: reportAction.timestamp,
560-
lastMessageText: ReportUtils.formatReportLastMessageText(messageText),
561-
lastActorEmail: reportAction.actorEmail,
562-
};
563-
564-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, updatedReportObject);
565-
566-
const reportActionsToMerge = {};
567-
if (reportAction.clientID) {
568-
// Remove the optimistic action from the report since we are about to replace it
569-
// with the real one (which has the true sequenceNumber)
570-
reportActionsToMerge[reportAction.clientID] = null;
571-
}
572-
573-
// Add the action into Onyx
574-
reportActionsToMerge[reportAction.sequenceNumber] = {
575-
...reportAction,
576-
isAttachment: ReportUtils.isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})),
577-
loading: false,
578-
};
579-
580-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge);
581-
}
582-
583535
/**
584536
* Get the private pusher channel name for a Report.
585537
*
@@ -604,13 +556,6 @@ function subscribeToUserEvents() {
604556
return;
605557
}
606558

607-
// Live-update a report's actions when a 'report comment' event is received.
608-
PusherUtils.subscribeToPrivateUserChannelEvent(
609-
Pusher.TYPE.REPORT_COMMENT,
610-
currentUserAccountID,
611-
pushJSON => updateReportWithNewAction(pushJSON.reportID, pushJSON.reportAction, pushJSON.notificationPreference),
612-
);
613-
614559
// Live-update a report's actions when an 'edit comment' event is received.
615560
PusherUtils.subscribeToPrivateUserChannelEvent(Pusher.TYPE.REPORT_COMMENT_EDIT,
616561
currentUserAccountID,
@@ -621,9 +566,9 @@ function subscribeToUserEvents() {
621566
* Setup reportComment push notification callbacks.
622567
*/
623568
function subscribeToReportCommentPushNotifications() {
624-
PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, reportAction}) => {
569+
PushNotification.onReceived(PushNotification.TYPE.REPORT_COMMENT, ({reportID, onyxData}) => {
625570
Log.info('[Report] Handled event sent by Airship', false, {reportID});
626-
updateReportWithNewAction(reportID, reportAction);
571+
Onyx.update(onyxData);
627572
});
628573

629574
// Open correct report when push notification is clicked
@@ -904,7 +849,7 @@ function fetchAllReports(
904849
* @param {String} text
905850
* @param {File} [file]
906851
*/
907-
function addAction(reportID, text, file) {
852+
function addComment(reportID, text, file) {
908853
// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
909854
// For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
910855
const parser = new ExpensiMark();
@@ -922,12 +867,12 @@ function addAction(reportID, text, file) {
922867
: parser.htmlToText(htmlForNewComment);
923868

924869
// Update the report in Onyx to have the new sequence number
925-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
870+
const optimisticReport = {
926871
maxSequenceNumber: newSequenceNumber,
927872
lastMessageTimestamp: moment().unix(),
928873
lastMessageText: ReportUtils.formatReportLastMessageText(textForNewComment),
929874
lastActorEmail: currentUserEmail,
930-
});
875+
};
931876

932877
// Generate a clientID so we can save the optimistic action to storage with the clientID as key. Later, we will
933878
// remove the optimistic action when we add the real action created in the server. We do this because it's not
@@ -943,12 +888,10 @@ function addAction(reportID, text, file) {
943888
// Store the optimistic action ID on the report the comment was added to. It will be removed later when refetching
944889
// report actions in order to clear out any stuck actions (i.e. actions where the client never received a Pusher
945890
// event, for whatever reason, from the server with the new action data
946-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
947-
optimisticReportActionIDs: [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID],
948-
});
891+
optimisticReport.optimisticReportActionIDs = [...(optimisticReportActionIDs[reportID] || []), optimisticReportActionID];
949892

950893
// Optimistically add the new comment to the store before waiting to save it to the server
951-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
894+
const optimisticReportActions = {
952895
[optimisticReportActionID]: {
953896
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
954897
actorEmail: currentUserEmail,
@@ -977,41 +920,70 @@ function addAction(reportID, text, file) {
977920
isFirstItem: false,
978921
isAttachment,
979922
attachmentInfo,
980-
loading: true,
923+
isLoading: true,
981924
shouldShow: true,
982925
},
983-
});
926+
};
984927

985-
DeprecatedAPI.Report_AddComment({
928+
const parameters = {
986929
reportID,
987930
file,
988931
reportComment: commentText,
989932
clientID: optimisticReportActionID,
990-
persist: true,
991-
})
992-
.then((response) => {
993-
if (response.jsonCode === 408) {
994-
Growl.error(Localize.translateLocal('reportActionCompose.fileUploadFailed'));
995-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
996-
[optimisticReportActionID]: null,
997-
});
998-
console.error(response.message);
999-
return;
1000-
}
1001-
1002-
if (response.jsonCode === 666 && reportID === conciergeChatReportID) {
1003-
Growl.error(Localize.translateLocal('reportActionCompose.blockedFromConcierge'));
1004-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
1005-
[optimisticReportActionID]: null,
1006-
});
933+
};
1007934

1008-
// The fact that the API is returning this error means the BLOCKED_FROM_CONCIERGE nvp in the user details has changed since the last time we checked, so let's update
1009-
User.getUserDetails();
1010-
return;
1011-
}
935+
const optimisticData = [
936+
{
937+
onyxMethod: CONST.ONYX.METHOD.MERGE,
938+
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
939+
value: optimisticReport,
940+
},
941+
{
942+
onyxMethod: CONST.ONYX.METHOD.MERGE,
943+
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
944+
value: optimisticReportActions,
945+
},
946+
];
1012947

1013-
updateReportWithNewAction(reportID, response.reportAction);
948+
// Update the timezone if it's been 5 minutes from the last time the user added a comment
949+
if (DateUtils.canUpdateTimezone()) {
950+
const timezone = DateUtils.getCurrentTimezone();
951+
parameters.timezone = JSON.stringify(timezone);
952+
optimisticData.push({
953+
onyxMethod: CONST.ONYX.METHOD.MERGE,
954+
key: ONYXKEYS.MY_PERSONAL_DETAILS,
955+
value: {timezone},
1014956
});
957+
optimisticData.push({
958+
onyxMethod: CONST.ONYX.METHOD.MERGE,
959+
key: ONYXKEYS.PERSONAL_DETAILS,
960+
value: {[currentUserEmail]: timezone},
961+
});
962+
DateUtils.setTimezoneUpdated();
963+
}
964+
965+
API.write(isAttachment ? 'AddAttachment' : 'AddComment', parameters, {
966+
optimisticData,
967+
failureData: [
968+
{
969+
onyxMethod: CONST.ONYX.METHOD.MERGE,
970+
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
971+
value: {
972+
[optimisticReportActionID]: {
973+
isLoading: false,
974+
},
975+
},
976+
},
977+
],
978+
});
979+
}
980+
981+
/**
982+
* @param {Number} reportID
983+
* @param {Object} file
984+
*/
985+
function addAttachment(reportID, file) {
986+
addComment(reportID, '', file);
1015987
}
1016988

1017989
/**
@@ -1525,7 +1497,8 @@ export {
15251497
fetchChatReportsByIDs,
15261498
fetchIOUReportByID,
15271499
fetchIOUReportByIDAndUpdateChatReport,
1528-
addAction,
1500+
addComment,
1501+
addAttachment,
15291502
updateLastReadActionID,
15301503
updateNotificationPreference,
15311504
setNewMarkerPosition,

src/libs/actions/ReportActions.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ Onyx.connect({
3434
const reportID = CollectionUtils.extractCollectionItemID(key);
3535
const actionsArray = _.toArray(actions);
3636
reportActions[reportID] = actionsArray;
37-
const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.loading);
37+
const mostRecentNonLoadingActionIndex = _.findLastIndex(actionsArray, action => !action.isLoading);
3838
const mostRecentAction = actionsArray[mostRecentNonLoadingActionIndex];
3939
if (!mostRecentAction || _.isUndefined(mostRecentAction.sequenceNumber)) {
4040
return;

src/pages/home/ReportScreen.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class ReportScreen extends React.Component {
145145
* @param {String} text
146146
*/
147147
onSubmitComment(text) {
148-
Report.addAction(getReportID(this.props.route), text);
148+
Report.addComment(getReportID(this.props.route), text);
149149
}
150150

151151
/**

src/pages/home/report/ReportActionCompose.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ import ReportActionComposeFocusManager from '../../../libs/ReportActionComposeFo
3636
import participantPropTypes from '../../../components/participantPropTypes';
3737
import ParticipantLocalTime from './ParticipantLocalTime';
3838
import {withPersonalDetails} from '../../../components/OnyxProvider';
39-
import DateUtils from '../../../libs/DateUtils';
4039
import * as User from '../../../libs/actions/User';
4140
import Tooltip from '../../../components/Tooltip';
4241
import EmojiPickerButton from '../../../components/EmojiPicker/EmojiPickerButton';
@@ -446,8 +445,6 @@ class ReportActionCompose extends React.Component {
446445
return;
447446
}
448447

449-
DateUtils.throttledUpdateTimezone();
450-
451448
this.props.onSubmit(trimmedComment);
452449
this.updateComment('');
453450
this.setTextInputShouldClear(true);
@@ -496,7 +493,7 @@ class ReportActionCompose extends React.Component {
496493
headerTitle={this.props.translate('reportActionCompose.sendAttachment')}
497494
onConfirm={(file) => {
498495
this.submitForm();
499-
Report.addAction(this.props.reportID, '', file);
496+
Report.addAttachment(this.props.reportID, file);
500497
this.setTextInputShouldClear(false);
501498
}}
502499
>

0 commit comments

Comments
 (0)