Adopt api client to audit and remove legacy mpt client

Author: Robert Segal

cli/core/accounts/containers.py

@@ -1,5 +1,4 @@
 from cli.core.accounts.app import get_active_account
-from cli.core.mpt.client import client_from_account
 from cli.core.mpt.mpt_client import create_api_mpt_client_from_account
 from dependency_injector import containers, providers
 
@@ -16,5 +15,4 @@ class AccountContainer(containers.DeclarativeContainer):
     """
 
     account = providers.Singleton(get_active_account)
-    mpt_client = providers.Singleton(client_from_account, account)
     api_mpt_client = providers.Singleton(create_api_mpt_client_from_account, account)

cli/core/mpt/client.py

@@ -2,43 +2,52 @@
 
 import typer
 from cli.core.console import console
+from mpt_api_client import MPTClient, RQLQuery
 
 
-def get_audit_trail(client: Any, record_id: str) -> dict[str, Any]:
+def get_audit_trail(client: MPTClient, record_id: str) -> dict[str, Any]:
     """Retrieve audit trail for a specific record."""
-    endpoint = f"/audit/records/{record_id}"
-    query_string = "render()&select=object,actor,details,documents,request.api.geolocation"
-
     try:
-        response = client.get(f"{endpoint}?{query_string}")
-        return response.json()
+        # TODO: Using select instead of get because render() query string isn't working with get().
+        select_fields = ["object", "actor", "details", "documents", "request.api.geolocation"]
+        audit_collection = client.audit.records.options(render=True)
+        audit_collection_filtered = audit_collection.filter(RQLQuery(id=record_id))
+        records = audit_collection_filtered.select(*select_fields).fetch_page(limit=1)
     except Exception as error:
         console.print(
             f"[red]Failed to retrieve audit trail for record {record_id}: {error!s}[/red]"
         )
+        raise typer.Exit(1) from error
+
+    if not records:
+        console.print(f"[red]No audit record found with ID {record_id}[/red]")
         raise typer.Exit(1)
 
+    return records[0].to_dict()
+
 
 def get_audit_records_by_object(
     client: Any, object_id: str, limit: int = 10
 ) -> list[dict[str, Any]]:
     """Retrieve all audit records for a specific object."""
-    endpoint = "/audit/records"
-    query_string = (
-        "render()&select=object,actor,details,documents,request.api.geolocation"
-        f"&eq(object.id,'{object_id}')&order=-timestamp&limit={limit}"
-    )
-
     try:
-        response = client.get(f"{endpoint}?{query_string}")
-        records = response.json().get("data", [])
-        if not records:
-            console.print(f"[red]No audit records found for object {object_id}[/red]")
-            raise typer.Exit(1)  # noqa: TRY301
+        select_fields = ["object", "actor", "details", "documents", "request.api.geolocation"]
+        object_id_filter = RQLQuery(object__id=object_id)
+        order_by = "-timestamp"
+        audit_records_collection = client.audit.records.options(render=True)
+        audit_records_filtered = audit_records_collection.filter(object_id_filter)
+        raw_records = (
+            audit_records_filtered.order_by(order_by).select(*select_fields).fetch_page(limit=limit)
+        )
+        records = [raw_record.to_dict() for raw_record in raw_records]
     except Exception as error:
         console.print(
             f"[red]Failed to retrieve audit records for object {object_id}: {error!s}[/red]"
         )
+        raise typer.Exit(1) from error
+
+    if not records:
+        console.print(f"[red]No audit records found for object {object_id}[/red]")
         raise typer.Exit(1)
 
     return records

cli/plugins/audit_plugin/api.py

@@ -3,7 +3,7 @@
 import typer
 from cli.core.accounts.app import get_active_account
 from cli.core.console import console
-from cli.core.mpt.client import client_from_account
+from cli.core.mpt.mpt_client import create_api_mpt_client_from_account
 from cli.plugins.audit_plugin.api import get_audit_records_by_object, get_audit_trail
 from cli.plugins.audit_plugin.audit_records import (
     display_audit_records,
@@ -92,7 +92,7 @@ def diff_by_object_id(
 
     """
     account = get_active_account()
-    client = client_from_account(account)
+    client = create_api_mpt_client_from_account(account)
 
     records = get_audit_records_by_object(client, object_id, limit)
     if len(records) < 2:
@@ -139,7 +139,7 @@ def diff_by_records_id(
 
     """
     account = get_active_account()
-    client = client_from_account(account)
+    client = create_api_mpt_client_from_account(account)
 
     source_trail = get_audit_trail(client, source)
     target_trail = get_audit_trail(client, target)

cli/plugins/audit_plugin/app.py

@@ -2,7 +2,6 @@
 import responses
 from cli.core.accounts.containers import AccountContainer
 from cli.core.accounts.models import Account as CLIAccount
-from cli.core.mpt.client import MPTClient as LegacyMPTClient
 from mpt_api_client import MPTClient
 
 
@@ -13,11 +12,6 @@ def requests_mocker():
         yield rsps
 
 
-@pytest.fixture
-def mpt_client():
-    return LegacyMPTClient("https://example.com", "token")
-
-
 @pytest.fixture
 def mock_mpt_api_client(mocker):
     return mocker.MagicMock(spec=MPTClient)
@@ -96,7 +90,6 @@ def active_vendor_account():
 def account_container_mock(mocker, active_operations_account):
     container = AccountContainer()
     container.account.override(mocker.MagicMock(return_value=active_operations_account))
-    container.mpt_client.override(mocker.MagicMock(spec=LegacyMPTClient))
     container.api_mpt_client.override(mocker.MagicMock(spec=MPTClient))
     mock = mocker.patch("cli.core.products.app.AccountContainer", autospec=True)
     mock.return_value = container

tests/cli/core/conftest.py

@@ -21,13 +21,8 @@ def mock_basic_config(**kwargs):
         root_logger.debug("Test debug message")
 
 
-def test_log_file_writes_to_file(tmp_path, mocker, active_vendor_account):
+def test_log_file_writes_to_file(tmp_path, mocker):
     mocker.patch("cli.core.accounts.app.list_accounts", return_value=[], autospec=True)
-    mocker.patch(
-        "cli.core.mpt.client.client_from_account",
-        return_value=active_vendor_account,
-        autospec=True,
-    )
     log_file = tmp_path / "test.log"
     mocker.patch("logging.basicConfig", side_effect=mock_basic_config, autospec=True)
 
@@ -47,13 +42,8 @@ def test_verbose_and_log_file_are_exclusive():
     assert "Cannot use both --verbose and --log-file together" in result.stdout
 
 
-def test_log_file_creates_parent_directories(tmp_path, mocker, active_vendor_account):
+def test_log_file_creates_parent_directories(tmp_path, mocker):
     mocker.patch("cli.core.accounts.app.list_accounts", return_value=[], autospec=True)
-    mocker.patch(
-        "cli.core.mpt.client.client_from_account",
-        return_value=active_vendor_account,
-        autospec=True,
-    )
     mocker.patch("logging.basicConfig", side_effect=mock_basic_config, autospec=True)
     nested_log_file = tmp_path / "nested" / "dir" / "test.log"