Eliminate redundant Api calls. Closes #174 #210
Conversation
| @@ -1,14 +1,11 @@ | |||
| import './transactions.less'; | |||
|
|
|||
| const UPDATE_INTERVAL = 20000; | |||
|
|
|||
| app.component('transactions', { | |||
| template: require('./transactions.pug')(), | |||
| controller: class transactions { | |||
| constructor($scope, $rootScope, $timeout, $q, Peers, Account, AccountApi) { | |||
There was a problem hiding this comment.
$timeout is should be removed from dependencies here.
src/app/services/account.js
Outdated
| } | ||
|
|
||
| // Calling listeners with the list of changes | ||
| $rootScope.$broadcast('onAccountChange', changes); | ||
| if (Object.keys(changes).length) { | ||
| $rootScope.$broadcast('accountChange', changes); |
There was a problem hiding this comment.
$rootScope.$broadcast should be outside of keys.forEach((key) => {, because on each balance change it gets triggered twice. Because two properties change: balance and UnconfirmedBalance. Of course, the logic has to be updated accordingly.
| } | ||
| this.$rootScope.$on('onAccountChange', () => { | ||
| this.init.call(this); | ||
| this.$rootScope.$on('accountChange', () => { |
There was a problem hiding this comment.
this.$rootScope.$on should be replaced with this.$scope.$on to prevent the callback being called multiple times, as described here:
http://stackoverflow.com/questions/30968948/angularjs-broadcast-once-on-twice
| @@ -18,30 +15,23 @@ app.component('transactions', { | |||
| this.transactions = []; | |||
| this.pendingTransactions = []; | |||
|
|
|||
| this.$rootScope.$on('transaction-sent', (event, transaction) => { | |||
| // Update transactions list if one was created | |||
| this.$rootScope.$on('transactionCreation', (event, transaction) => { | |||
There was a problem hiding this comment.
this.$rootScope.$on should be replaced with this.$scope.$on to prevent the callback being called multiple times, as described here:
http://stackoverflow.com/questions/30968948/angularjs-broadcast-once-on-twice
src/app/components/main/main.js
Outdated
|
|
||
| if (this.account.get() && this.account.get().publicKey) { | ||
| this.checkIfIsDelegate(); | ||
| this.$rootScope.$on('syncTick', this.update.bind(this)); |
There was a problem hiding this comment.
this.$rootScope.$on should be replaced with this.$scope.$on to prevent the callback being called multiple times, as described here:
http://stackoverflow.com/questions/30968948/angularjs-broadcast-once-on-twice
| return false; | ||
| } | ||
|
|
||
| const props1 = (ref1 instanceof Array) ? ref1.map((val, idx) => idx) : Object.keys(ref1).sort(); |
There was a problem hiding this comment.
This code is calling for unit test coverage ;-) Something like calling Account.set with various values that cause go through all the paths here.
There was a problem hiding this comment.
What this function does is to make sure we're updating list only when a real change is happening. so it doesn't perform a crucial task. though to meet 90+% it makes sense to cover it.
| .then(() => { | ||
| this.$rootScope.prelogged = false; | ||
| this.$rootScope.logged = true; | ||
| this.checkIfIsDelegate(); | ||
| if (this.$timeout) { |
There was a problem hiding this comment.
I don't understand the purpose of this if statement, but it looks weird. Maybe add some comment.
…o 174-status-update-request
…/lisk-nano into 174-status-update-request
accountChangeonly if values are actually changed