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.

Sign up for GitHub

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

feat: load policy categories only when needed #49756

Merged
merged 10 commits into from
Oct 7, 2024
Merged

feat: load policy categories only when needed #49756

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
be904bd
feat: load policy categories only when needed
jaydamani Sep 25, 2024
66db43f
fix ts error
jaydamani Sep 25, 2024
a966b2e
fix lint errors
jaydamani Sep 25, 2024
b873784
update API call for IOURequestStepCategory
jaydamani Sep 28, 2024
e262b38
withOnyx to useOnyx migration for IOURequestStepCategory
jaydamani Sep 28, 2024
e4a7807
add offline indicator for IOURequestStepCategory
jaydamani Sep 29, 2024
543716c
update offline 'categorize it' flow
jaydamani Oct 1, 2024
0a4ccd4
remove unused code
jaydamani Oct 2, 2024
a52e69e
update categorize it flow to show offline blocking view only when dat…
jaydamani Oct 4, 2024
4a7f23f
fix category selection in offline view for categorize it flow
jaydamani Oct 4, 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
36 changes: 12 additions & 24 deletions src/components/CategoryPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,34 +1,31 @@
import React, {useMemo} from 'react';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import useDebouncedState from '@hooks/useDebouncedState';
import useLocalize from '@hooks/useLocalize';
import useNetwork from '@hooks/useNetwork';
import * as OptionsListUtils from '@libs/OptionsListUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import SelectionList from './SelectionList';
import RadioListItem from './SelectionList/RadioListItem';
import type {ListItem} from './SelectionList/types';

type CategoryPickerOnyxProps = {
policyCategories: OnyxEntry<OnyxTypes.PolicyCategories>;
policyCategoriesDraft: OnyxEntry<OnyxTypes.PolicyCategories>;
policyRecentlyUsedCategories: OnyxEntry<OnyxTypes.RecentlyUsedCategories>;
};

type CategoryPickerProps = CategoryPickerOnyxProps & {
/** It's used by withOnyx HOC */
// eslint-disable-next-line react/no-unused-prop-types
type CategoryPickerProps = {
policyID: string;
selectedCategory?: string;
onSubmit: (item: ListItem) => void;
};

function CategoryPicker({selectedCategory, policyCategories, policyRecentlyUsedCategories, policyCategoriesDraft, onSubmit}: CategoryPickerProps) {
function CategoryPicker({selectedCategory, policyID, onSubmit}: CategoryPickerProps) {
const [policyCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`);
const [policyCategoriesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID}`);
const [policyRecentlyUsedCategories] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_CATEGORIES}${policyID}`);
const {isOffline} = useNetwork();

const {translate} = useLocalize();
const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState('');
const offlineMessage = isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : '';

const selectedOptions = useMemo(() => {
if (!selectedCategory) {
Expand Down Expand Up @@ -79,6 +76,7 @@ function CategoryPicker({selectedCategory, policyCategories, policyRecentlyUsedC
headerMessage={headerMessage}
textInputValue={searchValue}
textInputLabel={shouldShowTextInput ? translate('common.search') : undefined}
textInputHint={offlineMessage}
onChangeText={setSearchValue}
onSelectRow={onSubmit}
ListItem={RadioListItem}
Expand All @@ -90,14 +88,4 @@ function CategoryPicker({selectedCategory, policyCategories, policyRecentlyUsedC

CategoryPicker.displayName = 'CategoryPicker';

export default withOnyx<CategoryPickerProps, CategoryPickerOnyxProps>({
policyCategories: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${policyID}`,
},
policyCategoriesDraft: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${policyID}`,
},
policyRecentlyUsedCategories: {
key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_CATEGORIES}${policyID}`,
},
})(CategoryPicker);
export default CategoryPicker;
5 changes: 5 additions & 0 deletions src/libs/API/parameters/GetPolicyCategories.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type GetPolicyCategories = {
policyID: string;
};

export default GetPolicyCategories;
1 change: 1 addition & 0 deletions src/libs/API/parameters/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export type {default as ExpandURLPreviewParams} from './ExpandURLPreviewParams';
export type {default as GetMissingOnyxMessagesParams} from './GetMissingOnyxMessagesParams';
export type {default as GetNewerActionsParams} from './GetNewerActionsParams';
export type {default as GetOlderActionsParams} from './GetOlderActionsParams';
export type {default as GetPolicyCategoriesParams} from './GetPolicyCategories';
export type {default as GetReportPrivateNoteParams} from './GetReportPrivateNoteParams';
export type {default as GetRouteParams} from './GetRouteParams';
export type {default as GetStatementPDFParams} from './GetStatementPDFParams';
Expand Down
2 changes: 2 additions & 0 deletions src/libs/API/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,7 @@ const READ_COMMANDS = {
BEGIN_SIGNIN: 'BeginSignIn',
SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN: 'SignInWithShortLivedAuthToken',
SIGN_IN_WITH_SUPPORT_AUTH_TOKEN: 'SignInWithSupportAuthToken',
GET_POLICY_CATEGORIES: 'GetPolicyCategories',
OPEN_WORKSPACE: 'OpenWorkspace',
OPEN_WORKSPACE_MEMBERS_PAGE: 'OpenWorkspaceMembersPage',
OPEN_POLICY_CATEGORIES_PAGE: 'OpenPolicyCategoriesPage',
Expand Down Expand Up @@ -916,6 +917,7 @@ type ReadCommandParameters = {
[READ_COMMANDS.BEGIN_SIGNIN]: Parameters.BeginSignInParams;
[READ_COMMANDS.SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN]: Parameters.SignInWithShortLivedAuthTokenParams;
[READ_COMMANDS.SIGN_IN_WITH_SUPPORT_AUTH_TOKEN]: Parameters.SignInWithSupportAuthTokenParams;
[READ_COMMANDS.GET_POLICY_CATEGORIES]: Parameters.GetPolicyCategoriesParams;
[READ_COMMANDS.OPEN_WORKSPACE]: Parameters.OpenWorkspaceParams;
[READ_COMMANDS.OPEN_WORKSPACE_MEMBERS_PAGE]: Parameters.OpenWorkspaceMembersPageParams;
[READ_COMMANDS.OPEN_POLICY_CATEGORIES_PAGE]: Parameters.OpenPolicyCategoriesPageParams;
Expand Down
10 changes: 5 additions & 5 deletions src/libs/OptionsListUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,11 @@ Onyx.connect({
},
});

let allPolicyCategories: OnyxCollection<PolicyCategories> = {};
let allPolicies: OnyxCollection<Policy> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.POLICY_CATEGORIES,
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (val) => (allPolicyCategories = val),
callback: (val) => (allPolicies = val),
});

const lastReportActions: ReportActions = {};
Expand Down Expand Up @@ -1975,8 +1975,8 @@ function getOptions(
reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

if (action === CONST.IOU.ACTION.CATEGORIZE) {
const policyCategories = allPolicyCategories?.[`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${reportOption.policyID}`] ?? {};
if (getEnabledCategoriesCount(policyCategories) !== 0) {
const reportPolicy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${reportOption.policyID}`];
if (reportPolicy?.areCategoriesEnabled) {
recentReportOptions.push(reportOption);
}
} else {
Expand Down
15 changes: 15 additions & 0 deletions src/libs/actions/Policy/Category.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import Onyx from 'react-native-onyx';
import * as API from '@libs/API';
import type {
EnablePolicyCategoriesParams,
GetPolicyCategoriesParams,
OpenPolicyCategoriesPageParams,
RemovePolicyCategoryReceiptsRequiredParams,
SetPolicyCategoryApproverParams,
Expand Down Expand Up @@ -187,6 +188,19 @@ function openPolicyCategoriesPage(policyID: string) {
API.read(READ_COMMANDS.OPEN_POLICY_CATEGORIES_PAGE, params);
}

function getPolicyCategories(policyID: string) {
if (!policyID || policyID === '-1' || policyID === CONST.POLICY.ID_FAKE) {
Log.warn('GetPolicyCategories invalid params', {policyID});
return;
}

const params: GetPolicyCategoriesParams = {
policyID,
};

API.read(READ_COMMANDS.GET_POLICY_CATEGORIES, params);
}

function buildOptimisticPolicyRecentlyUsedCategories(policyID?: string, category?: string) {
if (!policyID || !category) {
return [];
Expand Down Expand Up @@ -1312,6 +1326,7 @@ function setPolicyCategoryTax(policyID: string, categoryName: string, taxID: str
}

export {
getPolicyCategories,
openPolicyCategoriesPage,
buildOptimisticPolicyRecentlyUsedCategories,
setWorkspaceCategoryEnabled,
Expand Down
117 changes: 28 additions & 89 deletions src/pages/iou/request/step/IOURequestStepCategory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import lodashIsEmpty from 'lodash/isEmpty';
import React, {useEffect} from 'react';
import {ActivityIndicator, View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import FullPageOfflineBlockingView from '@components/BlockingViews/FullPageOfflineBlockingView';
import Button from '@components/Button';
import CategoryPicker from '@components/CategoryPicker';
import FixedFooter from '@components/FixedFooter';
Expand All @@ -21,46 +21,18 @@ import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import * as ReportUtils from '@libs/ReportUtils';
import * as TransactionUtils from '@libs/TransactionUtils';
import * as IOU from '@userActions/IOU';
import * as PolicyActions from '@userActions/Policy/Policy';
import * as Category from '@userActions/Policy/Category';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type SCREENS from '@src/SCREENS';
import type {Policy, PolicyCategories, PolicyTagLists, ReportActions, Session, Transaction} from '@src/types/onyx';
import StepScreenWrapper from './StepScreenWrapper';
import type {WithFullTransactionOrNotFoundProps} from './withFullTransactionOrNotFound';
import withFullTransactionOrNotFound from './withFullTransactionOrNotFound';
import type {WithWritableReportOrNotFoundProps} from './withWritableReportOrNotFound';
import withWritableReportOrNotFound from './withWritableReportOrNotFound';

type IOURequestStepCategoryOnyxProps = {
/** The draft transaction that holds data to be persisted on the current transaction */
splitDraftTransaction: OnyxEntry<Transaction>;

/** The policy of the report */
policy: OnyxEntry<Policy>;

/** The draft policy of the report */
policyDraft: OnyxEntry<Policy>;

/** Collection of categories attached to a policy */
policyCategories: OnyxEntry<PolicyCategories>;

/** Collection of draft categories attached to a policy */
policyCategoriesDraft: OnyxEntry<PolicyCategories>;

/** Collection of tags attached to a policy */
policyTags: OnyxEntry<PolicyTagLists>;

/** The actions from the parent report */
reportActions: OnyxEntry<ReportActions>;

/** Session info for the currently logged in user. */
session: OnyxEntry<Session>;
};

type IOURequestStepCategoryProps = IOURequestStepCategoryOnyxProps &
WithWritableReportOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.STEP_CATEGORY> &
type IOURequestStepCategoryProps = WithWritableReportOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.STEP_CATEGORY> &
WithFullTransactionOrNotFoundProps<typeof SCREENS.MONEY_REQUEST.STEP_CATEGORY>;

function IOURequestStepCategory({
Expand All @@ -70,15 +42,24 @@ function IOURequestStepCategory({
params: {transactionID, backTo, action, iouType, reportActionID},
},
transaction,
splitDraftTransaction,
policy: policyReal,
policyDraft,
policyTags,
policyCategories: policyCategoriesReal,
policyCategoriesDraft,
reportActions,
session,
}: IOURequestStepCategoryProps) {
const [splitDraftTransaction] = useOnyx(`${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID ?? '-1'}`);
const [policyReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${IOU.getIOURequestPolicyID(transaction, reportReal)}`);
const [policyDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${IOU.getIOURequestPolicyID(transaction, reportDraft)}`);
const [policyCategoriesReal] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${IOU.getIOURequestPolicyID(transaction, reportReal)}`);
const [policyCategoriesDraft] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${IOU.getIOURequestPolicyID(transaction, reportDraft)}`);
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${IOU.getIOURequestPolicyID(transaction, reportReal)}`);
let reportID = '-1';
if (action === CONST.IOU.ACTION.EDIT && reportReal) {
if (iouType === CONST.IOU.TYPE.SPLIT) {
reportID = reportReal.reportID;
} else if (reportReal.parentReportID) {
reportID = reportReal.parentReportID;
}
}
const [reportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`, {canEvict: false});
const [session] = useOnyx(ONYXKEYS.SESSION);

const report = reportReal ?? reportDraft;
const policy = policyReal ?? policyDraft;
const policyCategories = policyCategoriesReal ?? policyCategoriesDraft;
Expand Down Expand Up @@ -108,11 +89,12 @@ function IOURequestStepCategory({
return;
}

PolicyActions.openDraftWorkspaceRequest(report?.policyID ?? '-1');
Category.getPolicyCategories(report?.policyID ?? '-1');
};
const {isOffline} = useNetwork({onReconnect: fetchData});
const isLoading = !isOffline && policyCategories === undefined;
const shouldShowEmptyState = !isLoading && !shouldShowCategory;
const shouldShowEmptyState = policyCategories !== undefined && !shouldShowCategory;
const shouldShowOfflineView = policyCategories === undefined && isOffline;

useEffect(() => {
fetchData();
Expand Down Expand Up @@ -159,6 +141,7 @@ function IOURequestStepCategory({
onBackButtonPress={navigateBack}
shouldShowWrapper
shouldShowNotFoundPage={shouldShowNotFoundPage}
shouldShowOfflineIndicator={policyCategories !== undefined}
testID={IOURequestStepCategory.displayName}
>
{isLoading && (
Expand All @@ -168,6 +151,7 @@ function IOURequestStepCategory({
color={theme.spinner}
/>
)}
{shouldShowOfflineView && <FullPageOfflineBlockingView>{null}</FullPageOfflineBlockingView>}
{shouldShowEmptyState && (
<View style={[styles.flex1]}>
<WorkspaceEmptyStateSection
Expand Down Expand Up @@ -198,7 +182,7 @@ function IOURequestStepCategory({
)}
</View>
)}
{!shouldShowEmptyState && !isLoading && (
{!shouldShowEmptyState && !isLoading && !shouldShowOfflineView && (
<>
<Text style={[styles.ph5, styles.pv3]}>{translate('iou.categorySelection')}</Text>
<CategoryPicker
Expand All @@ -214,53 +198,8 @@ function IOURequestStepCategory({

IOURequestStepCategory.displayName = 'IOURequestStepCategory';

const IOURequestStepCategoryWithOnyx = withOnyx<IOURequestStepCategoryProps, IOURequestStepCategoryOnyxProps>({
splitDraftTransaction: {
key: ({route}) => {
const transactionID = route?.params.transactionID ?? -1;
return `${ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT}${transactionID}`;
},
},
policy: {
key: ({report, transaction}) => `${ONYXKEYS.COLLECTION.POLICY}${IOU.getIOURequestPolicyID(transaction, report)}`,
},
policyDraft: {
key: ({reportDraft, transaction}) => `${ONYXKEYS.COLLECTION.POLICY_DRAFTS}${IOU.getIOURequestPolicyID(transaction, reportDraft)}`,
},
policyCategories: {
key: ({report, transaction}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES}${IOU.getIOURequestPolicyID(transaction, report)}`,
},
policyCategoriesDraft: {
key: ({reportDraft, transaction}) => `${ONYXKEYS.COLLECTION.POLICY_CATEGORIES_DRAFT}${IOU.getIOURequestPolicyID(transaction, reportDraft)}`,
},
policyTags: {
key: ({report, transaction}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${IOU.getIOURequestPolicyID(transaction, report)}`,
},
reportActions: {
key: ({
report,
route: {
params: {action, iouType},
},
}) => {
let reportID = '-1';
if (action === CONST.IOU.ACTION.EDIT && report) {
if (iouType === CONST.IOU.TYPE.SPLIT) {
reportID = report.reportID;
} else if (report.parentReportID) {
reportID = report.parentReportID;
}
}
return `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportID}`;
},
canEvict: false,
},
session: {
key: ONYXKEYS.SESSION,
},
})(IOURequestStepCategory);
/* eslint-disable rulesdir/no-negated-variables */
const IOURequestStepCategoryWithFullTransactionOrNotFound = withFullTransactionOrNotFound(IOURequestStepCategoryWithOnyx);
const IOURequestStepCategoryWithFullTransactionOrNotFound = withFullTransactionOrNotFound(IOURequestStepCategory);
/* eslint-disable rulesdir/no-negated-variables */
const IOURequestStepCategoryWithWritableReportOrNotFound = withWritableReportOrNotFound(IOURequestStepCategoryWithFullTransactionOrNotFound);
export default IOURequestStepCategoryWithWritableReportOrNotFound;
5 changes: 5 additions & 0 deletions src/pages/iou/request/step/StepScreenWrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type StepScreenWrapperProps = {
/** Whether or not to display not found page */
shouldShowNotFoundPage?: boolean;

/** Whether to show offline indicator */
shouldShowOfflineIndicator?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StepScreenWrapper renders ScreenWrapper which shows OfflineInidicator at bottom for small screen but our page would already show that user is offline so showing an indicator at bottom would feel weird.

App/src/components/ScreenWrapper.tsx

Line 123 in 9627f5f

shouldShowOfflineIndicator = true,

App/src/components/ScreenWrapper.tsx

Line 288 in 9627f5f

{isSmallScreenWidth && shouldShowOfflineIndicator && <OfflineIndicator style={offlineIndicatorStyle} />}

Basically, to avoid this:
image

Another approach I tried was to add below code to IOURequestStepCategory but then the header will not render because it comes from StepScreenWrapper so the current approach seemed better

if (policyCategories === undefined && isOffline) {
  // a children is mandatory for FullPageOfflineBlockingView
  return <FullPageOfflineBlockingView>{null}</FullPageOfflineBlockingView>
}


/** An ID used for unit testing */
testID: string;

Expand All @@ -44,6 +47,7 @@ function StepScreenWrapper({
shouldShowWrapper,
shouldShowNotFoundPage,
includeSafeAreaPaddingBottom,
shouldShowOfflineIndicator = true,
}: StepScreenWrapperProps) {
const styles = useThemeStyles();

Expand All @@ -57,6 +61,7 @@ function StepScreenWrapper({
onEntryTransitionEnd={onEntryTransitionEnd}
testID={testID}
shouldEnableMaxHeight={DeviceCapabilities.canUseTouchScreen()}
shouldShowOfflineIndicator={shouldShowOfflineIndicator}
>
{({insets, safeAreaPaddingBottomStyle, didScreenTransitionEnd}) => (
<FullPageNotFoundView shouldShow={shouldShowNotFoundPage}>
Expand Down
Loading