metamask-mobile
metamask-mobile copied to clipboard
fix: 9559 issue android json parse
Description
Fix issue on Android while handling notifications (JSON parse)
Related issues
Fixes: 9559
Manual testing steps
- Enable notifications feature (feature flag)
- Submit any transaction within the app
Screenshots/Recordings
Before
After
Pre-merge author checklist
- [x] I’ve followed MetaMask Coding Standards.
- [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 format if applicable
- [x] I’ve applied the right labels on the PR (see labeling guidelines). 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.
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.
Bitrise
❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌
Commit hash: 2e67dfb0c6a552a740c997c95c59698b2cf638be Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/bb0f5cef-ce58-4d6b-bf32-ce88df6dc7d1
[!NOTE]
- You can kick off another
pr_smoke_e2e_pipelineon Bitrise by removing and re-applying theRun Smoke E2Elabel on the pull request
Codecov Report
Attention: Patch coverage is 20.00000% with 20 lines in your changes are missing coverage. Please review.
Project coverage is 46.78%. Comparing base (
678c2f6) to head (b8c7c7a). Report is 5 commits behind head on main.
| Files | Patch % | Lines |
|---|---|---|
| app/util/notifications/hooks/index.ts | 26.31% | 14 Missing :warning: |
| app/components/Nav/Main/index.js | 0.00% | 6 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #9568 +/- ##
==========================================
+ Coverage 46.66% 46.78% +0.12%
==========================================
Files 1343 1344 +1
Lines 32805 32803 -2
Branches 3527 3525 -2
==========================================
+ Hits 15307 15347 +40
+ Misses 16554 16511 -43
- Partials 944 945 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Bitrise
✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅
Commit hash: d57cff4904725f785f2828ccb21b9795be56f215 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/26355779-bd92-4bca-811e-0404f2e09202
[!NOTE]
- You can kick off another
pr_smoke_e2e_pipelineon Bitrise by removing and re-applying theRun Smoke E2Elabel on the pull request
Quality Gate failed
Failed conditions
23.3% Coverage on New Code (required ≥ 40%)
Let's increase test coverage for the notifications hook. That should be able to clear the threshold.
Bitrise
❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌
Commit hash: 39b4461cbaf73bffbd22743ae0f29499132fa43c Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a92c9214-04b8-4a2e-b034-0d4ab1835371
[!NOTE]
- You can kick off another
pr_smoke_e2e_pipelineon Bitrise by removing and re-applying theRun Smoke E2Elabel on the pull request
Bitrise
✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅
Commit hash: f22f94996b589c77dbc9c65bb36fd2058b54ad72 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/81339d1e-90f7-442f-a5e1-f13f655de3df
[!NOTE]
- You can kick off another
pr_smoke_e2e_pipelineon Bitrise by removing and re-applying theRun Smoke E2Elabel on the pull request
hi @Jonathansoufer, could you fix this PR description and an explicit unit test to prove that MetaMask/metamask-mobile#9559 is fixed by this PR please? I would expect a unit test that uses a mock JSON similar to the one that made the app fail before to make sure we can consider this PR as a fix for the parsing issue. Thanks!
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
21.2% Coverage on New Code
0.0% Duplication on New Code