Skip to content

feat: settings DataTable pagination, connector UX, and misc fixes#3248

Open
joeyorlando wants to merge 4 commits intomainfrom
feat/settings-datatable-pagination
Open

feat: settings DataTable pagination, connector UX, and misc fixes#3248
joeyorlando wants to merge 4 commits intomainfrom
feat/settings-datatable-pagination

Conversation

@joeyorlando
Copy link
Contributor

@joeyorlando joeyorlando commented Mar 10, 2026

Summary

  • Settings pages DataTable pagination: Convert teams, users, and roles pages to use DataTable with server-side pagination, sorting, and search
  • Connector dialog UX improvements: Move Cloud Instance toggle out of Advanced section, add editable credentials (email/API token) to edit dialog, wider dialog, sticky footer
  • Self-hosted Jira/Confluence PAT auth: Fix 401 errors for Server instances using personal access tokens (uses Bearer auth instead of basic auth when no email provided)
  • Table name truncation: Truncate long agent/gateway/proxy names with tooltip only on overflow (agents, MCP gateways, LLM proxies tables)
  • Chat autosave fix: Fix textarea losing characters during autosave by removing systemPrompt from useEffect dependency array

Test plan

  • Verify settings pages (teams, users, roles) pagination, sorting, and search work correctly
  • Create/edit Jira and Confluence connectors — verify Cloud Instance toggle is visible outside Advanced
  • Edit connector and update API token — verify credentials are persisted
  • Create a self-hosted Jira/Confluence connector with PAT (no email) — verify connection succeeds
  • Verify long agent names truncate with tooltip, short names show no tooltip
  • Verify chat agent editor textarea doesn't lose characters while typing

…server-side pagination

- Convert /settings/teams, /settings/users, and /settings/roles from card-based
  layouts to DataTable with server-side pagination, search, and sorting
- Teams: search by name/description, sort by name/createdAt/memberCount
- Users: search by name/email (case-insensitive), filter by teams and role,
  merge auto-provisioned users into main table with appropriate actions
- Roles: search by name/description, predefined roles pinned at top,
  add description column to custom roles with create/edit dialog support
- Move Organization Token into MCP Gateway/A2A Gateway Tokens section
- Add paginated API endpoints: GET /api/teams, GET /api/roles,
  GET /api/organization/members/paginated
- Add database migration for organization_role description column
- Add comprehensive backend model tests (team, member, organization-role)
- Add e2e API tests for teams and users search/filter/pagination
- Update all consumers of teams/roles APIs for new paginated response shape
@claude
Copy link
Contributor

claude bot commented Mar 10, 2026

Claude finished @joeyorlando's task in 5m 0s —— View job


PR Review: Convert settings pages to DataTable with server-side pagination

Thorough review of all 55 changed files. This is a well-structured PR that consistently converts three settings pages to DataTable with proper server-side pagination. The backend models are well-designed with good N+1 avoidance, and all API consumers have been correctly updated for the new paginated response shape.


Issues

1. LIKE wildcard characters not escaped in search inputs (Low severity, 3 files)

Drizzle-orm's ilike() does not escape SQL LIKE wildcards (% and _). If a user searches for a string containing these characters, they'll be interpreted as wildcards rather than literals. This is a functional correctness issue, not a security issue (the values are parameterized).

Affected files:

  • platform/backend/src/models/team.ts:125 - ilike(schema.teamsTable.name, \%${search}%`)`
  • platform/backend/src/models/member.ts:325 - ilike(schema.usersTable.name, searchPattern)
  • platform/backend/src/models/organization-role.ts:435 - ilike(schema.organizationRolesTable.name, \%${search}%`)`

A helper like search.replace(/[%_]/g, '\\$&') before interpolation would fix this. Low priority since it's unlikely users search for % or _, but worth noting.

2. getAllPaginated in organization-role fetches ALL custom roles then paginates in-memory (organization-role.ts:446-485)

The getAllPaginated method fetches all custom roles from the database (no LIMIT/OFFSET on the DB query at line 452), then sorts and paginates in JavaScript. This approach works fine for small numbers of custom roles, but won't scale well if an organization creates many custom roles. The team model correctly does pagination at the DB level, so there's an inconsistency.

This is understandable given the complexity of interleaving predefined roles (which live in code) with custom roles (which live in the DB), but worth noting as a future optimization point if custom role counts grow.

3. Security scanner findings on e2e tests are false positives

The CodeQL alerts on platform/e2e-tests/tests/api/users-search.spec.ts (lines 78, 81, 305, 308) about "Incomplete URL substring sanitization" for example.com are false positives. These are test assertions checking that search results match the expected domain in test data, not URL validation logic. No action needed.


Suggestions (non-blocking)

4. Consider extracting duplicate SortIcon and URL-param handling into shared utilities

The SortIcon component and the URL parameter synchronization pattern (handleSortingChange, handlePaginationChange, useEffect to sync sorting with URL) are duplicated across users/page.tsx, teams-list.tsx, and roles-list.tsx/roles-list.ee.tsx. Extracting these into shared hooks/components would reduce duplication. Not blocking for this PR, but a good follow-up.

5. toTitleCase utility duplicated

toTitleCase() is defined at the bottom of users/page.tsx (line 802). Check if this already exists elsewhere in the codebase - it's a common utility that could live in lib/utils.ts.

6. Consider debouncing URL param updates on search

The SearchInput component pushes a new URL on every change. If it's not already debounced internally, rapid typing could cause excessive navigation and API calls. Worth verifying that SearchInput handles debouncing.


Looks Good

  • N+1 prevention: Team members and user teams are batch-loaded with getTeamMembersBatch and batch team-membership queries in findAllPaginatedByOrganization, avoiding N+1 queries.
  • Parallel queries: Count and data queries run in parallel via Promise.all in member.ts:370 and organization-role.ts:446.
  • Authorization: Team visibility is properly scoped - non-admin users only see their own teams (team.ts:134-140), roles route restricts custom role visibility to users with ac:create permission (organization-role.ts:53).
  • API consumer updates: All 8+ consumers of getTeams() correctly extract .data from the new paginated response (data?.data || []).
  • Comprehensive tests: Good coverage with backend model tests and e2e API tests. Tests properly clean up created resources in finally blocks.
  • Migration: Clean single-column addition (description on organization_role), minimal risk.
  • Proper pagination metadata: hasNext, hasPrev, totalPages, currentPage all computed correctly.
  • EE/non-EE split: Roles list properly separates enterprise features into roles-list.ee.tsx.

Overall this is a solid, well-implemented PR. The issues found are minor. The main architectural note is the in-memory pagination for roles (#2), which is fine for now but worth keeping in mind for scale.

@London-Cat
Copy link
Collaborator

London-Cat commented Mar 10, 2026

📊 Reputation Summary

User Rep Pull Requests Activity Assigned Core Reactions
joeyorlando ⚡ 2351 92✅ 7🔄 1❌ 100 issues, 50 comments 6

How is the score calculated? Read about it in the Reputation Bot repository 🤖

Comment on lines +76 to +78
const matchesName = member.name
?.toLowerCase()
.includes("example.com");

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

'
example.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 3 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

Comment on lines +79 to +81
const matchesEmail = member.email
?.toLowerCase()
.includes("example.com");

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

'
example.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 3 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

Comment on lines +303 to +305
const matchesName = member.name
?.toLowerCase()
.includes("example.com");

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

'
example.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 3 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

Comment on lines +306 to +308
const matchesEmail = member.email
?.toLowerCase()
.includes("example.com");

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

'
example.com
' can be anywhere in the URL, and arbitrary hosts may come before or after it.

Copilot Autofix

AI 3 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

The useEffect that syncs local instructions state depended on both
agent.id and agent.systemPrompt. When autosave completed and
invalidated the query cache, the updated systemPrompt triggered the
effect, resetting the textarea to the server value and wiping any
characters typed during the save round-trip. Fix by only resetting
on agent ID change.
Long agent names (e.g. "n8n workflow: Grafana exporter") caused text
wrapping which misaligned scope badges. Truncate names with ellipsis
and show full name in tooltip on hover. Applied to /agents,
/mcp/gateways, and /llm/proxies tables.
Self-hosted Jira/Confluence Server instances commonly use Personal
Access Tokens (PAT) via Bearer auth, but the connector always used
basic auth which fails with 401 when no username is provided.

- Use oauth2 (Bearer) auth when email is not provided, basic auth
  when it is
- Add noCheckAtlassianToken for XSRF compatibility
- Make email optional in the create connector form for non-Cloud
  instances
- Add tests for both auth paths
@joeyorlando joeyorlando changed the title feat: convert settings pages to DataTable with server-side pagination chore: convert settings pages to DataTable with server-side pagination Mar 10, 2026
@joeyorlando joeyorlando changed the title chore: convert settings pages to DataTable with server-side pagination feat: settings DataTable pagination, connector UX, and misc fixes Mar 10, 2026
@github-actions
Copy link
Contributor

Playwright test results

failed  17 failed
passed  596 passed
flaky  5 flaky
skipped  24 skipped

Details

stats  642 tests across 63 suites
duration  5 minutes, 60 seconds
commit  6dc0096

Failed tests

api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should create a new custom role
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should fail to create role with duplicate name
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should fail to create role with reserved predefined name
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should get a specific custom role by ID
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should update a custom role name
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should update a custom role permissions
api › api/access-control.ee.spec.ts › Organization Roles API - Custom Role CRUD Operations › should delete a custom role
api › api/access-control.ee.spec.ts › Organization Roles API - Permission Validation › should create role with multiple permissions
api › api/access-control.ee.spec.ts › Organization Roles API - Permission Validation › should create role with empty permissions
api › api/access-control.ee.spec.ts › Organization Roles API - Role Lifecycle › should handle complete role lifecycle: create, read, update, delete
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user with only mcpGateway permissions cannot access agents or llm proxies
api › api/auth-permissions.ee.spec.ts › Auth Permissions API - Custom Roles › should work with custom roles
api › api/invitation-custom-role.spec.ts › Invitation Custom Role Assignment › should create invitation with custom role
api › api/invitation-custom-role.spec.ts › Invitation Custom Role Assignment › should list pending invitations with correct roles
credentials-with-vault › ui/credentials-with-vault.ee.spec.ts › Chat API Keys with Readonly Vault › should create a team scoped chat API key with vault secret
chromium › ui/chat-auth-required.spec.ts › Chat - Auth Required Tool › renders AuthRequiredTool when tool call fails due to missing credentials
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider Team Sync E2E › should sync user to team based on SSO group membership

Flaky tests

api › api/llm-proxy/jwks-auth.spec.ts › LLM Proxy - External IdP JWKS Authentication › should authenticate with external IdP JWT and get model response
api › api/llm-proxy/token-cost-limits.spec.ts › LLMProxy-TokenCostLimits-Anthropic › blocks request when profile token cost limit is exceeded
api › api/mcp-gateway.spec.ts › MCP Gateway - External MCP Server Tests › should list internal-dev-test-server tool
api › api/mcp-gateway.spec.ts › MCP Gateway - External MCP Server Tests › should invoke internal-dev-test-server tool successfully
chromium › ui/chat-settings.spec.ts › Provider Settings - Virtual API Keys › Can create a virtual key from the Virtual API Keys tab

Skipped tests

api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user with only llmProxy permissions cannot access agents or mcp gateways
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user with only agent permissions cannot access mcp gateways or llm proxies
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user with mixed permissions can access allowed types only
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user permissions are checked on get/update/delete individual agent
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › admin can create shared agents with teams for all agent types
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user with team-admin can create and manage team-scoped agents
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › non-admin user can only create personal agents, not shared
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user permissions endpoint reflects new resource types
api › api/agent-type-permissions.ee.spec.ts › Agent Type Permission Isolation › user without agent:admin cannot see built-in agents
api › api/knowledge-bases.spec.ts › Knowledge Bases API › Connector CRUD › connectors are cascade-deleted when KG is deleted
credentials-with-vault › ui/credentials-with-vault.ee.spec.ts › Chat API Keys with Readonly Vault › should create a personal scoped chat API key with vault secret
credentials-with-vault › ui/credentials-with-vault.ee.spec.ts › Test self-hosted MCP server with Readonly Vault › Test self-hosted MCP server with Vault - with prompt on installation
credentials-with-vault › ui/credentials-with-vault.ee.spec.ts › Test self-hosted MCP server with Readonly Vault › Test self-hosted MCP server with Vault - without prompt on installation
chromium › ui/dynamic-credentials.spec.ts › Verify tool calling using dynamic credentials
chromium › ui/static-credentials-management.spec.ts › Custom Self-hosted MCP Server - installation and static credentials management (vault disabled, prompt-on-installation disabled) › Admin
chromium › ui/static-credentials-management.spec.ts › Custom Self-hosted MCP Server - installation and static credentials management (vault disabled, prompt-on-installation disabled) › Editor
chromium › ui/static-credentials-management.spec.ts › Custom Self-hosted MCP Server - installation and static credentials management (vault disabled, prompt-on-installation disabled) › Member
chromium › ui/static-credentials-management.spec.ts › Verify Manage Credentials dialog shows correct other users credentials
chromium › ui/static-credentials-management.spec.ts › Verify tool calling using different static credentials
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider OIDC E2E Flow with Keycloak › should configure OIDC provider, login via SSO, update, and delete
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider IdP Logout (RP-Initiated Logout) › should terminate IdP session on Archestra sign-out
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider Role Mapping E2E › should evaluate second rule when first rule does not match
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider Role Mapping E2E › should map admin group to admin role via OIDC
identity-providers › ui/identity-providers.ee.spec.ts › Identity Provider SAML E2E Flow with Keycloak › should configure SAML provider, login via SSO, update, and delete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants