Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

new TXs tab#1089

Merged
Agupane merged 102 commits intodevelopmentfrom
transactions_new
Aug 14, 2020
Merged

new TXs tab#1089
Agupane merged 102 commits intodevelopmentfrom
transactions_new

Conversation

@nicosampler
Copy link
Contributor

@nicosampler nicosampler commented Jul 2, 2020

Closes #354

  • Add a new tx hidden in production
  • Fetch TXs from the new endpoint
  • Save TXs information in a new key into the store
  • Create required selectors
  • Handle TXs updates

@ghost
Copy link

ghost commented Jul 2, 2020

Travis automatic deployment:
https://pr1089--safereact.review.gnosisdev.com/app

safeAddress: string
limit?: number
offset?: number
orderBy?: string // todo: maybe this should be key of MultiSigTransaction | keyof EthereumTransaction
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure to understand the comment

Copy link
Contributor

Choose a reason for hiding this comment

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

because the orderBy parameter is used to order the return fields based on the name of one transaction field (as far I understood the API docs). So basically each one of the keys of MultiSigTransaction / EthereumTransaction could be used

Copy link
Contributor

Choose a reason for hiding this comment

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

it's not 100% guaranteed that api will support sorting by all fields, so this has to be synced with api

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should wait for implementing once we got the final version of the middleware, what do you think? cc @nicosampler

mmv08 and others added 3 commits August 10, 2020 14:25
…to transactions_new

# Conflicts:
#	src/logic/safe/store/actions/__tests__/utils.test.ts
#	src/logic/safe/store/actions/addSafeModules.ts
#	src/logic/safe/store/reducer/safe.ts
#	src/logic/safe/store/reducer/types/safe.d.ts
#	src/routes/safe/components/Apps/index.tsx
#	src/routes/safe/components/Layout/Tabs/index.tsx
#	src/routes/safe/components/Settings/Advanced/dataFetcher.ts
#	src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx
#	src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/CustomDescription.tsx
#	src/routes/safe/container/hooks/useFetchTokens.tsx
#	src/routes/safe/container/hooks/useLoadSafe.tsx
#	src/routes/safe/store/reducer/types/safe.d.ts
#	src/routes/safe/store/reducer/types/safe.ts
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@ghost
Copy link

ghost commented Aug 10, 2020

Travis automatic deployment:
https://pr1089--safereact.review.gnosisdev.com/app

import { Transaction, TxType } from '../../../models/types/transactions'

export const isMultiSigTx = (tx: Transaction): boolean => {
return TxType[tx.txType] === TxType.MULTISIG_TRANSACTION
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: TxType[tx.type] looks weird, can we come up with a better name for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

what do you suggest?

}

const etagsByPage = {}
let totalTransactionsAmount = null
Copy link
Contributor

Choose a reason for hiding this comment

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

this number is unique for each safe but this action used for all safes, do we need really need to store it there?
could we maybe do something like

transactions: {
  safeAddress: {
    totalCount: 1000,
    data: []
  }
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It was done like that before and you asked me to change it

Choose a reason for hiding this comment

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

@mikheevm do you have access to previous implementation? if so, could you please verify if former one was correct? just to avoid re-writing it in case we go back to it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

In the old implementation totalTransactionsAmount was defined at top level instead of being specific to the safe

Copy link
Contributor

Choose a reason for hiding this comment

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

check it now

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

Add Balancer Pool and Exchange Apps.
@ghost
Copy link

ghost commented Aug 11, 2020

Travis automatic deployment:
https://pr1089--safereact.review.gnosisdev.com/app

@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

…to transactions_new

# Conflicts:
#	src/components/Header/components/ProviderDetails/UserDetails.tsx
#	src/routes/safe/components/Apps/index.tsx
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@ghost
Copy link

ghost commented Aug 14, 2020

Travis automatic deployment:
https://pr1089--safereact.review.gnosisdev.com/app

@Agupane Agupane merged commit 691ef98 into development Aug 14, 2020
@Agupane Agupane deleted the transactions_new branch August 14, 2020 15:15
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Transaction List] Use unified Tx endpoint

6 participants