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

when removing address, remove it only for that chain id #1202

Merged
merged 4 commits into from
Nov 28, 2024

Conversation

KillariDev
Copy link
Contributor

No description provided.

@KillariDev KillariDev added the bug Something isn't working label Nov 25, 2024
@KillariDev KillariDev requested a review from jubalm November 25, 2024 07:57
Comment on lines +86 to +89
await updateUserAddressBookEntries((previousContacts) => previousContacts.filter((contact) =>
!(contact.address === removeAddressBookEntry.data.address
&& (contact.chainId === removeAddressBookEntry.data.chainId || (contact.chainId === undefined && removeAddressBookEntry.data.chainId === 1n))))
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

while code works perfectly fine, it may be more readable with the conditions broken down a bit

Suggested change
await updateUserAddressBookEntries((previousContacts) => previousContacts.filter((contact) =>
!(contact.address === removeAddressBookEntry.data.address
&& (contact.chainId === removeAddressBookEntry.data.chainId || (contact.chainId === undefined && removeAddressBookEntry.data.chainId === 1n))))
)
await updateUserAddressBookEntries((previousContacts) => {
const shouldKeepContact = (contact: typeof previousContacts[number]) => {
const isNotTheAddressToRemove = contact.address !== removeAddressBookEntry.data.address
const isNotWithinTheSameChain = contact.chainId !== removeAddressBookEntry.data.chainId
const isChainIdNotUniversal = !(contact.chainId === undefined && removeAddressBookEntry.data.chainId === 1n)
if (isNotTheAddressToRemove) return true
return isNotWithinTheSameChain && isChainIdNotUniversal
}
return previousContacts.filter(shouldKeepContact)
})

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm I don't think this makes it cleaner as those variable names just say directly what the statement is

@KillariDev KillariDev merged commit 3fba424 into main Nov 28, 2024
1 check passed
@KillariDev KillariDev deleted the when-removing-address,-match-for-chain-id-to branch November 28, 2024 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants