diff --git a/packages/ramps-controller/CHANGELOG.md b/packages/ramps-controller/CHANGELOG.md index 1a9103c53b4..5d449b245bb 100644 --- a/packages/ramps-controller/CHANGELOG.md +++ b/packages/ramps-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `status` field to `ResourceState` to distinguish between uninitialized and empty-fetched states ([#8116](https://github.com/MetaMask/core/pull/8116)) + ## [10.2.0] ### Fixed diff --git a/packages/ramps-controller/src/RampsController.test.ts b/packages/ramps-controller/src/RampsController.test.ts index 9d745fe7011..6eabad3f657 100644 --- a/packages/ramps-controller/src/RampsController.test.ts +++ b/packages/ramps-controller/src/RampsController.test.ts @@ -92,6 +92,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -100,6 +101,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -107,12 +109,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -122,12 +126,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -135,6 +141,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -167,6 +174,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -175,6 +183,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -182,12 +191,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -197,12 +208,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -210,6 +223,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -743,6 +757,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -751,6 +766,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -758,12 +774,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -773,12 +791,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -786,6 +806,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -808,6 +829,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "orders": [], "paymentMethods": { @@ -815,18 +837,21 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "tokens": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -849,6 +874,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "orders": [], "providers": { @@ -856,12 +882,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "tokens": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -884,6 +912,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "nativeProviders": { "transak": { @@ -892,6 +921,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -899,12 +929,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, }, }, @@ -914,12 +946,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "providers": { "data": [], "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "requests": {}, "tokens": { @@ -927,6 +961,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userRegion": null, } @@ -4977,7 +5012,7 @@ describe('RampsController', () => { }); }); - it('returns null when service returns BuyWidget with null url', async () => { + it('returns null when service returns BuyWidget with empty url', async () => { await withController(async ({ controller, rootMessenger }) => { const quote: Quote = { provider: '/providers/transak-staging', @@ -4993,7 +5028,7 @@ describe('RampsController', () => { rootMessenger.registerActionHandler( 'RampsService:getBuyWidgetUrl', async () => ({ - url: null, + url: '', browser: 'APP_BROWSER' as const, orderId: null, }), @@ -5001,7 +5036,7 @@ describe('RampsController', () => { const widgetUrl = await controller.getWidgetUrl(quote); - expect(widgetUrl).toBeNull(); + expect(widgetUrl).toBe(''); }); }); }); @@ -5336,7 +5371,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'poll-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5404,7 +5442,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'status-change-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5441,7 +5482,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'no-change-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5471,7 +5515,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'terminal-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5501,7 +5548,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'unknown-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5531,7 +5581,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'error-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5581,7 +5634,10 @@ describe('RampsController', () => { const orderNoId = createMockOrder({ providerOrderId: '', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(orderNoId); @@ -5603,7 +5659,10 @@ describe('RampsController', () => { const orderNoWallet = createMockOrder({ providerOrderId: 'no-wallet-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '', }); controller.addOrder(orderNoWallet); @@ -5625,7 +5684,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'strip-prefix-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5654,7 +5716,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'backoff-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5684,7 +5749,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'poll-min-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', pollingSecondsMinimum: 120, }); @@ -5754,7 +5822,10 @@ describe('RampsController', () => { const completedOrder = createMockOrder({ providerOrderId: 'completed-1', status: RampsOrderStatus.Completed, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(completedOrder); @@ -5776,7 +5847,10 @@ describe('RampsController', () => { const order = createMockOrder({ providerOrderId: 'reset-err-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(order); @@ -5812,7 +5886,10 @@ describe('RampsController', () => { const pendingOrder = createMockOrder({ providerOrderId: 'race-1', status: RampsOrderStatus.Pending, - provider: { id: '/providers/transak', name: 'Transak' }, + provider: createMockProvider({ + id: '/providers/transak', + name: 'Transak', + }), walletAddress: '0xabc', }); controller.addOrder(pendingOrder); @@ -5975,6 +6052,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "isAuthenticated": false, "kycRequirement": { @@ -5982,12 +6060,14 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", }, "userDetails": { "data": null, "error": null, "isLoading": false, "selected": null, + "status": "idle", }, } `); @@ -6191,6 +6271,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -6333,6 +6414,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -6415,6 +6497,7 @@ describe('RampsController', () => { "error": null, "isLoading": false, "selected": null, + "status": "idle", } `); }); @@ -7196,6 +7279,7 @@ function createResourceState( selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; } @@ -7244,14 +7328,27 @@ function createMockDepositOrder(): TransakDepositOrder { }; } +function createMockProvider(overrides: Partial = {}): Provider { + return { + id: '/providers/test-provider', + name: 'Test Provider', + environmentType: 'production', + description: 'Test provider description', + hqAddress: '123 Test St', + links: [], + logos: { light: '', dark: '', height: 32, width: 32 }, + ...overrides, + }; +} + function createMockOrder(overrides: Partial = {}): RampsOrder { return { id: '/providers/transak-staging/orders/abc-123', isOnlyLink: false, - provider: { + provider: createMockProvider({ id: '/providers/transak-staging', name: 'Transak (Staging)', - }, + }), success: true, cryptoAmount: 0.05, fiatAmount: 100, diff --git a/packages/ramps-controller/src/RampsController.ts b/packages/ramps-controller/src/RampsController.ts index dde0bb9b645..6fc703d6b95 100644 --- a/packages/ramps-controller/src/RampsController.ts +++ b/packages/ramps-controller/src/RampsController.ts @@ -197,6 +197,11 @@ export type ResourceState = { * Error message if the fetch failed, or null. */ error: string | null; + /** + * The current status of the resource: 'idle' | 'loading' | 'success' | 'error'. + * Distinguishes between never-fetched ('idle') and successfully-fetched-empty ('success'). + */ + status: `${RequestStatus}`; }; /** @@ -338,6 +343,7 @@ function createDefaultResourceState( selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; } @@ -402,6 +408,7 @@ function resetResource( resource.selected = def.selected; resource.isLoading = def.isLoading; resource.error = def.error; + resource.status = def.status; } /** @@ -827,12 +834,17 @@ export class RampsController extends BaseController< const count = this.#pendingResourceCount.get(resourceType) ?? 0; this.#pendingResourceCount.set(resourceType, count + 1); if (count === 0) { - this.#setResourceLoading(resourceType, true); + this.#setResourceLoadingAndStatus( + resourceType, + true, + RequestStatus.LOADING, + ); } } // Create the fetch promise const promise = (async (): Promise => { + let terminalStatus: RequestStatus | undefined; try { const data = await fetcher(abortController.signal); @@ -850,6 +862,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, null); + terminalStatus = RequestStatus.SUCCESS; } } return data; @@ -868,6 +881,7 @@ export class RampsController extends BaseController< !options?.isResultCurrent || options.isResultCurrent(); if (isCurrent) { this.#setResourceError(resourceType, errorMessage); + terminalStatus = RequestStatus.ERROR; } } throw error; @@ -885,7 +899,11 @@ export class RampsController extends BaseController< const next = Math.max(0, count - 1); if (next === 0) { this.#pendingResourceCount.delete(resourceType); - this.#setResourceLoading(resourceType, false); + this.#setResourceLoadingAndStatus( + resourceType, + false, + terminalStatus ?? RequestStatus.IDLE, + ); } else { this.#pendingResourceCount.set(resourceType, next); } @@ -986,45 +1004,52 @@ export class RampsController extends BaseController< } /** - * Updates a single field (isLoading or error) on a resource state. - * All resources share the same ResourceState structure, so we use - * dynamic property access to avoid duplicating switch statements. + * Updates one or more fields on a resource state atomically in a single + * `this.update()` call. All resources share the same ResourceState structure, + * so we use dynamic property access to avoid duplicating switch statements. * * @param resourceType - The type of resource. - * @param field - The field to update ('isLoading' or 'error'). - * @param value - The value to set. + * @param fields - An object mapping field names to their new values. */ - #updateResourceField( + #updateResourceFields( resourceType: ResourceType, - field: 'isLoading' | 'error', - value: boolean | string | null, + fields: Partial< + Record<'isLoading' | 'error' | 'status', boolean | string | null> + >, ): void { this.update((state) => { const resource = state[resourceType]; if (resource) { - (resource as Record)[field] = value; + for (const [field, value] of Object.entries(fields)) { + (resource as Record)[field] = value; + } } }); } /** - * Sets the loading state for a resource type. + * Sets the error state for a resource type. * * @param resourceType - The type of resource. - * @param loading - Whether the resource is loading. + * @param error - The error message, or null to clear. */ - #setResourceLoading(resourceType: ResourceType, loading: boolean): void { - this.#updateResourceField(resourceType, 'isLoading', loading); + #setResourceError(resourceType: ResourceType, error: string | null): void { + this.#updateResourceFields(resourceType, { error }); } /** - * Sets the error state for a resource type. + * Sets the loading state and status for a resource type atomically. * * @param resourceType - The type of resource. - * @param error - The error message, or null to clear. + * @param loading - Whether the resource is loading. + * @param status - The status to set ('idle' | 'loading' | 'success' | 'error'). */ - #setResourceError(resourceType: ResourceType, error: string | null): void { - this.#updateResourceField(resourceType, 'error', error); + #setResourceLoadingAndStatus( + resourceType: ResourceType, + loading: boolean, + status: `${RequestStatus}`, + ): void { + this.#updateResourceFields(resourceType, { isLoading: loading, status }); } /** @@ -1866,7 +1891,7 @@ export class RampsController extends BaseController< 'RampsService:getBuyWidgetUrl', buyUrl, ); - return buyWidget.url ?? null; + return buyWidget.url; } catch { return null; } diff --git a/packages/ramps-controller/src/selectors.test.ts b/packages/ramps-controller/src/selectors.test.ts index 459ad64dedf..2a9d1e0d66b 100644 --- a/packages/ramps-controller/src/selectors.test.ts +++ b/packages/ramps-controller/src/selectors.test.ts @@ -3,6 +3,7 @@ import { createLoadingState, createSuccessState, createErrorState, + RequestStatus, } from './RequestCache'; import { createRequestSelector } from './selectors'; @@ -18,12 +19,14 @@ function createDefaultResourceState( selected: TSelected; isLoading: boolean; error: null; + status: `${RequestStatus}`; } { return { data, selected, isLoading: false, error: null, + status: RequestStatus.IDLE, }; }