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.

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

Provide pageable org users endpoint #2495

Merged
merged 1 commit into from
Sep 9, 2021
Merged

Conversation

ctlong
Copy link
Member

@ctlong ctlong commented Sep 8, 2021

Explained in #2489. Adds /v3/organizations/:guid/users.

  • I have reviewed the contributing guide

  • I have viewed, signed, and submitted the Contributor License Agreement

  • I have made this pull request to the main branch

  • I have run all the unit tests using bundle exec rake

  • I have run CF Acceptance Tests

@sweinstein22 sweinstein22 linked an issue Sep 8, 2021 that may be closed by this pull request
@ctlong ctlong force-pushed the org-users-endpoint branch 4 times, most recently from be1c3ec to 1a48578 Compare September 9, 2021 02:40
Copy link
Contributor

@sweinstein22 sweinstein22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running through preliminary acceptance even though this is still a draft as I'm going to be out of office today.

Behavior Check

Confirmed that result of running /v3/organizations/:guid/users returned fewer results than /v3/users and that output looked correct:

$ cf curl /v3/organizations/c2daec28-4340-4e9c-bf94-a026419b2b8c/users | grep total_results
    "total_results": 8,

$ cf curl /v3/users | grep total_results
    "total_results": 10,

Compared admin result to result as each role:

$ diff admin org-auditor

$ diff admin org-manager

$ diff admin space-auditor

$ diff admin space-manager

$ diff admin space-supporter

$ diff admin space-dev

Code Check

Pending further discussion with the team to talk about unifying our approach, but these changes look reasonable to me if this is the approach we decide on

Copy link
Contributor

@sweinstein22 sweinstein22 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs Check

Confirmed that docs render correctly, looks like we've got a couple typos in the work "organization" (currently spelled "organzation") - this should be adjusted in the sidebar, the title, and the definition of the section for this new endpoint

I believe we also chatted about changing up the wording to be 'List users for an organization', to prevent confusion given that an org user is an assignable role. If we do make this adjustment it would be great to align the space level endpoint with the new wording

@ctlong ctlong marked this pull request as ready for review September 9, 2021 18:38
@ctlong ctlong requested review from sweinstein22 and removed request for sweinstein22 September 9, 2021 18:38
@ctlong ctlong dismissed sweinstein22’s stale review September 9, 2021 18:51

Resolved; ready for a new review

@JenGoldstrich
Copy link
Contributor

Acceptance

`
cf curl /v3/organizations/15c8f550-9a9a-49f0-ac83-3b2a14891423/users
{
"pagination": {
"total_results": 2,
"total_pages": 1,
"first": {
"href": "https://api/v3/organizations/15c8f550-9a9a-49f0-ac83-3b2a14891423/users?page=1&per_page=50"
},
"last": {
"href": "https://api/v3/organizations/15c8f550-9a9a-49f0-ac83-3b2a14891423/users?page=1&per_page=50"
},
"next": null,
"previous": null
},
"resources": [
{
"guid": "f1639818-b0f7-4386-bd58-0f26fa5973c3",
"created_at": "2021-09-08T18:56:49Z",
"updated_at": "2021-09-08T18:56:49Z",
"username": "admin",
"presentation_name": "admin",
"origin": "uaa",
"metadata": {
"labels": {

    },
    "annotations": {

    }
  },
  "links": {
    "self": {
      "href": "https://api/v3/users/f1639818-b0f7-4386-bd58-0f26fa5973c3"
    }
  }
},
{
  "guid": "9bf359d3-82f7-431f-a6ef-b526a0090545",
  "created_at": "2021-09-09T19:00:10Z",
  "updated_at": "2021-09-09T19:00:10Z",
  "username": "cool",
  "presentation_name": "cool",
  "origin": "uaa",
  "metadata": {
    "labels": {

    },
    "annotations": {

    }
  },
  "links": {
    "self": {
      "href": "https://api/v3/users/9bf359d3-82f7-431f-a6ef-b526a0090545"
    }
  }
}

]
}
`

Checked this after assigning a second user to this org, LGTM, merging!

@JenGoldstrich JenGoldstrich merged commit 7aed55e into main Sep 9, 2021
@sweinstein22 sweinstein22 deleted the org-users-endpoint branch September 10, 2021 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No pageable way to gets users by space or org
3 participants