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

Unify "not enough funds" errors - Closes #405 #411

Merged
merged 4 commits into from
Jun 14, 2017

Conversation

slaweet
Copy link
Contributor

@slaweet slaweet commented Jun 14, 2017

No description provided.

@slaweet slaweet self-assigned this Jun 14, 2017
@slaweet slaweet requested a review from reyraa June 14, 2017 11:11
@slaweet slaweet force-pushed the 405-not-enough-funds-errors branch 2 times, most recently from 7ac7c87 to da5e7ae Compare June 14, 2017 11:48
@slaweet slaweet force-pushed the 405-not-enough-funds-errors branch from da5e7ae to ee3d27f Compare June 14, 2017 12:08
Copy link
Contributor

@reyraa reyraa left a comment

Choose a reason for hiding this comment

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

Thanks.
Overall looks good. just a few changes requested.

this.account = Account;
const notEnoughtLSK = lsk.normalize(this.account.get().balance) < this.fee;

this.text = notEnoughtLSK ?
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the typo. Also fix it in L23.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I would suggest insufficientFunds.

* @module app
* @submodule notEnoughBalance
*/
app.filter('notEnoughBalance', (Account, lsk) => amount => lsk.normalize(Account.get().balance) < amount);
Copy link
Contributor

Choose a reason for hiding this comment

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

And this name can be FundsSufficiency as in Filter by funds sufficiency.

@slaweet slaweet merged commit 5663feb into development Jun 14, 2017
@slaweet slaweet deleted the 405-not-enough-funds-errors branch June 14, 2017 14:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants