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

feat: smart tx small views #9448

Merged
merged 45 commits into from
May 16, 2024
Merged

feat: smart tx small views #9448

merged 45 commits into from
May 16, 2024

Conversation

infiniteflower
Copy link
Contributor

@infiniteflower infiniteflower commented Apr 30, 2024

Description

This PR only covers a smaller subset of the files from #9565 for ease of reviewing. Please refer to that issue for videos. This PR is mostly for Engine and the views for Smart Transactions (STX). Merging this PR will cause changes to the functionality and behavior of the app, effectively making the feature "live".

It covers the following areas:

  1. app/components
  2. app/core
  3. app/reducers
  4. app/selectors
  5. app/util/test
  6. patches
    • PreferencesController (PR here to apply update to main)
    • SwapsController (PR here to apply updates to main)

Also fixed an issue for network onboarding where it would never set onboarded to true. This needed to be fixed so we could properly show the STX Opt In Modal. We only show the Opt In modal if the user has onboarded onto the network, to prevent multiple modals from showing up at the same time. See discussion #9448 (comment)

Related issues

Fixes: n/a

Manual testing steps

Refer to #9565

Screenshots/Recordings

Refer to #9565

Before

After

Pre-merge author checklist

  • I’ve followed MetaMask 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.

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.

@infiniteflower infiniteflower changed the title Feat/smart tx small views feat: smart tx small views Apr 30, 2024
@infiniteflower infiniteflower marked this pull request as ready for review April 30, 2024 18:16
@infiniteflower infiniteflower requested review from a team as code owners April 30, 2024 18:16
@infiniteflower infiniteflower marked this pull request as draft April 30, 2024 18:17
@infiniteflower infiniteflower marked this pull request as ready for review April 30, 2024 18:20
@legobeat legobeat mentioned this pull request May 6, 2024
7 tasks
@infiniteflower infiniteflower marked this pull request as draft May 7, 2024 03:03
@infiniteflower
Copy link
Contributor Author

Have to get the newest changes from #9565

app/components/Nav/Main/RootRPCMethodsUI.js Outdated Show resolved Hide resolved
app/components/Nav/Main/RootRPCMethodsUI.js Outdated Show resolved Hide resolved
app/components/Nav/Main/RootRPCMethodsUI.js Outdated Show resolved Hide resolved
app/components/Nav/Main/RootRPCMethodsUI.js Outdated Show resolved Hide resolved
app/components/Nav/Main/RootRPCMethodsUI.js Outdated Show resolved Hide resolved
app/core/Engine.ts Outdated Show resolved Hide resolved
app/reducers/swaps/index.js Outdated Show resolved Hide resolved
app/reducers/swaps/index.js Outdated Show resolved Hide resolved
app/reducers/swaps/swaps.test.ts Outdated Show resolved Hide resolved
app/selectors/smartTransactionsController.ts Outdated Show resolved Hide resolved
@legobeat legobeat added the Run Smoke E2E Triggers smoke e2e on Bitrise label May 8, 2024
Copy link
Contributor

github-actions bot commented May 8, 2024

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: b4fe945
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6e9b0679-e17b-4083-9f03-6ca9b276d0f3

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

Base automatically changed from feat/smart-tx-small-logic to main May 16, 2024 02:21
@infiniteflower infiniteflower dismissed stale reviews from tommasini and vinistevam May 16, 2024 02:21

The base branch was changed.

infiniteflower added a commit that referenced this pull request May 16, 2024
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR only covers a smaller subset of the files from #9565 for ease of
reviewing. Please refer to that issue for videos. This PR is mostly for
the core logical parts of Smart Transactions (STX). This PR can be
merged as it won't change any existing behavior in the application
without a follow up PR to actually use these utility functions.

It covers the following areas

1. `/app/util`
- except `app/util/test/initial-background-state.json`,
`app/util/transactions/index.js`, `app/util/transactions/index.test.ts`
since it causes tests to fail
3. `/app/images`
4. `/e2e/selectors`
5. `app/core/RPCMethods/RPCMethodMiddleware.ts` (to fix tests)
6. `package.json` (Added `smart-transactions-controller` to this PR to
fix failing tests)
7. `app/components/Nav/Main/RootRPCMethodsUI.js` to fix failing tests
8. `@metamask+transaction-controller+13.0.0.patch`
(https://github.com/MetaMask/core/compare/patch/mobile-transaction-controller-13-0-0-smart-transactions)
9. `yarn.lock`
10. `jest.config.js`

## **Related issues**

- Broken out from #9565
- Functionality in this PR enabled in follow-up
#9448

## **Manual testing steps**

Refer to #9565

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

Refer to #9565

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [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](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
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.

---------

Co-authored-by: tommasini <46944231+tommasini@users.noreply.github.com>
tommasini
tommasini previously approved these changes May 16, 2024
Copy link
Contributor

@tommasini tommasini left a comment

Choose a reason for hiding this comment

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

LGTM! But please address the conversations pending! The ones that are not flagged for be closed after QA

vinistevam
vinistevam previously approved these changes May 16, 2024
@infiniteflower infiniteflower dismissed stale reviews from vinistevam and tommasini via aefbadd May 16, 2024 16:32
@infiniteflower infiniteflower added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels May 16, 2024
Copy link
Contributor

github-actions bot commented May 16, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: aefbadd
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7dacbc3a-4919-4a09-bc5b-f7a470ee6d55

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

tommasini
tommasini previously approved these changes May 16, 2024
@infiniteflower infiniteflower added Run Smoke E2E Triggers smoke e2e on Bitrise and removed Run Smoke E2E Triggers smoke e2e on Bitrise labels May 16, 2024
Copy link
Contributor

github-actions bot commented May 16, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: b0f9e3b
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/a9bb5680-4bb5-4655-9dc1-3d13ab37705c

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

Copy link

sonarcloud bot commented May 16, 2024

@infiniteflower
Copy link
Contributor Author

Links to commented code to remove after QA
#9448 (comment)
#9448 (comment)

@infiniteflower infiniteflower merged commit aa6ba68 into main May 16, 2024
35 checks passed
@infiniteflower infiniteflower deleted the feat/smart-tx-small-views branch May 16, 2024 23:37
@github-actions github-actions bot locked and limited conversation to collaborators May 16, 2024
@metamaskbot metamaskbot added the release-7.24.0 Issue or pull request that will be included in release 7.24.0 label May 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-7.24.0 Issue or pull request that will be included in release 7.24.0 Run Smoke E2E Triggers smoke e2e on Bitrise team-swaps
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

6 participants