Skip to content

fix(mgmt): add failoncontrol support for update email and phone Wait, l - #360

Merged
slavikm merged 1 commit into
mainfrom
shuni/17716-user-request-shuni-fix-it-in-i0gn65bo5d13kcnytiai4
Aug 13, 2026
Merged

fix(mgmt): add failoncontrol support for update email and phone Wait, l#360
slavikm merged 1 commit into
mainfrom
shuni/17716-user-request-shuni-fix-it-in-i0gn65bo5d13kcnytiai4

Conversation

@aviadl

@aviadl aviadl commented Aug 13, 2026

Copy link
Copy Markdown
Member

Fixes descope/etc#17716

View Shuni run

Fixed. Summary:

  • Added failOnConflict overloads to UserService.updateEmail/updatePhone (interface) and their implementations in UserServiceImpl.java, following the existing overload-delegation convention already used in this file (e.g. generateEmbeddedLink). The 3-arg methods delegate to the new 4-arg ones passing null, so existing callers are unaffected; when explicitly set, failOnConflict is added to the REST request body via addIfNotNull.
  • Added unit tests (testUpdateEmailWithFailOnConflictForSuccess, testUpdatePhoneWithFailOnConflictForSuccess) that capture the outgoing request map and assert failOnConflict is sent correctly.
  • Verified with mvn checkstyle:check, mvn test (all UserServiceImplTest unit tests pass; the 10 pre-existing testFunctional* failures are unrelated — they require live DESCOPE_PROJECT_ID/DESCOPE_MANAGEMENT_KEY credentials), and mvn package.
  • Committed as 24590fd.

Created by Shuni 🐕

The backend silently merges and deletes the conflicting user when
failOnConflict is omitted, matching the option already exposed by the
Node, Python, and Go SDKs. New overloads preserve backward compatibility
for existing 3-arg callers.

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

shuni-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🐕 Review complete — View session on Shuni Portal 🐾

@shuni-bot

shuni-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

🐕 Suggested Reviewers

Review assignment strategy: Selected 4 reviewers with comprehensive coverage of all changed files and strong domain expertise. slavikm (contributor 0) is the primary reviewer with 12 commits across all three files, making them the most qualified to ensure consistency with existing patterns. Bars92 (contributor 1) provides secondary validation across all files with 3 commits. dorsha (contributor 2) covers interface and implementation expertise. talberry (contributor 4) ensures test coverage quality and implementation consistency. This combination balances depth of expertise with breadth of file coverage.

Reviewer Reason
slavikm Primary reviewer with 12 commits across all three changed files (interface, implementation, tests). Deep expertise in UserService patterns and best positioned to validate the overload-delegation convention consistency.
Bars92 3 commits across all three files provide well-rounded perspective. Can verify interface design, implementation adherence to conventions, and test coverage.
dorsha 2 commits focused on UserService interface and implementation. Ideal for validating API contract changes and method signature consistency.
talberry 2 commits in implementation and tests. Essential for ensuring test quality and verifying the implementation follows existing patterns in UserServiceImpl.

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
Contributor

Choose a reason for hiding this comment

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

🐕 Shuni's Review

Adds failOnConflict overloads for updateEmail/updatePhone in UserService/UserServiceImpl, delegating the existing 3-arg methods to the new 4-arg ones (matches the generateEmbeddedLink overload convention already in the file).

No issues found — the delegation preserves backward compatibility, addIfNotNull is used consistently with sibling optional fields, and the new tests correctly capture and assert the request map. Good bones! 🐾


Declared coverage: FULL — 3/3 changed files reviewed.

@slavikm
slavikm merged commit 65e4209 into main Aug 13, 2026
20 checks passed
@slavikm
slavikm deleted the shuni/17716-user-request-shuni-fix-it-in-i0gn65bo5d13kcnytiai4 branch August 13, 2026 22:38
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