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 RBR transaction thread is disappearing from the LHN when navigating to another chat v2 #43502

Merged
merged 14 commits into from
Jun 28, 2024
29 changes: 22 additions & 7 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1686,6 +1686,26 @@ function getUserToInviteOption({
return userToInvite;
}

/**
* Check whether report has violations
*/
function shouldShowViolations(report: Report, betas: OnyxEntry<Beta[]>, transactionViolations: OnyxCollection<TransactionViolation[]>) {
if (!Permissions.canUseViolations(betas)) {
return false;
}
const {parentReportID, parentReportActionID} = report ?? {};
const canGetParentReport = parentReportID && parentReportActionID && allReportActions;
if (!canGetParentReport) {
return false;
}
const parentReportActions = allReportActions ? allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? {} : {};
const parentReportAction = parentReportActions[parentReportActionID] ?? null;
if (!parentReportAction) {
return false;
}
return ReportUtils.shouldDisplayTransactionThreadViolations(report, transactionViolations, parentReportAction);
}

/**
* filter options based on specific conditions
*/
Expand Down Expand Up @@ -1793,13 +1813,7 @@ function getOptions(
// Filter out all the reports that shouldn't be displayed
const filteredReportOptions = options.reports.filter((option) => {
const report = option.item;

const {parentReportID, parentReportActionID} = report ?? {};
const canGetParentReport = parentReportID && parentReportActionID && allReportActions;
const parentReportActions = allReportActions ? allReportActions[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? {} : {};
const parentReportAction = canGetParentReport ? parentReportActions[parentReportActionID] ?? null : null;
const doesReportHaveViolations =
(betas?.includes(CONST.BETAS.VIOLATIONS) && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction)) ?? false;
const doesReportHaveViolations = shouldShowViolations(report, betas, transactionViolations);

return ReportUtils.shouldReportBeInOptionList({
report,
Expand Down Expand Up @@ -2494,6 +2508,7 @@ export {
getTaxRatesSection,
getFirstKeyForList,
getUserToInviteOption,
shouldShowViolations,
};

export type {MemberForList, CategorySection, CategoryTreeSection, Options, OptionList, SearchOption, PayeePersonalDetails, Category, Tax, TaxRatesOption, Option, OptionTree};
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@ function isOneTransactionReport(reportID: string): boolean {
function isOneTransactionThread(reportID: string, parentReportID: string): boolean {
const parentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`] ?? ([] as ReportAction[]);
const transactionThreadReportID = ReportActionsUtils.getOneTransactionThreadReportID(parentReportID, parentReportActions);
return reportID === transactionThreadReportID;
return reportID?.toString() === transactionThreadReportID?.toString();
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't both of these already be strings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I checked and found out that one is a number, and the other one is a string. This bug is mentioned here

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm I've asked for clarification here on what's best to do. Ideally this would be fixed in the BE first, but of course that can take some time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jjcoffee So should we revert this fix return reportID?.toString() === transactionThreadReportID?.toString() from FE, then wait until BE fixes it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tienifr Could you respond to this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

I put up a PR to fix the type on the BE. We should be able to remove the cast to string once that's merged/deployed

}

/**
Expand Down
63 changes: 37 additions & 26 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,46 +81,53 @@ function getOrderedReportIDs(
const allReportsDictValues = Object.values(allReports ?? {});

// Filter out all the reports that shouldn't be displayed
let reportsToDisplay = allReportsDictValues.filter((report) => {
let reportsToDisplay: Array<ChatReportSelector & {hasErrorsOtherThanFailedReceipt?: boolean}> = [];
allReportsDictValues.forEach((report) => {
if (!report) {
return false;
return;
}

const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`;
const parentReportActions = allReportActions?.[parentReportActionsKey];
const reportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {};
const parentReportAction = parentReportActions?.find((action) => action && action?.reportActionID === report.parentReportActionID);
const doesReportHaveViolations = !!(
betas?.includes(CONST.BETAS.VIOLATIONS) &&
!!parentReportAction &&
ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction as OnyxEntry<ReportAction>)
);
const doesReportHaveViolations = OptionsListUtils.shouldShowViolations(report, betas ?? [], transactionViolations);
const isHidden = report.notificationPreference === CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN;
const isFocused = report.reportID === currentReportId;
const allReportErrors = OptionsListUtils.getAllReportErrors(report, reportActions) ?? {};
const hasErrorsOtherThanFailedReceipt = doesReportHaveViolations || Object.values(allReportErrors).some((error) => error?.[0] !== 'report.genericSmartscanFailureMessage');
if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0')) {
return;
}
if (hasErrorsOtherThanFailedReceipt) {
reportsToDisplay.push({
...report,
hasErrorsOtherThanFailedReceipt: true,
});
return;
}
const isSystemChat = ReportUtils.isSystemChat(report);
const shouldOverrideHidden = hasErrorsOtherThanFailedReceipt || isFocused || isSystemChat || report.isPinned;
if (isHidden && !shouldOverrideHidden) {
return false;
return;
}

const participantAccountIDs = Object.keys(report?.participants ?? {}).map(Number);

if (currentUserAccountID && AccountUtils.isAccountIDOddNumber(currentUserAccountID) && participantAccountIDs.includes(CONST.ACCOUNT_ID.NOTIFICATIONS)) {
return true;
reportsToDisplay.push(report);
return;
}
if (
ReportUtils.shouldReportBeInOptionList({
report,
currentReportId: currentReportId ?? '',
isInFocusMode,
betas,
policies: policies as OnyxCollection<Policy>,
excludeEmptyChats: true,
doesReportHaveViolations,
includeSelfDM: true,
})
) {
reportsToDisplay.push(report);
}

return ReportUtils.shouldReportBeInOptionList({
report,
currentReportId: currentReportId ?? '',
isInFocusMode,
betas,
policies: policies as OnyxCollection<Policy>,
excludeEmptyChats: true,
doesReportHaveViolations,
includeSelfDM: true,
});
});

// The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
Expand All @@ -136,6 +143,7 @@ function getOrderedReportIDs(
const draftReports: Array<OnyxEntry<Report>> = [];
const nonArchivedReports: Array<OnyxEntry<Report>> = [];
const archivedReports: Array<OnyxEntry<Report>> = [];
const errorReports: Array<OnyxEntry<Report>> = [];

if (currentPolicyID || policyMemberAccountIDs.length > 0) {
reportsToDisplay = reportsToDisplay.filter(
Expand All @@ -144,7 +152,7 @@ function getOrderedReportIDs(
}
// There are a few properties that need to be calculated for the report which are used when sorting reports.
reportsToDisplay.forEach((reportToDisplay) => {
let report = reportToDisplay as OnyxEntry<Report>;
let report = reportToDisplay;
if (report) {
report = {
...report,
Expand All @@ -160,13 +168,16 @@ function getOrderedReportIDs(
draftReports.push(report);
} else if (ReportUtils.isArchivedRoom(report)) {
archivedReports.push(report);
} else if (report?.hasErrorsOtherThanFailedReceipt) {
errorReports.push(report);
} else {
nonArchivedReports.push(report);
}
});

// Sort each group of reports accordingly
pinnedAndGBRReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
errorReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
draftReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));

if (isInDefaultMode) {
Expand All @@ -187,7 +198,7 @@ function getOrderedReportIDs(

// Now that we have all the reports grouped and sorted, they must be flattened into an array and only return the reportID.
// The order the arrays are concatenated in matters and will determine the order that the groups are displayed in the sidebar.
const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report?.reportID ?? '');
const LHNReports = [...pinnedAndGBRReports, ...errorReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report?.reportID ?? '');
return LHNReports;
}

Expand Down
Loading