Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions openrag/api/routers/admin/partitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,8 +383,10 @@ async def get_partition_config(
**Response:**
Returns list of partition members with:
- `user_id`: User identifier
- `display_name`: Human-readable name, when available
- `email`: Account email, when available
- `role`: User's role (owner, editor, or viewer)
- Additional user details
- `added_at`: Membership creation time

**Permissions:**
- Requires partition owner role
Expand All @@ -401,7 +403,7 @@ async def list_partition_users(
service=Depends(get_partition_service),
):
"""List all users who are members of the given partition."""
members = await service.list_members(partition=partition)
members = await service.list_members_with_identities(partition=partition)
return JSONResponse(status_code=status.HTTP_200_OK, content={"members": members})


Expand Down
2 changes: 2 additions & 0 deletions openrag/api/routers/admin/users.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
- `id`: User identifier
- `display_name`: User's display name
- `external_user_id`: External ID (if set)
- `email`: Account email (if set)
- `is_admin`: Admin status
- `created_at`: Account creation timestamp

Expand Down Expand Up @@ -141,6 +142,7 @@ async def create_user(
- `id`: User identifier
- `display_name`: User's display name
- `external_user_id`: External ID (if set)
- `email`: Account email (if set)
- `is_admin`: Admin status
- `created_at`: Account creation timestamp

Expand Down
3 changes: 3 additions & 0 deletions openrag/core/ports/user_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ async def create_user(self, user: User) -> User: ...
@abstractmethod
async def get_user(self, user_id: int) -> User | None: ...

@abstractmethod
async def get_users_by_ids(self, user_ids: list[int]) -> list[User]: ...

@abstractmethod
async def get_user_by_email(self, email: str) -> User | None: ...

Expand Down
13 changes: 13 additions & 0 deletions openrag/services/orchestrators/partition_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -673,9 +673,22 @@ def _meta(row: dict[str, Any]) -> dict[str, Any]:
# ------------------------------------------------------------------

async def list_members(self, partition: str) -> list[dict]:
"""Return role data without identity lookups for authorization callers."""
await self._ensure_partition(partition)
return await self._membership_repo.list_partition_members(partition)

async def list_members_with_identities(self, partition: str) -> list[dict]:
"""Enrich the admin-facing member list with one bulk user lookup."""
members = await self.list_members(partition)
users = {
user.id: user for user in await self._user_repo.get_users_by_ids([member["user_id"] for member in members])
}
for member in members:
user = users.get(member["user_id"])
member["display_name"] = user.display_name if user else None
member["email"] = user.email if user else None
return members

async def add_member(self, partition: str, user_id: int, role: str) -> None:
await self._ensure_partition(partition)
await self._ensure_user_exists(user_id)
Expand Down
10 changes: 10 additions & 0 deletions openrag/services/persistence/user_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,15 @@ async def get_user(self, user_id: int) -> User | None:
memberships = await self._fetch_memberships(user_id)
return self._row_to_user(row, memberships)

async def get_users_by_ids(self, user_ids: list[int]) -> list[User]:
if not user_ids:
return []
rows = await self.pool.fetch(
"SELECT * FROM users WHERE id = ANY($1::int[])",
user_ids,
)
return [self._row_to_user(row) for row in rows]

async def get_user_by_email(self, email: str) -> User | None:
row = await self.pool.fetchrow(
"SELECT * FROM users WHERE email = $1",
Expand Down Expand Up @@ -332,6 +341,7 @@ async def list_users_dict(self) -> list[dict]:
"id": r["id"],
"display_name": r["display_name"],
"external_user_id": r["external_user_id"],
"email": r["email"],
Comment thread
Ahmath-Gadji marked this conversation as resolved.
"is_admin": r["is_admin"],
"file_quota": r["file_quota"],
"file_count": r["file_count"],
Expand Down
81 changes: 80 additions & 1 deletion tests/unit/services/orchestrators/test_partition_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,12 +212,41 @@ async def query_chunks_by_filter(self, collection, filters, output_fields=None):


class FakeUserRepo:
def __init__(self, existing: set[int] | None = None):
def __init__(
self,
existing: set[int] | None = None,
display_names: dict[int, str] | None = None,
emails: dict[int, str] | None = None,
):
self._existing = existing if existing is not None else set()
self._display_names = display_names or {}
self._emails = emails or {}
self.requested_user_id_batches: list[list[int]] = []

async def user_exists(self, user_id: int) -> bool:
return user_id in self._existing

async def get_user(self, user_id: int):
if user_id not in self._existing:
return None
return SimpleNamespace(
id=user_id,
display_name=self._display_names.get(user_id),
email=self._emails.get(user_id),
)

async def get_users_by_ids(self, user_ids: list[int]):
self.requested_user_id_batches.append(list(user_ids))
return [
SimpleNamespace(
id=user_id,
display_name=self._display_names.get(user_id),
email=self._emails.get(user_id),
)
for user_id in user_ids
if user_id in self._existing
]


def _svc(
*,
Expand Down Expand Up @@ -830,6 +859,56 @@ async def test_list_members_missing_partition_404():
await _svc(prepo=FakePartitionRepo(set())).list_members("x")


@pytest.mark.asyncio
async def test_list_members_does_not_lookup_user_identities():
mrepo = FakeMembershipRepo(members={(9, "p")})
urepo = FakeUserRepo({9})
svc = _svc(prepo=FakePartitionRepo({"p"}), mrepo=mrepo, urepo=urepo)

members = await svc.list_members("p")

assert members == [{"user_id": 9, "role": "viewer"}]
assert urepo.requested_user_id_batches == []


@pytest.mark.asyncio
async def test_list_members_with_identities_uses_one_lookup():
mrepo = FakeMembershipRepo(members={(9, "p"), (10, "p")})
urepo = FakeUserRepo(
{9, 10},
display_names={9: "Alice", 10: "Bob"},
emails={9: "alice@example.com", 10: "bob@example.com"},
)
svc = _svc(prepo=FakePartitionRepo({"p"}), mrepo=mrepo, urepo=urepo)
members = await svc.list_members_with_identities("p")
assert {member["user_id"]: member for member in members} == {
9: {
"user_id": 9,
"role": "viewer",
"display_name": "Alice",
"email": "alice@example.com",
},
10: {
"user_id": 10,
"role": "viewer",
"display_name": "Bob",
"email": "bob@example.com",
},
}
assert len(urepo.requested_user_id_batches) == 1
assert set(urepo.requested_user_id_batches[0]) == {9, 10}


@pytest.mark.asyncio
async def test_list_members_missing_user_display_name_is_none():
mrepo = FakeMembershipRepo(members={(9, "p")})
urepo = FakeUserRepo(set()) # user_id 9 no longer exists
svc = _svc(prepo=FakePartitionRepo({"p"}), mrepo=mrepo, urepo=urepo)
members = await svc.list_members_with_identities("p")
assert members[0]["display_name"] is None
assert members[0]["email"] is None


@pytest.mark.asyncio
async def test_add_member_checks_partition_and_user():
mrepo = FakeMembershipRepo()
Expand Down
70 changes: 69 additions & 1 deletion tests/unit/services/persistence/test_user_repo_external_id.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,14 @@ def __init__(self):
self.last_query: str | None = None
self.last_params: tuple = ()
self._next_row: _FakeRow | None = None
self._rows: list[_FakeRow] = []

def set_next_row(self, **fields):
self._next_row = _FakeRow(fields)

def set_rows(self, *rows: _FakeRow):
self._rows = list(rows)

async def fetchrow(self, query: str, *params):
self.last_query = query
self.last_params = params
Expand All @@ -49,7 +53,9 @@ async def execute(self, query: str, *params):
async def fetch(self, query: str, *params):
self.last_query = query
self.last_params = params
return []
if "partition_memberships" in query:
return []
return self._rows
Comment thread
hedhoud marked this conversation as resolved.


def _make_user_with_ext(ext: str | None):
Expand Down Expand Up @@ -141,3 +147,65 @@ async def test_create_legacy_user_coerces_empty_external_id_to_none():
)
# Same column position (display_name, external_user_id, ...)
assert pool.last_params[1] is None


@pytest.mark.asyncio
async def test_list_users_dict_includes_email():
from services.persistence.user_repo import PgUserRepository

pool = _FakePool()
pool.set_rows(
_FakeRow(
id=42,
display_name="Alice",
external_user_id="kc-alice",
email="alice@example.com",
is_admin=False,
file_quota=None,
file_count=0,
created_at=__import__("datetime").datetime(2026, 1, 1),
)
)
repo = PgUserRepository(pool_getter=lambda: pool)

users = await repo.list_users_dict()

assert users[0]["email"] == "alice@example.com"


@pytest.mark.asyncio
async def test_get_users_by_ids_fetches_all_users_in_one_query():
from services.persistence.user_repo import PgUserRepository

pool = _FakePool()
pool.set_rows(
_FakeRow(
id=42,
display_name="Alice",
external_user_id="kc-alice",
email="alice@example.com",
token=None,
is_admin=False,
file_quota=None,
file_count=0,
created_at=__import__("datetime").datetime(2026, 1, 1),
),
_FakeRow(
id=84,
display_name="Bob",
external_user_id="kc-bob",
email="bob@example.com",
token=None,
is_admin=False,
file_quota=None,
file_count=0,
created_at=__import__("datetime").datetime(2026, 1, 2),
),
)
repo = PgUserRepository(pool_getter=lambda: pool)

users = await repo.get_users_by_ids([42, 84])

assert {user.id for user in users} == {42, 84}
assert pool.last_params == ([42, 84],)
assert "ANY($1::int[])" in (pool.last_query or "")
13 changes: 12 additions & 1 deletion ui/src/components/shared/data-table.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ interface BaseDataTableProps<TData, TValue> {
columns: ColumnDef<TData, TValue>[];
data: TData[];
pageSize?: number;
emptyMessage?: string;
initialSorting?: SortingState;
/** Reset pagination to the first page whenever this value changes. */
pageResetKey?: unknown;
/** Render a leading checkbox column. */
enableSelection?: boolean;
/** Optional row-level selection guard for pages with state-dependent bulk actions. */
Expand All @@ -54,7 +57,9 @@ export function DataTable<TData, TValue>({
columns,
data,
pageSize = 10,
emptyMessage = "No results.",
initialSorting = [],
pageResetKey,
enableSelection = false,
canSelectRow,
getRowId,
Expand All @@ -68,6 +73,12 @@ export function DataTable<TData, TValue>({
const rowSelection = controlledRowSelection ?? internalRowSelection;
const setRowSelection = onRowSelectionChange ?? setInternalRowSelection;

useEffect(() => {
setPagination((previous) =>
previous.pageIndex === 0 ? previous : { ...previous, pageIndex: 0 },
);
}, [pageResetKey]);

// Prepend a checkbox column when selection is enabled.
const tableColumns = useMemo<ColumnDef<TData, TValue>[]>(() => {
if (!enableSelection) return columns;
Expand Down Expand Up @@ -177,7 +188,7 @@ export function DataTable<TData, TValue>({
colSpan={columnCount}
className="h-24 text-center text-muted-foreground"
>
No results.
{emptyMessage}
</TableCell>
</TableRow>
)}
Expand Down
4 changes: 3 additions & 1 deletion ui/src/lib/api/partitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { request } from "./client";
// POST /partition/{p} create (name in path, NO body; caller becomes owner) → 201
// PATCH /partition/{p} update config → PartitionDetailResponse
// DELETE /partition/{p} delete → 204
// GET /partition/{p}/users members → { members: [{ user_id, role, added_at }] }
// GET /partition/{p}/users members → { members: [{ user_id, display_name, email, role, added_at }] }
// POST /partition/{p}/users add member (multipart: user_id, role)
// PATCH /partition/{p}/users/{user_id} change role (multipart: role)
// DELETE /partition/{p}/users/{user_id} remove member
Expand Down Expand Up @@ -194,6 +194,8 @@ export function listPartitionFiles(name: string, limit?: number): Promise<{ file

export interface PartitionMember {
user_id: number;
display_name: string | null;
email: string | null;
role: PartitionRole;
added_at: string | null;
}
Expand Down
Loading
Loading