From 2f2c0eefbf3acab3cdd2b37b8f5451fad66c1872 Mon Sep 17 00:00:00 2001 From: "Michael Bradley, Jr" Date: Sun, 5 May 2019 18:44:14 -0500 Subject: [PATCH] refactor(@cockpit/explorer): revise transactions explorer per techniques of PRs #1492, #1494 In addition to introducing improvements per #1492 and #1494, adjust the transactions request action to allow for a `blockLimit` argument, since that API parameter is supported by embark. Revise `changePage` to invoke `fetchTransactions` correctly, i.e. with arguments pertaining to a starting block number and a block limit. These changes together fix most of the pagination problems related to exploring transactions, i.e. badly mis-ordered results are no longer displayed. However, a *wart* still remains related to estimation of the total number of transactions and pages. Sometimes the calculated number of pages for the transactions explorer doesn't match up to the actual number of transactions on the blockchain (owing to estimation). The pagination controls and display of transactions will temporarily behave a little strangely if one jumps ahead in the pages, e.g. a jump from cockpit explorer overview's transactions page 1 to page 5 for embark's demo, if an additional transaction has been added and the explorer overview is freshly loaded. This behavior is related to the fact that actions such as `fetchBlocksFull` and `fetchTransactions` are async without any means to determine when all actions have completed and React re-/rendering has settled down. There are probably some architectural changes that could improve the situation, but they're outside the scope of this PR and in no way easy to solve by means of React lifecycle methods. In fact, attempts to make an improvement with `componentDidUpdate` (and watching what happens in the debugger) revealed the nature of the problem, as described above. --- packages/embark-ui/src/actions/index.js | 2 +- .../src/containers/TransactionsContainer.js | 92 ++++++++++++------- 2 files changed, 58 insertions(+), 36 deletions(-) diff --git a/packages/embark-ui/src/actions/index.js b/packages/embark-ui/src/actions/index.js index d7616eabd..55d80fa2e 100644 --- a/packages/embark-ui/src/actions/index.js +++ b/packages/embark-ui/src/actions/index.js @@ -100,7 +100,7 @@ export const block = { export const TRANSACTIONS = createRequestTypes('TRANSACTIONS'); export const transactions = { - request: (blockFrom) => action(TRANSACTIONS[REQUEST], {blockFrom}), + request: (blockFrom, blockLimit) => action(TRANSACTIONS[REQUEST], {blockFrom, blockLimit}), success: (transactions) => action(TRANSACTIONS[SUCCESS], {transactions}), failure: (error) => action(TRANSACTIONS[FAILURE], {error}) }; diff --git a/packages/embark-ui/src/containers/TransactionsContainer.js b/packages/embark-ui/src/containers/TransactionsContainer.js index 995ba872a..2a2e55d4f 100644 --- a/packages/embark-ui/src/containers/TransactionsContainer.js +++ b/packages/embark-ui/src/containers/TransactionsContainer.js @@ -7,9 +7,8 @@ import {blocksFull as blocksAction, stopBlockHeader, transactions as transactionsAction} from '../actions'; import Transactions from '../components/Transactions'; -import DataWrapper from "../components/DataWrapper"; -import PageHead from "../components/PageHead"; -import {getBlocksFull, getContracts} from "../reducers/selectors"; +import PageHead from '../components/PageHead'; +import {getBlocksFull, getContracts, getTransactions} from '../reducers/selectors'; const MAX_TXS = 10; // TODO use same constant as API @@ -19,6 +18,7 @@ class TransactionsContainer extends Component { this.numTxsToDisplay = this.props.numTxsToDisplay || MAX_TXS; this.numBlocksToFetch = this.numTxsToDisplay; + this.resetNums(); this.state = {currentPage: 1}; } @@ -34,34 +34,57 @@ class TransactionsContainer extends Component { } get numberOfBlocks() { - const blocks = this.props.blocks; - return !blocks.length ? 0 : blocks[0].number + 1; + if (this._numberOfBlocks === null) { + const blocks = this.props.blocks; + this._numberOfBlocks = !blocks.length ? 0 : blocks[0].number + 1; + } + return this._numberOfBlocks; } get estNumberOfTxs() { + if (this._estNumberOfTxs !== null) { + return this._estNumberOfTxs; + } const blocks = this.props.blocks; const numBlocksInProps = blocks.length; - const numTxsInPropsBlocks = blocks.reduce((txCount, block) => ( - txCount + block.transactions.length - ), 0); + const numTxsInPropsBlocks = blocks.reduce( + (txCount, block) => txCount + block.transactions.length, + 0 + ); const missingNumBlocks = this.numberOfBlocks - numBlocksInProps; - return missingNumBlocks + numTxsInPropsBlocks; + this._estNumberOfTxs = missingNumBlocks + numTxsInPropsBlocks; + return this._estNumberOfTxs; } - getNumberOfPages() { - return Math.ceil(this.estNumberOfTxs / this.numTxsToDisplay); + get numberOfPages() { + if (this._numberOfPages === null) { + this._numberOfPages = Math.ceil( + this.estNumberOfTxs / this.numTxsToDisplay + ); + } + return this._numberOfPages; + } + + resetNums() { + this._numberOfBlocks = null; + this._estNumberOfTxs = null; + this._numberOfPages = null; } changePage(newPage) { - this.setState({currentPage: newPage}); - this.props.fetchBlocksFull( - this.numberOfBlocks - 1 - (this.numBlocksToFetch * (newPage - 1)), - this.numBlocksToFetch - ); - this.props.fetchTransactions((newPage * MAX_TXS) + MAX_TXS); + if (newPage <= 0) { + newPage = 1; + } else if (newPage > this.numberOfPages) { + newPage = this.numberOfPages; + } + this.setState({ currentPage: newPage }); + const blockFrom = + this.numberOfBlocks - 1 - this.numBlocksToFetch * (newPage - 1); + this.props.fetchBlocksFull(blockFrom, this.numBlocksToFetch); + this.props.fetchTransactions(blockFrom, this.numTxsToDisplay); } - getCurrentTransactions() { + get currentTransactions() { if (!this.props.blocks.length) return []; let relativeBlock = this.numberOfBlocks - 1; let offset = 0; @@ -81,33 +104,28 @@ class TransactionsContainer extends Component { const estNumberOfTxs = this.estNumberOfTxs; return txs.filter((tx, idx) => { const txNumber = estNumberOfTxs - idx; - const index = ( - (estNumberOfTxs - - (this.numTxsToDisplay * (this.state.currentPage - 1))) - - txNumber + 1 - ); + const index = + estNumberOfTxs - + this.numTxsToDisplay * (this.state.currentPage - 1) - + txNumber + 1; return index <= this.numTxsToDisplay && index > 0; }); } render() { - const newTxs = this.getCurrentTransactions(); + this.resetNums(); return ( - ( - this.changePage(newPage)} - currentPage={this.state.currentPage} /> - )} /> + this.changePage(newPage)} + currentPage={this.state.currentPage} + numberOfPages={this.numberOfPages} /> ); } @@ -117,6 +135,7 @@ function mapStateToProps(state) { return { blocks: getBlocksFull(state), contracts: getContracts(state), + transactions: getTransactions(state), error: state.errorMessage, loading: state.loading }; @@ -125,8 +144,11 @@ function mapStateToProps(state) { TransactionsContainer.propTypes = { blocks: PropTypes.arrayOf(PropTypes.object), contracts: PropTypes.arrayOf(PropTypes.object), + transactions: PropTypes.arrayOf(PropTypes.object), fetchBlocksFull: PropTypes.func, fetchContracts: PropTypes.func, + fetchTransactions: PropTypes.func, + numTxsToDisplay: PropTypes.number, initBlockHeader: PropTypes.func, stopBlockHeader: PropTypes.func, error: PropTypes.string, @@ -142,5 +164,5 @@ export default connect( fetchTransactions: transactionsAction.request, initBlockHeader, stopBlockHeader - }, + } )(TransactionsContainer);