Skip to content
This repository has been archived by the owner on May 24, 2022. It is now read-only.

fix: Relates to #138. Prevents user from importing account that is already in account list #327

Merged
Merged
Expand Up @@ -5,8 +5,13 @@

import React, { Component } from 'react';
import { Card, Form as FetherForm } from 'fether-ui';
import { accountsInfo$, withoutLoading } from '@parity/light.js';
import light from '@parity/light.js-react';
import { inject, observer } from 'mobx-react';

@light({
accountsInfo: () => accountsInfo$().pipe(withoutLoading())
Copy link
Collaborator

Choose a reason for hiding this comment

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

})
@inject('createAccountStore')
@observer
class AccountImportOptions extends Component {
Expand All @@ -32,19 +37,29 @@ class AccountImportOptions extends Component {
handleSubmitPhrase = async () => {
const phrase = this.state.phrase.trim();
const {
createAccountStore,
createAccountStore: { setPhrase }
} = this.props;

this.setState({ isLoading: true, phrase });

try {
await setPhrase(phrase);

const addressForPhrase = createAccountStore.address.toLowerCase();

if (this.hasExistingAddressForImport(addressForPhrase)) {
return;
}

this.handleNextStep();
} catch (e) {
} catch (error) {
this.setState({
isLoading: false,
error:
'The passphrase was not recognized. Please verify that you entered your passphrase correctly.'
});
console.error(error);
}
};

Expand All @@ -54,7 +69,15 @@ class AccountImportOptions extends Component {
} = this.props;

this.setState({ isLoading: true });

try {
const json = JSON.parse(jsonString);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not parse here.

3 lines below, the await setJsonString(jsonString); actually parses the JSON, and createAccountStore.address will hold the parsed address. Do the hasExistingAddressForImport then.

const jsonAddress = `0x${json['address'].toLowerCase()}`;

if (this.hasExistingAddressForImport(jsonAddress)) {
return;
}

await setJsonString(jsonString);
this.handleNextStep();
} catch (error) {
Expand All @@ -67,6 +90,22 @@ class AccountImportOptions extends Component {
}
};

hasExistingAddressForImport = addressForImport => {
const { accountsInfo } = this.props;
const isExistingAddress = Object.keys(accountsInfo)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use account$, no need for Object.keys anymore since it returns a string[]

.map(address => address && address.toLowerCase())
.includes(addressForImport);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would put .toLowecase() on this line. So that the input argument can be in any case.


if (isExistingAddress) {
this.setState({
isLoading: false,
error: `Account already loaded. Address ${addressForImport} is already in the account list`
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would go for something much shorter along the line:
"Account 0x1234..1234 already listed!"
You don't have to see the whole address here.

});
}

return isExistingAddress;
};

render () {
const {
history,
Expand Down