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.

Sign up for GitHub

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

Jump to bottom

Use fromAccount instead of selectedAcccount for account updates on edit #15449

Merged
merged 3 commits into from Aug 3, 2022
Merged

Use fromAccount instead of selectedAcccount for account updates on edit #15449

merged 3 commits into from Aug 3, 2022

Conversation

brad-decker
Copy link
Member

Explanation

When converting to using fromAccount/selectedAccount the logic on the ACCOUNT_CHANGED case was incorrectly converted. It should be using the 'fromAccount' which is set by the editExistingTransaction method. I added additional safety here by checking for existence of both the payload.account and draftTransaction before doing anything.

Fixes #15400

@PeterYinusa PeterYinusa added this to the v10.18.2 milestone Aug 3, 2022
@brad-decker brad-decker marked this pull request as ready for review August 3, 2022 17:08
@brad-decker brad-decker requested a review from a team as a code owner August 3, 2022 17:08
danjm
danjm previously approved these changes Aug 3, 2022
View reviewed changes
adonesky1
adonesky1 reviewed Aug 3, 2022
View reviewed changes
ui/ducks/send/send.js Outdated Show resolved Hide resolved
@metamaskbot
Copy link
Collaborator

Builds ready [1bdd102]
Page Load Metrics (1794 ± 79 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint972182213452217
domContentLoaded16282231178416981
load16442231179416579
domInteractive16282231178416981

highlights:

storybook

adonesky1
adonesky1 previously approved these changes Aug 3, 2022
View reviewed changes
Copy link
Contributor

@adonesky1 adonesky1 left a comment

Choose a reason for hiding this comment

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

LGTM, just one little non-blocking nit

@brad-decker brad-decker dismissed stale reviews from adonesky1 and danjm via e0a6734 August 3, 2022 18:59
@metamaskbot
Copy link
Collaborator

Builds ready [e0a6734]
Page Load Metrics (1988 ± 85 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint981974237402193
domContentLoaded17342432196017484
load17342432198817885
domInteractive17342432196017484

highlights:

storybook

@brad-decker brad-decker merged commit 3b7074b into develop Aug 3, 2022
@brad-decker brad-decker deleted the fix-15400 branch August 3, 2022 19:59
@github-actions github-actions bot locked and limited conversation to collaborators Aug 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@danjm danjm danjm approved these changes

@adonesky1 adonesky1 adonesky1 approved these changes

@georgewrmarshall georgewrmarshall Awaiting requested review from georgewrmarshall georgewrmarshall is a code owner automatically assigned from MetaMask/extension-devs

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v10.18.2
Development

Successfully merging this pull request may close these issues.

undefined error when reading state.selectedAccount.address in send duck after trying to edit transaction
5 participants
@brad-decker @metamaskbot @danjm @adonesky1 @PeterYinusa