feat: enable STX by default with migration and notification #12857
+646
−7
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Enable STX for new users or those that have not enabled in settings - Preserves settings for users who explicitly opted out and have existing STX transactions - Adds a migration flag to track this change
…ror messages using team's established pattern.
- Mock Sentry error handling ensuring we can track when errors are captured - Setup `beforeEach()` restore/reset for all mocks - Define test cases for error handling - Each case has: invalid input state, expected error message, and description - Invalid States Test: how migration handles malformed data - Runs each invalid state test case - Verifies: Invalid state returns unchanged, Error is captured by Sentry, Error message matches expected format Also, Setup first test to ensure STX is enabled and migration flag is set when undefined opt-in status
- Null status test, similar to undefined opt-in status test - Tests migration with null status - Verifies that STX is enabled and migration flag is set
httpJunkie
added
team-confirmations
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
- create opt-out test - tests that users explicit opt-out is respected when STX transactions exist - verifies migration flag is set but preference is unchanged
- create opt-out test - tests that users explicit opt-out is respected when STX transactions exist - verifies migration flag is set but preference is unchanged
- tests that opt-out is overridden when user has no transaction history - verifies migration flag is set and preference is changed
- added test for initializing preferences - added test for when STX is already enabled - use `merge` with `initialRootState` consistently across all tests
dan437
reviewed
Jan 8, 2025
- uses redux selectors to access the state - checks flags we need (opt-in status and migration applied) - Returns (isEnabled, isMigrationApplied) for our banner to use - Provides proper TypeScript types
- check default states - ensure proper state reading - handle partial states
- test conditional rendering - check text content - ensure close button functionality works - ensure link functionality works
- useSmartTransactionsEnabled enhanced to include dismiss functionality - add a dispatch action to update the smartTransactionsOptInStatus - remove external onClose prop from SmartTransactionsEnabledBanner in favor of using the hook's dismiss functionality - update component and hook tests to account for new functionality These changes require more changes in the next commit to add the smartTransactionsBannerDismissed field to our PreferencesController state as well to add a reducer case for SET_SMART_TRANSACTIONS_BANNER_DISMISSED and update any types/interfaces in our codebase that reference PreferencesController.
…nitialize it. Update the migration and test to account for the new change
dan437
reviewed
Jan 10, 2025
...s/confirmations/components/SmartTransactionsEnabledBanner/SmartTransactionsEnabledBanner.tsx
Outdated
Show resolved
Hide resolved
app/components/hooks/useSmartTransactionsEnabled/useSmartTransactionsEnabled.ts
Outdated
Show resolved
Hide resolved
...irmations/components/SmartTransactionsEnabledBanner/SmartTransactionsEnabledBanner.styles.ts
Outdated
Show resolved
Hide resolved
…ask-mobile into feat/enable-stx-migration
…ask-mobile into feat/enable-stx-migration rename SmartTransactionsMigrationBanner
…tead. - Create new selectors file - Update the SmartTransactionsMigrationBanner component to use these selectors directly - Delete the hook file since its functionality would now be handled by selectors
dan437
reviewed
Jan 17, 2025
...nfirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.tsx
Outdated
Show resolved
Hide resolved
app/components/Views/confirmations/components/TransactionReview/index.js
Outdated
Show resolved
Hide resolved
…roller featureFlags - Move smartTransactionsMigrationApplied from PreferencesController root to featureFlags object - Aligns with existing pattern of using featureFlags for UI states - Maintains consistency with smartTransactionsBannerDismissed placement
|
I have read the CLA Document and I hereby sign the CLA |
|
dan437
reviewed
Jan 17, 2025
| } | ||
| onClose={dismissBanner} | ||
| style={styles.banner} | ||
| testID="smart-transactions-enabled-banner" |
There was a problem hiding this comment.
nit: I would call it after the component's name, in this case: smart-transactions-migration-banner
- TransactionReview - Swaps Quote View
…thin featureFlags on component test
httpJunkie
added
Run Smoke E2E
|
|
httpJunkie
added
Run Smoke E2E
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR enables Smart Transactions (STX) in MetaMask Mobile by default through migration number (TBD) for users who have either opted out or haven't interacted with the STX toggle, provided they have no recorded STX activity.
How it works (if user does not have STX enabled or prior STX Transaction history:
In the case a user migrates from a previous version of the Mobile app and the migration runs and sets STX toggle "ON" in
Settings > Advanced > Smart Transactions, they will receive an Alert on transaction confirmation screens until dismissed, or by clicking on the "Higher success rates" link within the alert. If they click on the link in the banner alert they will get sent to: What is 'Smart Transactions'? for more information. When returning to the confirmation they just navigated from the banner alert should not show and should never show again.Edge Cases:
If a user is new and setting up a wallet for the first time, they will not receive the Banner Alert. If a user imports a new wallet during a fresh install of the extension on a new browser or recovers a wallet, it's possible they may not see the alert if STX was on in a previous install. The STX Banner Alert is dismissed and will not show again if a user is in the state to get shown the banner and toggles STX off independently even if they do not physically dismiss the STX Banner Alert.
Migration Logic:
smartTransactionsOptInStatusisnull(new/never interacted)UI Components:
TBD, still exploring the Mobile UI for implementation solution.
Running Unit Tests
Migration Test:
yarn jest "./app/store/migrations/096.test.ts" --no-cacheComponent Test:
yarn jest "./app/components/Views/confirmations/components/SmartTransactionsMigrationBanner/SmartTransactionsMigrationBanner.test.ts" --no-cacheManual testing steps
Test Migration (using a wallet/account with no STX Transactions)
Start with an older repo version:
Terminal #1
yarn setup && yarn watchTerminal #1
Import or setup a wallet without STX transactions, launch the wallet (do not enable STX if prompted), check that toggle is OFF in:
Settings > Advanced > Smart TransactionsSwitch to feature branch and run app:
Terminal #1
yarn setup && yarn watchTerminal #1
0.0001ETH7 Ensure that Smart Transactions Banner Alert IS showing
0.0001ETHScreenshots/Recordings
N/A yet...
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist