Gh 3884 Fix Multiperson property (#3888)

* retrieve all team members for specific lists

* retrieve all team members for specific lists

* final fixes

* update to use enum

* unit test fixes

* lint fixes

* fix integration test

* fixup integration test

* fix issues with making board private

* update to make webapp and server enums match

* fix to add member in correct role

* remove unnecessary property, check Open/Private board

* cleanup confirm for non admin

* some final cleanup

* move default to webapp

* only allow greater roles in drop down, change property name to minimumRole

* update multiperson property to confirm and select same as person property

* implement hashing of keys to cause Select to reprocess initial load

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

webapp/src/properties/multiperson/multiperson.tsx

@@ -1,8 +1,9 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-import React, {useCallback} from 'react'
+import React, {useCallback, useState} from 'react'
-import Select from 'react-select'
+import {useIntl} from 'react-intl'
+import Select from 'react-select/async'
 import {CSSObject} from '@emotion/serialize'
 
 import {getSelectBaseStyle} from '../../theme'
@@ -11,10 +12,16 @@ import {Utils} from '../../utils'
 import mutator from '../../mutator'
 import {useAppSelector} from '../../store/hooks'
 import {getBoardUsers, getBoardUsersList} from '../../store/users'
+import {BoardMember, BoardTypeOpen, MemberRole} from '../../blocks/board'
 
 import {PropertyProps} from '../types'
 import {ClientConfig} from '../../config/clientConfig'
 import {getClientConfig} from '../../store/clientConfig'
+import {useHasPermissions} from '../../hooks/permissions'
+import {Permission} from '../../constants'
+import client from '../../octoClient'
+import ConfirmAddUserForNotifications from '../../components/confirmAddUserForNotifications'
+import GuestBadge from '../../widgets/guestBadge'
 
 import './multiperson.scss'
 
@@ -52,12 +59,21 @@ const selectStyles = {
     }),
 }
 
-const MultiPerson = (props: PropertyProps) => {
+const MultiPerson = (props: PropertyProps): JSX.Element => {
     const {card, board, propertyTemplate, propertyValue, readOnly} = props
+    const [confirmAddUser, setConfirmAddUser] = useState<IUser|null>(null)
 
     const clientConfig = useAppSelector<ClientConfig>(getClientConfig)
+    const intl = useIntl()
+
     const boardUsersById = useAppSelector<{[key: string]: IUser}>(getBoardUsers)
     const boardUsers = useAppSelector<IUser[]>(getBoardUsersList)
+    const boardUsersKey = Object.keys(boardUsersById) ? Utils.hashCode(JSON.stringify(Object.keys(boardUsersById))) : 0
+
+    const allowManageBoardRoles = useHasPermissions(board.teamId, board.id, [Permission.ManageBoardRoles])
+    const allowAddUsers = allowManageBoardRoles || board.type === BoardTypeOpen
+
+    const onChange = useCallback((newValue) => mutator.changePropertyValue(board.id, card, propertyTemplate.id, newValue), [board.id, card, propertyTemplate.id])
 
     const formatOptionLabel = (user: any): JSX.Element => {
         if (!user) {
@@ -80,12 +96,11 @@ const MultiPerson = (props: PropertyProps) => {
                     />
                 )}
                 {Utils.getUserDisplayName(user, clientConfig.teammateNameDisplay)}
+                <GuestBadge show={Boolean(user?.is_guest)}/>
             </div>
         )
     }
 
-    const onChange = useCallback((newValue) => mutator.changePropertyValue(board.id, card, propertyTemplate.id, newValue), [board.id, card, propertyTemplate.id])
-
     let users: IUser[] = []
 
     if (typeof propertyValue === 'string' && propertyValue !== '') {
@@ -94,6 +109,49 @@ const MultiPerson = (props: PropertyProps) => {
         users = propertyValue.map((id) => boardUsersById[id])
     }
 
+    const addUser = useCallback(async (userId: string, role: string) => {
+        const minimumRole = role || MemberRole.Viewer
+        const newMember = {
+            boardId: board.id,
+            userId,
+            roles: role,
+            schemeEditor: minimumRole === MemberRole.Editor,
+            schemeCommenter: minimumRole === MemberRole.Editor || minimumRole === MemberRole.Commenter,
+            schemeViewer: minimumRole === MemberRole.Editor || minimumRole === MemberRole.Commenter || minimumRole === MemberRole.Viewer,
+        } as BoardMember
+
+        setConfirmAddUser(null)
+        await mutator.createBoardMember(newMember)
+
+        if (users) {
+            const userIds = users.map((a) => a.id)
+            await mutator.changePropertyValue(board.id, card, propertyTemplate.id, [...userIds, newMember.userId])
+        } else {
+            await mutator.changePropertyValue(board.id, card, propertyTemplate.id, newMember.userId)
+        }
+    }, [board, card, propertyTemplate, users])
+
+    const loadOptions = useCallback(async (value: string) => {
+        if (!allowAddUsers) {
+            return boardUsers.filter((u) => u.username.toLowerCase().includes(value.toLowerCase()))
+        }
+        const excludeBots = true
+        const allUsers = await client.searchTeamUsers(value, excludeBots)
+        const usersInsideBoard: IUser[] = []
+        const usersOutsideBoard: IUser[] = []
+        for (const u of allUsers) {
+            if (boardUsersById[u.id]) {
+                usersInsideBoard.push(u)
+            } else {
+                usersOutsideBoard.push(u)
+            }
+        }
+        return [
+            {label: intl.formatMessage({id: 'PersonProperty.board-members', defaultMessage: 'Board members'}), options: usersInsideBoard},
+            {label: intl.formatMessage({id: 'PersonProperty.non-board-members', defaultMessage: 'Not board members'}), options: usersOutsideBoard},
+        ]
+    }, [boardUsers, allowAddUsers, boardUsersById])
+
     if (readOnly) {
         return (
             <div className={`MultiPerson ${props.property.valueClassName(true)}`}>
@@ -103,29 +161,50 @@ const MultiPerson = (props: PropertyProps) => {
     }
 
     return (
+        <>
+            {confirmAddUser &&
+                <ConfirmAddUserForNotifications
+                    allowManageBoardRoles={allowManageBoardRoles}
+                    minimumRole={board.minimumRole}
+                    user={confirmAddUser}
+                    onConfirm={addUser}
+                    onClose={() => setConfirmAddUser(null)}
+                />}
             <Select
+                key={boardUsersKey}
+                loadOptions={loadOptions}
                 isMulti={true}
-            options={boardUsers}
+                defaultOptions={true}
                 isSearchable={true}
                 isClearable={true}
-            placeholder={'Empty'}
+                backspaceRemovesValue={true}
                 className={`MultiPerson ${props.property.valueClassName(props.readOnly)}`}
                 classNamePrefix={'react-select'}
                 formatOptionLabel={formatOptionLabel}
                 styles={selectStyles}
+                placeholder={'Empty'}
                 getOptionLabel={(o: IUser) => o.username}
                 getOptionValue={(a: IUser) => a.id}
                 value={users}
-            onChange={(item, action) => {
+                onChange={(items, action) => {
                     if (action.action === 'select-option') {
-                    onChange(item.map((a) => a.id) || [])
+                        const confirmedIds: string[] = []
+                        items.forEach((item) => {
+                            if (boardUsersById[item.id]) {
+                                confirmedIds.push(item.id)
+                            } else {
+                                setConfirmAddUser(item)
+                            }
+                        })
+                        onChange(confirmedIds)
                     } else if (action.action === 'clear') {
                         onChange([])
                     } else if (action.action === 'remove-value') {
-                    onChange(item.filter((a) => a.id !== action.removedValue.id).map((b) => b.id) || [])
+                        onChange(items.filter((a) => a.id !== action.removedValue.id).map((b) => b.id) || [])
                     }
                 }}
             />
+        </>
     )
 }
 

webapp/src/properties/person/person.tsx

@@ -63,7 +63,9 @@ const Person = (props: PropertyProps): JSX.Element => {
     const {card, board, propertyTemplate, propertyValue, readOnly} = props
     const [confirmAddUser, setConfirmAddUser] = useState<IUser|null>(null)
 
+    const boardUsers = useAppSelector<IUser[]>(getBoardUsersList)
     const boardUsersById = useAppSelector<{[key: string]: IUser}>(getBoardUsers)
+    const boardUsersKey = Object.keys(boardUsersById) ? Utils.hashCode(JSON.stringify(Object.keys(boardUsersById))) : 0
     const onChange = useCallback((newValue) => mutator.changePropertyValue(board.id, card, propertyTemplate.id, newValue), [board.id, card, propertyTemplate.id])
 
     const me: IUser = boardUsersById[propertyValue as string]
@@ -108,8 +110,6 @@ const Person = (props: PropertyProps): JSX.Element => {
         mutator.updateBoardMember(newMember, {...newMember, schemeAdmin: false, schemeEditor: true, schemeCommenter: true, schemeViewer: true})
     }, [board, card, propertyTemplate])
 
-    const boardUsers = useAppSelector<IUser[]>(getBoardUsersList)
-
     const allowManageBoardRoles = useHasPermissions(board.teamId, board.id, [Permission.ManageBoardRoles])
     const allowAddUsers = allowManageBoardRoles || board.type === BoardTypeOpen
 
@@ -153,6 +153,7 @@ const Person = (props: PropertyProps): JSX.Element => {
                 onClose={() => setConfirmAddUser(null)}
             />}
             <Select
+                key={boardUsersKey}
                 loadOptions={loadOptions}
                 defaultOptions={true}
                 isSearchable={true}

webapp/src/utils.ts

@@ -165,6 +165,15 @@ class Utils {
         return output
     }
 
+    // general purpose (non-secure) hash
+    static hashCode(s: string) {
+        let h = 0
+        for (let i = 0; i < s.length; i++) {
+            h = Math.imul(31, h) + s.charCodeAt(i) | 0
+        }
+        return h
+    }
+
     static htmlToElement(html: string): HTMLElement {
         const template = document.createElement('template')
         template.innerHTML = html.trim()