Skip to content

Commit

Permalink
Merge pull request #24319 from samh-nl/fix/issue-21817
Browse files Browse the repository at this point in the history
fix: ensure processing of markup in comment
  • Loading branch information
neil-marcellini authored Sep 21, 2023
2 parents 68d03fa + a32f729 commit c57c625
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 8 deletions.
4 changes: 2 additions & 2 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,9 @@ const CONST = {
},

// Auth limit is 60k for the column but we store edits and other metadata along the html so let's use a lower limit to accommodate for it.
MAX_COMMENT_LENGTH: 15000,
MAX_COMMENT_LENGTH: 10000,

// Furthermore, applying markup is very resource-consuming, so let's set a slightly lower limit for that
// Use the same value as MAX_COMMENT_LENGTH to ensure the entire comment is parsed. Note that applying markup is very resource-consuming.
MAX_MARKUP_LENGTH: 10000,

MAX_THREAD_REPLIES_PREVIEW: 99,
Expand Down
11 changes: 10 additions & 1 deletion src/components/ExceededCommentLength.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {debounce} from 'lodash';
import {withOnyx} from 'react-native-onyx';
import CONST from '../CONST';
import * as ReportUtils from '../libs/ReportUtils';
import useLocalize from '../hooks/useLocalize';
import Text from './Text';
import styles from '../styles/styles';
import ONYXKEYS from '../ONYXKEYS';
Expand All @@ -25,6 +26,7 @@ const defaultProps = {
};

function ExceededCommentLength(props) {
const {numberFormat, translate} = useLocalize();
const [commentLength, setCommentLength] = useState(0);
const updateCommentLength = useMemo(
() =>
Expand All @@ -44,7 +46,14 @@ function ExceededCommentLength(props) {
return null;
}

return <Text style={[styles.textMicro, styles.textDanger, styles.chatItemComposeSecondaryRow, styles.mlAuto, styles.pl2]}>{`${commentLength}/${CONST.MAX_COMMENT_LENGTH}`}</Text>;
return (
<Text
style={[styles.textMicro, styles.textDanger, styles.chatItemComposeSecondaryRow, styles.mlAuto, styles.pl2]}
numberOfLines={1}
>
{translate('composer.commentExceededMaxLength', {formattedMaxLength: numberFormat(CONST.MAX_COMMENT_LENGTH)})}
</Text>
);
}

ExceededCommentLength.propTypes = propTypes;
Expand Down
2 changes: 2 additions & 0 deletions src/languages/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import type {
SetTheRequestParams,
UpdatedTheRequestParams,
RemovedTheRequestParams,
FormattedMaxLengthParams,
RequestedAmountMessageParams,
TagSelectionParams,
TranslationBase,
Expand Down Expand Up @@ -282,6 +283,7 @@ export default {
composer: {
noExtensionFoundForMimeType: 'No extension found for mime type',
problemGettingImageYouPasted: 'There was a problem getting the image you pasted',
commentExceededMaxLength: ({formattedMaxLength}: FormattedMaxLengthParams) => `The maximum comment length is ${formattedMaxLength} characters.`,
},
baseUpdateAppModal: {
updateApp: 'Update app',
Expand Down
2 changes: 2 additions & 0 deletions src/languages/es.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import type {
SetTheRequestParams,
UpdatedTheRequestParams,
RemovedTheRequestParams,
FormattedMaxLengthParams,
RequestedAmountMessageParams,
TagSelectionParams,
EnglishTranslation,
Expand Down Expand Up @@ -272,6 +273,7 @@ export default {
composer: {
noExtensionFoundForMimeType: 'No se encontró una extension para este tipo de contenido',
problemGettingImageYouPasted: 'Ha ocurrido un problema al obtener la imagen que has pegado',
commentExceededMaxLength: ({formattedMaxLength}: FormattedMaxLengthParams) => `El comentario debe tener máximo ${formattedMaxLength} caracteres.`,
},
baseUpdateAppModal: {
updateApp: 'Actualizar app',
Expand Down
3 changes: 3 additions & 0 deletions src/languages/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ type RemovedTheRequestParams = {valueName: string; oldValueToDisplay: string};

type UpdatedTheRequestParams = {valueName: string; newValueToDisplay: string; oldValueToDisplay: string};

type FormattedMaxLengthParams = {formattedMaxLength: string};

type TagSelectionParams = {tagName: string};

/* Translation Object types */
Expand Down Expand Up @@ -303,5 +305,6 @@ export type {
SetTheRequestParams,
UpdatedTheRequestParams,
RemovedTheRequestParams,
FormattedMaxLengthParams,
TagSelectionParams,
};
4 changes: 2 additions & 2 deletions src/libs/ReportUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -1817,15 +1817,15 @@ function hasReportNameError(report) {
}

/**
* For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
* 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, but still escape the text, and display plaintext for performance reasons. It takes over 40s to parse a 100k long string!!
*
* @param {String} text
* @returns {String}
*/
function getParsedComment(text) {
const parser = new ExpensiMark();
return text.length < CONST.MAX_MARKUP_LENGTH ? parser.replace(text) : _.escape(text);
return text.length <= CONST.MAX_MARKUP_LENGTH ? parser.replace(text) : _.escape(text);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions src/libs/actions/Report.js
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ const removeLinksFromHtml = (html, links) => {
*/
const handleUserDeletedLinksInHtml = (newCommentText, originalHtml) => {
const parser = new ExpensiMark();
if (newCommentText.length >= CONST.MAX_MARKUP_LENGTH) {
if (newCommentText.length > CONST.MAX_MARKUP_LENGTH) {
return newCommentText;
}
const markdownOriginalComment = parser.htmlToMarkdown(originalHtml).trim();
Expand All @@ -1093,10 +1093,10 @@ function editReportComment(reportID, originalReportAction, textForNewComment) {
const htmlForNewComment = handleUserDeletedLinksInHtml(textForNewComment, originalCommentHTML);
const reportComment = parser.htmlToText(htmlForNewComment);

// For comments shorter than 10k chars, convert the comment from MD into HTML because that's how it is stored in the database
// 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) {
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 Down

0 comments on commit c57c625

Please sign in to comment.