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

[No QA] [TS Migration] Standardize approach to Onyx pendingFields #34799

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
3d3aeaa
Create OfflineFeedback type
VickyStash Jan 19, 2024
3a42ec7
Use OfflineFeedback type instead of pendingAction
VickyStash Jan 19, 2024
4a1c14e
Fix lint errors
VickyStash Jan 19, 2024
284d119
Fix lint error in SidebarUtils
VickyStash Jan 19, 2024
3bce79c
Update PendingFields type and add OnyxItemWithOfflineFeedback type
VickyStash Jan 22, 2024
4aaad12
Use OfflineFeedback where possible
VickyStash Jan 23, 2024
abed5f1
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 23, 2024
25552df
Update PolicyReportField import
VickyStash Jan 23, 2024
60fe816
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 25, 2024
809707a
Update OnyxValueWithOfflineFeedback type and reuse it
VickyStash Jan 25, 2024
8682a4d
Remove extra export
VickyStash Jan 25, 2024
66808bf
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 26, 2024
aa6e0e3
Lint fix after conflicts resolution
VickyStash Jan 26, 2024
c028794
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 29, 2024
648e293
Lint fix
VickyStash Jan 29, 2024
a8e53ff
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 30, 2024
5886dcb
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Jan 31, 2024
97bba27
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 1, 2024
98925a5
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 6, 2024
187ffd9
TS fixes after merging main
VickyStash Feb 6, 2024
d559b2e
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 9, 2024
4578ca8
TS fix
VickyStash Feb 9, 2024
f0bb6ed
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 20, 2024
3b5834f
Minor code improvement
VickyStash Feb 21, 2024
939a88c
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 22, 2024
fc6c412
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 23, 2024
8163d2c
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 26, 2024
7d5e4e8
Fix TS error
VickyStash Feb 26, 2024
f52baac
Merge branch 'main' into ts-migration/standardize-pendingFields
VickyStash Feb 26, 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
2 changes: 1 addition & 1 deletion src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ function getOptionData({
result.isExpenseRequest = ReportUtils.isExpenseRequest(report);
result.isMoneyRequestReport = ReportUtils.isMoneyRequestReport(report);
result.shouldShowSubscript = ReportUtils.shouldReportShowSubscript(report);
result.pendingAction = report.pendingFields ? report.pendingFields.addWorkspaceRoom || report.pendingFields.createChat : undefined;
result.pendingAction = report.pendingFields ? report.pendingFields.addWorkspaceRoom ?? report.pendingFields.createChat : undefined;
VickyStash marked this conversation as resolved.
Show resolved Hide resolved
// @ts-expect-error TODO: Remove this once OptionsListUtils (https://github.com/Expensify/App/issues/24921) is migrated to TypeScript.
result.allReportErrors = OptionsListUtils.getAllReportErrors(report, reportActions);
result.brickRoadIndicator = hasErrors || hasViolations ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '';
Expand Down
7 changes: 2 additions & 5 deletions src/types/onyx/BankAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ type AdditionalData = {
country?: string;
};

type BankAccount = {
type BankAccount = OnyxCommon.OnyxValueWithOfflineFeedback<{
/** The bank account type */
accountType?: typeof CONST.PAYMENT_METHODS.PERSONAL_BANK_ACCOUNT;

Expand Down Expand Up @@ -40,10 +40,7 @@ type BankAccount = {

/** Any additional error message to show */
errors?: OnyxCommon.Errors;

/** Indicates the type of change made to the bank account that hasn't been synced with the server yet */
pendingAction?: OnyxCommon.PendingAction;
};
}>;

type BankAccountList = Record<string, BankAccount>;

Expand Down
5 changes: 2 additions & 3 deletions src/types/onyx/Fund.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ type AccountData = {
bank?: BankName;
};

type Fund = {
type Fund = OnyxCommon.OnyxValueWithOfflineFeedback<{
accountData?: AccountData;
accountType?: typeof CONST.PAYMENT_METHODS.DEBIT_CARD;
description?: string;
Expand All @@ -34,8 +34,7 @@ type Fund = {
title?: string;
isDefault?: boolean;
errors?: OnyxCommon.Errors;
pendingAction?: OnyxCommon.PendingAction;
};
}>;

type FundList = Record<string, Fund>;

Expand Down
30 changes: 15 additions & 15 deletions src/types/onyx/Login.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
import type * as OnyxCommon from './OnyxCommon';

type Login = {
/** Phone/Email associated with user */
partnerUserID?: string;
type Login = OnyxCommon.OnyxValueWithOfflineFeedback<
{
/** Phone/Email associated with user */
partnerUserID?: string;

/** Value of partner name */
partnerName?: string;
/** Value of partner name */
partnerName?: string;

/** Date login was validated, used to show info indicator status */
validatedDate?: string;
/** Date login was validated, used to show info indicator status */
validatedDate?: string;

/** Whether the user validation code was sent */
validateCodeSent?: boolean;
/** Whether the user validation code was sent */
validateCodeSent?: boolean;

/** Field-specific server side errors keyed by microtime */
errorFields?: OnyxCommon.ErrorFields;

/** Field-specific pending states for offline UI status */
pendingFields?: OnyxCommon.PendingFields;
};
/** Field-specific server side errors keyed by microtime */
errorFields?: OnyxCommon.ErrorFields;
},
'defaultLogin' | 'validateLogin' | 'addedLogin' | 'deletedLogin'
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm confused what the 2nd template argument for OnyxValueWithOfflineFeedback is for. Could you help explain? Maybe a README example here could be helpful too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2nd template argument for OnyxValueWithOfflineFeedback are additional keys to be provided as keys for pendingFields. So these keys 'defaultLogin' | 'validateLogin' | 'addedLogin' | 'deletedLogin' are outside of the Login model, but can be in pendingFields keys.

So for example, validateLogin can be used in pendingFields keys, but it doesn't exist in the Login model

pendingFields: {
validateLogin: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},

That's also was explained in this comment in 1st variant of implementation section.

Maybe it will be better to add a clarifying comment right next to the OnyxValueWithOfflineFeedback type, this way it should be easy to find and helpful.

Copy link
Contributor

Choose a reason for hiding this comment

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

@roryabraham Bump for reviewing

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for explaining

>;

type LoginList = Record<string, Login>;

Expand Down
16 changes: 13 additions & 3 deletions src/types/onyx/OnyxCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,19 @@ import type {ValueOf} from 'type-fest';
import type {AvatarSource} from '@libs/UserUtils';
import type CONST from '@src/CONST';

type PendingAction = ValueOf<typeof CONST.RED_BRICK_ROAD_PENDING_ACTION>;
type PendingAction = ValueOf<typeof CONST.RED_BRICK_ROAD_PENDING_ACTION> | null;

type PendingFields<TKey extends string = string> = Record<TKey, PendingAction | null | undefined>;
type PendingFields<TKey extends string> = {[key in Exclude<TKey, 'pendingAction' | 'pendingFields' | 'errorFields'>]?: PendingAction};

type OfflineFeedback<TKey extends string> = {
/** The type of action that's pending */
pendingAction?: PendingAction;

/** Field-specific pending states for offline updates */
pendingFields?: PendingFields<TKey>;
};

type OnyxValueWithOfflineFeedback<TOnyx, TKey extends string = never> = keyof TOnyx extends string ? TOnyx & OfflineFeedback<keyof TOnyx | TKey> : never;

type ErrorFields<TKey extends string = string> = Record<TKey, Errors | null | undefined>;

Expand All @@ -29,4 +39,4 @@ type Icon = {
fallbackIcon?: AvatarSource;
};

export type {Icon, PendingAction, PendingFields, ErrorFields, Errors, AvatarType};
export type {Icon, PendingAction, ErrorFields, Errors, AvatarType, OnyxValueWithOfflineFeedback};
7 changes: 2 additions & 5 deletions src/types/onyx/PersonalDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type Status = {
clearAfter: string; // ISO 8601 format;
};

type PersonalDetails = {
type PersonalDetails = OnyxCommon.OnyxValueWithOfflineFeedback<{
/** ID of the current user from their personal details */
accountID: number;

Expand Down Expand Up @@ -74,15 +74,12 @@ type PersonalDetails = {
/** Field-specific server side errors keyed by microtime */
errorFields?: OnyxCommon.ErrorFields<'avatar'>;

/** Field-specific pending states for offline UI status */
pendingFields?: OnyxCommon.PendingFields<'avatar' | 'originalFileName'>;

/** A fallback avatar icon to display when there is an error on loading avatar from remote URL. */
fallbackIcon?: string;

/** Status of the current user from their personal details */
status?: Status;
};
}>;

type PersonalDetailsList = Record<string, PersonalDetails | null>;

Expand Down
139 changes: 67 additions & 72 deletions src/types/onyx/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,124 +4,119 @@ import type * as OnyxCommon from './OnyxCommon';

type Unit = 'mi' | 'km';

type Rate = {
type Rate = OnyxCommon.OnyxValueWithOfflineFeedback<{
name?: string;
rate?: number;
currency?: string;
customUnitRateID?: string;
errors?: OnyxCommon.Errors;
pendingAction?: string;
};
}>;

type Attributes = {
unit: Unit;
};

type CustomUnit = {
type CustomUnit = OnyxCommon.OnyxValueWithOfflineFeedback<{
name: string;
customUnitID: string;
attributes: Attributes;
rates: Record<string, Rate>;
pendingAction?: string;
errors?: OnyxCommon.Errors;
};
}>;

type AutoReportingOffset = number | ValueOf<typeof CONST.POLICY.AUTO_REPORTING_OFFSET>;

type Policy = {
/** The ID of the policy */
id: string;
type Policy = OnyxCommon.OnyxValueWithOfflineFeedback<
{
/** The ID of the policy */
id: string;

/** The name of the policy */
name: string;
/** The name of the policy */
name: string;

/** The current user's role in the policy */
role: ValueOf<typeof CONST.POLICY.ROLE>;
/** The current user's role in the policy */
role: ValueOf<typeof CONST.POLICY.ROLE>;

/** The policy type */
type: ValueOf<typeof CONST.POLICY.TYPE>;
/** The policy type */
type: ValueOf<typeof CONST.POLICY.TYPE>;

/** The email of the policy owner */
owner: string;
/** The email of the policy owner */
owner: string;

/** The accountID of the policy owner */
ownerAccountID?: number;
/** The accountID of the policy owner */
ownerAccountID?: number;

/** The output currency for the policy */
outputCurrency: string;
/** The output currency for the policy */
outputCurrency: string;

/** The URL for the policy avatar */
avatar?: string;
/** The URL for the policy avatar */
avatar?: string;

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

/** Indicates the type of change made to the policy that hasn't been synced with the server yet */
pendingAction?: OnyxCommon.PendingAction;
/** A list of errors keyed by microtime */
errors?: OnyxCommon.Errors;

/** A list of errors keyed by microtime */
errors?: OnyxCommon.Errors;
/** Whether this policy was loaded from a policy summary, or loaded completely with all of its values */
isFromFullPolicy?: boolean;

/** Whether this policy was loaded from a policy summary, or loaded completely with all of its values */
isFromFullPolicy?: boolean;
/** When this policy was last modified */
lastModified?: string;

/** When this policy was last modified */
lastModified?: string;
/** The custom units data for this policy */
customUnits?: Record<string, CustomUnit>;

/** The custom units data for this policy */
customUnits?: Record<string, CustomUnit>;
/** Whether chat rooms can be created and used on this policy. Enabled manually by CQ/JS snippet. Always true for free policies. */
areChatRoomsEnabled: boolean;

/** Whether chat rooms can be created and used on this policy. Enabled manually by CQ/JS snippet. Always true for free policies. */
areChatRoomsEnabled: boolean;
/** Whether policy expense chats can be created and used on this policy. Enabled manually by CQ/JS snippet. Always true for free policies. */
isPolicyExpenseChatEnabled: boolean;

/** Whether policy expense chats can be created and used on this policy. Enabled manually by CQ/JS snippet. Always true for free policies. */
isPolicyExpenseChatEnabled: boolean;
/** Whether the auto reporting is enabled */
autoReporting?: boolean;

/** Whether the auto reporting is enabled */
autoReporting?: boolean;
/** The scheduled submit frequency set up on the this policy */
autoReportingFrequency?: ValueOf<typeof CONST.POLICY.AUTO_REPORTING_FREQUENCIES>;

/** The scheduled submit frequency set up on the this policy */
autoReportingFrequency?: ValueOf<typeof CONST.POLICY.AUTO_REPORTING_FREQUENCIES>;
/** Whether the scheduled submit is enabled */
isHarvestingEnabled?: boolean;

/** Whether the scheduled submit is enabled */
isHarvestingEnabled?: boolean;
/** Whether the scheduled submit is enabled */
isPreventSelfApprovalEnabled?: boolean;

/** Whether the scheduled submit is enabled */
isPreventSelfApprovalEnabled?: boolean;
/** When the monthly scheduled submit should happen */
autoReportingOffset?: AutoReportingOffset;

/** When the monthly scheduled submit should happen */
autoReportingOffset?: AutoReportingOffset;
/** The accountID of manager who the employee submits their expenses to on paid policies */
submitsTo?: number;

/** The accountID of manager who the employee submits their expenses to on paid policies */
submitsTo?: number;
/** The employee list of the policy */
employeeList?: [];

/** The employee list of the policy */
employeeList?: [];
/** The reimbursement choice for policy */
reimbursementChoice?: ValueOf<typeof CONST.POLICY.REIMBURSEMENT_CHOICES>;

/** The reimbursement choice for policy */
reimbursementChoice?: ValueOf<typeof CONST.POLICY.REIMBURSEMENT_CHOICES>;
/** The maximum report total allowed to trigger auto reimbursement. */
autoReimbursementLimit?: number;

/** The maximum report total allowed to trigger auto reimbursement. */
autoReimbursementLimit?: number;
/** Whether to leave the calling account as an admin on the policy */
makeMeAdmin?: boolean;

/** Whether to leave the calling account as an admin on the policy */
makeMeAdmin?: boolean;
/** Original file name which is used for the policy avatar */
originalFileName?: string;

/** Pending fields for the policy */
pendingFields?: Record<string, unknown>;
/** Alert message for the policy */
alertMessage?: string;

/** Original file name which is used for the policy avatar */
originalFileName?: string;
/** Informative messages about which policy members were added with primary logins when invited with their secondary login */
primaryLoginsInvited?: Record<string, string>;

/** Alert message for the policy */
alertMessage?: string;

/** Informative messages about which policy members were added with primary logins when invited with their secondary login */
primaryLoginsInvited?: Record<string, string>;

/** The approval mode set up on this policy */
approvalMode?: ValueOf<typeof CONST.POLICY.APPROVAL_MODE>;
};
/** The approval mode set up on this policy */
approvalMode?: ValueOf<typeof CONST.POLICY.APPROVAL_MODE>;
},
'generalSettings' | 'addWorkspaceRoom'
>;

export default Policy;

Expand Down
7 changes: 2 additions & 5 deletions src/types/onyx/PolicyMember.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type * as OnyxCommon from './OnyxCommon';

type PolicyMember = {
type PolicyMember = OnyxCommon.OnyxValueWithOfflineFeedback<{
/** Role of the user in the policy */
role?: string;

Expand All @@ -9,10 +9,7 @@ type PolicyMember = {
* {<timestamp>: 'error message', <timestamp2>: 'error message 2'}
*/
errors?: OnyxCommon.Errors;

/** Is this action pending? */
pendingAction?: OnyxCommon.PendingAction;
};
}>;

type PolicyMembers = Record<string, PolicyMember>;

Expand Down
6 changes: 2 additions & 4 deletions src/types/onyx/ReimbursementAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type ACHData = {
bankAccountID?: number;
};

type ReimbursementAccount = {
type ReimbursementAccount = OnyxCommon.OnyxValueWithOfflineFeedback<{
/** Whether we are loading the data via the API */
isLoading?: boolean;

Expand All @@ -44,9 +44,7 @@ type ReimbursementAccount = {

/** Draft step of the setup flow from Onyx */
draftStep?: BankAccountStep;

pendingAction?: OnyxCommon.PendingAction;
};
}>;

export default ReimbursementAccount;
export type {BankAccountStep, BankAccountSubStep};
Loading
Loading