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

[FDN-3587] Add new Orgs / Teams / Suites REST API Documentation #2612

Merged
merged 16 commits into from
Mar 7, 2024

Conversation

jfhinchcliffe
Copy link
Contributor

@jfhinchcliffe jfhinchcliffe commented Jan 11, 2024

Consolidates all the recent teams API additions into a single PR.

🚧 Should not be merged until we've enabled the API changes on the main app.

@mbelton-buildkite this should contain all the feedback that that you kindly provided on the #2598 and #2599 PRs. I just found it easier to put all this related work in a single PR as they'll most likely be released on the main site at the same time and it saves on merge conflicts on the nav changes etc.

As part of this I pulled 'Organization' out into its own nav item under APIS / REST, as these new endpoints are nested under the organization's / teams route in our API. eg:

curl --request GET \
  --url https://api.buildkite.com/v2/organizations/hinchys-fondue-factory/teams/c5e09619-8648-4896-a936-9d0b8b7b3fe9/suites \
  --header 'Authorization: Bearer TOKEN'

Screenshot 2024-01-18 at 3 51 29 pm

❓ We could think about adding another submenu under organization for Teams

Each commit represents another resource added to the API - would be simplest to review commit-by-commit

  • Team Pipelines
  • Team Members
  • Team Suites
  • Org Members
  • Org Teams

Linear ticket:
https://linear.app/buildkite/issue/FDN-3587/add-docs-pr-for-new-rest-api-functionality

@jfhinchcliffe jfhinchcliffe changed the title Jh/docs/team api docs overarching branch Add new Orgs / Teams / Suites REST API Documentation Jan 11, 2024
@jfhinchcliffe jfhinchcliffe marked this pull request as draft January 11, 2024 05:36
@buildkite-docs-bot
Copy link
Contributor

Preview URL: https://2612--bk-docs-preview.netlify.app

@jfhinchcliffe jfhinchcliffe force-pushed the jh/docs/team-api-docs-overarching-branch branch 4 times, most recently from 8b8d988 to 1de7a2f Compare January 12, 2024 00:49
@jfhinchcliffe jfhinchcliffe marked this pull request as ready for review January 12, 2024 01:08
@jfhinchcliffe jfhinchcliffe changed the title Add new Orgs / Teams / Suites REST API Documentation [FDN-3587] Add new Orgs / Teams / Suites REST API Documentation Jan 15, 2024
@jfhinchcliffe jfhinchcliffe force-pushed the jh/docs/team-api-docs-overarching-branch branch from 74d2c1c to 98dfe18 Compare January 18, 2024 04:52
@jameshill
Copy link
Contributor

@jfhinchcliffe could you give this a rebase ❤️

@jfhinchcliffe jfhinchcliffe force-pushed the jh/docs/team-api-docs-overarching-branch branch from 98dfe18 to 48ba88a Compare March 4, 2024 22:24
@jfhinchcliffe jfhinchcliffe requested a review from gilesgas as a code owner March 4, 2024 22:24
@jfhinchcliffe
Copy link
Contributor Author

@jameshill rebased!
Screenshot 2024-03-05 at 9 32 18 am

@jfhinchcliffe
Copy link
Contributor Author

Just re-testing / verifying the teams endpoint:
Teams Index ✅
Screenshot 2024-03-05 at 10 00 03 am

Team Show ✅
Screenshot 2024-03-05 at 10 00 44 am

Team Create ✅
Screenshot 2024-03-05 at 10 03 00 am

Team Update ✅
Screenshot 2024-03-05 at 10 03 45 am

Team Destroy ✅
Screenshot 2024-03-05 at 10 04 35 am

Team Members ✅
Screenshot 2024-03-05 at 10 01 33 am

TBC

@jfhinchcliffe jfhinchcliffe force-pushed the jh/docs/team-api-docs-overarching-branch branch from 2f28671 to 3b72032 Compare March 5, 2024 00:59
@gilesgas
Copy link
Contributor

gilesgas commented Mar 6, 2024

Thanks very much for all this work @jfhinchcliffe !

This is looking really good!

I've noticed a small issue with the left-hand nav—most likely caused by a combination of the repeated usage of item name higher up in the nav:

nav-problem-multiple-items-selected

I think I've got an idea of how to resolve this but would like to run this by @mbelton-buildkite first

data/nav.yml Outdated Show resolved Hide resolved
app/models/beta_pages.rb Outdated Show resolved Hide resolved
app/models/beta_pages.rb Outdated Show resolved Hide resolved
data/nav.yml Outdated Show resolved Hide resolved
@gilesgas
Copy link
Contributor

gilesgas commented Mar 6, 2024

Since we are substantially re-organising the page nav as part of this PR, we should also consider where we should move the following to:


Just had a follow-up discussion with @mbelton-buildkite and since the Organizations REST API endpoint appears to be used by Test Analytics too, which we treat as a separate product, then it makes sense to promote this page hierarchy to the top level.

The others I listed above are essentially only utilised by the core Buildkite product (Pipelines) and so at least for the time being, make sense being listed within the 'Pipelines' hierarchy.

Possible question marks about Emojis and User, but OK to keep within Pipelines and not have that hold up merging this PR.


Following a subsequent discussion with @jfhinchcliffe , Organizations was pulled out of the Pipelines hierarchy due to the extension of Teams (which was also originally nested as a single page within Pipelines).
The Teams endpoint is involved in both Pipelines and Test Analytics features. Therefore, it makes more sense now keeping Organizations and Teams out in their own top-level hierarchies within the REST section.

The other REST API endpoint docs listed above make more sense leaving within Pipelines, with the possible/likely exceptions of Emojis and User, but these can be dealt with later.

app/models/beta_pages.rb Outdated Show resolved Hide resolved
Comment on lines +455 to +458
- name: "Pipelines "
children:
- name: "Overview"
path: "apis/rest-api/pipelines"
Copy link
Contributor

@gilesgas gilesgas Mar 6, 2024

Choose a reason for hiding this comment

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

I needed to add a space after the word Pipelines here (i.e. "Pipelines ") to prevent the Pipelines section in the 'global' nav from being highlighted too when viewing its Overview page.

This seems to be a bit of a minor bug with our documentation site, whereby having this type of item structure within this YAML file:

        - name: "Something specific"
          ...
            - name: "Something else"
              ...

and then viewing any page within "Something specific" (e.g. "Something else") would cause any element that's also called "Something specific" elsewhere on the page to become highlighted too.

Here's an example of what I mean, where the "Something specific" in this case is "Pipelines":

multiple-elements-being-highlighted

Hence, making "Pipelines" unique by adding a space after it ("Pipelines ") seemed to work around this issue.

@dannymidnight , would you know if there's anyway to fix this issue with our docs site? (Feel free to set up a call if you need me to demo more context, and as an FYI, fixing this issue is minor and won't block merging this PR due to the workaround explained above.)

@github-actions github-actions bot added the agent label Mar 7, 2024
@gilesgas
Copy link
Contributor

gilesgas commented Mar 7, 2024

Hi @jfhinchcliffe ,

I've pushed through a few changes to the docs, which I can summarise as follows:

  • Changed the names of the nested 'Organizations' and 'Pipelines' pages to 'Overview' to avoid the minor docs site highlighting bug we discussed before (and described above).
  • Added intros to the tops of these two (now-named) 'Overview' pages to explain the new page re-organisation in the docs.
  • Modified the way 'beta' labels appear on each of the new 'Teams' pages - i.e. moved them away from the nav to the page headings themselves.
  • Made all the REST API calls (throughout this section of the docs) consistent (and added header for $TOKEN to indicate which calls require access token authentication). I started noticing too many different ways of representing curl calls throughout the docs, which is not great.

Please let me know if this is all good with you and I can merged it in.


```bash
curl -H "Authorization: Bearer $TOKEN" \
-X DELETE "https://api.buildkite.com/v2/organizations/{org.slug}/teams/{team.uuid}"
Copy link
Contributor

@gilesgas gilesgas Mar 7, 2024

Choose a reason for hiding this comment

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

On a few of these DELETE calls, I removed the -H "Content-Type: application/json" \ options as I strongly suspect these are not required (by virtue of no JSON payload being required/sent in the request).

@jfhinchcliffe jfhinchcliffe merged commit 39f2435 into main Mar 7, 2024
3 checks passed
@jfhinchcliffe jfhinchcliffe deleted the jh/docs/team-api-docs-overarching-branch branch March 7, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants