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
2025-04-06 16:12:08 +00:00 · 2023-08-30 08:20:57 -07:00 · 2023-08-30 08:20:57 -07:00 · 4019c69ea8
commit 4019c69ea8
parent 8a76991bd9
10 changed files with 151 additions and 97 deletions
--- 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">
--- 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 />
--- 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}
--- 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>
--- 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
@ -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>
+  )
+}
--- 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
@ -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>
+  )
+}
--- 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,
--- 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,
  }
 }
--- 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 () {
--- 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>
      )
    })