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.

Sign up for GitHub

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

#3293 update last message details on message deletion #3526

Closed
wants to merge 7 commits into from
+36 −1
Closed

#3293 update last message details on message deletion #3526

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d7250c5
#3293 update last message details on message deletion
dklymenk Jun 9, 2021
71867f6
#3293 add comment for last message html manipulations
dklymenk Jun 15, 2021
0c9d2fa
#3293 remove comma after CREATED
dklymenk Jun 17, 2021
f480ca5
#3293 use destructuring to avoid having reportActionItem and reportAc…
dklymenk Jun 17, 2021
3c54c02
#3293 use Str.stripHTML
dklymenk Jun 18, 2021
e0d282d
#3293 replace _.once with Onyx.disconnect
dklymenk Jun 20, 2021
6c57d8d
Merge branch 'Expensify:main' into 3293-update-last-message-details-o…
dklymenk Jun 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/libs/actions/Report.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,9 @@ function getChatReportName(fullReport, chatType) {
*/
function getSimplifiedReportObject(report) {
const reportActionList = lodashGet(report, ['reportActionList'], []);
const lastReportAction = !_.isEmpty(reportActionList) ? _.last(reportActionList) : null;
const lastReportAction = !_.isEmpty(reportActionList)
? _.find([...reportActionList].reverse(), reportAction => reportAction.message.html !== '')
NikkiWines marked this conversation as resolved.
Show resolved Hide resolved
: null;
const createTimestamp = lastReportAction ? lastReportAction.created : 0;
const lastMessageTimestamp = moment.utc(createTimestamp).unix();
const isLastMessageAttachment = /<img([^>]+)\/>/gi.test(lodashGet(lastReportAction, ['message', 'html'], ''));
Expand Down Expand Up @@ -1105,6 +1107,39 @@ function deleteReportComment(reportID, reportAction) {
};

Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, reportActionsToMerge);
} else {
// Update last message information in the report object
const connectionID = Onyx.connect({
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`,
callback: (reportActionList) => {
// To make sure the callback is only called once
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NAB: // Disconnect to ensure the callback is only called once

Onyx.disconnect(connectionID);

// Find last non-empty message
const lastReportAction = !_.isEmpty(reportActionList)
NikkiWines marked this conversation as resolved.
Show resolved Hide resolved
? _.find(Object.values(reportActionList).reverse(),
({message, actionName}) => _.last(message).html !== ''
&& actionName !== 'CREATED')
: null;

// Populate last message details
const lastMessageDetails = {};
if (lastReportAction) {
// Replace line breaks with spaces and remove all HTML from the message
lastMessageDetails.lastMessageText = lastReportAction.isAttachment
NikkiWines marked this conversation as resolved.
Show resolved Hide resolved
? '[Attachment]'
: Str.stripHTML(_.last(lastReportAction.message).html.replace(/((<br[^>]*>)+)/gi, ' '));
lastMessageDetails.lastMessageTimestamp = lastReportAction.timestamp;
lastMessageDetails.lastActorEmail = lodashGet(lastReportAction, 'actorEmail', '');
} else {
lastMessageDetails.lastMessageText = '';
lastMessageDetails.lastMessageTimestamp = 0;
lastMessageDetails.lastActorEmail = '';
}

Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`, lastMessageDetails);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement causes a recursion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please explain how does it cause a recursion? Is there some specific edge case I didn't see. The code runs only once after the response from BE arrives.

Copy link
Member

@parasharrajat parasharrajat Jun 20, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry didn't notice at first sight that this is a different subscription key.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please release the connection here before this statement. And remove _.once. Could you please try to think of your solution in an optimistic way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware this is not the best approach. As I stated in one of the messages above, ideally we move this entire code to updateReportActionMessage function and have the handling for delete action next to edit action. However, until #3524 is resolved this cannot be done for delete action.

},
});
}
});
}
Expand Down