Change to last seen paging method for address transactions

apps/explorer/lib/explorer/chain.ex

@@ -60,7 +60,6 @@ defmodule Explorer.Chain do
   """
   @type pagination :: map()
 
-  @typep direction_option :: {:direction, direction}
   @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}
@@ -135,19 +134,15 @@ defmodule Explorer.Chain do
 
   ## Options
 
-    * `:direction` - if specified, will filter transactions by address type. If `:to` is specified, only transactions
-      where the "to" address matches will be returned. Likewise, if `:from` is specified, only transactions where the
-      "from" address matches will be returned. If `:direction` is omitted, transactions either to or from the address
-      will be returned.
     * `:necessity_by_association` - use to load `t:association/0` as `:required` or `:optional`.  If an association is
       `:required`, and the `t:Explorer.Chain.Transaction.t/0` has no associated record for that association, then the
       `t:Explorer.Chain.Transaction.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 {block_number, index}) and. Results will be the transactions older than
+      the block number and index that are passed.
 
   """
-  @spec address_to_transactions(Address.t(), [
-          direction_option | necessity_by_association_option | pagination_option
-        ]) :: %Scrivener.Page{entries: [Transaction.t()]}
+  @spec address_to_transactions(Address.t(), [paging_options | necessity_by_association_option]) :: Transaction.t()
   def address_to_transactions(%Address{hash: hash}, options \\ []) when is_list(options) do
     address_hash_to_transactions(hash, options)
   end
@@ -2197,22 +2192,24 @@ defmodule Explorer.Chain do
 
   defp address_hash_to_transactions(
          %Hash{byte_count: unquote(Hash.Truncated.byte_count())} = address_hash,
-         named_arguments
+         options
        )
-       when is_list(named_arguments) do
-    direction = Keyword.get(named_arguments, :direction)
-    necessity_by_association = Keyword.get(named_arguments, :necessity_by_association, %{})
-    pagination = Keyword.get(named_arguments, :pagination, %{})
+       when is_list(options) do
+    direction = Keyword.get(options, :direction)
+    necessity_by_association = Keyword.get(options, :necessity_by_association, %{})
+    paging_options = Keyword.get(options, :paging_options, %PagingOptions{page_size: 50})
 
     Transaction
     |> load_contract_creation()
     |> select_merge([_, internal_transaction], %{
       created_contract_address_hash: internal_transaction.created_contract_address_hash
     })
-    |> join_associations(necessity_by_association)
-    |> reverse_chronologically()
     |> where_address_fields_match(address_hash, direction)
-    |> Repo.paginate(pagination)
+    |> page_transaction(paging_options)
+    |> limit(^paging_options.page_size)
+    |> order_by([transaction], desc: transaction.block_number, desc: transaction.index)
+    |> join_associations(necessity_by_association)
+    |> Repo.all()
   end
 
   @spec changes_list(params :: map, [{:for, module} | {:with, :atom}]) ::
@@ -2539,10 +2536,6 @@ defmodule Explorer.Chain do
     )
   end
 
-  defp reverse_chronologically(query) do
-    from(q in query, order_by: [desc: q.inserted_at, desc: q.hash])
-  end
-
   defp run_addresses(multi, ecto_schema_module_to_changes_list, options)
        when is_map(ecto_schema_module_to_changes_list) and is_list(options) do
     case ecto_schema_module_to_changes_list do

apps/explorer/test/explorer/chain_test.exs

@@ -3,8 +3,8 @@ defmodule Explorer.ChainTest do
 
   import Explorer.Factory
 
-  alias Explorer.{Chain, Repo, Factory}
-  alias Explorer.Chain.{Address, Block, InternalTransaction, Log, Transaction, Wei, SmartContract}
+  alias Explorer.{Chain, Factory, PagingOptions, Repo}
+  alias Explorer.Chain.{Address, Block, InternalTransaction, Log, SmartContract, Transaction, Wei}
   alias Explorer.Chain.Supply.ProofOfAuthority
 
   doctest Explorer.Chain
@@ -30,106 +30,70 @@ defmodule Explorer.ChainTest do
 
       assert Repo.aggregate(Transaction, :count, :hash) == 0
 
-      assert %Scrivener.Page{
-               entries: [],
-               page_number: 1,
-               total_entries: 0
-             } = Chain.address_to_transactions(address)
+      assert [] == Chain.address_to_transactions(address)
     end
 
     test "with from transactions" do
-      %Transaction{from_address_hash: from_address_hash, hash: transaction_hash} = insert(:transaction)
-      address = Repo.get!(Address, from_address_hash)
+      address = insert(:address)
+      transaction = insert(:transaction, from_address_hash: address.hash)
 
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^transaction_hash}],
-               page_number: 1,
-               total_entries: 1
-             } = Chain.address_to_transactions(address, direction: :from)
+      assert [transaction] == Chain.address_to_transactions(address, direction: :from)
     end
 
     test "with to transactions" do
-      %Transaction{to_address_hash: to_address_hash, hash: transaction_hash} = insert(:transaction)
-      address = Repo.get!(Address, to_address_hash)
+      address = insert(:address)
+      transaction = insert(:transaction, to_address_hash: address.hash)
 
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^transaction_hash}],
-               page_number: 1,
-               total_entries: 1
-             } = Chain.address_to_transactions(address, direction: :to)
+      assert [transaction] == Chain.address_to_transactions(address, direction: :to)
     end
 
     test "with to and from transactions and direction: :from" do
-      %Transaction{from_address: address, hash: from_transaction_hash} =
-        :transaction
-        |> insert()
-        |> Repo.preload(:from_address)
-
-      insert(:transaction, to_address: address)
+      address = insert(:address)
+      transaction = insert(:transaction, from_address_hash: address.hash)
+      insert(:transaction, to_address_hash: address.hash)
 
       # only contains "from" transaction
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^from_transaction_hash}],
-               page_number: 1,
-               total_entries: 1
-             } = Chain.address_to_transactions(address, direction: :from)
+      assert [transaction] == Chain.address_to_transactions(address, direction: :from)
     end
 
     test "with to and from transactions and direction: :to" do
-      %Transaction{from_address: address} =
-        :transaction
-        |> insert()
-        |> Repo.preload(:from_address)
-
-      %Transaction{hash: to_transaction_hash} = insert(:transaction, to_address: address)
+      address = insert(:address)
+      transaction = insert(:transaction, to_address_hash: address.hash)
+      insert(:transaction, from_address_hash: address.hash)
 
       # only contains "to" transaction
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^to_transaction_hash}],
-               page_number: 1,
-               total_entries: 1
-             } = Chain.address_to_transactions(address, direction: :to)
+      assert [transaction] == Chain.address_to_transactions(address, direction: :to)
     end
 
     test "with to and from transactions and no :direction option" do
-      %Transaction{from_address: address, hash: from_transaction_hash} =
-        :transaction
-        |> insert()
-        |> Repo.preload(:from_address)
-
-      %Transaction{hash: to_transaction_hash} = insert(:transaction, to_address: address)
+      address = insert(:address)
+      transaction1 = insert(:transaction, to_address_hash: address.hash)
+      transaction2 = insert(:transaction, from_address_hash: address.hash)
 
-      assert %Scrivener.Page{
-               entries: [
-                 %Transaction{hash: ^to_transaction_hash},
-                 %Transaction{hash: ^from_transaction_hash}
-               ],
-               page_number: 1,
-               total_entries: 2
-             } = Chain.address_to_transactions(address)
+      assert [transaction1, transaction2] == Chain.address_to_transactions(address)
     end
 
     test "with transactions can be paginated" do
       address = insert(:address)
-      transactions = insert_list(2, :transaction, to_address: address)
 
-      [%Transaction{hash: oldest_transaction_hash}, %Transaction{hash: newest_transaction_hash}] = transactions
+      second_page_hashes =
+        50
+        |> insert_list(:transaction, from_address_hash: address.hash)
+        |> with_block()
+        |> Enum.map(& &1.hash)
 
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^newest_transaction_hash}],
-               page_number: 1,
-               page_size: 1,
-               total_entries: 2,
-               total_pages: 2
-             } = Chain.address_to_transactions(address, pagination: %{page_size: 1})
+      %Transaction{block_number: block_number, index: index} =
+        :transaction
+        |> insert(from_address_hash: address.hash)
+        |> with_block()
 
-      assert %Scrivener.Page{
-               entries: [%Transaction{hash: ^oldest_transaction_hash}],
-               page_number: 2,
-               page_size: 1,
-               total_entries: 2,
-               total_pages: 2
-             } = Chain.address_to_transactions(address, pagination: %{page: 2, page_size: 1})
+      assert second_page_hashes ==
+               address
+               |> Chain.address_to_transactions(
+                 paging_options: %PagingOptions{key: {block_number, index}, page_size: 50}
+               )
+               |> Enum.map(& &1.hash)
+               |> Enum.reverse()
     end
   end
 

apps/explorer_web/lib/explorer_web/controllers/address_transaction_controller.ex

@@ -7,45 +7,77 @@ defmodule ExplorerWeb.AddressTransactionController do
 
   import ExplorerWeb.AddressController, only: [transaction_count: 1]
 
-  alias Explorer.{Chain, Market}
+  alias Explorer.{Chain, Market, PagingOptions}
   alias Explorer.ExchangeRates.Token
 
-  def index(conn, %{"address_id" => address_hash_string} = params) do
+  @default_paging_options %PagingOptions{page_size: 50}
+
+  def index(conn, %{"block_number" => block_number_string, "index" => index_string} = params) do
+    with {block_number, ""} <- Integer.parse(block_number_string),
+         {index, ""} <- Integer.parse(index_string) do
+      do_index(conn, Map.put(params, :paging_options, %{@default_paging_options | key: {block_number, index}}))
+    else
+      _ ->
+        unprocessable_entity(conn)
+    end
+  end
+
+  def index(conn, params), do: do_index(conn, params)
+
+  def do_index(conn, %{"address_id" => address_hash_string} = params) do
     with {:ok, address_hash} <- Chain.string_to_address_hash(address_hash_string),
          {:ok, address} <- Chain.hash_to_address(address_hash) do
-      options = [
-        necessity_by_association: %{
-          block: :required,
-          from_address: :optional,
-          to_address: :optional
-        },
-        pagination: params
-      ]
-
-      page =
-        Chain.address_to_transactions(
-          address,
-          Keyword.merge(options, current_filter(params))
+      full_options =
+        Keyword.merge(
+          [
+            necessity_by_association: %{
+              block: :required,
+              from_address: :optional,
+              to_address: :optional
+            },
+            paging_options: @default_paging_options
+          ],
+          current_filter(params)
         )
 
+      transactions = Chain.address_to_transactions(address, full_options)
+
       render(
         conn,
         "index.html",
         address: address,
+        earliest: earliest(transactions),
         exchange_rate: Market.get_exchange_rate(Explorer.coin()) || Token.null(),
         filter: params["filter"],
-        page: page,
+        transactions: transactions,
         transaction_count: transaction_count(address)
       )
     else
       :error ->
-        not_found(conn)
+        unprocessable_entity(conn)
 
       {:error, :not_found} ->
         not_found(conn)
     end
   end
 
+  defp earliest([]), do: nil
+
+  defp earliest(transactions) do
+    last = List.last(transactions)
+    %{block_number: last.block_number, index: last.index}
+  end
+
+  defp current_filter(%{paging_options: paging_options} = params) do
+    params
+    |> Map.get("filter")
+    |> case do
+      "to" -> [direction: :to, paging_options: paging_options]
+      "from" -> [direction: :from, paging_options: paging_options]
+      _ -> [paging_options: paging_options]
+    end
+  end
+
   defp current_filter(params) do
     params
     |> Map.get("filter")

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

@@ -93,7 +93,7 @@
             </tr>
           </thead>
           <tbody>
-            <%= for transaction <- @page do %>
+            <%= for transaction <- @transactions do %>
               <tr>
                 <td><div class="transaction__dot transaction__dot--<%= status(transaction) %>"></div></td>
                 <td>
@@ -134,20 +134,18 @@
         </table>
       </div>
     </div>
-
-    <div class="address__pagination">
-      <%= pagination_links(
-            @conn,
-            @page,
-            ["en", @conn.params["address_id"]],
-            distance: 1,
-            filter: @conn.params["filter"],
-            first: true,
-            next: Phoenix.HTML.raw("&rsaquo;"),
-            path: &address_transaction_path/5,
-            previous: Phoenix.HTML.raw("&lsaquo;"),
-            view_style: :bulma
-          ) %>
-    </div>
+    <%= if @earliest do %>
+      <%= link(
+        gettext("Older"),
+        class: "button button--secondary button--sm u-float-right mt-3",
+        to: address_transaction_path(
+          @conn,
+          :index,
+          @conn.assigns.locale,
+          @address,
+          %{"block_number" => @earliest.block_number, "index" => @earliest.index}
+        )
+      ) %>
+    <% end %>
   </section>
 </section>

apps/explorer_web/test/explorer_web/controllers/address_transaction_controller_test.exs

@@ -3,13 +3,14 @@ defmodule ExplorerWeb.AddressTransactionControllerTest do
 
   import ExplorerWeb.Router.Helpers, only: [address_transaction_path: 4]
 
+  alias Explorer.Chain.Transaction
   alias Explorer.ExchangeRates.Token
 
   describe "GET index/2" do
     test "with invalid address hash", %{conn: conn} do
       conn = get(conn, address_transaction_path(conn, :index, :en, "invalid_address"))
 
-      assert html_response(conn, 404)
+      assert html_response(conn, 422)
     end
 
     test "with valid address hash without address", %{conn: conn} do
@@ -36,8 +37,8 @@ defmodule ExplorerWeb.AddressTransactionControllerTest do
       conn = get(conn, address_transaction_path(conn, :index, :en, address))
 
       actual_transaction_hashes =
-        conn.assigns.page
-        |> Enum.map(fn transaction -> transaction.hash end)
+        conn.assigns.transactions
+        |> Enum.map(& &1.hash)
 
       assert html_response(conn, 200)
       assert Enum.member?(actual_transaction_hashes, from_transaction.hash)
@@ -53,9 +54,9 @@ defmodule ExplorerWeb.AddressTransactionControllerTest do
 
       assert html_response(conn, 200)
       assert conn.status == 200
-      assert Enum.empty?(conn.assigns.page)
+      assert Enum.empty?(conn.assigns.transactions)
       assert conn.status == 200
-      assert Enum.empty?(conn.assigns.page)
+      assert Enum.empty?(conn.assigns.transactions)
     end
 
     test "includes USD exchange rate value for address in assigns", %{conn: conn} do
@@ -65,5 +66,33 @@ defmodule ExplorerWeb.AddressTransactionControllerTest do
 
       assert %Token{} = conn.assigns.exchange_rate
     end
+
+    test "returns next page of results based on last seen transaction", %{conn: conn} do
+      address = insert(:address)
+
+      second_page_hashes =
+        50
+        |> insert_list(:transaction, from_address_hash: address.hash)
+        |> with_block()
+        |> Enum.map(& &1.hash)
+
+      %Transaction{block_number: block_number, index: index} =
+        :transaction
+        |> insert(from_address_hash: address.hash)
+        |> with_block()
+
+      conn =
+        get(conn, address_transaction_path(ExplorerWeb.Endpoint, :index, :en, address.hash), %{
+          "block_number" => Integer.to_string(block_number),
+          "index" => Integer.to_string(index)
+        })
+
+      actual_hashes =
+        conn.assigns.transactions
+        |> Enum.map(& &1.hash)
+        |> Enum.reverse()
+
+      assert second_page_hashes == actual_hashes
+    end
   end
 end