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

Add missing multi tag view #41351

Merged
merged 27 commits into from
May 12, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
8 changes: 6 additions & 2 deletions src/ROUTES.ts
Original file line number Diff line number Diff line change
Expand Up @@ -656,8 +656,8 @@ const ROUTES = {
getRoute: (policyID: string) => `settings/workspaces/${policyID}/tags/settings` as const,
},
WORKSPACE_EDIT_TAGS: {
route: 'settings/workspaces/:policyID/tags/edit',
getRoute: (policyID: string) => `settings/workspaces/${policyID}/tags/edit` as const,
route: 'settings/workspaces/:policyID/tags/:order/edit',
s77rt marked this conversation as resolved.
Show resolved Hide resolved
getRoute: (policyID: string, order: string) => `settings/workspaces/${policyID}/tags/${encodeURIComponent(order)}/edit` as const,
},
WORKSPACE_TAG_EDIT: {
route: 'settings/workspace/:policyID/tag/:tagName/edit',
Expand All @@ -667,6 +667,10 @@ const ROUTES = {
route: 'settings/workspaces/:policyID/tag/:tagName',
getRoute: (policyID: string, tagName: string) => `settings/workspaces/${policyID}/tag/${encodeURIComponent(tagName)}` as const,
},
WORKSPACE_TAG_LIST_VIEW: {
route: 'settings/workspaces/:policyID/tag-list/:order',
getRoute: (policyID: string, order: string) => `settings/workspaces/${policyID}/tag-list/${encodeURIComponent(order)}` as const,
},
WORKSPACE_TAXES: {
route: 'settings/workspaces/:policyID/taxes',
getRoute: (policyID: string) => `settings/workspaces/${policyID}/taxes` as const,
Expand Down
1 change: 1 addition & 0 deletions src/SCREENS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ const SCREENS = {
TAX_CREATE: 'Workspace_Tax_Create',
TAG_CREATE: 'Tag_Create',
TAG_SETTINGS: 'Tag_Settings',
TAG_LIST_VIEW: 'Tag_List_View',
CURRENCY: 'Workspace_Profile_Currency',
WORKFLOWS: 'Workspace_Workflows',
WORKFLOWS_PAYER: 'Workspace_Workflows_Payer',
Expand Down
34 changes: 34 additions & 0 deletions src/components/SelectionList/RightElementRequiredStatus.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import {View} from 'react-native';
import Icon from '@components/Icon';
import * as Expensicons from '@components/Icon/Expensicons';
import Text from '@components/Text';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';

type RightElementRequiredStatusProps = {
required?: boolean;
};

function RightElementRequiredStatus({required}: RightElementRequiredStatusProps) {
s77rt marked this conversation as resolved.
Show resolved Hide resolved
const styles = useThemeStyles();
const theme = useTheme();
const {translate} = useLocalize();

return (
<View style={styles.flexRow}>
{!!required && <Text style={[styles.alignSelfCenter, styles.textSupporting, styles.pl2, styles.label]}>{translate('common.required')}</Text>}
<View style={[styles.p1, styles.pl2]}>
<Icon
src={Expensicons.ArrowRight}
fill={theme.icon}
/>
</View>
</View>
);
}

RightElementRequiredStatus.displayName = 'RightElementRequiredStatus';

export default RightElementRequiredStatus;
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ const SettingsModalStackNavigator = createModalStackNavigator<SettingsNavigatorP
[SCREENS.WORKSPACE.DISTANCE_RATE_EDIT]: () => require('../../../../pages/workspace/distanceRates/PolicyDistanceRateEditPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAGS_SETTINGS]: () => require('../../../../pages/workspace/tags/WorkspaceTagsSettingsPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAG_SETTINGS]: () => require('../../../../pages/workspace/tags/TagSettingsPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAG_LIST_VIEW]: () => require('../../../../pages/workspace/tags/WorkspaceViewTagsPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAGS_EDIT]: () => require('../../../../pages/workspace/tags/WorkspaceEditTagsPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAG_CREATE]: () => require('../../../../pages/workspace/tags/WorkspaceCreateTagPage').default as React.ComponentType,
[SCREENS.WORKSPACE.TAG_EDIT]: () => require('../../../../pages/workspace/tags/EditTagPage').default as React.ComponentType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,14 @@ const FULL_SCREEN_TO_RHP_MAPPING: Partial<Record<FullScreenName, string[]>> = {
SCREENS.WORKSPACE.TAX_NAME,
SCREENS.WORKSPACE.TAX_VALUE,
],
[SCREENS.WORKSPACE.TAGS]: [SCREENS.WORKSPACE.TAGS_SETTINGS, SCREENS.WORKSPACE.TAGS_EDIT, SCREENS.WORKSPACE.TAG_CREATE, SCREENS.WORKSPACE.TAG_SETTINGS, SCREENS.WORKSPACE.TAG_EDIT],
[SCREENS.WORKSPACE.TAGS]: [
SCREENS.WORKSPACE.TAGS_SETTINGS,
SCREENS.WORKSPACE.TAGS_EDIT,
SCREENS.WORKSPACE.TAG_CREATE,
SCREENS.WORKSPACE.TAG_SETTINGS,
SCREENS.WORKSPACE.TAG_EDIT,
SCREENS.WORKSPACE.TAG_LIST_VIEW,
],
[SCREENS.WORKSPACE.CATEGORIES]: [SCREENS.WORKSPACE.CATEGORY_CREATE, SCREENS.WORKSPACE.CATEGORY_SETTINGS, SCREENS.WORKSPACE.CATEGORIES_SETTINGS, SCREENS.WORKSPACE.CATEGORY_EDIT],
[SCREENS.WORKSPACE.DISTANCE_RATES]: [
SCREENS.WORKSPACE.CREATE_DISTANCE_RATE,
Expand Down
9 changes: 9 additions & 0 deletions src/libs/Navigation/linkingConfig/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
},
[SCREENS.WORKSPACE.TAGS_EDIT]: {
path: ROUTES.WORKSPACE_EDIT_TAGS.route,
parse: {
order: (order: string) => decodeURIComponent(order),
},
},
[SCREENS.WORKSPACE.TAG_CREATE]: {
path: ROUTES.WORKSPACE_TAG_CREATE.route,
Expand All @@ -416,6 +419,12 @@ const config: LinkingOptions<RootStackParamList>['config'] = {
tagName: (tagName: string) => decodeURIComponent(tagName),
},
},
[SCREENS.WORKSPACE.TAG_LIST_VIEW]: {
path: ROUTES.WORKSPACE_TAG_LIST_VIEW.route,
parse: {
order: (order: string) => decodeURIComponent(order),
},
},
[SCREENS.WORKSPACE.TAXES_SETTINGS]: {
path: ROUTES.WORKSPACE_TAXES_SETTINGS.route,
},
Expand Down
6 changes: 5 additions & 1 deletion src/libs/Navigation/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,13 @@ type SettingsNavigatorParamList = {
policyID: string;
tagName: string;
};
[SCREENS.WORKSPACE.TAG_LIST_VIEW]: {
policyID: string;
order: string;
};
[SCREENS.WORKSPACE.TAGS_EDIT]: {
policyID: string;
tagName: string;
order: string;
};
[SCREENS.WORKSPACE.TAG_EDIT]: {
policyID: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ function QuickbooksImportPage({policy}: WithPolicyProps) {
},
];

if (policy?.connections?.quickbooksOnline.data.country !== CONST.COUNTRY.US) {
if (policy?.connections?.quickbooksOnline?.data?.country !== CONST.COUNTRY.US) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change needed? TS does not report that these optional chaining operators are necessary.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah - problem here - that we have crash on this line - seem like TS types for data and country is not expecting to be undefined, but from API side it could :-)
just fyi
Screenshot 2024-05-01 at 11 59 53

Copy link
Contributor

@roryabraham roryabraham May 1, 2024

Choose a reason for hiding this comment

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

thanks for the context! Let's update the TS types for data and country to be optional then? to make sure that we're safely accessing them everywhere, not just here.

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 have updated the type for data to be optional. We need to do the same for the Connection object: both quickbooksOnline and xero are optional. Tried to make that change here but caused ts errors on updatePolicyConnectionConfig. Let's create a separate issue to handle that

sections.push({
description: translate('workspace.qbo.taxes'),
action: () => Navigation.navigate(ROUTES.POLICY_ACCOUNTING_QUICKBOOKS_ONLINE_TAXES.getRoute(policyID)),
Expand Down
4 changes: 2 additions & 2 deletions src/pages/workspace/tags/WorkspaceEditTagsPage.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type {StackScreenProps} from '@react-navigation/stack';
import React, {useCallback, useMemo} from 'react';
import React, {useCallback} from 'react';
import {View} from 'react-native';
import {withOnyx} from 'react-native-onyx';
import type {OnyxEntry} from 'react-native-onyx';
Expand Down Expand Up @@ -41,7 +41,7 @@ const validateTagName = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.POLICY_TAG
function WorkspaceEditTagsPage({route, policyTags}: WorkspaceEditTagsPageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const taglistName = useMemo(() => PolicyUtils.getTagLists(policyTags)[0].name, [policyTags]);
const taglistName = PolicyUtils.getTagListName(policyTags, +route.params.order);
s77rt marked this conversation as resolved.
Show resolved Hide resolved
const {inputCallbackRef} = useAutoFocusInput();

const updateTaglistName = useCallback(
Expand Down
83 changes: 50 additions & 33 deletions src/pages/workspace/tags/WorkspaceTagsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import * as Illustrations from '@components/Icon/Illustrations';
import ScreenWrapper from '@components/ScreenWrapper';
import SelectionList from '@components/SelectionList';
import RightElementEnabledStatus from '@components/SelectionList/RightElementEnabledStatus';
import RightElementRequiredStatus from '@components/SelectionList/RightElementRequiredStatus';
import TableListItem from '@components/SelectionList/TableListItem';
import type {ListItem} from '@components/SelectionList/types';
import Text from '@components/Text';
Expand Down Expand Up @@ -68,6 +69,8 @@ function WorkspaceTagsPage({route, policy}: WorkspaceTagsPageProps) {
const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`);
const {environmentURL} = useEnvironment();
const isConnectedToAccounting = Object.keys(policy?.connections ?? {}).length > 0;
const policyTagsLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]);
s77rt marked this conversation as resolved.
Show resolved Hide resolved
const isOnlyOneTagsLists = Object.keys(policyTagsLists).length === 1;
s77rt marked this conversation as resolved.
Show resolved Hide resolved

const fetchTags = useCallback(() => {
s77rt marked this conversation as resolved.
Show resolved Hide resolved
Policy.openPolicyTagsPage(policyID);
Expand All @@ -88,31 +91,39 @@ function WorkspaceTagsPage({route, policy}: WorkspaceTagsPageProps) {
setSelectedTags({});
}, [isFocused]);

// We currently don't support multi level tags, so let's only get the first level tags.
const policyTagLists = useMemo(() => PolicyUtils.getTagLists(policyTags).slice(0, 1), [policyTags]);
const tagList = useMemo<PolicyForList[]>(
() =>
policyTagLists
.map((policyTagList) =>
lodashSortBy(Object.values(policyTagList.tags || []), 'name', localeCompare).map((value) => {
const tag = value as OnyxCommon.OnyxValueWithOfflineFeedback<OnyxTypes.PolicyTag>;
const isDisabled = tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
return {
value: tag.name,
text: PolicyUtils.getCleanedTagName(tag.name),
keyForList: tag.name,
isSelected: !!selectedTags[tag.name],
pendingAction: tag.pendingAction,
errors: tag.errors ?? undefined,
enabled: tag.enabled,
isDisabled,
rightElement: <RightElementEnabledStatus enabled={tag.enabled} />,
};
}),
)
.flat(),
[policyTagLists, selectedTags],
);
const tagList = useMemo<PolicyForList[]>(() => {
s77rt marked this conversation as resolved.
Show resolved Hide resolved
const policyTagLists = isOnlyOneTagsLists ? policyTagsLists.slice(0, 1) : policyTagsLists;
s77rt marked this conversation as resolved.
Show resolved Hide resolved
if (!isOnlyOneTagsLists) {
return Object.values(policyTagLists).map((policyTagList) => ({
s77rt marked this conversation as resolved.
Show resolved Hide resolved
value: policyTagList.name,
text: PolicyUtils.getCleanedTagName(policyTagList.name),
keyForList: policyTagList.orderWeight.toString(),
isSelected: !!selectedTags[policyTagList.name],
roryabraham marked this conversation as resolved.
Show resolved Hide resolved
enabled: true,
required: policyTagList.required,
rightElement: <RightElementRequiredStatus required={policyTagList.required} />,
}));
}
return policyTagLists
.map((policyTagList) =>
lodashSortBy(Object.values(policyTagList.tags || []), 'name', localeCompare).map((value) => {
const tag = value as OnyxCommon.OnyxValueWithOfflineFeedback<OnyxTypes.PolicyTag>;
const isDisabled = tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE;
return {
value: tag.name,
text: PolicyUtils.getCleanedTagName(tag.name),
keyForList: tag.name,
isSelected: !!selectedTags[tag.name],
pendingAction: tag.pendingAction,
errors: tag.errors ?? undefined,
enabled: tag.enabled,
isDisabled,
rightElement: <RightElementEnabledStatus enabled={tag.enabled} />,
};
}),
)
.flat();
}, [isOnlyOneTagsLists, policyTagsLists, selectedTags]);

const tagListKeyedByName = tagList.reduce<Record<string, PolicyForList>>((acc, tag) => {
s77rt marked this conversation as resolved.
Show resolved Hide resolved
acc[tag.value] = tag;
Expand Down Expand Up @@ -147,6 +158,10 @@ function WorkspaceTagsPage({route, policy}: WorkspaceTagsPageProps) {
};

const navigateToTagSettings = (tag: PolicyOption) => {
if (!isOnlyOneTagsLists) {
Navigation.navigate(ROUTES.WORKSPACE_TAG_LIST_VIEW.getRoute(policyID, tag.keyForList));
return;
}
Navigation.navigate(ROUTES.WORKSPACE_TAG_SETTINGS.getRoute(policyID, tag.keyForList));
};

Expand Down Expand Up @@ -233,14 +248,16 @@ function WorkspaceTagsPage({route, policy}: WorkspaceTagsPageProps) {

return (
<View style={[styles.w100, styles.flexRow, isSmallScreenWidth && styles.mb3]}>
<Button
medium
success
onPress={navigateToCreateTagPage}
icon={Expensicons.Plus}
text={translate('workspace.tags.addTag')}
style={[styles.mr3, isSmallScreenWidth && styles.w50]}
/>
{isOnlyOneTagsLists && (
<Button
medium
success
onPress={navigateToCreateTagPage}
icon={Expensicons.Plus}
text={translate('workspace.tags.addTag')}
style={[styles.mr3, isSmallScreenWidth && styles.w50]}
/>
)}
{policyTags && (
<Button
medium
Expand Down
30 changes: 17 additions & 13 deletions src/pages/workspace/tags/WorkspaceTagsSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,16 @@ type WorkspaceTagsSettingsPageProps = WorkspaceTagsSettingsPageOnyxProps & Stack
function WorkspaceTagsSettingsPage({route, policyTags}: WorkspaceTagsSettingsPageProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const policyTagName = useMemo(() => PolicyUtils.getTagLists(policyTags)?.[0]?.name ?? '', [policyTags]);
const policyTagsLists = useMemo(() => PolicyUtils.getTagLists(policyTags), [policyTags]);
const isOnlyOneTagsLists = Object.keys(policyTagsLists).length === 1;
s77rt marked this conversation as resolved.
Show resolved Hide resolved

const updateWorkspaceRequiresTag = useCallback(
(value: boolean) => {
Policy.setPolicyRequiresTag(route.params.policyID, value);
},
[route.params.policyID],
);

return (
<AccessOrNotFoundWrapper
accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
Expand Down Expand Up @@ -69,18 +71,20 @@ function WorkspaceTagsSettingsPage({route, policyTags}: WorkspaceTagsSettingsPag
</View>
</View>
</OfflineWithFeedback>
<OfflineWithFeedback
errors={policyTags?.[policyTagName]?.errors}
pendingAction={policyTags?.[policyTagName]?.pendingAction}
errorRowStyles={styles.mh5}
>
<MenuItemWithTopDescription
title={policyTagName}
description={translate(`workspace.tags.customTagName`)}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_EDIT_TAGS.getRoute(route.params.policyID))}
shouldShowRightIcon
/>
</OfflineWithFeedback>
{isOnlyOneTagsLists && (
<OfflineWithFeedback
errors={policyTags?.[policyTagsLists[0].name]?.errors}
pendingAction={policyTags?.[policyTagsLists[0].name]?.pendingAction}
errorRowStyles={styles.mh5}
>
<MenuItemWithTopDescription
title={policyTagsLists[0].name}
description={translate(`workspace.tags.customTagName`)}
onPress={() => Navigation.navigate(ROUTES.WORKSPACE_EDIT_TAGS.getRoute(route.params.policyID, policyTagsLists[0].orderWeight.toString()))}
shouldShowRightIcon
/>
</OfflineWithFeedback>
)}
</View>
</ScreenWrapper>
)}
Expand Down
Loading
Loading