[#2792] Bug/Update Approval List (#2864)

- Resolved nil pointer issue caused with no gerrit users found - Resolved wrong user search by using user_email string set field rather than lf_email Signed-off-by: Harold Wanyama <[email protected]>
communitybridge · Apr 10, 2021 · 71b020c · 71b020c
1 parent 27a4885
commit 71b020c
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 20 deletions.
diff --git a/cla-backend-go/go.sum b/cla-backend-go/go.sum
@@ -103,6 +103,7 @@ github.com/communitybridge/easycla v1.0.145 h1:ikhBSsOeEL2u3/EoyDsufh/j3HkjfFTiX
 github.com/communitybridge/easycla v2.0.10+incompatible h1:6eRJ5fxrMxRZHBkg8piYo+zHTcSowMrP85nZXzp5mpA=
 github.com/communitybridge/easycla v2.0.16+incompatible h1:I0hEApDh4IvlwRPyHV1LOsSYlSPbqBsGszjSTHwkdak=
 github.com/communitybridge/easycla v2.0.19+incompatible h1:HLaNt3jGDXPh3Au+rW/HKbJNkQf3daboVIrP9G6WYQ4=
+github.com/communitybridge/easycla v2.0.29+incompatible h1:kEply0yEyhsflUFDo7DZ0XBpncNPnsdgy6zn6ujkReY=
 github.com/coreos/bbolt v1.3.2 h1:wZwiHHUieZCquLkDL0B8UhzreNWsPHooDAG3q34zk0s=
 github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
 github.com/coreos/etcd v3.3.13+incompatible h1:8F3hqu9fGYLBifCmRCJsicFqDx/D68Rt3q1JMazcgBQ=

diff --git a/cla-backend-go/signatures/models.go b/cla-backend-go/signatures/models.go
@@ -47,3 +47,9 @@ type GerritUserResponse struct {
 	queryType           string
 	Error               error
 }
+
+// ICLAUserResponse is struct that supports ICLAUsers
+type ICLAUserResponse struct {
+	ICLASignature *models.IclaSignature
+	Error         error
+}
diff --git a/cla-backend-go/signatures/repository.go b/cla-backend-go/signatures/repository.go
@@ -2042,12 +2042,12 @@ func (repo repository) UpdateApprovalList(ctx context.Context, claManager *model
 			log.WithFields(f).Debugf("received gerrit user query results response for %s - took: %+v", results.queryType, time.Since(gerritQueryStartTime))
 			if results.Error != nil {
 				log.WithFields(f).WithError(results.Error).Warnf("problem retrieving gerrit users for %s, error: %+v", results.queryType, results.Error)
-				return nil, results.Error
-			}
-			for _, member := range results.gerritGroupResponse.Members {
-				gerritICLAECLAs = append(gerritICLAECLAs, member.Username)
+			} else {
+				for _, member := range results.gerritGroupResponse.Members {
+					gerritICLAECLAs = append(gerritICLAECLAs, member.Username)
+				}
+				log.WithFields(f).Debugf("updated gerrit user query results response for %s - list size is %d...", results.queryType, len(gerritICLAECLAs))
 			}
-			log.WithFields(f).Debugf("updated gerrit user query results response for %s - list size is %d...", results.queryType, len(gerritICLAECLAs))
 		}
 		log.WithFields(f).Debugf("received the gerrit user query results from %d go routines...", goRoutines)
 	}
@@ -2086,8 +2086,8 @@ func (repo repository) UpdateApprovalList(ctx context.Context, claManager *model
 				go func(email string) {
 					defer wg.Done()
 					log.WithFields(f).Debugf("getting cla user record for email: %s ", email)
-					claUser, userErr := repo.usersRepo.GetUserByEmail(email)
-					if userErr != nil || claUser == nil {
+					userSearch, userErr := repo.usersRepo.SearchUsers("user_emails", email, false)
+					if userErr != nil || userSearch == nil {
 						log.WithFields(f).Debugf("error getting user by email: %s ", email)
 						return
 					}
@@ -2109,21 +2109,41 @@ func (repo repository) UpdateApprovalList(ctx context.Context, claManager *model
 						approvalList.ECLAs = signs.Signatures
 					}
 
-					if claUser != nil {
-						icla, iclaErr := repo.GetIndividualSignature(ctx, projectID, claUser.UserID)
-						if iclaErr != nil || icla == nil {
-							log.WithFields(f).Debugf("unable to get icla signature for user: '%s'", email)
-						}
-						if icla != nil {
-							// Convert to IclSignature instance to leverage invalidateSignatures helper function
-							approvalList.ICLAs = []*models.IclaSignature{{
-								GithubUsername: icla.UserGHUsername,
-								LfUsername:     icla.UserLFID,
-								SignatureID:    icla.SignatureID,
-							}}
+					if len(userSearch.Users) > 0 {
+						// Try and grab iclaSignature records for users
+						results := make(chan *ICLAUserResponse, len(userSearch.Users))
+						go func() {
+							defer close(results)
+							for _, user := range userSearch.Users {
+								icla, iclaErr := repo.GetIndividualSignature(ctx, projectID, user.UserID)
+								if iclaErr != nil || icla == nil {
+									results <- &ICLAUserResponse{
+										Error: fmt.Errorf("unable to get icla for user: %s ", user.UserID),
+									}
+								} else {
+									results <- &ICLAUserResponse{
+										ICLASignature: &models.IclaSignature{
+											GithubUsername: icla.UserGHUsername,
+											LfUsername:     icla.UserLFID,
+											SignatureID:    icla.SignatureID,
+										},
+									}
+								}
+							}
+						}()
+
+						for result := range results {
+							if result.Error == nil {
+								log.WithFields(f).Debug("processing icla...")
+								approvalList.ICLAs = append(approvalList.ICLAs, result.ICLASignature)
+							}
 						}
+
 					}
 
+					// Invalidate signatures
+					repo.invalidateSignatures(ctx, &approvalList, claManager, eventArgs)
+
 					//update gerrit permissions
 					gerritUser, getGerritUserErr := repo.getGerritUserByEmail(ctx, email, gerritICLAECLAs)
 					if getGerritUserErr != nil || gerritUser == nil {
@@ -3146,7 +3166,7 @@ func (repo repository) getGerritUsers(ctx context.Context, authUser *auth.User,
 	}
 	log.WithFields(f).Debugf("querying gerrit for %s gerrit users...", claType)
 	gerritIclaUsers, getGerritQueryErr := repo.gerritService.GetUsersOfGroup(ctx, authUser, projectSFID, claType)
-	if getGerritQueryErr != nil {
+	if getGerritQueryErr != nil || gerritIclaUsers == nil {
 		msg := fmt.Sprintf("unable to fetch gerrit users for claGroup: %s , claType: %s ", projectSFID, claType)
 		log.WithFields(f).WithError(getGerritQueryErr).Warn(msg)
 		gerritResultChannel <- &GerritUserResponse{