feat: app event manager and attribution id parameters #11318
Conversation
|
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. |
abretonc7s
force-pushed
the
attributionid
branch
from
73a8150 to
97cc53b
Compare
abretonc7s
added
the
No QA Needed
<!-- 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 is an update to the SDK communication protocol that allows the following: - prevent deleting connection until server confirmation - emit rejection event ## **Related issues** - MetaMask/metamask-sdk#1020 - MetaMask/metamask-sdk#1022 ## **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** - [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** - [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.
abretonc7s
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11318 +/- ##
==========================================
+ Coverage 55.87% 55.89% +0.01%
==========================================
Files 1587 1593 +6
Lines 37651 37799 +148
Branches 4505 4533 +28
==========================================
+ Hits 21039 21127 +88
- Misses 16116 16173 +57
- Partials 496 499 +3 ☔ View full report in Codecov by Sentry. |
gauthierpetetin
added
the
release-7.32.0
|
Adding release-7.32.0 on this PR to make sure we cherry-pick it. cc @Cal-L @sethkfman for visibility |
There was a problem hiding this comment.
The new class could benefit from being split in a way that matches the actual process more semantically.
Understanding what this does is not easy and I see a risk of this becoming a "catch-all" place under the synonym "manager" name.
Refactoring into a more modular code now, given it's still fairly simple, could prevent many issues later and make this part more readable.
|
metamaskbot
added
the
release-7.33.0
Description
When a user is redirected to MetaMask Mobile app thanks to a deep link, including an attributionId parameter:
metamask://open.browser/website_url?attributionId=xyz
(deep link above is just a placeholder)
Then MM Mobile app should attach attributionId as a property to an event emitted right after Mobile app opens.
Given Mobile app would open and directly redirect the user to the in-app browser, the "App Opened" event might be a good candidate.
Related issues
Files: jira issue:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist