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

Fix validation code not sending on initial page load. #48047

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
8 changes: 7 additions & 1 deletion src/components/ValidateAccountMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import variables from '@styles/variables';
import * as Session from '@userActions/Session';
import * as User from '@userActions/User';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import Icon from './Icon';
Expand Down Expand Up @@ -41,7 +42,12 @@ function ValidateAccountMessage({backTo}: ValidateAccountMessageProps) {
<TextLink
fontSize={variables.fontSizeLabel}
onPress={() => {
const login = loginList?.[loginNames?.[0]] ?? {};
const loginName = loginNames?.[0];
const login = loginList?.[loginName] ?? {};
if (!login?.validatedDate && !login?.validateCodeSent) {
User.requestContactMethodValidateCode(loginName);
}

Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.getRoute(login?.partnerUserID ?? loginNames?.[0], backTo));
}}
>
Expand Down
1 change: 1 addition & 0 deletions src/libs/actions/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ function addNewContactMethod(contactMethod: string, validateCode = '') {
[contactMethod]: {
partnerUserID: contactMethod,
validatedDate: '',
validateCodeSent: true,
errorFields: {
addedLogin: null,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,6 @@ function ContactMethodDetailsPage({route}: ContactMethodDetailsPageProps) {
User.deleteContactMethod(contactMethod, loginList ?? {}, backTo);
}, [contactMethod, loginList, toggleDeleteModal, backTo]);

useEffect(() => {
if (isEmptyObject(loginData)) {
return;
}
User.resetContactMethodValidateCodeSentState(contactMethod);
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
}, []);

const prevValidatedDate = usePrevious(loginData?.validatedDate);
useEffect(() => {
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import useLocalize from '@hooks/useLocalize';
import useThemeStyles from '@hooks/useThemeStyles';
import Navigation from '@libs/Navigation/Navigation';
import type {SettingsNavigatorParamList} from '@libs/Navigation/types';
import * as User from '@userActions/User';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
Expand Down Expand Up @@ -80,7 +81,12 @@ function ContactMethodsPage({loginList, session, route}: ContactMethodsPageProps
<MenuItem
title={menuItemTitle}
description={description}
onPress={() => Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.getRoute(partnerUserID, navigateBackTo))}
onPress={() => {
if (!login?.validatedDate && !login?.validateCodeSent) {
User.requestContactMethodValidateCode(loginName);
}
Navigation.navigate(ROUTES.SETTINGS_CONTACT_METHOD_DETAILS.getRoute(partnerUserID, navigateBackTo));
}}
brickRoadIndicator={indicator}
shouldShowBasicTitle
shouldShowRightIcon
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change needed @wildan-m? This doesn't seem to be related to your changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@allroundexperts did you mean adding validateCodeSentIsPressedRef ? without that, the message Link has been re-sent would be shown on the initial load.

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean all of the changes in these files. It looks to me they are not needed. How were they working previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@allroundexperts

Before: The requestContactMethodValidateCode function is not triggered upon the initial page load, resulting in the validateCodeSent variable not being updated. It is only updated when the magicCodeNotReceived button is clicked, allowing validateCodeSent to solely indicate whether a magic code has been sent. Additionally, validateCodeSent is reset to false each time the component is mounted.

After: The function requestContactMethodValidateCode is invoked on each page call if validateCodeSent is false. When this function is called, validateCodeSent is changed to true. On the initial page load of BaseValidateCodeForm.tsx, the message Link has been re-sent will be displayed. We cannot rely solely on validateCodeSent because its value is modified before the page call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,8 @@ function BaseValidateCodeForm({
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- nullish coalescing doesn't achieve the same result in this case
const shouldDisableResendValidateCode = !!isOffline || account?.isLoading;
const focusTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const validateCodeSentIsPressedRef = useRef(false);
const [showDotIndicator, setShowDotIndicator] = useState(false);

useImperativeHandle(innerRef, () => ({
focus() {
Expand Down Expand Up @@ -143,6 +145,18 @@ function BaseValidateCodeForm({
inputValidateCodeRef.current?.clear();
}, [hasMagicCodeBeenSent]);

useEffect(() => {
// `validateCodeSent` is updated asynchronously,
// and `validateCodeSentIsPressedRef.current` is updated faster than `hasMagicCodeBeenSent`.
// This can cause the component to hide and show the `DotIndicatorMessage` multiple times
// in quick succession, leading to a flickering effect.
if ((hasMagicCodeBeenSent ?? !!pendingContact?.validateCodeSent) && validateCodeSentIsPressedRef.current) {
setShowDotIndicator(true);
} else {
setShowDotIndicator(false);
}
}, [hasMagicCodeBeenSent, pendingContact, validateCodeSentIsPressedRef]);

/**
* Request a validate code / magic code be sent to verify this contact method
*/
Expand All @@ -154,8 +168,8 @@ function BaseValidateCodeForm({
}

inputValidateCodeRef.current?.clear();
validateCodeSentIsPressedRef.current = true;
};

/**
* Handle text input and clear formError upon text change
*/
Expand Down Expand Up @@ -227,7 +241,7 @@ function BaseValidateCodeForm({
>
<Text style={[StyleUtils.getDisabledLinkStyles(shouldDisableResendValidateCode)]}>{translate('validateCodeForm.magicCodeNotReceived')}</Text>
</PressableWithFeedback>
{(hasMagicCodeBeenSent ?? !!pendingContact?.validateCodeSent) && (
{showDotIndicator && (
<DotIndicatorMessage
type="success"
style={[styles.mt6, styles.flex0]}
Expand Down
Loading