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

Migrate NVPs to their new keys #37556

Merged
merged 24 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 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
61 changes: 36 additions & 25 deletions src/ONYXKEYS.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ const ONYXKEYS = {
/** Holds the reportID for the report between the user and their account manager */
ACCOUNT_MANAGER_REPORT_ID: 'accountManagerReportID',

/** Boolean flag only true when first set */
NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER: 'isFirstTimeNewExpensifyUser',

/** Holds an array of client IDs which is used for multi-tabs on web in order to know
* which tab is the leader, and which ones are the followers */
ACTIVE_CLIENTS: 'activeClients',
Expand Down Expand Up @@ -106,27 +103,52 @@ const ONYXKEYS = {
STASHED_SESSION: 'stashedSession',
BETAS: 'betas',

/** NVP keys
/** NVP keys */

/** Boolean flag only true when first set */
NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER: 'nvp_isFirstTimeNewExpensifyUser',
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is causing regression. Engagement modal doesn't show after signup

Copy link
Contributor

Choose a reason for hiding this comment

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

Screen.Recording.2024-03-14.at.10.13.27.pm.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch! I was missing renaming this constant in the backend. Sending a PR now but it needs to go out to prod before you can re-check.
Was this the only bug you found? Can you check the rest of things to see if you find anything else broken please?

Copy link
Contributor

Choose a reason for hiding this comment

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

No issues found on any other nvp keys. But still going to retest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait a second, going to push a 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.

Done


/** Contains the user preference for the LHN priority mode */
NVP_PRIORITY_MODE: 'nvp_priorityMode',

/** Contains the users's block expiration (if they have one) */
NVP_BLOCKED_FROM_CONCIERGE: 'private_blockedFromConcierge',
NVP_BLOCKED_FROM_CONCIERGE: 'nvp_private_blockedFromConcierge',
Copy link
Contributor

Choose a reason for hiding this comment

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

It would really be great to group all these together under an NVP key like:

ONYXKEYS: {
    NVP: {
        BLOCKED_FROM_CONCIERGE: 'nvp_private_blockedFromConcierge',
    },
};

Or if not that, at least put all the NVPs next to each other in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving them to be together mostly because I don't know how to make my IDE refactor this automatically and I don't want to have to update 420 files.


/** A unique identifier that each user has that's used to send notifications */
NVP_PRIVATE_PUSH_NOTIFICATION_ID: 'private_pushNotificationID',
NVP_PRIVATE_PUSH_NOTIFICATION_ID: 'nvp_private_pushNotificationID',

/** The NVP with the last payment method used per policy */
NVP_LAST_PAYMENT_METHOD: 'nvp_lastPaymentMethod',
NVP_LAST_PAYMENT_METHOD: 'nvp_private_lastPaymentMethod',

/** This NVP holds to most recent waypoints that a person has used when creating a distance request */
NVP_RECENT_WAYPOINTS: 'expensify_recentWaypoints',

/** This NVP will be `true` if the user has ever dismissed the engagement modal on either OldDot or NewDot. If it becomes true it should stay true forever. */
NVP_HAS_DISMISSED_IDLE_PANEL: 'hasDismissedIdlePanel',
NVP_HAS_DISMISSED_IDLE_PANEL: 'nvp_hasDismissedIdlePanel',

/** This NVP contains the choice that the user made on the engagement modal */
NVP_INTRO_SELECTED: 'introSelected',
NVP_INTRO_SELECTED: 'nvp_introSelected',

/** This NVP contains the active policyID */
NVP_ACTIVE_POLICY_ID: 'nvp_expensify_activePolicyID',

/** This NVP contains the referral banners the user dismissed */
NVP_DISMISSED_REFERRAL_BANNERS: 'nvp_dismissedReferralBanners',

/** Indicates which locale should be used */
NVP_PREFERRED_LOCALE: 'nvp_preferredLocale',

/** Whether the user has tried focus mode yet */
NVP_TRY_FOCUS_MODE: 'nvp_tryFocusMode',

/** Whether the user has been shown the hold educational interstitial yet */
NVP_HOLD_USE_EXPLAINED: 'holdUseExplained',
Copy link
Contributor

Choose a reason for hiding this comment

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

should this one be nvp_holdUseExplained

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I did not migrate this one.... I can't find this neither in web-e nor auth, I think this might be a broken feature? I see App sets it and reads it _in onyx), but I don't see anything setting it or loading it in the servers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we delete it?

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 am not sure honestly, this seems broken and kind of outside of the scope of this PR to fix it or remove it./


/** Store preferred skintone for emoji */
PREFERRED_EMOJI_SKIN_TONE: 'nvp_expensify_preferredEmojiSkinTone',

/** Store frequently used emojis for this user */
FREQUENTLY_USED_EMOJIS: 'nvp_expensify_frequentlyUsedEmojis',

/** The NVP with the last distance rate used per policy */
NVP_LAST_SELECTED_DISTANCE_RATES: 'lastSelectedDistanceRates',
Expand All @@ -153,9 +175,6 @@ const ONYXKEYS = {
ONFIDO_TOKEN: 'onfidoToken',
ONFIDO_APPLICANT_ID: 'onfidoApplicantID',

/** Indicates which locale should be used */
NVP_PREFERRED_LOCALE: 'preferredLocale',

/** User's Expensify Wallet */
USER_WALLET: 'userWallet',

Expand All @@ -177,12 +196,6 @@ const ONYXKEYS = {
/** The user's cash card and imported cards (including the Expensify Card) */
CARD_LIST: 'cardList',

/** Whether the user has tried focus mode yet */
NVP_TRY_FOCUS_MODE: 'tryFocusMode',

/** Whether the user has been shown the hold educational interstitial yet */
NVP_HOLD_USE_EXPLAINED: 'holdUseExplained',

/** Boolean flag used to display the focus mode notification */
FOCUS_MODE_NOTIFICATION: 'focusModeNotification',

Expand All @@ -195,12 +208,6 @@ const ONYXKEYS = {
/** Stores information about the active reimbursement account being set up */
REIMBURSEMENT_ACCOUNT: 'reimbursementAccount',

/** Store preferred skintone for emoji */
PREFERRED_EMOJI_SKIN_TONE: 'preferredEmojiSkinTone',

/** Store frequently used emojis for this user */
FREQUENTLY_USED_EMOJIS: 'frequentlyUsedEmojis',

/** Stores Workspace ID that will be tied to reimbursement account during setup */
REIMBURSEMENT_ACCOUNT_WORKSPACE_ID: 'reimbursementAccountWorkspaceID',

Expand Down Expand Up @@ -294,7 +301,8 @@ const ONYXKEYS = {
POLICY_CATEGORIES: 'policyCategories_',
POLICY_RECENTLY_USED_CATEGORIES: 'policyRecentlyUsedCategories_',
POLICY_TAGS: 'policyTags_',
POLICY_RECENTLY_USED_TAGS: 'policyRecentlyUsedTags_',
POLICY_RECENTLY_USED_TAGS: 'nvp_recentlyUsedTags_',
OLD_POLICY_RECENTLY_USED_TAGS: 'policyRecentlyUsedTags_',
POLICY_REPORT_FIELDS: 'policyReportFields_',
WORKSPACE_INVITE_MEMBERS_DRAFT: 'workspaceInviteMembersDraft_',
WORKSPACE_INVITE_MESSAGE_DRAFT: 'workspaceInviteMessageDraft_',
Expand Down Expand Up @@ -506,6 +514,7 @@ type OnyxCollectionValuesMapping = {
[ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS]: OnyxTypes.TransactionViolations;
[ONYXKEYS.COLLECTION.SPLIT_TRANSACTION_DRAFT]: OnyxTypes.Transaction;
[ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_TAGS]: OnyxTypes.RecentlyUsedTags;
[ONYXKEYS.COLLECTION.OLD_POLICY_RECENTLY_USED_TAGS]: OnyxTypes.RecentlyUsedTags;
[ONYXKEYS.COLLECTION.SELECTED_TAB]: string;
[ONYXKEYS.COLLECTION.PRIVATE_NOTES_DRAFT]: string;
[ONYXKEYS.COLLECTION.NEXT_STEP]: OnyxTypes.ReportNextStep;
Expand Down Expand Up @@ -561,6 +570,8 @@ type OnyxValuesMapping = {
[ONYXKEYS.ONFIDO_TOKEN]: string;
[ONYXKEYS.ONFIDO_APPLICANT_ID]: string;
[ONYXKEYS.NVP_PREFERRED_LOCALE]: OnyxTypes.Locale;
[ONYXKEYS.NVP_ACTIVE_POLICY_ID]: string;
[ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS]: OnyxTypes.DismissedReferralBanners;
[ONYXKEYS.USER_WALLET]: OnyxTypes.UserWallet;
[ONYXKEYS.WALLET_ONFIDO]: OnyxTypes.WalletOnfido;
[ONYXKEYS.WALLET_ADDITIONAL_DETAILS]: OnyxTypes.WalletAdditionalDetails;
Expand Down
10 changes: 5 additions & 5 deletions src/components/ReferralProgramCTA.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {withOnyx} from 'react-native-onyx';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
Expand All @@ -8,15 +9,15 @@ import CONST from '@src/CONST';
import Navigation from '@src/libs/Navigation/Navigation';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {DismissedReferralBanners} from '@src/types/onyx/Account';
import type * as OnyxTypes from '@src/types/onyx';
import Icon from './Icon';
import {Close} from './Icon/Expensicons';
import {PressableWithoutFeedback} from './Pressable';
import Text from './Text';
import Tooltip from './Tooltip';

type ReferralProgramCTAOnyxProps = {
dismissedReferralBanners: DismissedReferralBanners;
dismissedReferralBanners: OnyxEntry<OnyxTypes.DismissedReferralBanners>;
};

type ReferralProgramCTAProps = ReferralProgramCTAOnyxProps & {
Expand All @@ -36,7 +37,7 @@ function ReferralProgramCTA({referralContentType, dismissedReferralBanners}: Ref
User.dismissReferralBanner(referralContentType);
};

if (!referralContentType || dismissedReferralBanners[referralContentType]) {
if (!referralContentType || dismissedReferralBanners?.[referralContentType]) {
return null;
}

Expand Down Expand Up @@ -82,7 +83,6 @@ function ReferralProgramCTA({referralContentType, dismissedReferralBanners}: Ref

export default withOnyx<ReferralProgramCTAProps, ReferralProgramCTAOnyxProps>({
dismissedReferralBanners: {
key: ONYXKEYS.ACCOUNT,
selector: (data) => data?.dismissedReferralBanners ?? {},
key: ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS,
},
})(ReferralProgramCTA);
6 changes: 2 additions & 4 deletions src/libs/actions/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,11 +960,9 @@ function dismissReferralBanner(type: ValueOf<typeof CONST.REFERRAL_PROGRAM.CONTE
const optimisticData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: ONYXKEYS.ACCOUNT,
key: ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS,
value: {
dismissedReferralBanners: {
[type]: true,
},
[type]: true,
},
},
];
Expand Down
3 changes: 2 additions & 1 deletion src/libs/migrateOnyx.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Log from './Log';
import KeyReportActionsDraftByReportActionID from './migrations/KeyReportActionsDraftByReportActionID';
import NVPMigration from './migrations/NVPMigration';
import RemoveEmptyReportActionsDrafts from './migrations/RemoveEmptyReportActionsDrafts';
import RenameReceiptFilename from './migrations/RenameReceiptFilename';
import TransactionBackupsToCollection from './migrations/TransactionBackupsToCollection';
Expand All @@ -10,7 +11,7 @@ export default function (): Promise<void> {

return new Promise((resolve) => {
// Add all migrations to an array so they are executed in order
const migrationPromises = [RenameReceiptFilename, KeyReportActionsDraftByReportActionID, TransactionBackupsToCollection, RemoveEmptyReportActionsDrafts];
const migrationPromises = [RenameReceiptFilename, KeyReportActionsDraftByReportActionID, TransactionBackupsToCollection, RemoveEmptyReportActionsDrafts, NVPMigration];

// Reduce all promises down to a single promise. All promises run in a linear fashion, waiting for the
// previous promise to finish before moving onto the next one.
Expand Down
86 changes: 86 additions & 0 deletions src/libs/migrations/NVPMigration.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import after from 'lodash/after';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';

// These are the oldKeyName: newKeyName of the NVPs we can migrate without any processing
const migrations = {

This comment was marked as resolved.

// eslint-disable-next-line @typescript-eslint/naming-convention
nvp_lastPaymentMethod: ONYXKEYS.NVP_LAST_PAYMENT_METHOD,
isFirstTimeNewExpensifyUser: ONYXKEYS.NVP_IS_FIRST_TIME_NEW_EXPENSIFY_USER,
preferredLocale: ONYXKEYS.NVP_PREFERRED_LOCALE,
preferredEmojiSkinTone: ONYXKEYS.PREFERRED_EMOJI_SKIN_TONE,
frequentlyUsedEmojis: ONYXKEYS.FREQUENTLY_USED_EMOJIS,
// eslint-disable-next-line @typescript-eslint/naming-convention
private_blockedFromConcierge: ONYXKEYS.NVP_BLOCKED_FROM_CONCIERGE,
// eslint-disable-next-line @typescript-eslint/naming-convention
private_pushNotificationID: ONYXKEYS.NVP_PRIVATE_PUSH_NOTIFICATION_ID,
tryFocusMode: ONYXKEYS.NVP_TRY_FOCUS_MODE,
introSelected: ONYXKEYS.NVP_INTRO_SELECTED,
hasDismissedIdlePanel: ONYXKEYS.NVP_HAS_DISMISSED_IDLE_PANEL,
};

// This migration changes the keys of all the NVP related keys so that they are standardized
export default function () {
return new Promise<void>((resolve) => {
// Resolve the migration when all the keys have been migrated. The number of keys is the size of the `migrations` object in addition to the ACCOUNT and OLD_POLICY_RECENTLY_USED_TAGS keys (which is why there is a +2).
const resolveWhenDone = after(Object.entries(migrations).length + 2, () => resolve());

for (const [oldKey, newKey] of Object.entries(migrations)) {
const connectionID = Onyx.connect({
// @ts-expect-error oldKey is a variable
key: oldKey,
callback: (value) => {
Onyx.disconnect(connectionID);
if (value === null) {
resolveWhenDone();
return;
}
// @ts-expect-error These keys are variables, so we can't check the type
Onyx.multiSet({
[newKey]: value,
[oldKey]: null,
}).then(resolveWhenDone);
},
});
}
const connectionIDAccount = Onyx.connect({
key: ONYXKEYS.ACCOUNT,
callback: (value) => {
Onyx.disconnect(connectionIDAccount);
// @ts-expect-error we are removing this property, so it is not in the type anymore
if (!value?.activePolicyID) {
resolveWhenDone();
return;
}
// @ts-expect-error we are removing this property, so it is not in the type anymore
const activePolicyID = value.activePolicyID;
const newValue = {...value};
// @ts-expect-error we are removing this property, so it is not in the type anymore
delete newValue.activePolicyID;
Onyx.multiSet({
[ONYXKEYS.NVP_ACTIVE_POLICY_ID]: activePolicyID,
[ONYXKEYS.ACCOUNT]: newValue,
}).then(resolveWhenDone);
},
});
const connectionIDRecentlyUsedTags = Onyx.connect({
key: ONYXKEYS.COLLECTION.OLD_POLICY_RECENTLY_USED_TAGS,
waitForCollectionCallback: true,
callback: (value) => {
Onyx.disconnect(connectionIDRecentlyUsedTags);
if (!value) {

This comment was marked as resolved.

resolveWhenDone();
return;
}
const newValue = {};
for (const key of Object.keys(value)) {
// @ts-expect-error We have no fixed types here
newValue[`nvp_${key}`] = value[key];
// @ts-expect-error We have no fixed types here
newValue[key] = null;
}
Onyx.multiSet(newValue).then(resolveWhenDone);
},
});
});
}
8 changes: 3 additions & 5 deletions src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import * as Report from '@userActions/Report';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import type {DismissedReferralBanners} from '@src/types/onyx/Account';

type NewChatPageWithOnyxProps = {
/** All reports shared with the user */
Expand All @@ -34,7 +33,7 @@ type NewChatPageWithOnyxProps = {
betas: OnyxEntry<OnyxTypes.Beta[]>;

/** An object that holds data about which referral banners have been dismissed */
dismissedReferralBanners: DismissedReferralBanners;
dismissedReferralBanners: OnyxEntry<OnyxTypes.DismissedReferralBanners>;

/** Whether we are searching for reports in the server */
isSearchingForReports: OnyxEntry<boolean>;
Expand Down Expand Up @@ -265,7 +264,7 @@ function NewChatPage({betas, isGroupChat, personalDetails, reports, isSearchingF
shouldPreventDefaultFocusOnSelectRow={!DeviceCapabilities.canUseTouchScreen()}
shouldShowOptions={isOptionsDataReady && didScreenTransitionEnd}
shouldShowConfirmButton
shouldShowReferralCTA={!dismissedReferralBanners[CONST.REFERRAL_PROGRAM.CONTENT_TYPES.START_CHAT]}
shouldShowReferralCTA={!dismissedReferralBanners?.[CONST.REFERRAL_PROGRAM.CONTENT_TYPES.START_CHAT]}
referralContentType={CONST.REFERRAL_PROGRAM.CONTENT_TYPES.START_CHAT}
confirmButtonText={selectedOptions.length > 1 ? translate('newChatPage.createGroup') : translate('newChatPage.createChat')}
textInputAlert={isOffline ? [`${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}`, {isTranslated: true}] : ''}
Expand All @@ -287,8 +286,7 @@ NewChatPage.displayName = 'NewChatPage';

export default withOnyx<NewChatPageProps, NewChatPageWithOnyxProps>({
dismissedReferralBanners: {
key: ONYXKEYS.ACCOUNT,
selector: (data) => data?.dismissedReferralBanners ?? {},
key: ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS,
},
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,8 +360,7 @@ MoneyTemporaryForRefactorRequestParticipantsSelector.displayName = 'MoneyTempora

export default withOnyx({
dismissedReferralBanners: {
key: ONYXKEYS.ACCOUNT,
selector: (data) => data.dismissedReferralBanners || {},
key: ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS,
},
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -373,8 +373,7 @@ MoneyRequestParticipantsSelector.defaultProps = defaultProps;

export default withOnyx({
dismissedReferralBanners: {
key: ONYXKEYS.ACCOUNT,
selector: (data) => data.dismissedReferralBanners || {},
key: ONYXKEYS.NVP_DISMISSED_REFERRAL_BANNERS,
},
reports: {
key: ONYXKEYS.COLLECTION.REPORT,
Expand Down
7 changes: 3 additions & 4 deletions src/pages/workspace/WorkspaceNewRoomPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {NewRoomForm} from '@src/types/form/NewRoomForm';
import INPUT_IDS from '@src/types/form/NewRoomForm';
import type {Account, Policy, Report as ReportType, Session} from '@src/types/onyx';
import type {Policy, Report as ReportType, Session} from '@src/types/onyx';
import type * as OnyxCommon from '@src/types/onyx/OnyxCommon';
import {isEmptyObject} from '@src/types/utils/EmptyObject';

Expand All @@ -53,7 +53,7 @@ type WorkspaceNewRoomPageOnyxProps = {
session: OnyxEntry<Session>;

/** policyID for main workspace */
activePolicyID: OnyxEntry<Required<Account>['activePolicyID']>;
activePolicyID: OnyxEntry<Required<string>>;
};

type WorkspaceNewRoomPageProps = WorkspaceNewRoomPageOnyxProps;
Expand Down Expand Up @@ -343,8 +343,7 @@ export default withOnyx<WorkspaceNewRoomPageProps, WorkspaceNewRoomPageOnyxProps
key: ONYXKEYS.SESSION,
},
activePolicyID: {
key: ONYXKEYS.ACCOUNT,
selector: (account) => account?.activePolicyID ?? null,
key: ONYXKEYS.NVP_ACTIVE_POLICY_ID,
initialValue: null,
},
})(WorkspaceNewRoomPage);
Loading
Loading