From f9ea258ad080c3f67bef4fd9a8deb9877aa23743 Mon Sep 17 00:00:00 2001 From: Felipe Renan Date: Wed, 5 Dec 2018 14:02:04 -0200 Subject: [PATCH] Refactor transactions page As we did in other pages, we removed the infinite scroll and make the first load async. --- .../assets/__tests__/pages/transactions.js | 115 ++++++++---------- .../assets/js/pages/transactions.js | 48 ++------ .../controllers/transaction_controller.ex | 57 +++------ .../templates/transaction/index.html.eex | 49 ++++---- .../transaction_controller_test.exs | 71 ++++++----- 5 files changed, 138 insertions(+), 202 deletions(-) diff --git a/apps/block_scout_web/assets/__tests__/pages/transactions.js b/apps/block_scout_web/assets/__tests__/pages/transactions.js index e1cef7aaa4..de5fe06b60 100644 --- a/apps/block_scout_web/assets/__tests__/pages/transactions.js +++ b/apps/block_scout_web/assets/__tests__/pages/transactions.js @@ -13,76 +13,63 @@ test('CHANNEL_DISCONNECTED', () => { describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { test('single transaction', () => { - const state = initialState + const state = Object.assign({}, initialState, { items: [] }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', msgs: [{ - transactionHtml: 'test' + transactionHtml: 'transaction_html' }] } const output = reducer(state, action) - expect(output.transactions).toEqual([{ transactionHtml: 'test' }]) + expect(output.items).toEqual(['transaction_html']) expect(output.transactionsBatch.length).toEqual(0) expect(output.transactionCount).toEqual(1) }) + test('large batch of transactions', () => { - const state = initialState + const state = Object.assign({}, initialState, { items: [] }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', - msgs: [{ - transactionHtml: 'test 1' - },{ - transactionHtml: 'test 2' - },{ - transactionHtml: 'test 3' - },{ - transactionHtml: 'test 4' - },{ - transactionHtml: 'test 5' - },{ - transactionHtml: 'test 6' - },{ - transactionHtml: 'test 7' - },{ - transactionHtml: 'test 8' - },{ - transactionHtml: 'test 9' - },{ - transactionHtml: 'test 10' - },{ - transactionHtml: 'test 11' - }] + msgs: [ + { transactionHtml: 'transaction_html_1' }, + { transactionHtml: 'transaction_html_2' }, + { transactionHtml: 'transaction_html_3' }, + { transactionHtml: 'transaction_html_4' }, + { transactionHtml: 'transaction_html_5' }, + { transactionHtml: 'transaction_html_6' }, + { transactionHtml: 'transaction_html_7' }, + { transactionHtml: 'transaction_html_8' }, + { transactionHtml: 'transaction_html_9' }, + { transactionHtml: 'transaction_html_10' }, + { transactionHtml: 'transaction_html_11' }, + ] } const output = reducer(state, action) - expect(output.transactions).toEqual([]) + expect(output.items).toEqual([]) expect(output.transactionsBatch.length).toEqual(11) expect(output.transactionCount).toEqual(11) }) + test('single transaction after single transaction', () => { - const state = Object.assign({}, initialState, { - transactions: [{ - transactionHtml: 'test 1' - }] - }) + const state = Object.assign({}, initialState, { items: [ 'transaction_html' ] }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', msgs: [{ - transactionHtml: 'test 2' + transactionHtml: 'another_transaction_html' }] } const output = reducer(state, action) - expect(output.transactions).toEqual([ - { transactionHtml: 'test 2' }, - { transactionHtml: 'test 1' } - ]) + expect(output.items).toEqual([ 'another_transaction_html', 'transaction_html' ]) expect(output.transactionsBatch.length).toEqual(0) }) + test('single transaction after large batch of transactions', () => { const state = Object.assign({}, initialState, { - transactionsBatch: [1,2,3,4,5,6,7,8,9,10,11] + items: [], + transactionsBatch: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', @@ -92,57 +79,51 @@ describe('RECEIVED_NEW_TRANSACTION_BATCH', () => { } const output = reducer(state, action) - expect(output.transactions).toEqual([]) + expect(output.items).toEqual([]) expect(output.transactionsBatch.length).toEqual(12) }) + test('large batch of transactions after large batch of transactions', () => { const state = Object.assign({}, initialState, { - transactionsBatch: [1,2,3,4,5,6,7,8,9,10,11] + items: [], + transactionsBatch: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11] }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', - msgs: [{ - transactionHtml: 'test 12' - },{ - transactionHtml: 'test 13' - },{ - transactionHtml: 'test 14' - },{ - transactionHtml: 'test 15' - },{ - transactionHtml: 'test 16' - },{ - transactionHtml: 'test 17' - },{ - transactionHtml: 'test 18' - },{ - transactionHtml: 'test 19' - },{ - transactionHtml: 'test 20' - },{ - transactionHtml: 'test 21' - },{ - transactionHtml: 'test 22' - }] + msgs: [ + { transactionHtml: 'transaction_html_12' }, + { transactionHtml: 'transaction_html_13' }, + { transactionHtml: 'transaction_html_14' }, + { transactionHtml: 'transaction_html_15' }, + { transactionHtml: 'transaction_html_16' }, + { transactionHtml: 'transaction_html_17' }, + { transactionHtml: 'transaction_html_18' }, + { transactionHtml: 'transaction_html_19' }, + { transactionHtml: 'transaction_html_20' }, + { transactionHtml: 'transaction_html_21' }, + { transactionHtml: 'transaction_html_22' } + ] } const output = reducer(state, action) - expect(output.transactions).toEqual([]) + expect(output.items).toEqual([]) expect(output.transactionsBatch.length).toEqual(22) }) + test('after disconnection', () => { const state = Object.assign({}, initialState, { + items: [], channelDisconnected: true }) const action = { type: 'RECEIVED_NEW_TRANSACTION_BATCH', msgs: [{ - transactionHtml: 'test' + transactionHtml: 'transaction_html' }] } const output = reducer(state, action) - expect(output.transactions).toEqual([]) + expect(output.items).toEqual([]) expect(output.transactionsBatch.length).toEqual(0) }) }) diff --git a/apps/block_scout_web/assets/js/pages/transactions.js b/apps/block_scout_web/assets/js/pages/transactions.js index 3e435f60f0..e23836ca36 100644 --- a/apps/block_scout_web/assets/js/pages/transactions.js +++ b/apps/block_scout_web/assets/js/pages/transactions.js @@ -3,25 +3,19 @@ import _ from 'lodash' import humps from 'humps' import numeral from 'numeral' import socket from '../socket' -import { createStore, connectElements } from '../lib/redux_helpers.js' -import { withInfiniteScroll, connectInfiniteScroll } from '../lib/infinite_scroll_helpers' +import { connectElements } from '../lib/redux_helpers' +import { createAsyncLoadStore } from '../lib/async_listing_load' import { batchChannel } from '../lib/utils' -import listMorph from '../lib/list_morph' const BATCH_THRESHOLD = 10 export const initialState = { channelDisconnected: false, - transactionCount: null, - - transactions: [], transactionsBatch: [] } -export const reducer = withInfiniteScroll(baseReducer) - -function baseReducer (state = initialState, action) { +export function reducer (state = initialState, action) { switch (action.type) { case 'ELEMENTS_LOAD': { return Object.assign({}, state, _.omit(action, 'type')) @@ -33,15 +27,15 @@ function baseReducer (state = initialState, action) { }) } case 'RECEIVED_NEW_TRANSACTION_BATCH': { - if (state.channelDisconnected) return state + if (state.channelDisconnected || state.beyondPageOne) return state const transactionCount = state.transactionCount + action.msgs.length if (!state.transactionsBatch.length && action.msgs.length < BATCH_THRESHOLD) { return Object.assign({}, state, { - transactions: [ - ...action.msgs.reverse(), - ...state.transactions + items: [ + ...action.msgs.map(msg => msg.transactionHtml).reverse(), + ...state.items ], transactionCount }) @@ -55,14 +49,6 @@ function baseReducer (state = initialState, action) { }) } } - case 'RECEIVED_NEXT_PAGE': { - return Object.assign({}, state, { - transactions: [ - ...state.transactions, - ...action.msg.transactions - ] - }) - } default: return state } @@ -90,30 +76,14 @@ const elements = { if (oldState.transactionCount === state.transactionCount) return $el.empty().append(numeral(state.transactionCount).format()) } - }, - '[data-selector="transactions-list"]': { - load ($el, store) { - return { - transactions: $el.children().map((index, el) => ({ - transactionHash: el.dataset.transactionHash, - transactionHtml: el.outerHTML - })).toArray() - } - }, - render ($el, state, oldState) { - if (oldState.transactions === state.transactions) return - const container = $el[0] - const newElements = _.map(state.transactions, ({ transactionHtml }) => $(transactionHtml)[0]) - listMorph(container, newElements, { key: 'dataset.transactionHash' }) - } } } const $transactionListPage = $('[data-page="transaction-list"]') if ($transactionListPage.length) { - const store = createStore(reducer) + const store = createAsyncLoadStore(reducer, initialState, 'dataset.transactionHash') + connectElements({ store, elements }) - connectInfiniteScroll(store) const transactionsChannel = socket.channel(`transactions:new_transaction`) transactionsChannel.join() diff --git a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex index 8ad991cef6..b40e6988e0 100644 --- a/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex +++ b/apps/block_scout_web/lib/block_scout_web/controllers/transaction_controller.ex @@ -5,7 +5,6 @@ defmodule BlockScoutWeb.TransactionController do alias BlockScoutWeb.TransactionView alias Explorer.Chain - alias Explorer.Chain.Hash alias Phoenix.View def index(conn, %{"type" => "JSON"} = params) do @@ -22,65 +21,42 @@ defmodule BlockScoutWeb.TransactionController do paging_options(params) ) - {transactions, next_page} = get_transactions_and_next_page(full_options) + transactions_plus_one = Chain.recent_collated_transactions(full_options) + {transactions, next_page} = split_list_by_page(transactions_plus_one) - next_page_url = + next_page_path = case next_page_params(next_page, transactions, params) do nil -> nil next_page_params -> - transaction_path( - conn, - :index, - next_page_params - ) + transaction_path(conn, :index, Map.delete(next_page_params, "type")) end json( conn, %{ - transactions: + items: Enum.map(transactions, fn transaction -> - %{ - transaction_hash: Hash.to_string(transaction.hash), - transaction_html: - View.render_to_string( - TransactionView, - "_tile.html", - transaction: transaction - ) - } + View.render_to_string( + TransactionView, + "_tile.html", + transaction: transaction + ) end), - next_page_url: next_page_url + next_page_path: next_page_path } ) end - def index(conn, params) do - full_options = - Keyword.merge( - [ - necessity_by_association: %{ - :block => :required, - [created_contract_address: :names] => :optional, - [from_address: :names] => :optional, - [to_address: :names] => :optional - } - ], - paging_options(%{}) - ) - - {transactions, next_page} = get_transactions_and_next_page(full_options) - + def index(conn, _params) do transaction_estimated_count = Chain.transaction_estimated_count() render( conn, "index.html", - next_page_params: next_page_params(next_page, transactions, params), - transaction_estimated_count: transaction_estimated_count, - transactions: transactions + current_path: current_path(conn), + transaction_estimated_count: transaction_estimated_count ) end @@ -102,9 +78,4 @@ defmodule BlockScoutWeb.TransactionController do redirect(conn, to: transaction_internal_transaction_path(conn, :index, id)) end end - - defp get_transactions_and_next_page(options) do - transactions_plus_one = Chain.recent_collated_transactions(options) - split_list_by_page(transactions_plus_one) - end end diff --git a/apps/block_scout_web/lib/block_scout_web/templates/transaction/index.html.eex b/apps/block_scout_web/lib/block_scout_web/templates/transaction/index.html.eex index 7010cff447..63a2875ed1 100644 --- a/apps/block_scout_web/lib/block_scout_web/templates/transaction/index.html.eex +++ b/apps/block_scout_web/lib/block_scout_web/templates/transaction/index.html.eex @@ -1,6 +1,6 @@
-
+

<%= gettext "Validated Transactions" %>

@@ -20,35 +20,38 @@

- - <%= for transaction <- @transactions do %> - <%= render BlockScoutWeb.TransactionView, "_tile.html", transaction: transaction %> - <% end %> - - +
+ + +
diff --git a/apps/block_scout_web/test/block_scout_web/controllers/transaction_controller_test.exs b/apps/block_scout_web/test/block_scout_web/controllers/transaction_controller_test.exs index 2819c6894b..1e62745824 100644 --- a/apps/block_scout_web/test/block_scout_web/controllers/transaction_controller_test.exs +++ b/apps/block_scout_web/test/block_scout_web/controllers/transaction_controller_test.exs @@ -7,14 +7,18 @@ defmodule BlockScoutWeb.TransactionControllerTest do describe "GET index/2" do test "returns a collated transactions", %{conn: conn} do - transaction = - :transaction - |> insert() - |> with_block() + :transaction + |> insert() + |> with_block() - conn = get(conn, "/txs") + conn = get(conn, transaction_path(conn, :index, %{"type" => "JSON"})) + + transactions_html = + conn + |> json_response(200) + |> Map.get("items") - assert List.first(conn.assigns.transactions).hash == transaction.hash + assert length(transactions_html) == 1 end test "returns a count of transactions", %{conn: conn} do @@ -28,16 +32,22 @@ defmodule BlockScoutWeb.TransactionControllerTest do end test "excludes pending transactions", %{conn: conn} do - %Transaction{hash: hash} = + %Transaction{hash: transaction_hash} = :transaction |> insert() |> with_block() - insert(:transaction) + %Transaction{hash: pending_transaction_hash} = insert(:transaction) - conn = get(conn, "/txs") + conn = get(conn, transaction_path(conn, :index, %{"type" => "JSON"})) + + transactions_html = + conn + |> json_response(200) + |> Map.get("items") - assert [%Transaction{hash: ^hash}] = conn.assigns.transactions + assert Enum.any?(transactions_html, &String.contains?(&1, to_string(transaction_hash))) + refute Enum.any?(transactions_html, &String.contains?(&1, to_string(pending_transaction_hash))) end test "returns next page of results based on last seen transaction", %{conn: conn} do @@ -53,33 +63,34 @@ defmodule BlockScoutWeb.TransactionControllerTest do |> with_block() conn = - get(conn, "/txs", %{ - "type" => "JSON", - "block_number" => Integer.to_string(block_number), - "index" => Integer.to_string(index) - }) - - {:ok, %{"transactions" => transactions}} = conn.resp_body |> Poison.decode() - - actual_hashes = - transactions - |> Enum.map(& &1["transaction_hash"]) - |> Enum.reverse() - - assert second_page_hashes == actual_hashes + get( + conn, + transaction_path(conn, :index, %{ + "type" => "JSON", + "block_number" => Integer.to_string(block_number), + "index" => Integer.to_string(index) + }) + ) + + transactions_html = + conn + |> json_response(200) + |> Map.get("items") + + assert length(second_page_hashes) == length(transactions_html) end test "next_page_params exist if not on last page", %{conn: conn} do address = insert(:address) - block = %Block{number: number} = insert(:block) + block = insert(:block) 60 |> insert_list(:transaction, from_address: address) |> with_block(block) - conn = get(conn, "/txs") + conn = get(conn, transaction_path(conn, :index, %{"type" => "JSON"})) - assert %{"block_number" => ^number, "index" => 10} = conn.assigns.next_page_params + assert conn |> json_response(200) |> Map.get("next_page_path") end test "next_page_params are empty if on last page", %{conn: conn} do @@ -89,15 +100,15 @@ defmodule BlockScoutWeb.TransactionControllerTest do |> insert(from_address: address) |> with_block() - conn = get(conn, "/txs") + conn = get(conn, transaction_path(conn, :index, %{"type" => "JSON"})) - refute conn.assigns.next_page_params + refute conn |> json_response(200) |> Map.get("next_page_path") end test "works when there are no transactions", %{conn: conn} do conn = get(conn, "/txs") - assert conn.assigns.transactions == [] + assert html_response(conn, 200) end end