Skip to content

Commit a974cf2

Browse files
authored
refactor(card): remove cardGeolocation property and selectors (#27747)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Refactors Card-related code so **geolocation is read only from `GeolocationController`** (`selectGeolocationLocation` → `engine.backgroundState.GeolocationController.location`) instead of a duplicated **`geoLocation` field on the card Redux slice** or from **`getCardholder`**. **Why**: Geo was stored in multiple places (card slice, async `getCardholder` + controller messenger). That duplicated state, complicated the `loadCardholderAccounts` payload, and risked drift. The single source of truth should be the engine’s GeolocationController. **What changed**: - **Card slice (`card/index.ts`)**: Removed `geoLocation` from `CardSliceState` and initial state; removed writing geo on `loadCardholderAccounts.fulfilled`; removed **`selectCardGeoLocation`**. **`selectIsUserInSupportedCardCountry`** now uses **`selectGeolocationLocation`** instead of card-stored geo. - **`getCardholder`**: Returns only **`{ cardholderAddresses }`**; removed parallel `GeolocationController:getGeolocation` call and **`geoLocation`** from the return type and error paths. - **Consumers**: **`SignUp`**, **`isBaanxLoginEnabled`** (pure helper + hook), **`useCardAuthenticationVerification`**, and deeplink handlers (**`handleCardKycNotification`**, **`handleCardHome`**, **`handleCardOnboarding`**) now use **`selectGeolocationLocation`**. **`isBaanxLoginEnabled`** params use **`geolocationLocation`** (and **`handleCardKycNotification`** passes that key correctly). - **Tests**: Updated **`SignUp.test.tsx`** (store includes `engine.backgroundState.GeolocationController` for selectors), **`card/index.test.ts`**, **`getCardholder.test.ts`**, deeplink handler tests, **`isBaanxLoginEnabled.test.ts`**, and **`handleCardKycNotification.test.ts`** (mocks **`geolocationController`**). ## **Changelog** CHANGELOG entry: Card geolocation now comes from GeolocationController only; removed duplicated `geoLocation` from card state and from `getCardholder` results. ## **Related issues** Fixes: ## **Manual testing steps** ```gherkin Feature: Card flows use GeolocationController geo consistently Scenario: Card sign-up country prefill from geo Given GeolocationController has a supported country code (e.g. US) When I open Card sign-up Then the country field may prefill from geo and user card location updates as before Scenario: Card button / Baanx gating vs supported countries Given remote config maps supported countries for Card When my location from GeolocationController is in that map and flags allow Then Card entry / Baanx login gating behaves as before Scenario: Card KYC notification deeplink Given I receive a card KYC notification deeplink When the handler runs Then feature gating still uses geo + flags correctly and navigation still works Scenario: Cardholder load When cardholder accounts load completes Then card slice only updates cardholder list / loaded state (no separate geo field on card) ``` ## **Screenshots/Recordings** No intentional UI copy or layout changes; behavior should match main if geo from the controller matches previous values. ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes the source of truth for geo-based feature gating and session clearing across onboarding, deeplinks, and auth verification; incorrect selector wiring or missing engine state could disable/enable Card flows unexpectedly. > > **Overview** > **Geolocation is now sourced only from `GeolocationController`.** The Card slice no longer stores `geoLocation` (and `selectCardGeoLocation` is removed), and country gating (`selectIsUserInSupportedCardCountry`, `isBaanxLoginEnabled`, deeplink handlers, and `SignUp`) now reads `selectGeolocationLocation` instead. > > `getCardholder` is simplified to return only `cardholderAddresses` (no parallel geolocation fetch), and `useCardAuthenticationVerification` tightens session-clearing to only reset auth once geolocation is *loaded* and not `UNKNOWN`. Tests are updated accordingly, including injecting `engine.backgroundState.GeolocationController.location` into the test store and updating mocks/expectations throughout. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8730edb. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent b42b69e commit a974cf2

16 files changed

Lines changed: 173 additions & 276 deletions

app/components/UI/Card/components/Onboarding/SignUp.test.tsx

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,9 +85,25 @@ jest.mock('./OnboardingStep', () => {
8585
});
8686

8787
// Create test store
88-
const createTestStore = (initialState = {}) =>
89-
configureStore({
88+
// SignUp reads geoLocation from state.engine.backgroundState.GeolocationController.location
89+
// via selectGeolocationLocation. Pass { geoLocation: 'US' } etc. to control it.
90+
const createTestStore = (initialState: Record<string, unknown> = {}) => {
91+
const { geoLocation, ...cardState } = initialState;
92+
const engineState = {
93+
backgroundState: {
94+
GeolocationController:
95+
typeof geoLocation === 'string' ? { location: geoLocation } : undefined,
96+
},
97+
};
98+
99+
return configureStore({
90100
reducer: {
101+
engine: (state = engineState, action = { type: '', payload: null }) => {
102+
switch (action.type) {
103+
default:
104+
return state;
105+
}
106+
},
91107
card: (
92108
state = {
93109
onboarding: {
@@ -97,8 +113,7 @@ const createTestStore = (initialState = {}) =>
97113
user: null,
98114
},
99115
userCardLocation: 'international',
100-
geoLocation: 'UNKNOWN',
101-
...initialState,
116+
...cardState,
102117
},
103118
action = { type: '', payload: null },
104119
) => {
@@ -114,6 +129,7 @@ const createTestStore = (initialState = {}) =>
114129
},
115130
},
116131
});
132+
};
117133

118134
describe('SignUp Component', () => {
119135
let store: ReturnType<typeof createTestStore>;

app/components/UI/Card/components/Onboarding/SignUp.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import { useDebouncedValue } from '../../../../hooks/useDebouncedValue';
3030
import useEmailVerificationSend from '../../hooks/useEmailVerificationSend';
3131
import useRegions from '../../hooks/useRegions';
3232
import {
33-
selectCardGeoLocation,
3433
setContactVerificationId,
3534
setUserCardLocation,
3635
} from '../../../../../core/redux/slices/card';
@@ -48,6 +47,7 @@ import {
4847
import SelectField from './SelectField';
4948
import { mapCountryToLocation } from '../../util/mapCountryToLocation';
5049
import type { Region } from '../../types';
50+
import { selectGeolocationLocation } from '../../../../../selectors/geolocationController';
5151

5252
const SignUp = () => {
5353
const navigation = useNavigation();
@@ -61,7 +61,7 @@ const SignUp = () => {
6161
const [isPasswordVisible, setIsPasswordVisible] = useState(false);
6262
const [selectedCountry, setSelectedCountry] = useState<Region | null>(null);
6363
const hasAutoSelectedCountry = useRef(false);
64-
const geoLocation = useSelector(selectCardGeoLocation);
64+
const geoLocation = useSelector(selectGeolocationLocation);
6565
const {
6666
signUpRegions,
6767
getRegionByCode,

app/components/UI/Card/hooks/isBaanxLoginEnabled.test.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,20 @@ const mockUseSelector = useSelector as jest.MockedFunction<typeof useSelector>;
1111
interface MockSelectorsParams {
1212
displayCardButtonFeatureFlag: boolean | null | undefined;
1313
alwaysShowCardButton: boolean | null | undefined;
14-
cardGeoLocation: string;
14+
geolocationLocation: string;
1515
cardSupportedCountries: Record<string, boolean> | null | undefined;
1616
}
1717

1818
const mockSelectors = ({
1919
displayCardButtonFeatureFlag,
2020
alwaysShowCardButton,
21-
cardGeoLocation,
21+
geolocationLocation,
2222
cardSupportedCountries,
2323
}: MockSelectorsParams) => {
2424
mockUseSelector
2525
.mockReturnValueOnce(displayCardButtonFeatureFlag)
2626
.mockReturnValueOnce(alwaysShowCardButton)
27-
.mockReturnValueOnce(cardGeoLocation)
27+
.mockReturnValueOnce(geolocationLocation)
2828
.mockReturnValueOnce(cardSupportedCountries);
2929
};
3030

@@ -38,7 +38,7 @@ describe('useIsBaanxLoginEnabled', () => {
3838
mockSelectors({
3939
displayCardButtonFeatureFlag: false,
4040
alwaysShowCardButton: true,
41-
cardGeoLocation: 'US',
41+
geolocationLocation: 'US',
4242
cardSupportedCountries: { US: false },
4343
});
4444

@@ -51,7 +51,7 @@ describe('useIsBaanxLoginEnabled', () => {
5151
mockSelectors({
5252
displayCardButtonFeatureFlag: true,
5353
alwaysShowCardButton: true,
54-
cardGeoLocation: 'XX',
54+
geolocationLocation: 'XX',
5555
cardSupportedCountries: {},
5656
});
5757

@@ -66,7 +66,7 @@ describe('useIsBaanxLoginEnabled', () => {
6666
mockSelectors({
6767
displayCardButtonFeatureFlag: true,
6868
alwaysShowCardButton: false,
69-
cardGeoLocation: 'US',
69+
geolocationLocation: 'US',
7070
cardSupportedCountries: { US: true },
7171
});
7272

@@ -79,7 +79,7 @@ describe('useIsBaanxLoginEnabled', () => {
7979
mockSelectors({
8080
displayCardButtonFeatureFlag: false,
8181
alwaysShowCardButton: false,
82-
cardGeoLocation: 'US',
82+
geolocationLocation: 'US',
8383
cardSupportedCountries: { US: true },
8484
});
8585

@@ -92,7 +92,7 @@ describe('useIsBaanxLoginEnabled', () => {
9292
mockSelectors({
9393
displayCardButtonFeatureFlag: true,
9494
alwaysShowCardButton: false,
95-
cardGeoLocation: 'XX',
95+
geolocationLocation: 'XX',
9696
cardSupportedCountries: { US: true, GB: true },
9797
});
9898

@@ -105,7 +105,7 @@ describe('useIsBaanxLoginEnabled', () => {
105105
mockSelectors({
106106
displayCardButtonFeatureFlag: true,
107107
alwaysShowCardButton: false,
108-
cardGeoLocation: 'US',
108+
geolocationLocation: 'US',
109109
cardSupportedCountries: { US: false },
110110
});
111111

@@ -120,7 +120,7 @@ describe('useIsBaanxLoginEnabled', () => {
120120
mockSelectors({
121121
displayCardButtonFeatureFlag: null,
122122
alwaysShowCardButton: null,
123-
cardGeoLocation: 'US',
123+
geolocationLocation: 'US',
124124
cardSupportedCountries: null,
125125
});
126126

@@ -133,7 +133,7 @@ describe('useIsBaanxLoginEnabled', () => {
133133
mockSelectors({
134134
displayCardButtonFeatureFlag: undefined,
135135
alwaysShowCardButton: undefined,
136-
cardGeoLocation: 'US',
136+
geolocationLocation: 'US',
137137
cardSupportedCountries: undefined,
138138
});
139139

@@ -146,7 +146,7 @@ describe('useIsBaanxLoginEnabled', () => {
146146
mockSelectors({
147147
displayCardButtonFeatureFlag: true,
148148
alwaysShowCardButton: false,
149-
cardGeoLocation: 'US',
149+
geolocationLocation: 'US',
150150
cardSupportedCountries: {},
151151
});
152152

@@ -155,11 +155,11 @@ describe('useIsBaanxLoginEnabled', () => {
155155
expect(result.current).toBe(false);
156156
});
157157

158-
it('returns false when cardGeoLocation is empty string', () => {
158+
it('returns false when geolocationLocation is empty string', () => {
159159
mockSelectors({
160160
displayCardButtonFeatureFlag: true,
161161
alwaysShowCardButton: false,
162-
cardGeoLocation: '',
162+
geolocationLocation: '',
163163
cardSupportedCountries: { US: true },
164164
});
165165

@@ -174,7 +174,7 @@ describe('useIsBaanxLoginEnabled', () => {
174174
mockSelectors({
175175
displayCardButtonFeatureFlag: true,
176176
alwaysShowCardButton: false,
177-
cardGeoLocation: 'GB',
177+
geolocationLocation: 'GB',
178178
cardSupportedCountries: { US: true, GB: true, CA: true },
179179
});
180180

@@ -187,7 +187,7 @@ describe('useIsBaanxLoginEnabled', () => {
187187
mockSelectors({
188188
displayCardButtonFeatureFlag: true,
189189
alwaysShowCardButton: false,
190-
cardGeoLocation: 'DE',
190+
geolocationLocation: 'DE',
191191
cardSupportedCountries: { US: true, GB: true, CA: true },
192192
});
193193

@@ -201,7 +201,7 @@ describe('useIsBaanxLoginEnabled', () => {
201201
mockSelectors({
202202
displayCardButtonFeatureFlag: false,
203203
alwaysShowCardButton: false,
204-
cardGeoLocation: 'US',
204+
geolocationLocation: 'US',
205205
cardSupportedCountries: { US: true },
206206
});
207207
const { result, rerender } = renderHook(() => useIsBaanxLoginEnabled());
@@ -211,7 +211,7 @@ describe('useIsBaanxLoginEnabled', () => {
211211
mockSelectors({
212212
displayCardButtonFeatureFlag: false,
213213
alwaysShowCardButton: true,
214-
cardGeoLocation: 'US',
214+
geolocationLocation: 'US',
215215
cardSupportedCountries: { US: true },
216216
});
217217
rerender();

app/components/UI/Card/hooks/isBaanxLoginEnabled.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,17 @@ import {
33
selectCardSupportedCountries,
44
selectDisplayCardButtonFeatureFlag,
55
} from '../../../../selectors/featureFlagController/card';
6-
import {
7-
selectAlwaysShowCardButton,
8-
selectCardGeoLocation,
9-
} from '../../../../core/redux/slices/card';
6+
import { selectAlwaysShowCardButton } from '../../../../core/redux/slices/card';
7+
import { selectGeolocationLocation } from '../../../../selectors/geolocationController';
108

119
export const isBaanxLoginEnabled = (params: {
1210
alwaysShowCardButton: boolean;
13-
cardGeoLocation: string;
11+
geolocationLocation: string;
1412
cardSupportedCountries: Record<string, boolean>;
1513
displayCardButtonFeatureFlag: boolean;
1614
}) =>
1715
params.alwaysShowCardButton ||
18-
(params.cardSupportedCountries?.[params.cardGeoLocation] === true &&
16+
(params.cardSupportedCountries?.[params.geolocationLocation] === true &&
1917
params.displayCardButtonFeatureFlag) ||
2018
false;
2119

@@ -24,15 +22,15 @@ const useIsBaanxLoginEnabled = () => {
2422
selectDisplayCardButtonFeatureFlag,
2523
);
2624
const alwaysShowCardButton = useSelector(selectAlwaysShowCardButton);
27-
const cardGeoLocation = useSelector(selectCardGeoLocation);
25+
const geolocationLocation = useSelector(selectGeolocationLocation);
2826
const cardSupportedCountries = useSelector(selectCardSupportedCountries);
2927

3028
// If user has explicitly enabled the experimental switch,
3129
// they should have full access to the feature including authentication/onboarding,
3230
// regardless of the progressive rollout flag state
3331
return isBaanxLoginEnabled({
3432
alwaysShowCardButton,
35-
cardGeoLocation,
33+
geolocationLocation,
3634
displayCardButtonFeatureFlag,
3735
cardSupportedCountries: cardSupportedCountries as Record<string, boolean>,
3836
});

app/components/UI/Card/hooks/useCardAuthenticationVerification.test.ts

Lines changed: 27 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,23 +33,23 @@ describe('useCardAuthenticationVerification', () => {
3333

3434
/**
3535
* Sets up mocks for the hook's dependencies.
36-
* useSelector is called 3 times in order: userLoggedIn, isAuthenticated, cardGeoLocation
36+
* useSelector is called 3 times in order: userLoggedIn, isAuthenticated, geolocationLocation
3737
*/
3838
const setupMocks = ({
3939
userLoggedIn,
4040
isBaanxLoginEnabled,
4141
isAuthenticated = false,
42-
cardGeoLocation = 'UNKNOWN',
42+
geolocationLocation = 'UNKNOWN',
4343
}: {
4444
userLoggedIn: boolean;
4545
isBaanxLoginEnabled: boolean;
4646
isAuthenticated?: boolean;
47-
cardGeoLocation?: string;
47+
geolocationLocation?: string;
4848
}) => {
4949
mockUseSelector
5050
.mockReturnValueOnce(userLoggedIn)
5151
.mockReturnValueOnce(isAuthenticated)
52-
.mockReturnValueOnce(cardGeoLocation);
52+
.mockReturnValueOnce(geolocationLocation);
5353
mockUseIsBaanxLoginEnabled.mockReturnValue(isBaanxLoginEnabled);
5454
};
5555

@@ -69,7 +69,7 @@ describe('useCardAuthenticationVerification', () => {
6969
setupMocks({
7070
userLoggedIn: true,
7171
isBaanxLoginEnabled: false,
72-
cardGeoLocation: 'UNKNOWN',
72+
geolocationLocation: 'UNKNOWN',
7373
});
7474

7575
renderHook(() => useCardAuthenticationVerification());
@@ -172,25 +172,38 @@ describe('useCardAuthenticationVerification', () => {
172172
});
173173

174174
describe('session clearing when isBaanxLoginEnabled is false', () => {
175-
it('does not reset auth when geoLocation is UNKNOWN (transient geo failure)', () => {
175+
it('does not reset auth when geolocationLocation is UNKNOWN (transient geo failure)', () => {
176176
setupMocks({
177177
userLoggedIn: true,
178178
isBaanxLoginEnabled: false,
179179
isAuthenticated: true,
180-
cardGeoLocation: 'UNKNOWN',
180+
geolocationLocation: 'UNKNOWN',
181181
});
182182

183183
renderHook(() => useCardAuthenticationVerification());
184184

185185
expect(mockDispatch).not.toHaveBeenCalled();
186186
});
187187

188-
it('resets auth when geoLocation is resolved and feature is disabled for that region', () => {
188+
it('does not reset auth when geolocationLocation is undefined (geo not loaded yet)', () => {
189189
setupMocks({
190190
userLoggedIn: true,
191191
isBaanxLoginEnabled: false,
192192
isAuthenticated: true,
193-
cardGeoLocation: 'FR',
193+
geolocationLocation: undefined,
194+
});
195+
196+
renderHook(() => useCardAuthenticationVerification());
197+
198+
expect(mockDispatch).not.toHaveBeenCalled();
199+
});
200+
201+
it('resets auth when geolocationLocation is resolved and feature is disabled for that region', () => {
202+
setupMocks({
203+
userLoggedIn: true,
204+
isBaanxLoginEnabled: false,
205+
isAuthenticated: true,
206+
geolocationLocation: 'FR',
194207
});
195208

196209
renderHook(() => useCardAuthenticationVerification());
@@ -203,7 +216,7 @@ describe('useCardAuthenticationVerification', () => {
203216
userLoggedIn: true,
204217
isBaanxLoginEnabled: false,
205218
isAuthenticated: false,
206-
cardGeoLocation: 'FR',
219+
geolocationLocation: 'FR',
207220
});
208221

209222
renderHook(() => useCardAuthenticationVerification());
@@ -217,7 +230,7 @@ describe('useCardAuthenticationVerification', () => {
217230
userLoggedIn: true,
218231
isBaanxLoginEnabled: true,
219232
isAuthenticated: true,
220-
cardGeoLocation: 'US',
233+
geolocationLocation: 'US',
221234
});
222235
const { rerender } = renderHook(() =>
223236
useCardAuthenticationVerification(),
@@ -232,7 +245,7 @@ describe('useCardAuthenticationVerification', () => {
232245
userLoggedIn: true,
233246
isBaanxLoginEnabled: false,
234247
isAuthenticated: true,
235-
cardGeoLocation: 'UNKNOWN',
248+
geolocationLocation: 'UNKNOWN',
236249
});
237250
rerender();
238251

@@ -245,7 +258,7 @@ describe('useCardAuthenticationVerification', () => {
245258
userLoggedIn: true,
246259
isBaanxLoginEnabled: false,
247260
isAuthenticated: true,
248-
cardGeoLocation: 'UNKNOWN',
261+
geolocationLocation: 'UNKNOWN',
249262
});
250263
const { rerender } = renderHook(() =>
251264
useCardAuthenticationVerification(),
@@ -258,7 +271,7 @@ describe('useCardAuthenticationVerification', () => {
258271
userLoggedIn: true,
259272
isBaanxLoginEnabled: false,
260273
isAuthenticated: true,
261-
cardGeoLocation: 'FR',
274+
geolocationLocation: 'FR',
262275
});
263276
rerender();
264277

0 commit comments

Comments
 (0)