Skip to content

Commit ea795fa

Browse files
PauloGasparSvOSBotify
authored andcommitted
Merge pull request #9350 from Expensify/marcaaron-updateReportWithNewAction2
Replace Report_AddComment with AddComment (Part 2) (cherry picked from commit 258c268)
1 parent b352f2a commit ea795fa

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
@@ -901,7 +846,7 @@ function fetchAllReports(
901846
* @param {String} text
902847
* @param {File} [file]
903848
*/
904-
function addAction(reportID, text, file) {
849+
function addComment(reportID, text, file) {
905850
// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
906851
// For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
907852
const parser = new ExpensiMark();
@@ -919,12 +864,12 @@ function addAction(reportID, text, file) {
919864
: parser.htmlToText(htmlForNewComment);
920865

921866
// Update the report in Onyx to have the new sequence number
922-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, {
867+
const optimisticReport = {
923868
maxSequenceNumber: newSequenceNumber,
924869
lastMessageTimestamp: moment().unix(),
925870
lastMessageText: ReportUtils.formatReportLastMessageText(textForNewComment),
926871
lastActorEmail: currentUserEmail,
927-
});
872+
};
928873

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

947890
// Optimistically add the new comment to the store before waiting to save it to the server
948-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
891+
const optimisticReportActions = {
949892
[optimisticReportActionID]: {
950893
actionName: CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT,
951894
actorEmail: currentUserEmail,
@@ -974,41 +917,70 @@ function addAction(reportID, text, file) {
974917
isFirstItem: false,
975918
isAttachment,
976919
attachmentInfo,
977-
loading: true,
920+
isLoading: true,
978921
shouldShow: true,
979922
},
980-
});
923+
};
981924

982-
DeprecatedAPI.Report_AddComment({
925+
const parameters = {
983926
reportID,
984927
file,
985928
reportComment: commentText,
986929
clientID: optimisticReportActionID,
987-
persist: true,
988-
})
989-
.then((response) => {
990-
if (response.jsonCode === 408) {
991-
Growl.error(Localize.translateLocal('reportActionCompose.fileUploadFailed'));
992-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
993-
[optimisticReportActionID]: null,
994-
});
995-
console.error(response.message);
996-
return;
997-
}
998-
999-
if (response.jsonCode === 666 && reportID === conciergeChatReportID) {
1000-
Growl.error(Localize.translateLocal('reportActionCompose.blockedFromConcierge'));
1001-
Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {
1002-
[optimisticReportActionID]: null,
1003-
});
930+
};
1004931

1005-
// 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
1006-
User.getUserDetails();
1007-
return;
1008-
}
932+
const optimisticData = [
933+
{
934+
onyxMethod: CONST.ONYX.METHOD.MERGE,
935+
key: `${ONYXKEYS.COLLECTION.REPORT}${reportID}`,
936+
value: optimisticReport,
937+
},
938+
{
939+
onyxMethod: CONST.ONYX.METHOD.MERGE,
940+
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
941+
value: optimisticReportActions,
942+
},
943+
];
1009944

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

1014986
/**
@@ -1522,7 +1494,8 @@ export {
15221494
fetchChatReportsByIDs,
15231495
fetchIOUReportByID,
15241496
fetchIOUReportByIDAndUpdateChatReport,
1525-
addAction,
1497+
addComment,
1498+
addAttachment,
15261499
updateLastReadActionID,
15271500
updateNotificationPreference,
15281501
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';
@@ -448,8 +447,6 @@ class ReportActionCompose extends React.Component {
448447
return;
449448
}
450449

451-
DateUtils.throttledUpdateTimezone();
452-
453450
this.props.onSubmit(trimmedComment);
454451
this.updateComment('');
455452
this.setTextInputShouldClear(true);
@@ -498,7 +495,7 @@ class ReportActionCompose extends React.Component {
498495
headerTitle={this.props.translate('reportActionCompose.sendAttachment')}
499496
onConfirm={(file) => {
500497
this.submitForm();
501-
Report.addAction(this.props.reportID, '', file);
498+
Report.addAttachment(this.props.reportID, file);
502499
this.setTextInputShouldClear(false);
503500
}}
504501
>

0 commit comments

Comments
 (0)