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

Adds transferFromTokens error to token addresses text box #304

Merged
merged 2 commits into from
Mar 19, 2024

Conversation

pedronfigueiredo
Copy link
Contributor

As part of #2224, we need to have a way to test whether the token allowance to transfer tokens is exceeded or not. This change shows the error message on the token addresses text box so that it can be asserted against in e2e tests.

Screenshot 2024-03-18 at 16 52 59

@pedronfigueiredo pedronfigueiredo self-assigned this Mar 18, 2024
src/index.js Fixed Show fixed Hide fixed
src/index.js Fixed Show fixed Hide fixed
@seaona
Copy link
Contributor

seaona commented Mar 19, 2024

Overall looks good. Just a nit: do you think we could maybe add a new field below the actions, to display the error there?
This html item is currently missing for the ERC20 section, and if we add it, it would make it more similar to what's in other contracts and we could advocate for this pattern in the future too.

Screenshot from 2024-03-19 09-43-45

Feel free to ignore this though, and it could also be done in another PR

seaona
seaona previously approved these changes Mar 19, 2024
src/index.js Dismissed Show dismissed Hide dismissed
@pedronfigueiredo pedronfigueiredo merged commit 9c0dced into main Mar 19, 2024
7 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/2224-2 branch March 19, 2024 11:43
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Mar 28, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

## **Description**

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

## **Related issues**

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

## **Manual testing steps**

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

## **Screenshots/Recordings**

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

### **Before**

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">


### **After**

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

## **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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 8, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 8, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 8, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 9, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 10, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
pedronfigueiredo added a commit to MetaMask/metamask-extension that referenced this pull request Apr 10, 2024
~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

- [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.
danjm pushed a commit to MetaMask/metamask-extension that referenced this pull request Apr 10, 2024
#23883)

~**NOTE: This PR is blocked by [changes to core
repo](MetaMask/core#4069) as well as to the
[test dApp](MetaMask/test-dapp#304) first.**~

Reuses the token approve confirmation screen for 'increaseAllowance'. It
also includes an e2e test for the complete flow.

[![Open in GitHub

Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23560?quickstart=1)

Fixes:
[#2224](MetaMask/MetaMask-planning#2224)

1. Go to the test dApp
2. Create a token
3. Approve token for a small amount
4. Try to transfer a higher amount of tokens from another account. (it
should fail)
5. Go back to the first account and increase the allowance.
6. Try to transfer again from the second account. (it should succeed)

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

<img width="472" alt="Screenshot 2024-03-18 at 17 54 12"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/243a529b-4710-419b-8764-f2c06be5a903">

<img width="472" alt="Screenshot 2024-03-18 at 17 54 38"
src="https://github.com/MetaMask/metamask-extension/assets/13814744/03843cf3-477d-4dbb-8c24-ae3b808a766f">

- [x] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've clearly explained what problem this PR is solving and how it
is solved.
- [x] I've linked related issues
- [x] I've included manual testing steps
- [x] I've included screenshots/recordings if applicable
- [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-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.
- [x] I’ve properly set the pull request status:
  - [x] In case it's not yet "ready for review", I've set it to "draft".
- [x] In case it's "ready for review", I've changed it from "draft" to
"non-draft".

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


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

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/23883?quickstart=1)

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**

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

### **Before**

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

### **After**

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

## **Pre-merge author checklist**

- [ ] I’ve followed [MetaMask Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [ ] 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](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants