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

[FIX] TypeError: undefined is not an object (evaluating 'n.find') #6122

Merged
merged 2 commits into from
Apr 6, 2023
Merged

[FIX] TypeError: undefined is not an object (evaluating 'n.find') #6122

merged 2 commits into from
Apr 6, 2023

Conversation

blackdevelopa
Copy link
Contributor

@blackdevelopa blackdevelopa commented Apr 5, 2023 •
edited

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
When you do not have a block explorer set for a network then you attempt to tap on a collectible contract address from the approval modal, it throws an error.

This PR is currently blocked by #6058

Screenshots/Recordings
Before:
https://recordit.co/rtJ8eijs9A

After:
https://recordit.co/OgV4qIg3DO

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

Issue

Progresses #6121

Checklist

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

@blackdevelopa blackdevelopa requested a review from a team as a code owner April 5, 2023 11:01
@github-actions
Copy link
Contributor

github-actions bot commented Apr 5, 2023

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.

@blackdevelopa blackdevelopa added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-confirmations-secure-ux-PR PR from the confirmations team labels Apr 5, 2023
@blackdevelopa blackdevelopa self-assigned this Apr 5, 2023
NiranjanaBinoy
NiranjanaBinoy approved these changes Apr 5, 2023
View reviewed changes
Copy link

@NiranjanaBinoy NiranjanaBinoy left a comment

Choose a reason for hiding this comment

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

LGTM

Cal-L
Cal-L approved these changes Apr 5, 2023
View reviewed changes
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

@Cal-L Cal-L added needs-qa Any New Features that needs a full manual QA prior to being added to a release. release-6.4.0 PR for release 6.4.0 team-mobile-client and removed needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) labels Apr 5, 2023
@chrisleewilcox chrisleewilcox added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 6, 2023
@chrisleewilcox
Copy link
Contributor

@blackdevelopa @Cal-L please review unresolved conversations. Not able to merge until unresolved convos are resolved.

Was this fix already into v6.3.0? When v6.3.0 is merged into main branch will this fix still be needed?

@seaona seaona added the QA in Progress QA has started on the feature. label Apr 6, 2023
@seaona
Copy link
Contributor

seaona commented Apr 6, 2023

I can see the issue fixed.
No the ERC721 token link is greyed out whenever there is no blockexplorer set.

no-blockexplorer-link-greyed.webm

In this branch, I am still seeing this issue though. Do you know if this it expected? @Cal-L

@blackdevelopa
Copy link
Contributor Author

@seaona the issue you referenced that you see is already fixed in 6.3. It'll be in main when v6.3.0 is merged to main.

@blackdevelopa
Copy link
Contributor Author

@chrisleewilcox no. This issue isn't fixed with 6.3. The code I highlighted however is used in 6.3 and will be cherrypicked when ready to be merged into main.

@seaona seaona added QA Passed A successful QA run through has been done and removed QA in Progress QA has started on the feature. labels Apr 6, 2023
@seaona
Copy link
Contributor

seaona commented Apr 6, 2023

thank you @blackdevelopa , good to merge on my side! Could you confirm we can merge? @Cal-L @chrisleewilcox thank you in advance!

@Cal-L Cal-L removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Apr 6, 2023
@Cal-L Cal-L merged commit 4c2c23a into main Apr 6, 2023
13 checks passed
@Cal-L Cal-L deleted the bg/6121 branch April 6, 2023 15:30
@github-actions github-actions bot locked and limited conversation to collaborators Apr 6, 2023
@Cal-L
Copy link
Contributor

Cal-L commented Apr 6, 2023

Looks like convos are resolved. 6.3 will be merged into main shortly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jpuri jpuri jpuri approved these changes

@Cal-L Cal-L Cal-L approved these changes

@NiranjanaBinoy NiranjanaBinoy NiranjanaBinoy approved these changes

Assignees

@blackdevelopa blackdevelopa

Labels
QA Passed A successful QA run through has been done release-6.4.0 PR for release 6.4.0 team-confirmations-secure-ux-PR PR from the confirmations team team-mobile-platform
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@blackdevelopa @chrisleewilcox @seaona @Cal-L @jpuri @NiranjanaBinoy @gauthierpetetin