diff --git a/src/__testing__/UserSearchFieldInput.test.tsx b/src/__testing__/UserSearchFieldInput.test.tsx index f6e421caa..b6d8d7280 100644 --- a/src/__testing__/UserSearchFieldInput.test.tsx +++ b/src/__testing__/UserSearchFieldInput.test.tsx @@ -66,3 +66,109 @@ describe('UserSearchField type-ahead wiring', () => { expect(screen.queryByText('Jane Doe')).not.toBeNull(); }); }); + +describe('UserSearchField v1beta3 user records', () => { + // The v1beta3 user construct identifies users by `id` (the wire `userId` is + // a deprecated alias) and reduced projections may carry only a username. + const v1beta3Alex = { + id: 'u-alex', + username: 'alex', + firstName: 'Alex', + lastName: 'Rivera', + email: 'alex@example.com' + }; + const usernameOnly = { id: 'u-min', username: 'minimal-user' }; + + it('renders and matches records that carry only the canonical id', async () => { + const { input } = renderField({ + usersSearch: 'a', + searchedUsers: [v1beta3Alex, usernameOnly], + // same person as v1beta3Alex, but shaped like a legacy record + currentUserData: { userId: 'u-alex' } + }); + + fireEvent.change(input, { target: { value: 'a' } }); + + // current user is filtered out via id<->userId identity matching + await waitFor(() => { + expect(screen.queryByText('minimal-user')).not.toBeNull(); + }); + expect(screen.queryByText('Alex Rivera')).toBeNull(); + }); + + it('renders soft-deleted v1beta3 records (deletedAt) as deleted options', async () => { + // v1beta3 signals soft deletion via `deletedAt`, not the component-local + // `deleted` boolean; both must render the "(deleted)" treatment. + const deletedByTimestamp = { + id: 'u-gone', + email: 'gone@example.com', + deletedAt: '2026-01-01T00:00:00Z' + }; + const { input } = renderField({ + usersSearch: 'gone', + searchedUsers: [deletedByTimestamp] + }); + + fireEvent.change(input, { target: { value: 'gone' } }); + + await waitFor(() => { + expect(screen.queryByText('gone@example.com (deleted)')).not.toBeNull(); + }); + }); + + it('removes only the targeted record when selected users carry no identifier', () => { + // Email-only invitees all collapse onto the '' identifier, so filtering + // by identifier string deleted every one of them at once. Deletion must + // match the record itself (isSameUser, then reference equality). + const inviteeOne = { email: 'one@example.com' }; + const inviteeTwo = { email: 'two@example.com' }; + const { props, container } = renderField({ usersData: [inviteeOne, inviteeTwo] }); + + fireEvent.click(screen.getByText('(+1)')); + + const deleteIcons = container.querySelectorAll('.MuiChip-deleteIcon'); + expect(deleteIcons).toHaveLength(2); + fireEvent.click(deleteIcons[0]); + + expect(props.setUsersData).toHaveBeenCalledWith([inviteeTwo]); + }); + + it('removes records carrying neither identifier nor email by reference', () => { + // isSameUser never matches two records with nothing to compare; reference + // equality is the last resort that keeps such rows deletable. + const ghost = { username: 'ghost' }; + const other = { username: 'other' }; + const { props, container } = renderField({ usersData: [ghost, other] }); + + fireEvent.click(screen.getByText('(+1)')); + + const deleteIcons = container.querySelectorAll('.MuiChip-deleteIcon'); + expect(deleteIcons).toHaveLength(2); + fireEvent.click(deleteIcons[0]); + + expect(props.setUsersData).toHaveBeenCalledWith([other]); + }); + + it('keeps MUI-generated option ids so aria-activedescendant resolves', async () => { + // renderOption must spread the Autocomplete-provided props untouched: + // overriding the option `id` with the user identifier orphans the + // input's aria-activedescendant reference and breaks keyboard focus. + const { input } = renderField({ + usersSearch: 'a', + searchedUsers: [v1beta3Alex, usernameOnly] + }); + + fireEvent.change(input, { target: { value: 'a' } }); + await waitFor(() => { + expect(screen.getAllByRole('option').length).toBeGreaterThan(0); + }); + + fireEvent.keyDown(input, { key: 'ArrowDown' }); + + const activeDescendant = input.getAttribute('aria-activedescendant'); + expect(activeDescendant).toBeTruthy(); + const highlighted = document.getElementById(activeDescendant as string); + expect(highlighted).not.toBeNull(); + expect(highlighted?.getAttribute('role')).toBe('option'); + }); +}); diff --git a/src/__testing__/UserShareSearch.test.tsx b/src/__testing__/UserShareSearch.test.tsx new file mode 100644 index 000000000..2fde982e4 --- /dev/null +++ b/src/__testing__/UserShareSearch.test.tsx @@ -0,0 +1,119 @@ +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; +import UserShareSearch from '../custom/UserSearchField/UserSearchField'; +import { SistentThemeProvider } from '../theme'; + +// Regression guard for the Share Design people-picker against the v1beta3 +// user construct. The searchable collaboration projection identifies users by +// `id` (the wire `userId` is a deprecated alias that newer providers omit) +// and may omit names/email on reduced projections. The picker must: +// - display first/last name and email for searchable-projection rows +// - fall back to username when a reduced projection omits names/email +// - key selection/identity on `id`, not on the retired `userId` + +type SearchProps = React.ComponentProps; + +const v1beta3Jane = { + id: 'b6e6d123-0000-4000-8000-000000000001', + username: 'jdoe', + firstName: 'Jane', + lastName: 'Doe', + email: 'jane@example.com' +}; + +const publicOnlyUser = { + id: 'b6e6d123-0000-4000-8000-000000000003', + username: 'publico' +}; + +const makeUseGetAllUsersQuery = + (users: unknown[]) => (_args: unknown, options?: { skip?: boolean }) => ({ + data: options?.skip ? undefined : { data: users }, + isLoading: false + }); + +const renderSearch = (overrides: Partial = {}) => { + const props: SearchProps = { + usersData: [], + shareWithNewUsers: jest.fn().mockResolvedValue({ error: '' }), + useGetAllUsersQuery: makeUseGetAllUsersQuery([v1beta3Jane, publicOnlyUser]), + ...overrides + }; + const utils = render( + + + + ); + const input = screen.getByRole('combobox') as HTMLInputElement; + return { props, input, ...utils }; +}; + +const typeSearch = async (input: HTMLInputElement, value: string) => { + fireEvent.change(input, { target: { value } }); + // the query is debounced by 300ms + await waitFor(() => expect(screen.queryByText('jane@example.com')).not.toBeNull(), { + timeout: 2000 + }); +}; + +describe('UserShareSearch with v1beta3 user records', () => { + it('displays name and email for searchable-projection rows', async () => { + const { input } = renderSearch(); + + await typeSearch(input, 'jane'); + + expect(screen.queryByText('Jane Doe')).not.toBeNull(); + expect(screen.queryByText('jane@example.com')).not.toBeNull(); + }); + + it('falls back to username for rows from reduced projections', async () => { + const { input } = renderSearch(); + + await typeSearch(input, 'p'); + + expect(screen.queryByText('publico')).not.toBeNull(); + }); + + it('selects and shares by canonical id when the wire userId is absent', async () => { + const { props, input } = renderSearch(); + + await typeSearch(input, 'jane'); + fireEvent.click(screen.getByText('Jane Doe')); + + fireEvent.click(screen.getByRole('button', { name: 'Share' })); + + await waitFor(() => + expect(props.shareWithNewUsers).toHaveBeenCalledWith([expect.objectContaining(v1beta3Jane)]) + ); + }); + + it('leaves the search field usable before any user is selected', () => { + // Only the Share button is gated on having a selection. Rendering the + // search field as disabled while it still accepts input tells the user + // the picker is unavailable when it is the only way to make a selection. + const { input } = renderSearch(); + + expect(input.disabled).toBe(false); + expect(input.closest('.MuiOutlinedInput-root')?.className).not.toContain('Mui-disabled'); + expect((screen.getByRole('button', { name: 'Share' }) as HTMLButtonElement).disabled).toBe(true); + }); + + it('disables the search field when the consumer disables the picker', () => { + const { input } = renderSearch({ disabled: true }); + + expect(input.disabled).toBe(true); + }); + + it('does not resurface users that already have access', async () => { + // Existing access list arrives as canonical v1beta3 records (id, no + // userId); the suggestion list must recognize them as the same user. + const { input } = renderSearch({ + usersData: [{ id: v1beta3Jane.id, email: v1beta3Jane.email }] + }); + + fireEvent.change(input, { target: { value: 'jane' } }); + + await waitFor(() => expect(screen.queryByText('publico')).not.toBeNull(), { timeout: 2000 }); + expect(screen.queryByText('Jane Doe')).toBeNull(); + }); +}); diff --git a/src/__testing__/permissions.test.ts b/src/__testing__/permissions.test.ts new file mode 100644 index 000000000..5cfcae397 --- /dev/null +++ b/src/__testing__/permissions.test.ts @@ -0,0 +1,55 @@ +import { canShareResourceWithNewUsers, canUpdateResource } from '../utils/permissions'; + +// The permission checks accept nullish users by contract: consumers call them +// during auth-state transitions (initial load, logout) when currentUser or the +// owner record is not yet available. No user means no permission. + +const privateResource = { visibility: 'private' }; +const publicResource = { visibility: 'public' }; + +describe('canUpdateResource', () => { + it('grants the resource owner, matching canonical id against the deprecated alias', () => { + expect(canUpdateResource(privateResource, { id: 'u-1' }, { userId: 'u-1' })).toBe(true); + }); + + it('grants admins even when they do not own the resource', () => { + expect( + canUpdateResource(privateResource, { id: 'u-2', roleNames: ['admin'] }, { id: 'u-1' }) + ).toBe(true); + }); + + it('denies non-owner non-admin users', () => { + expect(canUpdateResource(privateResource, { id: 'u-2' }, { id: 'u-1' })).toBe(false); + }); + + it('denies when currentUser is nullish (auth-state transitions)', () => { + expect(canUpdateResource(privateResource, null, { id: 'u-1' })).toBe(false); + expect(canUpdateResource(privateResource, undefined, { id: 'u-1' })).toBe(false); + }); + + it('never treats an identifier-less owner as matching an identifier-less user', () => { + expect(canUpdateResource(privateResource, {}, null)).toBe(false); + expect(canUpdateResource(privateResource, {}, {})).toBe(false); + }); + + it('denies when the resource is nullish, even for admins', () => { + expect(canUpdateResource(null, { id: 'u-1', roleNames: ['admin'] }, { id: 'u-1' })).toBe(false); + expect(canUpdateResource(undefined, { id: 'u-1' }, { id: 'u-1' })).toBe(false); + }); +}); + +describe('canShareResourceWithNewUsers', () => { + it('always allows sharing public resources', () => { + expect(canShareResourceWithNewUsers(publicResource, null, null)).toBe(true); + }); + + it('falls back to the update permission for private resources', () => { + expect(canShareResourceWithNewUsers(privateResource, { id: 'u-1' }, { id: 'u-1' })).toBe(true); + expect(canShareResourceWithNewUsers(privateResource, null, { id: 'u-1' })).toBe(false); + }); + + it('denies when the resource is nullish', () => { + expect(canShareResourceWithNewUsers(null, { id: 'u-1' }, { id: 'u-1' })).toBe(false); + expect(canShareResourceWithNewUsers(undefined, { id: 'u-1' }, { id: 'u-1' })).toBe(false); + }); +}); diff --git a/src/__testing__/userIdentity.test.ts b/src/__testing__/userIdentity.test.ts new file mode 100644 index 000000000..833b2f949 --- /dev/null +++ b/src/__testing__/userIdentity.test.ts @@ -0,0 +1,131 @@ +import { + getUserContactLabel, + getUserDisplayName, + getUserIdentifier, + getUserLabel, + isSameUser +} from '../utils/user'; + +// The v1beta3 user construct retired the wire `userId` (canonical identifier +// is `id`) and the hardened cloud endpoints serve reduced projections: the +// searchable collaboration projection carries names+email, the public +// directory only username+avatar. These helpers are the single place where +// display and identity logic absorbs those wire shapes. + +const v1beta3SearchableUser = { + id: 'b6e6d123-0000-4000-8000-000000000001', + username: 'jdoe', + firstName: 'Jane', + lastName: 'Doe', + email: 'jane@example.com', + avatarUrl: 'https://example.com/a.png' +}; + +const legacyUser = { + userId: 'legacy-0000-4000-8000-000000000002', + firstName: 'Lee', + lastName: 'Smith', + email: 'lee@example.com' +}; + +const publicDirectoryUser = { + id: 'b6e6d123-0000-4000-8000-000000000003', + username: 'publico', + avatarUrl: 'https://example.com/p.png' +}; + +describe('getUserIdentifier', () => { + it('prefers the canonical v1beta3 id', () => { + expect(getUserIdentifier({ id: 'abc', userId: 'legacy' })).toBe('abc'); + }); + + it('falls back to the deprecated userId alias for pre-cutover records', () => { + expect(getUserIdentifier(legacyUser)).toBe('legacy-0000-4000-8000-000000000002'); + }); + + it('returns an empty string when no identifier is present', () => { + expect(getUserIdentifier({})).toBe(''); + expect(getUserIdentifier(null)).toBe(''); + expect(getUserIdentifier(undefined)).toBe(''); + }); +}); + +describe('getUserDisplayName', () => { + it('joins first and last name when the projection carries them', () => { + expect(getUserDisplayName(v1beta3SearchableUser)).toBe('Jane Doe'); + }); + + it('handles a single name part without stray whitespace', () => { + expect(getUserDisplayName({ firstName: 'Jane' })).toBe('Jane'); + expect(getUserDisplayName({ lastName: 'Doe' })).toBe('Doe'); + }); + + it('falls back to username for reduced projections', () => { + expect(getUserDisplayName(publicDirectoryUser)).toBe('publico'); + }); + + it('falls back to email when neither names nor username are present', () => { + expect(getUserDisplayName({ email: 'only@example.com' })).toBe('only@example.com'); + }); + + it('returns an empty string for empty records', () => { + expect(getUserDisplayName({})).toBe(''); + expect(getUserDisplayName(null)).toBe(''); + }); +}); + +describe('getUserContactLabel', () => { + it('prefers email', () => { + expect(getUserContactLabel(v1beta3SearchableUser)).toBe('jane@example.com'); + }); + + it('falls back to username when email is not served', () => { + expect(getUserContactLabel(publicDirectoryUser)).toBe('publico'); + }); + + it('returns an empty string when neither is present', () => { + expect(getUserContactLabel({})).toBe(''); + }); +}); + +describe('getUserLabel', () => { + it('prefers the contact label', () => { + expect(getUserLabel(v1beta3SearchableUser)).toBe('jane@example.com'); + }); + + it('falls back to the display name, then the raw identifier', () => { + expect(getUserLabel({ id: 'u-1', firstName: 'Jane' })).toBe('Jane'); + expect(getUserLabel({ id: 'u-1' })).toBe('u-1'); + }); + + it('yields an empty string only for records with no identifier at all', () => { + expect(getUserLabel({})).toBe(''); + }); +}); + +describe('isSameUser', () => { + it('matches the same object reference even with no comparable fields', () => { + const bare = {}; + expect(isSameUser(bare, bare)).toBe(true); + }); + + it('matches canonical id against the deprecated userId alias', () => { + expect(isSameUser({ id: 'same' }, { userId: 'same' })).toBe(true); + }); + + it('does not match different identifiers even when emails collide', () => { + expect( + isSameUser({ id: 'a', email: 'shared@example.com' }, { id: 'b', email: 'shared@example.com' }) + ).toBe(false); + }); + + it('falls back to email when either record has no identifier', () => { + expect(isSameUser({ email: 'x@example.com' }, { id: 'a', email: 'x@example.com' })).toBe(true); + }); + + it('never matches distinct records with nothing to compare', () => { + expect(isSameUser({}, {})).toBe(false); + expect(isSameUser({ email: '' }, { email: '' })).toBe(false); + expect(isSameUser(null, { id: 'a' })).toBe(false); + }); +}); diff --git a/src/custom/ShareModal/ShareModal.tsx b/src/custom/ShareModal/ShareModal.tsx index a1ef6dfd4..b68db4ab6 100644 --- a/src/custom/ShareModal/ShareModal.tsx +++ b/src/custom/ShareModal/ShareModal.tsx @@ -16,11 +16,15 @@ import { VISIBILITY } from '../../constants/constants'; import { ChainIcon, DeleteIcon, LockIcon, PublicIcon } from '../../icons'; import { useTheme } from '../../theme'; import { BLACK, WHITE } from '../../theme/colors'; +import { canShareResourceWithNewUsers, canUpdateResourceVisibility } from '../../utils/permissions'; import { - canShareResourceWithNewUsers, - canUpdateResourceVisibility, + getUserContactLabel, + getUserDisplayName, + getUserIdentifier, + getUserLabel, + isSameUser, User -} from '../../utils/permissions'; +} from '../../utils/user'; import { CustomTooltip } from '../CustomTooltip'; import { Modal, ModalBody, ModalButtonSecondary, ModalFooter } from '../Modal'; import UserShareSearch from '../UserSearchField/UserSearchField'; @@ -44,7 +48,7 @@ const SHARE_MODE = VISIBILITY; interface AccessListProps { accessList: User[]; ownerData: User; - handleDelete: (email: string) => Promise<{ error: string }>; + handleDelete: (actor: User) => Promise<{ error: string }>; hostURL?: string | null; } @@ -52,7 +56,7 @@ interface AccessListActorProps { actor: User; isOwner: boolean; hostURL?: string | null; - handleDelete: (email: string) => Promise<{ error: string }>; + handleDelete: (actor: User) => Promise<{ error: string }>; } const AccessListActor: React.FC = ({ @@ -63,10 +67,10 @@ const AccessListActor: React.FC = ({ }) => { const theme = useTheme(); const [isRevoking, setIsRevoking] = useState(false); - const revokeAcess = async () => { + const revokeAccess = async () => { setIsRevoking(true); try { - await handleDelete(actorData.email); + await handleDelete(actorData); } finally { setIsRevoking(false); } @@ -77,20 +81,20 @@ const AccessListActor: React.FC = ({ }; return ( - + { - if (hostURL) openInNewTab(`${hostURL}/user/${actorData.id}`); + if (hostURL) openInNewTab(`${hostURL}/user/${getUserIdentifier(actorData)}`); }} /> = ({ {!isOwner && !isRevoking && ( - + @@ -135,10 +139,10 @@ const AccessList: React.FC = ({ {accessList.map((actorData) => ( ))} @@ -213,16 +217,25 @@ const ShareModal: React.FC = ({ useGetAllUsersQuery, shareableLink }: ShareModalProps): JSX.Element => { - console.log('amit selectdResource', selectedResource); const theme = useTheme(); const [openMenu, setMenu] = useState(false); const [shareUserData, setShareUserData] = useState([]); + // `?? ''` keeps the visibility Select controlled even when a broken + // caller passes a nullish or empty resource selection. const [resourceVisibility, setVisibility] = useState( - Array.isArray(selectedResource) ? selectedResource[0].visibility : selectedResource.visibility + (Array.isArray(selectedResource) + ? selectedResource[0]?.visibility + : selectedResource?.visibility) ?? '' ); - console.log('amit resourceVisibility', resourceVisibility); const [isUpdatingVisibility, setUpdatingVisibility] = useState(false); + // The prop type requires a resource, but this is a published component with + // untyped consumers; the access handlers read `.id` off it, so a nullish or + // empty selection has to be caught before it reaches a payload. + const hasSelectedResource = Array.isArray(selectedResource) + ? selectedResource.length > 0 + : Boolean(selectedResource); + const userCanUpdateVisibility = Array.isArray(selectedResource) ? selectedResource.every((resource) => canUpdateResourceVisibility(resource, currentUser, ownerData) @@ -246,11 +259,30 @@ const ShareModal: React.FC = ({ const resourceType = dataName === 'design' ? 'pattern' : dataName; const handleShareWithNewUsers = async (newUsers: User[]) => { + if (!hasSelectedResource) { + notify({ + message: `Unable to share ${dataName}: no ${dataName} is selected`, + event_type: 'error' + }); + return { error: 'no resource selected' }; + } + + // Fail fast on records without a usable identifier: an empty actor_id + // would produce an invalid grant_access payload and a confusing + // partial-share result. + if (newUsers.some((user) => !getUserIdentifier(user))) { + notify({ + message: `Unable to share ${dataName}: a selected user record is missing its identifier`, + event_type: 'error' + }); + return { error: 'missing user identifier' }; + } + const grantAccessList = newUsers.map((user) => ({ - actor_id: user.userId, + actor_id: getUserIdentifier(user), actor_type: 'user' })); - const emails = newUsers.map((u) => u.email); + const recipients = newUsers.map(getUserLabel); if (Array.isArray(selectedResource)) { const responses = await Promise.all( @@ -271,7 +303,7 @@ const ShareModal: React.FC = ({ if (!hasError) { notify({ - message: `${dataName}s shared with ${emails.join(', ')}`, + message: `${dataName}s shared with ${recipients.join(', ')}`, event_type: 'success' }); } else { @@ -298,7 +330,7 @@ const ShareModal: React.FC = ({ if (!response?.error) { notify({ - message: `${dataName} shared with ${emails.join(', ')}`, + message: `${dataName} shared with ${recipients.join(', ')}`, event_type: 'success' }); } @@ -316,11 +348,29 @@ const ShareModal: React.FC = ({ }; const handleRevokeAccess = async (revokedUsers: User[]) => { + if (!hasSelectedResource) { + notify({ + message: `Unable to revoke access to ${dataName}: no ${dataName} is selected`, + event_type: 'error' + }); + return { error: 'no resource selected' }; + } + + // Same guard as sharing: never issue a revoke_access entry with an + // empty actor_id. + if (revokedUsers.some((user) => !getUserIdentifier(user))) { + notify({ + message: `Unable to revoke access to ${dataName}: the user record is missing its identifier`, + event_type: 'error' + }); + return { error: 'missing user identifier' }; + } + const revokeAccessList = revokedUsers.map((user) => ({ - actor_id: user.id, + actor_id: getUserIdentifier(user), actor_type: 'user' })); - const emails = revokedUsers.map((u) => u.email); + const revokees = revokedUsers.map(getUserLabel); const response = await resourceAccessMutator({ resourceType, @@ -333,16 +383,15 @@ const ShareModal: React.FC = ({ }); if (!response?.error) { - const msg = `Access to ${dataName} revoked for ${emails.join(', ')} `; notify({ - message: msg, + message: `Access to ${dataName} revoked for ${revokees.join(', ')}`, event_type: 'success' }); } if (response?.error) { notify({ - message: `failed to revokke access to ${dataName}`, + message: `Failed to revoke access to ${dataName}`, event_type: 'error' }); } @@ -352,14 +401,7 @@ const ShareModal: React.FC = ({ }; }; - const handleDelete = async (email: string) => { - const revoked = shareUserData.find((user) => user.email == email); - if (!revoked) { - console.error('cant revoke user without acesss'); - return { error: '' }; - } - return handleRevokeAccess([revoked]); - }; + const handleDelete = async (actor: User) => handleRevokeAccess([actor]); /* eslint-disable @typescript-eslint/no-explicit-any */ const notifyVisibilityChange = (res: any, value: any) => { diff --git a/src/custom/UserSearchField/UserSearchField.tsx b/src/custom/UserSearchField/UserSearchField.tsx index 025f64266..01054d360 100644 --- a/src/custom/UserSearchField/UserSearchField.tsx +++ b/src/custom/UserSearchField/UserSearchField.tsx @@ -6,17 +6,14 @@ import { useDebounce } from 'use-debounce'; import { Avatar, Box, Chip, Grid2, TextField, Typography } from '../../base'; import { PersonIcon } from '../../icons/Person'; import { useTheme } from '../../theme'; -import { DeletedAt, isSoftDeleted } from '../../utils/nullTime'; - -interface User { - id: string; - userId: string; - firstName: string; - lastName: string; - email: string; - avatarUrl?: string; - deletedAt?: DeletedAt; -} +import { isSoftDeleted } from '../../utils/nullTime'; +import { + getUserContactLabel, + getUserDisplayName, + getUserLabel, + isSameUser, + User +} from '../../utils/user'; interface UserSearchFieldProps { // Array of user objects currently selected. @@ -27,6 +24,22 @@ interface UserSearchFieldProps { useGetAllUsersQuery: any; } +// Module scope keeps the component type stable across UserShareSearch +// renders; defining it inline would remount every chip on each render. +const UserChip = ({ avatarObj, ...props }: { avatarObj: User }) => ( + + {avatarObj.avatarUrl ? '' : getUserDisplayName(avatarObj).charAt(0)} + + } + label={getUserLabel(avatarObj)} + size="small" + {...props} + /> +); + const UserShareSearch: React.FC = ({ usersData, disabled = false, @@ -54,7 +67,6 @@ const UserShareSearch: React.FC = ({ // const open = inputValue.trim().length > 0 && suggestions?.length > 0 const handleShareWithNewUsers = async () => { - console.log('users to share with', usersToShareWith); try { setIsSharing(true); const result = await shareWithNewUsers(usersToShareWith); @@ -64,7 +76,7 @@ const UserShareSearch: React.FC = ({ setError(result.error); } } catch (e) { - console.log('error while sharing', e); + console.error('error while sharing', e); } finally { setIsSharing(false); } @@ -87,25 +99,18 @@ const UserShareSearch: React.FC = ({ } }; + // `?? []` tolerates JS consumers of the published package passing a + // nullish usersData, mirroring the nullish-currentUser tolerance in + // utils/permissions.ts. + const alreadySelectedUsers = usersToShareWith.concat(usersData ?? []); const filteredOptions = suggestions.filter( - (option: User) => !usersToShareWith.concat(usersData).find((u) => u.email === option.email) + (option: User) => !alreadySelectedUsers.some((u) => isSameUser(u, option)) ); - const isShareDisabled = disabled || isSharing || usersToShareWith.length === 0; - - const UserChip = ({ avatarObj, ...props }: { avatarObj: User }) => ( - - {avatarObj.avatarUrl ? '' : avatarObj.firstName?.charAt(0)} - - } - label={avatarObj.email} - size="small" - {...props} - /> - ); + // The picker itself stays usable until the consumer disables it or a share + // is in flight; only the Share button additionally requires a selection. + const isPickerDisabled = disabled || isSharing; + const isShareDisabled = isPickerDisabled || usersToShareWith.length === 0; return ( <> @@ -125,12 +130,12 @@ const UserShareSearch: React.FC = ({ filterSelectedOptions multiple disableListWrap - disabled={isSharing} + disabled={isPickerDisabled} // open={open} inputValue={inputValue} loading={searchUserLoading} value={usersToShareWith} - getOptionLabel={(user) => user.email} + getOptionLabel={(user) => getUserLabel(user)} noOptionsText={ searchUserLoading ? 'Loading...' @@ -140,7 +145,7 @@ const UserShareSearch: React.FC = ({ } onChange={handleAdd} onInputChange={handleInputChange} - isOptionEqualToValue={(option, value) => option.email === value.email} + isOptionEqualToValue={isSameUser} renderInput={(params) => ( = ({ helperText={error} fullWidth label="" - disabled={isShareDisabled} sx={{ '& .MuiOutlinedInput-root': { paddingInline: '0.5rem', @@ -167,36 +171,40 @@ const UserShareSearch: React.FC = ({ }} /> )} - renderOption={(props: React.HTMLAttributes, option) => ( - // @ts-expect-error Props need to be passed to BOX component to make sure styles getting updated - img': { mr: 2, flexShrink: 0 } }} {...props}> - - - - - {option.avatarUrl ? '' : } - - - - - {isSoftDeleted(option.deletedAt) ? ( - - {option.email} (deleted) - - ) : ( - <> - - {option.firstName} {option.lastName} - + renderOption={(props: React.HTMLAttributes, option) => { + const displayName = getUserDisplayName(option); + const contactLabel = getUserContactLabel(option); + return ( + // @ts-expect-error Props need to be passed to BOX component to make sure styles getting updated + img': { mr: 2, flexShrink: 0 } }} {...props}> + + + + + {option.avatarUrl ? '' : } + + + + + {isSoftDeleted(option.deletedAt) ? ( - {option.email} + {getUserLabel(option)} (deleted) - - )} + ) : ( + <> + {displayName} + {contactLabel && contactLabel !== displayName && ( + + {contactLabel} + + )} + + )} + - - - )} + + ); + }} />