Fetching board member in parellel (mattermost-community#3379)

* Translated using Weblate (Malayalam) Currently translated at 100.0% (303 of 303 strings) Translation: Focalboard/webapp Translate-URL: https://translate.mattermost.com/projects/focalboard/webapp/ml/ * Update translation files Updated by "Cleanup translation files" hook in Weblate. Translation: Focalboard/webapp Translate-URL: https://translate.mattermost.com/projects/focalboard/webapp/ Update translation files Updated by "Cleanup translation files" hook in Weblate. Translation: Focalboard/webapp Translate-URL: https://translate.mattermost.com/projects/focalboard/webapp/ Update translation files Updated by "Cleanup translation files" hook in Weblate. Translation: Focalboard/webapp Translate-URL: https://translate.mattermost.com/projects/focalboard/webapp/ * Translated using Weblate (German) Currently translated at 100.0% (312 of 312 strings) Translation: Focalboard/webapp Translate-URL: https://translate.mattermost.com/projects/focalboard/webapp/de/ * Added code for fetching all the users at one time. * Added the code for personal server users * Update-code * Linter fixes * Test cases updated, some minor changes * Update the code according to review comments * Cypress test fix * Update server/services/store/mattermostauthlayer/mattermostauthlayer.go Co-authored-by: Varghese Jose <[email protected]> Co-authored-by: Hosted Weblate <[email protected]> Co-authored-by: jprusch <[email protected]> Co-authored-by: Scott Bishel <[email protected]>
kamre · Jul 29, 2022 · ee395d4 · ee395d4
1 parent d8ffd30
commit ee395d4
Show file tree

Hide file tree

Showing 12 changed files with 168 additions and 10 deletions.
diff --git a/server/api/api.go b/server/api/api.go
@@ -134,6 +134,7 @@ func (a *API) RegisterRoutes(r *mux.Router) {
 	apiv2.HandleFunc("/teams/{teamID}/{boardID}/files", a.sessionRequired(a.handleUploadFile)).Methods("POST")
 
 	// User APIs
+	apiv2.HandleFunc("/users", a.sessionRequired(a.handleGetUsersList)).Methods("POST")
 	apiv2.HandleFunc("/users/me", a.sessionRequired(a.handleGetMe)).Methods("GET")
 	apiv2.HandleFunc("/users/me/memberships", a.sessionRequired(a.handleGetMyMemberships)).Methods("GET")
 	apiv2.HandleFunc("/users/{userID}", a.sessionRequired(a.handleGetUser)).Methods("GET")
@@ -1063,6 +1064,63 @@ func (a *API) handleGetUser(w http.ResponseWriter, r *http.Request) {
 	auditRec.Success()
 }
 
+func (a *API) handleGetUsersList(w http.ResponseWriter, r *http.Request) {
+	// swagger:operation POST /users getUser
+	//
+	// Returns a user[]
+	//
+	// ---
+	// produces:
+	// - application/json
+	// parameters:
+	// - name: userID
+	//   in: path
+	//   description: User ID
+	//   required: true
+	//   type: string
+	// security:
+	// - BearerAuth: []
+	// responses:
+	//   '200':
+	//     description: success
+	//     schema:
+	//       "$ref": "#/definitions/User"
+	//   default:
+	//     description: internal error
+	//     schema:
+	//       "$ref": "#/definitions/ErrorResponse"
+
+	requestBody, err := ioutil.ReadAll(r.Body)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	var userIDs []string
+	if err = json.Unmarshal(requestBody, &userIDs); err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusInternalServerError, "", err)
+		return
+	}
+
+	auditRec := a.makeAuditRecord(r, "getUsersList", audit.Fail)
+	defer a.audit.LogRecord(audit.LevelAuth, auditRec)
+
+	users, err := a.app.GetUsersList(userIDs)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusBadRequest, err.Error(), err)
+		return
+	}
+
+	usersList, err := json.Marshal(users)
+	if err != nil {
+		a.errorResponse(w, r.URL.Path, http.StatusBadRequest, "", err)
+		return
+	}
+
+	jsonStringResponse(w, http.StatusOK, string(usersList))
+	auditRec.Success()
+}
+
 func (a *API) handleGetMe(w http.ResponseWriter, r *http.Request) {
 	// swagger:operation GET /users/me getMe
 	//

diff --git a/server/app/auth.go b/server/app/auth.go
@@ -64,6 +64,18 @@ func (a *App) GetUser(id string) (*model.User, error) {
 	return user, nil
 }
 
+func (a *App) GetUsersList(userIDs []string) ([]*model.User, error) {
+	if len(userIDs) == 0 {
+		return nil, errors.New("No User IDs")
+	}
+
+	users, err := a.store.GetUsersList(userIDs)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to find users")
+	}
+	return users, nil
+}
+
 // Login create a new user session if the authentication data is valid.
 func (a *App) Login(username, email, password, mfaToken string) (string, error) {
 	var user *model.User

diff --git a/server/services/permissions/mmpermissions/mocks/mockpluginapi.go b/server/services/permissions/mmpermissions/mocks/mockpluginapi.go
diff --git a/server/services/store/mattermostauthlayer/mattermostauthlayer.go b/server/services/store/mattermostauthlayer/mattermostauthlayer.go
@@ -295,6 +295,28 @@ func (s *MattermostAuthLayer) GetUsersByTeam(teamID string) ([]*model.User, erro
 	return users, nil
 }
 
+func (s *MattermostAuthLayer) GetUsersList(userIDs []string) ([]*model.User, error) {
+	query := s.getQueryBuilder().
+		Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at",
+			"u.DeleteAt as delete_at", "b.UserId IS NOT NULL AS is_bot").
+		From("Users as u").
+		LeftJoin("Bots b ON ( b.UserId = Users.ID )").
+		Where(sq.Eq{"u.id": userIDs})
+
+	rows, err := query.Query()
+	if err != nil {
+		return nil, err
+	}
+	defer s.CloseRows(rows)
+
+	users, err := s.usersFromRows(rows)
+	if err != nil {
+		return nil, err
+	}
+
+	return users, nil
+}
+
 func (s *MattermostAuthLayer) SearchUsersByTeam(teamID string, searchQuery string) ([]*model.User, error) {
 	query := s.getQueryBuilder().
 		Select("u.id", "u.username", "u.email", "u.nickname", "u.firstname", "u.lastname", "u.props", "u.CreateAt as create_at", "u.UpdateAt as update_at",

diff --git a/server/services/store/mockstore/mockstore.go b/server/services/store/mockstore/mockstore.go
diff --git a/server/services/store/sqlstore/public_methods.go b/server/services/store/sqlstore/public_methods.go
diff --git a/server/services/store/sqlstore/user.go b/server/services/store/sqlstore/user.go
@@ -101,6 +101,10 @@ func (s *SQLStore) getUserByID(db sq.BaseRunner, userID string) (*model.User, er
 	return s.getUserByCondition(db, sq.Eq{"id": userID})
 }
 
+func (s *SQLStore) getUsersList(db sq.BaseRunner, userIDs []string) ([]*model.User, error) {
+	return s.getUsersByCondition(db, sq.Eq{"id": userIDs}, 0)
+}
+
 func (s *SQLStore) getUserByEmail(db sq.BaseRunner, email string) (*model.User, error) {
 	return s.getUserByCondition(db, sq.Eq{"email": email})
 }

diff --git a/server/services/store/store.go b/server/services/store/store.go
@@ -55,6 +55,7 @@ type Store interface {
 
 	GetRegisteredUserCount() (int, error)
 	GetUserByID(userID string) (*model.User, error)
+	GetUsersList(userIDs []string) ([]*model.User, error)
 	GetUserByEmail(email string) (*model.User, error)
 	GetUserByUsername(username string) (*model.User, error)
 	CreateUser(user *model.User) error

diff --git a/server/ws/mocks/mockpluginapi.go b/server/ws/mocks/mockpluginapi.go
diff --git a/webapp/cypress/integration/cardURLProperty.ts b/webapp/cypress/integration/cardURLProperty.ts
@@ -99,7 +99,7 @@ describe('Card URL Property', () => {
         cy.log(`**Add ${type} view**`)
         // Intercept and wait for getUser request because it is the last one in the effects for BoardPage
         // After this last request the BoardPage component will not have additional rerenders
-        cy.intercept('GET', '/api/v2/users/u*').as('getUser')
+        cy.intercept('POST', '/api/v2/users').as('getUser')
         cy.findByRole('button', {name: 'View menu'}).click()
         cy.findByText('Add view').realHover()
         cy.findByRole('button', {name: type}).click()

diff --git a/webapp/src/octoClient.ts b/webapp/src/octoClient.ts
@@ -187,6 +187,23 @@ class OctoClient {
         return user
     }
 
+    async getUsersList(userIds: string[]): Promise<IUser[] | []> {
+        const path = `/api/v2/users`
+        const body = JSON.stringify(userIds)
+        const response = await fetch(this.getBaseURL() + path, {
+            headers: this.headers(),
+            method: 'POST',
+            body,
+        })
+
+        if(response.status !== 200) {
+            return []
+        }
+
+        return (await this.getJson(response, [])) as IUser[]
+
+    }
+
     async patchUserConfig(userID: string, patch: UserConfigPatch): Promise<Record<string, string> | undefined> {
         const path = `/api/v2/users/${encodeURIComponent(userID)}/config`
         const body = JSON.stringify(patch)

diff --git a/webapp/src/store/boards.ts b/webapp/src/store/boards.ts
@@ -28,16 +28,10 @@ export const fetchBoardMembers = createAsyncThunk(
     async ({teamId, boardId}: {teamId: string, boardId: string}, thunkAPI: any) => {
         const members = await client.getBoardMembers(teamId, boardId)
         const users = [] as IUser[]
+        const userIDs = members.map((member) => member.userId)
 
-        /* eslint-disable no-await-in-loop */
-        for (const member of members) {
-            // TODO #2968 we should fetch this in bulk
-            const user = await client.getUser(member.userId)
-            if (user) {
-                users.push(user)
-            }
-        }
-        /* eslint-enable no-await-in-loop */
+        const usersData = await client.getUsersList(userIDs)
+        users.push(...usersData)
 
         thunkAPI.dispatch(setBoardUsers(users))
         return members