Billing core client API consistency: id, flat records, list_clients (#16)

gitea_saas/billing_core.py

@@ -246,7 +246,12 @@ class BillingCore:
 
     def create_client(self, name: str, tier: str = "free", status: str = "active",
                       meta: dict | None = None) -> tuple[bool, dict]:
-        """Create a client. Duplicate names are rejected (UNIQUE)."""
+        """Create a client. Duplicate names are rejected (UNIQUE).
+
+        Returns a flat client record whose identifier key is ``id``,
+        consistent with :meth:`get_client` and :meth:`list_clients` (issue
+        #16 -- the API must not call the same concept two different names).
+        """
         name = (name or "").strip()
         if not name:
             return _err(INVALID_NAME, "client name must be a non-empty string")
@@ -263,13 +268,19 @@ class BillingCore:
                 )
             except sqlite3.IntegrityError:
                 return _err(DUPLICATE_CLIENT, f"client name already exists: {name}")
-            return _ok({"client_id": cur.lastrowid, "name": name, "tier": tier,
+            return _ok({"id": cur.lastrowid, "name": name, "tier": tier,
                         "status": status, "meta": meta, "created_at": now})
 
         return self._mutate(fn)
 
     def get_client(self, client_id: int, include_balance: bool = True) -> tuple[bool, dict]:
-        """Fetch a client. Stale holds auto-release before the balance is read."""
+        """Fetch a client as a FLAT record with ``balance`` as a field.
+
+        Stale holds auto-release before the balance is read. The record's
+        key set matches :meth:`create_client` (``id``, ``name``, ``tier``,
+        ``status``, ``meta``, ``created_at``) plus ``balance`` when
+        requested -- no nesting (issue #16).
+        """
 
         def fn() -> tuple[bool, dict]:
             row = self._conn.execute("SELECT * FROM clients WHERE id = ?", (client_id,)).fetchone()
@@ -278,13 +289,35 @@ class BillingCore:
             client = dict(row)
             client["meta"] = _load_meta(client.pop("meta_json"))
             self._expire_stale_holds(client_id)
-            details: dict = {"client": client}
+            details: dict = dict(client)
             if include_balance:
                 details["balance"] = self._balance(client_id)
             return _ok(details)
 
         return self._mutate(fn)
 
+    def list_clients(self) -> tuple[bool, dict]:
+        """List all clients as flat records (same shape as
+        :meth:`get_client` with balance), newest-id first.
+
+        Consumed by the HTTP layer's ``GET /api/clients`` (issue #2) and
+        the dashboard (issue #9); its absence surfaced as a 501.
+        """
+
+        def fn() -> tuple[bool, dict]:
+            rows = self._conn.execute(
+                "SELECT * FROM clients ORDER BY id").fetchall()
+            clients = []
+            for row in rows:
+                client = dict(row)
+                client["meta"] = _load_meta(client.pop("meta_json"))
+                self._expire_stale_holds(client["id"])
+                client["balance"] = self._balance(client["id"])
+                clients.append(client)
+            return _ok({"clients": clients})
+
+        return self._mutate(fn)
+
     def get_balance(self, client_id: int) -> tuple[bool, dict]:
         """Available balance: credits + refunds - debits - active holds."""
 

tests/test_billing_core.py

@@ -49,7 +49,7 @@ def core(tmp_path):
 def make_client(core, name="alice"):
     ok, details = core.create_client(name)
     assert ok, details
-    return details["client_id"]
+    return details["id"]
 
 
 # ---------------------------------------------------------------------------
@@ -63,7 +63,7 @@ def test_create_tables_idempotent(core, tmp_path):
     core.create_tables()
     cid = make_client(core)
     ok, details = core.get_client(cid)
-    assert ok and details["client"]["id"] == cid
+    assert ok and details["id"] == cid
 
 
 # ---------------------------------------------------------------------------
@@ -354,7 +354,7 @@ def test_get_client_include_balance_false(core):
     core.credit(cid, 100, "c1")
     ok, details = core.get_client(cid, include_balance=False)
     assert ok and "balance" not in details
-    assert details["client"]["name"] == "alice"
+    assert details["name"] == "alice"
 
 
 def test_create_client_meta_roundtrip(core):
@@ -363,10 +363,80 @@ def test_create_client_meta_roundtrip(core):
                                    meta={"plan": "pro", "seats": 3})
     assert ok
     ok, details = core.get_client(cid)
-    assert ok and details["client"]["tier"] == "free"  # default tier
-    ok, details = core.get_client(carol["client_id"])
-    assert ok and details["client"]["name"] == "carol"
-    assert details["client"]["meta"] == {"plan": "pro", "seats": 3}
+    assert ok and details["tier"] == "free"  # default tier
+    ok, details = core.get_client(carol["id"])
+    assert ok and details["name"] == "carol"
+    assert details["meta"] == {"plan": "pro", "seats": 3}
+
+
+# ---------------------------------------------------------------------------
+# Issue #16: client-record shape consistency across methods
+# ---------------------------------------------------------------------------
+
+CLIENT_RECORD_KEYS = {"id", "name", "tier", "status", "meta", "created_at"}
+
+
+def test_client_record_shape_consistent_across_methods(core):
+    """create_client / get_client / list_clients must agree on the key set.
+
+    This is the cross-method test whose absence let the shapes drift
+    (issue #16): it fails if any one method's record keys diverge.
+    """
+    cid = make_client(core)
+    core.credit(cid, 250, "c1")
+    ok, created = core.create_client("bob")
+    assert ok
+    assert set(created) == CLIENT_RECORD_KEYS
+
+    ok, fetched = core.get_client(cid)
+    assert ok
+    assert set(fetched) == CLIENT_RECORD_KEYS | {"balance"}
+    assert fetched["id"] == cid and fetched["balance"] == 250
+
+    ok, listing = core.list_clients()
+    assert ok
+    by_id = {c["id"]: c for c in listing["clients"]}
+    assert set(by_id) == {cid, created["id"]}
+    for record in listing["clients"]:
+        assert set(record) == CLIENT_RECORD_KEYS | {"balance"}
+    assert by_id[cid]["id"] == cid
+    assert by_id[cid]["name"] == "alice" and by_id[cid]["balance"] == 250
+    assert by_id[created["id"]]["name"] == "bob" and by_id[created["id"]]["balance"] == 0
+
+
+def test_list_clients_flat_records_and_order(core):
+    a = make_client(core, "alpha")
+    b = make_client(core, "beta")
+    core.credit(b, 100, "c1")
+    ok, listing = core.list_clients()
+    assert ok
+    records = listing["clients"]
+    assert [r["id"] for r in records] == [a, b]
+    assert records[1]["balance"] == 100
+    assert "meta_json" not in records[0]  # normalized to flat 'meta'
+    assert records[0]["meta"] == {}
+
+
+def test_wire_shape_has_server_allowlist_fields(core):
+    """The HTTP layer's _client_out allow-list needs id/name/tier/status/
+    meta/created_at/balance at the TOP level of every client payload --
+    the exact rows from the issue #16 defect table."""
+    cid = make_client(core)
+    core.credit(cid, 500, "c1")
+    ok, created = core.create_client("wire-probe")
+    assert ok
+    ok, fetched = core.get_client(cid)
+    assert ok
+    ok, listing = core.list_clients()
+    assert ok
+    allow = {"id", "name", "tier", "status", "meta", "created_at", "balance"}
+    # POST /api/clients row: identifier present at top level (no client_id-only).
+    assert "id" in created and "client_id" not in created
+    assert set(created) <= allow
+    # GET /api/clients/{id} row: every allow-listed field present, flat.
+    assert set(fetched) == allow
+    # GET /api/clients row: list_clients exists and yields the same shape.
+    assert set(listing["clients"][0]) == allow
 
 
 def test_ledger_entries_expose_all_columns(core):