From 3f8e91734cc1bc1d5a7f93a459cecb6a46988796 Mon Sep 17 00:00:00 2001 From: Eric Han Date: Thu, 12 Oct 2023 13:02:31 +0800 Subject: [PATCH 1/2] Fix edit chat converts text to link when making no changes --- src/libs/actions/Report.js | 16 +++++++++----- .../PrivateNotes/PrivateNotesEditPage.js | 2 +- tests/actions/ReportTest.js | 21 +++++++++++-------- 3 files changed, 24 insertions(+), 15 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 27a02b1fc75f..0aa40fda1f8b 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1161,15 +1161,14 @@ 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} markdownOriginalComment original markdown of the comment before editing. * @returns {String} */ -const handleUserDeletedLinksInHtml = (newCommentText, originalHtml) => { +const handleUserDeletedLinksInHtml = (newCommentText, markdownOriginalComment) => { 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); return removeLinksFromHtml(htmlForNewComment, removedLinks); @@ -1190,7 +1189,14 @@ 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 markdownOriginalComment = parser.htmlToMarkdown(originalCommentHTML).trim(); + + // Skip the Edit if draft is not changed + if (markdownOriginalComment === textForNewComment) { + return; + } + + const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, markdownOriginalComment); 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 @@ -1198,7 +1204,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) { 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(markdownOriginalComment, autolinkFilter); } // Delete the comment if it's empty diff --git a/src/pages/PrivateNotes/PrivateNotesEditPage.js b/src/pages/PrivateNotes/PrivateNotesEditPage.js index 1bf99a6f5681..b61e7bca7a76 100644 --- a/src/pages/PrivateNotes/PrivateNotesEditPage.js +++ b/src/pages/PrivateNotes/PrivateNotesEditPage.js @@ -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(); diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index c7ef68547cdc..e9c9422c4cae 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -1,6 +1,7 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; import lodashGet from 'lodash/get'; +import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import {utcToZonedTime} from 'date-fns-tz'; import {beforeEach, beforeAll, afterEach, describe, it, expect} from '@jest/globals'; import ONYXKEYS from '../../src/ONYXKEYS'; @@ -426,11 +427,13 @@ describe('actions/Report', () => { global.fetch = TestHelper.getGlobalFetchMock(); + const parser = new ExpensiMark(); + // User edits comment to add link // We should generate link let originalCommentHTML = 'Original Comment'; let afterEditCommentText = 'Original Comment www.google.com'; - let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); let expectedOutput = 'Original Comment www.google.com'; expect(newCommentHTML).toBe(expectedOutput); @@ -438,7 +441,7 @@ describe('actions/Report', () => { // We should not generate link originalCommentHTML = 'Comment www.google.com'; afterEditCommentText = 'Comment www.google.com'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'Comment www.google.com'; expect(newCommentHTML).toBe(expectedOutput); @@ -446,7 +449,7 @@ describe('actions/Report', () => { // We should not generate link originalCommentHTML = 'Comment www.google.com'; afterEditCommentText = 'Comment [www.google.com]'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'Comment [www.google.com]'; expect(newCommentHTML).toBe(expectedOutput); @@ -454,7 +457,7 @@ describe('actions/Report', () => { // We should generate both links originalCommentHTML = 'Comment'; afterEditCommentText = 'Comment www.google.com www.facebook.com'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'Comment www.google.com ' + 'www.facebook.com'; @@ -464,7 +467,7 @@ describe('actions/Report', () => { // Should not generate link again for the deleted one originalCommentHTML = 'Comment www.google.com www.facebook.com'; afterEditCommentText = 'Comment www.google.com [www.facebook.com](https://www.facebook.com)'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'Comment www.google.com www.facebook.com'; expect(newCommentHTML).toBe(expectedOutput); @@ -472,7 +475,7 @@ describe('actions/Report', () => { // We should generate link originalCommentHTML = 'Comment'; afterEditCommentText = 'https://www.facebook.com/hashtag/__main/?__eep__=6'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'https://www.facebook.com/hashtag/__main/?__eep__=6'; expect(newCommentHTML).toBe(expectedOutput); @@ -480,7 +483,7 @@ describe('actions/Report', () => { // We should not generate link originalCommentHTML = '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, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'https://www.facebook.com/hashtag/__main/?__eep__=6'; expect(newCommentHTML).toBe(expectedOutput); @@ -488,7 +491,7 @@ describe('actions/Report', () => { // We should generate link originalCommentHTML = 'Comment'; afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'http://example.com/foo/*/bar/*/test.txt'; expect(newCommentHTML).toBe(expectedOutput); @@ -496,7 +499,7 @@ describe('actions/Report', () => { // We should not generate link originalCommentHTML = 'http://example.com/foo/*/bar/*/test.txt'; afterEditCommentText = 'http://example.com/foo/*/bar/*/test.txt'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentHTML); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); expectedOutput = 'http://example.com/foo/*/bar/*/test.txt'; expect(newCommentHTML).toBe(expectedOutput); }); From a3e09035c8a04ef644e957ce14c2dbee5708515e Mon Sep 17 00:00:00 2001 From: Eric Han Date: Thu, 12 Oct 2023 17:40:57 +0800 Subject: [PATCH 2/2] Fix naming and tidy test --- src/libs/actions/Report.js | 14 ++++++------- tests/actions/ReportTest.js | 39 +++++++++++++++++-------------------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/src/libs/actions/Report.js b/src/libs/actions/Report.js index 0aa40fda1f8b..de7dbe62afc1 100644 --- a/src/libs/actions/Report.js +++ b/src/libs/actions/Report.js @@ -1161,16 +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} markdownOriginalComment original markdown of the comment before editing. + * @param {String} originalCommentMarkdown original markdown of the comment before editing. * @returns {String} */ -const handleUserDeletedLinksInHtml = (newCommentText, markdownOriginalComment) => { +const handleUserDeletedLinksInHtml = (newCommentText, originalCommentMarkdown) => { const parser = new ExpensiMark(); if (newCommentText.length > CONST.MAX_MARKUP_LENGTH) { return newCommentText; } const htmlForNewComment = parser.replace(newCommentText); - const removedLinks = parser.getRemovedMarkdownLinks(markdownOriginalComment, newCommentText); + const removedLinks = parser.getRemovedMarkdownLinks(originalCommentMarkdown, newCommentText); return removeLinksFromHtml(htmlForNewComment, removedLinks); }; @@ -1189,14 +1189,14 @@ 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 markdownOriginalComment = parser.htmlToMarkdown(originalCommentHTML).trim(); + const originalCommentMarkdown = parser.htmlToMarkdown(originalCommentHTML).trim(); // Skip the Edit if draft is not changed - if (markdownOriginalComment === textForNewComment) { + if (originalCommentMarkdown === textForNewComment) { return; } - const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, markdownOriginalComment); + 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 @@ -1204,7 +1204,7 @@ function editReportComment(reportID, originalReportAction, textForNewComment) { let parsedOriginalCommentHTML = originalCommentHTML; if (textForNewComment.length <= CONST.MAX_MARKUP_LENGTH) { const autolinkFilter = {filterRules: _.filter(_.pluck(parser.rules, 'name'), (name) => name !== 'autolink')}; - parsedOriginalCommentHTML = parser.replace(markdownOriginalComment, autolinkFilter); + parsedOriginalCommentHTML = parser.replace(originalCommentMarkdown, autolinkFilter); } // Delete the comment if it's empty diff --git a/tests/actions/ReportTest.js b/tests/actions/ReportTest.js index e9c9422c4cae..5fa05873e63e 100644 --- a/tests/actions/ReportTest.js +++ b/tests/actions/ReportTest.js @@ -1,7 +1,6 @@ import _ from 'underscore'; import Onyx from 'react-native-onyx'; import lodashGet from 'lodash/get'; -import ExpensiMark from 'expensify-common/lib/ExpensiMark'; import {utcToZonedTime} from 'date-fns-tz'; import {beforeEach, beforeAll, afterEach, describe, it, expect} from '@jest/globals'; import ONYXKEYS from '../../src/ONYXKEYS'; @@ -427,37 +426,35 @@ describe('actions/Report', () => { global.fetch = TestHelper.getGlobalFetchMock(); - const parser = new ExpensiMark(); - // 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + let newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); let expectedOutput = 'Original Comment www.google.com'; expect(newCommentHTML).toBe(expectedOutput); // User deletes www.google.com link from comment but keeps link text // We should not generate link - originalCommentHTML = 'Comment www.google.com'; + originalCommentMarkdown = 'Comment [www.google.com](https://www.google.com)'; afterEditCommentText = 'Comment www.google.com'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); + 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 www.google.com'; + originalCommentMarkdown = 'Comment [www.google.com](https://www.google.com)'; afterEditCommentText = 'Comment [www.google.com]'; - newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, parser.htmlToMarkdown(originalCommentHTML).trim()); + 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); expectedOutput = 'Comment www.google.com ' + 'www.facebook.com'; @@ -465,41 +462,41 @@ describe('actions/Report', () => { // Comment has two links but user deletes only one of them // Should not generate link again for the deleted one - originalCommentHTML = 'Comment www.google.com www.facebook.com'; + 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); expectedOutput = 'Comment www.google.com www.facebook.com'; 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); expectedOutput = 'https://www.facebook.com/hashtag/__main/?__eep__=6'; expect(newCommentHTML).toBe(expectedOutput); // User edits and deletes the link containing underscores // We should not generate link - originalCommentHTML = 'https://www.facebook.com/hashtag/__main/?__eep__=6'; + 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); expectedOutput = 'http://example.com/foo/*/bar/*/test.txt'; expect(newCommentHTML).toBe(expectedOutput); // User edits and deletes the link containing asterisks // We should not generate link - originalCommentHTML = 'http://example.com/foo/*/bar/*/test.txt'; + 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, parser.htmlToMarkdown(originalCommentHTML).trim()); + newCommentHTML = Report.handleUserDeletedLinksInHtml(afterEditCommentText, originalCommentMarkdown); expectedOutput = 'http://example.com/foo/*/bar/*/test.txt'; expect(newCommentHTML).toBe(expectedOutput); });