From 6797e66b523593bf13a76e033f2697131f32bf45 Mon Sep 17 00:00:00 2001
From: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
Date: Thu, 6 Jun 2024 10:11:51 -0500
Subject: [PATCH] fix: update for API route change (#47)

* fix: update for API route change

* refactor: allow using address or alias

* fix: `/all-transactions` includes incoming transactions now (skip them)

* fix: just use page request `next` urls directly

* fix: accidentally was skipping ExecutedTxData because it was a subclass
---
 ape_safe/_cli/safe_mgmt.py  |  8 ++++++--
 ape_safe/client/__init__.py | 16 +++++++++-------
 ape_safe/client/base.py     | 14 +++++++++-----
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/ape_safe/_cli/safe_mgmt.py b/ape_safe/_cli/safe_mgmt.py
index 6e8e7d1..54700ff 100644
--- a/ape_safe/_cli/safe_mgmt.py
+++ b/ape_safe/_cli/safe_mgmt.py
@@ -132,12 +132,16 @@ def remove(cli_ctx: SafeCliContext, safe, skip_confirmation):
 
 @click.command(cls=ConnectedProviderCommand)
 @safe_cli_ctx()
-@click.argument("address", type=ChecksumAddress)
+@click.argument("account")
 @click.option("--confirmed", is_flag=True, default=None)
-def all_txns(cli_ctx: SafeCliContext, address, confirmed):
+def all_txns(cli_ctx: SafeCliContext, account, confirmed):
     """
     View and filter all transactions for a given Safe using Safe API
     """
+    if account in cli_ctx.account_manager.aliases:
+        account = cli_ctx.account_manager.load(account)
+
+    address = cli_ctx.conversion_manager.convert(account, AddressType)
 
     # NOTE: Create a client to support non-local safes.
     client = cli_ctx.safes.create_client(address)
diff --git a/ape_safe/client/__init__.py b/ape_safe/client/__init__.py
index 8bb6b6f..fb72949 100644
--- a/ape_safe/client/__init__.py
+++ b/ape_safe/client/__init__.py
@@ -77,22 +77,24 @@ def get_next_nonce(self) -> int:
 
     def _all_transactions(self) -> Iterator[SafeApiTxData]:
         """
-        confirmed: Confirmed if True, not confirmed if False, both if None
+        Get all transactions from safe, both confirmed and unconfirmed
         """
 
-        url = f"safes/{self.address}/transactions"
+        url = f"safes/{self.address}/all-transactions"
         while url:
             response = self._get(url)
             data = response.json()
 
             for txn in data.get("results"):
-                # TODO: Replace with `model_validate()` after ape 0.7.
                 # NOTE: Using construct because of pydantic v2 back import validation error.
-                if "isExecuted" in txn and txn["isExecuted"]:
-                    yield ExecutedTxData.model_validate(txn)
+                if "isExecuted" in txn:
+                    if txn["isExecuted"]:
+                        yield ExecutedTxData.model_validate(txn)
 
-                else:
-                    yield UnexecutedTxData.model_validate(txn)
+                    else:
+                        yield UnexecutedTxData.model_validate(txn)
+
+                # else it is an incoming transaction
 
             url = data.get("next")
 
diff --git a/ape_safe/client/base.py b/ape_safe/client/base.py
index 08e23e6..ad50fa2 100644
--- a/ape_safe/client/base.py
+++ b/ape_safe/client/base.py
@@ -96,7 +96,8 @@ def get_transactions(
                 elif confirmed and not is_confirmed:
                     continue  # NOTE: Skip not confirmed transactions
 
-            if txn.nonce < next_nonce and isinstance(txn, UnexecutedTxData):
+            # NOTE: use `type(txn) is ...` because ExecutedTxData is a subclass of UnexecutedTxData
+            if txn.nonce < next_nonce and type(txn) is UnexecutedTxData:
                 continue  # NOTE: Skip orphaned transactions
 
             if filter_by_ids and txn.safe_tx_hash not in filter_by_ids:
@@ -136,10 +137,13 @@ def _http(self):
         return urllib3.PoolManager(ca_certs=certifi.where())
 
     def _request(self, method: str, url: str, json: Optional[dict] = None, **kwargs) -> Response:
-        # **WARNING**: The trailing slash in the URL is CRITICAL!
-        # If you remove it, things will not work as expected.
-
-        api_url = f"{self.transaction_service_url}/api/v1/{url}/"
+        # NOTE: paged requests include full url already
+        if url.startswith(f"{self.transaction_service_url}/api/v1/"):
+            api_url = url
+        else:
+            # **WARNING**: The trailing slash in the URL is CRITICAL!
+            # If you remove it, things will not work as expected.
+            api_url = f"{self.transaction_service_url}/api/v1/{url}/"
         do_fail = not kwargs.pop("allow_failure", False)
 
         # Use `or 10` to handle when None is explicit.