Skip to content

Commit

Permalink
Merge pull request #29418 from eh2077/fix-29225-edit-chat-converts-te…
Browse files Browse the repository at this point in the history 
…xt-to-link-when-making-no-changes

Fix edit chat converts text to link when making no changes
  • Loading branch information
@tylerkaraszewski
tylerkaraszewski committed Oct 18, 2023
2 parents ee37ebe + a3e0903 commit f7fce26
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 25 deletions.
18 changes: 12 additions & 6 deletions src/libs/actions/Report.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1162,17 +1162,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 @@ -1191,15 +1190,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
View file
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
View file
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';
expect(newCommentHTML).toBe(expectedOutput);
});
Expand Down

0 comments on commit f7fce26

Please sign in to comment.