DEMOS-1056 and DEMOS-1067 - Manage Contacts Updates #513
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
nunesmlucas
changed the title
tresdonicf
left a comment
tresdonicf
left a comment
There was a problem hiding this comment.
I believe this probably makes the codebase better but am having a tough time reasoning about what it currently does or is supposed to do - could you put some logic inside helper functions to try to clear this up?
| const currentRoles = selectedContacts.filter((c) => c.contactType); | ||
|
|
||
| const currentPrimaryPO = currentRoles.find( | ||
| (c) => c.contactType === "Project Officer" && c.isPrimary | ||
| ); | ||
| const savedPrimaryPO = savedContacts.find( | ||
| (c) => c.contactType === "Project Officer" && c.isPrimary | ||
| ); | ||
|
|
||
| if ( | ||
| currentPrimaryPO && | ||
| savedPrimaryPO && | ||
| currentPrimaryPO.personId !== savedPrimaryPO.personId | ||
| ) { | ||
| await setDemonstrationRoles({ | ||
| variables: { | ||
| input: [ | ||
| { | ||
| demonstrationId, | ||
| personId: currentPrimaryPO.personId, | ||
| roleId: "Project Officer", | ||
| isPrimary: true, | ||
| }, | ||
| ], | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Would be nice to throw some stuff like this in a helper function, it's a bit hard to follow what this currently does or is supposed to do
|
|
||
| const handleSearch = (value: string) => { | ||
| setSearchTerm(value); | ||
| if (value.length < 2) { |
There was a problem hiding this comment.
NB: Would be nice to declare const MIN_CHARACTERS_FOR_SEARCH = 2 at the top of the file
connor-parke-icf
left a comment
connor-parke-icf
left a comment
There was a problem hiding this comment.
I feel like there may be more validation rules we're introducing here than are enforced in the database. I feel like there may have been conflicting requirements or a recent update that isnt in sync with what the database expects.
The main validation rules i was aware of are that
- a demonstration must have exactly one primary project officer
- a person can only be assigned to a role on a demonstration if they belong to the same state as the demonstration
- a person can only be assigned to a role that is allowed by their personType (idm role)
- a maximum of one assignment of each type can be the primary for that role
im seeing some validations around not being able to delete role assignments on primary non-project-officer roles for instance, that might should be allowed instead. Its kinda hard to tell, but i feel like we might can simplify some of the logic here; but i dont know if the requirements have changed. if they have we might should update the database to match (as a separate ticket).
ManageContactsDialog Improvements
This PR implements three key fixes to improve the contact management functionality: Fixed search filtering bug where the search field was showing all contacts instead of filtered results by adding robust client-side filtering with deduplication logic, Resolved Apollo constraint error "Cannot remove the primary project officer" by reordering mutations to set new primary contacts before removing old ones, and Enhanced user experience by removing auto-timeout behavior for primary contact warnings so they persist until the user explicitly saves or cancels the dialog.