Merge pull request #27015 from overleaf/ii-groups-pagination-2-revert

[web] Revert "Group members page pagination"

GitOrigin-RevId: 83c377e319c32110bdaa22970cc378f2aa89be7c

services/web/app/src/Features/UserMembership/UserMembershipHandler.js

@@ -79,11 +79,7 @@ async function getPopulatedListOfMembers(entity, attributes) {
     }
   }
 
-  const users = await Promise.all(
-    userObjects.map(userObject =>
-      UserMembershipViewModel.promises.buildAsync(userObject)
-    )
-  )
+  const users = await UserMembershipViewModel.promises.buildAsync(userObjects)
 
   for (const user of users) {
     if (

services/web/app/src/Features/UserMembership/UserMembershipViewModel.js

@@ -23,30 +23,59 @@ const UserMembershipViewModel = {
     }
   },
 
-  buildAsync(userOrIdOrEmail, callback) {
+  buildAsync(userOrIdOrEmailArray, callback) {
     if (callback == null) {
       callback = function () {}
     }
-    if (!isObjectIdInstance(userOrIdOrEmail)) {
-      // userOrIdOrEmail is a user or an email and can be parsed by #build
-      return callback(null, UserMembershipViewModel.build(userOrIdOrEmail))
-    }
 
-    const userId = userOrIdOrEmail
-    const projection = {
-      email: 1,
-      first_name: 1,
-      last_name: 1,
-      lastLoggedIn: 1,
-      lastActive: 1,
-      enrollment: 1,
-    }
-    return UserGetter.getUser(userId, projection, function (error, user) {
-      if (error != null || user == null) {
-        return callback(null, buildUserViewModelWithId(userId.toString()))
+    const userObjectIds = userOrIdOrEmailArray.filter(isObjectIdInstance)
+
+    return UserGetter.getUsers(
+      userObjectIds,
+      {
+        email: 1,
+        first_name: 1,
+        last_name: 1,
+        lastLoggedIn: 1,
+        lastActive: 1,
+        enrollment: 1,
+      },
+      function (error, users) {
+        const results = []
+
+        if (error != null) {
+          userOrIdOrEmailArray.forEach(item => {
+            if (isObjectIdInstance(item)) {
+              results.push(buildUserViewModelWithId(item.toString()))
+            } else {
+              // `item` is a user or an email and can be parsed by #build
+              results.push(UserMembershipViewModel.build(item))
+            }
+          })
+        } else {
+          const usersMap = new Map()
+          for (const user of users) {
+            usersMap.set(user._id.toString(), user)
+          }
+
+          userOrIdOrEmailArray.forEach(item => {
+            if (isObjectIdInstance(item)) {
+              const user = usersMap.get(item.toString())
+              if (user == null) {
+                results.push(buildUserViewModelWithId(item.toString()))
+              } else {
+                results.push(buildUserViewModel(user))
+              }
+            } else {
+              // `item` is a user or an email and can be parsed by #build
+              results.push(UserMembershipViewModel.build(item))
+            }
+          })
+        }
+
+        callback(null, results)
       }
-      return callback(null, buildUserViewModel(user))
-    })
+    )
   },
 }
 

services/web/frontend/extracted-translations.json

@@ -1025,6 +1025,7 @@
   "math_inline": "",
   "maximum_files_uploaded_together": "",
   "maybe_later": "",
+  "members_added": "",
   "members_management": "",
   "mendeley_dynamic_sync_description": "",
   "mendeley_groups_loading_error": "",

services/web/frontend/js/features/group-management/components/group-members.tsx

@@ -13,6 +13,7 @@ import OLCard from '@/features/ui/components/ol/ol-card'
 import OLButton from '@/features/ui/components/ol/ol-button'
 import OLFormControl from '@/features/ui/components/ol/ol-form-control'
 import OLFormText from '@/features/ui/components/ol/ol-form-text'
+import OLNotification from '@/features/ui/components/ol/ol-notification'
 
 export default function GroupMembers() {
   const { isReady } = useWaitForI18n()
@@ -26,6 +27,7 @@ export default function GroupMembers() {
     removeMemberError,
     inviteMemberLoading,
     inviteError,
+    memberAdded,
     paths,
   } = useGroupMembersContext()
   const [emailString, setEmailString] = useState<string>('')
@@ -140,6 +142,13 @@ export default function GroupMembers() {
                 data-testid="add-more-members-form"
               >
                 <p className="small">{t('invite_more_members')}</p>
+                {memberAdded && (
+                  <OLNotification
+                    content={t('members_added')}
+                    type="success"
+                    className="mt-2 mb-3"
+                  />
+                )}
                 <ErrorAlert error={inviteError} />
                 <form onSubmit={onAddMembersSubmit}>
                   <OLRow>

services/web/frontend/js/features/group-management/components/members-table/members-list.tsx

@@ -1,4 +1,4 @@
-import { useState, useRef, useEffect } from 'react'
+import { useState, useRef, useEffect, useMemo } from 'react'
 import { useTranslation } from 'react-i18next'
 import { User } from '../../../../../../types/group-management/user'
 import { useGroupMembersContext } from '../../context/group-members-context'
@@ -13,6 +13,9 @@ import getMeta from '@/utils/meta'
 import UnlinkUserModal from './unlink-user-modal'
 import OLTable from '@/features/ui/components/ol/ol-table'
 import OLTooltip from '@/features/ui/components/ol/ol-tooltip'
+import Pagination from '@/shared/components/pagination'
+
+const USERS_DISPLAY_LIMIT = 50
 
 type ManagedUsersListProps = {
   groupId: string
@@ -31,13 +34,30 @@ export default function MembersList({ groupId }: ManagedUsersListProps) {
   const managedUsersActive = getMeta('ol-managedUsersActive')
   const groupSSOActive = getMeta('ol-groupSSOActive')
   const tHeadRowRef = useRef<HTMLTableRowElement>(null)
-  const [colSpan, setColSpan] = useState(0)
+  const [pagination, setPagination] = useState({ currPage: 1, totalPages: 1 })
+
+  const usersForCurrentPage = useMemo(
+    () =>
+      users.slice(
+        (pagination.currPage - 1) * USERS_DISPLAY_LIMIT,
+        pagination.currPage * USERS_DISPLAY_LIMIT
+      ),
+    [users, pagination.currPage]
+  )
+
+  const handlePageClick = (
+    _e: React.MouseEvent<HTMLButtonElement>,
+    page: number
+  ) => {
+    setPagination(p => ({ ...p, currPage: page }))
+  }
 
   useEffect(() => {
-    if (tHeadRowRef.current) {
-      setColSpan(tHeadRowRef.current.querySelectorAll('th').length)
-    }
-  }, [])
+    setPagination(p => ({
+      ...p,
+      totalPages: Math.ceil(users.length / USERS_DISPLAY_LIMIT),
+    }))
+  }, [users.length])
 
   return (
     <div>
@@ -93,12 +113,17 @@ export default function MembersList({ groupId }: ManagedUsersListProps) {
         <tbody>
           {users.length === 0 && (
             <tr>
-              <td className="text-center" colSpan={colSpan}>
+              <td
+                className="text-center"
+                colSpan={
+                  tHeadRowRef.current?.querySelectorAll('th').length ?? 0
+                }
+              >
                 <small>{t('no_members')}</small>
               </td>
             </tr>
           )}
-          {users.map(user => (
+          {usersForCurrentPage.map(user => (
             <MemberRow
               key={user.email}
               user={user}
@@ -111,6 +136,15 @@ export default function MembersList({ groupId }: ManagedUsersListProps) {
           ))}
         </tbody>
       </OLTable>
+      {pagination.totalPages > 1 && (
+        <div className="d-flex justify-content-center">
+          <Pagination
+            handlePageClick={handlePageClick}
+            currentPage={pagination.currPage}
+            totalPages={pagination.totalPages}
+          />
+        </div>
+      )}
       {userToOffboard && (
         <OffboardManagedUserModal
           user={userToOffboard}

services/web/frontend/js/features/group-management/context/group-members-context.tsx

@@ -31,6 +31,7 @@ export type GroupMembersContextValue = {
   updateMemberView: (userId: string, updatedUser: User) => void
   inviteMemberLoading: boolean
   inviteError?: APIError
+  memberAdded: boolean
   paths: { [key: string]: string }
 }
 
@@ -58,6 +59,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
   const [inviteError, setInviteError] = useState<APIError>()
   const [removeMemberInflightCount, setRemoveMemberInflightCount] = useState(0)
   const [removeMemberError, setRemoveMemberError] = useState<APIError>()
+  const [memberAdded, setMemberAdded] = useState(false)
 
   const groupId = getMeta('ol-groupId')
 
@@ -74,7 +76,9 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
   const addMembers = useCallback(
     (emailString: string) => {
       setInviteError(undefined)
+      setMemberAdded(false)
       const emails = parseEmails(emailString)
+      let isError = false
       mapSeries(emails, async email => {
         setInviteUserInflightCount(count => count + 1)
         try {
@@ -94,8 +98,15 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
         } catch (error: unknown) {
           debugConsole.error(error)
           setInviteError((error as FetchError)?.data?.error || {})
+          isError = true
         }
-        setInviteUserInflightCount(count => count - 1)
+        setInviteUserInflightCount(count => {
+          const newCount = count - 1
+          if (newCount === 0 && !isError) {
+            setMemberAdded(true)
+          }
+          return newCount
+        })
       })
     },
     [paths.addMember, users, setUsers]
@@ -173,6 +184,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
       removeMemberError,
       inviteMemberLoading: inviteUserInflightCount > 0,
       inviteError,
+      memberAdded,
       paths,
     }),
     [
@@ -191,6 +203,7 @@ export function GroupMembersProvider({ children }: GroupMembersProviderProps) {
       removeMemberError,
       inviteUserInflightCount,
       inviteError,
+      memberAdded,
       paths,
     ]
   )

services/web/locales/en.json

@@ -1342,6 +1342,7 @@
   "may": "May",
   "maybe_later": "Maybe later",
   "member_picker": "Select number of users for group plan",
+  "members_added": "Member(s) added.",
   "members_management": "Members management",
   "mendeley": "Mendeley",
   "mendeley_dynamic_sync_description": "With the Mendeley integration, you can import your references into __appName__. You can either import all your references at once or dynamically search your Mendeley library directly from __appName__.",

services/web/test/frontend/features/group-management/components/members-table/members-list.spec.tsx

@@ -121,6 +121,16 @@ describe('MembersList', function () {
         users[1].last_name
       )
     })
+    it('should render the pagination navigation', function () {
+      cy.window().then(win => {
+        win.metaAttributesCache.set(
+          'ol-users',
+          Array.from({ length: 50 }).flatMap(() => users.flat())
+        )
+      })
+      mountManagedUsersList()
+      cy.findByRole('navigation', { name: /pagination navigation/i })
+    })
   })
 
   describe('empty user list', function () {

services/web/test/unit/src/UserMembership/UserMembershipHandlerTests.js

@@ -48,7 +48,7 @@ describe('UserMembershipHandler', function () {
 
     this.UserMembershipViewModel = {
       promises: {
-        buildAsync: sinon.stub().resolves({ _id: 'mock-member-id' }),
+        buildAsync: sinon.stub().resolves([{ _id: 'mock-member-id' }]),
       },
       build: sinon.stub().returns(this.newUser),
     }
@@ -118,26 +118,26 @@ describe('UserMembershipHandler', function () {
           this.subscription,
           EntityConfigs.group
         )
-        const expectedCallcount =
-          this.subscription.member_ids.length +
-          this.subscription.invited_emails.length +
-          this.subscription.teamInvites.length
         expect(
-          this.UserMembershipViewModel.promises.buildAsync.callCount
-        ).to.equal(expectedCallcount)
+          this.UserMembershipViewModel.promises.buildAsync
+        ).to.be.calledOnceWith(
+          this.subscription.invited_emails.concat(
+            this.subscription.teamInvites[0].email,
+            this.subscription.member_ids
+          )
+        )
       })
     })
 
-    describe('group mamagers', function () {
+    describe('group managers', function () {
       it('build view model for all managers', async function () {
         await this.UserMembershipHandler.promises.getUsers(
           this.subscription,
           EntityConfigs.groupManagers
         )
-        const expectedCallcount = this.subscription.manager_ids.length
         expect(
-          this.UserMembershipViewModel.promises.buildAsync.callCount
-        ).to.equal(expectedCallcount)
+          this.UserMembershipViewModel.promises.buildAsync
+        ).to.be.calledOnceWith(this.subscription.manager_ids)
       })
     })
 
@@ -147,11 +147,9 @@ describe('UserMembershipHandler', function () {
           this.institution,
           EntityConfigs.institution
         )
-
-        const expectedCallcount = this.institution.managerIds.length
         expect(
-          this.UserMembershipViewModel.promises.buildAsync.callCount
-        ).to.equal(expectedCallcount)
+          this.UserMembershipViewModel.promises.buildAsync
+        ).to.be.calledOnceWith(this.institution.managerIds)
       })
     })
   })

services/web/test/unit/src/UserMembership/UserMembershipViewModelTests.js

@@ -25,7 +25,7 @@ const {
 
 describe('UserMembershipViewModel', function () {
   beforeEach(function () {
-    this.UserGetter = { getUser: sinon.stub() }
+    this.UserGetter = { getUsers: sinon.stub() }
     this.UserMembershipViewModel = SandboxedModule.require(modulePath, {
       requires: {
         'mongodb-legacy': { ObjectId },
@@ -87,9 +87,10 @@ describe('UserMembershipViewModel', function () {
     })
 
     it('build email', function (done) {
+      this.UserGetter.getUsers.yields(null, [])
       return this.UserMembershipViewModel.buildAsync(
-        this.email,
-        (error, viewModel) => {
+        [this.email],
+        (error, [viewModel]) => {
           assertCalledWith(this.UserMembershipViewModel.build, this.email)
           return done()
         }
@@ -97,9 +98,10 @@ describe('UserMembershipViewModel', function () {
     })
 
     it('build user', function (done) {
+      this.UserGetter.getUsers.yields(null, [])
       return this.UserMembershipViewModel.buildAsync(
-        this.user,
-        (error, viewModel) => {
+        [this.user],
+        (error, [viewModel]) => {
           assertCalledWith(this.UserMembershipViewModel.build, this.user)
           return done()
         }
@@ -107,30 +109,34 @@ describe('UserMembershipViewModel', function () {
     })
 
     it('build user id', function (done) {
-      this.UserGetter.getUser.yields(null, this.user)
+      const user = {
+        ...this.user,
+        _id: new ObjectId(),
+      }
+      this.UserGetter.getUsers.yields(null, [user])
       return this.UserMembershipViewModel.buildAsync(
-        new ObjectId(),
-        (error, viewModel) => {
+        [user._id],
+        (error, [viewModel]) => {
           expect(error).not.to.exist
           assertNotCalled(this.UserMembershipViewModel.build)
-          expect(viewModel._id).to.equal(this.user._id)
-          expect(viewModel.email).to.equal(this.user.email)
-          expect(viewModel.first_name).to.equal(this.user.first_name)
+          expect(viewModel._id.toString()).to.equal(user._id.toString())
+          expect(viewModel.email).to.equal(user.email)
+          expect(viewModel.first_name).to.equal(user.first_name)
           expect(viewModel.invite).to.equal(false)
           expect(viewModel.email).to.exist
           expect(viewModel.enrollment).to.exist
-          expect(viewModel.enrollment).to.deep.equal(this.user.enrollment)
+          expect(viewModel.enrollment).to.deep.equal(user.enrollment)
           return done()
         }
       )
     })
 
     it('build user id with error', function (done) {
-      this.UserGetter.getUser.yields(new Error('nope'))
+      this.UserGetter.getUsers.yields(new Error('nope'), [])
       const userId = new ObjectId()
       return this.UserMembershipViewModel.buildAsync(
-        userId,
-        (error, viewModel) => {
+        [userId],
+        (error, [viewModel]) => {
           expect(error).not.to.exist
           assertNotCalled(this.UserMembershipViewModel.build)
           expect(viewModel._id).to.equal(userId.toString())