Refactor Generate Transaction (#960)

* Make generic modal

* Allow generic modals to be injected into send button component

* Refactor generate transaction, cleanup transaction sagas, simplify signing process

* Get passing unit tests

* Make previous test more comprehensive

* Fix ts errors

common/actions/transaction/actionCreators/sign.ts

@@ -1,9 +1,8 @@
 import {
   SignTransactionFailedAction,
-  SignLocalTransactionRequestedAction,
-  SignWeb3TransactionRequestedAction,
   SignLocalTransactionSucceededAction,
-  SignWeb3TransactionSucceededAction
+  SignWeb3TransactionSucceededAction,
+  SignTransactionRequestedAction
 } from '../actionTypes';
 import { TypeKeys } from '../constants';
 
@@ -12,6 +11,12 @@ const signTransactionFailed = (): SignTransactionFailedAction => ({
   type: TypeKeys.SIGN_TRANSACTION_FAILED
 });
 
+type TSignTransactionRequested = typeof signTransactionRequested;
+const signTransactionRequested = (payload: SignTransactionRequestedAction['payload']) => ({
+  type: TypeKeys.SIGN_TRANSACTION_REQUESTED,
+  payload
+});
+
 type TSignLocalTransactionSucceeded = typeof signLocalTransactionSucceeded;
 const signLocalTransactionSucceeded = (
   payload: SignLocalTransactionSucceededAction['payload']
@@ -20,14 +25,6 @@ const signLocalTransactionSucceeded = (
   payload
 });
 
-type TSignLocalTransactionRequested = typeof signLocalTransactionRequested;
-const signLocalTransactionRequested = (
-  payload: SignLocalTransactionRequestedAction['payload']
-): SignLocalTransactionRequestedAction => ({
-  type: TypeKeys.SIGN_LOCAL_TRANSACTION_REQUESTED,
-  payload
-});
-
 type TSignWeb3TransactionSucceeded = typeof signWeb3TransactionSucceeded;
 const signWeb3TransactionSucceeded = (
   payload: SignWeb3TransactionSucceededAction['payload']
@@ -36,23 +33,13 @@ const signWeb3TransactionSucceeded = (
   payload
 });
 
-type TSignWeb3TransactionRequested = typeof signWeb3TransactionRequested;
-const signWeb3TransactionRequested = (
-  payload: SignWeb3TransactionRequestedAction['payload']
-): SignWeb3TransactionRequestedAction => ({
-  type: TypeKeys.SIGN_WEB3_TRANSACTION_REQUESTED,
-  payload
-});
-
 export {
+  signTransactionRequested,
   signTransactionFailed,
   signLocalTransactionSucceeded,
-  signLocalTransactionRequested,
   signWeb3TransactionSucceeded,
-  signWeb3TransactionRequested,
   TSignLocalTransactionSucceeded,
-  TSignLocalTransactionRequested,
   TSignWeb3TransactionSucceeded,
-  TSignWeb3TransactionRequested,
-  TSignTransactionFailed
+  TSignTransactionFailed,
+  TSignTransactionRequested
 };

common/actions/transaction/actionTypes/sign.ts

@@ -8,8 +8,8 @@ import { TypeKeys } from 'actions/transaction/constants';
  */
 
 /* Signing / Async actions */
-interface SignLocalTransactionRequestedAction {
-  type: TypeKeys.SIGN_LOCAL_TRANSACTION_REQUESTED;
+interface SignTransactionRequestedAction {
+  type: TypeKeys.SIGN_TRANSACTION_REQUESTED;
   payload: EthTx;
 }
 interface SignLocalTransactionSucceededAction {
@@ -17,10 +17,6 @@ interface SignLocalTransactionSucceededAction {
   payload: { signedTransaction: Buffer; indexingHash: string; noVerify?: boolean }; // dont verify against fields, for pushTx
 }
 
-interface SignWeb3TransactionRequestedAction {
-  type: TypeKeys.SIGN_WEB3_TRANSACTION_REQUESTED;
-  payload: EthTx;
-}
 interface SignWeb3TransactionSucceededAction {
   type: TypeKeys.SIGN_WEB3_TRANSACTION_SUCCEEDED;
   payload: { transaction: Buffer; indexingHash: string; noVerify?: boolean };
@@ -30,16 +26,14 @@ interface SignTransactionFailedAction {
 }
 
 type SignAction =
-  | SignLocalTransactionRequestedAction
+  | SignTransactionRequestedAction
   | SignLocalTransactionSucceededAction
-  | SignWeb3TransactionRequestedAction
   | SignWeb3TransactionSucceededAction
   | SignTransactionFailedAction;
 
 export {
-  SignLocalTransactionRequestedAction,
+  SignTransactionRequestedAction,
   SignLocalTransactionSucceededAction,
-  SignWeb3TransactionRequestedAction,
   SignWeb3TransactionSucceededAction,
   SignTransactionFailedAction,
   SignAction

common/actions/transaction/constants.ts

@@ -12,9 +12,8 @@ export enum TypeKeys {
   GET_NONCE_SUCCEEDED = 'GET_NONCE_SUCCEEDED',
   GET_NONCE_FAILED = 'GET_NONCE_FAILED',
 
-  SIGN_WEB3_TRANSACTION_REQUESTED = 'SIGN_WEB3_TRANSACTION_REQUESTED',
+  SIGN_TRANSACTION_REQUESTED = 'SIGN_TRANSACTION_REQUESTED',
   SIGN_WEB3_TRANSACTION_SUCCEEDED = 'SIGN_WEB3_TRANSACTION_SUCCEEDED',
-  SIGN_LOCAL_TRANSACTION_REQUESTED = 'SIGN_LOCAL_TRANSACTION_REQUESTED',
   SIGN_LOCAL_TRANSACTION_SUCCEEDED = 'SIGN_LOCAL_TRANSACTION_SUCCEEDED',
   SIGN_TRANSACTION_FAILED = 'SIGN_TRANSACTION_FAILED',
 

common/components/GenerateTransaction.tsx

@@ -0,0 +1,13 @@
+import { GenerateTransactionFactory } from './GenerateTransactionFactory';
+import React from 'react';
+import translate from 'translations';
+
+export const GenerateTransaction: React.SFC<{}> = () => (
+  <GenerateTransactionFactory
+    withProps={({ disabled, isWeb3Wallet, onClick }) => (
+      <button disabled={disabled} className="btn btn-info btn-block" onClick={onClick}>
+        {isWeb3Wallet ? translate('Send to MetaMask / Mist') : translate('DEP_signtx')}
+      </button>
+    )}
+  />
+);

common/components/GenerateTransaction/Container.tsx

@@ -1,43 +0,0 @@
-import {
-  signLocalTransactionRequested,
-  signWeb3TransactionRequested,
-  TSignLocalTransactionRequested,
-  TSignWeb3TransactionRequested,
-  SignLocalTransactionRequestedAction,
-  SignWeb3TransactionRequestedAction
-} from 'actions/transaction';
-import React, { Component } from 'react';
-import { connect, Dispatch } from 'react-redux';
-import { bindActionCreators } from 'redux';
-import { AppState } from 'reducers';
-type Payload =
-  | SignLocalTransactionRequestedAction['payload']
-  | SignWeb3TransactionRequestedAction['payload'];
-type Signer = (
-  payload: Payload
-) => () => SignLocalTransactionRequestedAction | SignWeb3TransactionRequestedAction;
-
-interface DispatchProps {
-  signer: TSignLocalTransactionRequested | TSignWeb3TransactionRequested;
-}
-interface Props {
-  isWeb3: boolean;
-  withSigner(signer: Signer): React.ReactElement<any> | null;
-}
-
-class Container extends Component<DispatchProps & Props, {}> {
-  public render() {
-    return this.props.withSigner(this.sign);
-  }
-
-  private sign = (payload: Payload) => () => this.props.signer(payload);
-}
-
-export const WithSigner = connect(null, (dispatch: Dispatch<AppState>, ownProps: Props) => {
-  return bindActionCreators(
-    {
-      signer: ownProps.isWeb3 ? signWeb3TransactionRequested : signLocalTransactionRequested
-    },
-    dispatch
-  );
-})(Container);

common/components/GenerateTransaction/index.ts

@@ -1 +0,0 @@
-export * from './GenerateTransaction';

common/components/GenerateTransactionFactory/Container.tsx

@@ -0,0 +1,20 @@
+import { signTransactionRequested, TSignTransactionRequested } from 'actions/transaction';
+import React, { Component } from 'react';
+import { connect } from 'react-redux';
+
+interface DispatchProps {
+  signTransactionRequested: TSignTransactionRequested;
+}
+
+interface OwnProps {
+  isWeb3: boolean;
+  withSigner(signer: TSignTransactionRequested): React.ReactElement<any> | null;
+}
+
+class Container extends Component<DispatchProps & OwnProps, {}> {
+  public render() {
+    return this.props.withSigner(this.props.signTransactionRequested);
+  }
+}
+
+export const WithSigner = connect(null, { signTransactionRequested })(Container);

common/components/GenerateTransactionFactory/GenerateTransactionFactory.tsx

@@ -1,4 +1,3 @@
-import translate from 'translations';
 import { WithSigner } from './Container';
 import EthTx from 'ethereumjs-tx';
 import React, { Component } from 'react';
@@ -12,6 +11,12 @@ import {
 } from 'selectors/transaction';
 import { getWalletType } from 'selectors/wallet';
 
+export interface CallbackProps {
+  disabled: boolean;
+  isWeb3Wallet: boolean;
+  onClick(): void;
+}
+
 interface StateProps {
   transaction: EthTx;
   networkRequestPending: boolean;
@@ -21,16 +26,21 @@ interface StateProps {
   validGasLimit: boolean;
 }
 
-class GenerateTransactionClass extends Component<StateProps> {
+interface OwnProps {
+  withProps(props: CallbackProps): React.ReactElement<any> | null;
+}
+
+type Props = OwnProps & StateProps;
+
+class GenerateTransactionFactoryClass extends Component<Props> {
   public render() {
     const {
       isFullTransaction,
       isWeb3Wallet,
-      transaction,
       networkRequestPending,
-
       validGasPrice,
-      validGasLimit
+      validGasLimit,
+      transaction
     } = this.props;
 
     const isButtonDisabled =
@@ -38,24 +48,22 @@ class GenerateTransactionClass extends Component<StateProps> {
     return (
       <WithSigner
         isWeb3={isWeb3Wallet}
-        withSigner={signer => (
-          <button
-            disabled={isButtonDisabled}
-            className="btn btn-info btn-block"
-            onClick={signer(transaction)}
-          >
-            {isWeb3Wallet ? translate('Send to MetaMask / Mist') : translate('DEP_signtx')}
-          </button>
-        )}
+        withSigner={signer =>
+          this.props.withProps({
+            disabled: isButtonDisabled,
+            isWeb3Wallet,
+            onClick: () => signer(transaction)
+          })
+        }
       />
     );
   }
 }
 
-export const GenerateTransaction = connect((state: AppState) => ({
+export const GenerateTransactionFactory = connect((state: AppState) => ({
   ...getTransaction(state),
   networkRequestPending: isNetworkRequestPending(state),
   isWeb3Wallet: getWalletType(state).isWeb3Wallet,
   validGasPrice: isValidGasPrice(state),
   validGasLimit: isValidGasLimit(state)
-}))(GenerateTransactionClass);
+}))(GenerateTransactionFactoryClass);

common/components/GenerateTransactionFactory/index.ts

@@ -0,0 +1 @@
+export * from './GenerateTransactionFactory';

common/reducers/transaction/sign/sign.ts

@@ -14,7 +14,7 @@ const INITIAL_STATE: State = {
   pending: false
 };
 
-const signLocalTransactionRequested = (): State => ({
+const signTransactionRequested = (): State => ({
   ...INITIAL_STATE,
   pending: true
 });
@@ -30,7 +30,7 @@ const signLocalTransactionSucceeded = (
   web3: { transaction: null }
 });
 
-const signWeb3TranscationRequested = (
+const signWeb3TranscationSucceeded = (
   _: State,
   { payload }: SignWeb3TransactionSucceededAction
 ): State => ({
@@ -47,12 +47,12 @@ const reset = () => INITIAL_STATE;
 
 export const sign = (state: State = INITIAL_STATE, action: SignAction | ResetAction) => {
   switch (action.type) {
-    case TK.SIGN_LOCAL_TRANSACTION_REQUESTED:
-      return signLocalTransactionRequested();
+    case TK.SIGN_TRANSACTION_REQUESTED:
+      return signTransactionRequested();
     case TK.SIGN_LOCAL_TRANSACTION_SUCCEEDED:
       return signLocalTransactionSucceeded(state, action);
     case TK.SIGN_WEB3_TRANSACTION_SUCCEEDED:
-      return signWeb3TranscationRequested(state, action);
+      return signWeb3TranscationSucceeded(state, action);
     case TK.SIGN_TRANSACTION_FAILED:
       return signTransactionFailed();
     case TK.RESET:

common/sagas/transaction/current/currentValue.ts

@@ -12,11 +12,16 @@ import { SetCurrentValueAction, TypeKeys } from 'actions/transaction';
 import { toTokenBase } from 'libs/units';
 import { validateInput, IInput } from 'sagas/transaction/validationHelpers';
 import { validNumber, validDecimal } from 'libs/validators';
-export function* setCurrentValue({ payload }: SetCurrentValueAction): SagaIterator {
+
+export function* setCurrentValue(action: SetCurrentValueAction): SagaIterator {
   const etherTransaction = yield select(isEtherTransaction);
+  const setter = etherTransaction ? setValueField : setTokenValue;
+  return yield call(valueHandler, action, setter);
+}
+
+export function* valueHandler({ payload }: SetCurrentValueAction, setter) {
   const decimal: number = yield select(getDecimal);
   const unit: string = yield select(getUnit);
-  const setter = etherTransaction ? setValueField : setTokenValue;
 
   if (!validNumber(+payload) || !validDecimal(payload, decimal)) {
     return yield put(setter({ raw: payload, value: null }));
@@ -51,6 +56,7 @@ export function* reparseCurrentValue(value: IInput): SagaIterator {
     return null;
   }
 }
+
 export const currentValue = [
   takeEvery([TypeKeys.CURRENT_VALUE_SET], setCurrentValue),
   takeEvery([TypeKeys.GAS_LIMIT_FIELD_SET, TypeKeys.GAS_PRICE_FIELD_SET], revalidateCurrentValue)

common/sagas/transaction/signing/helpers.ts

@@ -4,12 +4,11 @@ import { getNetworkConfig } from 'selectors/config';
 import { select, call, put, take } from 'redux-saga/effects';
 import {
   signTransactionFailed,
-  SignWeb3TransactionRequestedAction,
-  SignLocalTransactionRequestedAction,
   GetFromFailedAction,
   GetFromSucceededAction,
   getFromRequested,
-  TypeKeys as TK
+  TypeKeys as TK,
+  SignTransactionRequestedAction
 } from 'actions/transaction';
 import Tx from 'ethereumjs-tx';
 import { NetworkConfig } from 'config';
@@ -22,7 +21,7 @@ interface IFullWalletAndTransaction {
 }
 
 const signTransactionWrapper = (func: (IWalletAndTx: IFullWalletAndTransaction) => SagaIterator) =>
-  function*(partialTx: SignLocalTransactionRequestedAction | SignWeb3TransactionRequestedAction) {
+  function*(partialTx: SignTransactionRequestedAction) {
     try {
       const IWalletAndTx: IFullWalletAndTransaction = yield call(
         getWalletAndTransaction,
@@ -40,9 +39,7 @@ const signTransactionWrapper = (func: (IWalletAndTx: IFullWalletAndTransaction)
  * the rest of the tx parameters from the action
  * @param partialTx
  */
-function* getWalletAndTransaction(
-  partialTx: (SignLocalTransactionRequestedAction | SignWeb3TransactionRequestedAction)['payload']
-) {
+function* getWalletAndTransaction(partialTx: SignTransactionRequestedAction['payload']) {
   // get the wallet we're going to sign with
   const wallet: null | IFullWallet = yield select(getWalletInst);
   if (!wallet) {

common/sagas/transaction/signing/signing.ts

@@ -7,11 +7,13 @@ import {
   TypeKeys,
   reset,
   SignWeb3TransactionSucceededAction,
-  SignLocalTransactionSucceededAction
+  SignLocalTransactionSucceededAction,
+  SignTransactionRequestedAction
 } from 'actions/transaction';
 import { computeIndexingHash } from 'libs/transaction';
 import { serializedAndTransactionFieldsMatch } from 'selectors/transaction';
 import { showNotification } from 'actions/notifications';
+import { getWalletType, IWalletType } from 'selectors/wallet';
 
 export function* signLocalTransactionHandler({
   tx,
@@ -64,9 +66,15 @@ function* verifyTransaction({
     yield put(reset());
   }
 }
+
+function* handleTransactionRequest(action: SignTransactionRequestedAction): SagaIterator {
+  const walletType: IWalletType = yield select(getWalletType);
+  const signingHandler = walletType.isWeb3Wallet ? signWeb3Transaction : signLocalTransaction;
+  return yield call(signingHandler, action);
+}
+
 export const signing = [
-  takeEvery(TypeKeys.SIGN_LOCAL_TRANSACTION_REQUESTED, signLocalTransaction),
-  takeEvery(TypeKeys.SIGN_WEB3_TRANSACTION_REQUESTED, signWeb3Transaction),
+  takeEvery(TypeKeys.SIGN_TRANSACTION_REQUESTED, handleTransactionRequest),
   takeEvery(
     [TypeKeys.SIGN_LOCAL_TRANSACTION_SUCCEEDED, TypeKeys.SIGN_WEB3_TRANSACTION_SUCCEEDED],
     verifyTransaction

spec/sagas/transaction/current/currentValue.spec.ts

@@ -6,8 +6,10 @@ import { validateInput } from 'sagas/transaction/validationHelpers';
 import {
   setCurrentValue,
   revalidateCurrentValue,
-  reparseCurrentValue
+  reparseCurrentValue,
+  valueHandler
 } from 'sagas/transaction/current/currentValue';
+import { cloneableGenerator, SagaIteratorClone } from 'redux-saga/utils';
 
 const itShouldBeDone = gen => {
   it('should be done', () => {
@@ -15,77 +17,69 @@ const itShouldBeDone = gen => {
   });
 };
 
-describe('setCurrentValue*', () => {
-  const sharedLogic = (gen, etherTransaction, decimal: number) => {
-    it('should select isEtherTransaction', () => {
-      expect(gen.next().value).toEqual(select(isEtherTransaction));
-    });
+describe('valueHandler', () => {
+  const action: any = { payload: '5.1' };
+  const setter = setValueField;
+  const decimal = 1;
+  const gen: { [key: string]: SagaIteratorClone } = {};
 
-    it('should select getDecimal', () => {
-      expect(gen.next(etherTransaction).value).toEqual(select(getDecimal));
-    });
-    it('should select getUnit', () => {
-      expect(gen.next(decimal).value).toEqual(select(getUnit));
-    });
+  const failCases = {
+    invalidDecimal: 0,
+    invalidNumber: {
+      decimal: 1,
+      action: { payload: 'x' }
+    }
   };
 
-  describe('when invalid number or decimal', () => {
-    const invalidDecimal: any = {
-      payload: '10.01'
-    };
-    const invalidNumber: any = {
-      payload: 'invalidNumber'
-    };
-    const etherTransaction = true;
-    const decimal = 1;
-    const gen1 = setCurrentValue(invalidNumber);
-    const gen2 = setCurrentValue(invalidDecimal);
+  gen.pass = cloneableGenerator(valueHandler)(action, setter);
+  gen.invalidNumber = cloneableGenerator(valueHandler)(
+    failCases.invalidNumber.action as any,
+    setter
+  );
+  const value = toTokenBase(action.payload, decimal);
+  const unit = 'eth';
 
-    sharedLogic(gen2, etherTransaction, decimal);
-    sharedLogic(gen1, etherTransaction, decimal);
+  it('should select getDecimal', () => {
+    expect(gen.pass.next().value).toEqual(select(getDecimal));
+    expect(gen.invalidNumber.next().value).toEqual(select(getDecimal));
+  });
+  it('should select getUnit', () => {
+    gen.invalidDecimal = gen.pass.clone();
 
-    it('should put setter', () => {
-      expect(gen1.next().value).toEqual(
-        put(setValueField({ raw: invalidNumber.payload, value: null }))
-      );
-      expect(gen2.next().value).toEqual(
-        put(setValueField({ raw: invalidDecimal.payload, value: null }))
-      );
-    });
-
-    itShouldBeDone(gen1);
-    itShouldBeDone(gen2);
+    expect(gen.pass.next(decimal).value).toEqual(select(getUnit));
+    expect(gen.invalidNumber.next(decimal).value).toEqual(select(getUnit));
+    expect(gen.invalidDecimal.next(failCases.invalidDecimal).value).toEqual(select(getUnit));
   });
 
-  describe('when valid number and decimal', () => {
-    const payload = '100';
-    const action: any = { payload };
-    const etherTransaction = true;
-    const unit = 'ether';
-    const decimal = 0;
-    const value = toTokenBase(payload, decimal);
-    const isValid = true;
-
-    const gen = setCurrentValue(action);
-
-    sharedLogic(gen, etherTransaction, decimal);
-    it('should call validateInput', () => {
-      expect(gen.next(unit).value).toEqual(call(validateInput, value, unit));
-    });
-
-    it('should put setter', () => {
-      expect(gen.next(isValid).value).toEqual(
-        put(
-          setValueField({
-            raw: payload,
-            value
-          })
-        )
-      );
-    });
-
-    itShouldBeDone(gen);
+  it('should fail on invalid number or decimal and put null as a value', () => {
+    expect(gen.invalidNumber.next(unit).value).toEqual(
+      put(setter({ raw: failCases.invalidNumber.action.payload, value: null }))
+    );
+    expect(gen.invalidDecimal.next(unit).value).toEqual(
+      put(setter({ raw: action.payload, value: null }))
+    );
   });
+
+  it('should call isValid', () => {
+    expect(gen.pass.next(unit).value).toEqual(call(validateInput, value, unit));
+  });
+  it('should put setter', () => {
+    expect(gen.pass.next(true).value).toEqual(put(setter({ raw: action.payload, value })));
+  });
+
+  itShouldBeDone(gen.pass);
+});
+
+describe('setCurrentValue*', () => {
+  const action: any = { payload: '5' };
+  const gen = setCurrentValue(action);
+  it('should select isEtherTransaction', () => {
+    expect(gen.next().value).toEqual(select(isEtherTransaction));
+  });
+  it('should call valueHandler', () => {
+    expect(gen.next(isEtherTransaction).value).toEqual(call(valueHandler, action, setValueField));
+  });
+  itShouldBeDone(gen);
 });
 
 describe('revalidateCurrentValue*', () => {