Transaction-list-item-details pop up to display the correct token information on token approve item #17422

NiranjanaBinoy · 2023-01-25T22:29:11Z

Explanation

Fixes #17228
Currently, for approve token pop up Transaction-list-item-details displays the details of only those tokens that are added to the wallet, if not a generic detail is displayed as shown in the ticket. With this fix, we are comparing the token address to the tokens available in detected tokens, tokens list, and custom tokens.

Screenshots/Screencaps

Before

After

Manual Testing Steps

Create a token from the test-dapp
Click approve token in the test-dapp
Once the token is approved, go to the activity tab and click Approve Token spending Cap and verify the amount in spending cap.

Pre-merge author checklist

I've clearly explained:
- What problem this PR is solving
- How this problem was solved
- How reviewers can test my changes
Sufficient automated test coverage has been added

Pre-merge reviewer checklist

Manual testing (e.g. pull and build branch, run in browser, test code being changed)
PR is linked to the appropriate GitHub issue
IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

github-actions · 2023-01-25T22:29:24Z

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.

ui/hooks/useTransactionDisplayData.js

ui/hooks/useAssetDetails.js

metamaskbot · 2023-02-03T17:42:23Z

Builds ready [d37ae19]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1156 ± 86 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	93	161	123	18	9
		domContentLoaded	967	1616	1140	181	87
		load	1003	1617	1156	179	86
		domInteractive	967	1616	1140	181	87

Bundle size diffs

background: 0 bytes
ui: 683 bytes
common: 0 bytes

test/e2e/metamask-ui.spec.js

jpuri

👍

ui/hooks/useAssetDetails.js

digiwand

tested on macOS x Chrome - LGTM!

left one minor suggestion above

metamaskbot · 2023-02-14T15:43:32Z

Builds ready [a00763f]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1419 ± 148 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	100	154	120	14	7
		domContentLoaded	1011	1983	1369	298	143
		load	1011	2086	1419	308	148
		domInteractive	1011	1983	1369	298	143

Bundle size diffs

background: 0 bytes
ui: 683 bytes
common: 0 bytes

metamaskbot · 2023-02-14T19:15:30Z

Builds ready [f37124e]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1309 ± 120 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	92	360	128	56	27
		domContentLoaded	1001	1689	1274	268	129
		load	1016	1689	1309	250	120
		domInteractive	1001	1689	1274	268	129

Bundle size diffs

background: 0 bytes
ui: 769 bytes
common: 0 bytes

jpuri · 2023-02-15T07:25:29Z

Hey @NiranjanaBinoy , @seaona : I am curious why some much e2e fixes were needed in this PR. Does it introduces delay at some point ?

metamaskbot · 2023-02-15T12:51:39Z

Builds ready [0b8ebb6]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1376 ± 117 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	102	186	123	21	10
		domContentLoaded	1082	1842	1335	254	122
		load	1094	1842	1376	243	117
		domInteractive	1082	1841	1335	254	122

Bundle size diffs

background: 0 bytes
ui: 769 bytes
common: 0 bytes

metamaskbot · 2023-02-15T16:05:47Z

Builds ready [0444a16]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1509 ± 54 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	95	151	110	15	7
		domContentLoaded	1236	1557	1461	90	43
		load	1255	1650	1509	112	54
		domInteractive	1236	1557	1461	90	43

Bundle size diffs

background: 0 bytes
ui: 769 bytes
common: 0 bytes

NiranjanaBinoy · 2023-02-15T17:03:41Z

Hey @NiranjanaBinoy , @seaona : I am curious why some much e2e fixes were needed in this PR. Does it introduces delay at some point ?

Hi @jpuri,
The tests are failing only on firefox Circle CI.

In this PR, we are introducing useAssetDetails to useTransactionDisplayData, which will be called when the transaction lists are displayed in the activity tab. This means useAssetDetails will be getting called for all the transactions, but inside the useAssetDetails, we are restricting the getAssetDetails calls based on the isTokenCategory so the getAssetDetails will get executed only for token approvals and transfers. I am not exactly sure, but I am suspecting that calling useAssetDetails for the transactions in the list might have introduced a slight delay but the functionalities inside the hook are not fully executed for any transaction other than token transfer or approval.

…enList and custom tokens for the approved token

…true

* e2e flaky test * Await overlay popup * Remove unnecessary steps * Wait for element not present overlay * metamask ui overlay * Fix lint * Remove waitforselector

metamaskbot · 2023-02-21T14:57:41Z

Builds ready [f65a939]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1633 ± 41 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	93	151	119	16	8
		domContentLoaded	1506	1734	1587	60	29
		load	1519	1788	1633	86	41
		domInteractive	1506	1734	1587	60	29

Bundle size diffs

background: 0 bytes
ui: 814 bytes
common: 0 bytes

ui/hooks/useTransactionDisplayData.js

jpuri

LGTM, just a small feedback added.

metamaskbot · 2023-02-21T17:44:32Z

Builds ready [8365d4a]

builds: chrome, firefox
builds (beta): chrome, firefox
builds (flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4
- common: 0, 1, 2, 3, 4, 5
- content-script: 0
- contentscript: contentscript
- disable-console: disable-console
- inpage: inpage
- policy-load: policy-load
- sentry-install: sentry-install
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (1579 ± 70 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	99	171	120	21	10
		domContentLoaded	1416	1979	1570	144	69
		load	1416	1980	1579	145	70
		domInteractive	1416	1979	1570	144	69

Bundle size diffs

background: 0 bytes
ui: 819 bytes
common: 0 bytes

NiranjanaBinoy requested a review from a team as a code owner January 25, 2023 22:29

NiranjanaBinoy self-assigned this Jan 25, 2023

NiranjanaBinoy requested a review from brad-decker January 25, 2023 22:29

NiranjanaBinoy added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. labels Jan 25, 2023

NiranjanaBinoy force-pushed the update-spending-cap branch from 32798a7 to c3677af Compare January 27, 2023 22:20

jpuri reviewed Jan 31, 2023

View reviewed changes

ui/hooks/useTransactionDisplayData.js Show resolved Hide resolved

jpuri reviewed Jan 31, 2023

View reviewed changes

ui/hooks/useAssetDetails.js Outdated Show resolved Hide resolved

NiranjanaBinoy force-pushed the update-spending-cap branch from 7d470ef to d37ae19 Compare February 3, 2023 19:22

NiranjanaBinoy requested a review from jpuri February 3, 2023 20:47

jpuri reviewed Feb 5, 2023

View reviewed changes

test/e2e/metamask-ui.spec.js Outdated Show resolved Hide resolved

jpuri previously approved these changes Feb 5, 2023

View reviewed changes

digiwand self-requested a review February 7, 2023 13:51

digiwand reviewed Feb 7, 2023

View reviewed changes

ui/hooks/useAssetDetails.js Outdated Show resolved Hide resolved

digiwand previously approved these changes Feb 7, 2023

View reviewed changes

seaona dismissed stale reviews from digiwand and jpuri via 5da026f February 7, 2023 17:39

NiranjanaBinoy force-pushed the update-spending-cap branch from 5da026f to a00763f Compare February 14, 2023 15:15

NiranjanaBinoy requested review from digiwand and jpuri February 14, 2023 19:53

NiranjanaBinoy added 2 commits February 17, 2023 21:17

updating the useTransctionDisplayData to check in detected token, tok…

8009bd1

…enList and custom tokens for the approved token

updating useAssetDetails to be executed only when isTokenCategory is …

8844cb8

…true

NiranjanaBinoy and others added 10 commits February 17, 2023 21:17

addressing e2e failure :(

8a42d34

addressing e2e failure :(

81fdf85

addressing e2e failure :(

3dfc2dc

addressing e2e failure :(

64482fb

addressing e2e failure :(

de8129d

addressing e2e fail

c7d85a8

addressing e2e fail

4f805f9

E2e update spending (#17640)

e034548

* e2e flaky test * Await overlay popup * Remove unnecessary steps * Wait for element not present overlay * metamask ui overlay * Fix lint * Remove waitforselector

adding optional object

49e4ed9

lint fix

6d77fe4

NiranjanaBinoy force-pushed the update-spending-cap branch from 0444a16 to 6d77fe4 Compare February 18, 2023 02:17

NiranjanaBinoy added 2 commits February 17, 2023 22:17

replacing useAssetDetails hook with getAssetDetails fucntion call

b81d772

updating token name in e2e

c959e4b

This comment was marked as spam.

Sign in to view

jpuri reviewed Feb 21, 2023

View reviewed changes

ui/hooks/useTransactionDisplayData.js Show resolved Hide resolved

jpuri previously approved these changes Feb 21, 2023

View reviewed changes

update based on review comments

b4dedd1

NiranjanaBinoy dismissed stale reviews from jpuri and Katew212 via b4dedd1 February 21, 2023 17:06

NiranjanaBinoy force-pushed the update-spending-cap branch from f65a939 to b4dedd1 Compare February 21, 2023 17:06

NiranjanaBinoy requested a review from jpuri February 21, 2023 17:06

NiranjanaBinoy added 2 commits February 21, 2023 12:08

Merge branch 'develop' into update-spending-cap

94e214e

Merge branch 'develop' into update-spending-cap

8365d4a

jpuri approved these changes Feb 21, 2023

View reviewed changes

digiwand approved these changes Feb 24, 2023

View reviewed changes

NiranjanaBinoy merged commit bc19856 into develop Feb 24, 2023

NiranjanaBinoy deleted the update-spending-cap branch February 24, 2023 19:21

github-actions bot locked and limited conversation to collaborators Feb 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transaction-list-item-details pop up to display the correct token information on token approve item #17422

Transaction-list-item-details pop up to display the correct token information on token approve item #17422

NiranjanaBinoy commented Jan 25, 2023 •

edited

Loading

github-actions bot commented Jan 25, 2023

metamaskbot commented Feb 3, 2023

jpuri left a comment

digiwand left a comment

metamaskbot commented Feb 14, 2023

metamaskbot commented Feb 14, 2023

jpuri commented Feb 15, 2023

metamaskbot commented Feb 15, 2023

metamaskbot commented Feb 15, 2023

NiranjanaBinoy commented Feb 15, 2023 •

edited

Loading

metamaskbot commented Feb 21, 2023

This comment was marked as spam.

jpuri left a comment

metamaskbot commented Feb 21, 2023

Transaction-list-item-details pop up to display the correct token information on token approve item #17422

Transaction-list-item-details pop up to display the correct token information on token approve item #17422

Conversation

NiranjanaBinoy commented Jan 25, 2023 • edited Loading

Explanation

Screenshots/Screencaps

Before

After

Manual Testing Steps

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Jan 25, 2023

metamaskbot commented Feb 3, 2023

jpuri left a comment

Choose a reason for hiding this comment

digiwand left a comment

Choose a reason for hiding this comment

metamaskbot commented Feb 14, 2023

metamaskbot commented Feb 14, 2023

jpuri commented Feb 15, 2023

metamaskbot commented Feb 15, 2023

metamaskbot commented Feb 15, 2023

NiranjanaBinoy commented Feb 15, 2023 • edited Loading

metamaskbot commented Feb 21, 2023

This comment was marked as spam.

jpuri left a comment

Choose a reason for hiding this comment

metamaskbot commented Feb 21, 2023

NiranjanaBinoy commented Jan 25, 2023 •

edited

Loading

NiranjanaBinoy commented Feb 15, 2023 •

edited

Loading