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 a switch for requiring tags on the multi level tags RHP #42972

Merged
merged 45 commits into from
Jun 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
45 commits
Select commit Hold shift + click to select a range
59c508b
feat: pass a new parameter to the requires tags api
Jun 2, 2024
6a2c8ee
chore: add new constant
Jun 2, 2024
336c036
feat: add the toggle button
Jun 2, 2024
5bcd53b
chore: disable the switch if all tags are not active
Jun 2, 2024
95c360b
fix: remove unnecessary import
Jun 2, 2024
75b21eb
fix: UI copy and error message
Jun 2, 2024
39f7fac
fix: component position
Jun 2, 2024
6a03be3
chore: remove unused error util
Jun 2, 2024
e1246a8
fix: swap the API to call
Jun 2, 2024
da8ca41
fix: add back the command to make all tags required
Jun 2, 2024
ec97f63
chore: add back the API param type
Jun 2, 2024
18e3d27
chore: add back the API command
Jun 2, 2024
ffd3507
chore: export the parameter type
Jun 2, 2024
21c49fe
fix: add back the old API call
Jun 2, 2024
b477d14
fix: remove extra parameters
Jun 2, 2024
3a450df
fix: remove null
Jun 3, 2024
276f6e3
fix: errors null location
Jun 3, 2024
53a9514
chore: remove redundant variable
Jun 3, 2024
abcfc1e
chore: remove zero
Jun 3, 2024
8b777f1
chore: use pendingFields
Jun 3, 2024
e2e37f2
chore: remove a const that is not used
Jun 3, 2024
55cd7bb
don't lock required switch when the switch is on
Jun 3, 2024
3c18dd6
chore: add generic error
Jun 3, 2024
7e5a4db
chore: add error fields
Jun 5, 2024
aa2127a
chore: clear pending fields
Jun 5, 2024
cdd4b49
fix: clear pending field
Jun 5, 2024
144ed26
fix: display error from the error field
Jun 5, 2024
fbbca9d
clear the error
Jun 5, 2024
fe47bb7
use action instead of fields
Jun 5, 2024
4a5851d
clear fields
Jun 5, 2024
1f083ef
clear the required field
Jun 5, 2024
9752216
fix: clear specific error field
Jun 5, 2024
86685bd
chore: define a function to clear a field error from policy tag list
Jun 5, 2024
b4422fb
refactor: get orderWeight instead of the tag name
Jun 5, 2024
4fc37fb
fix: the function parameter
Jun 5, 2024
7229be7
chore: make clearPolicyTagErrors take tag list index
Jun 6, 2024
70f7c6b
fix: use orderWeight from route params
Jun 6, 2024
1ca0837
chore: pass missing argument to the clear error function
Jun 6, 2024
327cc7d
Merge branch 'main' into hayata-add-requires-tags-button
Jun 6, 2024
83bca4b
fix: pass the missing argument to the function to clear the error
Jun 6, 2024
d6f5800
Merge branch 'main' into hayata-add-requires-tags-button
Jun 7, 2024
cdd4365
only clear policy tag error when the items renders are policy tags
Jun 7, 2024
050471f
fix: lint issue
Jun 7, 2024
bef9b95
return of policy tag name is undefined
Jun 8, 2024
9b97a25
Merge branch 'main' of github.com:Expensify/App into hayata-add-requi…
aldo-expensify Jun 12, 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
7 changes: 7 additions & 0 deletions src/libs/API/parameters/SetPolicyTagsRequired.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
type SetPolicyTagsRequired = {
policyID: string;
tagListIndex: number;
requireTagList: boolean;
};

export default SetPolicyTagsRequired;
1 change: 1 addition & 0 deletions src/libs/API/parameters/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export type {default as SetWorkspaceApprovalModeParams} from './SetWorkspaceAppr
export type {default as SetWorkspacePayerParams} from './SetWorkspacePayerParams';
export type {default as SetWorkspaceReimbursementParams} from './SetWorkspaceReimbursementParams';
export type {default as SetPolicyRequiresTag} from './SetPolicyRequiresTag';
export type {default as SetPolicyTagsRequired} from './SetPolicyTagsRequired';
export type {default as RenamePolicyTaglistParams} from './RenamePolicyTaglistParams';
export type {default as SwitchToOldDotParams} from './SwitchToOldDotParams';
export type {default as TrackExpenseParams} from './TrackExpenseParams';
Expand Down
2 changes: 2 additions & 0 deletions src/libs/API/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ const WRITE_COMMANDS = {
RENAME_POLICY_TAG: 'RenamePolicyTag',
SET_WORKSPACE_REQUIRES_CATEGORY: 'SetWorkspaceRequiresCategory',
DELETE_WORKSPACE_CATEGORIES: 'DeleteWorkspaceCategories',
SET_POLICY_TAGS_REQUIRED: 'SetPolicyTagsRequired',
SET_POLICY_REQUIRES_TAG: 'SetPolicyRequiresTag',
RENAME_POLICY_TAG_LIST: 'RenamePolicyTaglist',
DELETE_POLICY_TAGS: 'DeletePolicyTags',
Expand Down Expand Up @@ -345,6 +346,7 @@ type WriteCommandParameters = {
[WRITE_COMMANDS.SET_WORKSPACE_REQUIRES_CATEGORY]: Parameters.SetWorkspaceRequiresCategoryParams;
[WRITE_COMMANDS.DELETE_WORKSPACE_CATEGORIES]: Parameters.DeleteWorkspaceCategoriesParams;
[WRITE_COMMANDS.SET_POLICY_REQUIRES_TAG]: Parameters.SetPolicyRequiresTag;
[WRITE_COMMANDS.SET_POLICY_TAGS_REQUIRED]: Parameters.SetPolicyTagsRequired;
[WRITE_COMMANDS.RENAME_POLICY_TAG_LIST]: Parameters.RenamePolicyTaglistParams;
[WRITE_COMMANDS.CREATE_POLICY_TAG]: Parameters.CreatePolicyTagsParams;
[WRITE_COMMANDS.RENAME_POLICY_TAG]: Parameters.RenamePolicyTagsParams;
Expand Down
84 changes: 80 additions & 4 deletions src/libs/actions/Policy/Tag.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {NullishDeep, OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import * as API from '@libs/API';
import type {EnablePolicyTagsParams, OpenPolicyTagsPageParams, RenamePolicyTaglistParams, RenamePolicyTagsParams, SetPolicyTagsEnabled} from '@libs/API/parameters';
import type {EnablePolicyTagsParams, OpenPolicyTagsPageParams, RenamePolicyTaglistParams, RenamePolicyTagsParams, SetPolicyTagsEnabled, SetPolicyTagsRequired} from '@libs/API/parameters';
import {READ_COMMANDS, WRITE_COMMANDS} from '@libs/API/types';
import * as ErrorUtils from '@libs/ErrorUtils';
import getIsNarrowLayout from '@libs/getIsNarrowLayout';
Expand Down Expand Up @@ -161,6 +161,7 @@ function createPolicyTag(policyID: string, tagName: string) {
tags: {
[newTagName]: {
errors: ErrorUtils.getMicroSecondOnyxError('workspace.tags.genericFailureMessage'),
pendingAction: null,
},
},
},
Expand Down Expand Up @@ -329,8 +330,8 @@ function deletePolicyTags(policyID: string, tagsToDelete: string[]) {
API.write(WRITE_COMMANDS.DELETE_POLICY_TAGS, parameters, onyxData);
}

function clearPolicyTagErrors(policyID: string, tagName: string) {
const tagListName = Object.keys(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})[0];
function clearPolicyTagErrors(policyID: string, tagName: string, tagListIndex: number) {
const tagListName = Object.keys(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})[tagListIndex];
const tag = allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`]?.[tagListName].tags?.[tagName];
if (!tag) {
return;
Expand Down Expand Up @@ -359,10 +360,25 @@ function clearPolicyTagErrors(policyID: string, tagName: string) {
});
}

function clearPolicyTagListError(policyID: string, tagListIndex: number, errorField: string) {
const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.[tagListIndex] ?? {};

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
if (!policyTag.name) {
return;
}

The line above makes it looks like policyTag could end up being {}, what happens then if policyTag.name is undefined? I think we should avoid calling Onyx.merge in such case.

if policyTag is not expected to end up as {}, then I think we shouldn't have ?? {}.

if (!policyTag.name) {
return;
}

Onyx.merge(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, {
[policyTag.name]: {
errorFields: {
[errorField]: null,
},
},
});
}

function renamePolicyTag(policyID: string, policyTag: {oldName: string; newName: string}, tagListIndex: number) {
const tagList = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.[tagListIndex] ?? {};
const tag = tagList.tags?.[policyTag.oldName];

const oldTagName = policyTag.oldName;
const newTagName = PolicyUtils.escapeTagName(policyTag.newName);
const onyxData: OnyxData = {
Expand Down Expand Up @@ -611,15 +627,75 @@ function setPolicyRequiresTag(policyID: string, requiresTag: boolean) {
API.write(WRITE_COMMANDS.SET_POLICY_REQUIRES_TAG, parameters, onyxData);
}

function setPolicyTagsRequired(policyID: string, requiresTag: boolean, tagListIndex: number) {
const policyTag = PolicyUtils.getTagLists(allPolicyTags?.[`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`] ?? {})?.[tagListIndex] ?? {};
aldo-expensify marked this conversation as resolved.
Show resolved Hide resolved

if (!policyTag.name) {
return;
}

const onyxData: OnyxData = {
optimisticData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
value: {
[policyTag.name]: {
required: requiresTag,
pendingFields: {required: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE},
errorFields: {required: null},
},
},
},
],
successData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
value: {
[policyTag.name]: {
pendingFields: {required: null},
},
},
},
],
failureData: [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`,
value: {
[policyTag.name]: {
required: policyTag.required,
hayata-suenaga marked this conversation as resolved.
Show resolved Hide resolved
pendingFields: {required: null},
errorFields: {
required: ErrorUtils.getMicroSecondOnyxError('workspace.tags.genericFailureMessage'),
},
},
},
},
],
};

const parameters: SetPolicyTagsRequired = {
policyID,
tagListIndex,
requireTagList: requiresTag,
};

API.write(WRITE_COMMANDS.SET_POLICY_TAGS_REQUIRED, parameters, onyxData);
}

export {
openPolicyTagsPage,
buildOptimisticPolicyRecentlyUsedTags,
setPolicyRequiresTag,
setPolicyTagsRequired,
renamePolicyTaglist,
enablePolicyTags,
createPolicyTag,
renamePolicyTag,
clearPolicyTagErrors,
clearPolicyTagListError,
deletePolicyTags,
setWorkspaceTagEnabled,
};
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/tags/TagSettingsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function TagSettingsPage({route, policyTags, navigation}: TagSettingsPageProps)
errors={ErrorUtils.getLatestErrorMessageField(currentPolicyTag)}
pendingAction={currentPolicyTag.pendingFields?.enabled}
errorRowStyles={styles.mh5}
onClose={() => Tag.clearPolicyTagErrors(route.params.policyID, route.params.tagName)}
onClose={() => Tag.clearPolicyTagErrors(route.params.policyID, route.params.tagName, route.params.orderWeight)}
>
<View style={[styles.mt2, styles.mh5]}>
<View style={[styles.flexRow, styles.mb5, styles.mr2, styles.alignItemsCenter, styles.justifyContentBetween]}>
Expand Down
2 changes: 1 addition & 1 deletion src/pages/workspace/tags/WorkspaceTagsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ function WorkspaceTagsPage({route}: WorkspaceTagsPageProps) {
customListHeader={getCustomListHeader()}
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]}
onDismissError={(item) => Tag.clearPolicyTagErrors(policyID, item.value)}
onDismissError={(item) => !isMultiLevelTags && Tag.clearPolicyTagErrors(policyID, item.value, 0)}
listHeaderContent={isSmallScreenWidth ? getHeaderText() : null}
showScrollIndicator={false}
/>
Expand Down
22 changes: 18 additions & 4 deletions src/pages/workspace/tags/WorkspaceViewTagsPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import * as PolicyUtils from '@libs/PolicyUtils';
import type {SettingsNavigatorParamList} from '@navigation/types';
import NotFoundPage from '@pages/ErrorPage/NotFoundPage';
import AccessOrNotFoundWrapper from '@pages/workspace/AccessOrNotFoundWrapper';
import ToggleSettingOptionRow from '@pages/workspace/workflows/ToggleSettingsOptionRow';
import * as Tag from '@userActions/Policy/Tag';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -65,10 +66,9 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) {
setSelectedTags({});
}, [isFocused]);

const policyTagList = useMemo(() => PolicyUtils.getTagLists(policyTags).find((policyTag) => policyTag.name === currentTagListName), [currentTagListName, policyTags]);
const tagList = useMemo<TagListItem[]>(
() =>
Object.values(policyTagList?.tags ?? {})
Object.values(currentPolicyTag?.tags ?? {})
.sort((tagA, tagB) => localeCompare(tagA.name, tagB.name))
.map((tag) => ({
value: tag.name,
Expand All @@ -81,7 +81,7 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) {
isDisabled: tag.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,
rightElement: <ListItemRightCaretWithLabel labelText={tag.enabled ? translate('workspace.common.enabled') : translate('workspace.common.disabled')} />,
})),
[policyTagList, selectedTags, translate],
[currentPolicyTag, selectedTags, translate],
);

const tagListKeyedByName = useMemo(
Expand Down Expand Up @@ -234,6 +234,18 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) {
cancelText={translate('common.cancel')}
danger
/>
<View style={[styles.pv4, styles.ph5]}>
<ToggleSettingOptionRow
Copy link
Contributor

Choose a reason for hiding this comment

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

Coming from this issue #43758, we should have hide the required option for dependent tags, because dependent tag lists cannot be required. More context #43758 (comment) #43758 (comment)

title={translate('common.required')}
switchAccessibilityLabel={translate('common.required')}
isActive={!!currentPolicyTag?.required}
onToggle={(on) => Tag.setPolicyTagsRequired(policyID, on, route.params.orderWeight)}
pendingAction={currentPolicyTag.pendingFields?.required}
errors={currentPolicyTag?.errorFields?.required ?? undefined}
onCloseError={() => Tag.clearPolicyTagListError(policyID, route.params.orderWeight, 'required')}
disabled={!currentPolicyTag?.required && !Object.values(currentPolicyTag?.tags ?? {}).some((tag) => tag.enabled)}
/>
</View>
<OfflineWithFeedback
errors={currentPolicyTag.errors}
pendingAction={currentPolicyTag.pendingAction}
Expand Down Expand Up @@ -264,7 +276,9 @@ function WorkspaceViewTagsPage({route}: WorkspaceViewTagsProps) {
customListHeader={getCustomListHeader()}
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
listHeaderWrapperStyle={[styles.ph9, styles.pv3, styles.pb5]}
onDismissError={(item) => Tag.clearPolicyTagErrors(policyID, item.value)}
onDismissError={(item) => {
Tag.clearPolicyTagErrors(policyID, item.value, route.params.orderWeight);
}}
/>
)}
</ScreenWrapper>
Expand Down
3 changes: 3 additions & 0 deletions src/types/onyx/PolicyTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ type PolicyTagList<T extends string = string> = Record<

/** A list of errors keyed by microtime */
errors?: OnyxCommon.Errors;

/** Error objects keyed by field name containing errors keyed by microtime */
errorFields?: OnyxCommon.ErrorFields;
}>
>;

Expand Down
Loading