From 36654d661222b3ca52d9acff46b5a83933447157 Mon Sep 17 00:00:00 2001
From: bcmmbaga <bethuelmbaga12@gmail.com>
Date: Mon, 23 Dec 2024 22:15:20 +0300
Subject: [PATCH] Refactor and fix tests

Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
---
 management/server/idp/okta.go      | 36 ++++++----------
 management/server/idp/okta_test.go | 69 +++++++++++++-----------------
 2 files changed, 43 insertions(+), 62 deletions(-)

diff --git a/management/server/idp/okta.go b/management/server/idp/okta.go
index 76c79976785..0a36ed7e5c7 100644
--- a/management/server/idp/okta.go
+++ b/management/server/idp/okta.go
@@ -119,10 +119,7 @@ func (om *OktaManager) GetUserDataByID(ctx context.Context, userID string, appMe
 		return nil, fmt.Errorf("unable to get user %s, statusCode %d", userID, resp.StatusCode)
 	}
 
-	userData, err := parseOktaUser(user)
-	if err != nil {
-		return nil, err
-	}
+	userData := parseOktaUser(user)
 	userData.AppMetadata = appMetadata
 
 	return userData, nil
@@ -150,11 +147,7 @@ func (om *OktaManager) GetUserByEmail(_ context.Context, email string) ([]*UserD
 
 	usersData := make([]*UserData, 0, len(users))
 	for _, user := range users {
-		userData, err := parseOktaUser(&user)
-		if err != nil {
-			return nil, err
-		}
-		usersData = append(usersData, userData)
+		usersData = append(usersData, parseOktaUser(&user))
 	}
 
 	return usersData, nil
@@ -230,12 +223,7 @@ func (om *OktaManager) getAllUsers() ([]*UserData, error) {
 
 	users := make([]*UserData, 0, len(userList))
 	for _, user := range userList {
-		userData, err := parseOktaUser(&user)
-		if err != nil {
-			return nil, err
-		}
-
-		users = append(users, userData)
+		users = append(users, parseOktaUser(&user))
 	}
 
 	return users, nil
@@ -280,16 +268,20 @@ type oktaUser interface {
 }
 
 // parseOktaUser parse okta user to UserData.
-func parseOktaUser(user oktaUser) (*UserData, error) {
-	if user == nil {
-		return nil, fmt.Errorf("invalid okta user")
-	}
-
+func parseOktaUser(user oktaUser) *UserData {
 	profile := user.GetProfile()
 
+	var names []string
+	if firstName := profile.GetFirstName(); firstName != "" {
+		names = append(names, firstName)
+	}
+	if lastName := profile.GetLastName(); lastName != "" {
+		names = append(names, lastName)
+	}
+
 	return &UserData{
 		Email: profile.GetEmail(),
-		Name:  strings.Join([]string{profile.GetFirstName(), profile.GetLastName()}, " "),
+		Name:  strings.Join(names, " "),
 		ID:    user.GetId(),
-	}, nil
+	}
 }
diff --git a/management/server/idp/okta_test.go b/management/server/idp/okta_test.go
index 20df246f82a..13c963a07a3 100644
--- a/management/server/idp/okta_test.go
+++ b/management/server/idp/okta_test.go
@@ -3,63 +3,52 @@ package idp
 import (
 	"testing"
 
-	"github.com/okta/okta-sdk-golang/v2/okta"
+	"github.com/okta/okta-sdk-golang/v5/okta"
 	"github.com/stretchr/testify/assert"
 )
 
 func TestParseOktaUser(t *testing.T) {
 	type parseOktaUserTest struct {
 		name             string
-		inputProfile     *okta.User
+		inputUser        *okta.User
 		expectedUserData *UserData
 		assertErrFunc    assert.ErrorAssertionFunc
 	}
 
-	parseOktaTestCase1 := parseOktaUserTest{
-		name: "Good Request",
-		inputProfile: &okta.User{
-			Id: "123",
-			Profile: &okta.UserProfile{
-				"email":     "test@example.com",
-				"firstName": "John",
-				"lastName":  "Doe",
+	testCases := []parseOktaUserTest{
+		{
+			name: "valid okta user",
+			inputUser: &okta.User{
+				Id: okta.PtrString("123"),
+				Profile: &okta.UserProfile{
+					Email:     okta.PtrString("test@example.com"),
+					FirstName: *okta.NewNullableString(okta.PtrString("John")),
+					LastName:  *okta.NewNullableString(okta.PtrString("Doe")),
+				},
 			},
-		},
-		expectedUserData: &UserData{
-			Email: "test@example.com",
-			Name:  "John Doe",
-			ID:    "123",
-			AppMetadata: AppMetadata{
-				WTAccountID: "456",
+			expectedUserData: &UserData{
+				Email: "test@example.com",
+				Name:  "John Doe",
+				ID:    "123",
+				AppMetadata: AppMetadata{
+					WTAccountID: "456",
+				},
 			},
+			assertErrFunc: assert.NoError,
+		},
+		{
+			name:             "invalid okta user",
+			inputUser:        nil,
+			expectedUserData: &UserData{},
 		},
-		assertErrFunc: assert.NoError,
-	}
-
-	parseOktaTestCase2 := parseOktaUserTest{
-		name:             "Invalid okta user",
-		inputProfile:     nil,
-		expectedUserData: nil,
-		assertErrFunc:    assert.Error,
 	}
 
-	for _, testCase := range []parseOktaUserTest{parseOktaTestCase1, parseOktaTestCase2} {
-		t.Run(testCase.name, func(t *testing.T) {
-			userData, err := parseOktaUser(testCase.inputProfile)
-			testCase.assertErrFunc(t, err, testCase.assertErrFunc)
-
-			if err == nil {
-				assert.True(t, userDataEqual(testCase.expectedUserData, userData), "user data should match")
-			}
+	for _, tt := range testCases {
+		t.Run(tt.name, func(t *testing.T) {
+			userData := parseOktaUser(tt.inputUser)
+			assert.Equal(t, tt.expectedUserData, userData, "user data should match")
 		})
 
 	}
-}
 
-// userDataEqual helper function to compare UserData structs for equality.
-func userDataEqual(a, b *UserData) bool {
-	if a.Email != b.Email || a.Name != b.Name || a.ID != b.ID {
-		return false
-	}
-	return true
 }