Merge pull request #4667 from MetaMask/i#3896

transactions - remove rejected transactions from history

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Current Master
 
+  - Remove rejected transactions from transaction history
+
 ## 4.8.0 Thur Jun 14 2018
 
 - [#4513](https://github.com/MetaMask/metamask-extension/pull/4513): Attempting to import an empty private key will now show a clear error.

app/scripts/controllers/transactions/tx-state-manager.js

@@ -288,6 +288,7 @@ class TransactionStateManager extends EventEmitter {
   */
   setTxStatusRejected (txId) {
     this._setTxStatus(txId, 'rejected')
+    this._removeTx(txId)
   }
 
   /**
@@ -422,6 +423,11 @@ class TransactionStateManager extends EventEmitter {
   _saveTxList (transactions) {
     this.store.updateState({ transactions })
   }
+
+  _removeTx (txId) {
+    const transactionList = this.getFullTxList()
+    this._saveTxList(transactionList.filter((txMeta) => txMeta.id !== txId))
+  }
 }
 
 module.exports = TransactionStateManager

app/scripts/migrations/027.js

@@ -0,0 +1,35 @@
+// next version number
+const version = 27
+
+/*
+
+normalizes txParams on unconfirmed txs
+
+*/
+const clone = require('clone')
+
+module.exports = {
+  version,
+
+  migrate: async function (originalVersionedData) {
+    const versionedData = clone(originalVersionedData)
+    versionedData.meta.version = version
+    const state = versionedData.data
+    const newState = transformState(state)
+    versionedData.data = newState
+    return versionedData
+  },
+}
+
+function transformState (state) {
+  const newState = state
+
+  if (newState.TransactionController) {
+    if (newState.TransactionController.transactions) {
+      const transactions = newState.TransactionController.transactions
+      newState.TransactionController.transactions = transactions.filter((txMeta) => txMeta.status !== 'rejected')
+    }
+  }
+
+  return newState
+}

test/unit/app/controllers/transactions/tx-controller-test.js

@@ -353,9 +353,16 @@ describe('Transaction Controller', function () {
       ])
     })
 
-    it('should set the transaction to rejected from unapproved', async function () {
+    it('should emit a status change to rejected', function (done) {
-      await txController.cancelTransaction(0)
+      txController.once('tx:status-update', (txId, status) => {
-      assert.equal(txController.txStateManager.getTx(0).status, 'rejected')
+        try {
+          assert.equal(status, 'rejected', 'status should e rejected')
+          assert.equal(txId, 0, 'id should e 0')
+          done()
+        } catch (e) { done(e) }
+      })
+
+      txController.cancelTransaction(0)
     })
 
   })

test/unit/app/controllers/transactions/tx-state-manager-test.js

@@ -43,14 +43,13 @@ describe('TransactionStateManager', function () {
   })
 
   describe('#setTxStatusRejected', function () {
-    it('sets the tx status to rejected', function () {
+     it('sets the tx status to rejected and removes it from history', function () {
       const tx = { id: 1, status: 'unapproved', metamaskNetworkId: currentNetworkId, txParams: {} }
       txStateManager.addTx(tx)
       txStateManager.setTxStatusRejected(1)
       const result = txStateManager.getTxList()
       assert.ok(Array.isArray(result))
-      assert.equal(result.length, 1)
+      assert.equal(result.length, 0)
-      assert.equal(result[0].status, 'rejected')
     })
 
     it('should emit a rejected event to signal the exciton of callback', (done) => {
@@ -287,4 +286,18 @@ describe('TransactionStateManager', function () {
 
     })
   })
+
+  describe('#_removeTx', function () {
+    it('should remove the transaction from the storage', () => {
+      txStateManager._saveTxList([ {id: 1} ])
+      txStateManager._removeTx(1)
+      assert(!txStateManager.getFullTxList().length, 'txList should be empty')
+    })
+
+    it('should only remove the transaction with ID 1 from the storage', () => {
+      txStateManager._saveTxList([ {id: 1}, {id: 2} ])
+      txStateManager._removeTx(1)
+      assert.equal(txStateManager.getFullTxList()[0].id, 2, 'txList should have a id of 2')
+    })
+  })
 })

test/unit/migrations/027-test.js

@@ -0,0 +1,50 @@
+const assert = require('assert')
+const migration27 = require('../../../app/scripts/migrations/027')
+
+const oldStorage = {
+  'meta': {},
+  'data': {
+    'TransactionController': {
+      'transactions': [
+      ],
+    },
+  },
+}
+
+const transactions = []
+
+
+while (transactions.length < 9) {
+  transactions.push({status: 'rejected'})
+  transactions.push({status: 'unapproved'})
+  transactions.push({status: 'approved'})
+}
+
+
+oldStorage.data.TransactionController.transactions = transactions
+
+describe('migration #27', () => {
+  it('should remove rejected transactions', (done) => {
+    migration27.migrate(oldStorage)
+      .then((newStorage) => {
+        const newTransactions = newStorage.data.TransactionController.transactions
+        assert.equal(newTransactions.length, 6, 'transactions is expected to have the length of 6')
+        newTransactions.forEach((txMeta) => {
+          if (txMeta.status === 'rejected') done(new Error('transaction was found with a status of rejected'))
+        })
+        done()
+      })
+      .catch(done)
+  })
+
+  it('should successfully migrate first time state', (done) => {
+    migration27.migrate({
+      meta: {},
+      data: require('../../../app/scripts/first-time-state'),
+    })
+      .then((migratedData) => {
+        assert.equal(migratedData.meta.version, migration27.version)
+        done()
+      }).catch(done)
+  })
+})