metamask-mobile icon indicating copy to clipboard operation
metamask-mobile copied to clipboard

fix: 9559 issue android json parse

Open Jonathansoufer opened this issue 1 year ago • 7 comments

Description

Fix issue on Android while handling notifications (JSON parse)

Related issues

Fixes: 9559

Manual testing steps

  1. Enable notifications feature (feature flag)
  2. 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.

Jonathansoufer avatar May 07 '24 11:05 Jonathansoufer

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.

github-actions[bot] avatar May 07 '24 11:05 github-actions[bot]

https://bitrise.io/ 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_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar May 07 '24 11:05 github-actions[bot]

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.

codecov-commenter avatar May 07 '24 11:05 codecov-commenter

https://bitrise.io/ 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_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar May 09 '24 18:05 github-actions[bot]

Quality Gate Failed Quality Gate failed

Failed conditions 23.3% Coverage on New Code (required ≥ 40%)

See analysis details on SonarCloud

Let's increase test coverage for the notifications hook. That should be able to clear the threshold.

Cal-L avatar May 09 '24 20:05 Cal-L

https://bitrise.io/ 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_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar May 10 '24 11:05 github-actions[bot]

https://bitrise.io/ 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_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

github-actions[bot] avatar May 21 '24 14:05 github-actions[bot]

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!

NicolasMassart avatar May 21 '24 15:05 NicolasMassart