Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Bugfix: Registering delegate doesn't update UI - Closes #682 #688

Merged
merged 5 commits into from
Sep 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions features/menu.feature
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ Feature: Top right menu
And I fill in "test" to "username" field
And I click "register button"
Then I should see alert dialog with title "Success" and text "Delegate registration was successfully submitted. It can take several seconds before it is processed."
And I click "ok button"
And I wait 15 seconds
And I should see text "test" in "delegate name" element

Scenario: should not allow to register a delegate again
Given I'm logged in as "delegate"
Expand Down
4 changes: 4 additions & 0 deletions features/step_definitions/generic.step.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ defineSupportCode(({ Given, When, Then, setDefaultTimeout }) => {
waitForElemAndSendKeys(`${selectorClass} input, ${selectorClass} textarea`, secondPassphrase, callback);
});

When('I wait {seconds} seconds', (seconds, callback) => {
browser.sleep(seconds * 1000).then(callback);
});


Then('I should see "{value}" in "{fieldName}" field', (value, fieldName, callback) => {
const elem = element(by.css(`.${fieldName.replace(/ /g, '-')} input, .${fieldName.replace(/ /g, '-')} textarea`));
Expand Down
2 changes: 1 addition & 1 deletion src/components/account/address.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const Address = (props) => {
const title = props.isDelegate ? 'Delegate' : 'Address';
const content = props.isDelegate ?
(<div>
<p className="inner primary">
<p className="inner primary delegate-name">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove delegate-name class and use an ID instead of that. when we want to select a an element in js , it is better to use ID.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this class for e2e tests. All e2e tests use classes for selectors. It is not possible to change just this one. We can discuss if we should change classes to IDs in e2e tests, but it is definitely outside the scope of this PR.

{props.delegate.username}
</p>
<p className="inner secondary address">
Expand Down
2 changes: 1 addition & 1 deletion src/components/dialog/alert.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const Alert = props => (
<br />
<section className={`${grid.row} ${grid['between-xs']}`}>
<span />
<Button label='Ok' onClick={props.closeDialog}/>
<Button label='Ok' onClick={props.closeDialog} className='ok-button'/>
</section>
</div>
);
Expand Down
10 changes: 6 additions & 4 deletions src/components/transactions/amount.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@ import styles from './transactions.css';
import LiskAmount from '../liskAmount';
import { TooltipWrapper } from '../timestamp';
import ClickToSend from '../clickToSend';
import transactionTypes from '../../constants/transactionTypes';

const Amount = (props) => {
const params = {};
if (props.value.type === 0 &&
if (props.value.type === transactionTypes.send &&
props.value.senderId === props.value.recipientId) {
params.className = 'grayButton';
} else if (props.value.senderId !== props.address) {
params.className = 'inButton';
} else if (props.value.type !== 0 || props.value.recipientId !== props.address) {
} else if (props.value.type !== transactionTypes.send ||
props.value.recipientId !== props.address) {
params.className = 'outButton';
params.tooltipText = props.value.type === 0 ? 'Repeat the transaction' : undefined;
params.clickToSendEnabled = props.value.type === 0;
params.tooltipText = props.value.type === transactionTypes.send ? 'Repeat the transaction' : undefined;
params.clickToSendEnabled = props.value.type === transactionTypes.send;
}
return <TooltipWrapper tooltip={params.tooltipText}>
<ClickToSend rawAmount={props.value.amount}
Expand Down
7 changes: 7 additions & 0 deletions src/constants/transactionTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we’re creating src/constants/transactionTypes.js when there are some other places to use this but you only uses this in one file.So, we don't need this constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use it in three files:

src/components/transactions/amount.js
src/store/middlewares/delegateRegistration.js
src/store/middlewares/delegateRegistration.test.js

send: 0,
setSecondPassphrase: 1,
registerDelegate: 2,
vote: 3,
};

40 changes: 29 additions & 11 deletions src/store/middlewares/account.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
import { getAccountStatus, getAccount, transactions } from '../../utils/api/account';
import { accountUpdated } from '../../actions/account';
import { accountUpdated, accountLoggedIn } from '../../actions/account';
import { transactionsUpdated } from '../../actions/transactions';
import { activePeerUpdate } from '../../actions/peers';
import actionTypes from '../../constants/actions';
import { fetchAndUpdateForgedBlocks } from '../../actions/forging';
import { getDelegate } from '../../utils/api/delegate';
import transactionTypes from '../../constants/transactionTypes';

const updateAccountData = next => (store) => { // eslint-disable-line
const updateAccountData = (store) => { // eslint-disable-line
const { peers, account } = store.getState();

getAccount(peers.data, account.address).then((result) => {
if (result.balance !== account.balance) {
const maxBlockSize = 25;
transactions(peers.data, account.address, maxBlockSize)
.then(response => next(transactionsUpdated({
confirmed: response.transactions,
count: parseInt(response.count, 10),
})));
.then(response => store.dispatch(transactionsUpdated({
confirmed: response.transactions,
count: parseInt(response.count, 10),
})));
if (account.isDelegate) {
store.dispatch(fetchAndUpdateForgedBlocks({
activePeer: peers.data,
Expand All @@ -25,22 +27,38 @@ const updateAccountData = next => (store) => { // eslint-disable-line
}));
}
}
next(accountUpdated(result));
store.dispatch(accountUpdated(result));
});

return getAccountStatus(peers.data).then(() => {
next(activePeerUpdate({ online: true }));
store.dispatch(activePeerUpdate({ online: true }));
}).catch((res) => {
next(activePeerUpdate({ online: false, code: res.error.code }));
store.dispatch(activePeerUpdate({ online: false, code: res.error.code }));
});
};

const delegateRegistration = (store, action) => {
const delegateRegistrationTx = action.data.confirmed.filter(
transaction => transaction.type === transactionTypes.registerDelegate)[0];
const state = store.getState();

if (delegateRegistrationTx) {
getDelegate(state.peers.data, state.account.publicKey)
.then((delegateData) => {
store.dispatch(accountLoggedIn(Object.assign({},
{ delegate: delegateData.delegate, isDelegate: true })));
});
}
};

const accountMiddleware = store => next => (action) => {
next(action);
const update = updateAccountData(next);
switch (action.type) {
case actionTypes.metronomeBeat:
update(store);
updateAccountData(store);
break;
case actionTypes.transactionsUpdated:
delegateRegistration(store, action);
break;
default: break;
}
Expand Down
34 changes: 30 additions & 4 deletions src/store/middlewares/account.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,22 @@ import { expect } from 'chai';
import { spy, stub } from 'sinon';
import middleware from './account';
import * as accountApi from '../../utils/api/account';
import * as delegateApi from '../../utils/api/delegate';
import actionTypes from '../../constants/actions';
// import * as forgingActions from '../../actions/forging';
import transactionTypes from '../../constants/transactionTypes';

describe('Account middleware', () => {
let store;
let next;
let state;
const transactionsUpdatedAction = {
type: actionTypes.transactionsUpdated,
data: {
confirmed: [{
type: transactionTypes.registerDelegate,
}],
},
};

beforeEach(() => {
store = stub();
Expand All @@ -21,11 +30,11 @@ describe('Account middleware', () => {
balance: 0,
},
};
store.getState = () => (state);
next = spy();
});

it('should passes the action to next middleware', () => {
store.getState = () => (state);
const expectedAction = {
type: 'TEST_ACTION',
};
Expand All @@ -35,7 +44,6 @@ describe('Account middleware', () => {
});

it(`should call account API methods on ${actionTypes.metronomeBeat} action`, () => {
store.getState = () => (state);
const stubGetAccount = stub(accountApi, 'getAccount').resolves({ balance: 0 });
const stubGetAccountStatus = stub(accountApi, 'getAccountStatus').resolves(true);

Expand All @@ -49,7 +57,6 @@ describe('Account middleware', () => {
});

it(`should call transactions API methods on ${actionTypes.metronomeBeat} action if account.balance changes`, () => {
store.getState = () => (state);
const stubGetAccount = stub(accountApi, 'getAccount').resolves({ balance: 10e8 });
const stubTransactions = stub(accountApi, 'transactions').resolves(true);

Expand Down Expand Up @@ -78,5 +85,24 @@ describe('Account middleware', () => {
stubGetAccount.restore();
stubGetAccountStatus.restore();
});

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();

delegateApiMock.restore();
});

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();
});
});