Skip to content

Commit

Permalink
Merge pull request Expensify#40676 from nkdengineer/fix/39407
Browse files Browse the repository at this point in the history
Fix unread issue in combine report
  • Loading branch information
amyevans authored Apr 24, 2024
2 parents 7ddb044 + c07e107 commit 92acb42
Show file tree
Hide file tree
Showing 13 changed files with 61 additions and 5 deletions.
3 changes: 3 additions & 0 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ function LHNOptionsList({
const hasDraftComment = DraftCommentUtils.isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]);
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);
const lastReportAction = sortedReportActions[0];
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(reportID, itemReportActions);
const transactionThreadReport = reports?.[`${ONYXKEYS.COLLECTION.REPORT}${transactionThreadReportID}`] ?? null;

// Get the transaction for the last report action
let lastReportActionTransactionID = '';
Expand All @@ -129,6 +131,7 @@ function LHNOptionsList({
<OptionRowLHNData
reportID={reportID}
fullReport={itemFullReport}
transactionThreadReport={transactionThreadReport}
reportActions={itemReportActions}
parentReportAction={itemParentReportAction}
policy={itemPolicy}
Expand Down
5 changes: 5 additions & 0 deletions src/components/LHNOptionsList/OptionRowLHN.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ function OptionRowLHN({reportID, isFocused = false, onSelectRow = () => {}, opti
false,
optionItem.isPinned,
!!optionItem.isUnread,
[],
false,
() => {},
false,
optionItem.transactionThreadReportID,
);
};

Expand Down
3 changes: 3 additions & 0 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ function OptionRowLHNData({
lastReportActionTransaction = {},
transactionViolations,
canUseViolations,
transactionThreadReport,
...propsToForward
}: OptionRowLHNDataProps) {
const reportID = propsToForward.reportID;
Expand All @@ -47,6 +48,7 @@ function OptionRowLHNData({
policy,
parentReportAction,
hasViolations: !!shouldDisplayViolations,
transactionThreadReport,
});
if (deepEqual(item, optionItemRef.current)) {
return optionItemRef.current;
Expand All @@ -70,6 +72,7 @@ function OptionRowLHNData({
transactionViolations,
canUseViolations,
receiptTransactions,
transactionThreadReport,
]);

return (
Expand Down
3 changes: 3 additions & 0 deletions src/components/LHNOptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ type OptionRowLHNDataProps = {
/** The full data of the report */
fullReport: OnyxEntry<Report>;

/** The transaction thread report associated with the current report – applicable only for one-transaction money reports */
transactionThreadReport: OnyxEntry<Report>;

/** The policy which the user has access to and which the report could be tied to */
policy?: OnyxEntry<Policy>;

Expand Down
1 change: 1 addition & 0 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ type OptionData = {
reportID?: string;
enabled?: boolean;
data?: Partial<TaxRate>;
transactionThreadReportID?: string | null;
} & Report;

type OnyxDataTaskAssigneeChat = {
Expand Down
5 changes: 4 additions & 1 deletion src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ function getOptionData({
policy,
parentReportAction,
hasViolations,
transactionThreadReport,
}: {
report: OnyxEntry<Report>;
reportActions: OnyxEntry<ReportActions>;
Expand All @@ -197,6 +198,7 @@ function getOptionData({
policy: OnyxEntry<Policy> | undefined;
parentReportAction: OnyxEntry<ReportAction> | undefined;
hasViolations: boolean;
transactionThreadReport: OnyxEntry<Report>;
}): ReportUtils.OptionData | undefined {
// When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for
// this method to be called after the Onyx data has been cleared out. In that case, it's fine to do
Expand Down Expand Up @@ -233,6 +235,7 @@ function getOptionData({
isWaitingOnBankAccount: false,
isAllowedToComment: true,
isDeletedParentAction: false,
transactionThreadReportID: transactionThreadReport?.reportID,
};

let participantAccountIDs = report.participantAccountIDs ?? [];
Expand Down Expand Up @@ -265,7 +268,7 @@ function getOptionData({
result.statusNum = report.statusNum;
// When the only message of a report is deleted lastVisibileActionCreated is not reset leading to wrongly
// setting it Unread so we add additional condition here to avoid empty chat LHN from being bold.
result.isUnread = ReportUtils.isUnread(report) && !!report.lastActorAccountID;
result.isUnread = (ReportUtils.isUnread(report) && !!report.lastActorAccountID) || (ReportUtils.isUnread(transactionThreadReport) && !!transactionThreadReport?.lastActorAccountID);
result.isUnreadWithMention = ReportUtils.isUnreadWithMention(report);
result.isPinned = report.isPinned;
result.iouReportID = report.iouReportID;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ type BaseReportActionContextMenuProps = BaseReportActionContextMenuOnyxProps & {
// eslint-disable-next-line react/no-unused-prop-types
originalReportID: string;

/** The ID of transaction thread report associated with the current report, if any */
transactionThreadReportID: string;

/**
* If true, this component will be a small, row-oriented menu that displays icons but not text.
* If false, this component will be a larger, column-oriented menu that displays icons alongside text in each row.
Expand Down Expand Up @@ -117,6 +120,7 @@ function BaseReportActionContextMenu({
checkIfContextMenuActive,
disabledActions = [],
setIsEmojiPickerActive,
transactionThreadReportID,
}: BaseReportActionContextMenuProps) {
const StyleUtils = useStyleUtils();
const {translate} = useLocalize();
Expand Down Expand Up @@ -245,6 +249,7 @@ function BaseReportActionContextMenu({
interceptAnonymousUser,
openOverflowMenu,
setIsEmojiPickerActive,
transactionThreadReportID,
};

if ('renderContent' in contextAction) {
Expand Down
6 changes: 5 additions & 1 deletion src/pages/home/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ type ContextMenuActionPayload = {
event?: GestureResponderEvent | MouseEvent | KeyboardEvent;
setIsEmojiPickerActive?: (state: boolean) => void;
anchorRef?: MutableRefObject<View | null>;
transactionThreadReportID?: string;
};

type OnPress = (closePopover: boolean, payload: ContextMenuActionPayload, selection?: string, reportID?: string, draftMessage?: string) => void;
Expand Down Expand Up @@ -213,8 +214,11 @@ const ContextMenuActions: ContextMenuAction[] = [
successIcon: Expensicons.Checkmark,
shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport, reportID, isPinnedChat, isUnreadChat) =>
type === CONST.CONTEXT_MENU_TYPES.REPORT && isUnreadChat,
onPress: (closePopover, {reportID}) => {
onPress: (closePopover, {reportID, transactionThreadReportID}) => {
Report.readNewestAction(reportID);
if (transactionThreadReportID && transactionThreadReportID !== '0') {
Report.readNewestAction(transactionThreadReportID);
}
if (closePopover) {
hideContextMenu(true, ReportActionComposeFocusManager.focus);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
const reportActionRef = useRef<OnyxEntry<ReportAction>>(null);
const reportActionIDRef = useRef('0');
const originalReportIDRef = useRef('0');
const transactionThreadReportIDRef = useRef('0');
const selectionRef = useRef('');
const reportActionDraftMessageRef = useRef<string>();

Expand Down Expand Up @@ -171,6 +172,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
transactionThreadReportID = undefined,
) => {
const {pageX = 0, pageY = 0} = extractPointerEvent(event);
contextMenuAnchorRef.current = contextMenuAnchor;
Expand Down Expand Up @@ -212,6 +214,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
reportIDRef.current = reportID ?? '0';
reportActionIDRef.current = reportActionID ?? '0';
originalReportIDRef.current = originalReportID ?? '0';
transactionThreadReportIDRef.current = transactionThreadReportID ?? '0';
selectionRef.current = selection;
setIsPopoverVisible(true);
reportActionDraftMessageRef.current = draftMessage;
Expand Down Expand Up @@ -337,6 +340,7 @@ function PopoverReportActionContextMenu(_props: unknown, ref: ForwardedRef<Repor
type={typeRef.current}
reportID={reportIDRef.current}
reportActionID={reportActionIDRef.current}
transactionThreadReportID={transactionThreadReportIDRef.current}
draftMessage={reportActionDraftMessageRef.current}
selection={selectionRef.current}
isArchivedRoom={isRoomArchived}
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,
transactionThreadReportID?: string,
) => void;

type ReportActionContextMenu = {
Expand Down Expand Up @@ -119,6 +120,7 @@ function showContextMenu(
shouldCloseOnTarget = false,
setIsEmojiPickerActive = () => {},
isOverflowMenu = false,
transactionThreadReportID = '0',
) {
if (!contextMenuRef.current) {
return;
Expand Down Expand Up @@ -149,6 +151,7 @@ function showContextMenu(
shouldCloseOnTarget,
setIsEmojiPickerActive,
isOverflowMenu,
transactionThreadReportID,
);
}

Expand Down
1 change: 1 addition & 0 deletions src/pages/home/report/ReportActionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -919,6 +919,7 @@ function ReportActionItem({
isChronosReport={ReportUtils.chatIncludesChronos(originalReport)}
checkIfContextMenuActive={toggleContextMenuFromActiveReportAction}
setIsEmojiPickerActive={setIsEmojiPickerActive}
transactionThreadReportID={transactionThreadReport?.reportID ?? '0'}
/>
<View style={StyleUtils.getReportActionItemStyle(hovered || isWhisper || isContextMenuActive || !!isEmojiPickerActive || draftMessage !== undefined, !!onPress)}>
<OfflineWithFeedback
Expand Down
26 changes: 23 additions & 3 deletions src/pages/home/report/ReportActionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {RouteProp} from '@react-navigation/native';
import type {DebouncedFunc} from 'lodash';
import React, {memo, useCallback, useEffect, useMemo, useRef, useState} from 'react';
import {DeviceEventEmitter, InteractionManager} from 'react-native';
import type {LayoutChangeEvent, NativeScrollEvent, NativeSyntheticEvent, StyleProp, ViewStyle} from 'react-native';
import type {EmitterSubscription, LayoutChangeEvent, NativeScrollEvent, NativeSyntheticEvent, StyleProp, ViewStyle} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import Animated, {useAnimatedStyle, useSharedValue, withTiming} from 'react-native-reanimated';
import InvertedFlatList from '@components/InvertedFlatList';
Expand Down Expand Up @@ -200,7 +200,8 @@ function ReportActionsList({
);
const lastActionIndex = sortedVisibleReportActions[0]?.reportActionID;
const reportActionSize = useRef(sortedVisibleReportActions.length);
const hasNewestReportAction = sortedReportActions?.[0].created === report.lastVisibleActionCreated;
const hasNewestReportAction =
sortedReportActions?.[0].created === report.lastVisibleActionCreated || sortedReportActions?.[0].created === transactionThreadReport?.lastVisibleActionCreated;
const hasNewestReportActionRef = useRef(hasNewestReportAction);
hasNewestReportActionRef.current = hasNewestReportAction;
const previousLastIndex = useRef(lastActionIndex);
Expand Down Expand Up @@ -306,12 +307,28 @@ function ReportActionsList({
setMessageManuallyMarkedUnread(new Date().getTime());
});

let unreadActionSubscriptionForTransactionThread: EmitterSubscription | undefined;
let readNewestActionSubscriptionForTransactionThread: EmitterSubscription | undefined;
if (transactionThreadReport?.reportID) {
unreadActionSubscriptionForTransactionThread = DeviceEventEmitter.addListener(`unreadAction_${transactionThreadReport?.reportID}`, (newLastReadTime) => {
resetUnreadMarker(newLastReadTime);
setMessageManuallyMarkedUnread(new Date().getTime());
});

readNewestActionSubscriptionForTransactionThread = DeviceEventEmitter.addListener(`readNewestAction_${transactionThreadReport?.reportID}`, (newLastReadTime) => {
resetUnreadMarker(newLastReadTime);
setMessageManuallyMarkedUnread(0);
});
}

return () => {
unreadActionSubscription.remove();
readNewestActionSubscription.remove();
deletedReportActionSubscription.remove();
unreadActionSubscriptionForTransactionThread?.remove();
readNewestActionSubscriptionForTransactionThread?.remove();
};
}, [report.reportID]);
}, [report.reportID, transactionThreadReport?.reportID]);

useEffect(() => {
if (linkedReportActionID) {
Expand Down Expand Up @@ -399,6 +416,9 @@ function ReportActionsList({
reportScrollManager.scrollToBottom();
readActionSkipped.current = false;
Report.readNewestAction(report.reportID);
if (transactionThreadReport?.reportID) {
Report.readNewestAction(transactionThreadReport?.reportID);
}
};

/**
Expand Down
1 change: 1 addition & 0 deletions tests/perf-test/SidebarUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ describe('SidebarUtils', () => {
policy,
parentReportAction,
hasViolations: false,
transactionThreadReport: null,
}),
);
});
Expand Down

0 comments on commit 92acb42

Please sign in to comment.