Standardize queryparam processing

Co-authored-by: Stamates <stamates@hotmail.com>
Co-authored-by: Tim Mecklem <timothy@mecklem.com>

apps/explorer_web/lib/explorer_web/controllers/address_internal_transaction_controller.ex

@@ -13,43 +13,18 @@ defmodule ExplorerWeb.AddressInternalTransactionController do
   @page_size 50
   @default_paging_options %PagingOptions{page_size: @page_size + 1}
 
-  def index(
-        conn,
-        %{
-          "block_number" => block_number_string,
-          "transaction_index" => transaction_index_string,
-          "index" => index_string
-        } = params
-      ) do
-    with {block_number, ""} <- Integer.parse(block_number_string),
-         {transaction_index, ""} <- Integer.parse(transaction_index_string),
-         {index, ""} <- Integer.parse(index_string) do
-      do_index(
-        conn,
-        Map.put(params, :paging_options, %{@default_paging_options | key: {block_number, transaction_index, 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
+  def 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
       full_options =
-        Keyword.merge(
-          [
-            necessity_by_association: %{
-              from_address: :optional,
-              to_address: :optional
-            },
-            paging_options: @default_paging_options
-          ],
-          current_filter(params)
-        )
+        [
+          necessity_by_association: %{
+            from_address: :optional,
+            to_address: :optional
+          }
+        ]
+        |> Keyword.merge(paging_options(params))
+        |> Keyword.merge(current_filter(params))
 
       internal_transactions_plus_one = Chain.address_to_internal_transactions(address, full_options)
 
@@ -101,4 +76,20 @@ defmodule ExplorerWeb.AddressInternalTransactionController do
     {:ok, last_transaction} = Chain.hash_to_transaction(last.transaction_hash)
     %{block_number: last_transaction.block_number, transaction_index: last_transaction.index, index: last.index}
   end
+
+  defp paging_options(params) do
+    with %{
+           "block_number" => block_number_string,
+           "transaction_index" => transaction_index_string,
+           "index" => index_string
+         } <- params,
+         {block_number, ""} <- Integer.parse(block_number_string),
+         {transaction_index, ""} <- Integer.parse(transaction_index_string),
+         {index, ""} <- Integer.parse(index_string) do
+      [paging_options: %{@default_paging_options | key: {block_number, transaction_index, index}}]
+    else
+      _ ->
+        [paging_options: @default_paging_options]
+    end
+  end
 end

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

@@ -13,33 +13,19 @@ defmodule ExplorerWeb.AddressTransactionController do
   @page_size 50
   @default_paging_options %PagingOptions{page_size: @page_size + 1}
 
-  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
+  def 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
       full_options =
-        Keyword.merge(
-          [
-            necessity_by_association: %{
-              block: :required,
-              from_address: :optional,
-              to_address: :optional
-            },
-            paging_options: @default_paging_options
-          ],
-          current_filter(params)
-        )
+        [
+          necessity_by_association: %{
+            block: :required,
+            from_address: :optional,
+            to_address: :optional
+          }
+        ]
+        |> Keyword.merge(paging_options(params))
+        |> Keyword.merge(current_filter(params))
 
       transactions_plus_one = Chain.address_to_transactions(address, full_options)
 
@@ -90,4 +76,15 @@ defmodule ExplorerWeb.AddressTransactionController do
     last = List.last(transactions)
     %{block_number: last.block_number, index: last.index}
   end
+
+  defp paging_options(params) do
+    with %{"block_number" => block_number_string, "index" => index_string} <- params,
+         {block_number, ""} <- Integer.parse(block_number_string),
+         {index, ""} <- Integer.parse(index_string) do
+      [paging_options: %{@default_paging_options | key: {block_number, index}}]
+    else
+      _ ->
+        [paging_options: @default_paging_options]
+    end
+  end
 end

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

@@ -6,25 +6,7 @@ defmodule ExplorerWeb.TransactionController do
   @page_size 50
   @default_paging_options %PagingOptions{page_size: @page_size + 1}
 
-  def index(conn, %{"block_number" => block_number_string, "index" => index_string}) do
-    with {block_number, ""} <- Integer.parse(block_number_string),
-         {index, ""} <- Integer.parse(index_string) do
-      do_index(conn, paging_options: %{@default_paging_options | key: {block_number, index}})
-    else
-      _ ->
-        unprocessable_entity(conn)
-    end
-  end
-
-  def index(conn, _params) do
-    do_index(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
+  def index(conn, params) do
     full_options =
       Keyword.merge(
         [
@@ -32,10 +14,9 @@ defmodule ExplorerWeb.TransactionController do
             block: :required,
             from_address: :optional,
             to_address: :optional
-          },
-          paging_options: @default_paging_options
+          }
         ],
-        options
+        paging_options(params)
       )
 
     transactions_plus_one = Chain.recent_collated_transactions(full_options)
@@ -53,10 +34,25 @@ defmodule ExplorerWeb.TransactionController do
     )
   end
 
+  def show(conn, %{"id" => id, "locale" => locale}) do
+    redirect(conn, to: transaction_internal_transaction_path(conn, :index, locale, id))
+  end
+
   defp next_page_params(nil, _transactions), do: nil
 
   defp next_page_params(_, transactions) do
     last = List.last(transactions)
     %{block_number: last.block_number, index: last.index}
   end
+
+  defp paging_options(params) do
+    with %{"block_number" => block_number_string, "index" => index_string} <- params,
+         {block_number, ""} <- Integer.parse(block_number_string),
+         {index, ""} <- Integer.parse(index_string) do
+      [paging_options: %{@default_paging_options | key: {block_number, index}}]
+    else
+      _ ->
+        [paging_options: @default_paging_options]
+    end
+  end
 end

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

@@ -89,16 +89,6 @@ defmodule ExplorerWeb.TransactionControllerTest do
       refute conn.assigns.next_page_params
     end
 
-    test "guards against bad block_number input", %{conn: conn} do
-      conn = get(conn, "/en/transactions", %{"block_number" => "foo", "index" => "2"})
-      assert html_response(conn, 422)
-    end
-
-    test "guards against bad index input", %{conn: conn} do
-      conn = get(conn, "/en/transactions", %{"block_number" => "2", "index" => "bar"})
-      assert html_response(conn, 422)
-    end
-
     test "sends back the number of transactions", %{conn: conn} do
       insert(:transaction)
       |> with_block()