-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add api unit tests #74
Conversation
adac005
to
e651ebd
Compare
13267d1
to
a0d293b
Compare
@dawsbot i have changed the code relating to all your feedback. please review the recent commits |
@@ -63,7 +85,7 @@ export abstract class ApiParser { | |||
public convertToTokenRows(data: ApiResponse): TokenRows { | |||
const tokens = data.d.data.map((obj) => ({ | |||
tokenName: obj.tokenName, | |||
tokenSymbol: obj.tokenSymbol, | |||
tokenSymbol: "", |
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.
How come the tokenSymbol
is an empty string here?
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 found that token symbol is not on the API response and removed it from the type we made. this made it so that i could not just leave it as obj.tokenSymbol. it was essentially a blank string before as well because it is assigned in the filter data step.
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.
A more future-proof method in the future would be to leave out tokenSymbol
from this atall since it's hydrated later and create a new TS type here so that we lean on TS to enforce we hydrate tokenSymbol
in a later stage.
But in the effort to not over-complicate this is fine for now
@dawsbot please review this commit