Skip to content

feat(mgmt): add PatchTenant endpoint for partial tenant updates - #841

Merged
dorsha merged 3 commits into
mainfrom
feat/tenant-patch
Sep 1, 2026
Merged

feat(mgmt): add PatchTenant endpoint for partial tenant updates#841
dorsha merged 3 commits into
mainfrom
feat/tenant-patch

Conversation

@ruvenzx

@ruvenzx ruvenzx commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Related Issues

related to https://github.com/descope/etc/issues/17551

Description

Wraps the management endpoint PATCH /v1/mgmt/tenant/patch to support partial tenant structure updates.

Must

  • Tests
  • Documentation (if applicable)

Update() is a full overwrite of TenantRequest, so callers preserving
only the fields they know about silently clear newer fields (SSO
enforcement exclusions, role inheritance) added since they last
upgraded. Patch() uses pointer fields so only explicitly-set values
are sent, and the Update() doc comment now correctly describes its
full-overwrite scope instead of "name and domains".

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shuni-bot

shuni-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot

shuni-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🐕 Suggested Reviewers

This PR adds a new PatchTenant() endpoint for partial tenant updates, mirroring the existing User().Patch() pattern. The assignment strategy prioritizes: (1) broad codebase expertise for architectural consistency (dorsha as primary), (2) domain-specific tenant management knowledge (aviadl), (3) SDK integration review (omercnet), and (4) test coverage validation (LioriE). This balanced selection ensures the change is reviewed for API design correctness, implementation quality, test adequacy, and broader SDK integration concerns.

Reviewer Reason
dorsha Most experienced contributor (37 commits across 7 files including all key areas: README, api/client, tenant mgmt, sdk, and mocks). Essential for architectural review and ensuring consistency with existing patterns like User.Patch().
aviadl Strong domain knowledge of tenant management and SDK patterns (13 commits across 7 files). Can validate the PatchTenantRequest design, endpoint integration, and consistency with User.Patch() precedent.
omercnet Focused expertise in SDK integration layer (descope/sdk/mgmt.go) and management mocks. Important for reviewing the SDK exposure of the new Patch() method and mock implementation.
LioriE Specialist in tenant-specific implementation and testing (4 commits in descope/internal/mgmt/tenant.go and tenant_test.go). Critical for validating the core implementation logic and test coverage of the new endpoint.

Suggested by Shuni based on git history and PR context. Names are not @-mentioned to avoid notifying anyone — request a review from whoever fits best.

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐕 Shuni Review

Adds a Tenant().Patch() endpoint with a pointer-field PatchTenantRequest so partial tenant updates no longer reset unrelated fields, mirroring the existing User().Patch() pattern.

Actionable comments posted: 1

  • 1 🟡 Minor: FederatedAppIDs field on PatchTenantRequest has no corresponding field on Tenant/TenantRequest and no other reference in the codebase

Merge risk: 🟢 Low: the core patch logic mirrors the well-tested User().Patch() pattern and is covered by new unit tests; the one flagged field is an isolated, easily-verifiable addition.

See inline comments for details.


Review scope: Full review

Reviewed files (7)
  • README.md
  • descope/api/client.go
  • descope/internal/mgmt/tenant.go
  • descope/internal/mgmt/tenant_test.go
  • descope/sdk/mgmt.go
  • descope/tests/mocks/mgmt/managementmock.go
  • descope/types.go

Comment thread descope/types.go
@shuni-bot

shuni-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐕 Shuni Review

No new issues found.

1 unresolved prior finding

⚠️ Review follow-up issue: Unresolved prior review threads: PRRT_kwDOHT4RP86dtvjm.


Review scope: Incremental re-review

Reviewed new changes (5)
  • README.md
  • descope/api/client.go
  • descope/sdk/mgmt.go
  • descope/tests/mocks/mgmt/managementmock.go
  • descope/types.go
Previously reviewed and unchanged: 2 files.

FederatedAppIDs was only reachable through PatchTenantRequest, so it
had no counterpart in the read model or Create/Update, and couldn't
be verified via Load(). Wire it into Tenant and TenantRequest too so
it's symmetric with every other patchable field.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@shuni-bot

shuni-bot Bot commented Aug 31, 2026

Copy link
Copy Markdown

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot shuni-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🐕 Shuni Review

No new issues found.


Review scope: Incremental re-review

Reviewed new changes (3)
  • descope/internal/mgmt/tenant.go
  • descope/internal/mgmt/tenant_test.go
  • descope/types.go
Previously reviewed and unchanged: 4 files.

@ruvenzx ruvenzx self-assigned this Sep 1, 2026
@dorsha
dorsha merged commit 597ff5a into main Sep 1, 2026
16 checks passed
@dorsha
dorsha deleted the feat/tenant-patch branch September 1, 2026 07:56
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