fix: loader can display on top of login screen #11325

abretonc7s · 2024-09-19T15:23:38Z

Description

This PR addresses issue from updated async key exchange protocol.

Prevent double intiialization of the deeplinks
Fixes: Prevent sending empty account list when selectedAddress changes
Fixes: Terminate from desktop dApp not trigerring channel removal directly
This PR prevents the loading modal to display on top of the login screen.
Cleanup unnecessary logs in connection management screen.
Simplify sdk permission management to remove unnecessary checks linked to previous protocol version

Related issues

Fixes: Regression from protocol update #11314
Related to https://consensyssoftware.atlassian.net/jira/software/projects/SDK/boards/711?assignee=62440610247a4b00691c743a&selectedIssue=SDK-50&sprintStarted=true

Fixed by: #11457

Manual testing steps

Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

I’ve followed MetaMask Contributor Docs and MetaMask Mobile Coding Standards.
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 format if applicable
I’ve applied the right labels on the PR (see labeling guidelines). 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.

github-actions · 2024-09-19T15:23:49Z

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 · 2024-09-26T15:38:13Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: c5e4983
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/ca91a743-1637-4d06-a52a-b8051dda037a

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 · 2024-09-27T00:42:25Z

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 3348c4f
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/67125f28-5b42-461d-8418-494f942fbc42

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

christopherferreira9 · 2024-09-30T16:00:31Z

Looking good for QA. Waiting for a fix on e2e to be merged in order to merge this.

github-actions · 2024-09-30T17:56:59Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 0c65140
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/5afe6ee9-3863-4c45-9e3c-09ba32a69681

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

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

github-actions · 2024-10-01T10:02:22Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 7acf1f5
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/9d8a6a08-ddb2-4b41-b96a-68b2606a88f4

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

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

codecov-commenter · 2024-10-02T07:53:21Z

Codecov Report

Attention: Patch coverage is 40.00000% with 93 lines in your changes missing coverage. Please review.

Project coverage is 53.84%. Comparing base (b9dbf4d) to head (e597a03).
Report is 71 commits behind head on main.

Files with missing lines	Patch %	Lines
...DKConnect/ConnectionManagement/connectToChannel.ts	2.22%	44 Missing ⚠️
.../core/SDKConnect/ConnectionManagement/reconnect.ts	51.78%	17 Missing and 10 partials ⚠️
app/core/SDKConnect/handlers/handleDeeplink.ts	41.17%	6 Missing and 4 partials ⚠️
app/core/BackgroundBridge/BackgroundBridge.js	0.00%	3 Missing ⚠️
...ore/SDKConnect/handlers/handleConnectionMessage.ts	25.00%	2 Missing and 1 partial ⚠️
...re/SDKConnect/InitializationManagement/postInit.ts	75.00%	2 Missing ⚠️
app/core/SDKConnect/handlers/checkPermissions.ts	60.00%	1 Missing and 1 partial ⚠️
...re/SDKConnect/ConnectionManagement/reconnectAll.ts	80.00%	1 Missing ⚠️
...DKConnect/StateManagement/updateSDKLoadingState.ts	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11325      +/-   ##
==========================================
- Coverage   55.84%   53.84%   -2.01%     
==========================================
  Files        1594     1639      +45     
  Lines       37855    38239     +384     
  Branches     4545     4651     +106     
==========================================
- Hits        21142    20591     -551     
- Misses      16214    16225      +11     
- Partials      499     1423     +924

Flag	Coverage Δ
	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

github-actions · 2024-10-02T08:51:48Z

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: e597a03
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d2d8ffcd-6256-4e1f-8f41-1071103a5288

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

Tip

Check the documentation if you have any doubts on how to understand the failure on bitrise

christopherferreira9 · 2024-10-02T15:40:13Z

E2E passing here: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/8dbebdac-527d-4ddb-b379-2a818dee6ecb

(Swaps tests have been failing for about a weeb and are unrelated to these changes)

tommasini

LGTM! But can we first solve the unit test comment before merge?

app/core/DeeplinkManager/ParseManager/handleMetaMaskDeeplink.ts

app/core/SDKConnect/ConnectionManagement/reconnect.test.ts

sonarcloud · 2024-10-03T00:40:28Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
59.3% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

andreahaku

LGTM

abretonc7s requested review from a team as code owners September 19, 2024 15:23

github-actions bot added the team-sdk SDK team label Sep 19, 2024

abretonc7s force-pushed the androidloader branch 2 times, most recently from 93ccebe to 1d8c928 Compare September 20, 2024 04:09

abretonc7s changed the title ~~fix: android loader can display on top of login screen~~ fix: loader can display on top of login screen Sep 20, 2024

abretonc7s marked this pull request as draft September 20, 2024 09:41

abretonc7s added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label Sep 26, 2024

abretonc7s self-assigned this Sep 26, 2024

abretonc7s added 3 commits September 26, 2024 10:31

feat: cleanup

2fa0a22

fix: unit tests

9de171a

feat: cleanup

c9180b1

abretonc7s force-pushed the androidloader branch from 3e7dea6 to c9180b1 Compare September 26, 2024 05:11

abretonc7s added 2 commits September 26, 2024 13:36

fix: unit tests

99f8c8e

fix: unit tests

c5e4983

christopherferreira9 added the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Sep 26, 2024

abretonc7s marked this pull request as ready for review September 26, 2024 10:20

omridan159 previously approved these changes Sep 26, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into androidloader

68c4b20

legobeat added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 27, 2024

feat: wip

84c9c84

christopherferreira9 added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Sep 30, 2024

Merge branch 'main' into androidloader

0c65140

christopherferreira9 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 30, 2024

Merge branch 'main' into androidloader

7acf1f5

christopherferreira9 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 1, 2024

Merge remote-tracking branch 'origin/main' into androidloader

e597a03

christopherferreira9 added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 2, 2024

abretonc7s added 3 commits October 2, 2024 19:21

fix: e2e flow

adc17f4

feat: prevent unit tests

3c4427c

Merge remote-tracking branch 'origin/main' into androidloader

6fc3b0f

tommasini approved these changes Oct 2, 2024

View reviewed changes

app/core/DeeplinkManager/ParseManager/handleMetaMaskDeeplink.ts Show resolved Hide resolved

app/core/SDKConnect/ConnectionManagement/reconnect.test.ts Show resolved Hide resolved

Merge branch 'main' into androidloader

246a545

abretonc7s requested a review from a team as a code owner October 3, 2024 00:29

andreahaku approved these changes Oct 3, 2024

View reviewed changes

christopherferreira9 added this pull request to the merge queue Oct 3, 2024

Merged via the queue into main with commit a5bef50 Oct 3, 2024
36 of 37 checks passed

christopherferreira9 deleted the androidloader branch October 3, 2024 08:06

github-actions bot locked and limited conversation to collaborators Oct 3, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: loader can display on top of login screen #11325

fix: loader can display on top of login screen #11325

abretonc7s commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 26, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Sep 27, 2024 •

edited by metamaskbot

Loading

christopherferreira9 commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Oct 1, 2024 •

edited by metamaskbot

Loading

codecov-commenter commented Oct 2, 2024

github-actions bot commented Oct 2, 2024 •

edited by metamaskbot

Loading

christopherferreira9 commented Oct 2, 2024 •

edited

Loading

tommasini left a comment

sonarcloud bot commented Oct 3, 2024

andreahaku left a comment

fix: loader can display on top of login screen #11325

fix: loader can display on top of login screen #11325

Conversation

abretonc7s commented Sep 19, 2024 • edited Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Sep 19, 2024

github-actions bot commented Sep 26, 2024 • edited by metamaskbot Loading

Bitrise

github-actions bot commented Sep 27, 2024 • edited by metamaskbot Loading

Bitrise

christopherferreira9 commented Sep 30, 2024

github-actions bot commented Sep 30, 2024 • edited by metamaskbot Loading

Bitrise

github-actions bot commented Oct 1, 2024 • edited by metamaskbot Loading

Bitrise

codecov-commenter commented Oct 2, 2024

Codecov Report

github-actions bot commented Oct 2, 2024 • edited by metamaskbot Loading

Bitrise

christopherferreira9 commented Oct 2, 2024 • edited Loading

tommasini left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Oct 3, 2024

Quality Gate passed

andreahaku left a comment

Choose a reason for hiding this comment

abretonc7s commented Sep 19, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Sep 27, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Sep 30, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Oct 1, 2024 •

edited by metamaskbot

Loading

github-actions bot commented Oct 2, 2024 •

edited by metamaskbot

Loading

christopherferreira9 commented Oct 2, 2024 •

edited

Loading