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

Ask non-USD Workspaces to update currency before adding VBBA #19638

Merged
merged 22 commits into from
Jun 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 4 additions & 2 deletions src/languages/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -816,6 +816,7 @@ export default {
'In order to finish setting up your bank account, you must validate your account. Please check your email to validate your account, and return here to finish up!',
hasPhoneLoginError: 'To add a verified bank account please ensure your primary login is a valid email and try again. You can add your phone number as a secondary login.',
hasBeenThrottledError: 'There was an error adding your bank account. Please wait a few minutes and try again.',
hasCurrencyError: 'Oops! It appears that your workspace currency is set to a different currency than USD. To proceed, please set it to USD and try again',
error: {
noBankAccountAvailable: 'Sorry, no bank account is available',
noBankAccountSelected: 'Please choose an account',
Expand Down Expand Up @@ -1070,8 +1071,6 @@ export default {
reconcileCards: 'Reconcile cards',
settlementFrequency: 'Settlement frequency',
deleteConfirmation: 'Are you sure you want to delete this workspace?',
growlMessageOnDelete: 'Workspace deleted',
growlMessageOnDeleteError: 'This workspace cannot be deleted right now because reports are actively being processed',
unavailable: 'Unavailable workspace',
memberNotFound: 'Member not found. To invite a new member to the workspace, please use the Invite button above.',
notAuthorized: `You do not have access to this page. Are you trying to join the workspace? Please reach out to the owner of this workspace so they can add you as a member! Something else? Reach out to ${CONST.EMAIL.CONCIERGE}`,
Expand Down Expand Up @@ -1204,6 +1203,9 @@ export default {
bankAccountAnyTransactions: ' bank account. Any outstanding transactions for this account will still complete.',
clearProgress: 'Starting over will clear the progress you have made so far.',
areYouSure: 'Are you sure?',
workspaceCurrency: 'Workspace currency',
updateCurrencyPrompt: 'It looks like your Workspace is currently set to a different currency than USD. Please click the button below to update your currency to USD now.',
updateToUSD: 'Update to USD',
},
},
getAssistancePage: {
Expand Down
8 changes: 6 additions & 2 deletions src/languages/es.js
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,8 @@ export default {
hasPhoneLoginError:
'Para agregar una cuenta bancaria verificada, asegúrate de que tu nombre de usuario principal sea un correo electrónico válido y vuelve a intentarlo. Puedes agregar tu número de teléfono como nombre de usuario secundario.',
hasBeenThrottledError: 'Se produjo un error al intentar agregar tu cuenta bancaria. Por favor, espera unos minutos e inténtalo de nuevo.',
hasCurrencyError:
'¡Ups! Parece que la moneda de tu espacio de trabajo está configurada en una moneda diferente a USD. Para continuar, por favor configúrala en USD e inténtalo nuevamente.',
error: {
noBankAccountAvailable: 'Lo sentimos, no hay ninguna cuenta bancaria disponible',
noBankAccountSelected: 'Por favor, elige una cuenta bancaria',
Expand Down Expand Up @@ -1074,9 +1076,7 @@ export default {
issueAndManageCards: 'Emitir y gestionar tarjetas',
reconcileCards: 'Reconciliar tarjetas',
settlementFrequency: 'Frecuencia de liquidación',
growlMessageOnDelete: 'Espacio de trabajo eliminado',
deleteConfirmation: '¿Estás seguro de que quieres eliminar este espacio de trabajo?',
growlMessageOnDeleteError: 'No se puede eliminar el espacio de trabajo porque tiene informes que están siendo procesados',
unavailable: 'Espacio de trabajo no disponible',
memberNotFound: 'Miembro no encontrado. Para invitar a un nuevo miembro al espacio de trabajo, por favor, utiliza el botón Invitar que está arriba.',
notAuthorized: `No tienes acceso a esta página. ¿Estás tratando de unirte al espacio de trabajo? Comunícate con el propietario de este espacio de trabajo para que pueda agregarte como miembro. ¿Necesitas algo más? Comunícate con ${CONST.EMAIL.CONCIERGE}`,
Expand Down Expand Up @@ -1210,6 +1210,10 @@ export default {
bankAccountAnyTransactions: '. Los reembolsos pendientes serán completados sin problemas.',
clearProgress: 'Empezar de nuevo descartará lo completado hasta ahora.',
areYouSure: '¿Estás seguro?',
workspaceCurrency: 'Moneda del espacio de trabajo',
updateCurrencyPrompt:
'Parece que tu espacio de trabajo está configurado actualmente en una moneda diferente a USD. Por favor, haz clic en el botón de abajo para actualizar tu moneda a USD ahora.',
updateToUSD: 'Actualizar a USD',
},
},
getAssistancePage: {
Expand Down
5 changes: 4 additions & 1 deletion src/libs/actions/Policy.js
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,12 @@ function clearAvatarErrors(policyID) {
function updateGeneralSettings(policyID, name, currency) {
const optimisticData = [
{
onyxMethod: Onyx.METHOD.MERGE,
// We use SET because it's faster than merge and avoids a race condition when setting the currency and navigating the user to the Bank account page in confirmCurrencyChangeAndHideModal
onyxMethod: Onyx.METHOD.SET,
Copy link
Member

Choose a reason for hiding this comment

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

Coming from #37254 (comment)

Because we're using Onyx.set, we should have spread the old pendingFields.

key: `${ONYXKEYS.COLLECTION.POLICY}${policyID}`,
value: {
...allPolicies[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`],

pendingFields: {
generalSettings: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE,
},
Expand Down
37 changes: 21 additions & 16 deletions src/pages/ReimbursementAccount/ReimbursementAccountPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,35 +343,40 @@ class ReimbursementAccountPage extends React.Component {
);
}

let errorComponent;
const userHasPhonePrimaryEmail = Str.endsWith(this.props.session.email, CONST.SMS.DOMAIN);

if (userHasPhonePrimaryEmail) {
errorComponent = (
<View style={[styles.m5]}>
<Text>{this.props.translate('bankAccount.hasPhoneLoginError')}</Text>
</View>
if (this.state.shouldShowContinueSetupButton) {
return (
<ContinueBankAccountSetup
reimbursementAccount={this.props.reimbursementAccount}
continue={this.continue}
policyName={policyName}
/>
Comment on lines +346 to +352
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @luacmartins, This block is duplicated now, check line 384 ,we intentionally changed the order of rendering in this PR #20406.

Copy link
Contributor Author

@luacmartins luacmartins Jul 4, 2023

Choose a reason for hiding this comment

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

ah nice catch. Do we have a PR to fix this already?

Copy link
Contributor

Choose a reason for hiding this comment

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

No we don't have a PR yet, needed to confirm before moving forward. I will create a PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

PR is ready : #22253

);
}

let errorText;
const userHasPhonePrimaryEmail = Str.endsWith(this.props.session.email, CONST.SMS.DOMAIN);
const throttledDate = lodashGet(this.props.reimbursementAccount, 'throttledDate');
if (throttledDate) {
errorComponent = (
<View style={[styles.m5]}>
<Text>{this.props.translate('bankAccount.hasBeenThrottledError')}</Text>
</View>
);
const hasUnsupportedCurrency = lodashGet(this.props.policy, 'outputCurrency', '') !== CONST.CURRENCY.USD;
Copy link
Contributor

Choose a reason for hiding this comment

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

This line caused an edge case regression, when the policy is empty (deleted from other device) it will display the CurrencyError, we fixed that by displaying FullPageNotFoundView when policy is deleted.


if (userHasPhonePrimaryEmail) {
errorText = this.props.translate('bankAccount.hasPhoneLoginError');
} else if (throttledDate) {
errorText = this.props.translate('bankAccount.hasBeenThrottledError');
} else if (hasUnsupportedCurrency) {
errorText = this.props.translate('bankAccount.hasCurrencyError');
}

if (errorComponent) {
if (errorText) {
return (
<ScreenWrapper>
<HeaderWithBackButton
title={this.props.translate('workspace.common.connectBankAccount')}
subtitle={policyName}
onBackButtonPress={() => Navigation.goBack(ROUTES.SETTINGS_WORKSPACES)}
/>
{errorComponent}
<View style={[styles.m5]}>
<Text>{errorText}</Text>
</View>
</ScreenWrapper>
);
}
Expand Down
22 changes: 21 additions & 1 deletion src/pages/workspace/WorkspaceInitialPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ function dismissError(policyID) {
const WorkspaceInitialPage = (props) => {
const policy = props.policy;
const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false);
const [isCurrencyModalOpen, setIsCurrencyModalOpen] = useState(false);
const hasPolicyCreationError = Boolean(policy.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD && policy.errors);

/**
Expand All @@ -79,6 +80,15 @@ const WorkspaceInitialPage = (props) => {
Navigation.navigate(ROUTES.SETTINGS_WORKSPACES);
}, [props.reports, policy]);

/**
* Call update workspace currency and hide the modal
*/
const confirmCurrencyChangeAndHideModal = useCallback(() => {
Policy.updateGeneralSettings(policy.id, policy.name, CONST.CURRENCY.USD);
setIsCurrencyModalOpen(false);
ReimbursementAccount.navigateToBankAccountRoute(policy.id);
}, [policy]);

/**
* Navigates to workspace rooms
* @param {String} chatType
Expand Down Expand Up @@ -137,7 +147,7 @@ const WorkspaceInitialPage = (props) => {
{
translationKey: 'workspace.common.bankAccount',
icon: Expensicons.Bank,
action: () => ReimbursementAccount.navigateToBankAccountRoute(policy.id),
action: () => (policy.outputCurrency === CONST.CURRENCY.USD ? ReimbursementAccount.navigateToBankAccountRoute(policy.id) : setIsCurrencyModalOpen(true)),
brickRoadIndicator: !_.isEmpty(props.reimbursementAccount.errors) ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : '',
},
];
Expand Down Expand Up @@ -242,6 +252,16 @@ const WorkspaceInitialPage = (props) => {
</View>
</OfflineWithFeedback>
</ScrollView>
<ConfirmModal
title={props.translate('workspace.bankAccount.workspaceCurrency')}
isVisible={isCurrencyModalOpen}
onConfirm={confirmCurrencyChangeAndHideModal}
onCancel={() => setIsCurrencyModalOpen(false)}
prompt={props.translate('workspace.bankAccount.updateCurrencyPrompt')}
confirmText={props.translate('workspace.bankAccount.updateToUSD')}
cancelText={props.translate('common.cancel')}
danger
/>
<ConfirmModal
title={props.translate('workspace.common.delete')}
isVisible={isDeleteModalOpen}
Expand Down
Loading