[accounts] Password recovery - issue 480 #493

hunght · 2018-08-31T01:42:42Z

Changes

implemented reset password and reset pass code
wrote the test for new functions

Verification

Go to account then press Forgot password button
Enter mnemonic to verify.
Enter new password
Navigate user to login screen after reset pass success

seland

It works fine for me on iOS.
General note - naming validMnemonicWithAccount is not a verb and therefore should not be used for an action or function. Please change it to validateMnemonicWithAccount or like that.

seland · 2018-08-31T01:45:32Z

src/actions/accounts.js

@@ -95,6 +97,21 @@ export function login(accountId: string, password: string, deferred: boolean = f
  };
 }

+/**
+ * @desc Action creator for an action that is called to perform a login.


Docs are inconsistent with the method name, please fix one of them

seland · 2018-08-31T01:45:59Z

src/actions/accounts.js

+ * @param {string} accountId Id of account to login to.
+ * @param {function} callback Callback that is called with true if check is successful and false otherwise.
+ * @param {boolean} deferred Flag if login should wait for performDeferredLogin action to proceed.
+ * @return {LoginAction} An action.


Returned type is inconsistent with one on method declaration, please fix.

seland · 2018-08-31T01:46:49Z

src/global/translations/en.json

@@ -539,7 +539,8 @@
    "password": {
      "enterInstruction": "Enter password:",
      "createInstruction": "Enter new password:",
-      "verifyInstruction": "Enter password again:"
+      "verifyInstruction": "Enter password again:",
+      "forgetInstruction": "Forget Password?"


I guess it should be "Forgot password?" in past tense.

seland · 2018-08-31T01:50:05Z

src/screens/Passcode/Password/styles.js

@@ -22,6 +22,10 @@ const styles = MediaQueryStyleSheet.create({
    marginRight: 30,
    textAlign: 'center',
  },
+  forgetButton: {


Please move it to global styles as it repeats itself several times.

seland · 2018-08-31T01:51:15Z

src/screens/Passcode/PinCode/styles.js

@@ -22,6 +22,10 @@ const styles = MediaQueryStyleSheet.create({
    marginRight: 30,
    textAlign: 'center',
  },
+  forgetButton: {


Please move it to global styles as it repeats itself several times.

seland · 2018-08-31T01:53:13Z

src/screens/Key/Enter/index.js

@@ -179,7 +187,17 @@ class EnterKeyScreen extends NavigatorComponent<Actions & KeyState & Props, Stat
  }

  onDonePressed = () => {
-    this.props.validateMnemonic();
+    if (_.has(this.props, 'processLogin')) {


processLogin is not declared on Props type, please declare it there. Also, please use check like this.props.processLogin === true instead of using has.

seland · 2018-08-31T02:10:04Z

src/screens/Passcode/EnterPasscodeContainer.js

    } = this.props;
+    const shouldShowForget = !isLoggedIn && has(this.props, 'accountId');


Please use this.props.accountId != null or similar instead of has.

src/screens/Accounts/index.js

seland · 2018-08-31T02:13:43Z

src/services/accounts/index.js

+    }
+    const config = JSON.stringify({
+      encrypted_key_manager: accountStore,
+      // signed_profile: signedProfile,


Please remove the comment if it's not needed

seland · 2018-08-31T11:25:24Z

__tests__/src/sagas/accounts/index.js

@@ -21,6 +22,7 @@ import {
  CHECK_PASSWORD,
  CHECK_PIN_CODE,
  LOGIN,
+  VALID_MNEMONIC_WITH_ACCOUNT,


I guess it's better to update this naming to VALIDATE as well

seland · 2018-08-31T11:27:20Z

src/screens/Accounts/index.js

+        onPress: async () => {
+          if (Platform.OS === 'ios') {
+            await navigator.dismissModal();
+            navigator.pop();


navigator.pop() line is the same for both platforms, so can be moved outside of the if statement.

seland · 2018-08-31T11:27:36Z

src/screens/Accounts/index.js

+  static onForgetPasswordAccount = async (navigator: Navigator, currentAccountId: string) => {
+    if (Platform.OS === 'ios') {
+      await navigator.dismissModal();
+      navigator.push({


navigator.push(...) line is the same for both platforms, so can be moved outside of the if statement.

seland · 2018-08-31T11:29:01Z

src/screens/Passcode/EnterPasscodeContainer.js

    } = this.props;
+    const shouldShowForget = !isLoggedIn && this.props.accountId != null;


Please use isLoggedIn === false instead of !isLoggedIn to prevent coercion.

seland · 2018-08-31T11:30:04Z

src/screens/Key/Enter/index.js

@@ -179,7 +199,17 @@ class EnterKeyScreen extends NavigatorComponent<Actions & KeyState & Props, Stat
  }

  onDonePressed = () => {
-    this.props.validateMnemonic();
+    if (this.props.isOnResetPassProcess) {


Please use this.props.isOnResetPassProcess === true to prevent coercion.

seland

Nice work

albertoestarrona

Good job!

…overy [accounts] Password recovery - issue 480 Former-commit-id: af54da6

…overy [accounts] Password recovery - issue 480 Former-commit-id: 5de64c6 [formerly af54da6] Former-commit-id: 417a568

[accounts] implemented reset password and pass code

e4193ce

hunght requested review from seland and albertoestarrona August 31, 2018 01:42

hunght and others added 2 commits August 31, 2018 08:50

Merge branch 'develop' into feature/accounts-password-recovery

68cfa0c

[ui] Fixed password recovery alerts.

b3fa466

seland suggested changes Aug 31, 2018

View reviewed changes

seland and others added 2 commits August 31, 2018 05:17

[lint] Fixed issue.

b251bb8

[accounts] rename functions, added show alert when reset success

95efb68

seland reviewed Aug 31, 2018

View reviewed changes

[accounts] fixed coercion, refactored code

73e0ea3

seland approved these changes Aug 31, 2018

View reviewed changes

seland assigned hunght Aug 31, 2018

seland added this to the 1.2.0 milestone Aug 31, 2018

albertoestarrona approved these changes Aug 31, 2018

View reviewed changes

seland merged commit af54da6 into develop Aug 31, 2018

seland mentioned this pull request Aug 31, 2018

[accounts] Password recovery #480

Closed

7 tasks

marknuzz pushed a commit that referenced this pull request Nov 14, 2018

Merge pull request #493 from Bit-Nation/feature/accounts-password-rec…

417a568

…overy [accounts] Password recovery - issue 480 Former-commit-id: af54da6

marknuzz pushed a commit that referenced this pull request Nov 14, 2018

Merge pull request #493 from Bit-Nation/feature/accounts-password-rec…

42153ee

…overy [accounts] Password recovery - issue 480 Former-commit-id: 5de64c6 [formerly af54da6] Former-commit-id: 417a568

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[accounts] Password recovery - issue 480 #493

[accounts] Password recovery - issue 480 #493

hunght commented Aug 31, 2018

seland left a comment

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland Aug 31, 2018

hunght Aug 31, 2018

seland left a comment

albertoestarrona left a comment

		} = this.props;
		const shouldShowForget = !isLoggedIn && has(this.props, 'accountId');

[accounts] Password recovery - issue 480 #493

[accounts] Password recovery - issue 480 #493

Conversation

hunght commented Aug 31, 2018

Changes

Verification

seland left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seland left a comment

Choose a reason for hiding this comment

albertoestarrona left a comment

Choose a reason for hiding this comment