From c76d7190f56e417baa2d6bb76c35ca553fb1330c Mon Sep 17 00:00:00 2001 From: "Maruf Sharifi (via MelvinBot)" Date: Fri, 16 Oct 2026 06:52:02 +0000 Subject: [PATCH 1/5] Show a pending unlock request state for locked domain members After a domain admin requests an unlock for a locked member, the row now reads "Pending unlock request" with an hourglass icon. Tapping it again reopens the confirmation modal without sending a duplicate request, and a failed request rolls back the state and shows an error on the row. Co-authored-by: Maruf Sharifi --- src/CONST/index.ts | 2 + src/languages/de.ts | 1 + src/languages/el.ts | 1 + src/languages/en.ts | 1 + src/languages/es.ts | 1 + src/languages/fr.ts | 1 + src/languages/it.ts | 1 + src/languages/ja.ts | 1 + src/languages/nl.ts | 1 + src/languages/pl.ts | 1 + src/languages/pt-BR.ts | 1 + src/languages/zh-hans.ts | 1 + src/libs/DomainUtils.ts | 1 + src/libs/actions/User.ts | 70 ++++++++++++++++++- .../Members/DomainMemberDetailsPage.tsx | 27 ++++--- src/selectors/Domain.ts | 3 + src/types/onyx/Domain.ts | 3 +- src/types/onyx/DomainErrors.ts | 6 ++ tests/actions/UserTest.ts | 49 ++++++++++++- tests/unit/DomainSelectorsTest.ts | 21 ++++++ 20 files changed, 177 insertions(+), 16 deletions(-) diff --git a/src/CONST/index.ts b/src/CONST/index.ts index a80192489a8e..ffb5245a33ef 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -10596,6 +10596,8 @@ const CONST = { PRIVATE_VACATION_DELEGATE_PREFIX: 'private_vacationDelegate_', /** Onyx prefix for lock account IDs */ PRIVATE_LOCKED_ACCOUNT_PREFIX: 'private_lockAccountDetails_', + /** Onyx prefix for account IDs with a pending unlock request */ + PRIVATE_UNLOCK_REQUESTED_PREFIX: 'private_unlockRequested_', MEMBERS: { SECONDARY_ACTIONS: { diff --git a/src/languages/de.ts b/src/languages/de.ts index 0cea64457ada..394c3675412b 100644 --- a/src/languages/de.ts +++ b/src/languages/de.ts @@ -2536,6 +2536,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Konto sperren', lockMyAccount: 'Sperren Sie mein Konto', unlockAccount: 'Konto entsperren', + pendingUnlockRequest: 'Ausstehende Entsperranfrage', findYourSituation: 'Die meisten Probleme erfordern keine Sperrung Ihres Kontos! Finden Sie Ihre Situation unten:', lostCardOrCharges: 'Karte verloren oder unbekannte Abbuchungen: Sperren Sie Ihre Karte und kontaktieren Sie Concierge, um unbekannte Transaktionen anzufechten.', diff --git a/src/languages/el.ts b/src/languages/el.ts index 8ae8d6e7d983..247aab5483ae 100644 --- a/src/languages/el.ts +++ b/src/languages/el.ts @@ -2588,6 +2588,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Κλείδωμα λογαριασμού', lockMyAccount: 'Κλείδωμα του λογαριασμού μου', unlockAccount: 'Ξεκλειδώστε τον λογαριασμό', + pendingUnlockRequest: 'Εκκρεμές αίτημα ξεκλειδώματος', unlockTitle: 'Λάβαμε το αίτημά σας', unlockDescription: 'Θα εξετάσουμε τον λογαριασμό για να επιβεβαιώσουμε ότι είναι ασφαλές να ξεκλειδωθεί και θα επικοινωνήσουμε μέσω Concierge για τυχόν ερωτήσεις.', findYourSituation: 'Τα περισσότερα προβλήματα δεν απαιτούν κλείδωμα του λογαριασμού σας. Βρείτε την περίπτωσή σας παρακάτω:', diff --git a/src/languages/en.ts b/src/languages/en.ts index 38aa27019c37..018fb241ce99 100644 --- a/src/languages/en.ts +++ b/src/languages/en.ts @@ -2687,6 +2687,7 @@ const translations = { lockAccount: 'Lock account', lockMyAccount: 'Lock my account', unlockAccount: 'Unlock account', + pendingUnlockRequest: 'Pending unlock request', unlockTitle: 'We’ve received your request', unlockDescription: 'We’ll review the account to verify it’s safe to unlock and reach out via Concierge with any questions.', findYourSituation: 'Most issues don’t require locking your account! Find your situation below:', diff --git a/src/languages/es.ts b/src/languages/es.ts index 23fc4be3b766..e960cca08fde 100644 --- a/src/languages/es.ts +++ b/src/languages/es.ts @@ -2446,6 +2446,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Bloquear cuenta', lockMyAccount: 'Bloquear mi cuenta', unlockAccount: 'Desbloquear cuenta', + pendingUnlockRequest: 'Solicitud de desbloqueo pendiente', unlockTitle: 'Hemos recibido tu solicitud', unlockDescription: 'Revisaremos la cuenta para verificar que sea seguro desbloquearla y nos comunicaremos a través de Concierge si tenemos alguna pregunta.', findYourSituation: '¡La mayoría de los problemas no requieren bloquear tu cuenta! Busca tu situación a continuación:', diff --git a/src/languages/fr.ts b/src/languages/fr.ts index 7f37b163f1af..1383f0a3c436 100644 --- a/src/languages/fr.ts +++ b/src/languages/fr.ts @@ -2541,6 +2541,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Verrouiller le compte', lockMyAccount: 'Verrouiller mon compte', unlockAccount: 'Déverrouiller le compte', + pendingUnlockRequest: 'Demande de déverrouillage en attente', findYourSituation: 'La plupart des problèmes ne nécessitent pas de verrouiller votre compte ! Trouvez votre situation ci-dessous :', lostCardOrCharges: 'Carte perdue ou frais inconnus : Annulez votre carte et contactez Concierge pour contester les transactions inconnues.', diff --git a/src/languages/it.ts b/src/languages/it.ts index bd8eb402c6a5..c71660807831 100644 --- a/src/languages/it.ts +++ b/src/languages/it.ts @@ -2526,6 +2526,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Blocca account', lockMyAccount: 'Blocca il mio account', unlockAccount: 'Sblocca conto', + pendingUnlockRequest: 'Richiesta di sblocco in sospeso', findYourSituation: 'La maggior parte dei problemi non richiede il blocco del tuo account! Trova la tua situazione qui sotto:', lostCardOrCharges: 'Carta smarrita o addebiti non riconosciuti: Annulla la tua carta e contatta Concierge per contestare le transazioni sconosciute.', diff --git a/src/languages/ja.ts b/src/languages/ja.ts index 7da59e4d68dc..bc5c0beaab8f 100644 --- a/src/languages/ja.ts +++ b/src/languages/ja.ts @@ -2510,6 +2510,7 @@ const translations: TranslationDeepObject = { lockAccount: 'アカウントをロック', lockMyAccount: 'アカウントをロックする', unlockAccount: 'アカウントのロック解除', + pendingUnlockRequest: 'ロック解除リクエスト保留中', findYourSituation: 'ほとんどの問題ではアカウントをロックする必要はありません。以下からご自身の状況をお探しください。', lostCardOrCharges: 'カードの紛失または身に覚えのない請求：カードを解約し、不明な取引について異議申し立てをするために Concierge へご連絡ください。', diff --git a/src/languages/nl.ts b/src/languages/nl.ts index b96c6121e753..c0d23b8aa78e 100644 --- a/src/languages/nl.ts +++ b/src/languages/nl.ts @@ -2528,6 +2528,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Account vergrendelen', lockMyAccount: 'Account vergrendelen', unlockAccount: 'Account ontgrendelen', + pendingUnlockRequest: 'Ontgrendelingsverzoek in behandeling', findYourSituation: 'Voor de meeste problemen hoef je je account niet te blokkeren. Zoek hieronder jouw situatie:', lostCardOrCharges: 'Kaart kwijt of onbekende kosten: Annuleer je kaart en neem contact op met Concierge om onbekende transacties te betwisten.', diff --git a/src/languages/pl.ts b/src/languages/pl.ts index 95e7b90a2f26..5cf2fb65efdd 100644 --- a/src/languages/pl.ts +++ b/src/languages/pl.ts @@ -2567,6 +2567,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Zablokuj konto', lockMyAccount: 'Zablokuj moje konto', unlockAccount: 'Odblokuj konto', + pendingUnlockRequest: 'Oczekująca prośba o odblokowanie', findYourSituation: 'Większość problemów nie wymaga blokowania konta! Znajdź swoją sytuację poniżej:', lostCardOrCharges: 'Zgubiona karta lub nieznane obciążenia: Anuluj swoją kartę i skontaktuj się z Concierge, żeby zakwestionować nieznane transakcje.', diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts index 0a6753623215..7db552af86c5 100644 --- a/src/languages/pt-BR.ts +++ b/src/languages/pt-BR.ts @@ -2520,6 +2520,7 @@ const translations: TranslationDeepObject = { lockAccount: 'Bloquear conta', lockMyAccount: 'Bloquear minha conta', unlockAccount: 'Desbloquear conta', + pendingUnlockRequest: 'Solicitação de desbloqueio pendente', findYourSituation: 'A maioria dos problemas não exige bloquear sua conta! Encontre sua situação abaixo:', lostCardOrCharges: 'Cartão perdido ou cobranças desconhecidas: Cancele seu cartão e contate o Concierge para contestar transações desconhecidas.', diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts index 3b4ae9ffc316..e30139d24a35 100644 --- a/src/languages/zh-hans.ts +++ b/src/languages/zh-hans.ts @@ -2445,6 +2445,7 @@ const translations: TranslationDeepObject = { lockAccount: '锁定账户', lockMyAccount: '锁定我的账户', unlockAccount: '解锁账户', + pendingUnlockRequest: '解锁请求待处理', findYourSituation: '大多数问题都不需要锁定你的账户！请在以下列表中找到与你相符的情况：', lostCardOrCharges: '卡片遗失或出现不明扣款：请挂失您的卡片，并联系 Concierge 申诉未知交易。', diff --git a/src/libs/DomainUtils.ts b/src/libs/DomainUtils.ts index 31883529eba3..adec4359c8e0 100644 --- a/src/libs/DomainUtils.ts +++ b/src/libs/DomainUtils.ts @@ -163,6 +163,7 @@ function getMemberCustomRowProps(accountID: number, domainPendingActions: Domain const mergedErrors = { ...getLatestError(accountIDErrors?.errors), ...getLatestError(accountIDErrors?.lockAccountErrors), + ...getLatestError(accountIDErrors?.unlockAccountErrors), ...getLatestError({...accountIDErrors?.changeDomainSecurityGroupErrors, ...emailErrors?.changeDomainSecurityGroupErrors}), ...getLatestError(emailErrors?.errors), ...getLatestError(accountIDErrors?.vacationDelegateErrors), diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts index c60f3ab107ba..81a99416e31b 100644 --- a/src/libs/actions/User.ts +++ b/src/libs/actions/User.ts @@ -1413,6 +1413,7 @@ function lockAccount(currentUserAccountID: number, accountID: number | undefined let domainSuccessData: DomainOnyxUpdate[] = []; if (accountID && domainAccountID) { const userLockKey = `${CONST.DOMAIN.PRIVATE_LOCKED_ACCOUNT_PREFIX}${accountID}`; + const userUnlockRequestedKey = `${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`; domainOptimisticData = [ { @@ -1420,7 +1421,9 @@ function lockAccount(currentUserAccountID: number, accountID: number | undefined key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, value: { [userLockKey]: true, - } as PrefixedRecord, + // Clear any unlock request left over from a previous lock so the new lock doesn't show as pending unlock + [userUnlockRequestedKey]: false, + } as PrefixedRecord, }, { onyxMethod: Onyx.METHOD.MERGE, @@ -1550,12 +1553,72 @@ function lockAccount(currentUserAccountID: number, accountID: number | undefined return API.makeRequestWithSideEffects(SIDE_EFFECT_REQUEST_COMMANDS.LOCK_ACCOUNT, params, {optimisticData, successData, failureData}); } -function requestUnlockAccount(accountID: number) { +function requestUnlockAccount(accountID: number, domainAccountID?: number, domainName?: string) { + let optimisticData: DomainOnyxUpdate[] = []; + let failureData: DomainOnyxUpdate[] = []; + if (domainAccountID) { + const userUnlockRequestedKey = `${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`; + + optimisticData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, + value: { + [userUnlockRequestedKey]: true, + } as PrefixedRecord, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`, + value: { + memberErrors: { + [accountID]: { + unlockAccountErrors: null, + }, + }, + }, + }, + ]; + + failureData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, + value: { + [userUnlockRequestedKey]: false, + } as PrefixedRecord, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`, + value: { + memberErrors: { + [accountID]: { + unlockAccountErrors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('common.genericErrorMessage'), + }, + }, + }, + }, + ]; + } + const params: LockAccountParams = { accountID, + domainAccountID, + domainName, }; - API.write(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, params); + API.write(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, params, {optimisticData, failureData}); +} + +function clearUnlockAccountError(domainAccountID: number, accountID: number) { + Onyx.merge(`${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`, { + memberErrors: { + [accountID]: { + unlockAccountErrors: null, + }, + }, + }); } type RespondToProactiveAppReviewParams = { @@ -2051,6 +2114,7 @@ export { clearBetaOverrides, lockAccount, requestUnlockAccount, + clearUnlockAccountError, respondToProactiveAppReview, verifyAddSecondaryLoginCode, updateIsVerifiedValidateActionCode, diff --git a/src/pages/domain/Members/DomainMemberDetailsPage.tsx b/src/pages/domain/Members/DomainMemberDetailsPage.tsx index b012ff301f7b..495f14e69b32 100644 --- a/src/pages/domain/Members/DomainMemberDetailsPage.tsx +++ b/src/pages/domain/Members/DomainMemberDetailsPage.tsx @@ -23,7 +23,7 @@ import { setTwoFactorAuthExemptEmailForDomain, } from '@libs/actions/Domain'; import {clearDraftValues} from '@libs/actions/FormActions'; -import {requestUnlockAccount} from '@libs/actions/User'; +import {clearUnlockAccountError, requestUnlockAccount} from '@libs/actions/User'; import {getLatestError} from '@libs/ErrorUtils'; import Navigation from '@navigation/Navigation'; @@ -45,7 +45,7 @@ import type {Domain} from '@src/types/onyx'; import type {OnyxEntry} from 'react-native-onyx'; import {requiresTwoFactorAuthSelector} from '@selectors/Account'; -import {accountLockSelector, domainMemberSettingsSelector, domainNameSelector, selectSecurityGroupForAccount, vacationDelegateSelector} from '@selectors/Domain'; +import {accountLockSelector, domainMemberSettingsSelector, domainNameSelector, selectSecurityGroupForAccount, unlockRequestedSelector, vacationDelegateSelector} from '@selectors/Domain'; import React, {useCallback, useState} from 'react'; import {View} from 'react-native'; @@ -55,7 +55,7 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { const {domainAccountID, accountID} = route.params; const styles = useThemeStyles(); const {translate} = useLocalize(); - const icons = useMemoizedLazyExpensifyIcons(['RemoveMembers', 'Flag', 'Unlock', 'CircularArrowBackwards']); + const icons = useMemoizedLazyExpensifyIcons(['RemoveMembers', 'Flag', 'Unlock', 'Hourglass', 'CircularArrowBackwards']); const [isModalVisible, setIsModalVisible] = useState(false); const [shouldForceCloseAccount, setShouldForceCloseAccount] = useState(); // We need to use isSmallScreenWidth here because the DecisionModal is opening from RHP and ShouldUseNarrowLayout layout will not work in this place @@ -88,6 +88,7 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { const memberLogin = personalDetails?.login ?? ''; const [isAccountLocked] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: accountLockSelector(accountID)}); + const [isUnlockRequested] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: unlockRequestedSelector(accountID)}); const lockDomainError = getLatestError(domainErrors?.memberErrors?.[accountID]?.lockAccountErrors); const lockDomainErrorMessage = Object.values(lockDomainError).at(0); @@ -136,7 +137,9 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { ); const showUnlockAccountModal = () => { - requestUnlockAccount(accountID); + if (!isUnlockRequested) { + requestUnlockAccount(accountID, domainAccountID, domainName); + } showConfirmModal({ title: translate('lockAccountPage.unlockTitle'), prompt: translate('lockAccountPage.unlockDescription'), @@ -211,12 +214,18 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { )} {isAccountLocked ? ( - + errorRowStyles={styles.mh5} + errors={getLatestError(domainErrors?.memberErrors?.[accountID]?.unlockAccountErrors)} + onClose={() => clearUnlockAccountError(domainAccountID, accountID)} + > + + ) : ( ): DomainSecurityGroupWithID[] const accountLockSelector = (accountID: number) => (domain: OnyxEntry) => domain?.[`${CONST.DOMAIN.PRIVATE_LOCKED_ACCOUNT_PREFIX}${accountID}`]; +const unlockRequestedSelector = (accountID: number) => (domain: OnyxEntry) => domain?.[`${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`]; + function selectRestrictedPrimaryPolicyID(groupID?: string) { return (domain: OnyxEntry): string | undefined => { const targetGroup = groupsSelector(domain)?.find((g) => g.id === groupID); @@ -341,6 +343,7 @@ export { groupsSelector, vacationDelegateSelector, accountLockSelector, + unlockRequestedSelector, isAdminSelector, domainAccessSelector, hasPendingAdminshipRequestSelector, diff --git a/src/types/onyx/Domain.ts b/src/types/onyx/Domain.ts index fd587885cf1d..a3854380c06d 100644 --- a/src/types/onyx/Domain.ts +++ b/src/types/onyx/Domain.ts @@ -53,7 +53,8 @@ type Domain = OnyxCommon.OnyxValueWithOfflineFeedback<{ PrefixedRecord & PrefixedRecord & PrefixedRecord & - PrefixedRecord; + PrefixedRecord & + PrefixedRecord; /** Model of SAML metadata */ type SamlMetadata = { diff --git a/src/types/onyx/DomainErrors.ts b/src/types/onyx/DomainErrors.ts index 990e553fd19e..98b8633e8781 100644 --- a/src/types/onyx/DomainErrors.ts +++ b/src/types/onyx/DomainErrors.ts @@ -28,6 +28,12 @@ type DomainMemberErrors = { * Errors related to specific domain member lock account status. */ lockAccountErrors?: OnyxCommon.Errors; + + /** + * Errors related to requesting an unlock for a specific domain member account. + */ + unlockAccountErrors?: OnyxCommon.Errors; + /** * Errors related to changing domain member group. */ diff --git a/tests/actions/UserTest.ts b/tests/actions/UserTest.ts index 9c0a92698f4b..ba27cb1e0184 100644 --- a/tests/actions/UserTest.ts +++ b/tests/actions/UserTest.ts @@ -924,6 +924,7 @@ describe('actions/User', () => { const domainAccountID = 200; const domainName = 'expensify.com'; const userLockKey = `${CONST.DOMAIN.PRIVATE_LOCKED_ACCOUNT_PREFIX}${accountID}`; + const userUnlockRequestedKey = `${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`; it('should include correct domain optimistic, success, and failure data', async () => { UserActions.lockAccount(currentUserAccountID, accountID, domainAccountID, domainName); @@ -934,11 +935,11 @@ describe('actions/User', () => { const failureData = onyxData?.failureData ?? []; const successData = onyxData?.successData ?? []; - // Optimistic: sets lock flag to true + // Optimistic: sets lock flag to true and clears any unlock request from a previous lock expect(optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, - value: {[userLockKey]: true}, + value: {[userLockKey]: true, [userUnlockRequestedKey]: false}, }); // Optimistic: sets pending action to UPDATE @@ -1008,7 +1009,49 @@ describe('actions/User', () => { UserActions.requestUnlockAccount(accountID); await waitForBatchedUpdates(); - expect(mockAPI.write).toHaveBeenCalledWith(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, {accountID}); + expect(mockAPI.write).toHaveBeenCalledWith(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, {accountID}, {optimisticData: [], failureData: []}); + }); + + it('should mark the unlock as requested optimistically and roll it back on failure when called from a domain', async () => { + // Given a domain admin requesting an unlock for a locked domain member + const accountID = 123456; + const domainAccountID = 200; + const domainName = 'expensify.com'; + const userUnlockRequestedKey = `${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`; + + // When the unlock is requested + UserActions.requestUnlockAccount(accountID, domainAccountID, domainName); + await waitForBatchedUpdates(); + + // Then the domain identifiers are sent so the backend knows which domain the request came from + expect(mockAPI.write).toHaveBeenCalledWith(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, {accountID, domainAccountID, domainName}, expect.anything()); + + const onyxData = mockAPI.write.mock.calls.at(0)?.[2]; + const optimisticData = onyxData?.optimisticData ?? []; + const failureData = onyxData?.failureData ?? []; + + // Then the member is optimistically marked as having a pending unlock request so the UI can reflect it + expect(optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, + value: {[userUnlockRequestedKey]: true}, + }); + + // Then a failed request reverts the pending state so the admin can try again + expect(failureData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, + value: {[userUnlockRequestedKey]: false}, + }); + + // Then a failed request surfaces an error on the member + expect(failureData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`)).toEqual( + expect.objectContaining({ + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`, + value: {memberErrors: {[accountID]: {unlockAccountErrors: anyObject}}}, + }), + ); }); }); diff --git a/tests/unit/DomainSelectorsTest.ts b/tests/unit/DomainSelectorsTest.ts index 3e9b8c5a2ffa..b4a55a419d69 100644 --- a/tests/unit/DomainSelectorsTest.ts +++ b/tests/unit/DomainSelectorsTest.ts @@ -26,6 +26,7 @@ import { selectRestrictedPrimaryPolicyID, selectSecurityGroupForAccount, technicalContactSettingsSelector, + unlockRequestedSelector, vacationDelegateSelector, } from '@selectors/Domain'; @@ -1059,6 +1060,26 @@ describe('domainSelectors', () => { }); }); + describe('unlockRequestedSelector', () => { + it('Should return the unlock requested state for the given account ID', () => { + // Given a domain where an unlock was requested for one member + const accountID = 123; + const domain = createDomainFixture({boundaryEntries: {[`${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`]: true}}); + + // When reading the unlock requested state for that member and another member + // Then only the member with the request is reported as pending + expect(unlockRequestedSelector(accountID)(domain)).toBe(true); + expect(unlockRequestedSelector(456)(domain)).toBeUndefined(); + }); + + it('Should return undefined when the domain object is undefined', () => { + // Given the domain has not loaded yet + // When reading the unlock requested state + // Then nothing is reported as pending + expect(unlockRequestedSelector(123)(undefined)).toBeUndefined(); + }); + }); + describe('selectRestrictedPrimaryPolicyID', () => { const makeGroup = (enableRestrictedPrimaryPolicy: boolean, restrictedPrimaryPolicyID?: string): DomainSecurityGroup => createFixture(domainSecurityGroupFixture, {enableRestrictedPrimaryPolicy, restrictedPrimaryPolicyID}); From 92fe6b133b5c8c2741700f9904e0a02ba23c7b65 Mon Sep 17 00:00:00 2001 From: "Maruf Sharifi (via MelvinBot)" Date: Mon, 19 Oct 2026 19:32:54 +0000 Subject: [PATCH 2/5] Only show pending unlock request after the server receives it Mark a queued unlock request with a pending action instead of setting the unlock-requested flag optimistically, so offline the row is greyed out rather than showing Pending unlock request. The flag is now set in successData, and duplicate requests are blocked while one is queued. Co-authored-by: Maruf Sharifi --- src/libs/actions/User.ts | 43 ++++++++++++++++--- .../Members/DomainMemberDetailsPage.tsx | 5 ++- src/types/onyx/DomainPendingActions.ts | 5 +++ tests/actions/UserTest.ts | 32 ++++++++++---- 4 files changed, 70 insertions(+), 15 deletions(-) diff --git a/src/libs/actions/User.ts b/src/libs/actions/User.ts index 81a99416e31b..06e494e354de 100644 --- a/src/libs/actions/User.ts +++ b/src/libs/actions/User.ts @@ -1555,17 +1555,23 @@ function lockAccount(currentUserAccountID: number, accountID: number | undefined function requestUnlockAccount(accountID: number, domainAccountID?: number, domainName?: string) { let optimisticData: DomainOnyxUpdate[] = []; + let successData: DomainOnyxUpdate[] = []; let failureData: DomainOnyxUpdate[] = []; if (domainAccountID) { const userUnlockRequestedKey = `${CONST.DOMAIN.PRIVATE_UNLOCK_REQUESTED_PREFIX}${accountID}`; + // Only mark the request as pending while it's queued; the unlock-requested flag is set once the server receives it optimisticData = [ { onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, + key: `${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`, value: { - [userUnlockRequestedKey]: true, - } as PrefixedRecord, + member: { + [accountID]: { + unlockAccount: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, + }, + }, + }, }, { onyxMethod: Onyx.METHOD.MERGE, @@ -1580,14 +1586,39 @@ function requestUnlockAccount(accountID: number, domainAccountID?: number, domai }, ]; - failureData = [ + successData = [ { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, value: { - [userUnlockRequestedKey]: false, + [userUnlockRequestedKey]: true, } as PrefixedRecord, }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`, + value: { + member: { + [accountID]: { + unlockAccount: null, + }, + }, + }, + }, + ]; + + failureData = [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`, + value: { + member: { + [accountID]: { + unlockAccount: null, + }, + }, + }, + }, { onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domainAccountID}`, @@ -1608,7 +1639,7 @@ function requestUnlockAccount(accountID: number, domainAccountID?: number, domai domainName, }; - API.write(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, params, {optimisticData, failureData}); + API.write(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, params, {optimisticData, successData, failureData}); } function clearUnlockAccountError(domainAccountID: number, accountID: number) { diff --git a/src/pages/domain/Members/DomainMemberDetailsPage.tsx b/src/pages/domain/Members/DomainMemberDetailsPage.tsx index 495f14e69b32..9d6bf52312fd 100644 --- a/src/pages/domain/Members/DomainMemberDetailsPage.tsx +++ b/src/pages/domain/Members/DomainMemberDetailsPage.tsx @@ -89,6 +89,7 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { const [isAccountLocked] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: accountLockSelector(accountID)}); const [isUnlockRequested] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: unlockRequestedSelector(accountID)}); + const unlockAccountPendingAction = domainPendingActions?.member?.[accountID]?.unlockAccount; const lockDomainError = getLatestError(domainErrors?.memberErrors?.[accountID]?.lockAccountErrors); const lockDomainErrorMessage = Object.values(lockDomainError).at(0); @@ -137,7 +138,8 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { ); const showUnlockAccountModal = () => { - if (!isUnlockRequested) { + // Don't send another request while one is already queued or has been received + if (!isUnlockRequested && !unlockAccountPendingAction) { requestUnlockAccount(accountID, domainAccountID, domainName); } showConfirmModal({ @@ -217,6 +219,7 @@ function DomainMemberDetailsPage({route}: DomainMemberDetailsPageProps) { clearUnlockAccountError(domainAccountID, accountID)} > diff --git a/src/types/onyx/DomainPendingActions.ts b/src/types/onyx/DomainPendingActions.ts index 6b2b2a0cfdd9..a8ad66f65881 100644 --- a/src/types/onyx/DomainPendingActions.ts +++ b/src/types/onyx/DomainPendingActions.ts @@ -31,6 +31,11 @@ type DomainMemberPendingActions = { */ lockAccount?: OnyxCommon.PendingAction; + /** + * Pending action for a domain member unlock account request. + */ + unlockAccount?: OnyxCommon.PendingAction; + changeDomainSecurityGroup?: OnyxCommon.PendingAction; } & GeneralDomainMemberPendingAction; diff --git a/tests/actions/UserTest.ts b/tests/actions/UserTest.ts index ba27cb1e0184..1179f7a36ccb 100644 --- a/tests/actions/UserTest.ts +++ b/tests/actions/UserTest.ts @@ -1009,10 +1009,10 @@ describe('actions/User', () => { UserActions.requestUnlockAccount(accountID); await waitForBatchedUpdates(); - expect(mockAPI.write).toHaveBeenCalledWith(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, {accountID}, {optimisticData: [], failureData: []}); + expect(mockAPI.write).toHaveBeenCalledWith(WRITE_COMMANDS.REQUEST_UNLOCK_ACCOUNT, {accountID}, {optimisticData: [], successData: [], failureData: []}); }); - it('should mark the unlock as requested optimistically and roll it back on failure when called from a domain', async () => { + it('should only mark the unlock as requested once the server receives it when called from a domain', async () => { // Given a domain admin requesting an unlock for a locked domain member const accountID = 123456; const domainAccountID = 200; @@ -1028,20 +1028,36 @@ describe('actions/User', () => { const onyxData = mockAPI.write.mock.calls.at(0)?.[2]; const optimisticData = onyxData?.optimisticData ?? []; + const successData = onyxData?.successData ?? []; const failureData = onyxData?.failureData ?? []; + const pendingActionsKey = `${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`; + + // Then the queued request is only marked with a pending action, so offline it doesn't look like the server received it + expect(optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toBeUndefined(); + expect(optimisticData.find((update) => update.key === pendingActionsKey)).toEqual({ + onyxMethod: Onyx.METHOD.MERGE, + key: pendingActionsKey, + value: {member: {[accountID]: {unlockAccount: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE}}}, + }); - // Then the member is optimistically marked as having a pending unlock request so the UI can reflect it - expect(optimisticData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ + // Then a successful request marks the member as having a pending unlock request and clears the pending action + expect(successData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ onyxMethod: Onyx.METHOD.MERGE, key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, value: {[userUnlockRequestedKey]: true}, }); + expect(successData.find((update) => update.key === pendingActionsKey)).toEqual({ + onyxMethod: Onyx.METHOD.MERGE, + key: pendingActionsKey, + value: {member: {[accountID]: {unlockAccount: null}}}, + }); - // Then a failed request reverts the pending state so the admin can try again - expect(failureData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toEqual({ + // Then a failed request clears the pending action so the admin can try again + expect(failureData.find((update) => update.key === `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`)).toBeUndefined(); + expect(failureData.find((update) => update.key === pendingActionsKey)).toEqual({ onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, - value: {[userUnlockRequestedKey]: false}, + key: pendingActionsKey, + value: {member: {[accountID]: {unlockAccount: null}}}, }); // Then a failed request surfaces an error on the member From bc73674f36b94a5b96d80cdda390a711e50f632d Mon Sep 17 00:00:00 2001 From: "Maruf Sharifi (via MelvinBot)" Date: Wed, 21 Oct 2026 02:47:45 +0000 Subject: [PATCH 3/5] Keep queued lock and unlock pending actions when dismissing a member error Co-authored-by: Maruf Sharifi --- src/libs/actions/Domain.ts | 8 +++++++- tests/actions/DomainTest.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/libs/actions/Domain.ts b/src/libs/actions/Domain.ts index e4c83f87a9d1..d1d7a91c2d1d 100644 --- a/src/libs/actions/Domain.ts +++ b/src/libs/actions/Domain.ts @@ -1422,10 +1422,16 @@ function clearDomainMemberError(domainAccountID: number, accountID: number, emai }, }); + // Keep the lock and unlock pending actions: those requests may still be queued, and dropping them would let the user send a duplicate Onyx.merge(`${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`, { member: { [email]: null, - [accountID]: null, + [accountID]: { + pendingAction: null, + vacationDelegate: null, + twoFactorAuthExemptEmails: null, + changeDomainSecurityGroup: null, + }, }, }); } diff --git a/tests/actions/DomainTest.ts b/tests/actions/DomainTest.ts index 348a44789864..147d79f9514b 100644 --- a/tests/actions/DomainTest.ts +++ b/tests/actions/DomainTest.ts @@ -1056,6 +1056,36 @@ describe('actions/Domain', () => { }); }); + it('clearDomainMemberError - keeps queued lock and unlock pending actions', async () => { + const domainAccountID = 123; + const accountID = 456; + const email = 'user@test.com'; + + // Given a member with a failed vacation delegate update while an unlock request is still queued + await Onyx.set(`${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}` as const, { + member: { + [accountID]: { + vacationDelegate: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, + lockAccount: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, + unlockAccount: CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE, + }, + }, + }); + + // When the user dismisses the error from the members list + clearDomainMemberError(domainAccountID, accountID, email, ''); + + // Then the vacation delegate pending action is cleared, but the lock and unlock ones stay so a duplicate request can't be sent + await TestHelper.getOnyxData({ + key: `${ONYXKEYS.COLLECTION.DOMAIN_PENDING_ACTIONS}${domainAccountID}`, + callback: (pendingActions) => { + expect(pendingActions?.member?.[accountID]?.vacationDelegate).toBeFalsy(); + expect(pendingActions?.member?.[accountID]?.lockAccount).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); + expect(pendingActions?.member?.[accountID]?.unlockAccount).toBe(CONST.RED_BRICK_ROAD_PENDING_ACTION.UPDATE); + }, + }); + }); + it('clearDomainMemberError - clears the placeholder personal details for an optimistic account', async () => { const domainAccountID = 123; const email = 'user@test.com'; From 52aa62aa903b603d268ee98a16d83d165856db57 Mon Sep 17 00:00:00 2001 From: "Maruf Sharifi (via MelvinBot)" Date: Wed, 21 Oct 2026 17:43:24 +0000 Subject: [PATCH 4/5] Show the red dot for failed lock and unlock requests on Domain members Co-authored-by: Maruf Sharifi --- src/libs/DomainUtils.ts | 4 +++- tests/unit/DomainUtilsTest.ts | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/libs/DomainUtils.ts b/src/libs/DomainUtils.ts index adec4359c8e0..4811ab1ec11c 100644 --- a/src/libs/DomainUtils.ts +++ b/src/libs/DomainUtils.ts @@ -137,7 +137,9 @@ function hasDomainMemberDetailsErrors(memberDetailsErrors: DomainMemberErrors): return ( !isEmptyObject(memberDetailsErrors?.vacationDelegateErrors) || !isEmptyObject(memberDetailsErrors?.twoFactorAuthExemptEmailsError) || - !isEmptyObject(memberDetailsErrors?.changeDomainSecurityGroupErrors) + !isEmptyObject(memberDetailsErrors?.changeDomainSecurityGroupErrors) || + !isEmptyObject(memberDetailsErrors?.lockAccountErrors) || + !isEmptyObject(memberDetailsErrors?.unlockAccountErrors) ); } diff --git a/tests/unit/DomainUtilsTest.ts b/tests/unit/DomainUtilsTest.ts index 9607003e5683..05a91e9ca9b4 100644 --- a/tests/unit/DomainUtilsTest.ts +++ b/tests/unit/DomainUtilsTest.ts @@ -357,6 +357,28 @@ describe('DomainUtils', () => { }; expect(hasDomainMembersErrors(domainErrors)).toBe(true); }); + + it('should return true when a member has only a lock account error', () => { + // Given a member whose lock request failed, which is stored outside the generic `errors` field + const domainErrors: DomainErrors = { + errors: {}, + memberErrors: {user1: {errors: {}, lockAccountErrors:{timestamp1: 'Lock error'}}}, + }; + + // When / Then the Domain members row and the Domains list should still show the red dot + expect(hasDomainMembersErrors(domainErrors)).toBe(true); + }); + + it('should return true when a member has only an unlock account error', () => { + // Given a member whose unlock request failed, which is stored outside the generic `errors` field + const domainErrors: DomainErrors = { + errors: {}, + memberErrors: {user1: {errors: {}, unlockAccountErrors:{timestamp1: 'Unlock error'}}}, + }; + + // When / Then the Domain members row and the Domains list should still show the red dot + expect(hasDomainMembersErrors(domainErrors)).toBe(true); + }); }); describe('hasDomainMembersSettingsErrors', () => { From 364c27d697215f19c026dbdaecca7616122d5b33 Mon Sep 17 00:00:00 2001 From: "Maruf Sharifi (via MelvinBot)" Date: Wed, 21 Oct 2026 17:48:33 +0000 Subject: [PATCH 5/5] Fix: Apply Oxfmt formatting to DomainUtilsTest Co-authored-by: Maruf Sharifi --- tests/unit/DomainUtilsTest.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/DomainUtilsTest.ts b/tests/unit/DomainUtilsTest.ts index 05a91e9ca9b4..45082b4f3484 100644 --- a/tests/unit/DomainUtilsTest.ts +++ b/tests/unit/DomainUtilsTest.ts @@ -362,7 +362,7 @@ describe('DomainUtils', () => { // Given a member whose lock request failed, which is stored outside the generic `errors` field const domainErrors: DomainErrors = { errors: {}, - memberErrors: {user1: {errors: {}, lockAccountErrors:{timestamp1: 'Lock error'}}}, + memberErrors: {user1: {errors: {}, lockAccountErrors: {timestamp1: 'Lock error'}}}, }; // When / Then the Domain members row and the Domains list should still show the red dot @@ -373,7 +373,7 @@ describe('DomainUtils', () => { // Given a member whose unlock request failed, which is stored outside the generic `errors` field const domainErrors: DomainErrors = { errors: {}, - memberErrors: {user1: {errors: {}, unlockAccountErrors:{timestamp1: 'Unlock error'}}}, + memberErrors: {user1: {errors: {}, unlockAccountErrors: {timestamp1: 'Unlock error'}}}, }; // When / Then the Domain members row and the Domains list should still show the red dot