Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(1702-5): incoming transactions and network details settings into reusable components #11385

Merged

Conversation

vinnyhoward
Copy link
Contributor

@vinnyhoward vinnyhoward commented Sep 23, 2024

Description

This PR is part of a series that breaks down the original large PR #11127 into smaller, more manageable chunks.

The IncomingTransactions functionality and NetworkDetailsCheck functionality are valuable beyond its original implementation. By converting it into a reusable component, we can:

  1. Improve code maintainability
  2. Reduce duplication
  3. Enhance consistency across the application and ease of use

Changes

  • Extracted IncomingTransactions and NetworkDetailsCheck logic from its original location of SecuritySettings into separate component files
  • Included new tests
  • Updated the original screen to use the new components
  • Removed related tests/constants within SecuritySettings and moved them into the new component folder

Impact

This refactored component will be used in:

  • Its current original screen of SecuritySettings
  • New screens coming from the enhanced onboarding settings #1702 that is currently under progress

Related issues

Feature: #1702

Manual testing steps

Incoming Transactions Settings

  1. On wallet home screen click on the settings tab in the bottom right
  2. Tap on "Security & Privacy" and scroll down to "Show incoming transactions" and toggle off and on
  3. Make sure showIncomingTransactionsNetworks[chainId] is returning correct value

Network Details Settings

  1. On wallet home screen, tap the network picker and a bottom sheet will appear
  2. On the bottom sheet click on "Add network" - a list of networks will appear and click add on any of them
  3. No need to actually add them just make sure a warning doesn't appear saying "Network details check reduces your changes of connecting to malicious or incorrect network"
  4. Click on the settings tab in the bottom right and goto "Security & Privacy"
  5. Tap on "Network Details Check" to toggle to off. Repeat steps 1-3 but make sure the warning "Network details check reduces your changes of connecting to malicious or incorrect network" appears

Screenshots/Recordings

Incoming Transactions Settings

UPDATED: 10/07/24
networks

Network Details Settings

Toggle Off
network_details_toggle

Display NFT Media Settings

Before

NA

After

NA

Pre-merge author checklist

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.

Copy link
Contributor

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.

@vinnyhoward vinnyhoward added No QA Needed Apply this label when your PR does not need any QA effort. Run Smoke E2E Triggers smoke e2e on Bitrise needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed No QA Needed Apply this label when your PR does not need any QA effort. labels Sep 23, 2024
@vinnyhoward vinnyhoward marked this pull request as ready for review September 23, 2024 20:51
@vinnyhoward vinnyhoward requested a review from a team as a code owner September 23, 2024 20:51
Copy link
Contributor

github-actions bot commented Sep 23, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: a94ae40
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/ea83ea9c-2d98-4ef6-81d1-ee7d3e1e038e

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

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 24, 2024
Copy link
Contributor

github-actions bot commented Sep 24, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: d72822f
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/cc0b92ad-95d9-4dd8-8976-a9e3428181f0

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

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 26, 2024
Copy link
Contributor

github-actions bot commented Sep 26, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 4873ee4
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1186fce8-9b05-4a5b-ad3a-02138be70c61

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

Copy link
Contributor

@brianacnguyen brianacnguyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few comments. looking good so far!

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Sep 27, 2024
Copy link
Contributor

github-actions bot commented Sep 27, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 0ba2074
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/63c57cae-3205-4982-9389-26065c1b68c2

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

…nts' of github.com:MetaMask/metamask-mobile into feat-1702-5-refactor-settings-logic-to-reusable-components
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 2, 2024
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 3, 2024
Copy link
Contributor

github-actions bot commented Oct 3, 2024

https://bitrise.io/ Bitrise

🔄🔄🔄 pr_smoke_e2e_pipeline started on Bitrise...🔄🔄🔄

Commit hash: a3ac8e7
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/152d0770-59fa-4905-baed-7602e148bbf1

Note

  • This comment will auto-update when build completes
  • 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

EtherWizard33
EtherWizard33 previously approved these changes Oct 4, 2024
@vinnyhoward vinnyhoward dismissed brianacnguyen’s stale review October 4, 2024 16:08

Opting out of stories for now in favor of getting this out quickly to allow more time for Q4 features

@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 4, 2024
Copy link
Contributor

github-actions bot commented Oct 4, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 4fc3762
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/f4ebc0d1-49be-4b64-92fc-e6f8d8853fbf

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

EtherWizard33
EtherWizard33 previously approved these changes Oct 7, 2024
@vinnyhoward vinnyhoward added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels Oct 7, 2024
Copy link
Contributor

github-actions bot commented Oct 7, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 6d14f24
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d36cfd4d-56d9-4067-b422-8f070c4e08e1

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

@EtherWizard33 EtherWizard33 self-requested a review October 7, 2024 17:35
Copy link

sonarcloud bot commented Oct 7, 2024

Copy link
Member

@andreahaku andreahaku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vinnyhoward vinnyhoward added this pull request to the merge queue Oct 7, 2024
Merged via the queue into main with commit 4b90523 Oct 7, 2024
49 checks passed
@vinnyhoward vinnyhoward deleted the feat-1702-5-refactor-settings-logic-to-reusable-components branch October 7, 2024 18:04
@github-actions github-actions bot locked and limited conversation to collaborators Oct 7, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants