Cover members + invitations in org integration suite#182
Open
sdairs wants to merge 3 commits into
Open
Conversation
sdairs
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
sdairs
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
sdairs
had a problem deploying
to
cloud-integration
GitHub Actions
Failure
sdairs
force-pushed
the
issue-152-bootstrap-org-integration
branch
2 times, most recently
from
efa7803 to
4fbc39a
Compare
Adds two phases to `integration_org_test.rs`:
- Members: list/get the secondary user via `member_get_list` +
`member_get`, capture the pre-test role, flip it via `member_update`
(Admin <-> Developer), verify via GET, then eagerly restore. The
capture is registered with `CleanupRegistry::register_member_role_restore`
before any mutating call so teardown always restores the original
role even on panic. `member_delete` is intentionally out of scope —
it would kick the test fixture.
- Invitations: create an invitation to
`alasdair.brown+clickhousectl_{run_id}@clickhouse.com` (catch-all
alias is read by a real inbox but never actioned), register with
`CleanupRegistry::register_invitation`, list/get to confirm, then
cancel (delete) before accept. Accept is UI-only and out of scope.
All steps are NonBlocking per the parent plan (#151) so one CI run
reports every broken endpoint, not just the first.
`CleanupRegistry` gains `register_member_role_restore` /
`unregister_member_role_restore`; teardown runs role restores first
(member_update PATCH) before the existing invitation/role/api-key
deletes. 404s are swallowed (mirrors the existing api-key + invitation
patterns), so an already-restored member doesn't fail the run.
Closes #153
Parent: #151
Stacked on: #173 (#152 bootstrap)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The legacy `role` enum (Admin/Developer) on MemberPatchRequest is rejected with 400 by orgs that have migrated to Custom Roles: "Organization has migrated to Custom Roles. Use 'assignedRoleIds' instead of 'role'." The test fixture org is on the new model, so the round-trip has to use assignedRoleIds. Round-trip flow: - Capture the secondary user's current assignedRoleIds (from the GET response's Member.assigned_roles[].role_id list). - List org roles, pick a target role id not currently assigned. If none exists and the user already has at least one role, target an empty set (drop all assignments). If neither is possible, log a skip — no meaningful state change is reachable. - PATCH assignedRoleIds = target, verify via GET, then restore the original set. The cleanup registry stores the original list and teardown patches it back if the test body fails mid-flight. CleanupRegistry::member_role_restores now stores `(user_id, Vec<String>)` instead of `(user_id, MemberPatchRequestRole)`. Drop the legacy `flip_member_role` / `member_role_for_restore` helpers — the new flow has no use for the deprecated enum.
The previous run showed `member_update` returning 200 with the pre-change `assignedRoles` set echoed in the response body. The API treats the update as eventually consistent — the PATCH response can reflect pre-change state and the subsequent GET reflects the new state after a short propagation window. Two changes: 1. Drop the assertion on the PATCH response body. The flip step now only requires the call to return a result, not that the response echoes the new role ids. 2. The verify-via-GET step (and the restore step) now use `poll_until` with a 30s timeout to wait for propagation, instead of asserting on a single GET. This applies the same eventual-consistency pattern used by the ClickHouse settings round-trip in integration_test.rs.
sdairs
force-pushed
the
issue-153-members-invitations
branch
from
abe4a18 to
254dcd4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the members + invitations phases to the live org integration suite that #152 bootstrapped. Both areas were previously only mock-tested — this closes the live-coverage gap for
member_get_list,member_get,member_updateand the full invitation CRUD (invitation_create,invitation_get_list,invitation_get,invitation_delete).member_get_list, fetch them viamember_get, capture their pre-test role and register a restore task with the cleanup registry, then flip Admin <-> Developer viamember_update, verify via GET, and eagerly restore. The registry safety net runs even on panic.member_deleteis intentionally out of scope (would kick the fixture).alasdair.brown+clickhousectl_{run_id}@clickhouse.com(Gmail catch-all alias — real inbox, no human reads it), register withCleanupRegistry::register_invitation, exerciseinvitation_get_list+invitation_get, then cancel viainvitation_deletebefore the recipient could accept. Accept is UI-only and out of scope.All steps are
NonBlockingper the parent strategy in #151 so one CI run reports every broken endpoint rather than tripping on the first.CleanupRegistrygainsregister_member_role_restore/unregister_member_role_restore. Teardown PATCHes the captured original role back first, before the existing invitation/role/api-key deletes. 404s are swallowed (mirrors the api-key + invitation pattern).Closes #153
Parent: #151
Stacked PR
This PR is stacked on #173 (the #152 bootstrap). It is opened against
issue-152-bootstrap-org-integrationso GitHub shows the stacked diff. GitHub will auto-retarget tomainonce #173 merges — no action required here.Verification
cargo build -p clickhouse-cloud-apicleancargo clippy -p clickhouse-cloud-api --tests -- -D warningsclean on the changed files (integration_org_test,integration/support). Pre-existing clippy errors inspec_coverage_test.rs(6x collapsible-if) andintegration_test.rs(1x too-many-arguments) are also present onmain, confirmed via stash + checkout.cargo test -p clickhouse-cloud-api— all suites green;integration_org_test::cloud_org_lifecycleregisters as a single ignored lifecycle test as expected.--ignoredrun via cloud-integration workflow — runs as part of CI on this PR.Test plan
cleanup).🤖 Generated with Claude Code