Skip to content

Commit

Permalink
Merge pull request #40685 from software-mansion-labs/war-in/stop-send…
Browse files Browse the repository at this point in the history
…ing-mention-report-tag

Stop sending <mention-report> when not in a policy room
  • Loading branch information
rlinoz authored Apr 30, 2024
2 parents b62bbbb + e332742 commit 095a5c2
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 33 deletions.
6 changes: 3 additions & 3 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 @@ -108,7 +108,7 @@
"date-fns-tz": "^2.0.0",
"dom-serializer": "^0.2.2",
"domhandler": "^4.3.0",
"expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#c0f7f3b6558fbeda0527c80d68460d418afef219",
"expensify-common": "git+ssh://git@github.com/Expensify/expensify-common.git#9a68635cdcef4c81593c0f816a007bc9c707d46a",
"expo": "^50.0.3",
"expo-av": "~13.10.4",
"expo-image": "1.11.0",
Expand Down
2 changes: 1 addition & 1 deletion src/components/ReportActionItem/MoneyRequestView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ function MoneyRequestView({
const isApprover = ReportUtils.isMoneyRequestReport(moneyRequestReport) && moneyRequestReport?.managerID !== null && session?.accountID === moneyRequestReport?.managerID;
// A flag for verifying that the current report is a sub-report of a workspace chat
// if the policy of the report is either Collect or Control, then this report must be tied to workspace chat
const isPolicyExpenseChat = ReportUtils.isGroupPolicy(report);
const isPolicyExpenseChat = ReportUtils.isReportInGroupPolicy(report);

const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTagList), [policyTagList]);

Expand Down
5 changes: 3 additions & 2 deletions src/hooks/useHandleExceedMaxCommentLength.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
import _ from 'lodash';
import {useCallback, useMemo, useState} from 'react';
import * as ReportUtils from '@libs/ReportUtils';
import type {ParsingDetails} from '@libs/ReportUtils';
import CONST from '@src/CONST';

const useHandleExceedMaxCommentLength = () => {
const [hasExceededMaxCommentLength, setHasExceededMaxCommentLength] = useState(false);

const handleValueChange = useCallback(
(value: string) => {
if (ReportUtils.getCommentLength(value) <= CONST.MAX_COMMENT_LENGTH) {
(value: string, parsingDetails?: ParsingDetails) => {
if (ReportUtils.getCommentLength(value, parsingDetails) <= CONST.MAX_COMMENT_LENGTH) {
if (hasExceededMaxCommentLength) {
setHasExceededMaxCommentLength(false);
}
Expand Down
54 changes: 41 additions & 13 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,11 @@ type OutstandingChildRequest = {
hasOutstandingChildRequest?: boolean;
};

type ParsingDetails = {
shouldEscapeText?: boolean;
reportID?: string;
};

let currentUserEmail: string | undefined;
let currentUserPrivateDomain: string | undefined;
let currentUserAccountID: number | undefined;
Expand Down Expand Up @@ -881,12 +886,19 @@ function isControlPolicyExpenseChat(report: OnyxEntry<Report>): boolean {
return isPolicyExpenseChat(report) && getPolicyType(report, allPolicies) === CONST.POLICY.TYPE.CORPORATE;
}

/**
* Whether the provided policyType is a Free, Collect or Control policy type
*/
function isGroupPolicy(policyType: string): boolean {
return policyType === CONST.POLICY.TYPE.CORPORATE || policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.FREE;
}

/**
* Whether the provided report belongs to a Free, Collect or Control policy
*/
function isGroupPolicy(report: OnyxEntry<Report>): boolean {
function isReportInGroupPolicy(report: OnyxEntry<Report>): boolean {
const policyType = getPolicyType(report, allPolicies);
return policyType === CONST.POLICY.TYPE.CORPORATE || policyType === CONST.POLICY.TYPE.TEAM || policyType === CONST.POLICY.TYPE.FREE;
return isGroupPolicy(policyType);
}

/**
Expand Down Expand Up @@ -1454,7 +1466,7 @@ function canAddOrDeleteTransactions(moneyRequestReport: OnyxEntry<Report>): bool
return false;
}

if (isGroupPolicy(moneyRequestReport) && isProcessingReport(moneyRequestReport) && !PolicyUtils.isInstantSubmitEnabled(getPolicy(moneyRequestReport?.policyID))) {
if (isReportInGroupPolicy(moneyRequestReport) && isProcessingReport(moneyRequestReport) && !PolicyUtils.isInstantSubmitEnabled(getPolicy(moneyRequestReport?.policyID))) {
return false;
}

Expand Down Expand Up @@ -3288,15 +3300,23 @@ function addDomainToShortMention(mention: string): string | undefined {
* 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!!
*/
function getParsedComment(text: string, shouldEscapeText?: boolean): string {
function getParsedComment(text: string, parsingDetails?: ParsingDetails): string {
let isGroupPolicyReport = false;
if (parsingDetails?.reportID) {
const currentReport = getReport(parsingDetails?.reportID);
isGroupPolicyReport = isReportInGroupPolicy(currentReport);
}

const parser = new ExpensiMark();
const textWithMention = text.replace(CONST.REGEX.SHORT_MENTION, (match) => {
const mention = match.substring(1);
const mentionWithDomain = addDomainToShortMention(mention);
return mentionWithDomain ? `@${mentionWithDomain}` : match;
});

return text.length <= CONST.MAX_MARKUP_LENGTH ? parser.replace(textWithMention, {shouldEscapeText}) : lodashEscape(text);
return text.length <= CONST.MAX_MARKUP_LENGTH
? parser.replace(textWithMention, {shouldEscapeText: parsingDetails?.shouldEscapeText, disabledRules: isGroupPolicyReport ? [] : ['reportMentions']})
: lodashEscape(text);
}

function getReportDescriptionText(report: Report): string {
Expand Down Expand Up @@ -3355,9 +3375,16 @@ function buildOptimisticInviteReportAction(invitedUserDisplayName: string, invit
};
}

function buildOptimisticAddCommentReportAction(text?: string, file?: FileObject, actorAccountID?: number, createdOffset = 0, shouldEscapeText?: boolean): OptimisticReportAction {
function buildOptimisticAddCommentReportAction(
text?: string,
file?: FileObject,
actorAccountID?: number,
createdOffset = 0,
shouldEscapeText?: boolean,
reportID?: string,
): OptimisticReportAction {
const parser = new ExpensiMark();
const commentText = getParsedComment(text ?? '', shouldEscapeText);
const commentText = getParsedComment(text ?? '', {shouldEscapeText, reportID});
const isAttachmentOnly = file && !text;
const isTextOnly = text && !file;

Expand Down Expand Up @@ -3479,7 +3506,7 @@ function buildOptimisticTaskCommentReportAction(
childOldestFourAccountIDs?: string;
},
): OptimisticReportAction {
const reportAction = buildOptimisticAddCommentReportAction(text, undefined, undefined, createdOffset);
const reportAction = buildOptimisticAddCommentReportAction(text, undefined, undefined, createdOffset, undefined, taskReportID);
if (reportAction.reportAction.message?.[0]) {
reportAction.reportAction.message[0].taskReportID = taskReportID;
}
Expand Down Expand Up @@ -5209,8 +5236,8 @@ function getNewMarkerReportActionID(report: OnyxEntry<Report>, sortedAndFiltered
* Used for compatibility with the backend auth validator for AddComment, and to account for MD in comments
* @returns The comment's total length as seen from the backend
*/
function getCommentLength(textComment: string): number {
return getParsedComment(textComment)
function getCommentLength(textComment: string, parsingDetails?: ParsingDetails): number {
return getParsedComment(textComment, parsingDetails)
.replace(/[^ -~]/g, '\\u????')
.trim().length;
}
Expand Down Expand Up @@ -5340,7 +5367,7 @@ function canRequestMoney(report: OnyxEntry<Report>, policy: OnyxEntry<Policy>, o
// which is tied to their workspace chat.
if (isMoneyRequestReport(report)) {
const canAddTransactions = canAddOrDeleteTransactions(report);
return isGroupPolicy(report) ? isOwnPolicyExpenseChat && canAddTransactions : canAddTransactions;
return isReportInGroupPolicy(report) ? isOwnPolicyExpenseChat && canAddTransactions : canAddTransactions;
}

// In the case of policy expense chat, users can only submit expenses from their own policy expense chat
Expand Down Expand Up @@ -6241,7 +6268,7 @@ function canBeAutoReimbursed(report: OnyxEntry<Report>, policy: OnyxEntry<Policy
const reimbursableTotal = getMoneyRequestSpendBreakdown(report).totalDisplaySpend;
const autoReimbursementLimit = policy.autoReimbursementLimit ?? 0;
const isAutoReimbursable =
isGroupPolicy(report) &&
isReportInGroupPolicy(report) &&
policy.reimbursementChoice === CONST.POLICY.REIMBURSEMENT_CHOICES.REIMBURSEMENT_YES &&
autoReimbursementLimit >= reimbursableTotal &&
reimbursableTotal > 0 &&
Expand Down Expand Up @@ -6601,7 +6628,7 @@ export {
isExpensifyOnlyParticipantInReport,
isGroupChat,
isGroupChatAdmin,
isGroupPolicy,
isReportInGroupPolicy,
isHoldCreator,
isIOUOwnedByCurrentUser,
isIOUReport,
Expand Down Expand Up @@ -6688,4 +6715,5 @@ export type {
OptionData,
TransactionDetails,
OptimisticInviteReportAction,
ParsingDetails,
};
8 changes: 4 additions & 4 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ function addActions(reportID: string, text = '', file?: FileObject) {
let commandName: typeof WRITE_COMMANDS.ADD_COMMENT | typeof WRITE_COMMANDS.ADD_ATTACHMENT | typeof WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT = WRITE_COMMANDS.ADD_COMMENT;

if (text && !file) {
const reportComment = ReportUtils.buildOptimisticAddCommentReportAction(text);
const reportComment = ReportUtils.buildOptimisticAddCommentReportAction(text, undefined, undefined, undefined, undefined, reportID);
reportCommentAction = reportComment.reportAction;
reportCommentText = reportComment.commentText;
}
Expand All @@ -459,13 +459,13 @@ function addActions(reportID: string, text = '', file?: FileObject) {
// When we are adding an attachment we will call AddAttachment.
// It supports sending an attachment with an optional comment and AddComment supports adding a single text comment only.
commandName = WRITE_COMMANDS.ADD_ATTACHMENT;
const attachment = ReportUtils.buildOptimisticAddCommentReportAction(text, file);
const attachment = ReportUtils.buildOptimisticAddCommentReportAction(text, file, undefined, undefined, undefined, reportID);
attachmentAction = attachment.reportAction;
}

if (text && file) {
// When there is both text and a file, the text for the report comment needs to be parsed)
reportCommentText = ReportUtils.getParsedComment(text ?? '');
reportCommentText = ReportUtils.getParsedComment(text ?? '', {reportID});

// And the API command needs to go to the new API which supports combining both text and attachments in a single report action
commandName = WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT;
Expand Down Expand Up @@ -1844,7 +1844,7 @@ function updateDescription(reportID: string, previousValue: string, newValue: st
return;
}

const parsedDescription = ReportUtils.getParsedComment(newValue);
const parsedDescription = ReportUtils.getParsedComment(newValue, {reportID});

const optimisticData: OnyxUpdate[] = [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ function ComposerWithSuggestions(

const prepareCommentAndResetComposer = useCallback((): string => {
const trimmedComment = commentRef.current.trim();
const commentLength = ReportUtils.getCommentLength(trimmedComment);
const commentLength = ReportUtils.getCommentLength(trimmedComment, {reportID});

// Don't submit empty comments or comments that exceed the character limit
if (!commentLength || commentLength > CONST.MAX_COMMENT_LENGTH) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ function ReportActionCompose({
if (value.length === 0 && isComposerFullSize) {
Report.setIsComposerFullSize(reportID, false);
}
validateCommentMaxLength(value);
validateCommentMaxLength(value, {reportID});
}}
/>
<ReportDropUI
Expand Down
6 changes: 3 additions & 3 deletions src/pages/home/report/ReportActionItemMessageEdit.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,7 @@ function ReportActionItemMessageEdit(
*/
const publishDraft = useCallback(() => {
// Do nothing if draft exceed the character limit
if (ReportUtils.getCommentLength(draft) > CONST.MAX_COMMENT_LENGTH) {
if (ReportUtils.getCommentLength(draft, {reportID}) > CONST.MAX_COMMENT_LENGTH) {
return;
}

Expand Down Expand Up @@ -380,8 +380,8 @@ function ReportActionItemMessageEdit(
const focus = focusComposerWithDelay(textInputRef.current);

useEffect(() => {
validateCommentMaxLength(draft);
}, [draft, validateCommentMaxLength]);
validateCommentMaxLength(draft, {reportID});
}, [draft, reportID, validateCommentMaxLength]);

return (
<>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/HoldReasonPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function HoldReasonPage({route}: HoldReasonPageProps) {

// We first check if the report is part of a policy - if not, then it's a personal request (1:1 request)
// For personal requests, we need to allow both users to put the request on hold
const isWorkspaceRequest = ReportUtils.isGroupPolicy(report);
const isWorkspaceRequest = ReportUtils.isReportInGroupPolicy(report);
const parentReportAction = ReportActionsUtils.getReportAction(report?.parentReportID ?? '', report?.parentReportActionID ?? '');

const navigateBack = () => {
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepCategory.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function IOURequestStepCategory({

// The transactionCategory can be an empty string, so to maintain the logic we'd like to keep it in this shape until utils refactor
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const shouldShowCategory = ReportUtils.isGroupPolicy(report) && (!!transactionCategory || OptionsListUtils.hasEnabledOptions(Object.values(policyCategories ?? {})));
const shouldShowCategory = ReportUtils.isReportInGroupPolicy(report) && (!!transactionCategory || OptionsListUtils.hasEnabledOptions(Object.values(policyCategories ?? {})));

const isSplitBill = iouType === CONST.IOU.TYPE.SPLIT;
const canEditSplitBill = isSplitBill && reportAction && session?.accountID === reportAction.actorAccountID && TransactionUtils.areRequiredFieldsEmpty(transaction);
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepMerchant.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ function IOURequestStepMerchant({
const merchant = ReportUtils.getTransactionDetails(isEditingSplitBill && !isEmptyObject(splitDraftTransaction) ? splitDraftTransaction : transaction)?.merchant;
const isEmptyMerchant = merchant === '' || merchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT;

const isMerchantRequired = ReportUtils.isGroupPolicy(report) || transaction?.participants?.some((participant) => Boolean(participant.isPolicyExpenseChat));
const isMerchantRequired = ReportUtils.isReportInGroupPolicy(report) || transaction?.participants?.some((participant) => Boolean(participant.isPolicyExpenseChat));
const navigateBack = () => {
Navigation.goBack(backTo);
};
Expand Down
2 changes: 1 addition & 1 deletion src/pages/iou/request/step/IOURequestStepTag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ function IOURequestStepTag({
const canEditSplitBill = isSplitBill && reportAction && session?.accountID === reportAction.actorAccountID && TransactionUtils.areRequiredFieldsEmpty(transaction);
const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]);

const shouldShowTag = ReportUtils.isGroupPolicy(report) && (transactionTag || OptionsListUtils.hasEnabledTags(policyTagLists));
const shouldShowTag = ReportUtils.isReportInGroupPolicy(report) && (transactionTag || OptionsListUtils.hasEnabledTags(policyTagLists));

// eslint-disable-next-line rulesdir/no-negated-variables
const shouldShowNotFoundPage = !shouldShowTag || (isEditing && (isSplitBill ? !canEditSplitBill : reportAction && !canEditMoneyRequest(reportAction)));
Expand Down

0 comments on commit 095a5c2

Please sign in to comment.