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.

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

Fix edit chat converts text to link when making no changes #29418

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 12 additions & 6 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1161,17 +1161,16 @@ const removeLinksFromHtml = (html, links) => {
* 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 {String} originalHtml original html of the comment before editing.
* @param {String} originalCommentMarkdown original markdown of the comment before editing.
* @returns {String}
*/
const handleUserDeletedLinksInHtml = (newCommentText, originalHtml) => {
const handleUserDeletedLinksInHtml = (newCommentText, originalCommentMarkdown) => {
const parser = new ExpensiMark();
if (newCommentText.length > CONST.MAX_MARKUP_LENGTH) {
return newCommentText;
}
const markdownOriginalComment = parser.htmlToMarkdown(originalHtml).trim();
const htmlForNewComment = parser.replace(newCommentText);
const removedLinks = parser.getRemovedMarkdownLinks(markdownOriginalComment, newCommentText);
const removedLinks = parser.getRemovedMarkdownLinks(originalCommentMarkdown, newCommentText);
return removeLinksFromHtml(htmlForNewComment, removedLinks);
};

Expand All @@ -1190,15 +1189,22 @@ 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 htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, originalCommentHTML);
const originalCommentMarkdown = parser.htmlToMarkdown(originalCommentHTML).trim();

// Skip the Edit if draft is not changed
if (originalCommentMarkdown === textForNewComment) {
return;
}

const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, originalCommentMarkdown);
const reportComment = parser.htmlToText(htmlForNewComment);

// For comments shorter than or equal to 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 parsedOriginalCommentHTML = originalCommentHTML;
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);
parsedOriginalCommentHTML = parser.replace(originalCommentMarkdown, autolinkFilter);
}

// Delete the comment if it's empty
Expand Down
2 changes: 1 addition & 1 deletion src/pages/PrivateNotes/PrivateNotesEditPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ function PrivateNotesEditPage({route, personalDetailsList, session, report}) {

const savePrivateNote = () => {
const originalNote = lodashGet(report, ['privateNotes', route.params.accountID, 'note'], '');
const editedNote = Report.handleUserDeletedLinksInHtml(privateNote.trim(), originalNote);
const editedNote = Report.handleUserDeletedLinksInHtml(privateNote.trim(), parser.htmlToMarkdown(originalNote).trim());
Report.updatePrivateNotes(report.reportID, route.params.accountID, editedNote);
Keyboard.dismiss();

Expand Down
36 changes: 18 additions & 18 deletions tests/actions/ReportTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -428,75 +428,75 @@ describe('actions/Report', () => {

// User edits comment to add link
// We should generate link
let originalCommentHTML = 'Original Comment';
let originalCommentMarkdown = 'Original Comment';
let afterEditCommentText = 'Original Comment www.google.com';
let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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>';
originalCommentMarkdown = 'Comment [www.google.com](https://www.google.com)';
afterEditCommentText = 'Comment www.google.com';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
expectedOutput = 'Comment www.google.com';
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>';
originalCommentMarkdown = 'Comment [www.google.com](https://www.google.com)';
afterEditCommentText = 'Comment [www.google.com]';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
expectedOutput = 'Comment [www.google.com]';
expect(newCommentHTML).toBe(expectedOutput);

// User Generates multiple links in one edit
// We should generate both links
originalCommentHTML = 'Comment';
originalCommentMarkdown = 'Comment';
afterEditCommentText = 'Comment www.google.com www.facebook.com';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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>';
originalCommentMarkdown = 'Comment [www.google.com](https://www.google.com) [www.facebook.com](https://www.facebook.com)';
afterEditCommentText = 'Comment www.google.com [www.facebook.com](https://www.facebook.com)';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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';
originalCommentMarkdown = 'Comment';
afterEditCommentText = 'https://www.facebook.com/hashtag/__main/?__eep__=6';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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>';
originalCommentMarkdown = '[https://www.facebook.com/hashtag/__main/?__eep__=6](https://www.facebook.com/hashtag/__main/?__eep__=6)';
afterEditCommentText = 'https://www.facebook.com/hashtag/__main/?__eep__=6';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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';
originalCommentMarkdown = 'Comment';
afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
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>';
originalCommentMarkdown = '[http://example.com/foo/*/bar/*/test.txt](http://example.com/foo/*/bar/*/test.txt)';
afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt';
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML);
newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown);
expectedOutput = 'http://example.com/foo/*/bar/*/test.txt';
eh2077 marked this conversation as resolved.
Show resolved Hide resolved
expect(newCommentHTML).toBe(expectedOutput);
});
Expand Down
Loading