fix: breaking selector due to missing controller state

Prithpal-Sooriya commented 22 hours ago

Description

The selector is trying to access some engine state that is removed at build time through code-fences. This fix ensures that the selectors are using the engine state or default to the controllers default state.

Long term - we need to investigate and ensure that the UI is not calling the selectors.

Manual testing steps

Can you start up the application without it breaking?

Screenshots/Recordings

Before / After Recording https://www.loom.com/share/badc4b4cd3b0446486497973dcf337f6?sid=33bcfe23-c495-41bf-89f5-e57b4b72e3d2

I was not able to replicate the controller state being undefined/missing, so forced it to be undefined when testing.

Pre-merge author checklist

[x] I’ve followed MetaMask Contributor Docs and MetaMask Mobile 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.

github-actions[bot] commented 22 hours ago

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] commented 21 hours ago

Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4540de35edfab58060543a38312b2e211121c0e0 Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/fa2865f3-b5ee-4385-9ece-7bc947326d7d

[!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

sethkfman commented 21 hours ago

@Prithpal-Sooriya can you show manual confirmation this has fixed the issue?

github-actions[bot] commented 3 hours ago

Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 344002fdbed0f0f949b49257cc0c181735fa0d3c Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/27765754-fa23-4f5a-9405-4be22391be53

[!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

Prithpal-Sooriya commented 3 hours ago

@sethkfman I've attached a recording for a before and after the changes.

I've also sneaked in some other changes to ensure notification hooks are not called when the feature is disabled. If this is too many changes, I'm happy to make them separate PRs.