From 4019c69ea8e0dbf8323ceb4d87520677101f59ad Mon Sep 17 00:00:00 2001
From: M Fahru <m.fahru@overleaf.com>
Date: Wed, 30 Aug 2023 08:20:57 -0700
Subject: [PATCH] Merge pull request #14566 from
 overleaf/mf-fix-select-all-checkbox-managed-users

[web] Fix click all email checkbox behaviour on managed users

GitOrigin-RevId: 4c4e7171d4aed3d99bc08be4b029eb3badb0fac9
---
 .../components/group-members-list.tsx         | 30 +++++++---
 .../components/group-members.tsx              | 30 +---------
 .../managed-users/managed-user-row.tsx        | 35 +----------
 .../managed-users/managed-users-list.tsx      | 22 ++-----
 .../managed-users-select-all-checkbox.tsx     | 44 ++++++++++++++
 .../managed-users-select-user-checkbox.tsx    | 59 +++++++++++++++++++
 .../context/group-members-context.tsx         |  4 ++
 .../hooks/use-user-selection.ts               |  9 +++
 .../managed-group-members.spec.tsx            |  2 +
 .../managed-users/managed-users-list.spec.tsx | 13 +---
 10 files changed, 151 insertions(+), 97 deletions(-)
 create mode 100644 services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx
 create mode 100644 services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx

diff --git a/services/web/frontend/js/features/group-management/components/group-members-list.tsx b/services/web/frontend/js/features/group-management/components/group-members-list.tsx
index 8da85c3822..0e8c8d95aa 100644
--- a/services/web/frontend/js/features/group-management/components/group-members-list.tsx
+++ b/services/web/frontend/js/features/group-management/components/group-members-list.tsx
@@ -1,19 +1,31 @@
+import { useCallback } from 'react'
 import { Col, Row } from 'react-bootstrap'
 import { useTranslation } from 'react-i18next'
 import Tooltip from '../../../shared/components/tooltip'
 import { useGroupMembersContext } from '../context/group-members-context'
 import GroupMemberRow from './group-member-row'
 
-type GroupMembersListProps = {
-  handleSelectAllClick: (e: any) => void
-}
-
-export default function GroupMembersList({
-  handleSelectAllClick,
-}: GroupMembersListProps) {
+export default function GroupMembersList() {
   const { t } = useTranslation()
-  const { selectedUsers, users, selectUser, unselectUser } =
-    useGroupMembersContext()
+  const {
+    selectedUsers,
+    users,
+    selectUser,
+    unselectUser,
+    selectAllUsers,
+    unselectAllUsers,
+  } = useGroupMembersContext()
+
+  const handleSelectAllClick = useCallback(
+    (e: React.ChangeEvent<HTMLInputElement>) => {
+      if (e.target.checked) {
+        selectAllUsers()
+      } else {
+        unselectAllUsers()
+      }
+    },
+    [selectAllUsers, unselectAllUsers]
+  )
 
   return (
     <ul className="list-unstyled structured-list">
diff --git a/services/web/frontend/js/features/group-management/components/group-members.tsx b/services/web/frontend/js/features/group-management/components/group-members.tsx
index 933584dfce..481acd1809 100644
--- a/services/web/frontend/js/features/group-management/components/group-members.tsx
+++ b/services/web/frontend/js/features/group-management/components/group-members.tsx
@@ -1,7 +1,6 @@
 import React, { useCallback, useState } from 'react'
 import { Button, Col, Form, FormControl, Row } from 'react-bootstrap'
 import { Trans, useTranslation } from 'react-i18next'
-import { User } from '../../../../../types/group-management/user'
 import MaterialIcon from '../../../shared/components/material-icon'
 import useWaitForI18n from '../../../shared/hooks/use-wait-for-i18n'
 import getMeta from '../../../utils/meta'
@@ -16,8 +15,6 @@ export default function GroupMembers() {
   const {
     users,
     selectedUsers,
-    selectAllUsers,
-    unselectAllUsers,
     addMembers,
     removeMembers,
     removeMemberLoading,
@@ -33,17 +30,6 @@ export default function GroupMembers() {
   const groupSize: number = getMeta('ol-groupSize')
   const managedUsersActive: any = getMeta('ol-managedUsersActive')
 
-  const handleSelectAllClick = useCallback(
-    (e: React.ChangeEvent<HTMLInputElement>) => {
-      if (e.target.checked) {
-        selectAllUsers()
-      } else {
-        unselectAllUsers()
-      }
-    },
-    [selectAllUsers, unselectAllUsers]
-  )
-
   const handleEmailsChange = useCallback(
     e => {
       setEmailString(e.target.value)
@@ -55,13 +41,6 @@ export default function GroupMembers() {
     return null
   }
 
-  const shouldShowRemoveUsersButton = () => {
-    return (
-      selectedUsers.length > 0 &&
-      !selectedUsers.find((u: User) => !!u?.enrollment?.managedBy)
-    )
-  }
-
   const onAddMembersSubmit = (e: React.FormEvent<Form>) => {
     e.preventDefault()
     addMembers(emailString)
@@ -98,7 +77,7 @@ export default function GroupMembers() {
                   </Button>
                 ) : (
                   <>
-                    {shouldShowRemoveUsersButton() && (
+                    {selectedUsers.length > 0 && (
                       <Button bsStyle="danger" onClick={removeMembers}>
                         {t('remove_from_group')}
                       </Button>
@@ -111,12 +90,9 @@ export default function GroupMembers() {
             <div className="row-spaced-small">
               <ErrorAlert error={removeMemberError} />
               {managedUsersActive ? (
-                <ManagedUsersList
-                  handleSelectAllClick={handleSelectAllClick}
-                  groupId={groupId}
-                />
+                <ManagedUsersList groupId={groupId} />
               ) : (
-                <GroupMembersList handleSelectAllClick={handleSelectAllClick} />
+                <GroupMembersList />
               )}
             </div>
             <hr />
diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx
index 7b0b48f1ee..374067278c 100644
--- a/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx
+++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-user-row.tsx
@@ -1,13 +1,13 @@
 import moment from 'moment'
-import { type Dispatch, type SetStateAction, useCallback } from 'react'
+import { type Dispatch, type SetStateAction } from 'react'
 import { useTranslation } from 'react-i18next'
 import { User } from '../../../../../../types/group-management/user'
 import Badge from '../../../../shared/components/badge'
 import Tooltip from '../../../../shared/components/tooltip'
 import type { ManagedUserAlert } from '../../utils/types'
-import { useGroupMembersContext } from '../../context/group-members-context'
 import ManagedUserStatus from './managed-user-status'
 import ManagedUserDropdownButton from './managed-user-dropdown-button'
+import ManagedUsersSelectUserCheckbox from './managed-users-select-user-checkbox'
 
 type ManagedUserRowProps = {
   user: User
@@ -23,42 +23,13 @@ export default function ManagedUserRow({
   groupId,
 }: ManagedUserRowProps) {
   const { t } = useTranslation()
-  const { selectedUsers, selectUser, unselectUser } = useGroupMembersContext()
-
-  const handleSelectUser = useCallback(
-    (event, user) => {
-      if (event.target.checked) {
-        selectUser(user)
-      } else {
-        unselectUser(user)
-      }
-    },
-    [selectUser, unselectUser]
-  )
-
-  const selected = selectedUsers.includes(user)
 
   return (
     <tr
       key={`user-${user.email}`}
       className={`managed-user-row ${user.invite ? 'text-muted' : ''}`}
     >
-      <td className="cell-checkbox">
-        {user.enrollment?.managedBy ? null : (
-          <>
-            <label htmlFor={`select-user-${user.email}`} className="sr-only">
-              {t('select_user')}
-            </label>
-            <input
-              className="select-item"
-              id={`select-user-${user.email}`}
-              type="checkbox"
-              checked={selected}
-              onChange={e => handleSelectUser(e, user)}
-            />
-          </>
-        )}
-      </td>
+      <ManagedUsersSelectUserCheckbox user={user} />
       <td className="cell-email">
         <span>
           {user.email}
diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx
index d0dcab9811..1086dad7b2 100644
--- a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx
+++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-list.tsx
@@ -8,23 +8,20 @@ import type { ManagedUserAlert } from '../../utils/types'
 import ManagedUserRow from './managed-user-row'
 import OffboardManagedUserModal from './offboard-managed-user-modal'
 import ManagedUsersListAlert from './managed-users-list-alert'
+import ManagedUsersSelectAllCheckbox from './managed-users-select-all-checkbox'
 
 type ManagedUsersListProps = {
-  handleSelectAllClick: (e: any) => void
   groupId: string
 }
 
-export default function ManagedUsersList({
-  handleSelectAllClick,
-  groupId,
-}: ManagedUsersListProps) {
+export default function ManagedUsersList({ groupId }: ManagedUsersListProps) {
   const { t } = useTranslation()
   const [userToOffboard, setUserToOffboard] = useState<User | undefined>(
     undefined
   )
   const [managedUserAlert, setManagedUserAlert] =
     useState<ManagedUserAlert>(undefined)
-  const { selectedUsers, users } = useGroupMembersContext()
+  const { users } = useGroupMembersContext()
 
   return (
     <div>
@@ -42,18 +39,7 @@ export default function ManagedUsersList({
               <table className="managed-users-table">
                 <thead>
                   <tr>
-                    <td className="cell-checkbox">
-                      <label htmlFor="select-all" className="sr-only">
-                        {t('select_all')}
-                      </label>
-                      <input
-                        className="select-all"
-                        id="select-all"
-                        type="checkbox"
-                        onChange={handleSelectAllClick}
-                        checked={selectedUsers.length === users.length}
-                      />
-                    </td>
+                    <ManagedUsersSelectAllCheckbox />
                     <td className="cell-email">
                       <span className="header">{t('email')}</span>
                     </td>
diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx
new file mode 100644
index 0000000000..b4ca6400ed
--- /dev/null
+++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-all-checkbox.tsx
@@ -0,0 +1,44 @@
+import { useCallback } from 'react'
+import { useTranslation } from 'react-i18next'
+import { useGroupMembersContext } from '../../context/group-members-context'
+
+export default function ManagedUsersSelectAllCheckbox() {
+  const { t } = useTranslation()
+
+  const { selectedUsers, users, selectAllNonManagedUsers, unselectAllUsers } =
+    useGroupMembersContext()
+
+  const handleSelectAllNonManagedClick = useCallback(
+    (e: React.ChangeEvent<HTMLInputElement>) => {
+      if (e.target.checked) {
+        selectAllNonManagedUsers()
+      } else {
+        unselectAllUsers()
+      }
+    },
+    [selectAllNonManagedUsers, unselectAllUsers]
+  )
+
+  // Pending: user.enrollment will be `undefined`
+  // Not managed: user.enrollment will be an empty object
+  const nonManagedUsers = users.filter(user => !user.enrollment?.managedBy)
+
+  if (nonManagedUsers.length === 0) {
+    return null
+  }
+
+  return (
+    <td className="cell-checkbox">
+      <label htmlFor="select-all" className="sr-only">
+        {t('select_all')}
+      </label>
+      <input
+        className="select-all"
+        id="select-all"
+        type="checkbox"
+        onChange={handleSelectAllNonManagedClick}
+        checked={selectedUsers.length === nonManagedUsers.length}
+      />
+    </td>
+  )
+}
diff --git a/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx
new file mode 100644
index 0000000000..4493929658
--- /dev/null
+++ b/services/web/frontend/js/features/group-management/components/managed-users/managed-users-select-user-checkbox.tsx
@@ -0,0 +1,59 @@
+import { useTranslation } from 'react-i18next'
+import type { User } from '../../../../../../types/group-management/user'
+import { useGroupMembersContext } from '../../context/group-members-context'
+import { useCallback } from 'react'
+
+type ManagedUsersSelectUserCheckboxProps = {
+  user: User
+}
+
+export default function ManagedUsersSelectUserCheckbox({
+  user,
+}: ManagedUsersSelectUserCheckboxProps) {
+  const { t } = useTranslation()
+  const { users, selectedUsers, selectUser, unselectUser } =
+    useGroupMembersContext()
+
+  const handleSelectUser = useCallback(
+    (event, user) => {
+      if (event.target.checked) {
+        selectUser(user)
+      } else {
+        unselectUser(user)
+      }
+    },
+    [selectUser, unselectUser]
+  )
+
+  // Pending: user.enrollment will be `undefined`
+  // Non managed: user.enrollment will be an empty object
+  const nonManagedUsers = users.filter(user => !user.enrollment?.managedBy)
+
+  // Hide the entire `td` (entire column) if no more users available to be click
+  // because all users are currently managed
+  if (nonManagedUsers.length === 0) {
+    return null
+  }
+
+  const selected = selectedUsers.includes(user)
+
+  return (
+    <td className="cell-checkbox">
+      {/* the next check will hide the `checkbox` but still show the `td` */}
+      {user.enrollment?.managedBy ? null : (
+        <>
+          <label htmlFor={`select-user-${user.email}`} className="sr-only">
+            {t('select_user')}
+          </label>
+          <input
+            className="select-item"
+            id={`select-user-${user.email}`}
+            type="checkbox"
+            checked={selected}
+            onChange={e => handleSelectUser(e, user)}
+          />
+        </>
+      )}
+    </td>
+  )
+}
diff --git a/services/web/frontend/js/features/group-management/context/group-members-context.tsx b/services/web/frontend/js/features/group-management/context/group-members-context.tsx
index 3b61020837..34b78be24e 100644
--- a/services/web/frontend/js/features/group-management/context/group-members-context.tsx
+++ b/services/web/frontend/js/features/group-management/context/group-members-context.tsx
@@ -24,6 +24,7 @@ export type GroupMembersContextValue = {
   selectUser: (user: User) => void
   selectAllUsers: () => void
   unselectAllUsers: () => void
+  selectAllNonManagedUsers: () => void
   unselectUser: (user: User) => void
   addMembers: (emailString: string) => void
   removeMembers: (e: any) => void
@@ -50,6 +51,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
     selectedUsers,
     selectAllUsers,
     unselectAllUsers,
+    selectAllNonManagedUsers,
     selectUser,
     unselectUser,
   } = useUserSelection(getMeta('ol-users', []))
@@ -147,6 +149,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
       selectedUsers,
       selectAllUsers,
       unselectAllUsers,
+      selectAllNonManagedUsers,
       selectUser,
       unselectUser,
       addMembers,
@@ -163,6 +166,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
       selectedUsers,
       selectAllUsers,
       unselectAllUsers,
+      selectAllNonManagedUsers,
       selectUser,
       unselectUser,
       addMembers,
diff --git a/services/web/frontend/js/features/group-management/hooks/use-user-selection.ts b/services/web/frontend/js/features/group-management/hooks/use-user-selection.ts
index 5e5b92e8b5..70ae42c86b 100644
--- a/services/web/frontend/js/features/group-management/hooks/use-user-selection.ts
+++ b/services/web/frontend/js/features/group-management/hooks/use-user-selection.ts
@@ -8,6 +8,14 @@ export default function useUserSelection(initialUsers: User[]) {
   const selectAllUsers = () => setSelectedUsers(users)
   const unselectAllUsers = () => setSelectedUsers([])
 
+  const selectAllNonManagedUsers = useCallback(() => {
+    // Pending: user.enrollment will be `undefined`
+    // Not managed: user.enrollment will be an empty object
+    const nonManagedUsers = users.filter(user => !user.enrollment?.managedBy)
+
+    setSelectedUsers(nonManagedUsers)
+  }, [users])
+
   const selectUser = useCallback((user: User) => {
     setSelectedUsers(users => [...users, user])
   }, [])
@@ -24,5 +32,6 @@ export default function useUserSelection(initialUsers: User[]) {
     unselectUser,
     selectAllUsers,
     unselectAllUsers,
+    selectAllNonManagedUsers,
   }
 }
diff --git a/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx b/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx
index 8ce8d5b86a..c899cadea1 100644
--- a/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx
+++ b/services/web/test/frontend/features/group-management/components/managed-users/managed-group-members.spec.tsx
@@ -152,6 +152,8 @@ describe('group members, with managed users', function () {
         cy.get('.select-item').should('be.checked')
       })
     })
+
+    cy.get('button').contains('Remove from group').click()
   })
 
   it('remove a member', function () {
diff --git a/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx b/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx
index 162755a515..84b604e808 100644
--- a/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx
+++ b/services/web/test/frontend/features/group-management/components/managed-users/managed-users-list.spec.tsx
@@ -32,14 +32,10 @@ describe('ManagedUsersList', function () {
       cy.window().then(win => {
         win.metaAttributesCache.set('ol-users', users)
       })
-      const handleSelectAllClick = () => {}
 
       cy.mount(
         <GroupMembersProvider>
-          <ManagedUsersList
-            handleSelectAllClick={handleSelectAllClick}
-            groupId={groupId}
-          />
+          <ManagedUsersList groupId={groupId} />
         </GroupMembersProvider>
       )
     })
@@ -72,18 +68,13 @@ describe('ManagedUsersList', function () {
   })
 
   describe('empty user list', function () {
-    const handleSelectAllClick = () => {}
-
     beforeEach(function () {
       cy.window().then(win => {
         win.metaAttributesCache.set('ol-users', [])
       })
       cy.mount(
         <GroupMembersProvider>
-          <ManagedUsersList
-            handleSelectAllClick={handleSelectAllClick}
-            groupId={groupId}
-          />
+          <ManagedUsersList groupId={groupId} />
         </GroupMembersProvider>
       )
     })