Change to last seen paging and hide paging button on last page for

recent_pending_transactions

apps/explorer/lib/explorer/chain.ex

@@ -55,15 +55,8 @@ defmodule Explorer.Chain do
   """
   @type necessity_by_association :: %{association => necessity}
 
-  @typedoc """
-  Pagination params used by `scrivener`
-  """
-  @type pagination :: map()
-
-  @typep inserted_after_option :: {:inserted_after, DateTime.t()}
   @typep necessity_by_association_option :: {:necessity_by_association, necessity_by_association}
   @typep on_conflict_option :: {:on_conflict, :nothing | :replace_all}
-  @typep pagination_option :: {:pagination, pagination}
   @typep paging_options :: {:paging_options, PagingOptions.t()}
   @typep params_option :: {:params, [map()]}
   @typep timeout_option :: {:timeout, timeout}
@@ -1947,12 +1940,12 @@ defmodule Explorer.Chain do
   end
 
   @doc """
-  Return the list of pending transactions that occurred recently (10).
+  Return the list of pending transactions that occurred recently (8).
 
       iex> 2 |> insert_list(:transaction)
       iex> :transaction |> insert() |> with_block()
       iex> 8 |> insert_list(:transaction)
-      iex> %Scrivener.Page{entries: recent_pending_transactions} = Explorer.Chain.recent_pending_transactions()
+      iex> recent_pending_transactions = Explorer.Chain.recent_pending_transactions()
       iex> length(recent_pending_transactions)
       10
       iex> Enum.all?(recent_pending_transactions, fn %Explorer.Chain.Transaction{block_hash: block_hash} ->
@@ -1960,69 +1953,28 @@ defmodule Explorer.Chain do
       ...> end)
       true
 
-  A `t:Explorer.Chain.Transaction.t/0` `inserted_at` can be supplied to the `:inserted_after` option, then only pending
-  transactions inserted after that transaction will be returned.  This can be used to generate paging for pending
-  transactions.
-
-      iex> {:ok, first_inserted_at, 0} = DateTime.from_iso8601("2015-01-23T23:50:07Z")
-      iex> insert(:transaction, inserted_at: first_inserted_at)
-      iex> {:ok, second_inserted_at, 0} = DateTime.from_iso8601("2016-01-23T23:50:07Z")
-      iex> insert(:transaction, inserted_at: second_inserted_at)
-      iex> %Scrivener.Page{entries: after_first_transaction} = Explorer.Chain.recent_pending_transactions(
-      ...>   inserted_after: first_inserted_at
-      ...> )
-      iex> length(after_first_transaction)
-      1
-      iex> %Scrivener.Page{entries: after_second_transaction} = Explorer.Chain.recent_pending_transactions(
-      ...>   inserted_after: second_inserted_at
-      ...> )
-      iex> length(after_second_transaction)
-      0
-
-  When there are no pending transaction and a collated transaction's inserted_at is used, an empty list is returned
-
-      iex> {:ok, first_inserted_at, 0} = DateTime.from_iso8601("2015-01-23T23:50:07Z")
-      iex> :transaction |> insert(inserted_at: first_inserted_at) |> with_block()
-      iex> {:ok, second_inserted_at, 0} = DateTime.from_iso8601("2016-01-23T23:50:07Z")
-      iex> :transaction |> insert(inserted_at: second_inserted_at) |> with_block()
-      iex> %Scrivener.Page{entries: entries} = Explorer.Chain.recent_pending_transactions(
-      ...>   after_inserted_at: first_inserted_at
-      ...> )
-      iex> entries
-      []
-
   ## Options
 
     * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`.  If an association is
       `:required`, and the `t:Explorer.Chain.InternalTransaction.t/0` has no associated record for that association,
       then the `t:Explorer.Chain.InternalTransaction.t/0` will not be included in the list.
-    * `:pagination` - pagination params to pass to scrivener.
+    * `:paging_options` - a `t:Explorer.PagingOptions.t/0` used to specify the `:page_size` and
+      `:key` (a tuple of the lowest/oldest {inserted_at, hash}) and. Results will be the transactions older than
+      the inserted_at and hash that are passed.
 
   """
-  @spec recent_pending_transactions([inserted_after_option | necessity_by_association_option]) :: %Scrivener.Page{
+  @spec recent_pending_transactions([paging_options | necessity_by_association_option]) :: [Transaction.t()]
-          entries: [Transaction.t()]
-        }
   def recent_pending_transactions(options \\ []) when is_list(options) do
     necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
-    pagination = Keyword.get(options, :pagination, %{})
+    paging_options = Keyword.get(options, :paging_options, %PagingOptions{page_size: 50})
-
-    query =
-      from(
-        transaction in Transaction,
-        where: is_nil(transaction.block_hash),
-        order_by: [
-          desc: transaction.inserted_at,
-          # arbitary tie-breaker when inserted at is the same.  hash is random distribution, but using it keeps order
-          # consistent at least
-          desc: transaction.hash
-        ],
-        limit: 10
-      )
 
-    query
+    Transaction
-    |> inserted_after(options)
+    |> where([transaction], is_nil(transaction.block_hash))
+    |> page_pending_transaction(paging_options)
+    |> limit(^paging_options.page_size)
+    |> order_by([transaction], desc: transaction.inserted_at, desc: transaction.hash)
     |> join_associations(necessity_by_association)
-    |> Repo.paginate(pagination)
+    |> Repo.all()
   end
 
   @doc """
@@ -2154,14 +2106,28 @@ defmodule Explorer.Chain do
     * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`.  If an association is
       `:required`, and the `t:Explorer.Chain.Log.t/0` has no associated record for that association, then the
       `t:Explorer.Chain.Log.t/0` will not be included in the page `entries`.
-    * `:pagination` - pagination params to pass to scrivener.
+    * `:paging_options` - a `t:Explorer.PagingOptions.t/0` used to specify the `:page_size` and
+      `:key` (a tuple of the lowest/oldest {index}) and. Results will be the transactions older than
+      the index that are passed.
 
   """
-  @spec transaction_to_logs(Transaction.t(), [
+  @spec transaction_to_logs(Transaction.t(), [paging_options | necessity_by_association_option]) :: [Log.t()]
-          necessity_by_association_option | pagination_option
+  def transaction_to_logs(
-        ]) :: %Scrivener.Page{entries: [Log.t()]}
+        %Transaction{hash: %Hash{byte_count: unquote(Hash.Full.byte_count())} = transaction_hash},
-  def transaction_to_logs(%Transaction{hash: hash}, options \\ []) when is_list(options) do
+        options \\ []
-    transaction_hash_to_logs(hash, options)
+      )
+      when is_list(options) do
+    necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
+    paging_options = Keyword.get(options, :paging_options, %PagingOptions{page_size: 50})
+
+    Log
+    |> join(:inner, [log], transaction in assoc(log, :transaction))
+    |> where([_, transaction], transaction.hash == ^transaction_hash)
+    |> page_logs(paging_options)
+    |> limit(^paging_options.page_size)
+    |> order_by([log], asc: log.index)
+    |> join_associations(necessity_by_association)
+    |> Repo.all()
   end
 
   @doc """
@@ -2495,16 +2461,6 @@ defmodule Explorer.Chain do
     {:ok, for(transaction <- transactions, do: transaction.hash)}
   end
 
-  defp inserted_after(query, options) do
-    case Keyword.fetch(options, :inserted_after) do
-      {:ok, inserted_after} ->
-        from(transaction in query, where: ^inserted_after < transaction.inserted_at)
-
-      :error ->
-        query
-    end
-  end
-
   defp join_association(query, association, necessity) when is_atom(association) do
     case necessity do
       :optional ->
@@ -2556,6 +2512,23 @@ defmodule Explorer.Chain do
     |> where([internal_transaction], internal_transaction.index < ^index)
   end
 
+  defp page_logs(query, %PagingOptions{key: nil}), do: query
+
+  defp page_logs(query, %PagingOptions{key: {index}}) do
+    query
+    |> where([log], log.index < ^index)
+  end
+
+  defp page_pending_transaction(query, %PagingOptions{key: nil}), do: query
+
+  defp page_pending_transaction(query, %PagingOptions{key: {inserted_at, hash}}) do
+    query
+    |> where(
+      [transaction],
+      transaction.inserted_at < ^inserted_at or (transaction.inserted_at == ^inserted_at and transaction.hash < ^hash)
+    )
+  end
+
   defp page_transaction(query, %PagingOptions{key: nil}), do: query
 
   defp page_transaction(query, %PagingOptions{key: {block_number, index}}) do
@@ -2685,27 +2658,6 @@ defmodule Explorer.Chain do
     %{inserted_at: now, updated_at: now}
   end
 
-  defp transaction_hash_to_logs(
-         %Hash{byte_count: unquote(Hash.Full.byte_count())} = transaction_hash,
-         options
-       )
-       when is_list(options) do
-    necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
-    pagination = Keyword.get(options, :pagination, %{})
-
-    query =
-      from(
-        log in Log,
-        join: transaction in assoc(log, :transaction),
-        where: transaction.hash == ^transaction_hash,
-        order_by: [asc: :index]
-      )
-
-    query
-    |> join_associations(necessity_by_association)
-    |> Repo.paginate(pagination)
-  end
-
   defp where_address_fields_match(query, address_hash, direction \\ nil) do
     address_fields =
       case direction do

apps/explorer/test/explorer/chain_test.exs

@@ -524,6 +524,33 @@ defmodule Explorer.ChainTest do
     end
   end
 
+  describe "pending_transactions/0" do
+    test "without transactions" do
+      assert [] = Chain.recent_pending_transactions()
+    end
+
+    test "with transactions" do
+      %Transaction{hash: hash} = insert(:transaction)
+
+      assert [%Transaction{hash: ^hash}] = Chain.recent_pending_transactions()
+    end
+
+    test "with transactions can be paginated" do
+      second_page_hashes =
+        50
+        |> insert_list(:transaction)
+        |> Enum.map(& &1.hash)
+
+      %Transaction{inserted_at: inserted_at, hash: hash} = insert(:transaction)
+
+      assert second_page_hashes ==
+               [paging_options: %PagingOptions{key: {inserted_at, hash}, page_size: 50}]
+               |> Chain.recent_pending_transactions()
+               |> Enum.map(& &1.hash)
+               |> Enum.reverse()
+    end
+  end
+
   describe "transaction_to_internal_transactions/1" do
     test "with transaction without internal transactions" do
       transaction = insert(:transaction)
@@ -623,12 +650,7 @@ defmodule Explorer.ChainTest do
     test "without logs" do
       transaction = insert(:transaction)
 
-      assert %Scrivener.Page{
+      assert [] = Chain.transaction_to_logs(transaction)
-               entries: [],
-               page_number: 1,
-               total_entries: 0,
-               total_pages: 1
-             } = Chain.transaction_to_logs(transaction)
     end
 
     test "with logs" do
@@ -639,12 +661,7 @@ defmodule Explorer.ChainTest do
 
       %Log{id: id} = insert(:log, transaction: transaction)
 
-      assert %Scrivener.Page{
+      assert [%Log{id: ^id}] = Chain.transaction_to_logs(transaction)
-               entries: [%Log{id: ^id}],
-               page_number: 1,
-               total_entries: 1,
-               total_pages: 1
-             } = Chain.transaction_to_logs(transaction)
     end
 
     test "with logs can be paginated" do
@@ -653,25 +670,17 @@ defmodule Explorer.ChainTest do
         |> insert()
         |> with_block()
 
-      logs = Enum.map(0..1, &insert(:log, index: &1, transaction: transaction))
+      second_page_indexes =
-
+        1..50
-      [%Log{id: first_log_id}, %Log{id: second_log_id}] = logs
+        |> Enum.map(fn index -> insert(:log, transaction_hash: transaction.hash, index: index) end)
+        |> Enum.map(& &1.index)
 
-      assert %Scrivener.Page{
+      log = insert(:log, transaction_hash: transaction.hash, index: 51)
-               entries: [%Log{id: ^first_log_id}],
-               page_number: 1,
-               page_size: 1,
-               total_entries: 2,
-               total_pages: 2
-             } = Chain.transaction_to_logs(transaction, pagination: %{page_size: 1})
 
-      assert %Scrivener.Page{
+      assert second_page_indexes ==
-               entries: [%Log{id: ^second_log_id}],
+               transaction
-               page_number: 2,
+               |> Chain.transaction_to_logs(paging_options: %PagingOptions{key: {log.index}, page_size: 50})
-               page_size: 1,
+               |> Enum.map(& &1.index)
-               total_entries: 2,
-               total_pages: 2
-             } = Chain.transaction_to_logs(transaction, pagination: %{page: 2, page_size: 1})
     end
 
     test "with logs necessity_by_association loads associations" do
@@ -682,17 +691,7 @@ defmodule Explorer.ChainTest do
 
       insert(:log, transaction: transaction)
 
-      assert %Scrivener.Page{
+      assert [%Log{address: %Address{}, transaction: %Transaction{}}] =
-               entries: [
-                 %Log{
-                   address: %Address{},
-                   transaction: %Transaction{}
-                 }
-               ],
-               page_number: 1,
-               total_entries: 1,
-               total_pages: 1
-             } =
                Chain.transaction_to_logs(
                  transaction,
                  necessity_by_association: %{
@@ -701,17 +700,12 @@ defmodule Explorer.ChainTest do
                  }
                )
 
-      assert %Scrivener.Page{
+      assert [
-               entries: [
                %Log{
                  address: %Ecto.Association.NotLoaded{},
                  transaction: %Ecto.Association.NotLoaded{}
                }
-               ],
+             ] = Chain.transaction_to_logs(transaction)
-               page_number: 1,
-               total_entries: 1,
-               total_pages: 1
-             } = Chain.transaction_to_logs(transaction)
     end
   end
 

apps/explorer_web/lib/explorer_web/controllers/pending_transaction_controller.ex

@@ -1,35 +1,54 @@
 defmodule ExplorerWeb.PendingTransactionController do
   use ExplorerWeb, :controller
 
-  alias Explorer.Chain
+  alias Explorer.{Chain, PagingOptions}
+  # alias Explorer.Chain.Hash
+
+  @page_size 50
+  @default_paging_options %PagingOptions{page_size: @page_size + 1}
 
   def index(conn, params) do
-    with %{"last_seen_pending_inserted_at" => last_seen_pending_inserted_at_string} <- params,
+    full_options =
-         {:ok, last_seen_pending_inserted_at} = Timex.parse(last_seen_pending_inserted_at_string, "{ISO:Extended:Z}") do
+      Keyword.merge(
-      do_index(conn, inserted_after: last_seen_pending_inserted_at)
+        [
-    else
+          necessity_by_association: %{
-      _ -> do_index(conn)
+            from_address: :optional,
-    end
+            to_address: :optional
-  end
+          }
+        ],
+        paging_options(params)
+      )
+
+    transactions_plus_one = Chain.recent_pending_transactions(full_options)
+
+    {transactions, next_page} = Enum.split(transactions_plus_one, @page_size)
 
-  defp do_index(conn, options \\ []) when is_list(options) do
-    full_options = Keyword.merge([necessity_by_association: %{from_address: :optional, to_address: :optional}], options)
-    transactions = Chain.recent_pending_transactions(full_options)
-    last_seen_pending_inserted_at = last_seen_pending_inserted_at(transactions.entries)
     pending_transaction_count = Chain.pending_transaction_count()
 
     render(
       conn,
       "index.html",
-      last_seen_pending_inserted_at: last_seen_pending_inserted_at,
+      next_page_params: next_page_params(next_page, transactions),
       pending_transaction_count: pending_transaction_count,
       transactions: transactions
     )
   end
 
-  defp last_seen_pending_inserted_at([]), do: nil
+  defp next_page_params([], _transactions), do: nil
+
+  defp next_page_params(_, transactions) do
+    last = List.last(transactions)
+    %{inserted_at: DateTime.to_iso8601(last.inserted_at), hash: last.hash}
+  end
 
-  defp last_seen_pending_inserted_at(transactions) do
+  defp paging_options(params) do
-    List.last(transactions).inserted_at
+    with %{"inserted_at" => inserted_at_string, "hash" => hash_string} <- params,
+         {:ok, inserted_at, _} <- DateTime.from_iso8601(inserted_at_string),
+         {:ok, hash} <- Chain.string_to_transaction_hash(hash_string) do
+      [paging_options: %{@default_paging_options | key: {inserted_at, hash}}]
+    else
+      _ ->
+        [paging_options: @default_paging_options]
+    end
   end
 end

apps/explorer_web/lib/explorer_web/templates/pending_transaction/index.html.eex

@@ -64,18 +64,15 @@
       </table>
     </div>
   </div>
-  <%= if @last_seen_pending_inserted_at do %>
+  <%= if @next_page_params do %>
     <%= link(
-          gettext("Next Page"),
+      gettext("Older"),
       class: "button button--secondary button--sm u-float-right mt-3",
       to: pending_transaction_path(
         @conn,
         :index,
         @conn.assigns.locale,
-            %{
+        @next_page_params
-               "last_seen_inserted_at" =>
-                 Timex.format!(@last_seen_pending_inserted_at, "{ISO:Extended:Z}")
-            }
       )
     ) %>
   <% end %>

apps/explorer_web/test/explorer_web/controllers/pending_transaction_controller_test.exs

@@ -1,5 +1,6 @@
 defmodule ExplorerWeb.PendingTransactionControllerTest do
   use ExplorerWeb.ConnCase
+  alias Explorer.Chain.{Hash, Transaction}
 
   import ExplorerWeb.Router.Helpers, only: [pending_transaction_path: 3]
 
@@ -48,27 +49,49 @@ defmodule ExplorerWeb.PendingTransactionControllerTest do
       assert 1 == conn.assigns.pending_transaction_count
     end
 
-    test "paginates transactions using the last seen transaction", %{conn: conn} do
+    test "works when there are no transactions", %{conn: conn} do
-      {:ok, first_inserted_at, 0} = DateTime.from_iso8601("2015-01-23T23:50:07Z")
+      conn = get(conn, pending_transaction_path(conn, :index, :en))
-      insert(:transaction, inserted_at: first_inserted_at)
+
-      {:ok, second_inserted_at, 0} = DateTime.from_iso8601("2016-01-23T23:50:07Z")
+      assert html_response(conn, 200)
-      insert(:transaction, inserted_at: second_inserted_at)
+    end
+
+    test "returns next page of results based on last seen pending transaction", %{conn: conn} do
+      second_page_hashes =
+        50
+        |> insert_list(:transaction)
+        |> Enum.map(& &1.hash)
+
+      %Transaction{inserted_at: inserted_at, hash: hash} = insert(:transaction)
 
       conn =
-        get(
+        get(conn, pending_transaction_path(ExplorerWeb.Endpoint, :index, :en), %{
-          conn,
+          "inserted_at" => DateTime.to_iso8601(inserted_at),
-          pending_transaction_path(ExplorerWeb.Endpoint, :index, :en),
+          "hash" => Hash.to_string(hash)
-          last_seen_pending_inserted_at: Timex.format!(first_inserted_at, "{ISO:Extended:Z}")
+        })
-        )
 
-      assert html_response(conn, 200)
+      actual_hashes =
-      assert 1 == Enum.count(conn.assigns.transactions)
+        conn.assigns.transactions
+        |> Enum.map(& &1.hash)
+        |> Enum.reverse()
+
+      assert second_page_hashes == actual_hashes
     end
 
-    test "works when there are no transactions", %{conn: conn} do
+    test "next_page_params exist if not on last page", %{conn: conn} do
-      conn = get(conn, pending_transaction_path(conn, :index, :en))
+      60
+      |> insert_list(:transaction)
 
-      assert html_response(conn, 200)
+      conn = get(conn, pending_transaction_path(ExplorerWeb.Endpoint, :index, :en))
+
+      assert conn.assigns.next_page_params
+    end
+
+    test "next_page_params are empty if on last page", %{conn: conn} do
+      insert(:transaction)
+
+      conn = get(conn, pending_transaction_path(ExplorerWeb.Endpoint, :index, :en))
+
+      refute conn.assigns.next_page_params
     end
   end
 end