Skip to content
Merged
12 changes: 6 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
"babel-polyfill": "^6.26.0",
"dom-serializer": "^0.2.2",
"domhandler": "^4.3.0",
"expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#be625f55058111793bbcfe3f64788ea0a8f9d705",
"expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#c15668f73079d3f5f42618b70c852d36d387e121",
"fbjs": "^3.0.2",
"html-entities": "^1.3.1",
"htmlparser2": "^7.2.0",
Expand Down
45 changes: 21 additions & 24 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -823,42 +823,42 @@ const getRemovedMarkdownLinks = (oldComment, newComment) => {
};

/**
* Removes the links in a markdown comment.
* Removes the links in html of a comment.
* example:
* comment="test [link](https://www.google.com) test",
* html="test <a href="https://www.google.com" target="_blank" rel="noreferrer noopener">https://www.google.com</a> test"
* links=["https://www.google.com"]
* returns: "test link test"
* @param {String} comment
* returns: "test https://www.google.com test"
*
* @param {String} html
* @param {Array} links
* @returns {String}
*/
const removeLinks = (comment, links) => {
let commentCopy = comment.slice();
const removeLinksFromHtml = (html, links) => {
let htmlCopy = html.slice();
_.forEach(links, (link) => {
const regex = new RegExp(`\\[([^\\[\\]]*)\\]\\(${link}\\)`, 'gm');
const linkMatch = regex.exec(commentCopy);
const linkText = linkMatch && linkMatch[1];
commentCopy = commentCopy.replace(`[${linkText}](${link})`, linkText);
// We want to match the anchor tag of the link and replace the whole anchor tag with the text of the anchor tag
const regex = new RegExp(`<(a)[^><]*href\\s*=\\s*(['"])(${Str.escapeForRegExp(link)})\\2(?:".*?"|'.*?'|[^'"><])*>([\\s\\S]*?)<\\/\\1>(?![^<]*(<\\/pre>|<\\/code>))`, 'gi');

@fedirjh fedirjh Apr 18, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@eh2077 Can we add a comment here explaining the logic behind it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@fedirjh Updated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This caused the following bug:
#21417

We should not have ignored the case flag.

htmlCopy = htmlCopy.replace(regex, '$4');
});
return commentCopy;
return htmlCopy;
};

/**
* This function will handle removing only links that were purposely removed by the user while editing.
*
* @param {String} newCommentText text of the comment after editing.
* @param {Array} originalHtml original html of the comment before editing
* @param {String} originalHtml original html of the comment before editing.
* @returns {String}
*/
const handleUserDeletedLinks = (newCommentText, originalHtml) => {
const handleUserDeletedLinksInHtml = (newCommentText, originalHtml) => {
const parser = new ExpensiMark();
if (newCommentText.length >= CONST.MAX_MARKUP_LENGTH) {
return newCommentText;
}
const htmlWithAutoLinks = parser.replace(newCommentText);
const markdownWithAutoLinks = parser.htmlToMarkdown(htmlWithAutoLinks).trim();
const markdownOriginalComment = parser.htmlToMarkdown(originalHtml).trim();
const htmlForNewComment = parser.replace(newCommentText);
const removedLinks = getRemovedMarkdownLinks(markdownOriginalComment, newCommentText);
return removeLinks(markdownWithAutoLinks, removedLinks);
return removeLinksFromHtml(htmlForNewComment, removedLinks);
};

/**
Expand All @@ -875,16 +875,13 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
// https://github.com/Expensify/App/issues/9090
// https://github.com/Expensify/App/issues/13221
const originalCommentHTML = lodashGet(originalReportAction, 'message[0].html');
const markdownForNewComment = handleUserDeletedLinks(textForNewComment, originalCommentHTML);

const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), name => name !== 'autolink')};
const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, originalCommentHTML);

// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
// For longer comments, skip parsing and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
let htmlForNewComment = markdownForNewComment;
let parsedOriginalCommentHTML = originalCommentHTML;
if (markdownForNewComment.length < CONST.MAX_MARKUP_LENGTH) {
htmlForNewComment = parser.replace(markdownForNewComment, autolinkFilter);
if (textForNewComment.length < CONST.MAX_MARKUP_LENGTH) {
const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), name => name !== 'autolink')};
parsedOriginalCommentHTML = parser.replace(parser.htmlToMarkdown(originalCommentHTML).trim(), autolinkFilter);
}

Expand All @@ -909,7 +906,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
...originalMessage,
isEdited: true,
html: htmlForNewComment,
text: markdownForNewComment,
text: textForNewComment,
}],
},
};
Expand Down Expand Up @@ -1476,7 +1473,7 @@ export {
broadcastUserIsTyping,
togglePinnedState,
editReportComment,
handleUserDeletedLinks,
handleUserDeletedLinksInHtml,
saveReportActionDraft,
saveReportActionDraftNumberOfLines,
deleteReportComment,
Expand Down
59 changes: 46 additions & 13 deletions tests/actions/ReportTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -430,41 +430,74 @@ describe('actions/Report', () => {
// We should generate link
let originalCommentHTML = 'Original Comment';
let afterEditCommentText = 'Original Comment www.google.com';
let newCommentMarkdown = Report.handleUserDeletedLinks(afterEditCommentText, originalCommentHTML);
let expectedOutput = 'Original Comment [www.google.com](https://www.google.com)';
expect(newCommentMarkdown).toBe(expectedOutput);
let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
let expectedOutput = 'Original Comment <a href="https://www.google.com" target="_blank" rel="noreferrer noopener">www.google.com</a>';
expect(newCommentHTML).toBe(expectedOutput);

// User deletes www.google.com link from comment but keeps link text
// We should not generate link
originalCommentHTML = 'Comment <a href="https://www.google.com" target="_blank">www.google.com</a>';
afterEditCommentText = 'Comment www.google.com';
newCommentMarkdown = Report.handleUserDeletedLinks(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment www.google.com';
expect(newCommentMarkdown).toBe(expectedOutput);
expect(newCommentHTML).toBe(expectedOutput);

// User Delete only () part of link but leaves the []
// We should not generate link
originalCommentHTML = 'Comment <a href="https://www.google.com" target="_blank">www.google.com</a>';
afterEditCommentText = 'Comment [www.google.com]';
newCommentMarkdown = Report.handleUserDeletedLinks(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment [www.google.com]';
expect(newCommentMarkdown).toBe(expectedOutput);
expect(newCommentHTML).toBe(expectedOutput);

// User Generates multiple links in one edit
// We should generate both links
originalCommentHTML = 'Comment';
afterEditCommentText = 'Comment www.google.com www.facebook.com';
newCommentMarkdown = Report.handleUserDeletedLinks(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment [www.google.com](https://www.google.com) [www.facebook.com](https://www.facebook.com)';
expect(newCommentMarkdown).toBe(expectedOutput);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment <a href="https://www.google.com" target="_blank" rel="noreferrer noopener">www.google.com</a> '
+ '<a href="https://www.facebook.com" target="_blank" rel="noreferrer noopener">www.facebook.com</a>';
expect(newCommentHTML).toBe(expectedOutput);

// Comment has two links but user deletes only one of them
// Should not generate link again for the deleted one
originalCommentHTML = 'Comment <a href="https://www.google.com" target="_blank">www.google.com</a> <a href="https://www.facebook.com" target="_blank">www.facebook.com</a>';
afterEditCommentText = 'Comment www.google.com [www.facebook.com](https://www.facebook.com)';
newCommentMarkdown = Report.handleUserDeletedLinks(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment www.google.com [www.facebook.com](https://www.facebook.com)';
expect(newCommentMarkdown).toBe(expectedOutput);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'Comment www.google.com <a href="https://www.facebook.com" target="_blank" rel="noreferrer noopener">www.facebook.com</a>';
expect(newCommentHTML).toBe(expectedOutput);

// User edits and replaces comment with a link containing underscores
// We should generate link
originalCommentHTML = 'Comment';
afterEditCommentText = 'https://www.facebook.com/hashtag/__main/?__eep__=6';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = '<a href="https://www.facebook.com/hashtag/__main/?__eep__=6" target="_blank" rel="noreferrer noopener">https://www.facebook.com/hashtag/__main/?__eep__=6</a>';
expect(newCommentHTML).toBe(expectedOutput);

// User edits and deletes the link containing underscores
// We should not generate link
originalCommentHTML = '<a href="https://www.facebook.com/hashtag/__main/?__eep__=6" target="_blank" rel="noreferrer noopener">https://www.facebook.com/hashtag/__main/?__eep__=6</a>';
afterEditCommentText = 'https://www.facebook.com/hashtag/__main/?__eep__=6';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'https://www.facebook.com/hashtag/__main/?__eep__=6';
expect(newCommentHTML).toBe(expectedOutput);

// User edits and replaces comment with a link containing asterisks
// We should generate link
originalCommentHTML = 'Comment';
afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = '<a href="http://example.com/foo/*/bar/*/test.txt" target="_blank" rel="noreferrer noopener">http://example.com/foo/*/bar/*/test.txt</a>';
expect(newCommentHTML).toBe(expectedOutput);

// User edits and deletes the link containing asterisks
// We should not generate link
originalCommentHTML = '<a href="http://example.com/foo/*/bar/*/test.txt" target="_blank" rel="noreferrer noopener">http://example.com/foo/*/bar/*/test.txt</a>';
afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
expectedOutput = 'http://example.com/foo/*/bar/*/test.txt';
expect(newCommentHTML).toBe(expectedOutput);
});

it('should show a notification for report action updates with shouldNotify', () => {
Expand Down