From 7d718b0d2699bc2b13e763333fcfd864324b345c Mon Sep 17 00:00:00 2001 From: Gina Contrino Date: Tue, 5 Jun 2018 16:46:13 +0200 Subject: [PATCH 1/4] Move transaction related functions to their own action and api files --- src/actions/account.js | 33 +---- src/actions/account.test.js | 85 ++--------- src/actions/search.js | 11 +- src/actions/transactions.js | 71 +++++++-- src/actions/transactions.test.js | 139 ++++++++++++++++-- src/components/sendReadable/index.js | 2 +- .../explorerTransactions/index.test.js | 5 +- .../walletTransactions/index.test.js | 4 +- src/store/middlewares/account.js | 5 +- src/store/middlewares/account.test.js | 3 +- src/utils/api/account.js | 34 ----- src/utils/api/account.test.js | 23 +-- src/utils/api/search.js | 5 +- src/utils/api/transactions.js | 30 ++++ src/utils/api/transactions.test.js | 25 ++++ test/integration/wallet.test.js | 3 +- 16 files changed, 271 insertions(+), 207 deletions(-) create mode 100644 src/utils/api/transactions.js create mode 100644 src/utils/api/transactions.test.js diff --git a/src/actions/account.js b/src/actions/account.js index b4fedba14f..30ad88c9c4 100644 --- a/src/actions/account.js +++ b/src/actions/account.js @@ -1,12 +1,11 @@ import i18next from 'i18next'; import actionTypes from '../constants/actions'; -import { setSecondPassphrase, send, getAccount } from '../utils/api/account'; +import { setSecondPassphrase, getAccount } from '../utils/api/account'; import { registerDelegate, getDelegate, getVotes, getVoters } from '../utils/api/delegate'; import { loadTransactionsFinish } from './transactions'; import { delegateRegisteredFailure } from './delegate'; import { errorAlertDialogDisplayed } from './dialog'; import Fees from '../constants/fees'; -import { toRawLsk } from '../utils/lsk'; import transactionTypes from '../constants/transactionTypes'; /** @@ -142,36 +141,6 @@ export const delegateRegistered = ({ dispatch(passphraseUsed(passphrase)); }; -/** - * - */ -export const sent = ({ - activePeer, account, recipientId, amount, passphrase, secondPassphrase, -}) => - (dispatch) => { - send(activePeer, recipientId, toRawLsk(amount), passphrase, secondPassphrase) - .then((data) => { - dispatch({ - data: { - id: data.transactionId, - senderPublicKey: account.publicKey, - senderId: account.address, - recipientId, - amount: toRawLsk(amount), - fee: Fees.send, - type: transactionTypes.send, - }, - type: actionTypes.transactionAdded, - }); - }) - .catch((error) => { - const errorMessage = error && error.message ? `${error.message}.` : i18next.t('An error occurred while creating the transaction.'); - dispatch({ data: { errorMessage }, type: actionTypes.transactionFailed }); - }); - dispatch(passphraseUsed(passphrase)); - }; - - export const loadDelegate = ({ activePeer, publicKey }) => (dispatch) => { getDelegate(activePeer, { publicKey }).then((response) => { diff --git a/src/actions/account.test.js b/src/actions/account.test.js index 5a11787c99..9b5516fcfa 100644 --- a/src/actions/account.test.js +++ b/src/actions/account.test.js @@ -1,14 +1,20 @@ import { expect } from 'chai'; import sinon from 'sinon'; import actionTypes from '../constants/actions'; -import { accountUpdated, accountLoggedOut, - secondPassphraseRegistered, delegateRegistered, sent, removePassphrase, passphraseUsed, loadDelegate } from './account'; +import { + accountUpdated, + accountLoggedOut, + secondPassphraseRegistered, + delegateRegistered, + removePassphrase, + passphraseUsed, + loadDelegate, +} from './account'; import { errorAlertDialogDisplayed } from './dialog'; import { delegateRegisteredFailure } from './delegate'; import * as accountApi from '../utils/api/account'; import * as delegateApi from '../utils/api/delegate'; import Fees from '../constants/fees'; -import { toRawLsk } from '../utils/lsk'; import transactionTypes from '../constants/transactionTypes'; import networks from '../constants/networks'; import accounts from '../../test/constants/accounts'; @@ -190,79 +196,6 @@ describe('actions: account', () => { }); }); - describe('sent', () => { - let accountApiMock; - const data = { - activePeer: {}, - recipientId: '15833198055097037957L', - amount: 100, - passphrase: 'sample passphrase', - secondPassphrase: null, - account: { - publicKey: 'test_public-key', - address: 'test_address', - }, - }; - const actionFunction = sent(data); - let dispatch; - - beforeEach(() => { - accountApiMock = sinon.stub(accountApi, 'send'); - dispatch = sinon.spy(); - }); - - afterEach(() => { - accountApiMock.restore(); - }); - - it('should create an action function', () => { - expect(typeof actionFunction).to.be.deep.equal('function'); - }); - - it('should dispatch transactionAdded action if resolved', () => { - accountApiMock.returnsPromise().resolves({ transactionId: '15626650747375562521' }); - const expectedAction = { - id: '15626650747375562521', - senderPublicKey: 'test_public-key', - senderId: 'test_address', - recipientId: data.recipientId, - amount: toRawLsk(data.amount), - fee: Fees.send, - type: transactionTypes.send, - }; - - actionFunction(dispatch); - expect(dispatch).to.have.been - .calledWith({ data: expectedAction, type: actionTypes.transactionAdded }); - }); - - it('should dispatch transactionFailed action if caught', () => { - accountApiMock.returnsPromise().rejects({ message: 'sample message' }); - - actionFunction(dispatch); - const expectedAction = { - data: { - errorMessage: 'sample message.', - }, - type: actionTypes.transactionFailed, - }; - expect(dispatch).to.have.been.calledWith(expectedAction); - }); - - it('should dispatch transactionFailed action if caught but no message returned', () => { - accountApiMock.returnsPromise().rejects({}); - - actionFunction(dispatch); - const expectedAction = { - data: { - errorMessage: 'An error occurred while creating the transaction.', - }, - type: actionTypes.transactionFailed, - }; - expect(dispatch).to.have.been.calledWith(expectedAction); - }); - }); - describe('removePassphrase', () => { it('should create an action to remove passphrase', () => { const data = { diff --git a/src/actions/search.js b/src/actions/search.js index 4088457144..1eb3f6c705 100644 --- a/src/actions/search.js +++ b/src/actions/search.js @@ -1,6 +1,7 @@ import actionTypes from '../constants/actions'; import { loadingStarted, loadingFinished } from '../utils/loading'; -import { transactions, getAccount } from '../utils/api/account'; +import { getAccount } from '../utils/api/account'; +import { getTransactions } from '../utils/api/transactions'; import { getDelegate, getVoters, getVotes } from '../utils/api/delegate'; import { searchAll } from '../utils/api/search'; @@ -62,9 +63,7 @@ export const searchTransactions = ({ }) => (dispatch) => { if (showLoading) loadingStarted(actionTypes.searchTransactions); - transactions({ - activePeer, address, limit, filter, - }) + getTransactions({ activePeer, address, limit, filter }) .then((transactionsResponse) => { dispatch({ data: { @@ -83,9 +82,7 @@ export const searchMoreTransactions = ({ activePeer, address, limit, offset, filter, }) => (dispatch) => { - transactions({ - activePeer, address, limit, offset, filter, - }) + getTransactions({ activePeer, address, limit, offset, filter }) .then((transactionsResponse) => { dispatch({ data: { diff --git a/src/actions/transactions.js b/src/actions/transactions.js index 3539e0c2fe..41df9404b5 100644 --- a/src/actions/transactions.js +++ b/src/actions/transactions.js @@ -1,16 +1,20 @@ +import i18next from 'i18next'; import actionTypes from '../constants/actions'; import { loadingStarted, loadingFinished } from '../utils/loading'; -import { transactions, transaction, unconfirmedTransactions } from '../utils/api/account'; +import { send, getTransactions, getSingleTransaction, unconfirmedTransactions } from '../utils/api/transactions'; import { getDelegate } from '../utils/api/delegate'; import { loadDelegateCache } from '../utils/delegates'; import { extractAddress } from '../utils/account'; -import { loadAccount } from './account'; +import { loadAccount, passphraseUsed } from './account'; +import Fees from '../constants/fees'; +import { toRawLsk } from '../utils/lsk'; +import transactionTypes from '../constants/transactionTypes'; export const transactionsFilterSet = ({ activePeer, address, limit, filter, }) => (dispatch) => { - transactions({ + getTransactions({ activePeer, address, limit, @@ -50,7 +54,7 @@ export const loadTransactions = ({ activePeer, publicKey, address }) => const lastActiveAddress = publicKey && extractAddress(publicKey); const isSameAccount = lastActiveAddress === address; loadingStarted(actionTypes.transactionsLoad); - transactions({ activePeer, address, limit: 25 }) + getTransactions({ activePeer, address, limit: 25 }) .then((transactionsResponse) => { dispatch(loadAccount({ activePeer, @@ -68,17 +72,9 @@ export const loadTransactions = ({ activePeer, publicKey, address }) => }); }; -/** - * - * - */ -export const transactionsRequested = ({ - activePeer, address, limit, offset, filter, -}) => +export const transactionsRequested = ({ activePeer, address, limit, offset, filter }) => (dispatch) => { - transactions({ - activePeer, address, limit, offset, filter, - }) + getTransactions({ activePeer, address, limit, offset, filter }) .then((response) => { dispatch({ data: { @@ -95,7 +91,7 @@ export const transactionsRequested = ({ export const loadTransaction = ({ activePeer, id }) => (dispatch) => { dispatch({ type: actionTypes.transactionCleared }); - transaction({ activePeer, id }) + getSingleTransaction({ activePeer, id }) .then((response) => { const added = (response.transaction.votes && response.transaction.votes.added) || []; const deleted = (response.transaction.votes && response.transaction.votes.deleted) || []; @@ -147,3 +143,48 @@ export const loadTransaction = ({ activePeer, id }) => dispatch({ data: error, type: actionTypes.transactionLoadFailed }); }); }; + +export const transactionsUpdated = ({ activePeer, address, limit, filter, pendingTransactions }) => + (dispatch) => { + getTransactions({ activePeer, address, limit, filter }) + .then((response) => { + dispatch({ + data: { + confirmed: response.transactions, + count: parseInt(response.count, 10), + }, + type: actionTypes.transactionsUpdated, + }); + if (pendingTransactions.length) { + dispatch(transactionsUpdateUnconfirmed({ + activePeer, + address, + pendingTransactions, + })); + } + }); + }; + +export const sent = ({ activePeer, account, recipientId, amount, passphrase, secondPassphrase }) => + (dispatch) => { + send(activePeer, recipientId, toRawLsk(amount), passphrase, secondPassphrase) + .then((data) => { + dispatch({ + data: { + id: data.transactionId, + senderPublicKey: account.publicKey, + senderId: account.address, + recipientId, + amount: toRawLsk(amount), + fee: Fees.send, + type: transactionTypes.send, + }, + type: actionTypes.transactionAdded, + }); + }) + .catch((error) => { + const errorMessage = error && error.message ? `${error.message}.` : i18next.t('An error occurred while creating the transaction.'); + dispatch({ data: { errorMessage }, type: actionTypes.transactionFailed }); + }); + dispatch(passphraseUsed(passphrase)); + }; diff --git a/src/actions/transactions.test.js b/src/actions/transactions.test.js index e43c5d298d..558b1eb36d 100644 --- a/src/actions/transactions.test.js +++ b/src/actions/transactions.test.js @@ -2,14 +2,53 @@ import { expect } from 'chai'; import sinon from 'sinon'; import actionTypes from '../constants/actions'; import txFilters from './../constants/transactionFilters'; -import { transactionsRequested, loadTransaction } from './transactions'; -import * as accountApi from '../utils/api/account'; +import { sent, transactionsRequested, loadTransaction, transactionsUpdated } from './transactions'; +import * as transactionsApi from '../utils/api/transactions'; import * as delegateApi from '../utils/api/delegate'; import accounts from '../../test/constants/accounts'; +import transactionTypes from '../constants/transactionTypes'; +import Fees from '../constants/fees'; +import { toRawLsk } from '../utils/lsk'; describe('actions: transactions', () => { + describe('transactionsUpdated', () => { + let transactionsApiMock; + const data = { + activePeer: {}, + address: '15626650747375562521', + limit: 20, + offset: 0, + filter: txFilters.all, + }; + const actionFunction = transactionsUpdated(data); + let dispatch; + + beforeEach(() => { + transactionsApiMock = sinon.stub(transactionsApi, 'getTransactions'); + dispatch = sinon.spy(); + }); + + afterEach(() => { + transactionsApiMock.restore(); + }); + + it('should dispatch transactionsLoaded action if resolved', () => { + transactionsApiMock.returnsPromise().resolves({ transactions: [], count: '0' }); + const expectedAction = { + count: 0, + confirmed: [], + }; + + actionFunction(dispatch); + expect(dispatch).to.have.been.calledWith({ + data: expectedAction, + type: actionTypes.transactionsUpdated, + }); + }); + }); + describe('transactionsRequested', () => { - let accountApiMock; + let transactionsApiMock; const data = { activePeer: {}, address: '15626650747375562521', @@ -21,12 +60,12 @@ describe('actions: transactions', () => { let dispatch; beforeEach(() => { - accountApiMock = sinon.stub(accountApi, 'transactions'); + transactionsApiMock = sinon.stub(transactionsApi, 'getTransactions'); dispatch = sinon.spy(); }); afterEach(() => { - accountApiMock.restore(); + transactionsApiMock.restore(); }); it('should create an action function', () => { @@ -34,7 +73,7 @@ describe('actions: transactions', () => { }); it('should dispatch transactionsLoaded action if resolved', () => { - accountApiMock.returnsPromise().resolves({ transactions: [], count: '0' }); + transactionsApiMock.returnsPromise().resolves({ transactions: [], count: '0' }); const expectedAction = { count: 0, confirmed: [], @@ -49,7 +88,7 @@ describe('actions: transactions', () => { }); describe('loadTransaction', () => { - let accountApiMock; + let transactionApiMock; let delegateApiMock; const data = { activePeer: { @@ -66,13 +105,13 @@ describe('actions: transactions', () => { let dispatch; beforeEach(() => { - accountApiMock = sinon.stub(accountApi, 'transaction'); + transactionApiMock = sinon.stub(transactionsApi, 'getSingleTransaction'); delegateApiMock = sinon.stub(delegateApi, 'getDelegate'); dispatch = sinon.spy(); }); afterEach(() => { - accountApiMock.restore(); + transactionApiMock.restore(); delegateApiMock.restore(); }); @@ -83,7 +122,7 @@ describe('actions: transactions', () => { it('should dispatch one transactionAddDelegateName action when transaction contains one vote added', () => { const delegateResponse = { delegate: { username: 'peterpan' } }; const transactionResponse = { transaction: { votes: { added: [accounts.delegate.publicKey] }, count: '0' } }; - accountApiMock.returnsPromise().resolves(transactionResponse); + transactionApiMock.returnsPromise().resolves(transactionResponse); delegateApiMock.returnsPromise().resolves(delegateResponse); const expectedActionPayload = { ...delegateResponse, @@ -100,7 +139,7 @@ describe('actions: transactions', () => { it('should dispatch one transactionAddDelegateName action when transaction contains one vote deleted', () => { const delegateResponse = { delegate: { username: 'peterpan' } }; const transactionResponse = { transaction: { votes: { deleted: [accounts.delegate.publicKey] }, count: '0' } }; - accountApiMock.returnsPromise().resolves(transactionResponse); + transactionApiMock.returnsPromise().resolves(transactionResponse); delegateApiMock.returnsPromise().resolves(delegateResponse); const expectedActionPayload = { ...delegateResponse, @@ -114,4 +153,82 @@ describe('actions: transactions', () => { .calledWith({ data: expectedActionPayload, type: actionTypes.transactionAddDelegateName }); }); }); + + describe('sent', () => { + let transactionsApiMock; + const data = { + activePeer: {}, + recipientId: '15833198055097037957L', + amount: 100, + passphrase: 'sample passphrase', + secondPassphrase: null, + account: { + publicKey: 'test_public-key', + address: 'test_address', + }, + }; + const actionFunction = sent(data); + let dispatch; + + beforeEach(() => { + transactionsApiMock = sinon.stub(transactionsApi, 'send'); + dispatch = sinon.spy(); + }); + + afterEach(() => { + transactionsApiMock.restore(); + }); + + it('should create an action function', () => { + expect(typeof actionFunction).to.be.deep.equal('function'); + }); + + it('should dispatch transactionAdded action if resolved', () => { + transactionsApiMock.returnsPromise().resolves({ transactionId: '15626650747375562521' }); + const expectedAction = { + id: '15626650747375562521', + senderPublicKey: 'test_public-key', + senderId: 'test_address', + recipientId: data.recipientId, + amount: toRawLsk(data.amount), + fee: Fees.send, + type: transactionTypes.send, + }; + + actionFunction(dispatch); + expect(dispatch).to.have.been + .calledWith({ data: expectedAction, type: actionTypes.transactionAdded }); + }); + + it('should dispatch transactionFailed action if caught', () => { + transactionsApiMock.returnsPromise().rejects({ message: 'sample message' }); + + actionFunction(dispatch); + const expectedAction = { + data: { + errorMessage: 'sample message.', + }, + type: actionTypes.transactionFailed, + }; + expect(dispatch).to.have.been.calledWith(expectedAction); + }); + + it('should dispatch transactionFailed action if caught but no message returned', () => { + transactionsApiMock.returnsPromise().rejects({}); + + actionFunction(dispatch); + const expectedAction = { + data: { + errorMessage: 'An error occurred while creating the transaction.', + }, + type: actionTypes.transactionFailed, + }; + expect(dispatch).to.have.been.calledWith(expectedAction); + }); + }); + + + // describe('accountLoggedOut', () => { + // it('should create an action to reset the account', () => { + // }); }); diff --git a/src/components/sendReadable/index.js b/src/components/sendReadable/index.js index 638543e4ba..cc4d990de6 100644 --- a/src/components/sendReadable/index.js +++ b/src/components/sendReadable/index.js @@ -1,7 +1,7 @@ import { connect } from 'react-redux'; import { translate } from 'react-i18next'; -import { sent } from '../../actions/account'; +import { sent } from '../../actions/transactions'; import Send from './send'; const mapStateToProps = state => ({ diff --git a/src/components/transactions/explorerTransactions/index.test.js b/src/components/transactions/explorerTransactions/index.test.js index 587096e670..9f8a9971c2 100644 --- a/src/components/transactions/explorerTransactions/index.test.js +++ b/src/components/transactions/explorerTransactions/index.test.js @@ -8,6 +8,7 @@ import { BrowserRouter as Router } from 'react-router-dom'; import { prepareStore } from '../../../../test/utils/applicationInit'; import * as accountAPI from '../../../../src/utils/api/account'; import * as delegateAPI from '../../../../src/utils/api/delegate'; +import * as transactionsAPI from '../../../../src/utils/api/transactions'; import peersReducer from '../../../store/reducers/peers'; import accountReducer from '../../../store/reducers/account'; import transactionReducer from '../../../store/reducers/transaction'; @@ -44,8 +45,8 @@ describe('ExplorerTransactions Component', () => { }, [thunk]); beforeEach(() => { - transactionsActionStub = stub(accountAPI, 'transactions'); - transactionActionStub = stub(accountAPI, 'transaction'); + transactionsActionStub = stub(transactionsAPI, 'getTransactions'); + transactionActionStub = stub(transactionsAPI, 'getSingleTransaction'); accountStub = stub(accountAPI, 'getAccount'); delegateStub = stub(delegateAPI, 'getDelegate'); delegateVotesStub = stub(delegateAPI, 'getVotes'); diff --git a/src/components/transactions/walletTransactions/index.test.js b/src/components/transactions/walletTransactions/index.test.js index 0a5c0af373..88b34e050f 100644 --- a/src/components/transactions/walletTransactions/index.test.js +++ b/src/components/transactions/walletTransactions/index.test.js @@ -6,8 +6,8 @@ import { expect } from 'chai'; import { Provider } from 'react-redux'; import { BrowserRouter as Router } from 'react-router-dom'; import { prepareStore } from '../../../../test/utils/applicationInit'; -import * as accountAPI from '../../../../src/utils/api/account'; import * as delegateAPI from '../../../../src/utils/api/delegate'; +import * as transactionsAPI from '../../../../src/utils/api/transactions'; import peersReducer from '../../../store/reducers/peers'; import accountReducer from '../../../store/reducers/account'; import transactionsReducer from '../../../store/reducers/transactions'; @@ -40,7 +40,7 @@ describe('WalletTransactions Component', () => { }, [thunk]); beforeEach(() => { - transactionsActionsStub = stub(accountAPI, 'transactions'); + transactionsActionsStub = stub(transactionsAPI, 'getTransactions'); delegateVotesStub = stub(delegateAPI, 'getVotes'); delegateVotersStub = stub(delegateAPI, 'getVoters'); diff --git a/src/store/middlewares/account.js b/src/store/middlewares/account.js index 22872ea697..0a52f46181 100644 --- a/src/store/middlewares/account.js +++ b/src/store/middlewares/account.js @@ -1,4 +1,4 @@ -import { getAccount, transactions as getTransactions } from '../../utils/api/account'; +import { getAccount } from '../../utils/api/account'; import { accountUpdated } from '../../actions/account'; import { transactionsUpdateUnconfirmed } from '../../actions/transactions'; import { activePeerUpdate } from '../../actions/peers'; @@ -6,6 +6,7 @@ import { votesFetched } from '../../actions/voting'; import actionTypes from '../../constants/actions'; import accountConfig from '../../constants/account'; import { getDelegate } from '../../utils/api/delegate'; +import { getTransactions } from '../../utils/api/transactions'; import transactionTypes from '../../constants/transactionTypes'; const { lockDuration } = accountConfig; @@ -45,6 +46,7 @@ const hasRecentTransactions = txs => ( const updateAccountData = (store, action) => { const { peers, account, transactions } = store.getState(); + // all dispatches getAccount(peers.data, account.address).then((result) => { if (result.balance !== account.balance) { if (!action.data.windowIsFocused || !hasRecentTransactions(transactions)) { @@ -74,6 +76,7 @@ const delegateRegistration = (store, action) => { const state = store.getState(); if (delegateRegistrationTx) { + // all dispatches no api calls and no dispatches after api call getDelegate(state.peers.data, { publicKey: state.account.publicKey }) .then((delegateData) => { store.dispatch(accountUpdated(Object.assign( diff --git a/src/store/middlewares/account.test.js b/src/store/middlewares/account.test.js index 701a009967..795d7c2ea3 100644 --- a/src/store/middlewares/account.test.js +++ b/src/store/middlewares/account.test.js @@ -5,6 +5,7 @@ import accountConfig from '../../constants/account'; import { activePeerUpdate } from '../../actions/peers'; import * as votingActions from '../../actions/voting'; import * as accountApi from '../../utils/api/account'; +import * as transactionsApi from '../../utils/api/transactions'; import accounts from '../../../test/constants/accounts'; import actionTypes from '../../constants/actions'; import * as delegateApi from '../../utils/api/delegate'; @@ -82,7 +83,7 @@ describe('Account middleware', () => { next = spy(); stubGetAccount = stub(accountApi, 'getAccount').returnsPromise(); - stubTransactions = stub(accountApi, 'transactions').returnsPromise().resolves(true); + stubTransactions = stub(transactionsApi, 'getTransactions').returnsPromise().resolves(true); }); afterEach(() => { diff --git a/src/utils/api/account.js b/src/utils/api/account.js index c7bd44263c..7633044ea2 100644 --- a/src/utils/api/account.js +++ b/src/utils/api/account.js @@ -1,5 +1,4 @@ import { requestToActivePeer } from './peers'; -import txFilters from './../../constants/transactionFilters'; export const getAccount = (activePeer, address) => new Promise((resolve, reject) => { @@ -25,36 +24,3 @@ export const getAccount = (activePeer, address) => export const setSecondPassphrase = (activePeer, secondSecret, publicKey, secret) => requestToActivePeer(activePeer, 'signatures', { secondSecret, publicKey, secret }); -export const send = (activePeer, recipientId, amount, secret, secondSecret = null) => - requestToActivePeer( - activePeer, 'transactions', - { - recipientId, amount, secret, secondSecret, - }, - ); - -export const transactions = ({ - activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc', filter = txFilters.all, -}) => { - let params = { - recipientId: (filter === txFilters.incoming || filter === txFilters.all) ? address : undefined, - senderId: (filter === txFilters.outgoing || filter === txFilters.all) ? address : undefined, - limit, - offset, - orderBy, - }; - params = JSON.parse(JSON.stringify(params)); - return requestToActivePeer(activePeer, 'transactions', params); -}; - -export const transaction = ({ activePeer, id }) => requestToActivePeer(activePeer, 'transactions/get', { id }); - -export const unconfirmedTransactions = (activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc') => - requestToActivePeer(activePeer, 'transactions/unconfirmed', { - senderId: address, - recipientId: address, - limit, - offset, - orderBy, - }); - diff --git a/src/utils/api/account.test.js b/src/utils/api/account.test.js index 89ca659a05..c17ea68347 100644 --- a/src/utils/api/account.test.js +++ b/src/utils/api/account.test.js @@ -1,6 +1,6 @@ import { expect } from 'chai'; import { mock } from 'sinon'; -import { getAccount, setSecondPassphrase, send, transactions, unconfirmedTransactions } from './account'; +import { getAccount, setSecondPassphrase } from './account'; describe('Utils: Account API', () => { const address = '1449310910991872227L'; @@ -53,25 +53,4 @@ describe('Utils: Account API', () => { expect(typeof promise.then).to.be.equal('function'); }); }); - - describe('send', () => { - it('should return a promise', () => { - const promise = send(); - expect(typeof promise.then).to.be.equal('function'); - }); - }); - - describe('transactions', () => { - it('should return a promise', () => { - const promise = transactions({ activePeer: {} }); - expect(typeof promise.then).to.be.equal('function'); - }); - }); - - describe('unconfirmedTransactions', () => { - it('should return a promise', () => { - const promise = unconfirmedTransactions(); - expect(typeof promise.then).to.be.equal('function'); - }); - }); }); diff --git a/src/utils/api/search.js b/src/utils/api/search.js index 0cfe37eeb8..889b43a551 100644 --- a/src/utils/api/search.js +++ b/src/utils/api/search.js @@ -1,4 +1,5 @@ -import { getAccount, transaction } from './account'; +import { getAccount } from './account'; +import { getSingleTransaction } from './transactions'; import { listDelegates } from './delegate'; import regex from './../../utils/regex'; @@ -16,7 +17,7 @@ const searchDelegates = ({ activePeer, searchTerm }) => new Promise((resolve, re .catch(() => reject({ delegates: [] }))); const searchTransactions = ({ activePeer, searchTerm }) => new Promise((resolve, reject) => - transaction({ + getSingleTransaction({ activePeer, id: searchTerm, }).then(response => resolve({ transactions: [response.transaction] })) diff --git a/src/utils/api/transactions.js b/src/utils/api/transactions.js new file mode 100644 index 0000000000..90494b00b0 --- /dev/null +++ b/src/utils/api/transactions.js @@ -0,0 +1,30 @@ +import { requestToActivePeer } from './peers'; +import txFilters from './../../constants/transactionFilters'; + +export const send = (activePeer, recipientId, amount, secret, secondSecret = null) => + requestToActivePeer(activePeer, 'transactions', + { recipientId, amount, secret, secondSecret }); + +export const getTransactions = ({ activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc', filter = txFilters.all }) => { + let params = { + recipientId: (filter === txFilters.incoming || filter === txFilters.all) ? address : undefined, + senderId: (filter === txFilters.outgoing || filter === txFilters.all) ? address : undefined, + limit, + offset, + orderBy, + }; + params = JSON.parse(JSON.stringify(params)); + return requestToActivePeer(activePeer, 'transactions', params); +}; + +export const getSingleTransaction = ({ activePeer, id }) => requestToActivePeer(activePeer, 'transactions/get', { id }); + +export const unconfirmedTransactions = (activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc') => + requestToActivePeer(activePeer, 'transactions/unconfirmed', { + senderId: address, + recipientId: address, + limit, + offset, + orderBy, + }); + diff --git a/src/utils/api/transactions.test.js b/src/utils/api/transactions.test.js new file mode 100644 index 0000000000..58e07c75a9 --- /dev/null +++ b/src/utils/api/transactions.test.js @@ -0,0 +1,25 @@ +import { expect } from 'chai'; +import { send, getTransactions, unconfirmedTransactions } from './transactions'; + +describe('Utils: Transactions API', () => { + describe('send', () => { + it('should return a promise', () => { + const promise = send(); + expect(typeof promise.then).to.be.equal('function'); + }); + }); + + describe('transactions', () => { + it('should return a promise', () => { + const promise = getTransactions({ activePeer: {} }); + expect(typeof promise.then).to.be.equal('function'); + }); + }); + + describe('unconfirmedTransactions', () => { + it('should return a promise', () => { + const promise = unconfirmedTransactions(); + expect(typeof promise.then).to.be.equal('function'); + }); + }); +}); diff --git a/test/integration/wallet.test.js b/test/integration/wallet.test.js index 1b082c9447..3de341f48c 100644 --- a/test/integration/wallet.test.js +++ b/test/integration/wallet.test.js @@ -6,6 +6,7 @@ import { stub, match, spy } from 'sinon'; import * as peers from '../../src/utils/api/peers'; import * as accountAPI from '../../src/utils/api/account'; +import * as transactionsAPI from '../../src/utils/api/transactions'; import * as delegateAPI from '../../src/utils/api/delegate'; import * as liskServiceApi from '../../src/utils/api/liskService'; import { prepareStore, renderWithRouter } from '../utils/applicationInit'; @@ -317,7 +318,7 @@ describe('@integration: Wallet', () => { describe('Transactions', () => { beforeEach(() => { - getTransactionsStub = stub(accountAPI, 'transactions'); + getTransactionsStub = stub(transactionsAPI, 'getTransactions'); getTransactionsStub.withArgs({ activePeer: match.any, From a498f45ea094ae34dbb7a4a6b3ad2f52787ef6ae Mon Sep 17 00:00:00 2001 From: Gina Contrino Date: Fri, 8 Jun 2018 11:12:18 +0200 Subject: [PATCH 2/4] Refactor transactionsUpdated function containing API call into action --- src/store/middlewares/account.js | 30 ++++++---------------- src/store/middlewares/account.test.js | 36 +++++++++------------------ 2 files changed, 20 insertions(+), 46 deletions(-) diff --git a/src/store/middlewares/account.js b/src/store/middlewares/account.js index 0a52f46181..6719b4634e 100644 --- a/src/store/middlewares/account.js +++ b/src/store/middlewares/account.js @@ -1,12 +1,11 @@ import { getAccount } from '../../utils/api/account'; import { accountUpdated } from '../../actions/account'; -import { transactionsUpdateUnconfirmed } from '../../actions/transactions'; +import { transactionsUpdated } from '../../actions/transactions'; import { activePeerUpdate } from '../../actions/peers'; import { votesFetched } from '../../actions/voting'; import actionTypes from '../../constants/actions'; import accountConfig from '../../constants/account'; import { getDelegate } from '../../utils/api/delegate'; -import { getTransactions } from '../../utils/api/transactions'; import transactionTypes from '../../constants/transactionTypes'; const { lockDuration } = accountConfig; @@ -18,24 +17,13 @@ const updateTransactions = (store, peers) => { ? state.transactions.account.address : state.account.address; - getTransactions({ - activePeer: peers.data, address, limit: 25, filter, - }).then((response) => { - store.dispatch({ - data: { - confirmed: response.transactions, - count: parseInt(response.count, 10), - }, - type: actionTypes.transactionsUpdated, - }); - if (state.transactions.pending.length) { - store.dispatch(transactionsUpdateUnconfirmed({ - activePeer: peers.data, - address, - pendingTransactions: state.transactions.pending, - })); - } - }); + store.dispatch(transactionsUpdated({ + pendingTransactions: state.transactions.pending, + activePeer: peers.data, + address, + limit: 25, + filter, + })); }; const hasRecentTransactions = txs => ( @@ -46,7 +34,6 @@ const hasRecentTransactions = txs => ( const updateAccountData = (store, action) => { const { peers, account, transactions } = store.getState(); - // all dispatches getAccount(peers.data, account.address).then((result) => { if (result.balance !== account.balance) { if (!action.data.windowIsFocused || !hasRecentTransactions(transactions)) { @@ -76,7 +63,6 @@ const delegateRegistration = (store, action) => { const state = store.getState(); if (delegateRegistrationTx) { - // all dispatches no api calls and no dispatches after api call getDelegate(state.peers.data, { publicKey: state.account.publicKey }) .then((delegateData) => { store.dispatch(accountUpdated(Object.assign( diff --git a/src/store/middlewares/account.test.js b/src/store/middlewares/account.test.js index 795d7c2ea3..aee85a5428 100644 --- a/src/store/middlewares/account.test.js +++ b/src/store/middlewares/account.test.js @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { spy, stub, useFakeTimers, match } from 'sinon'; import { accountUpdated } from '../../actions/account'; +import * as transactionsActions from '../../actions/transactions'; import accountConfig from '../../constants/account'; import { activePeerUpdate } from '../../actions/peers'; import * as votingActions from '../../actions/voting'; @@ -19,6 +20,7 @@ describe('Account middleware', () => { let state; let stubGetAccount; let stubTransactions; + let transactionsActionsStub; const { passphrase } = accounts.genesis; const transactions = { transactions: [{ senderId: 'sample_address', receiverId: 'some_address' }] }; @@ -83,10 +85,12 @@ describe('Account middleware', () => { next = spy(); stubGetAccount = stub(accountApi, 'getAccount').returnsPromise(); + transactionsActionsStub = spy(transactionsActions, 'transactionsUpdated'); stubTransactions = stub(transactionsApi, 'getTransactions').returnsPromise().resolves(true); }); afterEach(() => { + transactionsActionsStub.restore(); stubGetAccount.restore(); stubTransactions.restore(); clock.restore(); @@ -119,20 +123,17 @@ describe('Account middleware', () => { it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes`, () => { stubGetAccount.resolves({ balance: 10e8 }); - middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(stubTransactions).to.have.been.calledWith(); + expect(transactionsActionsStub).to.have.been.calledWith(); }); it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes and the window is in blur`, () => { stubGetAccount.resolves({ balance: 10e8 }); middleware(store)(next)(inactiveNewBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(stubTransactions).to.have.been.calledWith(); + expect(transactionsActionsStub).to.have.been.calledWith(); }); it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes the user has no transactions yet`, () => { @@ -143,22 +144,21 @@ describe('Account middleware', () => { expect(stubGetAccount).to.have.been.calledWith(); // eslint-disable-next-line no-unused-expressions - expect(stubTransactions).to.have.been.calledOnce; + expect(transactionsActionsStub).to.have.been.calledOnce; }); it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if the window is in focus and there are recent transactions`, () => { stubGetAccount.resolves({ balance: 0 }); middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(stubTransactions).to.have.been.calledWith(match({ address: 'test_address' })); + expect(transactionsActionsStub).to.have.been.calledWith(match({ address: 'test_address' })); }); it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if block.transactions contains null element`, () => { middleware(store)(next)(blockWithNullTransaction); - expect(store.dispatch).to.have.been.calledWith(match.has('type', actionTypes.transactionsUpdated)); + expect(transactionsActionsStub).to.have.been.calledWith(); }); it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confired does not contain recent transaction. Case with transactions address`, () => { @@ -175,7 +175,7 @@ describe('Account middleware', () => { middleware(store)(next)(newBlockCreated); expect(stubGetAccount).to.have.been.calledWith({}); - expect(store.dispatch).to.have.been.calledWith(match.has('type', actionTypes.transactionsUpdated)); + expect(transactionsActionsStub).to.have.been.calledWith(); }); it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confired does not contain recent transaction. Case with confirmed address`, () => { @@ -193,26 +193,14 @@ describe('Account middleware', () => { middleware(store)(next)(newBlockCreated); expect(stubGetAccount).to.have.been.calledWith({}); - expect(store.dispatch).to.have.been.calledWith(match.has('type', actionTypes.transactionsUpdated)); - }); - - it(`should fetch delegate info on ${actionTypes.newBlockCreated} action if account.balance changes and account.isDelegate`, () => { - const delegateApiMock = stub(delegateApi, 'getDelegate').returnsPromise().resolves({ success: true, delegate: {} }); - stubGetAccount.resolves({ balance: 10e8 }); - state.account.isDelegate = true; - store.getState = () => (state); - - middleware(store)(next)(newBlockCreated); - expect(store.dispatch).to.have.been.calledWith(); - - delegateApiMock.restore(); + expect(transactionsActionsStub).to.have.been.calledWith(); }); it(`should fetch delegate info on ${actionTypes.transactionsUpdated} action if action.data.confirmed contains delegateRegistration transactions`, () => { const delegateApiMock = stub(delegateApi, 'getDelegate').returnsPromise().resolves({ success: true, delegate: {} }); middleware(store)(next)(transactionsUpdatedAction); - expect(store.dispatch).to.have.been.calledWith(); + expect(delegateApiMock).to.have.been.calledWith(); delegateApiMock.restore(); }); From 9b0f375f4367ac2a3fc991f1e35a330f065745cb Mon Sep 17 00:00:00 2001 From: Gina Contrino Date: Mon, 11 Jun 2018 15:22:29 +0200 Subject: [PATCH 3/4] Refactor more functions from account middleware to account action and update tests --- src/actions/account.js | 47 ++++++- src/actions/account.test.js | 179 +++++++++++++++++++++++++- src/actions/search.js | 8 +- src/actions/transactions.js | 20 ++- src/actions/transactions.test.js | 2 +- src/store/middlewares/account.js | 65 +++------- src/store/middlewares/account.test.js | 118 +++++------------ src/utils/api/transactions.js | 12 +- 8 files changed, 301 insertions(+), 150 deletions(-) diff --git a/src/actions/account.js b/src/actions/account.js index 30ad88c9c4..66572099d7 100644 --- a/src/actions/account.js +++ b/src/actions/account.js @@ -2,9 +2,10 @@ import i18next from 'i18next'; import actionTypes from '../constants/actions'; import { setSecondPassphrase, getAccount } from '../utils/api/account'; import { registerDelegate, getDelegate, getVotes, getVoters } from '../utils/api/delegate'; -import { loadTransactionsFinish } from './transactions'; +import { loadTransactionsFinish, transactionsUpdated } from './transactions'; import { delegateRegisteredFailure } from './delegate'; import { errorAlertDialogDisplayed } from './dialog'; +import { activePeerUpdate } from './peers'; import Fees from '../constants/fees'; import transactionTypes from '../constants/transactionTypes'; @@ -159,7 +160,6 @@ export const loadAccount = ({ transactionsResponse, isSameAccount, }) => - (dispatch) => { getAccount(activePeer, address) .then((response) => { @@ -184,3 +184,46 @@ export const loadAccount = ({ dispatch(loadTransactionsFinish(accountDataUpdated)); }); }; + +export const updateTransactionsIfNeeded = ({ transactions, activePeer, account }, windowFocus) => + (dispatch) => { + const hasRecentTransactions = txs => ( + txs.confirmed.filter(tx => tx.confirmations < 1000).length !== 0 || + txs.pending.length !== 0 + ); + + if (windowFocus || !hasRecentTransactions(transactions)) { + const { filter } = transactions; + const address = transactions.account ? transactions.account.address : account.address; + + dispatch(transactionsUpdated({ + pendingTransactions: transactions.pending, + activePeer, + address, + limit: 25, + filter, + })); + } + }; + +export const accountDataUpdated = ({ + peers, account, windowIsFocused, transactions, +}) => + (dispatch) => { + getAccount(peers.data, account.address).then((result) => { + if (result.balance !== account.balance) { + dispatch(updateTransactionsIfNeeded( + { + transactions, + activePeer: peers.data, + account, + }, + !windowIsFocused, + )); + } + dispatch(accountUpdated(result)); + dispatch(activePeerUpdate({ online: true })); + }).catch((res) => { + dispatch(activePeerUpdate({ online: false, code: res.error.code })); + }); + }; diff --git a/src/actions/account.test.js b/src/actions/account.test.js index 9b5516fcfa..1e784dbd34 100644 --- a/src/actions/account.test.js +++ b/src/actions/account.test.js @@ -1,5 +1,5 @@ import { expect } from 'chai'; -import sinon from 'sinon'; +import { spy, stub } from 'sinon'; import actionTypes from '../constants/actions'; import { accountUpdated, @@ -9,6 +9,9 @@ import { removePassphrase, passphraseUsed, loadDelegate, + loadAccount, + accountDataUpdated, + updateTransactionsIfNeeded, } from './account'; import { errorAlertDialogDisplayed } from './dialog'; import { delegateRegisteredFailure } from './delegate'; @@ -18,6 +21,8 @@ import Fees from '../constants/fees'; import transactionTypes from '../constants/transactionTypes'; import networks from '../constants/networks'; import accounts from '../../test/constants/accounts'; +import * as peersActions from './peers'; +import * as transactionsActions from './transactions'; describe('actions: account', () => { describe('accountUpdated', () => { @@ -58,8 +63,8 @@ describe('actions: account', () => { let dispatch; beforeEach(() => { - accountApiMock = sinon.stub(accountApi, 'setSecondPassphrase'); - dispatch = sinon.spy(); + accountApiMock = stub(accountApi, 'setSecondPassphrase'); + dispatch = spy(); }); afterEach(() => { @@ -119,8 +124,8 @@ describe('actions: account', () => { let dispatch; beforeEach(() => { - delegateApiMock = sinon.stub(delegateApi, 'registerDelegate'); - dispatch = sinon.spy(); + delegateApiMock = stub(delegateApi, 'registerDelegate'); + dispatch = spy(); }); afterEach(() => { @@ -175,8 +180,8 @@ describe('actions: account', () => { const actionFunction = loadDelegate(data); beforeEach(() => { - delegateApiMock = sinon.stub(delegateApi, 'getDelegate'); - dispatch = sinon.spy(); + delegateApiMock = stub(delegateApi, 'getDelegate'); + dispatch = spy(); }); afterEach(() => { @@ -212,4 +217,164 @@ describe('actions: account', () => { expect(removePassphrase(data)).to.be.deep.equal(expectedAction); }); }); + + describe('loadAccount', () => { + let getAccountStub; + let transactionsActionsStub; + + const dispatch = spy(); + + beforeEach(() => { + getAccountStub = stub(accountApi, 'getAccount').returnsPromise(); + transactionsActionsStub = spy(transactionsActions, 'loadTransactionsFinish'); + }); + + afterEach(() => { + getAccountStub.restore(); + transactionsActionsStub.restore(); + }); + + it('should finish transactions load and load delegate if not own account', () => { + getAccountStub.resolves({ + balance: 10e8, + publicKey: accounts.genesis.publicKey, + isDelegate: false, + }); + + const data = { + activePeer: {}, + address: accounts.genesis.address, + transactionsResponse: { count: 0, transactions: [] }, + isSameAccount: false, + }; + + loadAccount(data)(dispatch); + expect(transactionsActionsStub).to.have.been.calledWith({ + confirmed: [], + count: 0, + balance: 10e8, + address: accounts.genesis.address, + }); + }); + + it('should finish transactions load and should not load delegate if own account', () => { + getAccountStub.resolves({ + balance: 10e8, + publicKey: accounts.genesis.publicKey, + isDelegate: true, + delegate: 'delegate information', + }); + + const data = { + activePeer: {}, + address: accounts.genesis.address, + transactionsResponse: { count: 0, transactions: [] }, + isSameAccount: true, + }; + + loadAccount(data)(dispatch); + expect(transactionsActionsStub).to.have.been.calledWith({ + confirmed: [], + count: 0, + balance: 10e8, + address: accounts.genesis.address, + delegate: 'delegate information', + }); + }); + }); + + describe('accountDataUpdated', () => { + let peersActionsStub; + let getAccountStub; + let transactionsActionsStub; + + const dispatch = spy(); + + beforeEach(() => { + peersActionsStub = spy(peersActions, 'activePeerUpdate'); + getAccountStub = stub(accountApi, 'getAccount').returnsPromise(); + transactionsActionsStub = spy(transactionsActions, 'transactionsUpdated'); + }); + + afterEach(() => { + getAccountStub.restore(); + peersActionsStub.restore(); + transactionsActionsStub.restore(); + }); + + it(`should call account API methods on ${actionTypes.newBlockCreated} action when online`, () => { + getAccountStub.resolves({ balance: 10e8 }); + + const data = { + windowIsFocused: false, + peers: { data: {} }, + transactions: { + pending: [{ + id: 12498250891724098, + }], + confirmed: [], + account: { address: 'test_address', balance: 0 }, + }, + account: { address: accounts.genesis.address, balance: 0 }, + }; + + accountDataUpdated(data)(dispatch); + expect(dispatch).to.have.callCount(3); + expect(peersActionsStub).to.have.not.been.calledWith({ online: false, code: 'EUNAVAILABLE' }); + }); + + it(`should call account API methods on ${actionTypes.newBlockCreated} action when offline`, () => { + getAccountStub.rejects({ error: { code: 'EUNAVAILABLE' } }); + + const data = { + windowIsFocused: true, + peers: { data: {} }, + transactions: { + pending: [{ id: 12498250891724098 }], + confirmed: [], + account: { address: 'test_address', balance: 0 }, + }, + account: { address: accounts.genesis.address }, + }; + + accountDataUpdated(data)(dispatch); + expect(peersActionsStub).to.have.been.calledWith({ online: false, code: 'EUNAVAILABLE' }); + }); + }); + + describe('updateTransactionsIfNeeded', () => { + let transactionsActionsStub; + + const dispatch = spy(); + + beforeEach(() => { + transactionsActionsStub = spy(transactionsActions, 'transactionsUpdated'); + }); + + afterEach(() => { + transactionsActionsStub.restore(); + }); + + it('should update transactions when window is in focus', () => { + const data = { + activePeer: {}, + transactions: { confirmed: [{ confirmations: 10 }], pending: [] }, + account: { address: accounts.genesis.address }, + }; + + updateTransactionsIfNeeded(data, true)(dispatch); + expect(transactionsActionsStub).to.have.been.calledWith(); + }); + + it('should update transactions when there are no recent transactions', () => { + const data = { + activePeer: {}, + transactions: { confirmed: [{ confirmations: 10000 }], pending: [] }, + account: { address: accounts.genesis.address }, + }; + + updateTransactionsIfNeeded(data, false)(dispatch); + expect(transactionsActionsStub).to.have.been.calledWith(); + }); + }); }); diff --git a/src/actions/search.js b/src/actions/search.js index 1eb3f6c705..847ecb853f 100644 --- a/src/actions/search.js +++ b/src/actions/search.js @@ -63,7 +63,9 @@ export const searchTransactions = ({ }) => (dispatch) => { if (showLoading) loadingStarted(actionTypes.searchTransactions); - getTransactions({ activePeer, address, limit, filter }) + getTransactions({ + activePeer, address, limit, filter, + }) .then((transactionsResponse) => { dispatch({ data: { @@ -82,7 +84,9 @@ export const searchMoreTransactions = ({ activePeer, address, limit, offset, filter, }) => (dispatch) => { - getTransactions({ activePeer, address, limit, offset, filter }) + getTransactions({ + activePeer, address, limit, offset, filter, + }) .then((transactionsResponse) => { dispatch({ data: { diff --git a/src/actions/transactions.js b/src/actions/transactions.js index 41df9404b5..e8d668da62 100644 --- a/src/actions/transactions.js +++ b/src/actions/transactions.js @@ -72,9 +72,13 @@ export const loadTransactions = ({ activePeer, publicKey, address }) => }); }; -export const transactionsRequested = ({ activePeer, address, limit, offset, filter }) => +export const transactionsRequested = ({ + activePeer, address, limit, offset, filter, +}) => (dispatch) => { - getTransactions({ activePeer, address, limit, offset, filter }) + getTransactions({ + activePeer, address, limit, offset, filter, + }) .then((response) => { dispatch({ data: { @@ -144,9 +148,13 @@ export const loadTransaction = ({ activePeer, id }) => }); }; -export const transactionsUpdated = ({ activePeer, address, limit, filter, pendingTransactions }) => +export const transactionsUpdated = ({ + activePeer, address, limit, filter, pendingTransactions, +}) => (dispatch) => { - getTransactions({ activePeer, address, limit, filter }) + getTransactions({ + activePeer, address, limit, filter, + }) .then((response) => { dispatch({ data: { @@ -165,7 +173,9 @@ export const transactionsUpdated = ({ activePeer, address, limit, filter, pendin }); }; -export const sent = ({ activePeer, account, recipientId, amount, passphrase, secondPassphrase }) => +export const sent = ({ + activePeer, account, recipientId, amount, passphrase, secondPassphrase, +}) => (dispatch) => { send(activePeer, recipientId, toRawLsk(amount), passphrase, secondPassphrase) .then((data) => { diff --git a/src/actions/transactions.test.js b/src/actions/transactions.test.js index 558b1eb36d..458bcf4ea7 100644 --- a/src/actions/transactions.test.js +++ b/src/actions/transactions.test.js @@ -32,7 +32,7 @@ describe('actions: transactions', () => { transactionsApiMock.restore(); }); - it('should dispatch transactionsLoaded action if resolved', () => { + it('should dispatch transactionsUpdated action if resolved', () => { transactionsApiMock.returnsPromise().resolves({ transactions: [], count: '0' }); const expectedAction = { count: 0, diff --git a/src/store/middlewares/account.js b/src/store/middlewares/account.js index 6719b4634e..68d89c01a3 100644 --- a/src/store/middlewares/account.js +++ b/src/store/middlewares/account.js @@ -1,7 +1,4 @@ -import { getAccount } from '../../utils/api/account'; -import { accountUpdated } from '../../actions/account'; -import { transactionsUpdated } from '../../actions/transactions'; -import { activePeerUpdate } from '../../actions/peers'; +import { accountUpdated, accountDataUpdated, updateTransactionsIfNeeded } from '../../actions/account'; import { votesFetched } from '../../actions/voting'; import actionTypes from '../../constants/actions'; import accountConfig from '../../constants/account'; @@ -10,41 +7,15 @@ import transactionTypes from '../../constants/transactionTypes'; const { lockDuration } = accountConfig; -const updateTransactions = (store, peers) => { - const state = store.getState(); - const { filter } = state.transactions; - const address = state.transactions.account - ? state.transactions.account.address - : state.account.address; - - store.dispatch(transactionsUpdated({ - pendingTransactions: state.transactions.pending, - activePeer: peers.data, - address, - limit: 25, - filter, - })); -}; - -const hasRecentTransactions = txs => ( - txs.confirmed.filter(tx => tx.confirmations < 1000).length !== 0 || - txs.pending.length !== 0 -); - const updateAccountData = (store, action) => { const { peers, account, transactions } = store.getState(); - getAccount(peers.data, account.address).then((result) => { - if (result.balance !== account.balance) { - if (!action.data.windowIsFocused || !hasRecentTransactions(transactions)) { - updateTransactions(store, peers, account); - } - } - store.dispatch(accountUpdated(result)); - store.dispatch(activePeerUpdate({ online: true })); - }).catch((res) => { - store.dispatch(activePeerUpdate({ online: false, code: res.error.code })); - }); + store.dispatch(accountDataUpdated({ + windowIsFocused: action.data.windowIsFocused, + transactions, + account, + peers, + })); }; const getRecentTransactionOfType = (transactionsList, type) => ( @@ -89,23 +60,27 @@ const votePlaced = (store, action) => { }; const passphraseUsed = (store, action) => { + const data = { expireTime: Date.now() + lockDuration }; + if (!store.getState().account.passphrase) { - store.dispatch(accountUpdated({ - passphrase: action.data, - expireTime: Date.now() + lockDuration, - })); - } else { - store.dispatch(accountUpdated({ expireTime: Date.now() + lockDuration })); + data.passphrase = action.data; } + + store.dispatch(accountUpdated(data)); }; const checkTransactionsAndUpdateAccount = (store, action) => { const state = store.getState(); const { peers, account, transactions } = state; - if (action.data.windowIsFocused && hasRecentTransactions(transactions)) { - updateTransactions(store, peers, account); - } + store.dispatch(updateTransactionsIfNeeded( + { + transactions, + activePeer: peers.data, + account, + }, + action.data.windowIsFocused, + )); const tx = action.data.block.transactions; const accountAddress = state.account.address; diff --git a/src/store/middlewares/account.test.js b/src/store/middlewares/account.test.js index aee85a5428..78ec6734fe 100644 --- a/src/store/middlewares/account.test.js +++ b/src/store/middlewares/account.test.js @@ -1,9 +1,8 @@ import { expect } from 'chai'; -import { spy, stub, useFakeTimers, match } from 'sinon'; -import { accountUpdated } from '../../actions/account'; +import { spy, stub, useFakeTimers } from 'sinon'; +import * as accountActions from '../../actions/account'; import * as transactionsActions from '../../actions/transactions'; import accountConfig from '../../constants/account'; -import { activePeerUpdate } from '../../actions/peers'; import * as votingActions from '../../actions/voting'; import * as accountApi from '../../utils/api/account'; import * as transactionsApi from '../../utils/api/transactions'; @@ -43,22 +42,6 @@ describe('Account middleware', () => { }, }; - const inactiveNewBlockCreated = { - type: actionTypes.newBlockCreated, - data: { - windowIsFocused: false, - block: transactions, - }, - }; - - const blockWithNullTransaction = { - type: actionTypes.newBlockCreated, - data: { - windowIsFocused: true, - block: { transactions: [null] }, - }, - }; - let clock; beforeEach(() => { @@ -84,12 +67,14 @@ describe('Account middleware', () => { store.getState = () => (state); next = spy(); + spy(accountActions, 'updateTransactionsIfNeeded'); stubGetAccount = stub(accountApi, 'getAccount').returnsPromise(); transactionsActionsStub = spy(transactionsActions, 'transactionsUpdated'); stubTransactions = stub(transactionsApi, 'getTransactions').returnsPromise().resolves(true); }); afterEach(() => { + accountActions.updateTransactionsIfNeeded.restore(); transactionsActionsStub.restore(); stubGetAccount.restore(); stubTransactions.restore(); @@ -102,67 +87,23 @@ describe('Account middleware', () => { }); it(`should call account API methods on ${actionTypes.newBlockCreated} action when online`, () => { - // does this matter? - stubGetAccount.resolves({ balance: 0 }); - - middleware(store)(next)(newBlockCreated); - - expect(stubGetAccount).to.have.been.calledWith(); - expect(store.dispatch).to.have.been.calledWith(activePeerUpdate({ online: true })); - }); - - it(`should call account API methods on ${actionTypes.newBlockCreated} action when offline`, () => { - const errorCode = 'EUNAVAILABLE'; - stubGetAccount.rejects({ error: { code: errorCode } }); - - middleware(store)(next)(newBlockCreated); - - expect(store.dispatch).to.have.been - .calledWith(activePeerUpdate({ online: false, code: errorCode })); - }); - - it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes`, () => { - stubGetAccount.resolves({ balance: 10e8 }); - middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(transactionsActionsStub).to.have.been.calledWith(); - }); - - it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes and the window is in blur`, () => { - stubGetAccount.resolves({ balance: 10e8 }); - - middleware(store)(next)(inactiveNewBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(transactionsActionsStub).to.have.been.calledWith(); - }); - - it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if account.balance changes the user has no transactions yet`, () => { - stubGetAccount.resolves({ balance: 10e8 }); - - state.transactions.count = 0; - middleware(store)(next)(newBlockCreated); - - expect(stubGetAccount).to.have.been.calledWith(); - // eslint-disable-next-line no-unused-expressions - expect(transactionsActionsStub).to.have.been.calledOnce; - }); - - it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if the window is in focus and there are recent transactions`, () => { - stubGetAccount.resolves({ balance: 0 }); - + const accountDataUpdatedSpy = spy(accountActions, 'accountDataUpdated'); middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith(); - expect(transactionsActionsStub).to.have.been.calledWith(match({ address: 'test_address' })); - }); - it(`should call transactions API methods on ${actionTypes.newBlockCreated} action if block.transactions contains null element`, () => { - middleware(store)(next)(blockWithNullTransaction); + const data = { + windowIsFocused: true, + peers: state.peers, + account: state.account, + transactions: state.transactions, + }; - expect(transactionsActionsStub).to.have.been.calledWith(); + expect(accountDataUpdatedSpy).to.have.been.calledWith(data); + expect(accountActions.updateTransactionsIfNeeded).to.have.been.calledWith(); + accountDataUpdatedSpy.restore(); }); - it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confired does not contain recent transaction. Case with transactions address`, () => { - stubGetAccount.resolves({ balance: 0 }); + it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confirmed does not contain recent transaction. Case with transactions address`, () => { + const accountDataUpdatedSpy = spy(accountActions, 'accountDataUpdated'); store.getState = () => ({ ...state, @@ -174,12 +115,12 @@ describe('Account middleware', () => { }); middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith({}); - expect(transactionsActionsStub).to.have.been.calledWith(); + expect(accountDataUpdatedSpy).to.have.been.calledWith(); + accountDataUpdatedSpy.restore(); }); - it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confired does not contain recent transaction. Case with confirmed address`, () => { - stubGetAccount.resolves({ balance: 0 }); + it(`should call API methods on ${actionTypes.newBlockCreated} action if state.transaction.transactions.confirmed does not contain recent transaction. Case with confirmed address`, () => { + const accountDataUpdatedSpy = spy(accountActions, 'accountDataUpdated'); store.getState = () => ({ ...state, @@ -192,8 +133,8 @@ describe('Account middleware', () => { }); middleware(store)(next)(newBlockCreated); - expect(stubGetAccount).to.have.been.calledWith({}); - expect(transactionsActionsStub).to.have.been.calledWith(); + expect(accountDataUpdatedSpy).to.have.been.calledWith(); + accountDataUpdatedSpy.restore(); }); it(`should fetch delegate info on ${actionTypes.transactionsUpdated} action if action.data.confirmed contains delegateRegistration transactions`, () => { @@ -227,16 +168,22 @@ describe('Account middleware', () => { }); it(`should dispatch accountUpdated({passphrase}) action on ${actionTypes.passphraseUsed} action if store.account.passphrase is not set`, () => { + const accountUpdatedSpy = spy(accountActions, 'accountUpdated'); + const action = { type: actionTypes.passphraseUsed, data: passphrase, }; middleware(store)(next)(action); - expect(store.dispatch).to.have.been - .calledWith(accountUpdated({ passphrase, expireTime: clock.now + lockDuration })); + expect(accountUpdatedSpy).to.have.been.calledWith({ + passphrase, + expireTime: clock.now + lockDuration, + }); + accountUpdatedSpy.restore(); }); it(`should not dispatch accountUpdated action on ${actionTypes.passphraseUsed} action if store.account.passphrase is already set`, () => { + const accountUpdatedSpy = spy(accountActions, 'accountUpdated'); const action = { type: actionTypes.passphraseUsed, data: passphrase, @@ -245,8 +192,9 @@ describe('Account middleware', () => { ...state, account: { ...state.account, passphrase, expireTime: clock.now + lockDuration }, }); + middleware(store)(next)(action); - expect(store.dispatch).to.have.been - .calledWith(accountUpdated({ expireTime: clock.now + lockDuration })); + expect(accountUpdatedSpy).to.have.been.calledWith({ expireTime: clock.now + lockDuration }); + accountUpdatedSpy.restore(); }); }); diff --git a/src/utils/api/transactions.js b/src/utils/api/transactions.js index 90494b00b0..a744c54111 100644 --- a/src/utils/api/transactions.js +++ b/src/utils/api/transactions.js @@ -2,10 +2,16 @@ import { requestToActivePeer } from './peers'; import txFilters from './../../constants/transactionFilters'; export const send = (activePeer, recipientId, amount, secret, secondSecret = null) => - requestToActivePeer(activePeer, 'transactions', - { recipientId, amount, secret, secondSecret }); + requestToActivePeer( + activePeer, 'transactions', + { + recipientId, amount, secret, secondSecret, + }, + ); -export const getTransactions = ({ activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc', filter = txFilters.all }) => { +export const getTransactions = ({ + activePeer, address, limit = 20, offset = 0, orderBy = 'timestamp:desc', filter = txFilters.all, +}) => { let params = { recipientId: (filter === txFilters.incoming || filter === txFilters.all) ? address : undefined, senderId: (filter === txFilters.outgoing || filter === txFilters.all) ? address : undefined, From 41319a98e868bc225a053cded62e2ad9db779c8a Mon Sep 17 00:00:00 2001 From: Gina Contrino Date: Mon, 11 Jun 2018 17:38:52 +0200 Subject: [PATCH 4/4] move delegate api call to account actions --- src/actions/account.js | 12 ++++++++++++ src/actions/account.test.js | 26 ++++++++++++++++++++++++++ src/store/middlewares/account.js | 18 +++++++++--------- src/store/middlewares/account.test.js | 12 +++--------- 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/actions/account.js b/src/actions/account.js index 66572099d7..1b95a9fe2c 100644 --- a/src/actions/account.js +++ b/src/actions/account.js @@ -113,6 +113,18 @@ export const secondPassphraseRegistered = ({ dispatch(passphraseUsed(passphrase)); }; + +export const updateDelegateAccount = ({ activePeer, publicKey }) => + (dispatch) => { + getDelegate(activePeer, { publicKey }) + .then((delegateData) => { + dispatch(accountUpdated(Object.assign( + {}, + { delegate: delegateData.delegate, isDelegate: true }, + ))); + }); + }; + /** * */ diff --git a/src/actions/account.test.js b/src/actions/account.test.js index 1e784dbd34..46a22b5988 100644 --- a/src/actions/account.test.js +++ b/src/actions/account.test.js @@ -12,6 +12,7 @@ import { loadAccount, accountDataUpdated, updateTransactionsIfNeeded, + updateDelegateAccount, } from './account'; import { errorAlertDialogDisplayed } from './dialog'; import { delegateRegisteredFailure } from './delegate'; @@ -377,4 +378,29 @@ describe('actions: account', () => { expect(transactionsActionsStub).to.have.been.calledWith(); }); }); + + describe('updateDelegateAccount', () => { + const dispatch = spy(); + + beforeEach(() => { + stub(delegateApi, 'getDelegate').returnsPromise(); + }); + + afterEach(() => { + delegateApi.getDelegate.restore(); + }); + + it('should fetch delegate and update account', () => { + delegateApi.getDelegate.resolves({ delegate: 'delegate data' }); + const data = { + activePeer: {}, + publicKey: accounts.genesis.publicKey, + }; + + updateDelegateAccount(data)(dispatch); + + const accountUpdatedAction = accountUpdated(Object.assign({}, { delegate: 'delegate data', isDelegate: true })); + expect(dispatch).to.have.been.calledWith(accountUpdatedAction); + }); + }); }); diff --git a/src/store/middlewares/account.js b/src/store/middlewares/account.js index 68d89c01a3..fe9e59c8f4 100644 --- a/src/store/middlewares/account.js +++ b/src/store/middlewares/account.js @@ -1,8 +1,11 @@ -import { accountUpdated, accountDataUpdated, updateTransactionsIfNeeded } from '../../actions/account'; +import { accountUpdated, + accountDataUpdated, + updateTransactionsIfNeeded, + updateDelegateAccount, +} from '../../actions/account'; import { votesFetched } from '../../actions/voting'; import actionTypes from '../../constants/actions'; import accountConfig from '../../constants/account'; -import { getDelegate } from '../../utils/api/delegate'; import transactionTypes from '../../constants/transactionTypes'; const { lockDuration } = accountConfig; @@ -34,13 +37,10 @@ const delegateRegistration = (store, action) => { const state = store.getState(); if (delegateRegistrationTx) { - getDelegate(state.peers.data, { publicKey: state.account.publicKey }) - .then((delegateData) => { - store.dispatch(accountUpdated(Object.assign( - {}, - { delegate: delegateData.delegate, isDelegate: true }, - ))); - }); + store.dispatch(updateDelegateAccount({ + activePeer: state.peers.data, + publicKey: state.account.publicKey, + })); } }; diff --git a/src/store/middlewares/account.test.js b/src/store/middlewares/account.test.js index 78ec6734fe..8b2574876a 100644 --- a/src/store/middlewares/account.test.js +++ b/src/store/middlewares/account.test.js @@ -8,7 +8,6 @@ import * as accountApi from '../../utils/api/account'; import * as transactionsApi from '../../utils/api/transactions'; import accounts from '../../../test/constants/accounts'; import actionTypes from '../../constants/actions'; -import * as delegateApi from '../../utils/api/delegate'; import middleware from './account'; import transactionTypes from '../../constants/transactionTypes'; @@ -68,12 +67,14 @@ describe('Account middleware', () => { next = spy(); spy(accountActions, 'updateTransactionsIfNeeded'); + spy(accountActions, 'updateDelegateAccount'); stubGetAccount = stub(accountApi, 'getAccount').returnsPromise(); transactionsActionsStub = spy(transactionsActions, 'transactionsUpdated'); stubTransactions = stub(transactionsApi, 'getTransactions').returnsPromise().resolves(true); }); afterEach(() => { + accountActions.updateDelegateAccount.restore(); accountActions.updateTransactionsIfNeeded.restore(); transactionsActionsStub.restore(); stubGetAccount.restore(); @@ -138,22 +139,15 @@ describe('Account middleware', () => { }); it(`should fetch delegate info on ${actionTypes.transactionsUpdated} action if action.data.confirmed contains delegateRegistration transactions`, () => { - const delegateApiMock = stub(delegateApi, 'getDelegate').returnsPromise().resolves({ success: true, delegate: {} }); - middleware(store)(next)(transactionsUpdatedAction); - expect(delegateApiMock).to.have.been.calledWith(); - - delegateApiMock.restore(); + expect(accountActions.updateDelegateAccount).to.have.been.calledWith(); }); it(`should not fetch delegate info on ${actionTypes.transactionsUpdated} action if action.data.confirmed does not contain delegateRegistration transactions`, () => { - const delegateApiMock = stub(delegateApi, 'getDelegate').returnsPromise().resolves({ success: true, delegate: {} }); transactionsUpdatedAction.data.confirmed[0].type = transactionTypes.send; middleware(store)(next)(transactionsUpdatedAction); expect(store.dispatch).to.not.have.been.calledWith(); - - delegateApiMock.restore(); }); it(`should dispatch ${actionTypes.votesFetched} action on ${actionTypes.transactionsUpdated} action if action.data.confirmed contains delegateRegistration transactions`, () => {