[xDai] Parametrize Native Coin by matextrem · Pull Request #1444 · 5afe/safe-react

matextrem · 2020-10-08T00:29:14Z

…safe-react into feature/#1418-xDai-parametrize-native-coin

github-actions · 2020-10-08T00:29:30Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-10-08T00:33:54Z

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	1	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

src/routes/open/components/ReviewInformation/index.tsx Line 143 - Argument 'controls' should be typed.
src/routes/open/components/ReviewInformation/index.tsx Line 143 - Argument 'props' should be typed.

^{Report generated by eslint-plus-action}

ghost · 2020-10-08T00:41:07Z

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

src/routes/open/components/ReviewInformation/index.tsx

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/RejectTxModal/index.tsx

src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx

...utes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx

src/routes/safe/components/Balances/SendModal/screens/ReviewCollectible/index.tsx

src/routes/safe/components/Balances/SendModal/screens/ReviewTx/index.tsx

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx

ghost · 2020-10-08T20:24:09Z

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

fernandomg

Just suggestions on the code-base related stuff.

I'll test it and complete my feedback.

src/logic/tokens/utils/humanReadableValue.ts

...utes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx

fernandomg · 2020-10-09T13:50:47Z

src/routes/safe/components/Balances/SendModal/screens/SendFunds/index.tsx

                    <TokenSelectField
                      initialValue={selectedToken}
-                      isValid={tokenAddress && String(tokenAddress).toUpperCase() !== 'ETHER'}
+                      isValid={tokenAddress && String(tokenAddress).toUpperCase() !== nativeCoin.name.toUpperCase()}


I know it's not related to this... but OMG, that tokenAddress compared to 'ETHER' just blew my mind.

…ai-parametrize-native-coin

ghost · 2020-10-09T14:11:37Z

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

fernandomg

Using xDai chain:

Also, added a fix around the generateBatchRequest, there's a type that should remain 'eth', as it's referencing the web3.eth lib

ghost · 2020-10-09T15:05:13Z

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

src/logic/safe/store/actions/fetchSafe.ts

src/logic/tokens/utils/humanReadableValue.ts

src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx

ghost · 2020-10-09T16:44:46Z

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

...utes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx

src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/ApproveTxModal/index.tsx

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/RejectTxModal/index.tsx

nicosampler

Please remove const { nativeCoin } = getNetworkInfo() inside de componentes for all the ocurrences.

ghost · 2020-10-09T18:55:15Z

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

ghost · 2020-10-09T18:56:40Z

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

Agupane · 2020-10-13T14:28:54Z

Not sure if this comment should be here but:

export const getNetworkInfo = (): NetworkSettings => getConfig()?.network

the ? should be removed, network is not optional, that will let us use this code:

const { nativeCoin } = getNetworkInfo()

Without the risks of network being undefined

Agupane · 2020-10-13T14:30:11Z

src/logic/safe/store/actions/transactions/fetchTransactions/loadIncomingTransactions.ts


 const batchIncomingTxsTokenDataRequest = (txs: IncomingTxServiceModel[]) => {
  const batch = new web3ReadOnly.BatchRequest()
+  const { nativeCoin } = getNetworkInfo()


Please check my comment about getNetworkInfo

I think it's a good point. But this should be added in another PR

ghost · 2020-10-13T15:20:04Z

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

Simplify tests logic

ghost · 2020-10-13T15:30:45Z

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

ghost · 2020-10-13T15:32:46Z

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

matextrem added 2 commits October 7, 2020 21:15

Replace all harcoded values

19d82ec

Merge branch 'feature/#1353-xDai-compatibility' of github.com:gnosis/…

8bcd253

…safe-react into feature/#1418-xDai-parametrize-native-coin

matextrem added the Feature 👑 New functionality label Oct 8, 2020

matextrem requested review from dasanra and nicosampler October 8, 2020 00:29

matextrem self-assigned this Oct 8, 2020

dasanra reviewed Oct 8, 2020

View reviewed changes

src/routes/open/components/ReviewInformation/index.tsx Show resolved Hide resolved

dasanra suggested changes Oct 8, 2020

View reviewed changes

Fix conflicts + PR feedback comments

9a789fa

matextrem requested review from dasanra and fernandomg October 8, 2020 20:17

fernandomg requested a review from Agupane October 9, 2020 13:12

fernandomg reviewed Oct 9, 2020

View reviewed changes

Merge branch 'feature/#1353-xDai-compatibility' into feature/#1418-xD…

75bb10d

…ai-parametrize-native-coin

fernandomg added the Critical Only for bugs in released apps, needs to be fixed asap and hotfix needs to be shipped. label Oct 9, 2020

fernandomg suggested changes Oct 9, 2020

View reviewed changes

fix generateBatchRequest calls

7a8f2c7

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/logic/safe/store/actions/fetchSafe.ts Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/logic/tokens/utils/humanReadableValue.ts Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Balances/SendModal/screens/ContractInteraction/Review/index.tsx Outdated Show resolved Hide resolved

matextrem added 2 commits October 9, 2020 13:27

Fix some ETH harcoded values + Add tests

38e8e83

Fix null or undefinded value

e019125

matextrem requested review from fernandomg and nicosampler October 9, 2020 16:39

fernandomg mentioned this pull request Oct 9, 2020

(Feature) Spending Limit #1351

Closed

nicosampler reviewed Oct 9, 2020

View reviewed changes

...utes/safe/components/Balances/SendModal/screens/ContractInteraction/ReviewCustomTx/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Settings/ManageOwners/AddOwnerModal/screens/Review/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Settings/ManageOwners/RemoveOwnerModal/screens/Review/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Settings/ManageOwners/ReplaceOwnerModal/screens/Review/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Settings/ThresholdSettings/ChangeThreshold/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/ApproveTxModal/index.tsx Outdated Show resolved Hide resolved

nicosampler reviewed Oct 9, 2020

View reviewed changes

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/RejectTxModal/index.tsx Outdated Show resolved Hide resolved

nicosampler suggested changes Oct 9, 2020

View reviewed changes

matextrem added 2 commits October 9, 2020 15:43

Move nativeCoin outside component + fix toTokenInput function

93a4e76

Remove native coin from useEffect

fe728c3

nicosampler approved these changes Oct 9, 2020

View reviewed changes

fernandomg approved these changes Oct 9, 2020

View reviewed changes

Agupane reviewed Oct 13, 2020

View reviewed changes

Delete FETCH_HEAD

70cb3f3

Daniel Sanchez added 2 commits October 13, 2020 17:20

Fix app not running correctly

015f700

Simplify tests logic

Fix await order

23e8552

dasanra approved these changes Oct 13, 2020

View reviewed changes

Agupane self-requested a review October 13, 2020 15:27

Agupane approved these changes Oct 13, 2020

View reviewed changes

dasanra merged commit f5f57dd into feature/#1353-xDai-compatibility Oct 13, 2020

dasanra deleted the feature/#1418-xDai-parametrize-native-coin branch October 13, 2020 15:35

github-actions bot locked and limited conversation to collaborators Oct 13, 2020

Conversation

matextrem commented Oct 8, 2020

Uh oh!

github-actions bot commented Oct 8, 2020

Uh oh!

github-actions bot commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ESLint Summary View Full Report

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

Uh oh!

ghost commented Oct 8, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Oct 8, 2020

Uh oh!

fernandomg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fernandomg Oct 9, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 9, 2020

Uh oh!

fernandomg left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented Oct 9, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nicosampler left a comment

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 9, 2020

Uh oh!

ghost commented Oct 9, 2020

Uh oh!

Agupane commented Oct 13, 2020

Uh oh!

Agupane Oct 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dasanra Oct 13, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 13, 2020

Uh oh!

ghost commented Oct 13, 2020

Uh oh!

ghost commented Oct 13, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

github-actions bot commented Oct 8, 2020 •

edited

Loading

Agupane Oct 13, 2020 •

edited

Loading