-
Notifications
You must be signed in to change notification settings - Fork 4k
Remove the manual distance beta #71474
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
Changes from all commits
03c183d
b75888e
9e38504
3441bdc
e08f406
1cdfd57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ import type {ValueOf} from 'type-fest'; | |
| import useLocalize from '@hooks/useLocalize'; | ||
| import useNetwork from '@hooks/useNetwork'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import usePermissions from '@hooks/usePermissions'; | ||
| import usePrevious from '@hooks/usePrevious'; | ||
| import useThemeStyles from '@hooks/useThemeStyles'; | ||
| import {convertToDisplayString} from '@libs/CurrencyUtils'; | ||
|
|
@@ -274,8 +273,6 @@ function MoneyRequestConfirmationListFooter({ | |
| const styles = useThemeStyles(); | ||
| const {translate, toLocaleDigit, localeCompare} = useLocalize(); | ||
| const {isOffline} = useNetwork(); | ||
| const {isBetaEnabled} = usePermissions(); | ||
| const isManualDistanceEnabled = isBetaEnabled(CONST.BETAS.MANUAL_DISTANCE); | ||
|
|
||
| const [allPolicies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {canBeMissing: true}); | ||
| const [allReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: true}); | ||
|
|
@@ -505,7 +502,7 @@ function MoneyRequestConfirmationListFooter({ | |
| item: ( | ||
| <MenuItemWithTopDescription | ||
| key={translate('common.rate')} | ||
| shouldShowRightIcon={!!rate && !isReadOnly && (isPolicyExpenseChat || isManualDistanceEnabled)} | ||
| shouldShowRightIcon={!!rate && !isReadOnly} | ||
| title={DistanceRequestUtils.getRateForDisplay(unit, rate, currency, translate, toLocaleDigit, isOffline)} | ||
| description={translate('common.rate')} | ||
| style={[styles.moneyRequestMenuItem]} | ||
|
|
@@ -515,7 +512,7 @@ function MoneyRequestConfirmationListFooter({ | |
| return; | ||
| } | ||
|
|
||
| if (isManualDistanceEnabled && !isPolicyExpenseChat) { | ||
| if (!isPolicyExpenseChat) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ PERF-6The conditional logic here has been simplified by removing the Consider adding a comment explaining why this specific navigation path is taken when not in a policy expense chat context. |
||
| Navigation.navigate( | ||
| ROUTES.MONEY_REQUEST_UPGRADE.getRoute({ | ||
| action, | ||
|
|
@@ -533,7 +530,7 @@ function MoneyRequestConfirmationListFooter({ | |
| }} | ||
| brickRoadIndicator={shouldDisplayDistanceRateError ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined} | ||
| disabled={didConfirm} | ||
| interactive={!!rate && !isReadOnly && (isPolicyExpenseChat || isManualDistanceEnabled)} | ||
| interactive={!!rate && !isReadOnly} | ||
| /> | ||
| ), | ||
| shouldShow: isDistanceRequest, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,7 +114,7 @@ | |
|
|
||
| let allTransactions: OnyxCollection<Transaction> = {}; | ||
|
|
||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -126,7 +126,7 @@ | |
| }); | ||
|
|
||
| let allReports: OnyxCollection<Report> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.REPORT, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => { | ||
|
|
@@ -135,7 +135,7 @@ | |
| }); | ||
|
|
||
| let allTransactionViolations: OnyxCollection<TransactionViolations> = {}; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS, | ||
| waitForCollectionCallback: true, | ||
| callback: (value) => (allTransactionViolations = value), | ||
|
|
@@ -143,7 +143,7 @@ | |
|
|
||
| let currentUserEmail = ''; | ||
| let currentUserAccountID = -1; | ||
| Onyx.connect({ | ||
| key: ONYXKEYS.SESSION, | ||
| callback: (val) => { | ||
| currentUserEmail = val?.email ?? ''; | ||
|
|
@@ -216,22 +216,16 @@ | |
| return isCardTransaction(transaction) && transaction?.comment?.liabilityType === CONST.TRANSACTION.LIABILITY_TYPE.RESTRICT; | ||
| } | ||
|
|
||
| function getRequestType(transaction: OnyxEntry<Transaction>, isManualDistanceEnabled?: boolean): IOURequestType { | ||
| if (isManualDistanceEnabled) { | ||
| if (isManualDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; | ||
| } | ||
| if (isMapDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } | ||
| function getRequestType(transaction: OnyxEntry<Transaction>): IOURequestType { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❌ PERF-2The function signature has been simplified by removing the Current implementation already follows good practices by checking |
||
| if (isManualDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MANUAL; | ||
| } | ||
| if (isDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE; | ||
| if (isMapDistanceRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } | ||
| if (isScanRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.SCAN; | ||
| } | ||
|
|
||
| if (isPerDiemRequest(transaction)) { | ||
| return CONST.IOU.REQUEST_TYPE.PER_DIEM; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,15 +11,10 @@ type NavigateToQuickActionParams = { | |
| isValidReport: boolean; | ||
| quickAction: QuickAction; | ||
| selectOption: (onSelected: () => void, shouldRestrictAction: boolean) => void; | ||
| isManualDistanceTrackingEnabled?: boolean; | ||
| lastDistanceExpenseType?: DistanceExpenseType; | ||
| }; | ||
|
|
||
| function getQuickActionRequestType( | ||
| action: QuickActionName | undefined, | ||
| lastDistanceExpenseType?: DistanceExpenseType, | ||
| isManualDistanceTrackingEnabled?: boolean, | ||
| ): IOURequestType | undefined { | ||
| function getQuickActionRequestType(action: QuickActionName | undefined, lastDistanceExpenseType?: DistanceExpenseType): IOURequestType | undefined { | ||
| if (!action) { | ||
| return; | ||
| } | ||
|
|
@@ -30,11 +25,7 @@ function getQuickActionRequestType( | |
| } else if ([CONST.QUICK_ACTIONS.REQUEST_SCAN, CONST.QUICK_ACTIONS.SPLIT_SCAN, CONST.QUICK_ACTIONS.TRACK_SCAN].some((a) => a === action)) { | ||
| requestType = CONST.IOU.REQUEST_TYPE.SCAN; | ||
| } else if ([CONST.QUICK_ACTIONS.REQUEST_DISTANCE, CONST.QUICK_ACTIONS.SPLIT_DISTANCE, CONST.QUICK_ACTIONS.TRACK_DISTANCE].some((a) => a === action)) { | ||
| if (isManualDistanceTrackingEnabled) { | ||
| requestType = lastDistanceExpenseType ?? CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } else { | ||
| requestType = CONST.IOU.REQUEST_TYPE.DISTANCE; | ||
| } | ||
| requestType = lastDistanceExpenseType ?? CONST.IOU.REQUEST_TYPE.DISTANCE_MAP; | ||
| } else if (action === CONST.QUICK_ACTIONS.PER_DIEM) { | ||
| requestType = CONST.IOU.REQUEST_TYPE.PER_DIEM; | ||
| } | ||
|
|
@@ -43,9 +34,9 @@ function getQuickActionRequestType( | |
| } | ||
|
|
||
| function navigateToQuickAction(params: NavigateToQuickActionParams) { | ||
| const {isValidReport, quickAction, selectOption, isManualDistanceTrackingEnabled, lastDistanceExpenseType} = params; | ||
| const {isValidReport, quickAction, selectOption, lastDistanceExpenseType} = params; | ||
| const reportID = isValidReport && quickAction?.chatReportID ? quickAction?.chatReportID : generateReportID(); | ||
| const requestType = getQuickActionRequestType(quickAction?.action, lastDistanceExpenseType, isManualDistanceTrackingEnabled); | ||
| const requestType = getQuickActionRequestType(quickAction?.action, lastDistanceExpenseType); | ||
|
|
||
| switch (quickAction?.action) { | ||
| case CONST.QUICK_ACTIONS.REQUEST_MANUAL: | ||
|
|
@@ -69,18 +60,10 @@ function navigateToQuickAction(params: NavigateToQuickActionParams) { | |
| selectOption(() => startMoneyRequest(CONST.IOU.TYPE.TRACK, reportID, requestType, true), false); | ||
| break; | ||
| case CONST.QUICK_ACTIONS.REQUEST_DISTANCE: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Code SimplificationGood simplification! The removal of the This change eliminates the complexity of having two different code paths based on the beta flag and ensures consistent behavior for distance request handling. |
||
| if (isManualDistanceTrackingEnabled) { | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.SUBMIT, reportID, requestType, true), false); | ||
| return; | ||
| } | ||
| selectOption(() => startMoneyRequest(CONST.IOU.TYPE.SUBMIT, reportID, requestType, true), true); | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.SUBMIT, reportID, requestType, true), false); | ||
| break; | ||
| case CONST.QUICK_ACTIONS.TRACK_DISTANCE: | ||
| if (isManualDistanceTrackingEnabled) { | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.TRACK, reportID, requestType, true), false); | ||
| return; | ||
| } | ||
| selectOption(() => startMoneyRequest(CONST.IOU.TYPE.TRACK, reportID, requestType, true), false); | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.TRACK, reportID, requestType, true), false); | ||
| break; | ||
| default: | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,7 +14,6 @@ import Tooltip from '@components/Tooltip/PopoverAnchorTooltip'; | |
| import useEnvironment from '@hooks/useEnvironment'; | ||
| import useLocalize from '@hooks/useLocalize'; | ||
| import useOnyx from '@hooks/useOnyx'; | ||
| import usePermissions from '@hooks/usePermissions'; | ||
| import usePrevious from '@hooks/usePrevious'; | ||
| import useReportIsArchived from '@hooks/useReportIsArchived'; | ||
| import useResponsiveLayout from '@hooks/useResponsiveLayout'; | ||
|
|
@@ -131,12 +130,9 @@ function AttachmentPickerWithMenuItems({ | |
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`, {canBeMissing: true}); | ||
| const [lastDistanceExpenseType] = useOnyx(ONYXKEYS.NVP_LAST_DISTANCE_EXPENSE_TYPE, {canBeMissing: true}); | ||
| const {isProduction} = useEnvironment(); | ||
| const {isBetaEnabled} = usePermissions(); | ||
| const {setIsLoaderVisible} = useFullScreenLoader(); | ||
| const isReportArchived = useReportIsArchived(report?.reportID); | ||
|
|
||
| const isManualDistanceTrackingEnabled = isBetaEnabled(CONST.BETAS.MANUAL_DISTANCE); | ||
|
|
||
| const selectOption = useCallback( | ||
| (onSelected: () => void, shouldRestrictAction: boolean) => { | ||
| if (shouldRestrictAction && policy && shouldRestrictUserBillableActions(policy.id)) { | ||
|
|
@@ -172,17 +168,12 @@ function AttachmentPickerWithMenuItems({ | |
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => selectOption(() => startMoneyRequest(CONST.IOU.TYPE.SUBMIT, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID)), true), | ||
| }, | ||
| ...(isManualDistanceTrackingEnabled | ||
| ? [ | ||
| { | ||
| icon: Expensicons.Location, | ||
| text: translate('quickAction.recordDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.SUBMIT, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), lastDistanceExpenseType), true), | ||
| }, | ||
| ] | ||
| : []), | ||
| { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Code SimplificationExcellent refactoring! The removal of the conditional array spread based on The distance tracking menu item is now always available, which provides a more consistent user experience and eliminates the need for runtime beta flag checks. |
||
| icon: Expensicons.Location, | ||
| text: translate('quickAction.recordDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => selectOption(() => startDistanceRequest(CONST.IOU.TYPE.SUBMIT, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), lastDistanceExpenseType), true), | ||
| }, | ||
| ], | ||
| [CONST.IOU.TYPE.PAY]: [ | ||
| { | ||
|
|
@@ -207,17 +198,12 @@ function AttachmentPickerWithMenuItems({ | |
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => selectOption(() => startMoneyRequest(CONST.IOU.TYPE.TRACK, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID)), true), | ||
| }, | ||
| ...(isManualDistanceTrackingEnabled | ||
| ? [ | ||
| { | ||
| icon: Expensicons.Location, | ||
| text: translate('iou.trackDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => | ||
| selectOption(() => startDistanceRequest(CONST.IOU.TYPE.TRACK, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), lastDistanceExpenseType), true), | ||
| }, | ||
| ] | ||
| : []), | ||
| { | ||
| icon: Expensicons.Location, | ||
| text: translate('iou.trackDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => selectOption(() => startDistanceRequest(CONST.IOU.TYPE.TRACK, report?.reportID ?? String(CONST.DEFAULT_NUMBER_ID), lastDistanceExpenseType), true), | ||
| }, | ||
| ], | ||
| [CONST.IOU.TYPE.INVOICE]: [ | ||
| { | ||
|
|
@@ -241,7 +227,6 @@ function AttachmentPickerWithMenuItems({ | |
| selectOption, | ||
| isDelegateAccessRestricted, | ||
| showDelegateNoAccessModal, | ||
| isManualDistanceTrackingEnabled, | ||
| isReportArchived, | ||
| lastDistanceExpenseType, | ||
| ]); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -135,7 +135,6 @@ function FloatingActionButtonAndPopover({onHideCreateMenu, onShowCreateMenu, ref | |
| const {isOffline} = useNetwork(); | ||
| const {isBetaEnabled} = usePermissions(); | ||
| const isBlockedFromSpotnanaTravel = isBetaEnabled(CONST.BETAS.PREVENT_SPOTNANA_TRAVEL); | ||
| const isManualDistanceTrackingEnabled = isBetaEnabled(CONST.BETAS.MANUAL_DISTANCE); | ||
| const [primaryLogin] = useOnyx(ONYXKEYS.ACCOUNT, {selector: accountPrimaryLoginSelector, canBeMissing: true}); | ||
| const primaryContactMethod = primaryLogin ?? session?.email ?? ''; | ||
| const [travelSettings] = useOnyx(ONYXKEYS.NVP_TRAVEL_SETTINGS, {canBeMissing: true}); | ||
|
|
@@ -353,7 +352,7 @@ function FloatingActionButtonAndPopover({onHideCreateMenu, onShowCreateMenu, ref | |
| showDelegateNoAccessModal(); | ||
| return; | ||
| } | ||
| navigateToQuickAction({isValidReport, quickAction, selectOption, isManualDistanceTrackingEnabled, lastDistanceExpenseType}); | ||
| navigateToQuickAction({isValidReport, quickAction, selectOption, lastDistanceExpenseType}); | ||
| }); | ||
| }; | ||
| return [ | ||
|
|
@@ -413,7 +412,6 @@ function FloatingActionButtonAndPopover({onHideCreateMenu, onShowCreateMenu, ref | |
| isDelegateAccessRestricted, | ||
| showDelegateNoAccessModal, | ||
| isReportArchived, | ||
| isManualDistanceTrackingEnabled, | ||
| lastDistanceExpenseType, | ||
| allTransactionDrafts, | ||
| ]); | ||
|
|
@@ -438,31 +436,27 @@ function FloatingActionButtonAndPopover({onHideCreateMenu, onShowCreateMenu, ref | |
|
|
||
| const menuItems = [ | ||
| ...expenseMenuItems, | ||
| ...(isManualDistanceTrackingEnabled | ||
| ? [ | ||
| { | ||
| icon: Expensicons.Location, | ||
| text: translate('iou.trackDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => { | ||
| interceptAnonymousUser(() => { | ||
| if (shouldRedirectToExpensifyClassic) { | ||
| setModalVisible(true); | ||
| return; | ||
| } | ||
| // Start the flow to start tracking a distance request | ||
| startDistanceRequest( | ||
| CONST.IOU.TYPE.CREATE, | ||
| // When starting to create an expense from the global FAB, there is not an existing report yet. A random optimistic reportID is generated and used | ||
| // for all of the routes in the creation flow. | ||
| generateReportID(), | ||
| lastDistanceExpenseType, | ||
| ); | ||
| }); | ||
| }, | ||
| }, | ||
| ] | ||
| : []), | ||
| { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Performance and Code Quality ImprovementGreat change! The removal of the conditional array spread based on
The distance tracking option is now always available, providing consistent UX and reducing code complexity. |
||
| icon: Expensicons.Location, | ||
| text: translate('iou.trackDistance'), | ||
| shouldCallAfterModalHide: shouldUseNarrowLayout, | ||
| onSelected: () => { | ||
| interceptAnonymousUser(() => { | ||
| if (shouldRedirectToExpensifyClassic) { | ||
| setModalVisible(true); | ||
| return; | ||
| } | ||
| // Start the flow to start tracking a distance request | ||
| startDistanceRequest( | ||
| CONST.IOU.TYPE.CREATE, | ||
| // When starting to create an expense from the global FAB, there is not an existing report yet. A random optimistic reportID is generated and used | ||
| // for all of the routes in the creation flow. | ||
| generateReportID(), | ||
| lastDistanceExpenseType, | ||
| ); | ||
| }); | ||
| }, | ||
| }, | ||
| ...(shouldShowCreateReportOption | ||
| ? [ | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -179,7 +179,6 @@ function IOURequestStartPage({ | |
| }, [headerWithBackBtnContainerElement, tabBarContainerElement, activeTabContainerElement]); | ||
|
|
||
| const {isBetaEnabled} = usePermissions(); | ||
| const manualDistanceTrackingEnabled = isBetaEnabled(CONST.BETAS.MANUAL_DISTANCE); | ||
| const setTestReceiptAndNavigateRef = useRef<() => void>(() => {}); | ||
| const {shouldShowProductTrainingTooltip, renderProductTrainingTooltip} = useProductTrainingContext( | ||
| CONST.PRODUCT_TRAINING_TOOLTIP_NAMES.SCAN_TEST_TOOLTIP, | ||
|
|
@@ -266,7 +265,7 @@ function IOURequestStartPage({ | |
| </TabScreenWithFocusTrapWrapper> | ||
| )} | ||
| </TopTab.Screen> | ||
| {(!manualDistanceTrackingEnabled || iouType === CONST.IOU.TYPE.SPLIT) && ( | ||
| {iouType === CONST.IOU.TYPE.SPLIT && ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| <TopTab.Screen name={CONST.TAB_REQUEST.DISTANCE}> | ||
| {() => ( | ||
| <TabScreenWithFocusTrapWrapper> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,7 +77,7 @@ function IOURequestStepAmount({ | |
| const textInput = useRef<BaseTextInputRef | null>(null); | ||
| const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null); | ||
| const isSaveButtonPressed = useRef(false); | ||
| const iouRequestType = getRequestType(transaction, isBetaEnabled(CONST.BETAS.MANUAL_DISTANCE)); | ||
| const iouRequestType = getRequestType(transaction); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ✅ Performance ImprovementGood refactoring! The
The function now has more predictable behavior without the beta flag dependency. |
||
| const policyID = report?.policyID; | ||
|
|
||
| const isReportArchived = useReportIsArchived(report?.reportID); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❌ PERF-6
This line changes from a more specific conditional check to a broader one that may cause unnecessary re-renders. The original code included
isPolicyExpenseChatas part of the condition, providing more granular control.The removed condition
(isPolicyExpenseChat || isManualDistanceEnabled)has been simplified to just checking the rate and read-only state, but this could lead to the right icon being shown in cases where it previously wouldn't have been, potentially affecting user experience and performance.