Skip to content
This repository has been archived by the owner on Jun 10, 2022. It is now read-only.

transaction details in tables #199

Merged
merged 5 commits into from
Jan 29, 2020
Merged

transaction details in tables #199

merged 5 commits into from
Jan 29, 2020

Conversation

decentraliser
Copy link
Contributor

Pushing a WIP for a preliminary review @dgarcia360

Currently, the account transactions command is stable

I'd like to have your preferences in terms of files naming and files locations, as well as any other comment

Also, would you have any idea about a clean way to test all transaction types against the tables, please tell me

Copy link
Contributor

@dgarcia360 dgarcia360 left a comment

Choose a reason for hiding this comment

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

Well done! Only a few minor comments. As the next steps we need to:

  1. Think about testing each table and builder.
  2. Replace every transaction command table formatter (e.g. https://github.com/nemtech/nem2-cli/blob/master/src/commands/transaction/accountaddressrestriction.ts#L79) for the new one.

src/views/transactions/TransactionViewHeader.ts Outdated Show resolved Hide resolved
@dgarcia360 dgarcia360 self-assigned this Jan 26, 2020
@decentraliser
Copy link
Contributor Author

I fixed the unit test of multisigService.getAddressesFromGraphInfo as well

Copy link
Contributor

@dgarcia360 dgarcia360 left a comment

Choose a reason for hiding this comment

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

Thanks @decentraliser, only a few class + files names changes to match the SDK.

@dgarcia360
Copy link
Contributor

@decentraliser Could add unit tests for the following files?

views/details/transaction-details/transactionNameFromType.ts
views/details/transaction-details/transactionDetailViewFactory.ts
views/details/transaction.view
views/moasaic.view
views/namespaces.view
views/recipient.view

Also, I was wondering if makes sense for you to move

views/details/transaction-details/transactionNameFromType.ts
views/details/transaction-details/transactionDetailViewFactory.ts

to

views/details/transactionNameFromType.ts
views/details/transactionDetailViewFactory.ts

@dgarcia360 dgarcia360 merged commit 54b9917 into master Jan 29, 2020
@dgarcia360 dgarcia360 deleted the tx-details branch January 29, 2020 10:28
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.

None yet

2 participants