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

On-ramp: Add redux-thunk, refactor successful order handler #6257

Merged
merged 3 commits into from
May 4, 2023

Conversation

wachunei
Copy link
Member

@wachunei wachunei commented Apr 24, 2023

Development & PR Process

  1. Follow MetaMask Mobile Coding Standards
  2. Add release-xx label to identify the PR slated for a upcoming release (will be used in release discussion)
  3. Add needs-dev-review label when work is completed
  4. Add needs-qa label when dev review is completed
  5. Add QA Passed label when QA has signed off

Description

This PR

  • adds redux-thunk as a dependency.
  • refactors multiple instances of handling a successful order into one single hook
  • checks the state before adding an order, in case it already exists

When finishing a purchases and adding the order to the state, it must remain the same with no behavior changes.

Screenshots/Recordings

If applicable, add screenshots and/or recordings to visualize the before and after of your change

Issue

Progresses #???

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

@wachunei wachunei added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-ramp issues related to Ramp features needs-ramp-qa Tickets that need feature QA on the ramps flows release-6.6.0 Issue or pull request that will be included in release 6.6.0 labels Apr 24, 2023
@wachunei wachunei requested a review from a team as a code owner April 24, 2023 18:49
@socket-security
Copy link

socket-security bot commented Apr 24, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Pull request alert summary
Issue Status
Critical CVE ✅ 0 issues
CVE ✅ 0 issues
Mild CVE ✅ 0 issues
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
GitHub dependency ✅ 0 issues
No bug tracker ✅ 0 issues
No contributors or author data ✅ 0 issues
No README ✅ 0 issues
Deprecated ✅ 0 issues
Unmaintained ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
AI detected security risk ✅ 0 issues
AI warning ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
redux-thunk@2.4.2 None +0 acemarke

Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

LGTM!. The failed socket check is just saying that react-native-v8 hasn't been touched by us in a while. Can just ignore for now.

@Cal-L Cal-L removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label May 3, 2023
@bkirb
Copy link
Contributor

bkirb commented May 4, 2023

@wachunei I finished verifying this change, lgtm 👍

@wachunei wachunei added needs-ramp-qa Tickets that need feature QA on the ramps flows ramp-qa-passed and removed needs-ramp-qa Tickets that need feature QA on the ramps flows labels May 4, 2023
@wachunei wachunei merged commit a601d3a into main May 4, 2023
@wachunei wachunei deleted the feature/onramp-add-order-thunk branch May 4, 2023 13:14
@github-actions github-actions bot locked and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ramp-qa-passed release-6.6.0 Issue or pull request that will be included in release 6.6.0 team-ramp issues related to Ramp features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants