Update metrics, refactoring #374
Conversation
Update a test
src/SmartTransactionsController.ts
Outdated
| [chainId]: smartTransactionsState.smartTransactions[chainId].map( | ||
| (item, index) => { | ||
| return index === currentIndex | ||
| ? { ...item, ...smartTransaction } |
There was a problem hiding this comment.
Just a heads-up: this method updates only the properties present in the provided smartTransaction. This merge-like behavior might be surprising for some users. Case in point, on line 593 in this PR, if "merge" were the expected bahaviour, you could just write {confirmed: true}.
There was a problem hiding this comment.
We could only pass { confirmed: true } on line 593, but the #updateSmartTransaction function expects the SmartTransaction type for the smartTransaction input param, which also requires uuid. I believe that for simplicity it's easier to just pass the whole smartTransaction to be updated in state.
src/SmartTransactionsController.ts
Outdated
| smartTransactions: { | ||
| ...smartTransactionsState.smartTransactions, | ||
| [chainId]: smartTransactionsState.smartTransactions[chainId].map( | ||
| (item, index) => { |
There was a problem hiding this comment.
Hey, consider naming the item parameter something more specific like existingSmartTransaction to make it clearer.
|
Great work! |
Description
This PR updates metrics for 2 events and includes a bit of refactoring, which was needed to make the events work for non-swaps transactions.