Use redirect pattern for transaction->internal transaction list

Co-authored-by: jimmay5469 <jimmay5469@gmail.com>

apps/explorer/lib/explorer/chain/internal_transaction.ex

@@ -22,7 +22,7 @@ defmodule Explorer.Chain.InternalTransaction do
    * `to_address_hash` - hash of the sink of the `value`
    * `trace_address` - list of traces
    * `transaction` - transaction in which this transaction occured
-   * `transaction_id` - foreign key for `transaction`
+   * `transaction_hash` - foreign key for `transaction`
    * `type` - type of internal transaction
    * `value` - value of transfered from `from_address` to `to_address`
   """

apps/explorer_web/lib/explorer_web/controllers/transaction_controller.ex

@@ -12,38 +12,8 @@ defmodule ExplorerWeb.TransactionController do
     end
   end
 
-  def show(conn, %{"id" => hash_string}) do
-    with {:ok, hash} <- Chain.string_to_transaction_hash(hash_string),
-         {:ok, transaction} <-
-           Chain.hash_to_transaction(
-             hash,
-             necessity_by_association: %{
-               block: :optional,
-               from_address: :optional,
-               to_address: :optional,
-               receipt: :optional
-             }
-           ) do
-      internal_transactions =
-        Chain.transaction_hash_to_internal_transactions(
-          transaction.hash,
-          necessity_by_association: %{from_address: :required, to_address: :optional}
-        )
-
-      render(
-        conn,
-        "show.html",
-        internal_transactions: internal_transactions,
-        max_block_number: max_block_number(),
-        transaction: transaction
-      )
-    else
-      :error ->
-        not_found(conn)
-
-      {:error, :not_found} ->
-        not_found(conn)
-    end
+  def show(conn, %{"id" => id, "locale" => locale}) do
+    redirect(conn, to: transaction_internal_transaction_path(conn, :index, locale, id))
   end
 
   defp do_index(conn, options \\ []) when is_list(options) do
@@ -78,11 +48,4 @@ defmodule ExplorerWeb.TransactionController do
   defp last_seen_collated_hash(transactions) do
     List.last(transactions).hash
   end
-
-  defp max_block_number do
-    case Chain.max_block_number() do
-      {:ok, number} -> number
-      {:error, :not_found} -> 0
-    end
-  end
 end

apps/explorer_web/lib/explorer_web/controllers/transaction_internal_transaction_controller.ex

@@ -0,0 +1,48 @@
+defmodule ExplorerWeb.TransactionInternalTransactionController do
+  use ExplorerWeb, :controller
+
+  alias Explorer.Chain
+
+  def index(conn, %{"transaction_id" => hash_string}) do
+    with {:ok, hash} <- Chain.string_to_transaction_hash(hash_string),
+         {:ok, transaction} <-
+           Chain.hash_to_transaction(
+             hash,
+             necessity_by_association: %{
+               block: :optional,
+               from_address: :optional,
+               to_address: :optional,
+               receipt: :optional
+             }
+           ) do
+      internal_transactions =
+        Chain.transaction_hash_to_internal_transactions(
+          transaction.hash,
+          necessity_by_association: %{from_address: :required, to_address: :optional}
+        )
+
+      max_block_number = max_block_number()
+
+      render(
+        conn,
+        "index.html",
+        internal_transactions: internal_transactions,
+        max_block_number: max_block_number,
+        transaction: transaction
+      )
+    else
+      :error ->
+        not_found(conn)
+
+      {:error, :not_found} ->
+        not_found(conn)
+    end
+  end
+
+  defp max_block_number do
+    case Chain.max_block_number() do
+      {:ok, number} -> number
+      {:error, :not_found} -> 0
+    end
+  end
+end

apps/explorer_web/lib/explorer_web/router.ex

@@ -40,6 +40,13 @@ defmodule ExplorerWeb.Router do
     resources("/pending_transactions", PendingTransactionController, only: [:index])
 
     resources "/transactions", TransactionController, only: [:index, :show] do
+      resources(
+        "/internal_transactions",
+        TransactionInternalTransactionController,
+        only: [:index],
+        as: :internal_transaction
+      )
+
       resources("/logs", TransactionLogController, only: [:index], as: :log)
     end
 

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

@@ -1,5 +1,5 @@
 <section class="container__section">
-  <%= render "overview.html", assigns %>
+  <%= render ExplorerWeb.TransactionView, "overview.html", assigns %>
   <div>
     <ul class="nav nav-tabs">
       <li class="nav-item">

apps/explorer_web/lib/explorer_web/views/transaction_internal_transaction_view.ex

@@ -0,0 +1,9 @@
+defmodule ExplorerWeb.TransactionInternalTransactionView do
+  use ExplorerWeb, :view
+  @dialyzer :no_match
+
+  alias ExplorerWeb.TransactionView
+
+  defdelegate value(txn, opts), to: TransactionView
+  defdelegate gas(txn), to: TransactionView
+end

apps/explorer_web/test/explorer_web/controllers/transaction_controller_test.exs

@@ -1,7 +1,7 @@
 defmodule ExplorerWeb.TransactionControllerTest do
   use ExplorerWeb.ConnCase
 
-  import ExplorerWeb.Router.Helpers, only: [transaction_path: 4]
+  import ExplorerWeb.Router.Helpers, only: [transaction_path: 4, transaction_internal_transaction_path: 4]
 
   describe "GET index/2" do
     test "returns a transaction with a receipt", %{conn: conn} do
@@ -73,59 +73,12 @@ defmodule ExplorerWeb.TransactionControllerTest do
   end
 
   describe "GET show/3" do
-    test "with invalid transaction hash", %{conn: conn} do
-      conn = get(conn, transaction_path(conn, :show, :en, "invalid_transaction_hash"))
+    test "redirects to transactions/:transaction_id/internal_transactions", %{conn: conn} do
+      locale = "en"
+      hash = "0x9"
+      conn = get(conn, transaction_path(ExplorerWeb.Endpoint, :show, locale, hash))
 
-      assert html_response(conn, 404)
-    end
-
-    test "with valid transaction hash without transaction", %{conn: conn} do
-      conn =
-        get(
-          conn,
-          transaction_path(conn, :show, :en, "0x3a3eb134e6792ce9403ea4188e5e79693de9e4c94e499db132be086400da79e6")
-        )
-
-      assert html_response(conn, 404)
-    end
-
-    test "when there is an associated block, it returns a transaction with block data", %{
-      conn: conn
-    } do
-      block = insert(:block, %{number: 777})
-      transaction = insert(:transaction, block_hash: block.hash, index: 0)
-
-      conn = get(conn, transaction_path(conn, :show, :en, transaction))
-
-      assert html_response(conn, 200)
-      assert transaction.hash == conn.assigns.transaction.hash
-      assert block.number == conn.assigns.transaction.block.number
-    end
-
-    test "returns a transaction without associated block data", %{conn: conn} do
-      transaction = insert(:transaction)
-
-      conn = get(conn, transaction_path(conn, :show, :en, transaction))
-
-      assert html_response(conn, 200)
-      assert transaction.hash == conn.assigns.transaction.hash
-    end
-
-    test "returns internal transactions for the transaction", %{conn: conn} do
-      transaction = insert(:transaction)
-      expected_internal_transaction = insert(:internal_transaction, transaction_hash: transaction.hash, index: 0)
-      insert(:internal_transaction, transaction_hash: transaction.hash, index: 1)
-
-      path = transaction_path(ExplorerWeb.Endpoint, :show, :en, transaction)
-
-      conn = get(conn, path)
-
-      actual_internal_transaction_ids =
-        conn.assigns.internal_transactions.entries
-        |> Enum.map(fn it -> it.id end)
-
-      assert conn.assigns.transaction.hash == transaction.hash
-      assert Enum.member?(actual_internal_transaction_ids, expected_internal_transaction.id)
+      assert redirected_to(conn) =~ transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, locale, hash)
     end
   end
 end

apps/explorer_web/test/explorer_web/controllers/transaction_internal_transaction_controller_test.exs

@@ -0,0 +1,45 @@
+defmodule ExplorerWeb.TransactionInternalTransactionControllerTest do
+  use ExplorerWeb.ConnCase
+
+  import ExplorerWeb.Router.Helpers, only: [transaction_internal_transaction_path: 4]
+
+  describe "GET index/3" do
+    test "without transaction", %{conn: conn} do
+      conn = get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, "nope"))
+
+      assert html_response(conn, 404)
+    end
+
+    test "includes transaction data", %{conn: conn} do
+      block = insert(:block, %{number: 777})
+
+      transaction =
+        :transaction
+        |> insert()
+        |> with_block(block)
+
+      conn = get(conn, transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash))
+
+      assert html_response(conn, 200)
+      assert conn.assigns.transaction.hash == transaction.hash
+    end
+
+    test "includes internal transactions for the transaction", %{conn: conn} do
+      transaction = insert(:transaction)
+      expected_internal_transaction = insert(:internal_transaction, transaction_hash: transaction.hash, index: 0)
+      insert(:internal_transaction, transaction_hash: transaction.hash, index: 1)
+
+      path = transaction_internal_transaction_path(ExplorerWeb.Endpoint, :index, :en, transaction.hash)
+
+      conn = get(conn, path)
+
+      actual_internal_transaction_ids =
+        conn.assigns.internal_transactions.entries
+        |> Enum.map(fn it -> it.id end)
+
+      assert html_response(conn, 200)
+
+      assert Enum.member?(actual_internal_transaction_ids, expected_internal_transaction.id)
+    end
+  end
+end

apps/explorer_web/test/support/conn_case.ex

@@ -25,6 +25,7 @@ defmodule ExplorerWeb.ConnCase do
       @endpoint ExplorerWeb.Endpoint
 
       import Explorer.Factory
+      import ExplorerWeb.Factory
     end
   end
 

apps/explorer_web/test/support/factory.ex

@@ -28,6 +28,9 @@ defmodule ExplorerWeb.Factory do
         where: transaction.block_hash == ^block_hash
       )
 
-    Repo.one!(query) + 1
+    case Repo.one(query) do
+      nil -> 0
+      index -> index + 1
+    end
   end
 end