Enhance accounts service with pagination and comprehensive testing

internal/testlib/server.go

@@ -148,3 +148,20 @@ func AddDeleteErrorToMux(uri, body string, statusCode int) {
 	}
 	addResponse(uri, body, http.MethodDelete, statusCode)
 }
+
+func AddPatchResponseToMux(uri, body string, statusCode int) {
+	if statusCode == 0 {
+		statusCode = http.StatusOK
+	}
+	if statusCode != http.StatusOK {
+		warn("non standard status code return for PATCH %s, expected %v, want %v\n", uri, http.StatusOK, statusCode)
+	}
+	addResponse(uri, body, http.MethodPatch, statusCode)
+}
+
+func AddPatchErrorToMux(uri, body string, statusCode int) {
+	if statusCode == 0 {
+		statusCode = http.StatusInternalServerError
+	}
+	addResponse(uri, body, http.MethodPatch, statusCode)
+}

pkg/services/accounts.go

@@ -50,15 +50,19 @@ type AccountService struct {
 	client *ServiceClient
 }
 
-// Returns a new instance of AccountService using the conneciton as specified
-// by c
+// NewAccountService creates and returns a new instance of AccountService using the
+// client connection specified by c. The service provides methods for managing
+// Itential Platform user accounts.
 func NewAccountService(c client.Client) *AccountService {
 	return &AccountService{
 		client: NewServiceClient(c),
 	}
 }
 
-// GetAll calls `GET /authorization/accounts`
+// GetAll retrieves all user accounts from the Itential Platform by calling
+// GET /authorization/accounts. This method handles pagination automatically,
+// fetching all accounts across multiple pages if necessary. Returns a slice
+// of Account structs or an error if the request fails.
 func (svc *AccountService) GetAll() ([]Account, error) {
 	logger.Trace()
 
@@ -67,16 +71,39 @@ func (svc *AccountService) GetAll() ([]Account, error) {
 		Total   int       `json:"total"`
 	}
 
-	var res *Response
+	var accounts []Account
+	var limit = 100
+	var skip = 0
 
-	if err := svc.client.Get("/authorization/accounts", &res); err != nil {
-		return nil, err
+	for {
+		var res *Response
+
+		if err := svc.client.GetRequest(&Request{
+			uri:    "/authorization/accounts",
+			params: &QueryParams{Limit: limit, Skip: skip},
+		}, &res); err != nil {
+			return nil, err
+		}
+
+		for _, ele := range res.Results {
+			accounts = append(accounts, ele)
+		}
+
+		if len(accounts) == res.Total {
+			break
+		}
+
+		skip += limit
 	}
 
-	return res.Results, nil
+	logger.Info("Found %v account(s)", len(accounts))
+
+	return accounts, nil
 }
 
-// Get invokes `GET /authorization/accounts/{id}`
+// Get retrieves a specific user account by ID from the Itential Platform by
+// calling GET /authorization/accounts/{id}. Returns a pointer to the Account
+// struct or an error if the account is not found or the request fails.
 func (svc *AccountService) Get(id string) (*Account, error) {
 	logger.Trace()
 
@@ -91,10 +118,10 @@ func (svc *AccountService) Get(id string) (*Account, error) {
 	return res, nil
 }
 
-// GetByName accepts a single argument which is the username of the acccount to
-// retrieve.  This function will iterate through all accounts and attempt to
-// match the username against the returned data.  If a match is found, an
-// Account is returned.  If a match is not found, an error is returned.
+// GetByName retrieves a user account by username from the Itential Platform.
+// This method fetches all accounts and searches for a matching username.
+// Returns a pointer to the matching Account struct or an error if no account
+// with the specified username is found or if the request fails.
 func (svc *AccountService) GetByName(name string) (*Account, error) {
 	logger.Trace()
 
@@ -119,6 +146,9 @@ func (svc *AccountService) GetByName(name string) (*Account, error) {
 	return res, nil
 }
 
+// Deactivate sets an account to inactive status by calling PATCH /authorization/accounts/{id}
+// with inactive=true. The account will no longer be able to access the system.
+// Returns an error if the request fails or the account is not found.
 func (svc *AccountService) Deactivate(id string) error {
 	logger.Trace()
 	return svc.client.PatchRequest(&Request{
@@ -127,6 +157,9 @@ func (svc *AccountService) Deactivate(id string) error {
 	}, nil)
 }
 
+// Activate sets an account to active status by calling PATCH /authorization/accounts/{id}
+// with inactive=false. The account will be able to access the system again.
+// Returns an error if the request fails or the account is not found.
 func (svc *AccountService) Activate(id string) error {
 	logger.Trace()
 	return svc.client.PatchRequest(&Request{

pkg/services/accounts_test.go

@@ -32,12 +32,21 @@ func TestAccountGetAll(t *testing.T) {
 		response := testlib.Fixture(
 			filepath.Join(fixtureRoot, ele, accountsGetAllSuccess),
 		)
+		// The new pagination implementation uses GetRequest, so we need to handle query parameters
 		testlib.AddGetResponseToMux("/authorization/accounts", response, 0)
 
 		res, err := svc.GetAll()
 
 		assert.Nil(t, err)
 		assert.Equal(t, 2, len(res))
+		if len(res) > 0 {
+			assert.NotEmpty(t, res[0].Id)
+			assert.NotEmpty(t, res[0].Username)
+		}
+		if len(res) > 1 {
+			assert.NotEmpty(t, res[1].Id)
+			assert.NotEmpty(t, res[1].Username)
+		}
 	}
 }
 
@@ -62,22 +71,23 @@ func TestAccountGet(t *testing.T) {
 			filepath.Join(fixtureRoot, ele, accountsGetSuccess),
 		)
 
-		testlib.AddGetResponseToMux("/authorization/accounts/{id}", response, 0)
+		// Use specific path that matches the Get method implementation
+		testlib.AddGetResponseToMux("/authorization/accounts/ID", response, 0)
 
 		res, err := svc.Get("ID")
 
 		assert.Nil(t, err)
 		assert.NotNil(t, res)
 		assert.Equal(t, reflect.TypeOf((*Account)(nil)), reflect.TypeOf(res))
-		assert.True(t, res.Id != "")
+		assert.NotEmpty(t, res.Id)
 	}
 }
 
 func TestAccountGetError(t *testing.T) {
 	svc := setupAccountService()
 	defer testlib.Teardown()
 
-	testlib.AddGetErrorToMux("/authorization/accounts", "", 0)
+	testlib.AddGetErrorToMux("/authorization/accounts/TEST", "", 0)
 
 	res, err := svc.Get("TEST")
 
@@ -100,8 +110,12 @@ func TestAccountGetByName(t *testing.T) {
 		assert.Nil(t, err)
 		assert.NotNil(t, res)
 		assert.Equal(t, reflect.TypeOf((*Account)(nil)), reflect.TypeOf(res))
-		assert.True(t, res.Id != "")
-		assert.True(t, res.Username == "admin@pronghorn")
+		assert.NotEmpty(t, res.Id)
+		assert.Equal(t, "admin@pronghorn", res.Username)
+		assert.Equal(t, "admin", res.FirstName)
+		assert.Equal(t, "local_aaa", res.Provenance)
+		assert.False(t, res.Inactive)
+		assert.True(t, res.LoggedIn)
 
 	}
 }
@@ -118,3 +132,76 @@ func TestAccountGetByNameError(t *testing.T) {
 	assert.Nil(t, res)
 	assert.Equal(t, reflect.TypeOf((*Account)(nil)), reflect.TypeOf(res))
 }
+
+func TestAccountGetByNameNotFound(t *testing.T) {
+	svc := setupAccountService()
+	defer testlib.Teardown()
+
+	for _, ele := range fixtureSuites {
+		response := testlib.Fixture(
+			filepath.Join(fixtureRoot, ele, accountsGetAllSuccess),
+		)
+		testlib.AddGetResponseToMux("/authorization/accounts", response, 0)
+
+		res, err := svc.GetByName("nonexistent@user")
+
+		assert.NotNil(t, err)
+		assert.Nil(t, res)
+		assert.Equal(t, "account not found", err.Error())
+	}
+}
+
+func TestAccountActivate(t *testing.T) {
+	svc := setupAccountService()
+	defer testlib.Teardown()
+
+	testlib.AddPatchResponseToMux("/authorization/accounts/test-id", "", 0)
+
+	err := svc.Activate("test-id")
+
+	assert.Nil(t, err)
+}
+
+func TestAccountActivateError(t *testing.T) {
+	svc := setupAccountService()
+	defer testlib.Teardown()
+
+	testlib.AddPatchErrorToMux("/authorization/accounts/test-id", "", 0)
+
+	err := svc.Activate("test-id")
+
+	assert.NotNil(t, err)
+}
+
+func TestAccountDeactivate(t *testing.T) {
+	svc := setupAccountService()
+	defer testlib.Teardown()
+
+	testlib.AddPatchResponseToMux("/authorization/accounts/test-id", "", 0)
+
+	err := svc.Deactivate("test-id")
+
+	assert.Nil(t, err)
+}
+
+func TestAccountDeactivateError(t *testing.T) {
+	svc := setupAccountService()
+	defer testlib.Teardown()
+
+	testlib.AddPatchErrorToMux("/authorization/accounts/test-id", "", 0)
+
+	err := svc.Deactivate("test-id")
+
+	assert.NotNil(t, err)
+}
+
+func TestNewAccountService(t *testing.T) {
+	client := testlib.Setup()
+	defer testlib.Teardown()
+
+	svc := NewAccountService(client)
+
+	assert.NotNil(t, svc)
+	assert.NotNil(t, svc.client)
+	assert.Equal(t, reflect.TypeOf((*AccountService)(nil)), reflect.TypeOf(svc))
+}