diff --git a/gitea_saas/billing_core.py b/gitea_saas/billing_core.py index c64acf2..f8d5e87 100644 --- a/gitea_saas/billing_core.py +++ b/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.""" diff --git a/tests/test_billing_core.py b/tests/test_billing_core.py index f51fa0b..84e72a0 100644 --- a/tests/test_billing_core.py +++ b/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):