Temp/notification integration sdk fixes #10116
Closed
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
…s management UI redesign (#9961) <!-- 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** Adding search to the network bottomsheet as part of the networks management UI redesign. Here is a 2 minute overview demo: https://www.loom.com/share/dc29313a5cd34e9893c76a0caeba0fc1 Here is a link to the related Figma that includes the adding of the search box to the bottom sheet: https://www.figma.com/design/76R5BvAVubUhWaN2Ld7MAv/Default-Networks?node-id=1303-9439&t=yhxfHHCsG0CimP0w-0 <!-- 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? --> ## **Related issues** Fixes: MetaMask/MetaMask-planning#2495 ## **Manual testing steps** 1. Add a the following env var that will be used as a feature flag to enable to the feature export MM_NETWORK_UI_REDESIGN_ENABLED="1" 2. From the header in the home screen, click the network selector 3. The network bottom sheet should come up and you should see the search box 4. Type ava in the search box and it should filter out all other networks and show only avalanche 5. For a demo please see the video in the description above ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. --------- Co-authored-by: salimtb <[email protected]>
…notifications-integration-sdk
|
CLA Signature Action: Thank you for your submission, we really appreciate it. We ask that you all read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:
By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository. 14 out of 15 committers have signed the CLA. |
Comment on lines
+1087
to
+1094
| env: { | ||
| isPushIntegrated: false, // temporary until we integrate push notifications | ||
| featureAnnouncements: { | ||
| platform: 'mobile', | ||
| accessToken: 'TODO from env', | ||
| spaceId: 'TODO from env', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
New fields in controller.
| @@ -1003,6 +1003,34 @@ class Engine { | |||
| ], | |||
| }); | |||
|
|
|||
| const snapController = new SnapController({ | |||
There was a problem hiding this comment.
Optional - just moved this up.
There was a problem hiding this comment.
NOTE - we need to discuss how we will ship notifications when snaps are not in main.
We reply on the snap controller (and potentially other snap functionality) to call our pre-installed snap. So we kinda need snaps to be enabled (at least from the background perspective, we can still hide in UI)
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/@metamask/[email protected] |
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is network access?This module accesses the network. Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
also fixes some global imports that should've been relative imports
## **Description** Refactors a few parts of the Snaps integration in mobile and fixes some problems: - Fixes RPC methods being called by dapps - which were broken due to moving the PermissionController - Move PermissionController middleware to execute before all other middlewares when handling requests from Snaps - Simplify NPM implementation significantly by using a base class `BaseNpmLocation` which handles most of the complexity - Support `snap_getFile`, which was broken due to a missing hook - Add proper feature flags for basic functionality and allowing local Snaps --------- Co-authored-by: Jonathan Ferreira <[email protected]>
<!-- 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? --> This PR aims to fix send flow after getting `nonce too low` error. ## **Related issues** Fixes: #9729 ## **Manual testing steps** 1. Go to send flow 2. Before send, change it to lower nonce to get error 3. Get error and see it's redirected to the latest screen you're on 4. Try send flow again 5. It should successfully send ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** Forwarded from tests https://github.com/MetaMask/metamask-mobile/assets/104780023/655e0a17-0768-422b-a337-7909bc560e84 ### **After** https://github.com/MetaMask/metamask-mobile/assets/7644512/a1265c59-b870-4fab-9b29-a2657cb78008 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
## **Description** Awaiting for the result when adding a transaction before swapping for non approved tokens ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
<!-- 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? --> This PR fixes an issue with Ramps and STX missing an origin. This will disable STX for all Ramps Send transactions. ## **Related issues** #10100 Fixes: ## **Manual testing steps** 1. Turn on STX 2. Start an off ramp 3. Should be a normal tx ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
<!-- 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? --> This PR adds the `add-team-label` GitHub workflow and the accompanying `add-team-label-to-pr` script. Most of the implementation follows the `add-release-label` GitHub workflow and the `add-release-label-to-pr-and-linked-issues` script. To make the implementation easier, a new helper function, `addLabelByIdToLabelable` was also added, and the previously non-exported `retrieveLabel` function was exported. When a new PR is opened, it will automatically add the author's team to the labels. ## **Related issues** Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2447 ## **Manual testing steps** 1. Open PR, and see that the correct label is added. ## **Screenshots/Recordings** Not applicable ## **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.
… network UI redesign (#10005) <!-- 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** This PR shows the 'Add custom network' form directly after clicking the button, where the user can add and connect to a custom network. A demo can be viewed here: https://www.loom.com/share/bd380aca1065428ea584b0421ce69fb9 Here is a link to the figma: https://www.figma.com/design/76R5BvAVubUhWaN2Ld7MAv/Default-Networks?node-id=1303-34400&m=dev <!-- 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? --> ## **Related issues** Ensures that the form is displayed directly after clicking since the list of popular networks are now integrated in the list. ## **Manual testing steps** 1. From the home page, click the network selector at the top of the screen, a bottom sheet comes up with a list of networks 2. At the bottom of the bottom sheet, click Add custom network button, a form should show up allowing to add a network 3. Fill the form with relevant information and add the network ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
<!-- 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** This PR aims to add a new test for increase allowance. I am additionally upgrading the test-dapp package to v8.8.0. <!-- 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? --> ## **Related issues** Fixes: #9939 ## **Manual testing steps** 1. run E2E smoke tests. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **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.
Ads the `security_alert_source` property to the transaction and signature events, with the value `api` or `local`. Fix the `ReadOnlyNetworkStore` to remove the initialisation side effect on construction.
<!-- 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** Update Notification's related directories on CODEOWNER to the Notifications Team. ## **Related issues** Fixes: ## **Manual testing steps** N/A ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** N/A ### **After** N/A ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask 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** - [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: sethkfman <[email protected]> Co-authored-by: sethkfman <[email protected]>
<!-- 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** This PR adds i18n to for the add/edit network ui re-design <!-- 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? --> ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
This is the release candidate for version 7.24.4. --------- Co-authored-by: metamaskbot <[email protected]> Co-authored-by: sethkfman <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: hesterbruikman <[email protected]> Co-authored-by: EtherWizard33 <[email protected]> Co-authored-by: EtherWizard33 <[email protected]> Co-authored-by: sethkfman <[email protected]> Co-authored-by: Nico MASSART <[email protected]> Co-authored-by: tommasini <[email protected]> Co-authored-by: Cal Leung <[email protected]> Co-authored-by: Omri Dan <[email protected]> Co-authored-by: Christopher Ferreira <[email protected]> Co-authored-by: Daniel <[email protected]> Co-authored-by: yande <[email protected]> Co-authored-by: Derek Brans <[email protected]> Co-authored-by: LeoTM <[email protected]> Co-authored-by: Jony Bursztyn <[email protected]> Co-authored-by: Curtis David <[email protected]> Co-authored-by: salimtb <[email protected]> Co-authored-by: Wietze Bronkema <[email protected]> Co-authored-by: Owen Craston <[email protected]> Co-authored-by: Pedro Pablo Aste Kompen <[email protected]> Co-authored-by: Cal Leung <[email protected]> Co-authored-by: Mark Stacey <[email protected]> Co-authored-by: Vinicius Stevam <[email protected]> Co-authored-by: legobeat <[email protected]> Co-authored-by: infiniteflower <[email protected]>
<!-- 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. --> This PR adds tests using [api-specs](https://github.com/MetaMask/api-specs) via the [@open-rpc/test-coverage](https://github.com/open-rpc/test-coverage) tool. <!-- 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? --> Fixes: https://github.com/MetaMask/MetaMask-planning/issues/2238 1. `yarn setup` 2. `yarn test:e2e:ios:debug:build` 3. `yarn test:api-specs`   <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> - [x] I’ve followed [MetaMask 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. - [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: Shane Jonas <[email protected]> Co-authored-by: Curtis <[email protected]>
) This PR removes any code that triggers polling on NftDetectionController. Calling `detectNfts()` function is now only triggered when the user clicks on the NFT tab. It also Enables NFTDetection by default for new users. >[!NOTE] > this PR has an asset-cpntroller patch from this [core PR > This patch keeps NFTDetection controller extending the polling controller while the PR makes it extend BaseController but it makes sure the polling is not triggered anymore in the client code. > It will be fully updated in newer versions of assets-controllers > I removed what triggers the `start()` of the polling, and now instead, mobile calls `detectNfts()` directly. > Then i made updates on the fct `getOwnerNfts` so that it only fetches information for a specific cursor instead of looping through all user nfts. > The loop is now being done inside the `detectNfts()` fct, and we basically fetch the first page of NFTs and save it to state so its available to clients to view. **New functionalities highlight:** _1- Enable NFT autodetection by default to new users. 2- Old users who have the NFT autodetection off will see the new NFT detection modal to encourage them to enable the modal. 3- When users click on the banner notice, they are no longer directed to settings and instead we enable the NFT detection after they click on “enable nft autodetection” and we show the Toast. We wanted to reduce friction and we removed that redirection to settings. 4- The code should not do any of the 3mins polling in the background anymore._ Related: MetaMask/core#4281 1. Import MM application 2. Go to settings => Security and privacy and you should see that the NFT detection is enabled. 3. Go back to home page and click on NFT tab; you should be able to see your NFTs being added. 4. Switch to another account that has NFTs 5. Click on NFTs tab and you should also see your NFTs being added to sate. 6. Go to settings => Security and privacy and turn OFF the NFT detection toggle. 7. You should see a new modal. Clicking on "allow" button should re-enable the NFT detection toggle. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> https://github.com/MetaMask/metamask-mobile/assets/10994169/83e86852-3455-4b7c-bfc6-a658dc20c1b0 Testing a fresh wallet import: https://github.com/MetaMask/metamask-mobile/assets/10994169/331668b1-3a27-493f-a648-3568e4ba67c2 Testing an upgrade scenario: First build on main, where you should see the NFT auto detection disabled by default if the user did not enable it Then build on this PR: You should see the new NFT detection modal and clicking on `allow` button should enable the NFT detection in the settings. https://github.com/MetaMask/metamask-mobile/assets/10994169/6bcb77f5-13ef-429e-a1f5-4d907145ee30 NFT detection banner with new Toast: https://github.com/MetaMask/metamask-mobile/assets/10994169/2f7465c0-4c42-409b-bfe4-ed70d9e1ca2c - [ ] I’ve followed [MetaMask Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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. - [ ] 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. --------- Co-authored-by: MetaMask Bot <[email protected]> Co-authored-by: metamaskbot <[email protected]> Co-authored-by: sethkfman <[email protected]> Co-authored-by: Nico MASSART <[email protected]>
## **Description** Fixes the changelog for 7.24.4 ## **Related issues** Fixes: NA ## **Manual testing steps** 1. NA 2. 3. ## **Screenshots/Recordings** NA ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
<!-- 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** Auto add 'rc-cherry-picked' label to PRs cherry-picked <!-- 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? --> ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
## **Description** This pull request updates the bitrise e2e check workflow to only trigger from branches within the MetaMask Mobile repository. This brings it in line with other patterns we see across MetaMask such as in [core](https://github.com/MetaMask/core/blob/4fe6141a24aa11bb6672ea31a6e33efe041ffcc1/.github/workflows/publish-preview.yml#L13). ## **Manual testing steps** **Pull request on repository (non-fork)** 1. Look at CI on this pull request 2. See the successful run of `Run Bitrise E2E Check`. **Pull Request From Fork** 1. Visit https://github.com/NicholasEllul/metamask-mobile/pull/3 2. See in the CI output that the `run-bitrise-e2e-check` was [skipped](https://github.com/NicholasEllul/metamask-mobile/actions/runs/9686283967/job/26728373509?pr=3). ## **Screenshots/Recordings** ### **Before** ### **After** ## **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.
…#10134) <!-- 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** Within the E2E tests that use fixtures, we want to disable the what's new modal in addition to the privacy policy toast message that appears as soon as you land on the wallet view. This shaves off ~7 mins of E2E run time on CI. A new state (legalNotices) was added to the default fixture for the privacy policy. Furthermore, the Whats New modal version used in the default fixture was updated to match the most recent release: 7.24.3. ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** See main before the PR is merged: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9f51cf6c-ffd3-4a7a-8aa4-f420a81547f9 ### **After** Here is main after: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f15f1200-4164-4d56-a740-32687b3f5134 ## **Pre-merge author checklist** - [ ] 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). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] 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.
#9914) <!-- 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? --> ## **Related issues** MetaMask/metamask-sdk#862 Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** https://github.com/MetaMask/metamask-mobile/assets/61094771/cbead00e-f411-40d7-9c0b-0572361e6796 <!-- [screenshots/recordings] --> ## **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.
<!-- 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. --> This PR implements a new feature flag specific for snaps to enable the usage of snaps (embedded) while avoid users to install it (for now). This replaces the old `snaps` tag with `preinstalled-snaps` and `external-snaps` tags. The whole installation flow is under `external-snaps`. Fixes: 1. Go to this page... 2. 3. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> <!-- [screenshots/recordings] --> - [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. - [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.
<!-- 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** We were failing to properly restore persisted state for some BaseControllerV1 controllers. This resulted in state being lost when the application was restarted. The only confirmed affected controller so far is the `NftController`. The context here is that for some older controllers, rather than passing the initial state into the controller directly when it is constructed, we call the `update` function directly. This has always been a bad practice (controllers should update their own state), and we're forced to fix this for controllers that we update to `BaseControllerV2` (because directly updating state like this is no longer possible. But this method of rehydrating controller state was still relied upon in some cases. This was broken recently in #9570 when a condition was added to fix a type error. The condition was meant to check that the controller had a `subscribe` function. Unfortunately this `hasProperty` check only looks at owned properties, not inherited properties, and the `subscribe` function was inherited from the base class. It has been updated to use the `in` operator instead, which does look up the entire prototype chain. ## **Related issues** Fixes #10057 ## **Manual testing steps** See #10057 ## **Screenshots/Recordings** ### **Before** See #10057 ### **After** https://github.com/MetaMask/metamask-mobile/assets/2459287/b7cf3e09-086b-4964-8180-e58195969e17 ## **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.
…Notifications FCM. (#10085) <!-- 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. --> This PR aims to handle ONLY the addition of Firebase related libraries to our codebase as well implements iOS and Android specific setup to enable Push Notifications FCM on MetaMask Mobile. No changes on consuming Push Notifications will take place on THIS PR since we're breaking this implementation down. No visual changes are introduced as well nor ways to test it, since the video updated is just to increase the understanding of what the changes will empower. Documentation used for implementing it, [here](https://rnfirebase.io/) Fixes: 1. Go to this page... 2. 3. <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> <!-- [screenshots/recordings] --> https://github.com/MetaMask/metamask-mobile/assets/44679989/dd9f7570-a4cb-4831-9cb2-23bc5ce920a4 - [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. - [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.
|
Not sure what happened here, I'm closing and re-opening. |
|
Closing this, had a little sub-branch merge. Reopened this PR here: #10147 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist