Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#4348] Bug Icla and Company check #4382

Merged
merged 1 commit into from
Jul 11, 2024
Merged

[#4348] Bug Icla and Company check #4382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions cla-backend-go/users/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,22 @@ package users

// DBUser data model
type DBUser struct {
UserID string `json:"user_id"`
UserExternalID string `json:"user_external_id"`
LFEmail string `json:"lf_email"`
Admin bool `json:"admin"`
LFUsername string `json:"lf_username"`
DateCreated string `json:"date_created"`
DateModified string `json:"date_modified"`
UserName string `json:"user_name"`
Version string `json:"version"`
UserEmails UserEmails `json:"user_emails"`
UserGithubID string `json:"user_github_id"`
UserGithubUsername string `json:"user_github_username"`
UserGitlabID string `json:"user_gitlab_id"`
UserGitlabUsername string `json:"user_gitlab_username"`
UserCompanyID string `json:"user_company_id"`
Note string `json:"note"`
UserID string `json:"user_id"`
UserExternalID string `json:"user_external_id"`
LFEmail string `json:"lf_email"`
Admin bool `json:"admin"`
LFUsername string `json:"lf_username"`
DateCreated string `json:"date_created"`
DateModified string `json:"date_modified"`
UserName string `json:"user_name"`
Version string `json:"version"`
UserEmails []string `json:"user_emails"`
UserGithubID string `json:"user_github_id"`
UserGithubUsername string `json:"user_github_username"`
UserGitlabID string `json:"user_gitlab_id"`
UserGitlabUsername string `json:"user_gitlab_username"`
UserCompanyID string `json:"user_company_id"`
Note string `json:"note"`
}

type UserEmails struct {
Expand Down
7 changes: 1 addition & 6 deletions cla-backend-go/users/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1254,11 +1254,6 @@ func (repo repository) UpdateUserCompanyID(userID, companyID, note string) error

// convertDBUserModel translates a dyanamoDB data model into a service response model
func convertDBUserModel(user DBUser) *models.User {
var emails []string
if user.UserEmails.SS != nil {
emails = user.UserEmails.SS
}

return &models.User{
UserID: user.UserID,
UserExternalID: user.UserExternalID,
Expand All @@ -1269,7 +1264,7 @@ func convertDBUserModel(user DBUser) *models.User {
DateModified: user.DateModified,
Username: user.UserName,
Version: user.Version,
Emails: emails,
Emails: user.UserEmails,
GithubID: user.UserGithubID,
GithubUsername: user.UserGithubUsername,
GitlabID: user.UserGitlabID,
Expand Down
11 changes: 6 additions & 5 deletions cla-backend-go/v2/signatures/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,8 @@ func (s *Service) IsUserAuthorized(ctx context.Context, lfid, claGroupId string)
log.WithFields(f).Debug("user has signed ICLA")
response.ICLA = true
hasSigned = true
} else {
log.WithFields(f).Debug("user has not signed ICLA")
}

// fetch company
Expand All @@ -505,13 +507,12 @@ func (s *Service) IsUserAuthorized(ctx context.Context, lfid, claGroupId string)
} else {
log.WithFields(f).Debug("fetching company")
companyModel, err := s.v1CompanyService.GetCompany(ctx, user.CompanyID)
if err != nil {
if companyErr, ok := err.(*utils.CompanyNotFound); ok {
log.WithFields(f).WithError(companyErr).Debug("company not found")
response.CompanyAffiliation = false
} else if err != nil {
log.WithFields(f).WithError(err).Debug("unable to fetch company")
return nil, err
}
if companyModel == nil {
log.WithFields(f).Debug("company not found")
response.CompanyAffiliation = false
} else {
log.WithFields(f).Debug("company found")
response.CompanyAffiliation = true
Expand Down
72 changes: 66 additions & 6 deletions cla-backend-go/v2/signatures/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ package signatures
import (
"context"
"errors"
"fmt"
"testing"

v1Models "github.com/communitybridge/easycla/cla-backend-go/gen/v1/models"
"github.com/communitybridge/easycla/cla-backend-go/gen/v2/models"
"github.com/communitybridge/easycla/cla-backend-go/utils"

// mock_signatures "github.com/communitybridge/easycla/cla-backend-go/v2/signatures/mock_v1_signatures"
mock_company "github.com/communitybridge/easycla/cla-backend-go/company/mocks"
Expand All @@ -24,6 +24,7 @@ import (

func TestService_IsUserAuthorized(t *testing.T) {
type testCase struct {
name string
lfid string
projectID string
userID string
Expand All @@ -40,10 +41,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA bool
expectedCCLA bool
expectedCompanyAffiliation bool
getCompanyResult *v1Models.Company
getCompanyError error
}

cases := []testCase{
{
name: "claGroupRequiresICLA",
lfid: "foobar_1",
projectID: "project-123",
userID: "user-123",
Expand All @@ -66,8 +70,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: true,
expectedCCLA: true,
expectedCompanyAffiliation: true,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "claGroupDoesNotRequireICLA",
lfid: "foobar_2",
projectID: "project-123",
userID: "user-123",
Expand All @@ -90,8 +99,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: true,
expectedCCLA: true,
expectedCompanyAffiliation: true,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "icla signature found",
lfid: "foobar_3",
projectID: "project-123",
userID: "user-123",
Expand All @@ -114,8 +128,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: true,
expectedCCLA: false,
expectedCompanyAffiliation: true,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "icla signature not found",
lfid: "foobar_4",
projectID: "project-123",
userID: "user-123",
Expand All @@ -136,8 +155,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: false,
expectedCCLA: true,
expectedCompanyAffiliation: true,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "individual signature error",
lfid: "foobar_5",
projectID: "project-123",
userID: "user-123",
Expand All @@ -157,8 +181,13 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: false,
expectedCCLA: false,
expectedCompanyAffiliation: true,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "user has not signed ccla and icla",
lfid: "foobar_6",
projectID: "project-123",
userID: "user-123",
Expand All @@ -171,11 +200,42 @@ func TestService_IsUserAuthorized(t *testing.T) {
expectedICLA: false,
expectedCCLA: false,
expectedCompanyAffiliation: false,
getCompanyResult: &v1Models.Company{
CompanyID: "company-123",
},
getCompanyError: nil,
},
{
name: "user has icla and has company id that does not exist",
lfid: "foobar_7",
projectID: "project-123",
userID: "user-123",
companyID: "company-123",
getUserByLFUsernameResult: &v1Models.User{
UserID: "user-123",
CompanyID: "company-123",
},
getUserByLFUsernameError: nil,
claGroupRequiresICLA: false,
expectedAuthorized: true,
expectedCCLARequiresICLA: false,
expectedICLA: true,
expectedCCLA: false,
expectedCompanyAffiliation: false,
getCompanyResult: nil,
getCompanyError: &utils.CompanyNotFound{
Message: "no company matching company record",
CompanyID: "company-123",
},
getIndividualSignatureResult: &v1Models.Signature{
SignatureID: "signature-123",
},
getIndividualSignatureError: nil,
},
}

for _, tc := range cases {
t.Run(fmt.Sprintf("LFID=%s ProjectID=%s", tc.lfid, tc.projectID), func(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

Expand Down Expand Up @@ -204,12 +264,12 @@ func TestService_IsUserAuthorized(t *testing.T) {
signed := true
mockSignatureService.EXPECT().GetIndividualSignature(context.Background(), tc.projectID, tc.userID, &approved, &signed).Return(tc.getIndividualSignatureResult, tc.getIndividualSignatureError)

mockSignatureService.EXPECT().ProcessEmployeeSignature(context.Background(), gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.processEmployeeSignatureResult, tc.processEmployeeSignatureError)
if tc.getCompanyError == nil {
mockSignatureService.EXPECT().ProcessEmployeeSignature(context.Background(), gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.processEmployeeSignatureResult, tc.processEmployeeSignatureError)
}

mockCompanyService := mock_company.NewMockIService(ctrl)
mockCompanyService.EXPECT().GetCompany(context.Background(), tc.companyID).Return(&v1Models.Company{
CompanyID: tc.companyID,
}, nil)
mockCompanyService.EXPECT().GetCompany(context.Background(), tc.companyID).Return(tc.getCompanyResult, tc.getCompanyError)

service := NewService(awsSession, "", mockProjectService, mockCompanyService, mockSignatureService, nil, nil, mockUserService, nil)

Expand Down
Loading