The Wayback Machine - https://web.archive.org/web/20220217190514/https://github.com/gnosis/safe-react/pull/3512
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

Jump to bottom

Fix: undefined address in reverse ENS lookup #3512

Merged
merged 1 commit into from Feb 17, 2022
Merged

Fix: undefined address in reverse ENS lookup #3512

merged 1 commit into from Feb 17, 2022

Conversation

@katspaugh
Copy link
Collaborator

@katspaugh katspaugh commented Feb 15, 2022

What it solves

Resolves #3507

How this PR fixes it

Just a quick fix that checks that the address isn't undefined.

I couldn't reproduce the error from Sentry. We actually don't allow empty addresses.
But this should fix it.

How to test it

Try creating a Safe with several owners, some of which are ENS owners.
@katspaugh katspaugh requested a review from usame-algan Feb 15, 2022
@github-actions
Copy link

@github-actions github-actions bot commented Feb 15, 2022

CLA Assistant Lite All Contributors have signed the CLA.
katspaugh
katspaugh reviewed Feb 15, 2022
View changes
src/routes/CreateSafePage/steps/NameNewSafeStep.tsx
owners
.filter(({ addressFieldName }) => !!formValues[addressFieldName])
.map(async ({ addressFieldName }) => {
const address = formValues[addressFieldName]
Copy link
Collaborator Author

@katspaugh katspaugh Feb 15, 2022

Choose a reason for hiding this comment

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

The error on Sentry was coming from here. address was undefined somehow.
@github-actions
Copy link

@github-actions github-actions bot commented Feb 15, 2022

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total
Report generated by eslint-plus-action
@github-actions
Copy link

@github-actions github-actions bot commented Feb 15, 2022

Deployment links

🟠 Rinkeby ⚪ Mainnet 🟣 Polygon 🟡 BSC ⚫ Arbitrum 🟢 Gnosis Chain
@coveralls
Copy link

@coveralls coveralls commented Feb 15, 2022

Pull Request Test Coverage Report for Build 1847477254

  • 5 of 5 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.02%) to 33.507%
Totals Coverage Status
Change from base Build 1845972783: 0.02%
Covered Lines: 3229
Relevant Lines: 8614
💛 - Coveralls
@github-actions
Copy link

@github-actions github-actions bot commented Feb 15, 2022

E2E Tests Failed
Check the results here: https://github.com/gnosis/safe-react-e2e-tests/actions/runs/1847519301

Failed tests:

  • ❌ Read-only transaction creation and review Read-only transaction creation and review
  • ❌ Safe Balances Safe Balances
iamacook
iamacook approved these changes Feb 15, 2022
View changes
Copy link
Contributor

@iamacook iamacook left a comment

💪
usame-algan
usame-algan approved these changes Feb 15, 2022
View changes
Copy link
Contributor

@usame-algan usame-algan left a comment

Nice catch!
@francovenica
Copy link
Contributor

@francovenica francovenica commented Feb 17, 2022

Looks good to me
ENS Names loaded properly with address that have it:
0x61a0c717d18232711bC788F19C9Cd56a43cc8872
0x7724b234c9099C205F03b458944942bcEBA13408

Names in an AB also loaded properly
@katspaugh katspaugh merged commit 0c587ac into dev Feb 17, 2022
11 checks passed
@katspaugh katspaugh deleted the fix-ens-error branch Feb 17, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
5 participants
@katspaugh @coveralls @francovenica @usame-algan @iamacook