Repository: cloudflare/cloudflare-go
Pull request #1405 (2023-09-18 00:11:44)

Update account roles
## Description

Updates Account Roles to be able to paginate.
Also updates the methods to use the new format and using the resource container
Needed for https://github.com/cloudflare/terraform-provider-cloudflare/issues/2768
## Has your change been tested?

Updated unit tests pass

## Screenshots (if appropriate):

## Types of changes

What sort of change does your code introduce/modify?

- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [X] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist:

- [X] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [X] I have added tests to cover my changes.
- [X] All new and existing tests passed.
- [X] This change is using publicly documented in [cloudflare/api-schemas](https://github.com/cloudflare/api-schemas) 
      and relies on stable APIs.



Patch:
diff --git a/.changelog/1405.txt b/.changelog/1405.txt
new file mode 100644
index 000000000..353994093
--- /dev/null
+++ b/.changelog/1405.txt
@@ -0,0 +1,11 @@
+```release-note:bug
+account_role: autopaginate all available results instead of a static number
+```
+
+```release-note:breaking-change
+account_role: `AccountRoles` has been renamed to `ListAccountRoles` to align with the updated method conventions
+```
+
+```release-note:breaking-change
+account_role: `AccountRole` has been renamed to `GetAccountRole` to align with the updated method conventions
+```
diff --git a/account_roles.go b/account_roles.go
index dec1da81c..d216ae310 100644
--- a/account_roles.go
+++ b/account_roles.go
@@ -40,35 +40,65 @@ type AccountRoleDetailResponse struct {
 	Result   AccountRole `json:"result"`
 }
 
-// AccountRoles returns all roles of an account.
+type ListAccountRolesParams struct {
+	ResultInfo
+}
+
+// ListAccountRoles returns all roles of an account.
 //
-// API reference: https://api.cloudflare.com/#account-roles-list-roles
-func (api *API) AccountRoles(ctx context.Context, accountID string) ([]AccountRole, error) {
-	uri := fmt.Sprintf("/accounts/%s/roles?per_page=50", accountID)
+// API reference: https://developers.cloudflare.com/api/operations/account-roles-list-roles
+func (api *API) ListAccountRoles(ctx context.Context, rc *ResourceContainer, params ListAccountRolesParams) ([]AccountRole, error) {
+	if rc.Identifier == "" {
+		return []AccountRole{}, ErrMissingAccountID
+	}
+	autoPaginate := true
+	if params.PerPage >= 1 || params.Page >= 1 {
+		autoPaginate = false
+	}
 
-	res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
-	if err != nil {
-		return []AccountRole{}, err
+	if params.PerPage < 1 {
+		params.PerPage = 25
 	}
 
-	var accountRolesListResponse AccountRolesListResponse
-	err = json.Unmarshal(res, &accountRolesListResponse)
-	if err != nil {
-		return []AccountRole{}, fmt.Errorf("%s: %w", errUnmarshalError, err)
+	if params.Page < 1 {
+		params.Page = 1
 	}
+	var roles []AccountRole
+	var r AccountRolesListResponse
+	for {
+		uri := buildURI(fmt.Sprintf("/accounts/%s/roles", rc.Identifier), params)
+
+		res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
+		if err != nil {
+			return []AccountRole{}, err
+		}
 
-	return accountRolesListResponse.Result, nil
+		err = json.Unmarshal(res, &r)
+		if err != nil {
+			return []AccountRole{}, fmt.Errorf("%s: %w", errUnmarshalError, err)
+		}
+		roles = append(roles, r.Result...)
+		params.ResultInfo = r.ResultInfo.Next()
+		if params.ResultInfo.Done() || !autoPaginate {
+			break
+		}
+	}
+
+	return roles, nil
 }
 
-// AccountRole returns the details of a single account role.
+// GetAccountRole returns the details of a single account role.
 //
-// API reference: https://api.cloudflare.com/#account-roles-role-details
-func (api *API) AccountRole(ctx context.Context, accountID string, roleID string) (AccountRole, error) {
-	uri := fmt.Sprintf("/accounts/%s/roles/%s", accountID, roleID)
+// API reference: https://developers.cloudflare.com/api/operations/account-roles-role-details
+func (api *API) GetAccountRole(ctx context.Context, rc *ResourceContainer, roleID string) (AccountRole, error) {
+	if rc.Identifier == "" {
+		return AccountRole{}, ErrMissingAccountID
+	}
+	uri := fmt.Sprintf("/accounts/%s/roles/%s", rc.Identifier, roleID)
 
 	res, err := api.makeRequestContext(ctx, http.MethodGet, uri, nil)
 	if err != nil {
-		return AccountRole{}, err
+		return AccountRole{}, fmt.Errorf("%s: %w", errMakeRequestError, err)
 	}
 
 	var accountRole AccountRoleDetailResponse


Test patch:
diff --git a/account_roles_test.go b/account_roles_test.go
index 7658f3dfe..22d0ac905 100644
--- a/account_roles_test.go
+++ b/account_roles_test.go
@@ -57,10 +57,14 @@ func TestAccountRoles(t *testing.T) {
 		`)
 	}
 
-	mux.HandleFunc("/accounts/01a7362d577a6c3019a474fd6f485823/roles", handler)
+	mux.HandleFunc(fmt.Sprintf("/accounts/%s/roles", testAccountID), handler)
 	want := []AccountRole{expectedAccountRole}
+	_, err := client.ListAccountRoles(context.Background(), AccountIdentifier(""), ListAccountRolesParams{})
+	if assert.Error(t, err, "Expected error when no account ID is provided") {
+		assert.Equal(t, ErrMissingAccountID, err)
+	}
 
-	actual, err := client.AccountRoles(context.Background(), "01a7362d577a6c3019a474fd6f485823")
+	actual, err := client.ListAccountRoles(context.Background(), testAccountRC, ListAccountRolesParams{})
 
 	if assert.NoError(t, err) {
 		assert.Equal(t, want, actual)
@@ -103,9 +107,13 @@ func TestAccountRole(t *testing.T) {
 		`)
 	}
 
-	mux.HandleFunc("/accounts/01a7362d577a6c3019a474fd6f485823/roles/3536bcfad5faccb999b47003c79917fb", handler)
+	mux.HandleFunc(fmt.Sprintf("/accounts/%s/roles/3536bcfad5faccb999b47003c79917fb", testAccountID), handler)
+	_, err := client.GetAccountRole(context.Background(), AccountIdentifier(""), "3536bcfad5faccb999b47003c79917fb")
+	if assert.Error(t, err, "Expected error when no account ID is provided") {
+		assert.Equal(t, ErrMissingAccountID, err)
+	}
 
-	actual, err := client.AccountRole(context.Background(), "01a7362d577a6c3019a474fd6f485823", "3536bcfad5faccb999b47003c79917fb")
+	actual, err := client.GetAccountRole(context.Background(), testAccountRC, "3536bcfad5faccb999b47003c79917fb")
 
 	if assert.NoError(t, err) {
 		assert.Equal(t, expectedAccountRole, actual)