Feature #336: Confirmation required notification for non-owners fix (#338)

* Refactors grantedSelector with isUserOwner function
Checks if the user is owner of the safe before sending notification

* Adds safeParamAddressFromStateSelector
Refactors notificationsMiddleware with new selector

* Remove old size check

* safe notifications middleware fixes

src/logic/safe/transactions/awaitingTransactions.js

@@ -1,34 +1,41 @@
 // @flow
-import { List } from 'immutable'
+import { Map, List } from 'immutable'
 import type { Transaction } from '~/routes/safe/store/models/transaction'
 
-export const getAwaitingTransactions = (allTransactions: List<Transaction>, userAccount: string): List<Transaction> => {
+export const getAwaitingTransactions = (
+  allTransactions: Map<string, List<Transaction>>,
+  userAccount: string,
+): Map<string, List<Transaction>> => {
   if (!allTransactions) {
-    return List([])
+    return Map({})
   }
 
   const allAwaitingTransactions = allTransactions.map((safeTransactions) => {
-    const nonCancelledTransactions = safeTransactions.filter((transaction: Transaction) => {
-      // If transactions are not executed, but there's a transaction with the same nonce EXECUTED later
-      // it means that the transaction was cancelled (Replaced) and shouldn't get executed
-      if (!transaction.isExecuted) {
-        const replacementTransaction = safeTransactions.findLast(
-          (tx) => tx.isExecuted && tx.nonce === transaction.nonce,
-        )
-        if (replacementTransaction) {
-          // eslint-disable-next-line no-param-reassign
-          transaction = transaction.set('cancelled', true)
+    const nonCancelledTransactions = safeTransactions.filter(
+      (transaction: Transaction) => {
+        // If transactions are not executed, but there's a transaction with the same nonce EXECUTED later
+        // it means that the transaction was cancelled (Replaced) and shouldn't get executed
+        if (!transaction.isExecuted) {
+          const replacementTransaction = safeTransactions.findLast(
+            (tx) => tx.isExecuted && tx.nonce === transaction.nonce,
+          )
+          if (replacementTransaction) {
+            // eslint-disable-next-line no-param-reassign
+            transaction = transaction.set('cancelled', true)
+          }
         }
-      }
-      // The transaction is not executed and is not cancelled, so it's still waiting confirmations
-      if (!transaction.executionTxHash && !transaction.cancelled) {
-        // Then we check if the waiting confirmations are not from the current user, otherwise, filters this transaction
-        const transactionWaitingUser = transaction.confirmations.filter((confirmation) => confirmation.owner && confirmation.owner.address !== userAccount)
+        // The transaction is not executed and is not cancelled, so it's still waiting confirmations
+        if (!transaction.executionTxHash && !transaction.cancelled) {
+          // Then we check if the waiting confirmations are not from the current user, otherwise, filters this transaction
+          const transactionWaitingUser = transaction.confirmations.filter(
+            (confirmation) => confirmation.owner && confirmation.owner.address !== userAccount,
+          )
 
-        return transactionWaitingUser.size > 0
-      }
-      return false
-    })
+          return transactionWaitingUser.size > 0
+        }
+        return false
+      },
+    )
     return nonCancelledTransactions
   })
 

src/logic/wallets/ethAddresses.js

@@ -1,4 +1,8 @@
 // @flow
+import { List } from 'immutable'
+import type { Safe } from '~/routes/safe/store/models/safe'
+import type { Owner } from '~/routes/safe/store/models/owner'
+
 export const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'
 
 export const sameAddress = (firstAddress: string, secondAddress: string): boolean => {
@@ -25,3 +29,20 @@ export const shortVersionOf = (value: string, cut: number) => {
 
   return `${value.substring(0, cut)}...${value.substring(final)}`
 }
+
+export const isUserOwner = (safe: Safe, userAccount: string): boolean => {
+  if (!safe) {
+    return false
+  }
+
+  if (!userAccount) {
+    return false
+  }
+
+  const { owners }: List<Owner> = safe
+  if (!owners) {
+    return false
+  }
+
+  return owners.find((owner: Owner) => sameAddress(owner.address, userAccount)) !== undefined
+}

src/routes/safe/components/Transactions/TxsTable/columns.js

@@ -90,15 +90,13 @@ const getTransactionTableData = (tx: Transaction): TransactionRow => {
   }
 }
 
-export const getTxTableData = (transactions: List<Transaction | IncomingTransaction>): List<TransactionRow> => {
-  return transactions.map((tx) => {
-    if (tx.type === INCOMING_TX_TYPE) {
-      return getIncomingTxTableData(tx)
-    }
+export const getTxTableData = (transactions: List<Transaction | IncomingTransaction>): List<TransactionRow> => transactions.map((tx) => {
+  if (tx.type === INCOMING_TX_TYPE) {
+    return getIncomingTxTableData(tx)
+  }
 
-    return getTransactionTableData(tx)
-  })
-}
+  return getTransactionTableData(tx)
+})
 
 export const generateColumns = () => {
   const nonceColumn: Column = {

src/routes/safe/container/selector.js

@@ -13,9 +13,8 @@ import {
 } from '~/routes/safe/store/selectors'
 import { providerNameSelector, userAccountSelector, networkSelector } from '~/logic/wallets/store/selectors'
 import { type Safe } from '~/routes/safe/store/models/safe'
-import { type Owner } from '~/routes/safe/store/models/owner'
 import { type GlobalState } from '~/store'
-import { sameAddress } from '~/logic/wallets/ethAddresses'
+import { isUserOwner } from '~/logic/wallets/ethAddresses'
 import { orderedTokenListSelector, tokensSelector } from '~/logic/tokens/store/selectors'
 import { type Token } from '~/logic/tokens/store/model/token'
 import { type Transaction, type TransactionStatus } from '~/routes/safe/store/models/transaction'
@@ -63,22 +62,7 @@ const getTxStatus = (tx: Transaction, userAddress: string, safe: Safe): Transact
 export const grantedSelector: Selector<GlobalState, RouterProps, boolean> = createSelector(
   userAccountSelector,
   safeSelector,
-  (userAccount: string, safe: Safe | typeof undefined): boolean => {
-    if (!safe) {
-      return false
-    }
-
-    if (!userAccount) {
-      return false
-    }
-
-    const { owners }: List<Owner> = safe
-    if (!owners) {
-      return false
-    }
-
-    return owners.find((owner: Owner) => sameAddress(owner.address, userAccount)) !== undefined
-  },
+  (userAccount: string, safe: Safe | typeof undefined): boolean => isUserOwner(safe, userAccount),
 )
 
 const safeEthAsTokenSelector: Selector<GlobalState, RouterProps, ?Token> = createSelector(

src/routes/safe/store/middleware/notificationsMiddleware.js

@@ -1,27 +1,28 @@
 // @flow
-import type { AnyAction, Store } from 'redux'
+import type { Action, Store } from 'redux'
+import { List } from 'immutable'
 import { push } from 'connected-react-router'
-import { Map } from 'immutable'
 import { type GlobalState } from '~/store/'
 import { ADD_TRANSACTIONS } from '~/routes/safe/store/actions/addTransactions'
 import { ADD_INCOMING_TRANSACTIONS } from '~/routes/safe/store/actions/addIncomingTransactions'
 import { getAwaitingTransactions } from '~/logic/safe/transactions/awaitingTransactions'
 import { userAccountSelector } from '~/logic/wallets/store/selectors'
 import enqueueSnackbar from '~/logic/notifications/store/actions/enqueueSnackbar'
-import { enhanceSnackbarForAction, NOTIFICATIONS, SUCCESS } from '~/logic/notifications'
+import { enhanceSnackbarForAction, NOTIFICATIONS } from '~/logic/notifications'
 import closeSnackbarAction from '~/logic/notifications/store/actions/closeSnackbar'
 import { getIncomingTxAmount } from '~/routes/safe/components/Transactions/TxsTable/columns'
 import updateSafe from '~/routes/safe/store/actions/updateSafe'
 import { loadFromStorage } from '~/utils/storage'
 import { SAFES_KEY } from '~/logic/safe/utils'
 import { RECURRING_USER_KEY } from '~/utils/verifyRecurringUser'
+import { safesMapSelector } from '~/routes/safe/store/selectors'
+import { isUserOwner } from '~/logic/wallets/ethAddresses'
 
-const watchedActions = [
-  ADD_TRANSACTIONS,
-  ADD_INCOMING_TRANSACTIONS,
-]
+const watchedActions = [ADD_TRANSACTIONS, ADD_INCOMING_TRANSACTIONS]
 
-const notificationsMiddleware = (store: Store<GlobalState>) => (next: Function) => async (action: AnyAction) => {
+const notificationsMiddleware = (store: Store<GlobalState>) => (
+  next: Function,
+) => async (action: Action<*>) => {
   const handledAction = next(action)
   const { dispatch } = store
 
@@ -31,28 +32,52 @@ const notificationsMiddleware = (store: Store<GlobalState>) => (next: Function)
       case ADD_TRANSACTIONS: {
         const transactionsList = action.payload
         const userAddress: string = userAccountSelector(state)
-        const awaitingTransactions = getAwaitingTransactions(transactionsList, userAddress)
+        const safeAddress = action.payload.keySeq().get(0)
+        const awaitingTransactions = getAwaitingTransactions(
+          transactionsList,
+          userAddress,
+        )
+        const awaitingTransactionsList = awaitingTransactions.get(
+          safeAddress,
+          List([]),
+        )
+        const safes = safesMapSelector(state)
+        const currentSafe = safes.get(safeAddress)
 
+        if (
+          !isUserOwner(currentSafe, userAddress)
+          || awaitingTransactionsList.size === 0
+        ) {
+          break
+        }
+
+        const notificationKey = `${safeAddress}-${userAddress}`
+        const onNotificationClicked = () => {
+          dispatch(closeSnackbarAction({ key: notificationKey }))
+          dispatch(push(`/safes/${safeAddress}/transactions`))
+        }
+        dispatch(
+          enqueueSnackbar(
+            enhanceSnackbarForAction(
+              NOTIFICATIONS.TX_WAITING_MSG,
+              notificationKey,
+              onNotificationClicked,
+            ),
+          ),
+        )
 
-        awaitingTransactions.map((awaitingTransactionsList, safeAddress) => {
-          const convertedList = awaitingTransactionsList.toJS()
-          const notificationKey = `${safeAddress}-${userAddress}`
-          const onNotificationClicked = () => {
-            dispatch(closeSnackbarAction({ key: notificationKey }))
-            dispatch(push(`/safes/${safeAddress}/transactions`))
-          }
-          if (convertedList.length > 0) {
-            dispatch(enqueueSnackbar(enhanceSnackbarForAction(NOTIFICATIONS.TX_WAITING_MSG, notificationKey, onNotificationClicked)))
-          }
-        })
         break
       }
       case ADD_INCOMING_TRANSACTIONS: {
         action.payload.forEach(async (incomingTransactions, safeAddress) => {
           const storedSafes = await loadFromStorage(SAFES_KEY)
-          const latestIncomingTxBlock = storedSafes ? storedSafes[safeAddress].latestIncomingTxBlock : 0
+          const latestIncomingTxBlock = storedSafes
+            ? storedSafes[safeAddress].latestIncomingTxBlock
+            : 0
 
-          const newIncomingTransactions = incomingTransactions.filter((tx) => tx.blockNumber > latestIncomingTxBlock)
+          const newIncomingTransactions = incomingTransactions.filter(
+            (tx) => tx.blockNumber > latestIncomingTxBlock,
+          )
           const { message, ...TX_INCOMING_MSG } = NOTIFICATIONS.TX_INCOMING_MSG
           const recurringUser = await loadFromStorage(RECURRING_USER_KEY)
 
@@ -62,9 +87,9 @@ const notificationsMiddleware = (store: Store<GlobalState>) => (next: Function)
                 enqueueSnackbar(
                   enhanceSnackbarForAction({
                     ...TX_INCOMING_MSG,
-                    message: 'Multiple incoming transfers'
-                  })
-                )
+                    message: 'Multiple incoming transfers',
+                  }),
+                ),
               )
             } else {
               newIncomingTransactions.forEach((tx) => {

src/routes/safe/store/selectors/index.js

@@ -1,14 +1,14 @@
 // @flow
 import { Map, List, Set } from 'immutable'
-import { type Match } from 'react-router-dom'
+import { type Match, matchPath } from 'react-router-dom'
 import { createSelector, createStructuredSelector, type Selector } from 'reselect'
 import { type GlobalState } from '~/store/index'
-import { SAFE_PARAM_ADDRESS } from '~/routes/routes'
+import { SAFE_PARAM_ADDRESS, SAFELIST_ADDRESS } from '~/routes/routes'
 import { type Safe } from '~/routes/safe/store/models/safe'
 import { type State as TransactionsState, TRANSACTIONS_REDUCER_ID } from '~/routes/safe/store/reducer/transactions'
 import {
   type IncomingState as IncomingTransactionsState,
-  INCOMING_TRANSACTIONS_REDUCER_ID
+  INCOMING_TRANSACTIONS_REDUCER_ID,
 } from '~/routes/safe/store/reducer/incomingTransactions'
 import { type Transaction } from '~/routes/safe/store/models/transaction'
 import { type Confirmation } from '~/routes/safe/store/models/confirmation'
@@ -83,7 +83,7 @@ export const safeIncomingTransactionsSelector: Selector<GlobalState, RouterProps
     }
 
     return incomingTransactions.get(address) || List([])
-  }
+  },
 )
 
 export const confirmationsTransactionSelector: Selector<GlobalState, TransactionProps, number> = createSelector(