Skip to content

Commit f258651

Browse files
tuna1207lionellbrionesmetamaskbotchaitanyapotti
authored
feat: update subscription check out url handling (#36161)
<!-- 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** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> Previously we were using extension identity API's `launchWebAuthFlow` for handling subscription checkout url which should have been used for oauth only and will open a controlled window which doesn't support other extension. This PR open new tab to handle checkout with check for success url redirect instead from both popup and full screen mode. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/36161?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: Subscription check out URL open new tab ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to `#shield-plan` page 2. Start subscription check out ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> https://github.com/user-attachments/assets/d83c675a-7dc5-45ca-91ec-7e9acf2ce83d https://github.com/user-attachments/assets/79dbd261-2143-4a6d-80fc-3acf8f19f91e ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/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-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [x] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [x] 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. --------- Co-authored-by: Lionell Briones <[email protected]> Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: Chaitanya Potti <[email protected]>
1 parent fd1ecb6 commit f258651

File tree

7 files changed

+94
-27
lines changed

7 files changed

+94
-27
lines changed

‎app/scripts/metamask-controller.js‎

Lines changed: 67 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ import { NameControllerInit } from './controller-init/confirmations/name-control
382382
import { GasFeeControllerInit } from './controller-init/confirmations/gas-fee-controller-init';
383383
import { SelectedNetworkControllerInit } from './controller-init/selected-network-controller-init';
384384
import { SubscriptionControllerInit } from './controller-init/subscription';
385-
import { webAuthenticatorFactory } from './services/oauth/web-authenticator-factory';
385+
import { getIdentityAPI } from './services/oauth/web-authenticator-factory';
386386
import { AccountTrackerControllerInit } from './controller-init/account-tracker-controller-init';
387387
import { OnboardingControllerInit } from './controller-init/onboarding-controller-init';
388388
import { RemoteFeatureFlagControllerInit } from './controller-init/remote-feature-flag-controller-init';
@@ -2229,29 +2229,77 @@ export default class MetamaskController extends EventEmitter {
22292229
return publicConfigStore;
22302230
}
22312231

2232-
async startSubscriptionWithCard(params) {
2233-
const webAuthenticator = webAuthenticatorFactory();
2232+
async startSubscriptionWithCard(
2233+
params,
2234+
/* current tab can be undefined if open from non tab context (e.g. popup, background) */
2235+
currentTabId,
2236+
) {
2237+
const identityAPI = getIdentityAPI();
2238+
const redirectUrl = identityAPI.getRedirectURL();
2239+
22342240
const { checkoutSessionUrl } =
2235-
await this.subscriptionController.startShieldSubscriptionWithCard(params);
2236-
// TODO: use chrome.tabs manually to have full browser feature in checkout session (e.g auto-fill form, etc.)
2237-
// use same launchWebAuthFlow api as oauth service to launch the stripe checkout session and get redirected back to extension from a pop up
2238-
// without having to handle chrome.windows.create and chrome.tabs.onUpdated event explicitly
2241+
await this.subscriptionController.startShieldSubscriptionWithCard({
2242+
...params,
2243+
successUrl: redirectUrl,
2244+
});
2245+
2246+
const checkoutTab = await this.platform.openTab({
2247+
url: checkoutSessionUrl,
2248+
});
2249+
2250+
// --- We will define our listeners here so we can reference them for cleanup ---
2251+
// eslint-disable-next-line prefer-const
2252+
let onTabUpdatedListener;
2253+
// eslint-disable-next-line prefer-const
2254+
let onTabRemovedListener;
2255+
22392256
await new Promise((resolve, reject) => {
2240-
webAuthenticator.launchWebAuthFlow(
2241-
{
2242-
url: checkoutSessionUrl,
2243-
interactive: true,
2244-
},
2245-
(responseUrl) => {
2246-
try {
2247-
resolve(responseUrl);
2248-
} catch (error) {
2249-
reject(error);
2257+
let checkoutSucceeded = false;
2258+
const cleanupListeners = () => {
2259+
// Important: Remove both listeners to prevent memory leaks
2260+
if (onTabUpdatedListener) {
2261+
this.platform.removeTabUpdatedListener(onTabUpdatedListener);
2262+
}
2263+
if (onTabRemovedListener) {
2264+
this.platform.removeTabRemovedListener(onTabRemovedListener);
2265+
}
2266+
};
2267+
2268+
// Set up a listener to watch for navigation on that specific tab
2269+
onTabUpdatedListener = (tabId, changeInfo, _tab) => {
2270+
// We only care about updates to our specific checkout tab
2271+
if (tabId === checkoutTab.id && changeInfo.url) {
2272+
if (changeInfo.url.startsWith(redirectUrl)) {
2273+
// Payment was successful!
2274+
checkoutSucceeded = true;
2275+
2276+
// Clean up: close the tab
2277+
this.platform.closeTab(tabId);
22502278
}
2251-
},
2252-
);
2279+
// TODO: handle cancel url ?
2280+
}
2281+
};
2282+
this.platform.addTabUpdatedListener(onTabUpdatedListener);
2283+
2284+
// Set up a listener to watch for tab removal
2285+
onTabRemovedListener = (tabId) => {
2286+
if (tabId === checkoutTab.id) {
2287+
cleanupListeners();
2288+
if (checkoutSucceeded) {
2289+
resolve();
2290+
} else {
2291+
reject(new Error('Checkout failed'));
2292+
}
2293+
}
2294+
};
2295+
this.platform.addTabRemovedListener(onTabRemovedListener);
22532296
});
22542297

2298+
if (!currentTabId) {
2299+
// open extension browser shield settings if open from pop up (no current tab)
2300+
this.platform.openExtensionInBrowser('/settings/transaction-shield');
2301+
}
2302+
22552303
// fetch latest user subscriptions after checkout
22562304
const subscriptions = await this.subscriptionController.getSubscriptions();
22572305
return subscriptions;

‎app/scripts/platforms/extension.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,22 @@ export default class ExtensionPlatform {
147147
browser.windows.onRemoved.addListener(listener);
148148
}
149149

150+
addTabRemovedListener(listener) {
151+
browser.tabs.onRemoved.addListener(listener);
152+
}
153+
154+
removeTabRemovedListener(listener) {
155+
browser.tabs.onRemoved.removeListener(listener);
156+
}
157+
158+
addTabUpdatedListener(listener) {
159+
browser.tabs.onUpdated.addListener(listener);
160+
}
161+
162+
removeTabUpdatedListener(listener) {
163+
browser.tabs.onUpdated.removeListener(listener);
164+
}
165+
150166
async getAllWindows() {
151167
const windows = await browser.windows.getAll();
152168
return windows;

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@
358358
"@metamask/solana-wallet-snap": "^2.4.0",
359359
"@metamask/solana-wallet-standard": "^0.6.0",
360360
"@metamask/streams": "^0.4.0",
361-
"@metamask/subscription-controller": "^0.1.0",
361+
"@metamask/subscription-controller": "^0.2.0",
362362
"@metamask/transaction-controller": "^60.2.0",
363363
"@metamask/user-operation-controller": "^39.0.0",
364364
"@metamask/utils": "^11.4.2",

‎ui/pages/settings/transaction-shield-tab/cancel-membership-modal.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ const mockSubscription: Subscription = {
2020
card: {
2121
brand: 'Visa',
2222
last4: '1234',
23+
displayBrand: 'Visa',
2324
},
2425
},
2526
};

‎ui/pages/settings/transaction-shield-tab/transaction-shield.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ describe('Transaction Shield Page', () => {
4444
card: {
4545
brand: 'Visa',
4646
last4: '1234',
47+
displayBrand: 'Visa',
4748
},
4849
},
4950
} satisfies Subscription,

‎ui/store/actions.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,9 +392,10 @@ export function startSubscriptionWithCard(params: {
392392
recurringInterval: RecurringInterval;
393393
}): ThunkAction<Subscription[], MetaMaskReduxState, unknown, AnyAction> {
394394
return async (_dispatch: MetaMaskReduxDispatch) => {
395+
const currentTab = await global.platform.currentTab();
395396
const subscriptions = await submitRequestToBackground<Subscription[]>(
396397
'startSubscriptionWithCard',
397-
[params],
398+
[params, currentTab?.id],
398399
);
399400

400401
return subscriptions;

‎yarn.lock‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7602,16 +7602,16 @@ __metadata:
76027602
languageName: node
76037603
linkType: hard
76047604

7605-
"@metamask/subscription-controller@npm:^0.1.0":
7606-
version: 0.1.0
7607-
resolution: "@metamask/subscription-controller@npm:0.1.0"
7605+
"@metamask/subscription-controller@npm:^0.2.0":
7606+
version: 0.2.0
7607+
resolution: "@metamask/subscription-controller@npm:0.2.0"
76087608
dependencies:
7609-
"@metamask/base-controller": "npm:^8.3.0"
7609+
"@metamask/base-controller": "npm:^8.4.0"
76107610
"@metamask/controller-utils": "npm:^11.14.0"
76117611
"@metamask/utils": "npm:^11.8.0"
76127612
peerDependencies:
76137613
"@metamask/profile-sync-controller": ^25.0.0
7614-
checksum: 10/f99dd74b6b057a357676447222f75a3753ce7522e2ff61002de46809bd67bcc3b8d71d246260aae302c3a458fec62ff543b03cf018d89bf77f1734292d24e6ca
7614+
checksum: 10/c97a25966f6a4f3670bb096e4b31e5c458705fd8224bbe6bc861104ed03ac775ae4d7d1038c62a31e3327cc079ae0cf1920e64407b868f5e4d147aebb057789b
76157615
languageName: node
76167616
linkType: hard
76177617

@@ -32164,7 +32164,7 @@ __metadata:
3216432164
"@metamask/solana-wallet-snap": "npm:^2.4.0"
3216532165
"@metamask/solana-wallet-standard": "npm:^0.6.0"
3216632166
"@metamask/streams": "npm:^0.4.0"
32167-
"@metamask/subscription-controller": "npm:^0.1.0"
32167+
"@metamask/subscription-controller": "npm:^0.2.0"
3216832168
"@metamask/superstruct": "npm:^3.2.1"
3216932169
"@metamask/test-bundler": "npm:^1.0.0"
3217032170
"@metamask/test-dapp": "npm:9.3.0"

0 commit comments

Comments
 (0)