Skip to content

Improve user details UX on member page (#1889) #1934

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Improve user details UX on member page (#1889) #1934

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
19b2131
Improve user details UX on member page (#1889)
adithya-naik Jul 31, 2025
a6b50ae
✅ refactor: cleanup and optimizations in DetailsCard as suggested-#1889
adithya-naik Jul 31, 2025
2cfad49
refactor/handled ReDoS vulnerability
adithya-naik Aug 1, 2025
23984ac
removed excess coomments-#1889
adithya-naik Aug 5, 2025
7c7ac79
Resolved merge conflict in CardDetailsPage.tsx
adithya-naik Aug 5, 2025
fbe45d3
Merge branch 'main' into enhance/improve-member-details-ux-#1889
kasya Aug 9, 2025
866da53
Fix tests
kasya Aug 9, 2025
467b995
Merge branch 'main' into enhance/improve-member-details-ux-#1889
kasya Aug 9, 2025
c79faf5
Update n/a vs unknown
kasya Aug 9, 2025
fd717a9
Merge branch 'main' into enhance/improve-member-details-ux-#1889
kasya Aug 9, 2025
e431022
Merge branch 'main' of github.com:OWASP/Nest into pr/adithya-naik/1934-1
kasya Aug 30, 2025
5a498f3
Merge branch 'main' into enhance/improve-member-details-ux-#1889
kasya Aug 30, 2025
112b703
unknown to N/A
adithya-naik Aug 31, 2025
bfa5c0d
in tests -- unknown to N/A
adithya-naik Aug 31, 2025
6c9b194
Merge branch 'main' into enhance/improve-member-details-ux-#1889
kasya Sep 13, 2025
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
2 changes: 1 addition & 1 deletion frontend/__tests__/unit/pages/UserDetails.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ describe('UserDetailsPage', () => {

render(<UserDetailsPage />)
await waitFor(() => {
expect(screen.getAllByText('N/A').length).toBe(3)
expect(screen.getAllByText('Unknown').length).toBe(3)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Make this assertion more resilient and verify link semantics for email/company

Counting all "Unknown" instances can become brittle as the page evolves. Prefer field-scoped assertions (email/company/location) and add tests for:

  • email renders as mailto link when valid, plain text when invalid
  • company starting with "@" renders as GitHub link, otherwise plain text

Example tests to add:

test('renders email as a mailto link when valid', async () => {
  ;(useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, email: '[email protected]' } },
    loading: false,
    error: null,
  })
  render(<UserDetailsPage />)
  await waitFor(() => {
    const emailLink = screen.getByRole('link', { name: '[email protected]' })
    expect(emailLink).toHaveAttribute('href', 'mailto:[email protected]')
  })
})

test('does not render email as link when invalid', async () => {
  ;(useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, email: 'invalid@' } },
    loading: false,
    error: null,
  })
  render(<UserDetailsPage />)
  await waitFor(() => {
    expect(screen.getByText('invalid@')).toBeInTheDocument()
    expect(screen.queryByRole('link', { name: 'invalid@' })).toBeNull()
  })
})

test('renders company starting with "@" as GitHub link', async () => {
  ;(useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, company: '@acme' } },
    loading: false,
    error: null,
  })
  render(<UserDetailsPage />)
  await waitFor(() => {
    const companyLink = screen.getByRole('link', { name: '@acme' })
    expect(companyLink).toHaveAttribute('href', 'https://github.com/acme')
  })
})

test('renders company without "@" as plain text', async () => {
  ;(useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, company: 'Acme Inc' } },
    loading: false,
    error: null,
  })
  render(<UserDetailsPage />)
  await waitFor(() => {
    const companyText = screen.getByText('Acme Inc')
    expect(companyText.closest('a')).toBeNull()
  })
})

Quick check to see if such tests already exist:

🏁 Script executed:

#!/bin/bash
# Verify there are tests asserting mailto/company link semantics.
rg -n --hidden --glob "frontend/**/__tests__/**" \
  -e 'mailto:' \
  -e 'toHaveAttribute\\(.*href.*,.*mailto:' \
  -e 'https://github.com/' \
  -e 'toHaveAttribute\\(.*href.*,.*https://github.com/'

Length of output: 427

I’ve adjusted the grep patterns to avoid the regex error and split the search for clarity. Let’s verify whether any mailto or GitHub link assertions already exist in our tests:

🏁 Script executed:

#!/bin/bash
# Search for mailto link assertions
rg -n --hidden --glob "frontend/**/__tests__/**" "mailto:"

# Search for GitHub link assertions
rg -n --hidden --glob "frontend/**/__tests__/**" "github.com/"

Length of output: 13714

Let’s inspect the existing UserDetails tests around the GitHub link checks to see if any cover the company field (and confirm there are truly no mailto/email tests):

🏁 Script executed:

#!/bin/bash
sed -n '330,380p' frontend/__tests__/unit/pages/UserDetails.test.tsx

Length of output: 1650

Make “Unknown” assertion field-scoped and add email/company link tests

The catch-all

expect(screen.getAllByText('Unknown').length).toBe(3)

in frontend/tests/unit/pages/UserDetails.test.tsx (around line 468) is brittle and doesn’t verify email or company link behavior. Please:

  • Replace it with explicit checks for each field’s “Unknown” fallback (email, company, location).
  • Add new tests for link semantics:
    • Email
    – valid address → renders as a mailto: link
    – invalid address → renders as plain text (no <a>)
    • Company
    – starts with “@” → renders as a GitHub link (https://github.com/<org>)
    – otherwise → renders plain text

Example tests to guide you:

test('renders valid email as mailto link', async () => {
  (useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, email: '[email protected]' } },
    loading: false, error: null,
  });
  render(<UserDetailsPage />);
  await waitFor(() => {
    const link = screen.getByRole('link', { name: '[email protected]' });
    expect(link).toHaveAttribute('href', 'mailto:[email protected]');
  });
});

test('renders company starting with "@" as GitHub link', async () => {
  (useQuery as jest.Mock).mockReturnValue({
    data: { ...mockUserDetailsData, user: { ...mockUserDetailsData.user, company: '@acme' } },
    loading: false, error: null,
  });
  render(<UserDetailsPage />);
  await waitFor(() => {
    const link = screen.getByRole('link', { name: '@acme' });
    expect(link).toHaveAttribute('href', 'https://github.com/acme');
  });
});
🤖 Prompt for AI Agents 
In frontend/__tests__/unit/pages/UserDetails.test.tsx at line 468, replace the
broad check for 'Unknown' appearing three times with explicit assertions for
each field's fallback text—email, company, and location—to ensure clarity and
robustness. Additionally, add new tests to verify link rendering behavior: for
email, test that valid addresses render as mailto links and invalid ones as
plain text without links; for company, test that names starting with '@' render
as GitHub links and others as plain text. Use the provided example tests as a
guide to implement these specific and comprehensive checks.

})
})
test('does not render sponsor block', async () => {
Expand Down
63 changes: 62 additions & 1 deletion frontend/src/components/CardDetailsPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
} from '@fortawesome/free-solid-svg-icons'
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'
import Link from 'next/link'
import { useCallback } from 'react'
import type { JSX } from 'react'
import type { DetailsCardProps } from 'types/card'
import { capitalize } from 'utils/capitalize'
import { IS_PROJECT_HEALTH_ENABLED } from 'utils/credentials'
Expand All @@ -29,6 +31,8 @@ import SecondaryCard from 'components/SecondaryCard'
import SponsorCard from 'components/SponsorCard'
import ToggleableList from 'components/ToggleableList'
import TopContributorsList from 'components/TopContributorsList'
const EMAIL_REGEX = /^[^\s@]+@[^\s@]+\.[a-z]{2,}$/i
const sanitizeForUrl = (str: string) => encodeURIComponent(str.trim())

const DetailsCard = ({
description,
Expand All @@ -55,6 +59,63 @@ const DetailsCard = ({
type,
userSummary,
}: DetailsCardProps) => {
const renderDetailValue = useCallback(
(detail: { label: string; value: string | JSX.Element }) => {
const { label, value } = detail

if (
value == null ||
value === '' ||
value === 'N/A' ||
value === 'Not available' ||
value === 'Unknown'
) {
return 'Unknown'
}

if (typeof value !== 'string') {
return value
}

switch (label) {
case 'Email':
if (!EMAIL_REGEX.test(value)) {
return value
}
return (
<a
href={`mailto:${sanitizeForUrl(value)}`}
className="text-blue-400 hover:underline"
aria-label={`Send email to ${value}`}
>
{value}
</a>
)

case 'Company':
if (value.startsWith('@')) {
const companyName = sanitizeForUrl(value.slice(1))
return (
<a
href={`https://github.com/${companyName}`}
target="_blank"
rel="noopener noreferrer"
className="text-blue-400 hover:underline"
aria-label={`Visit ${value} on GitHub`}
>
{value}
</a>
)
}
return value

default:
return value
}
},
[]
)

return (
<div className="min-h-screen bg-white p-8 text-gray-600 dark:bg-[#212529] dark:text-gray-300">
<div className="mx-auto max-w-6xl">
Expand Down Expand Up @@ -108,7 +169,7 @@ const DetailsCard = ({
</div>
) : (
<div key={detail.label} className="pb-1">
<strong>{detail.label}:</strong> {detail?.value || 'Unknown'}
<strong>{detail.label}:</strong> {renderDetailValue(detail)}
</div>
)
)}
Expand Down