Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { useToast } from "@/hooks/use-toast";
import { TreeView } from '@/components/ui/tree-view';
import { CatalogItem, DataAssetReviewRequest, DataAssetReviewRequestCreate, AssetType } from '@/types/data-asset-review';
import { Loader2, AlertCircle, Folder, FolderOpen, Table, Layout, FunctionSquare } from 'lucide-react';
import { PrincipalPicker } from '@/components/common/principal-picker';

// Define user info type based on backend response
interface UserInfo {
Expand Down Expand Up @@ -339,16 +340,17 @@ export default function CreateReviewRequestDialog({ isOpen, onOpenChange, api, o
/>
</div>
<div>
<Label htmlFor="reviewer-email">Reviewer Email *</Label>
<Input
<Label htmlFor="reviewer-email">Reviewer *</Label>
<PrincipalPicker
id="reviewer-email"
type="email"
value={reviewerEmail}
onChange={(e) => {
setReviewerEmail(e.target.value);
accepts={['user']}
value={reviewerEmail || null}
onChange={(next) => {
setReviewerEmail(next ?? '');
setFormError(null);
}}
required
placeholder="user@example.com"
aria-label="Reviewer"
/>
</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Button } from '@/components/ui/button'
import { Input } from '@/components/ui/input'
import { Label } from '@/components/ui/label'
import { useToast } from '@/hooks/use-toast'
import { PrincipalPicker } from '@/components/common/principal-picker'
import { buildContractTeamMember } from '@/lib/team-members'
import type { TeamMember } from '@/types/data-contract'

type TeamMemberFormProps = {
Expand Down Expand Up @@ -53,12 +55,11 @@ export default function TeamMemberFormDialog({ isOpen, onOpenChange, onSubmit, i

setIsSubmitting(true)
try {
const member: TeamMember = {
username: email.trim(), // ODCS uses username
role: role.trim(),
email: email.trim(), // Keep for backward compatibility
name: name.trim() || undefined,
}
const member: TeamMember = buildContractTeamMember({
emailOrUsername: email,
role,
name,
})

await onSubmit(member)
onOpenChange(false)
Expand Down Expand Up @@ -100,11 +101,13 @@ export default function TeamMemberFormDialog({ isOpen, onOpenChange, onSubmit, i
<Label htmlFor="email">
Email/Username <span className="text-destructive">*</span>
</Label>
<Input
<PrincipalPicker
id="email"
value={email}
onChange={(e) => setEmail(e.target.value)}
accepts={['user']}
value={email || null}
onChange={(next) => setEmail(next ?? '')}
placeholder="user@example.com or username"
aria-label="Email or username"
/>
</div>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { Input } from '@/components/ui/input';
import { Label } from '@/components/ui/label';
import { Textarea } from '@/components/ui/textarea';
import { useToast } from '@/hooks/use-toast';
import { PrincipalPicker } from '@/components/common/principal-picker';
import type { TeamMember } from '@/types/data-product';

type TeamMemberFormProps = {
Expand Down Expand Up @@ -92,15 +93,16 @@ export default function TeamMemberFormDialog({ isOpen, onOpenChange, onSubmit, i
<Label htmlFor="username">
Username <span className="text-destructive">*</span>
</Label>
<Input
<PrincipalPicker
id="username"
value={username}
onChange={(e) => setUsername(e.target.value)}
accepts={['user']}
value={username || null}
onChange={(next) => setUsername(next ?? '')}
placeholder="e.g., user@example.com or username"
autoFocus
aria-label="Username"
/>
<p className="text-xs text-muted-foreground">
User's username or email address (REQUIRED in ODPS)
User&apos;s username or email address (REQUIRED in ODPS)
</p>
</div>

Expand Down
23 changes: 16 additions & 7 deletions src/frontend/src/components/mdm/create-review-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,15 @@ import {
DialogTitle,
} from '@/components/ui/dialog';
import { Button } from '@/components/ui/button';
import { Input } from '@/components/ui/input';
import { Label } from '@/components/ui/label';
import { Textarea } from '@/components/ui/textarea';
import { Loader2, FileCheck } from 'lucide-react';

import { useApi } from '@/hooks/use-api';
import { useToast } from '@/hooks/use-toast';
import { MdmCreateReviewRequest, MdmCreateReviewResponse } from '@/types/mdm';
import { PrincipalPicker } from '@/components/common/principal-picker';
import { Controller } from 'react-hook-form';

const formSchema = z.object({
reviewer_email: z.string().email('Valid email is required'),
Expand Down Expand Up @@ -129,12 +130,20 @@ export default function CreateReviewDialog({
</div>

<div className="space-y-2">
<Label htmlFor="reviewer_email">Reviewer Email</Label>
<Input
id="reviewer_email"
type="email"
placeholder="data-steward@company.com"
{...form.register('reviewer_email')}
<Label htmlFor="reviewer_email">Reviewer</Label>
<Controller
name="reviewer_email"
control={form.control}
render={({ field }) => (
<PrincipalPicker
id="reviewer_email"
accepts={['user']}
value={field.value || null}
onChange={(next) => field.onChange(next ?? '')}
placeholder="data-steward@company.com"
aria-label="Reviewer"
/>
)}
/>
{form.formState.errors.reviewer_email && (
<p className="text-sm text-destructive">
Expand Down
11 changes: 7 additions & 4 deletions src/frontend/src/components/settings/tags-settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import { Label } from '@/components/ui/label';
import { Textarea } from '@/components/ui/textarea';
import { useApi } from '@/hooks/use-api';
import { useToast } from '@/hooks/use-toast';
import { PrincipalPicker } from '@/components/common/principal-picker';
import { RelativeDate } from '@/components/common/relative-date';
import { useAppSettingsStore } from '@/stores/app-settings-store';

Expand Down Expand Up @@ -796,12 +797,14 @@ export default function TagsSettings() {
</DialogHeader>
<div className="space-y-4">
<div>
<Label htmlFor="permission-group">Group ID</Label>
<Input
<Label htmlFor="permission-group">Group</Label>
<PrincipalPicker
id="permission-group"
value={permissionForm.group_id}
onChange={(e) => setPermissionForm({ ...permissionForm, group_id: e.target.value })}
accepts={['group']}
value={permissionForm.group_id || null}
onChange={(next) => setPermissionForm({ ...permissionForm, group_id: next ?? '' })}
placeholder="e.g., data-engineers, analysts"
aria-label="Group"
/>
</div>
<div>
Expand Down
40 changes: 28 additions & 12 deletions src/frontend/src/components/teams/team-form-dialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { Button } from '@/components/ui/button';
import { Input } from '@/components/ui/input';
import { Textarea } from '@/components/ui/textarea';
import TagSelector from '@/components/ui/tag-selector';
import { PrincipalPicker } from '@/components/common/principal-picker';
import { principalAcceptsForMemberType } from '@/lib/team-members';
import {
Dialog,
DialogContent,
Expand Down Expand Up @@ -416,18 +418,32 @@ export function TeamFormDialog({
<FormField
control={form.control}
name={`members.${index}.member_identifier`}
render={({ field }) => (
<FormItem>
<FormLabel>{t('teams:form.labels.identifier')}</FormLabel>
<FormControl>
<Input
placeholder={form.watch(`members.${index}.member_type`) === 'user' ? t('teams:form.placeholders.userEmail') : t('teams:form.placeholders.groupName')}
{...field}
/>
</FormControl>
<FormMessage />
</FormItem>
)}
render={({ field }) => {
// Picker accepts narrows to the chosen
// member_type so users only see users in
// the dropdown, groups only see groups.
const memberType = form.watch(`members.${index}.member_type`);
const accepts = principalAcceptsForMemberType(memberType);
return (
<FormItem>
<FormLabel>{t('teams:form.labels.identifier')}</FormLabel>
<FormControl>
<PrincipalPicker
accepts={accepts}
value={field.value || null}
onChange={(next) => field.onChange(next ?? '')}
placeholder={
memberType === 'group'
? t('teams:form.placeholders.groupName')
: t('teams:form.placeholders.userEmail')
}
aria-label={t('teams:form.labels.identifier')}
/>
</FormControl>
<FormMessage />
</FormItem>
);
}}
/>

<FormField
Expand Down
80 changes: 80 additions & 0 deletions src/frontend/src/lib/team-members.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* Tests for the Phase 3 team-member helpers.
*
* - principalAcceptsForMemberType pins the Teams row's picker filter to
* the chosen member_type. This is the Phase 3 acceptance criterion
* "switching member_type from user to group resets the picker's
* accepts filter".
* - buildContractTeamMember pins the dual ``username`` + ``email``
* population that keeps the ODCS endpoint happy after migration.
*/

import { describe, expect, it } from 'vitest';

import {
buildContractTeamMember,
principalAcceptsForMemberType,
} from './team-members';

describe('principalAcceptsForMemberType', () => {
it('returns ["user"] for "user"', () => {
expect(principalAcceptsForMemberType('user')).toEqual(['user']);
});

it('returns ["group"] for "group"', () => {
expect(principalAcceptsForMemberType('group')).toEqual(['group']);
});

it('defaults to ["user"] for null / undefined / unknown values', () => {
expect(principalAcceptsForMemberType(null)).toEqual(['user']);
expect(principalAcceptsForMemberType(undefined)).toEqual(['user']);
expect(principalAcceptsForMemberType('')).toEqual(['user']);
expect(principalAcceptsForMemberType('robot')).toEqual(['user']);
});
});

describe('buildContractTeamMember', () => {
it('populates both username and email with the picked principal id', () => {
expect(
buildContractTeamMember({
emailOrUsername: 'alice@example.com',
role: 'Data Owner',
name: 'Alice',
}),
).toEqual({
username: 'alice@example.com',
email: 'alice@example.com',
role: 'Data Owner',
name: 'Alice',
});
});

it('trims surrounding whitespace on every field', () => {
expect(
buildContractTeamMember({
emailOrUsername: ' bob@x ',
role: ' Steward ',
name: ' Bob ',
}),
).toEqual({
username: 'bob@x',
email: 'bob@x',
role: 'Steward',
name: 'Bob',
});
});

it('omits name entirely when blank / undefined', () => {
const m1 = buildContractTeamMember({
emailOrUsername: 'c@x',
role: 'r',
name: ' ',
});
expect(m1).toEqual({ username: 'c@x', email: 'c@x', role: 'r' });
expect('name' in m1).toBe(false);

const m2 = buildContractTeamMember({ emailOrUsername: 'd@x', role: 'r' });
expect(m2).toEqual({ username: 'd@x', email: 'd@x', role: 'r' });
expect('name' in m2).toBe(false);
});
});
55 changes: 55 additions & 0 deletions src/frontend/src/lib/team-members.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/**
* Helpers shared by the Teams form, Data-product team-member, and
* Data-contract team-member dialogs after their Phase 3 migration
* onto ``PrincipalPicker``.
*
* Kept as pure functions so the integration logic is unit-testable
* without mounting the Radix dialogs (which hang in jsdom in this
* repo -- see the skipped ``team-form-dialog.test.tsx`` for context).
*/

import type { PrincipalType } from '@/types/directory';

/**
* Narrow the picker's ``accepts`` filter based on the Teams row's
* ``member_type``. ``user`` is the default; any value other than
* ``group`` falls back to it so unrecognised future values are
* handled gracefully.
*/
export function principalAcceptsForMemberType(
memberType: string | null | undefined,
): Exclude<PrincipalType, 'unknown'>[] {
return memberType === 'group' ? ['group'] : ['user'];
}

/**
* Construct the ODCS-compatible team-member payload from the
* data-contract dialog state. ODCS uses ``username``; the existing
* backend still inspects ``email`` for backward compatibility, so we
* populate both with the same picked principal id.
*/
export interface ContractTeamMemberInput {
emailOrUsername: string;
role: string;
name?: string | null;
}

export interface ContractTeamMember {
username: string;
email: string;
role: string;
name?: string;
}

export function buildContractTeamMember(
input: ContractTeamMemberInput,
): ContractTeamMember {
const trimmed = input.emailOrUsername.trim();
const name = input.name?.trim();
return {
username: trimmed,
email: trimmed,
role: input.role.trim(),
...(name ? { name } : {}),
};
}