From 71b020cf81c911c607fd47b868d58205c9fbcf50 Mon Sep 17 00:00:00 2001 From: Harold Wanyama <81645226+nickmango@users.noreply.github.com> Date: Sat, 10 Apr 2021 21:20:51 +0300 Subject: [PATCH] [#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 --- cla-backend-go/go.sum | 1 + cla-backend-go/signatures/models.go | 6 +++ cla-backend-go/signatures/repository.go | 60 ++++++++++++++++--------- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/cla-backend-go/go.sum b/cla-backend-go/go.sum index 70303eb34..d094865eb 100644 --- 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 index 41a3d0e2e..ae46a2304 100644 --- 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 index 30d071826..a8c9158b6 100644 --- 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{