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

[NoQA] Revert lhn order #38081

Merged
merged 3 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 0 additions & 2 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1498,8 +1498,6 @@ const CONST = {
ALPHABETIC_AND_LATIN_CHARS: /^[\p{Script=Latin} ]*$/u,
NON_ALPHABETIC_AND_NON_LATIN_CHARS: /[^\p{Script=Latin}]/gu,
ACCENT_LATIN_CHARS: /[\u00C0-\u017F]/g,
INVALID_DISPLAY_NAME_LHN: /[^\p{L}\p{N}\u00C0-\u017F\s-]/gu,
INVALID_DISPLAY_NAME_ONLY_LHN: /^[^\p{L}\p{N}\u00C0-\u017F]$/gu,
POSITIVE_INTEGER: /^\d+$/,
PO_BOX: /\b[P|p]?(OST|ost)?\.?\s*[O|o|0]?(ffice|FFICE)?\.?\s*[B|b][O|o|0]?[X|x]?\.?\s+[#]?(\d+)\b/,
ANY_VALUE: /^.+$/,
Expand Down
4 changes: 0 additions & 4 deletions src/components/LHNOptionsList/LHNOptionsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ function LHNOptionsList({
draftComments = {},
transactionViolations = {},
onFirstItemRendered = () => {},
reportIDsWithErrors = {},
}: LHNOptionsListProps) {
const styles = useThemeStyles();
const {canUseViolations} = usePermissions();
Expand Down Expand Up @@ -64,7 +63,6 @@ function LHNOptionsList({
const itemComment = draftComments?.[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] ?? '';
const sortedReportActions = ReportActionsUtils.getSortedReportActionsForDisplay(itemReportActions);
const lastReportAction = sortedReportActions[0];
const reportErrors = reportIDsWithErrors[reportID] ?? {};

// Get the transaction for the last report action
let lastReportActionTransactionID = '';
Expand Down Expand Up @@ -93,7 +91,6 @@ function LHNOptionsList({
transactionViolations={transactionViolations}
canUseViolations={canUseViolations}
onLayout={onLayoutItem}
reportErrors={reportErrors}
/>
);
},
Expand All @@ -112,7 +109,6 @@ function LHNOptionsList({
transactionViolations,
canUseViolations,
onLayoutItem,
reportIDsWithErrors,
],
);

Expand Down
4 changes: 1 addition & 3 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ function OptionRowLHNData({
lastReportActionTransaction = {},
transactionViolations,
canUseViolations,
reportErrors,
...propsToForward
}: OptionRowLHNDataProps) {
const reportID = propsToForward.reportID;
Expand All @@ -41,11 +40,11 @@ function OptionRowLHNData({
// Note: ideally we'd have this as a dependent selector in onyx!
const item = SidebarUtils.getOptionData({
report: fullReport,
reportActions,
personalDetails,
preferredLocale: preferredLocale ?? CONST.LOCALES.DEFAULT,
policy,
parentReportAction,
reportErrors,
hasViolations: !!hasViolations,
});
if (deepEqual(item, optionItemRef.current)) {
Expand All @@ -70,7 +69,6 @@ function OptionRowLHNData({
transactionViolations,
canUseViolations,
receiptTransactions,
reportErrors,
]);

useEffect(() => {
Expand Down
7 changes: 0 additions & 7 deletions src/components/LHNOptionsList/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import type {CurrentReportIDContextValue} from '@components/withCurrentReportID'
import type CONST from '@src/CONST';
import type {OptionData} from '@src/libs/ReportUtils';
import type {Locale, PersonalDetailsList, Policy, Report, ReportAction, ReportActions, Transaction, TransactionViolation} from '@src/types/onyx';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import type {EmptyObject} from '@src/types/utils/EmptyObject';

type OptionMode = ValueOf<typeof CONST.OPTION_MODE>;
Expand Down Expand Up @@ -59,9 +58,6 @@ type CustomLHNOptionsListProps = {

/** Callback to fire when the list is laid out */
onFirstItemRendered: () => void;

/** Report IDs with errors mapping to their corresponding error objects */
reportIDsWithErrors: Record<string, OnyxCommon.Errors>;
};

type LHNOptionsListProps = CustomLHNOptionsListProps & CurrentReportIDContextValue & LHNOptionsListOnyxProps;
Expand Down Expand Up @@ -117,9 +113,6 @@ type OptionRowLHNDataProps = {

/** Callback to execute when the OptionList lays out */
onLayout?: (event: LayoutChangeEvent) => void;

/** The report errors */
reportErrors: OnyxCommon.Errors | undefined;
};

type OptionRowLHNProps = {
Expand Down
4 changes: 2 additions & 2 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ function getSearchText(
/**
* Get an object of error messages keyed by microtime by combining all error objects related to the report.
*/
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>, transactions: OnyxCollection<Transaction> = allTransactions): OnyxCommon.Errors {
function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>): OnyxCommon.Errors {
const reportErrors = report?.errors ?? {};
const reportErrorFields = report?.errorFields ?? {};
const reportActionErrors: OnyxCommon.ErrorFields = Object.values(reportActions ?? {}).reduce(
Expand All @@ -492,7 +492,7 @@ function getAllReportErrors(report: OnyxEntry<Report>, reportActions: OnyxEntry<

if (parentReportAction?.actorAccountID === currentUserAccountID && ReportActionUtils.isTransactionThread(parentReportAction)) {
const transactionID = parentReportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.IOU ? parentReportAction?.originalMessage?.IOUTransactionID : null;
const transaction = transactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxError('report.genericSmartscanFailureMessage');
}
Expand Down
45 changes: 15 additions & 30 deletions src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetails, PersonalDetailsList, TransactionViolation} from '@src/types/onyx';
import type Beta from '@src/types/onyx/Beta';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import type Policy from '@src/types/onyx/Policy';
import type Report from '@src/types/onyx/Report';
import type {ReportActions} from '@src/types/onyx/ReportAction';
Expand Down Expand Up @@ -58,13 +57,6 @@ function compareStringDates(a: string, b: string): 0 | 1 | -1 {
return 0;
}

function filterDisplayName(displayName: string): string {
if (CONST.REGEX.INVALID_DISPLAY_NAME_ONLY_LHN.test(displayName)) {
return displayName;
}
return displayName.replace(CONST.REGEX.INVALID_DISPLAY_NAME_LHN, '').trim();
}

/**
* @returns An array of reportIDs sorted in the proper order
*/
Expand All @@ -74,27 +66,22 @@ function getOrderedReportIDs(
betas: Beta[],
policies: Record<string, Policy>,
priorityMode: ValueOf<typeof CONST.PRIORITY_MODE>,
allReportActions: OnyxCollection<ReportActions>,
allReportActions: OnyxCollection<ReportAction[]>,
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
reportIDsWithErrors: Record<string, OnyxCommon.Errors> = {},
canUseViolations = false,
): string[] {
const isInGSDMode = priorityMode === CONST.PRIORITY_MODE.GSD;
const isInDefaultMode = !isInGSDMode;
const allReportsDictValues = Object.values(allReports);

const reportIDsWithViolations = new Set<string>();

// Filter out all the reports that shouldn't be displayed
let reportsToDisplay = allReportsDictValues.filter((report) => {
const parentReportActionsKey = `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report?.parentReportID}`;
const parentReportAction = allReportActions?.[parentReportActionsKey]?.[report.parentReportActionID ?? ''];
const doesReportHaveViolations = canUseViolations && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction);
if (doesReportHaveViolations) {
reportIDsWithViolations.add(report.reportID);
}
const parentReportActions = allReportActions?.[parentReportActionsKey];
const parentReportAction = parentReportActions?.find((action) => action && report && action?.reportActionID === report?.parentReportActionID);
const doesReportHaveViolations =
betas.includes(CONST.BETAS.VIOLATIONS) && !!parentReportAction && ReportUtils.doesTransactionThreadHaveViolations(report, transactionViolations, parentReportAction);
return ReportUtils.shouldReportBeInOptionList({
report,
currentReportId: currentReportId ?? '',
Expand All @@ -116,15 +103,15 @@ function getOrderedReportIDs(
}

// The LHN is split into four distinct groups, and each group is sorted a little differently. The groups will ALWAYS be in this order:
// 1. Pinned/GBR/RBR - Always sorted by reportDisplayName
// 1. Pinned/GBR - Always sorted by reportDisplayName
// 2. Drafts - Always sorted by reportDisplayName
// 3. Non-archived reports and settled IOUs
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
// 4. Archived reports
// - Sorted by lastVisibleActionCreated in default (most recent) view mode
// - Sorted by reportDisplayName in GSD (focus) view mode
const pinnedAndBrickRoadReports: Report[] = [];
const pinnedAndGBRReports: Report[] = [];
const draftReports: Report[] = [];
const nonArchivedReports: Report[] = [];
const archivedReports: Report[] = [];
Expand All @@ -140,14 +127,12 @@ function getOrderedReportIDs(
// However, this code needs to be very performant to handle thousands of reports, so in the interest of speed, we're just going to disable this lint rule and add
// the reportDisplayName property to the report object directly.
// eslint-disable-next-line no-param-reassign
report.displayName = filterDisplayName(ReportUtils.getReportName(report));

const hasRBR = report.reportID in reportIDsWithErrors || reportIDsWithViolations.has(report.reportID);
report.displayName = ReportUtils.getReportName(report);

const isPinned = report.isPinned ?? false;
const reportAction = ReportActionsUtils.getReportAction(report.parentReportID ?? '', report.parentReportActionID ?? '');
if (isPinned || hasRBR || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndBrickRoadReports.push(report);
if (isPinned || ReportUtils.requiresAttentionFromCurrentUser(report, reportAction)) {
pinnedAndGBRReports.push(report);
} else if (report.hasDraft) {
draftReports.push(report);
} else if (ReportUtils.isArchivedRoom(report)) {
Expand All @@ -158,7 +143,7 @@ function getOrderedReportIDs(
});

// Sort each group of reports accordingly
pinnedAndBrickRoadReports.sort((a, b) => (a?.displayName && b?.displayName ? localeCompare(a.displayName, b.displayName) : 0));
pinnedAndGBRReports.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 @@ -179,7 +164,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 = [...pinnedAndBrickRoadReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
const LHNReports = [...pinnedAndGBRReports, ...draftReports, ...nonArchivedReports, ...archivedReports].map((report) => report.reportID);
return LHNReports;
}

Expand All @@ -188,19 +173,19 @@ function getOrderedReportIDs(
*/
function getOptionData({
report,
reportActions,
personalDetails,
preferredLocale,
policy,
parentReportAction,
reportErrors,
hasViolations,
}: {
report: OnyxEntry<Report>;
reportActions: OnyxEntry<ReportActions>;
personalDetails: OnyxEntry<PersonalDetailsList>;
preferredLocale: DeepValueOf<typeof CONST.LOCALES>;
policy: OnyxEntry<Policy> | undefined;
parentReportAction: OnyxEntry<ReportAction> | undefined;
reportErrors: OnyxCommon.Errors | undefined;
hasViolations: boolean;
}): ReportUtils.OptionData | undefined {
// When a user signs out, Onyx is cleared. Due to the lazy rendering with a virtual list, it's possible for
Expand All @@ -213,7 +198,7 @@ function getOptionData({
const result: ReportUtils.OptionData = {
text: '',
alternateText: null,
allReportErrors: reportErrors,
allReportErrors: OptionsListUtils.getAllReportErrors(report, reportActions),
brickRoadIndicator: null,
tooltipText: null,
subtitle: null,
Expand Down
3 changes: 1 addition & 2 deletions src/pages/home/sidebar/SidebarLinks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const propTypes = {
isActiveReport: PropTypes.func.isRequired,
};

function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport, isCreateMenuOpen, activePolicy, reportIDsWithErrors}) {
function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priorityMode = CONST.PRIORITY_MODE.DEFAULT, isActiveReport, isCreateMenuOpen, activePolicy}) {
const styles = useThemeStyles();
const StyleUtils = useStyleUtils();
const modal = useRef({});
Expand Down Expand Up @@ -154,7 +154,6 @@ function SidebarLinks({onLinkClick, insets, optionListItems, isLoading, priority
shouldDisableFocusOptions={isSmallScreenWidth}
optionMode={viewMode}
onFirstItemRendered={App.setSidebarLoaded}
reportIDsWithErrors={reportIDsWithErrors}
/>
{isLoading && optionListItems.length === 0 && (
<View style={[StyleSheet.absoluteFillObject, styles.appBG]}>
Expand Down
Loading
Loading