Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

(Feature) [xDai] - Use tx service for loading token info#1435

Merged
fernandomg merged 7 commits intofeature/#1353-xDai-compatibilityfrom
feature/#1407-use-tx-service-loading-token
Oct 8, 2020
Merged

(Feature) [xDai] - Use tx service for loading token info#1435
fernandomg merged 7 commits intofeature/#1353-xDai-compatibilityfrom
feature/#1407-use-tx-service-loading-token

Conversation

@Agupane
Copy link
Contributor

@Agupane Agupane commented Oct 6, 2020

Closes #1407

Description

  • Replaces getRelayUrl with getTxServiceUrl
  • Removes unused function fetchToken

@Agupane Agupane self-assigned this Oct 6, 2020
@Agupane Agupane changed the base branch from development to feature/#1353-xDai-compatibility October 6, 2020 14:19
@github-actions
Copy link

github-actions bot commented Oct 6, 2020

CLA Assistant Lite All Contributors have signed the CLA.

@Agupane Agupane requested review from dasanra and fernandomg October 6, 2020 14:20
@Agupane Agupane marked this pull request as ready for review October 6, 2020 14:20
@github-actions
Copy link

github-actions bot commented Oct 6, 2020

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 2 0
Ignored 6 N/A
  • Result: ✅ success

  • Annotations: 2 total


[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods


Report generated by eslint-plus-action

@ghost
Copy link

ghost commented Oct 6, 2020

Travis automatic deployment:
https://pr1435--safereact.review.gnosisdev.com/app

Copy link
Contributor

@fernandomg fernandomg left a comment

Choose a reason for hiding this comment

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

We'll need to filter what's returned, as this service exposes both ERC20 and ERC721 tokens.

image

Bear in mind that #1408 will serve from this same endpoint, so you can do something reusable for that PR.

@Agupane
Copy link
Contributor Author

Agupane commented Oct 7, 2020

It's fixed now, I just filtered the ERC721, on the PR for #1408 we should aim to use the same function to fetch both of them and save it together into redux and removing the old functions. Didn't want to do it here to make things simpler, let's work on that on the next pr

@Agupane Agupane requested a review from fernandomg October 7, 2020 12:58
@ghost
Copy link

ghost commented Oct 7, 2020

Travis automatic deployment:
https://pr1435--safereact.review.gnosisdev.com/app

1 similar comment
@ghost
Copy link

ghost commented Oct 7, 2020

Travis automatic deployment:
https://pr1435--safereact.review.gnosisdev.com/app

} = await fetchTokenList()

if (currentSavedTokens && currentSavedTokens.size === tokenList.length) {
const erc20Tokens = tokenList.filter((token) => token.type.toLowerCase() === 'erc20')
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we move erc20 string to a constant variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think it's mandatory now. Probably when we work on #1408 we'll address this in a most consistent way.

@dasanra
Copy link
Collaborator

dasanra commented Oct 8, 2020

Didn't notice this before, but I see we are not having a convention in API URL parameters. We are adding final slash in some of them but we aren't on the others.

We should stick to not add slash on the end of the API URL parameters so we start the routes with slash.

@fernandomg
Copy link
Contributor

Didn't notice this before, but I see we are not having a convention in API URL parameters. We are adding final slash in some of them but we aren't on the others.

We should stick to not add slash on the end of the API URL parameters so we start the routes with slash.

Ok, good call. Actually, we may have missed some, but we intentionally wanted to add a slash to the end of all the URLs in the config files 😁

@ghost
Copy link

ghost commented Oct 8, 2020

Travis automatic deployment:
https://pr1435--safereact.review.gnosisdev.com/app

@ghost
Copy link

ghost commented Oct 8, 2020

Travis automatic deployment:
https://pr1435--safereact.review.gnosisdev.com/app

@fernandomg fernandomg merged commit 8f85a2f into feature/#1353-xDai-compatibility Oct 8, 2020
@fernandomg fernandomg deleted the feature/#1407-use-tx-service-loading-token branch October 8, 2020 18:53
@github-actions github-actions bot locked and limited conversation to collaborators Oct 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use tx service for loading token info

5 participants