Skip to content

Document writing team priority levels on the Preview update endpoint - #626

Merged
ryadhtaher-coder merged 2 commits into
mainfrom
ryadhtaher/teams-api-priority-levels
Aug 20, 2026
Merged

Document writing team priority levels on the Preview update endpoint#626
ryadhtaher-coder merged 2 commits into
mainfrom
ryadhtaher/teams-api-priority-levels

Conversation

@ryadhtaher-coder

Copy link
Copy Markdown
Contributor

Why?

The Preview spec for the team membership update endpoint said the priority levels could not be set. They can now, so a caller reading the spec would be told the wrong thing.

How?

The request schema gains an optional priority-levels field of the same shape the team response returns, and the description covers which teammates a payload has to name, what happens to one it leaves out, and when a secondary member is refused.

Generated with Claude Code

The update_team_members_request schema listed admin_ids alone and said
the priority levels could not be set. They can now, so the request gains
an optional admin_priority_level of the same shape the team response
returns, and the description covers the rules a caller needs: which
teammates a payload has to name, what happens to one it leaves out, and
when a secondary member is refused.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ryadhtaher-coder
ryadhtaher-coder requested review from a team and anubhav-intercom August 18, 2026 21:33
@zilleeizad-inter

Copy link
Copy Markdown
Contributor

Reviewed alongside intercom#560507.

The description holds up on the parts most at risk of being docs-only. I verified against the monolith that the "only a team that balances assignment can have secondary members, so naming one on any other team returns a 400" claim is genuinely enforced backend-side (app/commands/ember/team_memberships/team_membership_by_team_command.rb:29-37), and that it checks both the add and the update paths. Reusing the response admin_priority_level schema for the request is also fine: its nullable: true is honoured, because Mutations discards an explicit nil on an optional filter rather than rejecting it.

One clause to qualify:

An id belonging to neither the team nor admin_ids is a mistake and returns a 400, as does naming the same id at both levels.

The second half reads as unconditional, but the implementation deliberately permits naming the same id at both levels when the team genuinely holds that teammate on rows at both — that shape round-trips a read rather than failing, and there is a spec locking it in ("round-trips a team whose duplicate rows sit at both levels, repairing the surplus"). So a caller reading the spec would expect a 400 in a case that succeeds.

Suggest dropping the clause or qualifying it — e.g. "…as does naming the same id at both levels, unless the team really does hold that teammate at both."

~ Automated via Claude

@anubhav-intercom anubhav-intercom left a comment

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.

Approve. Two comments, neither blocking.

Shape, optionality and version placement all check out against Api::V3::TeamMemberships::Update — the field is a duck with nils: false, and descriptions/0/ is right for a Preview-only addition.

@zilleeizad-inter already flagged the duplicate-level clause, so I'm not repeating it.

~ Automated via Claude

example:
- 493881
- 493882
admin_priority_level:

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.

Give admin_priority_level a request-scoped example — the $ref imports response-side ones that generate a body this PR's own rule rejects.

The shared schema's secondary_admin_ids.example: [814865] isn't in the operation's admin_ids example [493881, 493882]. Postman synthesizes request bodies from the resolved schema, so the public Preview collection ships an example that 400s on "an id belonging to neither the team nor admin_ids".

Comment thread descriptions/0/api.intercom.io.yaml Outdated
members. Priority levels cannot be set here, so `admin_priority_level` follows
from `admin_ids`: it is accepted when sent back unchanged and returns a 400 when
the request tries to change it.
On a team that balances assignment across its members, `admin_priority_level`

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.

Say what happens on a team that doesn't balance assignment — the paragraph only covers balanced ones.

There's no distribution_method gate on the primary path in update.rb: primary levels are accepted on any team, while secondary returns a 400. A caller can't even classify their team from this spec — distribution_method appears nowhere in descriptions/0/.

- Qualify the duplicate-level clause: naming a teammate at both levels is
  refused unless the team really holds them at both, which a read reports and
  sending it back settles at a single level.
- Give the request its own priority-level schema so the generated example is a
  body a caller can send, rather than one built from the response-side ids.
- Say what a team that does not balance assignment does: secondary members are
  refused, primary ones are accepted but neither reported back nor used.
- Back-port distribution_method and assignment_limit from 2.16, which Preview
  was missing even though the API returns them, so a caller can tell which kind
  of team they have.
- Correct the read-only 400 example, which still named admin_ids alone, and add
  the two priority-level 400s the endpoint actually returns.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ryadhtaher-coder
ryadhtaher-coder merged commit 624f317 into main Aug 20, 2026
3 checks passed
@ryadhtaher-coder
ryadhtaher-coder deleted the ryadhtaher/teams-api-priority-levels branch August 20, 2026 12:29
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.

3 participants