-
Notifications
You must be signed in to change notification settings - Fork 41
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
[BX-890] GET addys url for user assets #745
Conversation
BX-890 GET addys url for user assets
https://rainbowhaus.slack.com/archives/C02C8JQ313N/p1677880541883289 Remove WS code and fetch user assets for addys url |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
const response = await addysHttp.get(url); | ||
const data = response.data as AddressAssetsReceivedMessage; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw you can also pass the type as a generic
it's still casting internally 😆 not a change request, just a note cause I added it not long ago
const { data } = await addysHttp.get<AddressAssetsReceivedMessage>(url);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooo, yeah. Much cleaner. Ty sir
const a: Record<string, ParsedAddressAsset> = {}; | ||
newParsedUserAssetsByUniqueId.forEach( | ||
(parseAsset) => (a[parseAsset.uniqueId] = parseAsset), | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd find a reduce easier to understand here, but up to you
return newParsedUserAssetsByUniqueId.reduce((acc, parsedAsset) => {
acc[parsedAsset.uniqueId] = parsedAsset
return acc
}, {})
4b6936a
to
8dfa0c1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤌
Here's the packed extension for this build: |
} catch (e) { | ||
logger.error( | ||
new RainbowError( | ||
`userAssetsByChain.ts - chain: ${chain} - address: ${address}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't have set to stone patterns for this but i believe we're using a different one
{location without .ts}: {message}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥 🔥 🔥 🔥 🔥
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
Here's the packed extension for this build: |
What changed (plus any additional context for devs)
I removed the web socket code we were using to request user assets by chain. I replaced this a GET request to addys. I also fixed how we were grabbing cached data from react-query in the event of an error or bad request.
Screen recordings / screenshots
What to test
There shouldn't be any change in behavior. Just make sure that the user's assets are showing up correctly.
Final checklist
yarn build
).