Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Room - Create room whisper reappears when interacting with it after workspace is deleted. #50692

Merged
merged 31 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
beb006f
fix: Room - Create room whisper reappears when interacting with it af…
Krishna2323 Oct 13, 2024
921f474
fix lint issues.
Krishna2323 Oct 13, 2024
597e243
minor update.
Krishna2323 Oct 13, 2024
352abf8
minor update.
Krishna2323 Oct 13, 2024
534c19e
fix lint issue.
Krishna2323 Oct 13, 2024
9a9ba02
Merge branch 'Expensify:main' into krishna2323/issue/49940
Krishna2323 Oct 20, 2024
176872b
Merge branch 'main' into krishna2323/issue/49940
Krishna2323 Oct 30, 2024
e602e69
pass 'canUserPerformWriteAction' to shouldReportActionBeVisible.
Krishna2323 Oct 30, 2024
7b32415
fix tests.
Krishna2323 Oct 30, 2024
16ca72b
fix: Unchanged files with check annotations.
Krishna2323 Oct 30, 2024
95bec1f
minor fixes.
Krishna2323 Oct 30, 2024
0ac8933
fix: TypeScript Checks.
Krishna2323 Oct 30, 2024
5fb1937
fix: TypeScript Checks.
Krishna2323 Oct 31, 2024
4016e70
Merge branch 'Expensify:main' into krishna2323/issue/49940
Krishna2323 Nov 1, 2024
47735bf
Merge branch 'Expensify:main' into krishna2323/issue/49940
Krishna2323 Nov 6, 2024
edf7c52
remove reportID parameter from shouldReportActionBeVisible.
Krishna2323 Nov 6, 2024
2bd8fe5
Merge branch 'Expensify:main' into krishna2323/issue/49940
Krishna2323 Nov 6, 2024
ea27cc3
remove redundant code.
Krishna2323 Nov 7, 2024
40b7435
minor update.
Krishna2323 Nov 7, 2024
606d8ad
Merge branch 'main' into krishna2323/issue/49940
Krishna2323 Nov 11, 2024
c870f75
minor update.
Krishna2323 Nov 11, 2024
dfdfb19
minor fix.
Krishna2323 Nov 13, 2024
2aacbe8
Merge branch 'Expensify:main' into krishna2323/issue/49940
Krishna2323 Nov 16, 2024
0e1da04
add tests for ReportActionsUtils.getSortedReportActionsForDisplay.
Krishna2323 Nov 16, 2024
676c822
fix unit tests.
Krishna2323 Nov 17, 2024
af332d7
minor update.
Krishna2323 Nov 17, 2024
ed6c4ff
fix tests.
Krishna2323 Nov 17, 2024
a360422
update test input data and comments.
Krishna2323 Nov 19, 2024
68d8fd6
update comments and input data.
Krishna2323 Nov 19, 2024
7d32b41
minor update.
Krishna2323 Nov 19, 2024
716c953
remove 'eslint-disable-next-line' comment.
Krishna2323 Nov 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {ValueOf} from 'type-fest';
import type {Attachment} from '@components/Attachments/types';
import * as FileUtils from '@libs/fileDownload/FileUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import tryResolveUrlFromApiRoot from '@libs/tryResolveUrlFromApiRoot';
import CONST from '@src/CONST';
import type {ReportAction, ReportActions} from '@src/types/onyx';
Expand All @@ -19,10 +20,13 @@ function extractAttachments(
accountID,
parentReportAction,
reportActions,
}: {privateNotes?: Record<number, Note>; accountID?: number; parentReportAction?: OnyxEntry<ReportAction>; reportActions?: OnyxEntry<ReportActions>},
reportID,
}: {privateNotes?: Record<number, Note>; accountID?: number; parentReportAction?: OnyxEntry<ReportAction>; reportActions?: OnyxEntry<ReportActions>; reportID: string},
) {
const targetNote = privateNotes?.[Number(accountID)]?.note ?? '';
const attachments: Attachment[] = [];
const report = ReportUtils.getReport(reportID);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const report = ReportUtils.getReport(reportID);
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractAttachments is a util function so we can't use useOnyx here.

const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

// We handle duplicate image sources by considering the first instance as original. Selecting any duplicate
// and navigating back (<) shows the image preceding the first instance, not the selected duplicate's position.
Expand Down Expand Up @@ -95,7 +99,7 @@ function extractAttachments(

const actions = [...(parentReportAction ? [parentReportAction] : []), ...ReportActionsUtils.getSortedReportActions(Object.values(reportActions ?? {}))];
actions.forEach((action, key) => {
if (!ReportActionsUtils.shouldReportActionBeVisible(action, key) || ReportActionsUtils.isMoneyRequestAction(action)) {
if (!ReportActionsUtils.shouldReportActionBeVisible(action, key, canUserPerformWriteAction) || ReportActionsUtils.isMoneyRequestAction(action)) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let newAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID, reportID: report.reportID});
} else {
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions, reportID: report.reportID});
}

let newIndex = newAttachments.findIndex(compareImage);
Expand Down
18 changes: 15 additions & 3 deletions src/components/Attachments/AttachmentCarousel/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
const parentReportAction = report.parentReportActionID && parentReportActions ? parentReportActions[report.parentReportActionID] : undefined;
let newAttachments: Attachment[] = [];
if (type === CONST.ATTACHMENT_TYPE.NOTE && accountID) {
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.NOTE, {privateNotes: report.privateNotes, accountID, reportID: report.reportID});
} else {
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined});
newAttachments = extractAttachments(CONST.ATTACHMENT_TYPE.REPORT, {parentReportAction, reportActions: reportActions ?? undefined, reportID: report.reportID});
}

if (isEqual(attachments, newAttachments)) {
Expand Down Expand Up @@ -117,7 +117,19 @@ function AttachmentCarousel({report, source, onNavigate, setDownloadButtonVisibi
onNavigate(attachment);
}
}
}, [report.privateNotes, reportActions, parentReportActions, compareImage, report.parentReportActionID, attachments, setDownloadButtonVisibility, onNavigate, accountID, type]);
}, [
report.privateNotes,
reportActions,
parentReportActions,
compareImage,
report.parentReportActionID,
attachments,
setDownloadButtonVisibility,
onNavigate,
accountID,
type,
report.reportID,
]);

// Scroll position is affected when window width is resized, so we readjust it on width changes
useEffect(() => {
Expand Down
5 changes: 4 additions & 1 deletion src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import * as DraftCommentUtils from '@libs/DraftCommentUtils';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import variables from '@styles/variables';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -139,7 +140,9 @@ function LHNOptionsList({style, contentContainerStyles, data, onSelectRow, optio
: '-1';
const itemTransaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
const hasDraftComment = DraftCommentUtils.isValidDraftComment(draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`]);
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);

const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(itemFullReport);
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions, canUserPerformWriteAction);
const lastReportAction = sortedReportActions.at(0);

// Get the transaction for the last report action
Expand Down
7 changes: 6 additions & 1 deletion src/components/ParentNavigationSubtitle.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import React from 'react';
import type {StyleProp, ViewStyle} from 'react-native';
import {useOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import CONST from '@src/CONST';
import type {ParentNavigationSummaryParams} from '@src/languages/params';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import PressableWithoutFeedback from './Pressable/PressableWithoutFeedback';
import Text from './Text';
Expand All @@ -29,6 +32,8 @@ function ParentNavigationSubtitle({parentNavigationSubtitleData, parentReportAct
const {workspaceName, reportName} = parentNavigationSubtitleData;
const {isOffline} = useNetwork();
const {translate} = useLocalize();
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${parentReportID}`);
const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

// We should not display the parent navigation subtitle if the user does not have access to the parent chat (the reportName is empty in this case)
if (!reportName) {
Expand All @@ -39,7 +44,7 @@ function ParentNavigationSubtitle({parentNavigationSubtitleData, parentReportAct
<PressableWithoutFeedback
onPress={() => {
const parentAction = ReportActionsUtils.getReportAction(parentReportID, parentReportActionID ?? '-1');
const isVisibleAction = ReportActionsUtils.shouldReportActionBeVisible(parentAction, parentAction?.reportActionID ?? '-1');
const isVisibleAction = ReportActionsUtils.shouldReportActionBeVisible(parentAction, parentAction?.reportActionID ?? '-1', canUserPerformWriteAction);
// Pop the thread report screen before navigating to the chat report.
Navigation.goBack(ROUTES.REPORT_WITH_ID.getRoute(parentReportID));
if (isVisibleAction && !isOffline) {
Expand Down
5 changes: 4 additions & 1 deletion src/hooks/usePaginatedReportActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {useMemo} from 'react';
import {useOnyx} from 'react-native-onyx';
import PaginationUtils from '@libs/PaginationUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import ONYXKEYS from '@src/ONYXKEYS';

/**
Expand All @@ -11,10 +12,12 @@ function usePaginatedReportActions(reportID?: string, reportActionID?: string) {
// Use `||` instead of `??` to handle empty string.
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const reportIDWithDefault = reportID || '-1';
const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${reportID}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, we need to use reportIDWithDefault here. This caused this issue: #52864

const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

const [sortedAllReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportIDWithDefault}`, {
canEvict: false,
selector: (allReportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, true),
selector: (allReportActions) => ReportActionsUtils.getSortedReportActionsForDisplay(allReportActions, canUserPerformWriteAction, true),
});
const [reportActionPages] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_PAGES}${reportIDWithDefault}`);

Expand Down
6 changes: 3 additions & 3 deletions src/libs/Middleware/Pagination.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type PagedResource<TResourceKey extends OnyxCollectionKey> = OnyxValues[TResourc
type PaginationCommonConfig<TResourceKey extends OnyxCollectionKey = OnyxCollectionKey, TPageKey extends OnyxPagesKey = OnyxPagesKey> = {
resourceCollectionKey: TResourceKey;
pageCollectionKey: TPageKey;
sortItems: (items: OnyxValues[TResourceKey]) => Array<PagedResource<TResourceKey>>;
sortItems: (items: OnyxValues[TResourceKey], reportID: string) => Array<PagedResource<TResourceKey>>;
getItemID: (item: PagedResource<TResourceKey>) => string;
};

Expand Down Expand Up @@ -96,7 +96,7 @@ const Pagination: Middleware = (requestResponse, request) => {

// Create a new page based on the response
const pageItems = (response.onyxData.find((data) => data.key === resourceKey)?.value ?? {}) as OnyxValues[typeof resourceCollectionKey];
const sortedPageItems = sortItems(pageItems);
const sortedPageItems = sortItems(pageItems, resourceID);
if (sortedPageItems.length === 0) {
// Must have at least 1 action to create a page.
Log.hmmm(`[Pagination] Did not receive any items in the response to ${request.command}`);
Expand All @@ -115,7 +115,7 @@ const Pagination: Middleware = (requestResponse, request) => {
const resourceCollections = resources.get(resourceCollectionKey) ?? {};
const existingItems = resourceCollections[resourceKey] ?? {};
const allItems = fastMerge(existingItems, pageItems, true);
const sortedAllItems = sortItems(allItems);
const sortedAllItems = sortItems(allItems, resourceID);

const pagesCollections = pages.get(pageCollectionKey) ?? {};
const existingPages = pagesCollections[pageKey] ?? [];
Expand Down
7 changes: 5 additions & 2 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,11 +316,14 @@ Onyx.connect({
lastReportActions[reportID] = firstReportAction;
}

const report = ReportUtils.getReport(reportID);
const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

// The report is only visible if it is the last action not deleted that
// does not match a closed or created state.
const reportActionsForDisplay = sortedReportActions.filter(
(reportAction, actionKey) =>
ReportActionUtils.shouldReportActionBeVisible(reportAction, actionKey) &&
ReportActionUtils.shouldReportActionBeVisible(reportAction, actionKey, canUserPerformWriteAction) &&
!ReportActionUtils.isWhisperAction(reportAction) &&
reportAction.actionName !== CONST.REPORT.ACTIONS.TYPE.CREATED &&
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE &&
Expand Down Expand Up @@ -578,7 +581,7 @@ function getLastMessageTextForReport(report: OnyxEntry<Report>, lastActorDetails
const iouReport = ReportUtils.getReportOrDraftReport(ReportActionUtils.getIOUReportIDFromReportActionPreview(lastReportAction));
const lastIOUMoneyReportAction = allSortedReportActions[iouReport?.reportID ?? '-1']?.find(
(reportAction, key): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU> =>
ReportActionUtils.shouldReportActionBeVisible(reportAction, key) &&
ReportActionUtils.shouldReportActionBeVisible(reportAction, key, ReportUtils.canUserPerformWriteAction(report)) &&
reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE &&
ReportActionUtils.isMoneyRequestAction(reportAction),
);
Expand Down
Loading
Loading