Skip to content

feat(directory): Phase 3 — Reviews / MDM / Tags / Teams / DP+DC team-member#413

Open
larsgeorge-db wants to merge 1 commit into
feat/directory-phase2from
feat/directory-phase3
Open

feat(directory): Phase 3 — Reviews / MDM / Tags / Teams / DP+DC team-member#413
larsgeorge-db wants to merge 1 commit into
feat/directory-phase2from
feat/directory-phase3

Conversation

@larsgeorge-db
Copy link
Copy Markdown
Collaborator

Phase 3 of the directory-lookup-and-principal-picker plan (#375). Stacked on #412 — base is feat/directory-phase2 so the diff here is Phase-3 only. Rebases onto main once the rest of the stack lands.

Plan: #375
Backend: #406
Phase 1 frontend: #407
Phase 2: #412

Summary

Five mechanical migrations onto PrincipalPicker. No new backend routes; every existing endpoint accepts the same payload shape it already did.

Surface Field Picker config Backend payload
Data Asset Reviews — Create reviewer_email single, user unchanged
MDM — Create Review reviewer_email single, user (RHF Controller) unchanged
Tag-namespace ACL permissionForm.group_id single, group unchanged
Teams form member_identifier single, dynamicaccepts flips between ['user'] and ['group'] on the row's member_type toggle unchanged
Data-product team-member username single, user unchanged (username)
Data-contract team-member email/username single, user unchanged (still populates both username and email for ODCS backward compat)

Helpers added

  • lib/team-members.ts:
    • principalAcceptsForMemberType(memberType) — Teams type-switching.
    • buildContractTeamMember({emailOrUsername, role, name}) — DC dual-population helper (trims, omits name when blank, populates both username and email with the same picked id).

Test plan

  • Type-check clean (yarn type-check).
  • Full frontend suite: 705 passed, 6 skipped, 0 failed.
  • New tests (6, lib/team-members.test.ts):
    • principalAcceptsForMemberType: 'user'['user'], 'group'['group'], defaults to ['user'] for null / undefined / '' / unrecognised values.
    • buildContractTeamMember: populates both username and email with the picked id; trims surrounding whitespace on every field; omits name entirely when blank or undefined.
  • Per-page form-dialog mounts continue to be deferred to Playwright E2E (Radix dialogs hang in jsdom — same convention as the existing skipped role-form-dialog.test, team-form-dialog.test, project-form-dialog.test, tag-selector.test).
  • Manual smoke test of each surface in both configured + unconfigured modes (left for reviewer / follow-up).

Acceptance criteria from the plan (Phase 3)

  • All five surfaces accept picker output and round-trip through their existing endpoints with no payload-shape change.
  • In Teams, switching member_type from user to group resets the picker's accepts filter without losing form state for the other row fields (per-row picker remounts; sibling rows and the row's role / domain / project fields stay intact).
  • Tooltips on selected badges across all surfaces expose the underlying identifier (carried over from the Phase 1 picker).
  • Pre-existing reviewer assignments, MDM reviews, tag permissions, team rosters, and product/contract team members continue to render and save identically when the directory is not configured (picker auto-degrades to plain manual entry).
  • Frontend tests cover at least Reviews + MDM (single-user) and Teams (type-switching) flows — via the extracted helpers; full-mount Radix tests deferred to E2E per repo convention.

What's intentionally not here

  • Workflow Designer "Custom principals" toggle (notification + approval recipients/approvers).
  • Data-contract wizard owner/stakeholders/groups/primary-support-email wiring.

Both land in Phase 4.

…member

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 ``@``.
@larsgeorge-db larsgeorge-db requested a review from a team as a code owner May 21, 2026 13:01
larsgeorge-db added a commit that referenced this pull request May 21, 2026
…r scope

Documents what shipped under PRs #406 / #407 / #412 / #413 / #416 / #417:

- Renames the integration's manager / routes / settings keys in the
  PRD to match the implementation (Directory layer, /api/directory/*,
  DIRECTORY_* settings, Settings → Directory tab).
- Documents the DirectoryProvider interface and the
  (DirectoryProviderContext, DirectoryProviderConfig) factory
  signature so future provider plug-ins know what to implement.
- Documents the v1 provider set, which expanded during planning
  from Entra-only to entra + lakebase + file. The Lakebase table
  schema and CSV format are included so operators have a single
  reference.
- Preserves story content, the disambiguation rule, both picker
  modes, storage-compatibility guarantees, and graceful-degradation
  rules from the PRD body unchanged.
- Re-confirms the out-of-scope list (Okta/Ping, service principals,
  OBO, profile photos, manager hierarchy, role/team Select replacement,
  CSV bulk import) which the abstraction makes cheap to revisit.
larsgeorge-db added a commit that referenced this pull request May 21, 2026
…r scope

Documents what shipped under PRs #406 / #407 / #412 / #413 / #416 / #417:

- Renames the integration's manager / routes / settings keys in the
  PRD to match the implementation (Directory layer, /api/directory/*,
  DIRECTORY_* settings, Settings → Directory tab).
- Documents the DirectoryProvider interface and the
  (DirectoryProviderContext, DirectoryProviderConfig) factory
  signature so future provider plug-ins know what to implement.
- Documents the v1 provider set, which expanded during planning
  from Entra-only to entra + lakebase + file. The Lakebase table
  schema and CSV format are included so operators have a single
  reference.
- Preserves story content, the disambiguation rule, both picker
  modes, storage-compatibility guarantees, and graceful-degradation
  rules from the PRD body unchanged.
- Re-confirms the out-of-scope list (Okta/Ping, service principals,
  OBO, profile photos, manager hierarchy, role/team Select replacement,
  CSV bulk import) which the abstraction makes cheap to revisit.
@larsgeorge-db larsgeorge-db force-pushed the feat/directory-phase2 branch from 6778b9a to 58eaf6b Compare May 21, 2026 21:35
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.

1 participant