fix(memory-admin): pass RBAC user_groups (not YAML config) to /corporate-memory/admin (#253)

* fix(memory-admin): pass RBAC user_groups (not YAML config) to /corporate-memory/admin

`GET /corporate-memory/admin` was passing `corporate_memory.groups` from
instance.yaml (a dict, default `{}`) as the template's `groups=`. The
template's `renderItemCard` does `GROUPS.map(g => ...)` to build the
mandate-form audience picker, which throws `{}.map is not a function`
the moment any pending item renders. The thrown TypeError bubbles up
through `renderReviewItems` and gets swallowed by the `loadReviewQueue`
catch block, which paints "Error loading pending items." over a
perfectly valid `/api/memory/admin/pending` response.

Bug was dormant since the initial system commit because `renderItemCard`
only runs when at least one pending item exists, so test fixtures and
empty queues never tripped it.

Mandate form actually targets RBAC user_groups (audience strings like
`group:Admin`), not the unrelated `corporate_memory.groups` YAML
section. Route now passes the user_groups list shaped as
`[{name, members_count}]`. Template additionally guards the `.map`
call with `Array.isArray(GROUPS) ? GROUPS : []` so a future shape
regression degrades to "no group options" instead of crashing the list.

No DB migration; no API change.

* test(memory-admin): assert /corporate-memory/admin renders GROUPS as array

Server-side regression for the bug Vojta's previous commit fixed: the route
used to pass `corporate_memory.groups` (a dict, default `{}`) into the
template as `groups=`, so `const GROUPS = {{ groups | tojson }}` rendered
as `{}` and `GROUPS.map(...)` inside renderItemCard threw at runtime.

The bug was dormant because renderItemCard only runs when at least one
pending item exists — empty queues never tripped it. Test seeds one
pending knowledge_item and asserts the rendered HTML contains
`const GROUPS = [` (array prefix), so any future regression that flips
the variable back to a dict shape fails immediately rather than waiting
for an operator to seed the queue and notice the broken admin page.

* release: 0.51.1 — corporate-memory admin pending-banner hotfix

Patch bump: ships the /corporate-memory/admin GROUPS-shape fix
(dict → array of {name, members_count}) and its server-side regression
test. No DB migration, no API change, no operator action required —
upgrades land transparently.

---------

Co-authored-by: ZdenekSrotyr <zdenek.srotyr@keboola.com>
This commit is contained in:
Vojtech 2026-05-12 17:16:01 +04:00 committed by GitHub
parent cd0a97d269
commit 9ae2dd19fe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 48 additions and 3 deletions

View file

@ -10,6 +10,11 @@ CalVer image tags (`stable-YYYY.MM.N`, `dev-YYYY.MM.N`) are produced for every C
## [Unreleased]
## [0.51.1] — 2026-05-12
### Fixed
- **`/corporate-memory/admin` no longer fails with "Error loading pending items." once pending knowledge items exist.** `GET /corporate-memory/admin` was passing the `corporate_memory.groups` YAML section (a dict, default `{}`) into the template as `groups=`, but `renderItemCard` evaluates `GROUPS.map(g => ...)` to build the mandate-form audience picker — `{}.map is not a function` threw inside the template literal, bubbled up to `renderReviewItems`, and the `loadReviewQueue` catch block painted the misleading "Error loading pending items." banner over a perfectly valid `/api/memory/admin/pending` response. Bug was dormant since the initial system commit because `renderItemCard` only runs when at least one pending item exists, so test fixtures and empty queues never tripped it. Fix: route now passes RBAC user_groups (`user_groups` table) shaped as `[{name, members_count}]`, which is what the mandate form actually targets (audience targeting is `group:<rbac-group-name>`, not `corporate_memory.groups`); template hardens the `.map` call with `Array.isArray(GROUPS) ? GROUPS : []` so a future shape regression degrades to "no group options" instead of crashing the whole list. No DB migration; no API change.
## [0.51.0] — 2026-05-12
### Fixed

View file

@ -974,6 +974,21 @@ async def corporate_memory_admin(
"WHERE relation_type = 'likely_duplicate' AND resolved = FALSE"
).fetchone()[0]
# Mandate-form audience picker needs RBAC user_groups, not the
# `corporate_memory.groups` YAML section — those are unrelated.
# Template expects an array of {name, members_count} so it can render
# `<option value="group:<name>">` rows in the per-item mandate form;
# the previous shape (`{}` from the YAML config) crashed renderItemCard
# with "GROUPS.map is not a function" the moment any pending item rendered.
from src.repositories.user_groups import UserGroupsRepository as _UserGroupsRepo
from src.repositories.user_group_members import UserGroupMembersRepository as _UserGroupMembersRepo
_groups_repo = _UserGroupsRepo(conn)
_members_repo = _UserGroupMembersRepo(conn)
user_groups_for_ui = [
{"name": g["name"], "members_count": _members_repo.count_members(g["id"])}
for g in _groups_repo.list_all()
]
ctx = _build_context(
request, user=user,
pending_items=pending,
@ -989,7 +1004,7 @@ async def corporate_memory_admin(
"duplicates": duplicates_count,
},
governance=get_corporate_memory_config(),
groups=get_corporate_memory_config().get("groups", {}),
groups=user_groups_for_ui,
contradictions=contradictions,
audit_entries=[],
)

View file

@ -1738,7 +1738,7 @@
<label style="margin-top: var(--space-3);">Target Audience</label>
<select id="mandate-audience-${esc(item.id)}">
<option value="all">All users</option>
${GROUPS.map(g => `<option value="group:${esc(g.name)}">${esc(g.name)} (${g.members_count} members)</option>`).join('')}
${(Array.isArray(GROUPS) ? GROUPS : []).map(g => `<option value="group:${esc(g.name)}">${esc(g.name)} (${g.members_count} members)</option>`).join('')}
</select>
<div class="mandate-form-actions">
<button class="btn btn-mandate" onclick="submitMandate('${esc(item.id)}')">Confirm</button>

View file

@ -1,6 +1,6 @@
[project]
name = "agnes-the-ai-analyst"
version = "0.51.0"
version = "0.51.1"
description = "Agnes — AI Data Analyst platform for AI analytical systems"
requires-python = ">=3.11,<3.14"
license = "MIT"

View file

@ -70,3 +70,28 @@ class TestNonAdminBlocked:
token = seeded_app["analyst_token"]
resp = c.get("/corporate-memory", headers=_auth(token))
assert resp.status_code == 403
class TestAdminGroupsContract:
def test_admin_page_renders_groups_as_array_not_dict(self, seeded_app):
"""`/corporate-memory/admin` must serialize `groups` as a JS array
of `{name, members_count}` rows. Earlier the route passed the
`corporate_memory.groups` YAML config (a dict, default `{}`),
so `GROUPS.map(...)` inside `renderItemCard` threw
`{}.map is not a function` and the page surfaced a misleading
"Error loading pending items" banner over a perfectly valid
pending payload. Bug was dormant because `renderItemCard` only
runs when 1 pending item exists. This test seeds one pending
item and asserts the array shape so the regression can't return."""
_seed_pending_item("groups_shape_1")
c = seeded_app["client"]
token = seeded_app["admin_token"]
resp = c.get("/corporate-memory/admin", headers=_auth(token))
assert resp.status_code == 200
body = resp.text
# JS literal must be an array — even when empty it is `[]`, never `{}`.
assert "const GROUPS = [" in body, (
"groups must serialize as a JS array of {name, members_count}; "
"a `{` prefix means we regressed to passing a dict and "
"renderItemCard's GROUPS.map(...) will crash at runtime."
)