From 49d44039c22494c139f50512a5b23cc6378363fb Mon Sep 17 00:00:00 2001 From: Sergio Alexander Florez Galeano Date: Tue, 14 Jul 2026 13:04:35 +0000 Subject: [PATCH] fix(kudos): parse the real wall-of-fame payload and render the leaderboard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `kudos wall-of-fame` showed "Top receiver —" / "Top giver —" and a meaningless "Leaderboard entries 4" because the renderer misread the API payload on three counts. ## Change Log - top_receiver / top_giver: read the name from the nested user.full_name (the old code looked for a top-level full_name that never exists) - leaderboard: unwrap the { count, next, previous, results } envelope (the old code did len() on the dict, counting its 4 keys) - leaderboard_summary: surface it as "Your position: N of M" instead of overwriting it with a local count - Render the ranked leaderboard table and the company-values (kudos DNA) distribution, both previously dropped on the floor - Move all rendering into display.py::print_kudos_wall_of_fame (rule 9); the command callback now only dispatches JSON vs. human output - Rewrite the wall-of-fame test fixture to mirror the real payload shape (the old fixture was flat, which is why the bug shipped) + 2 new tests - Document the response shape in docs/API_REFERENCE.md ## Risks - None for --json users: the payload passthrough is unchanged - Human output changes shape (panel + 2 tables instead of a 3-row panel), which is the point of the fix Co-Authored-By: Claude Fable 5 --- dailybot_cli/commands/kudos.py | 19 +----- dailybot_cli/display.py | 106 ++++++++++++++++++++++++++++++ docs/API_REFERENCE.md | 23 ++++++- tests/kudos_read_commands_test.py | 80 ++++++++++++++++++++-- 4 files changed, 205 insertions(+), 23 deletions(-) diff --git a/dailybot_cli/commands/kudos.py b/dailybot_cli/commands/kudos.py index be16df2..828dceb 100644 --- a/dailybot_cli/commands/kudos.py +++ b/dailybot_cli/commands/kudos.py @@ -20,10 +20,10 @@ from dailybot_cli.commands.query_options import build_query_params, query_options, resolve_fetch_all from dailybot_cli.display import ( console, - print_detail_panel, print_error, print_kudos_result, print_kudos_table, + print_kudos_wall_of_fame, print_pagination_footer, ) @@ -394,19 +394,4 @@ def kudos_wall_of_fame(limit: int | None, json_mode: bool) -> None: if json_mode: emit_json(data) return - leaderboard: Any = data.get("leaderboard") or [] - fields: list[tuple[str, str]] = [ - ("Top receiver", "top_receiver"), - ("Top giver", "top_giver"), - ("Leaderboard entries", "leaderboard_summary"), - ] - summary: dict[str, Any] = { - "top_receiver": (data.get("top_receiver") or {}).get("full_name") - if isinstance(data.get("top_receiver"), dict) - else data.get("top_receiver"), - "top_giver": (data.get("top_giver") or {}).get("full_name") - if isinstance(data.get("top_giver"), dict) - else data.get("top_giver"), - "leaderboard_summary": len(leaderboard), - } - print_detail_panel("Kudos — Wall of Fame", summary, fields) + print_kudos_wall_of_fame(data) diff --git a/dailybot_cli/display.py b/dailybot_cli/display.py index ded7039..0948985 100644 --- a/dailybot_cli/display.py +++ b/dailybot_cli/display.py @@ -64,6 +64,112 @@ def print_kudos_table(kudos: list[dict[str, Any]]) -> None: console.print(table) +def _kudos_person_name(entry: Any) -> str | None: + """Extract a display name from a wall-of-fame person entry. + + The API nests the person under ``user`` (``{"user": {"full_name": ...}, + "kudos_received": ...}``); a top-level ``full_name`` is accepted as a + fallback so partial payloads degrade to a dash instead of crashing. + """ + if not isinstance(entry, dict): + return None + user: Any = entry.get("user") + if isinstance(user, dict) and user.get("full_name"): + return str(user["full_name"]) + name: Any = entry.get("full_name") + return str(name) if name else None + + +def print_kudos_wall_of_fame(data: dict[str, Any]) -> None: + """Render the kudos wall of fame: top receiver/giver, caller position, + company-values distribution, and the ranked leaderboard. + + ``GET /v1/kudos/wall-of-fame/`` returns ``top_receiver`` / ``top_giver`` + with the person nested under ``user``, the leaderboard wrapped in a + ``{count, next, previous, results}`` envelope, and the caller's own + standing in ``leaderboard_summary`` (``{position, total}``). + """ + top_receiver: Any = data.get("top_receiver") + top_giver: Any = data.get("top_giver") + receiver_name: str | None = _kudos_person_name(top_receiver) + giver_name: str | None = _kudos_person_name(top_giver) + if receiver_name and isinstance(top_receiver, dict): + received: Any = top_receiver.get("kudos_received") + if received is not None: + receiver_name = f"{receiver_name} ({received} received)" + if giver_name and isinstance(top_giver, dict): + given: Any = top_giver.get("kudos_given") + if given is not None: + giver_name = f"{giver_name} ({given} given)" + + summary_raw: Any = data.get("leaderboard_summary") + position: str | None = None + if isinstance(summary_raw, dict) and summary_raw.get("position") is not None: + total_ranked: Any = summary_raw.get("total") + position = ( + f"{summary_raw['position']} of {total_ranked}" + if total_ranked + else str(summary_raw["position"]) + ) + + print_detail_panel( + "Kudos — Wall of Fame", + {"top_receiver": receiver_name, "top_giver": giver_name, "position": position}, + [ + ("Top receiver", "top_receiver"), + ("Top giver", "top_giver"), + ("Your position", "position"), + ], + ) + + dna: Any = data.get("dna_distribution") + if isinstance(dna, list) and dna: + dna_table: Table = Table(title="Company values (kudos DNA)") + dna_table.add_column("Value", style="cyan") + dna_table.add_column("Kudos", justify="right") + dna_table.add_column("%", justify="right", style="dim") + for item in dna: + value_raw: Any = item.get("company_value") if isinstance(item, dict) else None + value: dict[str, Any] = value_raw if isinstance(value_raw, dict) else {} + label: str = f"{value.get('emoji', '')} {value.get('value', '—')}".strip() + dna_table.add_row(label, str(item.get("count", "—")), str(item.get("percentage", "—"))) + console.print(dna_table) + + board_raw: Any = data.get("leaderboard") + if isinstance(board_raw, dict): + entries: list[Any] = list(board_raw.get("results") or []) + total: int | None = ( + board_raw.get("count") if isinstance(board_raw.get("count"), int) else None + ) + else: + entries = list(board_raw or []) + total = len(entries) + if not entries: + console.print("[dim]No leaderboard entries.[/dim]") + return + table: Table = Table(title="Leaderboard") + table.add_column("#", justify="right", style="dim") + table.add_column("Name", style="cyan") + table.add_column("Kudos", justify="right") + table.add_column("+Kudos", justify="right") + for entry in entries: + if not isinstance(entry, dict): + continue + table.add_row( + str(entry.get("position", "—")), + _kudos_person_name(entry) or "—", + str(entry.get("score", "—")), + str(entry.get("total_plus_kudos", "—")), + ) + console.print(table) + print_pagination_footer( + len(entries), + total, + has_more=total is not None and total > len(entries), + more_hint="raise --limit to fetch more", + ) + + def print_workflows_table(workflows: list[dict[str, Any]]) -> None: """Render a compact table of workflows (name, trigger, active, runs).""" if not workflows: diff --git a/docs/API_REFERENCE.md b/docs/API_REFERENCE.md index d84c67c..8bff231 100644 --- a/docs/API_REFERENCE.md +++ b/docs/API_REFERENCE.md @@ -301,7 +301,26 @@ Browse every kudos in the organization via `GET /v1/kudos/organization/` — the #### `dailybot kudos wall-of-fame [--limit N] [--json]` -Leaderboard of top kudos recipients via `GET /v1/kudos/wall-of-fame/`. `--limit` caps the number of entries returned. +Leaderboard of top kudos recipients via `GET /v1/kudos/wall-of-fame/`. `--limit` caps the number of leaderboard entries returned. + +The response is a stats object (not a list envelope) with this shape — note that every person is **nested under a `user` key**, and the leaderboard is itself a paginated envelope: + +```json +{ + "top_receiver": { "user": { "uuid": "…", "full_name": "…", "image": "…" }, + "kudos_received": 11, "kudos_given": 5, + "total_plus_kudos_received": 18, "total_plus_kudos_given": 2 }, + "top_giver": { "user": { … }, "kudos_given": 14, … }, + "dna_distribution": [ { "company_value": { "id": "…", "value": "…", "emoji": "…" }, + "count": 10, "percentage": 38.5 } ], + "leaderboard": { "count": 11, "next": false, "previous": false, + "results": [ { "position": 1, "user": { … }, + "score": 11, "total_plus_kudos": 18 } ] }, + "leaderboard_summary": { "position": 1, "total": 11 } +} +``` + +`leaderboard_summary` is the **caller's own standing** (`position` out of `total` ranked members). The human rendering (`display.py::print_kudos_wall_of_fame`) shows the top receiver/giver with their counts, the caller's position, the company-values (kudos DNA) distribution, and the ranked leaderboard table; `--json` emits the payload verbatim. --- @@ -653,7 +672,7 @@ key, so all of these commands work with `DAILYBOT_API_KEY` set even without | `POST` | `/v1/kudos/` | `{ content, receivers: [...uuid], users_receivers?: [...], teams_receivers?: [...], company_value? }` | `{ uuid }` | `receivers` = users+teams merged (validation); `users_receivers`/`teams_receivers` drive team expansion. Payload contract is being reconciled server-side — see the integration prompt. 406 = daily limit | | `GET` | `/v1/kudos/` | `?filter=kudos_received\|kudos_given` (the CLI accepts `received`/`given` and the `KUDOS_*` forms and normalizes them), shared list params (all optional) | `{ count, next, previous, results }` | `kudos list` | | `GET` | `/v1/kudos/organization/` | list flags | `{count, next, previous, results}` | `kudos org`; admin-only (Bearer or X-API-KEY) | -| `GET` | `/v1/kudos/wall-of-fame/` | `?limit` (optional) | `{ count, next, previous, results }` | `kudos wall-of-fame` | +| `GET` | `/v1/kudos/wall-of-fame/` | `?limit` (optional) | `{ top_receiver, top_giver, dna_distribution, leaderboard: { count, next, previous, results }, leaderboard_summary }` | `kudos wall-of-fame` | | `GET` | `/v1/workflows/` | shared list params (all optional) | `{ count, next, previous, results }` | `workflow list`; plan-gated (403 `plan_upgrade_required`) | | `GET` | `/v1/workflows//` | — | `{ uuid, name, ... }` | `workflow get`; plan-gated | diff --git a/tests/kudos_read_commands_test.py b/tests/kudos_read_commands_test.py index 7065904..d6ae303 100644 --- a/tests/kudos_read_commands_test.py +++ b/tests/kudos_read_commands_test.py @@ -77,16 +77,88 @@ def test_kudos_org_forwards_filters(monkeypatch: Any) -> None: assert kwargs["fetch_all"] is False -def test_kudos_wall_of_fame(monkeypatch: Any) -> None: +# Mirrors the real GET /v1/kudos/wall-of-fame/ payload: people are nested under +# "user", the leaderboard is a paginated envelope, and "leaderboard_summary" is +# the caller's own position. +_WALL_OF_FAME_PAYLOAD: dict[str, Any] = { + "top_receiver": { + "user": {"uuid": "u-1", "full_name": "Zoe", "image": ""}, + "kudos_received": 11, + "kudos_given": 5, + "total_plus_kudos_received": 18, + "total_plus_kudos_given": 2, + }, + "top_giver": { + "user": {"uuid": "u-2", "full_name": "Gus", "image": ""}, + "kudos_given": 14, + "kudos_received": 3, + "total_plus_kudos_given": 1, + "total_plus_kudos_received": 9, + }, + "dna_distribution": [ + { + "company_value": {"id": "v-1", "value": "Cares deeply", "emoji": "❤️"}, + "count": 10, + "percentage": 38.5, + } + ], + "leaderboard": { + "count": 11, + "next": False, + "previous": False, + "results": [ + { + "position": 1, + "user": {"uuid": "u-1", "full_name": "Zoe", "image": ""}, + "score": 11, + "total_plus_kudos": 18, + }, + { + "position": 2, + "user": {"uuid": "u-3", "full_name": "Sam", "image": ""}, + "score": 7, + "total_plus_kudos": 8, + }, + ], + }, + "leaderboard_summary": {"position": 1, "total": 11}, +} + + +def test_kudos_wall_of_fame_renders_real_payload(monkeypatch: Any) -> None: + """Top receiver/giver come from the nested ``user`` object, the leaderboard + from the ``{count, next, previous, results}`` envelope.""" + client = _client(monkeypatch) + client.get_kudos_wall_of_fame.return_value = _WALL_OF_FAME_PAYLOAD + result = CliRunner().invoke(cli, ["kudos", "wall-of-fame", "--limit", "5"]) + assert result.exit_code == 0 + assert "Zoe" in result.output # top receiver (nested user.full_name) + assert "Gus" in result.output # top giver (nested user.full_name) + assert "Sam" in result.output # leaderboard entry + assert "1 of 11" in result.output # caller position from leaderboard_summary + assert "Showing 2 of 11" in result.output # envelope count, not dict-key count + assert client.get_kudos_wall_of_fame.call_args[1]["limit"] == 5 + + +def test_kudos_wall_of_fame_tolerates_partial_payload(monkeypatch: Any) -> None: + """Missing/flat fields degrade to dashes instead of crashing.""" client = _client(monkeypatch) client.get_kudos_wall_of_fame.return_value = { "top_receiver": {"full_name": "Zoe"}, - "leaderboard": [1, 2, 3], + "leaderboard": [{"position": 1, "user": {"full_name": "Zoe"}, "score": 3}], } - result = CliRunner().invoke(cli, ["kudos", "wall-of-fame", "--limit", "5"]) + result = CliRunner().invoke(cli, ["kudos", "wall-of-fame"]) assert result.exit_code == 0 assert "Zoe" in result.output - assert client.get_kudos_wall_of_fame.call_args[1]["limit"] == 5 + + +def test_kudos_wall_of_fame_json_passthrough(monkeypatch: Any) -> None: + client = _client(monkeypatch) + client.get_kudos_wall_of_fame.return_value = _WALL_OF_FAME_PAYLOAD + result = CliRunner().invoke(cli, ["kudos", "wall-of-fame", "--json"]) + assert result.exit_code == 0 + assert '"leaderboard_summary"' in result.output + assert '"full_name": "Gus"' in result.output def test_kudos_org_api_error(monkeypatch: Any) -> None: