Add initial methods #3
Conversation
wachunei
force-pushed
the
feature/initial-methods
branch
from
ea61b00
to
e69a30b
Compare
| @@ -20,36 +20,37 @@ describe('SmartTransactionsController', () => { | |||
| await smartTransactionsController.stop(); | |||
| }); | |||
|
|
|||
| it('should initialize with default config', () => { | |||
| it('initializes with default config', () => { | |||
src/SmartTransactionsController.ts
Outdated
| APIType, | ||
| SmartTransaction, | ||
| SignedTransaction, | ||
| SignedCancellation, |
There was a problem hiding this comment.
nit: Is that signed canceled transaction? If yes, we could maybe call it: SignedCanceledTransaction
src/SmartTransactionsController.ts
Outdated
| this.update({ | ||
| smartTransactions: { | ||
| ...this.state.smartTransactions, | ||
| [chainId]: [ |
There was a problem hiding this comment.
nit: For updating a smart transaction in the array we could also use array.map, which is a recommended way on the Redux website: https://redux.js.org/usage/structuring-reducers/immutable-update-patterns#updating-an-item-in-an-array
|
|
||
| setOptInState(state: boolean | undefined): void { | ||
| this.update({ userOptIn: state }); | ||
| this.poll(); |
There was a problem hiding this comment.
If we have more than 1 polling here, do we want to be more specific? E.g. pollSmartTransactionsStatus, which would allow us to check multiple smart transactions at the same request.
src/SmartTransactionsController.ts
Outdated
| // | ||
| const { smartTransactions } = this.state; | ||
| const { chainId } = this.config; | ||
| const currentChainIdSmartTransactions = smartTransactions[chainId]; |
There was a problem hiding this comment.
What will be the structure of the smartTransactions object? [chainId] as a key and a value will be an array of smart transactions?
src/SmartTransactionsController.ts
Outdated
| const currentChainIdSmartTransactions = smartTransactions[chainId]; | ||
|
|
||
| const transactionsToUpdate: string[] = []; | ||
| currentChainIdSmartTransactions.forEach((smartTransaction) => { |
There was a problem hiding this comment.
nit: I usually prefer that when I iterate, I like to use plural and then a singular version. In this case it would be:
currentChainIdSmartTransactions.forEach((currentChainIdSmartTransaction => {
I'm wondering if we could maybe rename this.state.smartTransactions to something else (e.g.this.state.chainIdsSmartTransactions) and then this one could be just simply:
smartTransactions.forEach((smartTransaction => {
| } | ||
| } | ||
|
|
||
| // ! Ask backend API to accept list of UUIDs as params |
No description provided.