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

fix: display msg if tokenId already exists #20940

Merged
merged 1 commit into from
Sep 29, 2023
Merged

Conversation

sahar-fehri
Copy link
Contributor

Explanation

This PR adds displaying an error msg when the nft has been already imported.
This avoids seeing the same nft twice when the user first imports tokenId(decimal value) then imports it again (hex value).

  • Fixes #MMASSETS-27

Screenshots/Screencaps

Before

Github issue : #18957

After

1- Mint and import nft with id 577

image

2- Try importing Nft again with id 577

image

3- Try importing Nft again with hex value: 0x241

image

Manual Testing Steps

Pre-merge author checklist

  • I've clearly explained:
    • [x ] What problem this PR is solving
    • [ x] How this problem was solved
    • [x ] 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

@sahar-fehri sahar-fehri requested a review from a team as a code owner September 18, 2023 20:17
@github-actions
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.

@sahar-fehri sahar-fehri added the team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead label Sep 19, 2023
@sahar-fehri sahar-fehri force-pushed the fix/MMASSETS-27 branch 3 times, most recently from 945161e to 724c1d7 Compare September 19, 2023 11:52
@sahar-fehri sahar-fehri requested review from a team as code owners September 19, 2023 13:41
Copy link
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

Just some small nits. This is awesome!

ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
ui/helpers/utils/util.js Outdated Show resolved Hide resolved
bergeron
bergeron previously approved these changes Sep 19, 2023
@metamaskbot
Copy link
Collaborator

Builds ready [4642172]
Page Load Metrics (1631 ± 50 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint122183145199
domContentLoaded14171764163010350
load14171764163110450
domInteractive14171764163010350
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 684 Bytes (0.01%)

@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (77b3b80) 68.46% compared to head (38e62ae) 68.48%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #20940      +/-   ##
===========================================
+ Coverage    68.46%   68.48%   +0.02%     
===========================================
  Files         1012     1012              
  Lines        40449    40472      +23     
  Branches     10790    10799       +9     
===========================================
+ Hits         27693    27715      +22     
- Misses       12756    12757       +1     
Files Coverage Δ
ui/helpers/utils/util.js 84.87% <100.00%> (+0.76%) ⬆️
.../multichain/import-nfts-modal/import-nfts-modal.js 94.37% <90.91%> (-0.72%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

hold on, just noticed something... comment incoming

@metamaskbot
Copy link
Collaborator

Builds ready [6304f83]
Page Load Metrics (1059 ± 339 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8612599136
domContentLoaded7412192136
load8518321059705339
domInteractive7412192136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [825b36c]
Page Load Metrics (548 ± 334 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint87148101147
domContentLoaded7212091136
load831750548696334
domInteractive7212091136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

@sahar-fehri sahar-fehri force-pushed the fix/MMASSETS-27 branch 3 times, most recently from 56bbd44 to 2132d39 Compare September 28, 2023 14:01
@metamaskbot
Copy link
Collaborator

Builds ready [2132d39]
Page Load Metrics (842 ± 386 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint87154117189
domContentLoaded76130107157
load871870842804386
domInteractive76130107157
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

brad-decker
brad-decker previously approved these changes Sep 28, 2023
danjm
danjm previously approved these changes Sep 28, 2023
@metamaskbot
Copy link
Collaborator

Builds ready [9229826]
Page Load Metrics (914 ± 356 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8412099115
domContentLoaded7112093136
load861726914741356
domInteractive7111993136
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

@sahar-fehri sahar-fehri force-pushed the fix/MMASSETS-27 branch 2 times, most recently from 6a6074d to 87e4c87 Compare September 29, 2023 10:29
@metamaskbot
Copy link
Collaborator

Builds ready [87e4c87]
Page Load Metrics (913 ± 395 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint83158104189
domContentLoaded67142952110
load782044913824395
domInteractive67142952110
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

@metamaskbot
Copy link
Collaborator

Builds ready [38e62ae]
Page Load Metrics (529 ± 321 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint7912296136
domContentLoaded6411486157
load761696529669321
domInteractive6411486157
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 0 Bytes (0.00%)
  • ui: 413 Bytes (0.00%)
  • common: 649 Bytes (0.01%)

@sahar-fehri sahar-fehri merged commit 707b2f4 into develop Sep 29, 2023
9 checks passed
@sahar-fehri sahar-fehri deleted the fix/MMASSETS-27 branch September 29, 2023 15:59
@github-actions github-actions bot locked and limited conversation to collaborators Sep 29, 2023
@metamaskbot metamaskbot added the release-11.3.0 Issue or pull request that will be included in release 11.3.0 label Sep 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-11.3.0 Issue or pull request that will be included in release 11.3.0 team-extension-ux DEPRECATED: please use "team-wallet-ux" label instead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants