fix: show email in person dropdown to distinguish duplicate names #2642
Conversation
✅ Deploy Preview for flanksource-demo-stable ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for clerk-saas-ui ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for goofy-euclid-75956c ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull Request Overview
Adds email display in the people picker to distinguish duplicate names and enhances permission editing by fetching full permission details. Also restructures permission resource/object fields and updates default subject selection logic.
- Show email alongside person name in dropdown options.
- Fetch full permission data on edit and refactor resource/object field names (e.g. config_object, playbook_object).
- Adjust subject type initialization and resource selection UI logic.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/Permissions/PermissionsTable.tsx | Switch to new *_object fields and add contextual labels for displayed related objects. |
| src/components/Permissions/ManagePermissions/Forms/PermissionSubjectControls.tsx | Change default subject to Person and add effect to initialize subject_type. |
| src/components/Permissions/ManagePermissions/Forms/PermissionForm.tsx | Add fetch-on-edit logic, spinner state, and refactor to use permissionData abstraction. |
| src/components/Permissions/ManagePermissions/Forms/FormikPermissionSelectResourceFields.tsx | Add dynamic initial tab selection based on existing form values. |
| src/components/Forms/Formik/FormikPeopleDropdown.tsx | Display person email beside name in dropdown. |
| src/api/types/permissions.ts | Strongly type object field and introduce explicit *_object fields and selector typing. |
| src/api/services/permissions.ts | Add fetchPermissionById helper. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| export async function fetchPermissionById(id: string) { | ||
| const response = await IncidentCommander.get<PermissionTable[]>( | ||
| `/permissions?id=eq.${id}` | ||
| ); |
There was a problem hiding this comment.
If the API returns an empty array or an error scenario masked as a 200 with no rows, this returns undefined silently, causing the edit flow to proceed as if creating a new permission. Consider validating the response and throwing (or returning a rejected promise) when no record is found: e.g. if (!response.data.length) throw new Error('Permission not found'); so the UI can show an error state instead of falling back to 'Add'.
| ); | |
| ); | |
| if (!response.data || response.data.length === 0) { | |
| throw new Error('Permission not found'); | |
| } |
Copilot uses AI. Check for mistakes.
| ); | ||
|
|
||
| // Use fetched data if editing, otherwise use provided data | ||
| const permissionData = permissionId ? fetchedPermission : data; |
There was a problem hiding this comment.
If the fetch completes with an error or returns no record, fetchedPermission will be undefined and permissionData becomes undefined, causing the form to treat an edit as a creation (title, submit path) and potentially insert a duplicate. Use the stable permissionId or original data as a fallback (e.g. const permissionData = permissionId ? fetchedPermission ?? data : data) and gate add vs update logic on permissionId, not permissionData?.id.
| const permissionData = permissionId ? fetchedPermission : data; | |
| const permissionData = permissionId ? (fetchedPermission ?? data) : data; |
Copilot uses AI. Check for mistakes.
| if (!permissionData?.id) { | ||
| return add({ | ||
| ...v | ||
| } as PermissionTable); |
There was a problem hiding this comment.
Relying on permissionData?.id to choose add vs update can misclassify an edit when permissionData failed to load; this will attempt an add instead of update. Use the original permissionId prop (or a separate flag like !!permissionId) to decide the branch, ensuring failed fetches do not create duplicates.
Copilot uses AI. Check for mistakes.
src/components/Permissions/ManagePermissions/Forms/PermissionForm.tsx
Outdated
Show resolved
Hide resolved
src/components/Permissions/ManagePermissions/Forms/PermissionSubjectControls.tsx
Show resolved
Hide resolved
resolves: #2641