From a764a23e660252f5ed71d6c3e22fefde97497d65 Mon Sep 17 00:00:00 2001 From: Fernando Date: Wed, 23 Sep 2020 09:39:25 -0300 Subject: [PATCH] (Fix) (development) Missing collectibles and Safe version (#1375) * use `updateSafe` instead of `addSafe` * fix SAFE_UPDATE reducer - treat every key individually * allow to load owners on the first request * Set UPDATE_SAFE to individually handling all props * Handle List special case * Add comment to list check Co-authored-by: Daniel Sanchez --- src/logic/safe/store/actions/fetchSafe.ts | 51 +++++++++++------------ src/logic/safe/store/reducer/safe.ts | 28 ++++++++++++- 2 files changed, 51 insertions(+), 28 deletions(-) diff --git a/src/logic/safe/store/actions/fetchSafe.ts b/src/logic/safe/store/actions/fetchSafe.ts index 80ca31a5..0b54ed9c 100644 --- a/src/logic/safe/store/actions/fetchSafe.ts +++ b/src/logic/safe/store/actions/fetchSafe.ts @@ -6,7 +6,6 @@ import { getLocalSafe, getSafeName } from 'src/logic/safe/utils' import { enabledFeatures, safeNeedsUpdate } from 'src/logic/safe/utils/safeVersion' import { sameAddress } from 'src/logic/wallets/ethAddresses' import { getBalanceInEtherOf } from 'src/logic/wallets/getWeb3' -import addSafe from 'src/logic/safe/store/actions/addSafe' import addSafeOwner from 'src/logic/safe/store/actions/addSafeOwner' import removeSafeOwner from 'src/logic/safe/store/actions/removeSafeOwner' import updateSafe from 'src/logic/safe/store/actions/updateSafe' @@ -115,7 +114,7 @@ export const checkAndUpdateSafe = (safeAdd: string) => async (dispatch: Dispatch ]) // Converts from [ { address, ownerName} ] to address array - const localOwners = localSafe ? localSafe.owners.map((localOwner) => localOwner.address) : undefined + const localOwners = localSafe ? localSafe.owners.map((localOwner) => localOwner.address) : [] dispatch( updateSafe({ @@ -127,30 +126,27 @@ export const checkAndUpdateSafe = (safeAdd: string) => async (dispatch: Dispatch ) // If the remote owners does not contain a local address, we remove that local owner - if (localOwners) { - localOwners.forEach((localAddress) => { - const remoteOwnerIndex = remoteOwners.findIndex((remoteAddress) => sameAddress(remoteAddress, localAddress)) - if (remoteOwnerIndex === -1) { - dispatch(removeSafeOwner({ safeAddress, ownerAddress: localAddress })) - } - }) + localOwners.forEach((localAddress) => { + const remoteOwnerIndex = remoteOwners.findIndex((remoteAddress) => sameAddress(remoteAddress, localAddress)) + if (remoteOwnerIndex === -1) { + dispatch(removeSafeOwner({ safeAddress, ownerAddress: localAddress })) + } + }) - // If the remote has an owner that we don't have locally, we add it - remoteOwners.forEach((remoteAddress) => { - const localOwnerIndex = localOwners.findIndex((localAddress) => sameAddress(remoteAddress, localAddress)) - if (localOwnerIndex === -1) { - dispatch( - addSafeOwner({ - safeAddress, - ownerAddress: remoteAddress, - ownerName: 'UNKNOWN', - }), - ) - } - }) - } + // If the remote has an owner that we don't have locally, we add it + remoteOwners.forEach((remoteAddress) => { + const localOwnerIndex = localOwners.findIndex((localAddress) => sameAddress(remoteAddress, localAddress)) + if (localOwnerIndex === -1) { + dispatch( + addSafeOwner({ + safeAddress, + ownerAddress: remoteAddress, + ownerName: 'UNKNOWN', + }), + ) + } + }) } - export default (safeAdd: string) => async ( dispatch: Dispatch, getState: () => AppReduxState, @@ -161,9 +157,12 @@ export default (safeAdd: string) => async ( const latestMasterContractVersion = latestMasterContractVersionSelector(getState()) const safeProps = await buildSafe(safeAddress, safeName, latestMasterContractVersion) - dispatch(addSafe(safeProps)) + // `updateSafe`, as `loadSafesFromStorage` will populate the store previous to this call + // and `addSafe` will only add a newly non-existent safe + // For the case where the safe does not exist in the localStorage, + // `updateSafe` uses a default `notSetValue` to add the Safe to the store + dispatch(updateSafe(safeProps)) } catch (err) { - // eslint-disable-next-line console.error('Error while updating Safe information: ', err) return Promise.resolve() diff --git a/src/logic/safe/store/reducer/safe.ts b/src/logic/safe/store/reducer/safe.ts index becddb83..094cdf80 100644 --- a/src/logic/safe/store/reducer/safe.ts +++ b/src/logic/safe/store/reducer/safe.ts @@ -1,4 +1,4 @@ -import { Map, Set } from 'immutable' +import { Map, Set, List } from 'immutable' import { handleActions } from 'redux-actions' import { ACTIVATE_TOKEN_FOR_ALL_SAFES } from 'src/logic/safe/store/actions/activateTokenForAllSafes' @@ -51,7 +51,31 @@ export default handleActions( return state.updateIn( ['safes', safeAddress], makeSafe({ name: 'LOADED SAFE', address: safeAddress }), - (prevSafe) => prevSafe.merge(safe), + (prevSafe) => { + return prevSafe.withMutations((record) => { + // Every property is updated individually to overcome the issue with nested data being overwritten + const safeProperties = Object.keys(safe) + + // We check each safe property sent in action.payload + safeProperties.forEach((key) => { + if (safe[key] && typeof safe[key] === 'object') { + if (safe[key].length) { + // If type is array we update the array + record.update(key, () => safe[key]) + } else if (safe[key].size) { + // If type is Immutable List we replace current List + // If type is Object we do a merge + List.isList(safe[key]) + ? record.update(key, (current) => current.set(safe[key])) + : record.update(key, (current) => current.merge(safe[key])) + } + } else { + // By default we overwrite the value. This is for strings, numbers and unset values + record.set(key, safe[key]) + } + }) + }) + }, ) }, [ACTIVATE_TOKEN_FOR_ALL_SAFES]: (state: SafeReducerMap, action) => {