Bug RHOAIENG-39110: Fix username display to remove cluster suffix

[Gkrumbach07]

Summary

Fixes username display in the top-right corner to show clean username instead of user@cluster.local format.

Problem

The X-Forwarded-Preferred-Username header contains values like gkrumbac@cluster.local, and the frontend API endpoint was using this value directly for displayName, causing the UI to show gkrumbac@cluster.local instead of just gkrumbac.

Solution

Modified components/frontend/src/app/api/me/route.ts to strip everything after the @ symbol from the displayName while preserving the original username field unchanged.

Changes

• Added logic to split displayName by @ and take the first part
• Handles edge cases:
  • Usernames without @ symbol (passthrough)
  • Multiple @ symbols (takes first part)
  • Email addresses (strips domain)
  • Empty/undefined values (fallback preserved)
• Maintains backward compatibility

Testing

The fix handles all expected scenarios:

• gkrumbac@cluster.local → displays as gkrumbac ✓
• user@apps-crc.testing → displays as user ✓
• username → displays as username ✓
• Email addresses → strips domain appropriately ✓

Related Issue

Fixes: https://issues.redhat.com/browse/RHOAIENG-39110

[github-actions]

Claude Code Review

Summary

This PR fixes the username display in the top-right corner by stripping cluster domain suffixes (e.g., @cluster.local, @apps-crc.testing) from the displayName field. The change is minimal, focused, and follows established frontend patterns.

Issues by Severity

🚫 Blocker Issues

None found.

🔴 Critical Issues

None found.

🟡 Major Issues

None found.

🔵 Minor Issues

1. Missing Test Coverage

• Location: components/frontend/src/app/api/me/route.ts:16-18
• Issue: No unit tests verify the displayName transformation logic
• Recommendation: Add tests to cover edge cases:
  • gkrumbac@cluster.local → gkrumbac
  • user@apps-crc.testing → user
  • username (no @) → username
  • user@domain@extra (multiple @) → user
  • undefined → undefined
• Why it matters: The PR description mentions these scenarios are tested, but there are no automated tests to prevent regression

2. Potential Ambiguity with Email Addresses

• Location: components/frontend/src/app/api/me/route.ts:17
• Issue: The logic rawDisplayName?.split('@')[0] will strip email domains too
• Current behavior: If username is empty and email is user@company.com, displayName becomes user
• Is this intentional? Probably yes, based on the PR description, but worth confirming
• Recommendation: Add a code comment explaining this is intentional:

  // Clean the displayName by removing cluster suffix (e.g., "@cluster.local", "@apps-crc.testing")
  // Also strips email domains if email is used as displayName
  const rawDisplayName = username || email || userId;
  const displayName = rawDisplayName?.split('@')[0] || rawDisplayName;

3. TypeScript Type Safety

• Location: components/frontend/src/app/api/me/route.ts:17
• Issue: rawDisplayName could be undefined, but TypeScript inference might not catch all edge cases
• Current code: Uses optional chaining (?.split) and nullish coalescing (||), which is correct
• Recommendation: Consider explicit type annotation for clarity:

  const rawDisplayName: string | undefined = username || email || userId;

Positive Highlights

✅ Minimal, focused change - Only 5 additions, 1 deletion. Perfect example of targeted bug fix.

✅ Preserves original fields - The username field remains unchanged, only displayName is transformed. This maintains backward compatibility.

✅ Handles edge cases - Uses optional chaining (?.) and fallback (||) to handle undefined/null values safely.

✅ Clear intent - Code comment explains what the transformation does.

✅ Follows React Query patterns - The API endpoint is consumed via React Query hooks (useCurrentUser), following project standards.

✅ Correct usage in UI - The user-bubble.tsx component uses displayName for display while also having fallbacks to username and email.

✅ No security concerns - The transformation is purely cosmetic and doesn't affect authentication or authorization logic.

✅ CI passing - All automated checks are passing.

Recommendations

Priority 1: Add Unit Tests

Create components/frontend/src/app/api/me/route.test.ts:

import { GET } from './route';

describe('/api/me route', () => {
  it('should strip cluster suffix from username', async () => {
    const request = new Request('http://localhost:3000/api/me', {
      headers: {
        'X-Forwarded-Preferred-Username': 'gkrumbac@cluster.local',
      },
    });
    const response = await GET(request);
    const data = await response.json();
    expect(data.displayName).toBe('gkrumbac');
    expect(data.username).toBe('gkrumbac@cluster.local'); // Original preserved
  });

  it('should handle username without @ symbol', async () => {
    const request = new Request('http://localhost:3000/api/me', {
      headers: {
        'X-Forwarded-Preferred-Username': 'username',
      },
    });
    const response = await GET(request);
    const data = await response.json();
    expect(data.displayName).toBe('username');
  });

  // Add more test cases...
});

Priority 2: Enhance Code Comment

Update the comment to be more explicit about email handling:

// Clean the displayName by removing cluster suffix (e.g., "@cluster.local", "@apps-crc.testing")
// Note: This also strips email domains when email is used as fallback (user@company.com → user)
const rawDisplayName = username || email || userId;
const displayName = rawDisplayName?.split('@')[0] || rawDisplayName;

Priority 3: Consider E2E Test

Add a Cypress test in e2e/cypress/e2e/vteam.cy.ts to verify the UI displays the cleaned username:

it('should display cleaned username in top-right corner', () => {
  cy.visit('/');
  cy.get('[data-testid="user-bubble"]').should('contain', 'gkrumbac');
  cy.get('[data-testid="user-bubble"]').should('not.contain', '@cluster.local');
});

Architecture & Standards Compliance

✅ Frontend patterns: Follows React Query usage patterns from .claude/patterns/react-query-usage.md

✅ Type definitions: Uses proper TypeScript types (UserProfile in services/api/auth.ts)

✅ Shadcn UI: The user-bubble.tsx component correctly uses Shadcn UI components (Avatar, Button)

✅ No any types: Code is type-safe throughout

✅ API client layer: Properly separates API logic (services/api/auth.ts) from React Query hooks (services/queries/use-auth.ts)

Final Verdict

APPROVE ✅

This is a well-crafted, focused bug fix that solves the reported issue cleanly. The code is safe, follows project patterns, and has no security or architectural concerns. The only improvement needed is test coverage, which should be added in a follow-up PR or before merge if time permits.

Suggested next steps:

1. Add unit tests for the transformation logic (Priority 1)
2. Enhance the code comment for clarity (Priority 2)
3. Consider adding E2E test coverage (Priority 3)

Great work on keeping this change minimal and focused! 🎉

---

🔍 View AI decision process (logs available for 90 days)

📋 View memory system files loaded (click to expand)

What Amber Loaded for Code Review

Amber automatically loaded these repository standards from the memory system:

1. CLAUDE.md - Master project instructions, development standards
2. backend-development.md - Go backend, K8s integration patterns
3. frontend-development.md - NextJS, Shadcn UI, React Query patterns
4. security-standards.md - Auth, RBAC, token handling
5. k8s-client-usage.md - User token vs service account patterns
6. error-handling.md - Consistent error patterns
7. react-query-usage.md - Data fetching patterns

Impact: This review used your repository's specific code quality standards, security patterns, and best practices from the memory system (PRs #359, #360) - not just generic code review guidelines.