Skip to content

Add missing fields to Team struct#295

Merged
mcncl merged 3 commits into
mainfrom
add-missing-team-fields
Apr 8, 2026
Merged

Add missing fields to Team struct#295
mcncl merged 3 commits into
mainfrom
add-missing-team-fields

Conversation

@omehegan

@omehegan omehegan commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Problem

The Team struct is missing several fields that the Buildkite API returns in its team responses (presenter):

  • GraphQLID
  • DefaultMemberRole
  • MembersCanCreatePipelines
  • MembersCanCreateSuites
  • MembersCanCreateRegistries
  • MembersCanDestroyRegistries
  • MembersCanDestroyPackages

These fields are silently dropped during JSON deserialization when reading a team from the API.

Impact

This causes a data-loss bug for any consumer performing a read-modify-write update pattern (GET team → modify a field → PATCH team). Since the CreateTeam struct used for updates includes MembersCanCreatePipelines as a non-omitempty bool, the PATCH request will always send members_can_create_pipelines: false, resetting the value regardless of the team's actual setting.

This directly affects buildkite/cli#727 which adds a bk team update command using this pattern.

Changes

  • Added the 7 missing fields to the Team struct
  • Updated TestTeamsService_List and TestTeamsService_GetTeam to verify the new fields deserialize correctly

The Team struct was missing several fields that the API returns:
GraphQLID, DefaultMemberRole, MembersCanCreatePipelines,
MembersCanCreateSuites, MembersCanCreateRegistries,
MembersCanDestroyRegistries, and MembersCanDestroyPackages.

Without these fields, API consumers performing read-modify-write
updates (eg GET team, modify a field, PATCH team) would silently
reset these values to their zero values. In particular,
MembersCanCreatePipelines would always be sent as false on updates
because the current team state could never be read into the struct.
@omehegan omehegan requested a review from a team as a code owner April 7, 2026 06:24
Comment thread teams.go
Comment on lines +30 to +33
MembersCanCreateSuites bool `json:"members_can_create_suites"`
MembersCanCreateRegistries bool `json:"members_can_create_registries"`
MembersCanDestroyRegistries bool `json:"members_can_destroy_registries"`
MembersCanDestroyPackages bool `json:"members_can_destroy_packages"`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't these also need to be added to the CreateTeam struct?

@mcncl

mcncl commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

We're not checking on the Create/Update tests for the new fields

@mcncl

mcncl commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

ListForUser tests should also contain the new fields?

omehegan added 2 commits April 8, 2026 11:32
Address review feedback: ensure ListForUser, CreateTeam, and
UpdateTeam tests also verify the new fields (GraphQLID,
DefaultMemberRole, MembersCanCreatePipelines, etc.) are correctly
deserialized in API responses.
The API accepts members_can_create_suites, members_can_create_registries,
members_can_destroy_registries, and members_can_destroy_packages on
create and update, but they were missing from CreateTeam. Without them,
update operations would silently reset these permissions to false.
@omehegan omehegan requested a review from mcncl April 8, 2026 23:14
@mcncl mcncl merged commit 47eafe1 into main Apr 8, 2026
1 check passed
@mcncl mcncl deleted the add-missing-team-fields branch April 8, 2026 23:27
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.

2 participants