From a7d5080e6db74c17d9c0bf738ffb489831bd7f3b Mon Sep 17 00:00:00 2001 From: Lars George Date: Thu, 21 May 2026 14:54:21 +0200 Subject: [PATCH] =?UTF-8?q?feat(directory):=20Phase=203=20=E2=80=94=20Revi?= =?UTF-8?q?ews=20/=20MDM=20/=20Tags=20/=20Teams=20/=20DP+DC=20team-member?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacks on Phase 2 (#412). Five mechanical surfaces migrated onto PrincipalPicker per plan #375. No backend changes. UI changes: - data-asset-reviews/create-review-request-dialog.tsx — reviewer_email Input -> PrincipalPicker(single, user). - mdm/create-review-dialog.tsx — reviewer_email Input (RHF-managed) wrapped in a Controller bound to PrincipalPicker(single, user). - settings/tags-settings.tsx — permissionForm.group_id Input -> PrincipalPicker(single, group). Backend ACL contract unchanged. - teams/team-form-dialog.tsx — member_identifier Input -> PrincipalPicker, accepts narrows on member_type via the new principalAcceptsForMemberType helper. - data-products/team-member-form-dialog.tsx — username Input -> PrincipalPicker(single, user). Backend payload (username) unchanged. - data-contracts/team-member-form-dialog.tsx — email/username Input -> PrincipalPicker(single, user). buildContractTeamMember helper preserves the dual ``username`` + ``email`` field population the ODCS endpoint expects. Helpers (pure, fully tested): - lib/team-members.ts — principalAcceptsForMemberType for Teams type-switching, buildContractTeamMember for the DC dual-population. Tests (6 new, 705 total passing): - lib/team-members.test.ts: accepts filter narrows correctly for user/group/unknown member_type; defaults to ['user'] on null / undefined / unrecognised values. buildContractTeamMember populates both username and email with the picked id, trims whitespace on every field, and omits ``name`` entirely when blank. Per-page form-dialog mounts continue to be deferred to E2E (Radix dialogs hang in jsdom — same convention as the existing skipped role/team/project form tests). The DC dialog still preserves its basic ``@`` -> email-shape validation; PrincipalPicker can emit either an email or a plain username so the regex only fires when the value contains ``@``. --- .../create-review-request-dialog.tsx | 16 ++-- .../team-member-form-dialog.tsx | 21 ++--- .../data-products/team-member-form-dialog.tsx | 12 +-- .../components/mdm/create-review-dialog.tsx | 23 ++++-- .../src/components/settings/tags-settings.tsx | 11 ++- .../src/components/teams/team-form-dialog.tsx | 40 +++++++--- src/frontend/src/lib/team-members.test.ts | 80 +++++++++++++++++++ src/frontend/src/lib/team-members.ts | 55 +++++++++++++ 8 files changed, 214 insertions(+), 44 deletions(-) create mode 100644 src/frontend/src/lib/team-members.test.ts create mode 100644 src/frontend/src/lib/team-members.ts diff --git a/src/frontend/src/components/data-asset-reviews/create-review-request-dialog.tsx b/src/frontend/src/components/data-asset-reviews/create-review-request-dialog.tsx index 3c14dc88..7f0a1713 100644 --- a/src/frontend/src/components/data-asset-reviews/create-review-request-dialog.tsx +++ b/src/frontend/src/components/data-asset-reviews/create-review-request-dialog.tsx @@ -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 { @@ -339,16 +340,17 @@ export default function CreateReviewRequestDialog({ isOpen, onOpenChange, api, o />
- - Reviewer * + { - setReviewerEmail(e.target.value); + accepts={['user']} + value={reviewerEmail || null} + onChange={(next) => { + setReviewerEmail(next ?? ''); setFormError(null); }} - required + placeholder="user@example.com" + aria-label="Reviewer" />
diff --git a/src/frontend/src/components/data-contracts/team-member-form-dialog.tsx b/src/frontend/src/components/data-contracts/team-member-form-dialog.tsx index bbf1e456..c4cd01ed 100644 --- a/src/frontend/src/components/data-contracts/team-member-form-dialog.tsx +++ b/src/frontend/src/components/data-contracts/team-member-form-dialog.tsx @@ -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 = { @@ -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) @@ -100,11 +101,13 @@ export default function TeamMemberFormDialog({ isOpen, onOpenChange, onSubmit, i - setEmail(e.target.value)} + accepts={['user']} + value={email || null} + onChange={(next) => setEmail(next ?? '')} placeholder="user@example.com or username" + aria-label="Email or username" /> diff --git a/src/frontend/src/components/data-products/team-member-form-dialog.tsx b/src/frontend/src/components/data-products/team-member-form-dialog.tsx index 13c215b4..25a3639f 100644 --- a/src/frontend/src/components/data-products/team-member-form-dialog.tsx +++ b/src/frontend/src/components/data-products/team-member-form-dialog.tsx @@ -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 = { @@ -92,15 +93,16 @@ export default function TeamMemberFormDialog({ isOpen, onOpenChange, onSubmit, i - 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" />

- User's username or email address (REQUIRED in ODPS) + User's username or email address (REQUIRED in ODPS)

diff --git a/src/frontend/src/components/mdm/create-review-dialog.tsx b/src/frontend/src/components/mdm/create-review-dialog.tsx index 0a3651fb..66fbbe8d 100644 --- a/src/frontend/src/components/mdm/create-review-dialog.tsx +++ b/src/frontend/src/components/mdm/create-review-dialog.tsx @@ -12,7 +12,6 @@ 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'; @@ -20,6 +19,8 @@ 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'), @@ -129,12 +130,20 @@ export default function CreateReviewDialog({
- - Reviewer + ( + field.onChange(next ?? '')} + placeholder="data-steward@company.com" + aria-label="Reviewer" + /> + )} /> {form.formState.errors.reviewer_email && (

diff --git a/src/frontend/src/components/settings/tags-settings.tsx b/src/frontend/src/components/settings/tags-settings.tsx index c8344b6c..4faa98ef 100644 --- a/src/frontend/src/components/settings/tags-settings.tsx +++ b/src/frontend/src/components/settings/tags-settings.tsx @@ -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'; @@ -796,12 +797,14 @@ export default function TagsSettings() {

- - Group + 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" />
diff --git a/src/frontend/src/components/teams/team-form-dialog.tsx b/src/frontend/src/components/teams/team-form-dialog.tsx index ad8d6402..e85c049c 100644 --- a/src/frontend/src/components/teams/team-form-dialog.tsx +++ b/src/frontend/src/components/teams/team-form-dialog.tsx @@ -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, @@ -416,18 +418,32 @@ export function TeamFormDialog({ ( - - {t('teams:form.labels.identifier')} - - - - - - )} + 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 ( + + {t('teams:form.labels.identifier')} + + field.onChange(next ?? '')} + placeholder={ + memberType === 'group' + ? t('teams:form.placeholders.groupName') + : t('teams:form.placeholders.userEmail') + } + aria-label={t('teams:form.labels.identifier')} + /> + + + + ); + }} /> { + 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); + }); +}); diff --git a/src/frontend/src/lib/team-members.ts b/src/frontend/src/lib/team-members.ts new file mode 100644 index 00000000..954cab96 --- /dev/null +++ b/src/frontend/src/lib/team-members.ts @@ -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[] { + 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 } : {}), + }; +}