Skip to content

Commit

Permalink
Merge pull request #51721 from bernhardoj/fix/50262-reply-in-thread-s…
Browse files Browse the repository at this point in the history
…hows-for-thread-parent

Fix reply in thread shows for thread parent
  • Loading branch information
neil-marcellini authored Nov 21, 2024
2 parents c32ad42 + 39acd92 commit e04d145
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 38 deletions.
26 changes: 8 additions & 18 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1599,13 +1599,6 @@ function isWorkspaceThread(report: OnyxEntry<Report>): boolean {
return isThread(report) && isChatReport(report) && CONST.WORKSPACE_ROOM_TYPES.some((type) => chatType === type);
}

/**
* Returns true if reportAction is the first chat preview of a Thread
*/
function isThreadFirstChat(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string): boolean {
return reportAction?.childReportID?.toString() === reportID;
}

/**
* Checks if a report is a child report.
*/
Expand Down Expand Up @@ -1851,10 +1844,9 @@ function canDeleteReportAction(reportAction: OnyxInputOrEntry<ReportAction>, rep
return false;
}

const linkedReport = isThreadFirstChat(reportAction, reportID) ? getReportOrDraftReport(report?.parentReportID) : report;
if (isActionOwner) {
if (!isEmptyObject(linkedReport) && (isMoneyRequestReport(linkedReport) || isInvoiceReport(linkedReport))) {
return canDeleteTransaction(linkedReport);
if (!isEmptyObject(report) && (isMoneyRequestReport(report) || isInvoiceReport(report))) {
return canDeleteTransaction(report);
}
return true;
}
Expand Down Expand Up @@ -7270,10 +7262,9 @@ function getOriginalReportID(reportID: string, reportAction: OnyxInputOrEntry<Re
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`];
const currentReportAction = reportActions?.[reportAction?.reportActionID ?? '-1'] ?? null;
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportID, reportActions ?? ([] as ReportAction[]));
const isThreadReportParentAction = reportAction?.childReportID?.toString() === reportID;
if (Object.keys(currentReportAction ?? {}).length === 0) {
return isThreadFirstChat(reportAction, reportID)
? ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.parentReportID
: transactionThreadReportID ?? reportID;
return isThreadReportParentAction ? ReportConnection.getAllReports()?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.parentReportID : transactionThreadReportID ?? reportID;
}
return reportID;
}
Expand Down Expand Up @@ -7741,9 +7732,9 @@ function hasOnlyHeldExpenses(iouReportID: string): boolean {
/**
* Checks if thread replies should be displayed
*/
function shouldDisplayThreadReplies(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string): boolean {
function shouldDisplayThreadReplies(reportAction: OnyxInputOrEntry<ReportAction>, isThreadReportParentAction: boolean): boolean {
const hasReplies = (reportAction?.childVisibleActionCount ?? 0) > 0;
return hasReplies && !!reportAction?.childCommenterCount && !isThreadFirstChat(reportAction, reportID);
return hasReplies && !!reportAction?.childCommenterCount && !isThreadReportParentAction;
}

/**
Expand Down Expand Up @@ -7801,7 +7792,7 @@ function getNonHeldAndFullAmount(iouReport: OnyxEntry<Report>, policy: OnyxEntry
* - The action is a whisper action and it's neither a report preview nor IOU action
* - The action is the thread's first chat
*/
function shouldDisableThread(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string): boolean {
function shouldDisableThread(reportAction: OnyxInputOrEntry<ReportAction>, reportID: string, isThreadReportParentAction: boolean): boolean {
const isSplitBillAction = ReportActionsUtils.isSplitBillAction(reportAction);
const isDeletedAction = ReportActionsUtils.isDeletedAction(reportAction);
const isReportPreviewAction = ReportActionsUtils.isReportPreviewAction(reportAction);
Expand All @@ -7816,7 +7807,7 @@ function shouldDisableThread(reportAction: OnyxInputOrEntry<ReportAction>, repor
(isDeletedAction && !reportAction?.childVisibleActionCount) ||
(isArchivedReport && !reportAction?.childVisibleActionCount) ||
(isWhisperAction && !isReportPreviewAction && !isIOUAction) ||
isThreadFirstChat(reportAction, reportID)
isThreadReportParentAction
);
}

Expand Down Expand Up @@ -8672,7 +8663,6 @@ export {
isSystemChat,
isTaskReport,
isThread,
isThreadFirstChat,
isTrackExpenseReport,
isUnread,
isUnreadWithMention,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ type BaseReportActionContextMenuProps = {
/** Flag to check if the chat is unread in the LHN. Used for the Mark as Read/Unread action */
isUnreadChat?: boolean;

/**
* Is the action a thread's parent reportAction viewed from within the thread report?
* It will be false if we're viewing the same parent report action from the report it belongs to rather than the thread.
*/
isThreadReportParentAction?: boolean;

/** Content Ref */
contentRef?: RefObject<View>;

Expand All @@ -101,6 +107,7 @@ function BaseReportActionContextMenu({
isVisible = false,
isPinnedChat = false,
isUnreadChat = false,
isThreadReportParentAction = false,
selection = '',
draftMessage = '',
reportActionID,
Expand Down Expand Up @@ -194,6 +201,7 @@ function BaseReportActionContextMenu({
reportID,
isPinnedChat,
isUnreadChat,
isThreadReportParentAction,
isOffline: !!isOffline,
isMini,
isProduction,
Expand Down Expand Up @@ -284,6 +292,7 @@ function BaseReportActionContextMenu({
true,
() => {},
true,
isThreadReportParentAction,
);
};

Expand Down
19 changes: 13 additions & 6 deletions src/pages/home/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ type ShouldShow = (args: {
reportID: string;
isPinnedChat: boolean;
isUnreadChat: boolean;
isThreadReportParentAction: boolean;
isOffline: boolean;
isMini: boolean;
isProduction: boolean;
Expand Down Expand Up @@ -178,11 +179,11 @@ const ContextMenuActions: ContextMenuAction[] = [
isAnonymousAction: false,
textTranslateKey: 'reportActionContextMenu.replyInThread',
icon: Expensicons.ChatBubbleReply,
shouldShow: ({type, reportAction, reportID}) => {
shouldShow: ({type, reportAction, reportID, isThreadReportParentAction}) => {
if (type !== CONST.CONTEXT_MENU_TYPES.REPORT_ACTION) {
return false;
}
return !ReportUtils.shouldDisableThread(reportAction, reportID);
return !ReportUtils.shouldDisableThread(reportAction, reportID, isThreadReportParentAction);
},
onPress: (closePopover, {reportAction, reportID}) => {
const originalReportID = ReportUtils.getOriginalReportID(reportID, reportAction);
Expand Down Expand Up @@ -301,16 +302,22 @@ const ContextMenuActions: ContextMenuAction[] = [
isAnonymousAction: false,
textTranslateKey: 'reportActionContextMenu.joinThread',
icon: Expensicons.Bell,
shouldShow: ({reportAction, isArchivedRoom, reportID}) => {
shouldShow: ({reportAction, isArchivedRoom, isThreadReportParentAction}) => {
const childReportNotificationPreference = ReportUtils.getChildReportNotificationPreference(reportAction);
const isDeletedAction = ReportActionsUtils.isDeletedAction(reportAction);
const shouldDisplayThreadReplies = ReportUtils.shouldDisplayThreadReplies(reportAction, reportID);
const shouldDisplayThreadReplies = ReportUtils.shouldDisplayThreadReplies(reportAction, isThreadReportParentAction);
const subscribed = childReportNotificationPreference !== 'hidden';
const isThreadFirstChat = ReportUtils.isThreadFirstChat(reportAction, reportID);
const isWhisperAction = ReportActionsUtils.isWhisperAction(reportAction) || ReportActionsUtils.isActionableTrackExpense(reportAction);
const isExpenseReportAction = ReportActionsUtils.isMoneyRequestAction(reportAction) || ReportActionsUtils.isReportPreviewAction(reportAction);
const isTaskAction = ReportActionsUtils.isCreatedTaskReportAction(reportAction);
return !subscribed && !isWhisperAction && !isTaskAction && !isExpenseReportAction && !isThreadFirstChat && (shouldDisplayThreadReplies || (!isDeletedAction && !isArchivedRoom));
return (
!subscribed &&
!isWhisperAction &&
!isTaskAction &&
!isExpenseReportAction &&
!isThreadReportParentAction &&
(shouldDisplayThreadReplies || (!isDeletedAction && !isArchivedRoom))
);
},
onPress: (closePopover, {reportAction, reportID}) => {
const childReportNotificationPreference = ReportUtils.getChildReportNotificationPreference(reportAction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
const [isChronosReportEnabled, setIsChronosReportEnabled] = useState(false);
const [isChatPinned, setIsChatPinned] = useState(false);
const [hasUnreadMessages, setHasUnreadMessages] = useState(false);
const [isThreadReportParentAction, setIsThreadReportParentAction] = useState(false);
const [disabledActions, setDisabledActions] = useState<ContextMenuAction[]>([]);
const [shoudSwitchPositionIfOverflow, setShoudSwitchPositionIfOverflow] = useState(false);

Expand Down Expand Up @@ -172,6 +173,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
isThreadReportParentActionParam = false,
) => {
const {pageX = 0, pageY = 0} = extractPointerEvent(event);
contextMenuAnchorRef.current = contextMenuAnchor;
Expand Down Expand Up @@ -220,6 +222,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
setIsChronosReportEnabled(isChronosReport);
setIsChatPinned(isPinnedChat);
setHasUnreadMessages(isUnreadChat);
setIsThreadReportParentAction(isThreadReportParentActionParam);
setShoudSwitchPositionIfOverflow(isOverflowMenu);
});
};
Expand Down Expand Up @@ -347,6 +350,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
isChronosReport={isChronosReportEnabled}
isPinnedChat={isChatPinned}
isUnreadChat={hasUnreadMessages}
isThreadReportParentAction={isThreadReportParentAction}
anchor={contextMenuTargetNode}
contentRef={contentRef}
originalReportID={originalReportIDRef.current}
Expand Down
3 changes: 3 additions & 0 deletions src/pages/home/report/ContextMenu/ReportActionContextMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type ShowContextMenu = (
shouldCloseOnTarget?: boolean,
setIsEmojiPickerActive?: (state: boolean) => void,
isOverflowMenu?: boolean,
isThreadReportParentAction?: boolean,
) => void;

type ReportActionContextMenu = {
Expand Down Expand Up @@ -118,6 +119,7 @@ function showContextMenu(
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
isThreadReportParentAction = false,
) {
if (!contextMenuRef.current) {
return;
Expand All @@ -142,6 +144,7 @@ function showContextMenu(
shouldCloseOnTarget,
setIsEmojiPickerActive,
isOverflowMenu,
isThreadReportParentAction,
);
};

Expand Down
28 changes: 23 additions & 5 deletions src/pages/home/report/ReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,15 @@ type ReportActionItemProps = {
/** If this is the first visible report action */
isFirstVisibleReportAction: boolean;

/**
* Is the action a thread's parent reportAction viewed from within the thread report?
* It will be false if we're viewing the same parent report action from the report it belongs to rather than the thread.
*/
isThreadReportParentAction?: boolean;

/** IF the thread divider line will be used */
shouldUseThreadDividerLine?: boolean;

hideThreadReplies?: boolean;

/** Whether context menu should be displayed */
shouldDisplayContextMenu?: boolean;
};
Expand All @@ -154,8 +158,8 @@ function ReportActionItem({
shouldShowSubscriptAvatar = false,
onPress = undefined,
isFirstVisibleReportAction = false,
isThreadReportParentAction = false,
shouldUseThreadDividerLine = false,
hideThreadReplies = false,
shouldDisplayContextMenu = true,
parentReportActionForTransactionThread,
}: ReportActionItemProps) {
Expand Down Expand Up @@ -356,9 +360,22 @@ function ReportActionItem({
disabledActions,
false,
setIsEmojiPickerActive as () => void,
undefined,
isThreadReportParentAction,
);
},
[draftMessage, action, reportID, toggleContextMenuFromActiveReportAction, originalReportID, shouldDisplayContextMenu, disabledActions, isArchivedRoom, isChronosReport],
[
draftMessage,
action,
reportID,
toggleContextMenuFromActiveReportAction,
originalReportID,
shouldDisplayContextMenu,
disabledActions,
isArchivedRoom,
isChronosReport,
isThreadReportParentAction,
],
);

// Handles manual scrolling to the bottom of the chat when the last message is an actionable whisper and it's resolved.
Expand Down Expand Up @@ -784,7 +801,7 @@ function ReportActionItem({
}
const numberOfThreadReplies = action.childVisibleActionCount ?? 0;

const shouldDisplayThreadReplies = !hideThreadReplies && ReportUtils.shouldDisplayThreadReplies(action, reportID);
const shouldDisplayThreadReplies = ReportUtils.shouldDisplayThreadReplies(action, isThreadReportParentAction);
const oldestFourAccountIDs =
action.childOldestFourAccountIDs
?.split(',')
Expand Down Expand Up @@ -970,6 +987,7 @@ function ReportActionItem({
displayAsGroup={displayAsGroup}
disabledActions={disabledActions}
isVisible={hovered && draftMessage === undefined && !hasErrors}
isThreadReportParentAction={isThreadReportParentAction}
draftMessage={draftMessage}
isChronosReport={isChronosReport}
checkIfContextMenuActive={toggleContextMenuFromActiveReportAction}
Expand Down
2 changes: 1 addition & 1 deletion src/pages/home/report/ReportActionItemParentAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ function ReportActionItemParentAction({
index={index}
isFirstVisibleReportAction={isFirstVisibleReportAction}
shouldUseThreadDividerLine={shouldUseThreadDividerLine}
hideThreadReplies
isThreadReportParentAction
/>
)}
</OfflineWithFeedback>
Expand Down
16 changes: 8 additions & 8 deletions tests/unit/ReportUtilsTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ describe('ReportUtils', () => {

it('should disable on thread-disabled actions', () => {
const reportAction = ReportUtils.buildOptimisticCreatedReportAction('[email protected]');
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeTruthy();
});

it('should disable thread on split expense actions', () => {
Expand All @@ -805,7 +805,7 @@ describe('ReportUtils', () => {
[{login: '[email protected]'}, {login: '[email protected]'}],
NumberUtils.rand64(),
) as ReportAction;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeTruthy();
});

it('should disable on deleted and not-thread actions', () => {
Expand All @@ -821,10 +821,10 @@ describe('ReportUtils', () => {
],
childVisibleActionCount: 1,
} as ReportAction;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeFalsy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeFalsy();

reportAction.childVisibleActionCount = 0;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeTruthy();
});

it('should disable on archived reports and not-thread actions', () => {
Expand All @@ -837,10 +837,10 @@ describe('ReportUtils', () => {
const reportAction = {
childVisibleActionCount: 1,
} as ReportAction;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeFalsy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeFalsy();

reportAction.childVisibleActionCount = 0;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeTruthy();
});
});

Expand All @@ -851,14 +851,14 @@ describe('ReportUtils', () => {
whisperedTo: [123456],
},
} as ReportAction;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, false)).toBeTruthy();
});

it('should disable on thread first chat', () => {
const reportAction = {
childReportID: reportID,
} as ReportAction;
expect(ReportUtils.shouldDisableThread(reportAction, reportID)).toBeTruthy();
expect(ReportUtils.shouldDisableThread(reportAction, reportID, true)).toBeTruthy();
});
});

Expand Down

0 comments on commit e04d145

Please sign in to comment.