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

Display warning for passphrase format - Closes #337 #644

Merged
merged 4 commits into from
Nov 9, 2018

Conversation

mitsuaki-u
Copy link
Contributor

What was the problem?

No warning when user inputs invalid mnemonic for passphrase.

How did I fix it?

Modified getInputsFromSources to fetch warnings from elements.

How to test it?

Enter invalid mnemonic passphrase for any command that requires passphrase input.

Review checklist

error => error.code !== 'INVALID_MNEMONIC',
);
uniquePassphraseErrors.forEach(error =>
console.info(`Warning: ${error.message}`),
Copy link
Contributor

Choose a reason for hiding this comment

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

prefer not using the console
Validate right before executing the command?

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean return back the errors from the utils and handle it with this.print in each command?

@@ -13,6 +13,7 @@
* Removal or modification of this copyright notice is prohibited.
*
*/
import * as elements from 'lisk-elements';
Copy link
Contributor

Choose a reason for hiding this comment

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

i think u can destruct here or not use *

shuse2
shuse2 previously requested changes Nov 8, 2018
const errors = passphraseModule.validation
.getPassphraseValidationErrors(input)
.filter(error => error.message);
if (accumulator.length === 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this length check?

passphrase: stdin,
});
await getInputsFromSources(inputs);
expect(console.warn.getCall(0).args[0].trim()).to.equal(
Copy link
Contributor

Choose a reason for hiding this comment

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

calledWith doesn't work here?

shuse2
shuse2 previously requested changes Nov 9, 2018
test/utils/input/index.js Outdated Show resolved Hide resolved
test/utils/input/index.js Outdated Show resolved Hide resolved
@shuse2 shuse2 merged commit 9efb897 into development Nov 9, 2018
@shuse2 shuse2 deleted the 337-passphrase_validation branch November 9, 2018 12:59
@shuse2 shuse2 removed this from the Version 2.1.0 milestone Nov 28, 2018
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.

2 participants