16212: Refactors PeopleSelect component used on 'Share' dialog.
authorLucas Di Pentima <lucas@di-pentima.com.ar>
Mon, 27 Apr 2020 22:06:29 +0000 (19:06 -0300)
committerLucas Di Pentima <lucas@di-pentima.com.ar>
Wed, 29 Apr 2020 20:24:36 +0000 (17:24 -0300)
* Now named ParticipantSelect as it also retrieves Groups.
* Search for 'any' field on Users instead of just email.
* Don't request groups if 'onlyPeople' prop passed.
* Show users' display name, including email.
* Fix chip rendering to show the same as what's listed.

TBD: ParticipantSelect retrieves only 5 items per request when autocompleting.
This may not be what users expect, but listing too many items require UI
adjustments.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

src/models/user.test.ts
src/models/user.ts
src/store/group-details-panel/group-details-panel-actions.ts
src/store/groups-panel/groups-panel-actions.ts
src/views-components/dialog-forms/add-group-member-dialog.tsx
src/views-components/dialog-forms/create-group-dialog.tsx
src/views-components/sharing-dialog/participant-select.tsx [moved from src/views-components/sharing-dialog/people-select.tsx with 60% similarity]
src/views-components/sharing-dialog/sharing-invitation-form-component.tsx

index baa86da91bb7bef06ba70319f777149e7fff29eb..4fab59d5db8db13008069d96b4252001f31b599c 100644 (file)
@@ -8,6 +8,7 @@ describe('User', () => {
     it('gets the user display name', () => {
         type UserCase = {
             caseName: string;
+            withEmail?: boolean;
             user: User;
             expect: string;
         };
@@ -23,6 +24,18 @@ describe('User', () => {
                 },
                 expect: 'Some User'
             },
+            {
+                caseName: 'Full data available (with email)',
+                withEmail: true,
+                user: {
+                    email: 'someuser@example.com', username: 'someuser',
+                    firstName: 'Some', lastName: 'User',
+                    uuid: 'zzzzz-tpzed-someusersuuid',
+                    ownerUuid: 'zzzzz-tpzed-someusersowneruuid',
+                    prefs: {}, isAdmin: false, isActive: true
+                },
+                expect: 'Some User <<someuser@example.com>>'
+            },
             {
                 caseName: 'Missing first name',
                 user: {
@@ -56,6 +69,18 @@ describe('User', () => {
                 },
                 expect: 'someuser@example.com'
             },
+            {
+                caseName: 'Missing first & last names (with email)',
+                withEmail: true,
+                user: {
+                    email: 'someuser@example.com', username: 'someuser',
+                    firstName: '', lastName: '',
+                    uuid: 'zzzzz-tpzed-someusersuuid',
+                    ownerUuid: 'zzzzz-tpzed-someusersowneruuid',
+                    prefs: {}, isAdmin: false, isActive: true
+                },
+                expect: 'someuser@example.com'
+            },
             {
                 caseName: 'Missing first & last names, and email address',
                 user: {
@@ -67,6 +92,18 @@ describe('User', () => {
                 },
                 expect: 'someuser'
             },
+            {
+                caseName: 'Missing first & last names, and email address (with email)',
+                withEmail: true,
+                user: {
+                    email: '', username: 'someuser',
+                    firstName: '', lastName: '',
+                    uuid: 'zzzzz-tpzed-someusersuuid',
+                    ownerUuid: 'zzzzz-tpzed-someusersowneruuid',
+                    prefs: {}, isAdmin: false, isActive: true
+                },
+                expect: 'someuser'
+            },
             {
                 caseName: 'Missing all data (should not happen)',
                 user: {
@@ -80,7 +117,7 @@ describe('User', () => {
             },
         ];
         testCases.forEach(c => {
-            const dispName = getUserDisplayName(c.user);
+            const dispName = getUserDisplayName(c.user, c.withEmail);
             expect(dispName).toEqual(c.expect);
         })
     });
index 1e9a026055b7b54364289a9521cdd3705cc81c9f..3f0bcf47fc082c89c65b04a4dad2420e4afb73d1 100644 (file)
@@ -32,8 +32,12 @@ export const getUserFullname = (user: User) => {
         : "";
 };
 
-export const getUserDisplayName = (user: User) => {
-    return getUserFullname(user) || user.email || user.username || user.uuid;
+export const getUserDisplayName = (user: User, withEmail = false) => {
+    const displayName = getUserFullname(user) || user.email || user.username || user.uuid;
+    if (withEmail && user.email && displayName !== user.email) {
+        return `${displayName} <<${user.email}>>`;
+    }
+    return displayName;
 };
 
 export interface UserResource extends Resource, User {
index 41fb690f2787c74e799e4edb369d7e5dbdad844e..b2a7280720941aa3e320c6eaefa9030e79a07ea0 100644 (file)
@@ -6,7 +6,7 @@ import { bindDataExplorerActions } from '~/store/data-explorer/data-explorer-act
 import { Dispatch } from 'redux';
 import { propertiesActions } from '~/store/properties/properties-actions';
 import { getProperty } from '~/store/properties/properties';
-import { Person } from '~/views-components/sharing-dialog/people-select';
+import { Participant } from '~/views-components/sharing-dialog/participant-select';
 import { dialogActions } from '~/store/dialog/dialog-actions';
 import { reset, startSubmit } from 'redux-form';
 import { addGroupMember, deleteGroupMember } from '~/store/groups-panel/groups-panel-actions';
@@ -36,7 +36,7 @@ export const loadGroupDetailsPanel = (groupUuid: string) =>
 export const getCurrentGroupDetailsPanelUuid = getProperty<string>(GROUP_DETAILS_PANEL_ID);
 
 export interface AddGroupMembersFormData {
-    [ADD_GROUP_MEMBERS_USERS_FIELD_NAME]: Person[];
+    [ADD_GROUP_MEMBERS_USERS_FIELD_NAME]: Participant[];
 }
 
 export const openAddGroupMembersDialog = () =>
index 35ec413c0ed1378b4671929baaed62d29319a9ba..b5ca37759b56e29642f6fea54d21b507e198d6e1 100644 (file)
@@ -6,7 +6,7 @@ import { Dispatch } from 'redux';
 import { reset, startSubmit, stopSubmit, FormErrors } from 'redux-form';
 import { bindDataExplorerActions } from "~/store/data-explorer/data-explorer-action";
 import { dialogActions } from '~/store/dialog/dialog-actions';
-import { Person } from '~/views-components/sharing-dialog/people-select';
+import { Participant } from '~/views-components/sharing-dialog/participant-select';
 import { RootState } from '~/store/store';
 import { ServiceRepository } from '~/services/services';
 import { getResource } from '~/store/resources/resources';
@@ -65,7 +65,7 @@ export const openRemoveGroupDialog = (uuid: string) =>
 
 export interface CreateGroupFormData {
     [CREATE_GROUP_NAME_FIELD_NAME]: string;
-    [CREATE_GROUP_USERS_FIELD_NAME]?: Person[];
+    [CREATE_GROUP_USERS_FIELD_NAME]?: Participant[];
 }
 
 export const createGroup = ({ name, users = [] }: CreateGroupFormData) =>
index 2bd2109e8e0a595636998fd47da13725be4512e6..53917f54b2ede6471b3c685ea2a18a70f58fc620 100644 (file)
@@ -7,7 +7,7 @@ import { compose } from "redux";
 import { reduxForm, InjectedFormProps, WrappedFieldArrayProps, FieldArray } from 'redux-form';
 import { withDialog, WithDialogProps } from "~/store/dialog/with-dialog";
 import { FormDialog } from '~/components/form-dialog/form-dialog';
-import { PeopleSelect, Person } from '~/views-components/sharing-dialog/people-select';
+import { ParticipantSelect, Participant } from '~/views-components/sharing-dialog/participant-select';
 import { ADD_GROUP_MEMBERS_DIALOG, ADD_GROUP_MEMBERS_FORM, AddGroupMembersFormData, ADD_GROUP_MEMBERS_USERS_FIELD_NAME, addGroupMembers } from '~/store/group-details-panel/group-details-panel-actions';
 import { minLength } from '~/validators/min-length';
 
@@ -39,8 +39,8 @@ const UsersField = () =>
 
 const UsersFieldValidation = [minLength(1, () => 'Select at least one user')];
 
-const UsersSelect = ({ fields }: WrappedFieldArrayProps<Person>) =>
-    <PeopleSelect
+const UsersSelect = ({ fields }: WrappedFieldArrayProps<Participant>) =>
+    <ParticipantSelect
         onlyPeople
         autofocus
         label='Enter email adresses '
index ff692fbd7243e4307affc4326297ab5e8cc4d1ae..7af5317d534c03470d76e151eecbd5a74c9110c9 100644 (file)
@@ -11,7 +11,7 @@ import { CREATE_GROUP_DIALOG, CREATE_GROUP_FORM, createGroup, CreateGroupFormDat
 import { TextField } from '~/components/text-field/text-field';
 import { maxLength } from '~/validators/max-length';
 import { require } from '~/validators/require';
-import { PeopleSelect, Person } from '~/views-components/sharing-dialog/people-select';
+import { ParticipantSelect, Participant } from '~/views-components/sharing-dialog/participant-select';
 
 export const CreateGroupDialog = compose(
     withDialog(CREATE_GROUP_DIALOG),
@@ -54,8 +54,8 @@ const UsersField = () =>
         name={CREATE_GROUP_USERS_FIELD_NAME}
         component={UsersSelect} />;
 
-const UsersSelect = ({ fields }: WrappedFieldArrayProps<Person>) =>
-    <PeopleSelect
+const UsersSelect = ({ fields }: WrappedFieldArrayProps<Participant>) =>
+    <ParticipantSelect
         onlyPeople
         label='Enter email adresses '
         items={fields.getAll() || []}
similarity index 60%
rename from src/views-components/sharing-dialog/people-select.tsx
rename to src/views-components/sharing-dialog/participant-select.tsx
index 90235cd5831507314d238a7704faf1efa21cb7d4..5d062da0ef7ae612179c2e9edfc14e5622eec739 100644 (file)
@@ -10,38 +10,50 @@ import { FilterBuilder } from '../../services/api/filter-builder';
 import { debounce } from 'debounce';
 import { ListItemText, Typography } from '@material-ui/core';
 import { noop } from 'lodash/fp';
-import { GroupClass } from '~/models/group';
+import { GroupClass, GroupResource } from '~/models/group';
+import { getUserDisplayName, UserResource } from '~/models/user';
+import { ResourceKind } from '~/models/resource';
+import { ListResults } from '~/services/common-service/common-service';
 
-export interface Person {
+export interface Participant {
     name: string;
-    email: string;
     uuid: string;
 }
 
-export interface PeopleSelectProps {
+type ParticipantResource = GroupResource & UserResource;
 
-    items: Person[];
+interface ParticipantSelectProps {
+    items: Participant[];
     label?: string;
     autofocus?: boolean;
     onlyPeople?: boolean;
 
     onBlur?: (event: React.FocusEvent<HTMLInputElement>) => void;
     onFocus?: (event: React.FocusEvent<HTMLInputElement>) => void;
-    onCreate?: (person: Person) => void;
+    onCreate?: (person: Participant) => void;
     onDelete?: (index: number) => void;
-    onSelect?: (person: Person) => void;
-
+    onSelect?: (person: Participant) => void;
 }
 
-export interface PeopleSelectState {
+interface ParticipantSelectState {
     value: string;
-    suggestions: any[];
+    suggestions: ParticipantResource[];
 }
 
-export const PeopleSelect = connect()(
-    class PeopleSelect extends React.Component<PeopleSelectProps & DispatchProp, PeopleSelectState> {
-
-        state: PeopleSelectState = {
+const getDisplayName = (item: GroupResource & UserResource) => {
+    switch(item.kind) {
+        case ResourceKind.USER:
+            return getUserDisplayName(item, true);
+        case ResourceKind.GROUP:
+            return item.name;
+        default:
+            return item.uuid;
+    }
+};
+
+export const ParticipantSelect = connect()(
+    class ParticipantSelect extends React.Component<ParticipantSelectProps & DispatchProp, ParticipantSelectState> {
+        state: ParticipantSelectState = {
             value: '',
             suggestions: []
         };
@@ -67,21 +79,20 @@ export const PeopleSelect = connect()(
             );
         }
 
-        renderChipValue({ name, uuid }: Person) {
-            return name ? name : uuid;
+        renderChipValue(chipValue: Participant) {
+            const { name, uuid } = chipValue;
+            return name || uuid;
         }
 
-        renderSuggestion({ firstName, lastName, email, name }: any) {
+        renderSuggestion(item: ParticipantResource) {
             return (
                 <ListItemText>
-                    {name ?
-                        <Typography noWrap>{name}</Typography> :
-                        <Typography noWrap>{`${firstName} ${lastName} <<${email}>>`}</Typography>}
+                    <Typography noWrap>{getDisplayName(item)}</Typography>
                 </ListItemText>
             );
         }
 
-        handleDelete = (_: Person, index: number) => {
+        handleDelete = (_: Participant, index: number) => {
             const { onDelete = noop } = this.props;
             onDelete(index);
         }
@@ -91,19 +102,18 @@ export const PeopleSelect = connect()(
             if (onCreate) {
                 this.setState({ value: '', suggestions: [] });
                 onCreate({
-                    email: '',
                     name: '',
                     uuid: this.state.value,
                 });
             }
         }
 
-        handleSelect = ({ email, firstName, lastName, uuid, name }: any) => {
+        handleSelect = (selection: ParticipantResource) => {
+            const { uuid } = selection;
             const { onSelect = noop } = this.props;
             this.setState({ value: '', suggestions: [] });
             onSelect({
-                email,
-                name: `${name ? name : `${firstName} ${lastName}`}`,
+                name: getDisplayName(selection),
                 uuid,
             });
         }
@@ -116,16 +126,23 @@ export const PeopleSelect = connect()(
 
         requestSuggestions = async (_: void, __: void, { userService, groupsService }: ServiceRepository) => {
             const { value } = this.state;
+            const limit = 5; // FIXME: Does this provide a good UX?
+
+            const filterUsers = new FilterBuilder()
+                .addILike('any', value)
+                .getFilters();
+            const userItems: ListResults<any> = await userService.list({ filters: filterUsers, limit });
+
             const filterGroups = new FilterBuilder()
                 .addNotIn('group_class', [GroupClass.PROJECT])
                 .addILike('name', value)
                 .getFilters();
-            const groupItems = await groupsService.list({ filters: filterGroups, limit: 5 });
-            const filterUsers = new FilterBuilder()
-                .addILike('email', value)
-                .getFilters();
-            const userItems: any = await userService.list({ filters: filterUsers, limit: 5 });
-            const items = groupItems.items.concat(userItems.items);
-            this.setState({ suggestions: this.props.onlyPeople ? userItems.items : items });
+
+            const groupItems: ListResults<any> = await groupsService.list({ filters: filterGroups, limit });
+            this.setState({
+                suggestions: this.props.onlyPeople
+                    ? userItems.items
+                    : userItems.items.concat(groupItems.items)
+            });
         }
     });
index 5aec8febed3252b9e4bf3768466ac04fb20ef4a3..59456fb3f046119b22d8e0d3b00a668e1690b21f 100644 (file)
@@ -6,7 +6,7 @@ import * as React from 'react';
 import { Field, WrappedFieldProps, FieldArray, WrappedFieldArrayProps } from 'redux-form';
 import { Grid, FormControl, InputLabel } from '@material-ui/core';
 import { PermissionSelect, parsePermissionLevel, formatPermissionLevel } from './permission-select';
-import { PeopleSelect, Person } from './people-select';
+import { ParticipantSelect, Participant } from './participant-select';
 
 export default () =>
     <Grid container spacing={8}>
@@ -24,8 +24,8 @@ const InvitedPeopleField = () =>
         component={InvitedPeopleFieldComponent} />;
 
 
-const InvitedPeopleFieldComponent = ({ fields }: WrappedFieldArrayProps<Person>) =>
-    <PeopleSelect
+const InvitedPeopleFieldComponent = ({ fields }: WrappedFieldArrayProps<Participant>) =>
+    <ParticipantSelect
         items={fields.getAll() || []}
         onSelect={fields.push}
         onDelete={fields.remove} />;