Skip to content

deprecate metadata field in user update#415

Draft
brunol95 wants to merge 3 commits into
mainfrom
bruno/deprecate-user-update-metadata-fields
Draft

deprecate metadata field in user update#415
brunol95 wants to merge 3 commits into
mainfrom
bruno/deprecate-user-update-metadata-fields

Conversation

@brunol95
Copy link
Copy Markdown

@brunol95 brunol95 commented May 19, 2026

In the next API version we introducing a breaking change. Metadata updates will not be accepted via the PATCH /v1/me endpoint. This PR updates the sdk to be backwards compatible with this change

Summary of changes

  • Sources/ClerkKit/Domains/User/UserUpdateParams.swift
    • Split User.UpdateParams.init into two overloads:
      • Primary (non-deprecated): drops the unsafeMetadata: parameter entirely.
      • Deprecated overload: requires unsafeMetadata: JSON? (no default) so it only resolves when the caller
        explicitly passes unsafeMetadata:.
    • Carries the same deprecation message as the existing computed unsafeMetadata property, so warnings now
      surface at the initializer call site too.
  • Tests/Domains/User/UserUpdateRoutingTests.swift
    • Annotated the four tests that intentionally exercise the deprecated metadata path with @available(*,
      deprecated) to suppress the new warnings:
      • onlyMetadataIssuesSingleUpdateMetadataCallWithComputedPatch
      • mixedFieldsAndMetadataIssueUpdateThenUpdateMetadataInOrder
      • identicalMetadataShortCircuitsWithoutMetadataCall
      • identicalMetadataWithOtherFieldsReturnsTheUpdateMeResponse

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3238ba81-829f-4017-ab95-1b5759fabf5f

📥 Commits

Reviewing files that changed from the base of the PR and between 63ef50a and 8f2e64c.

📒 Files selected for processing (8)
  • Sources/ClerkKit/Domains/User/User.swift
  • Sources/ClerkKit/Domains/User/UserUpdateParams.swift
  • Sources/ClerkKit/Mocks/MockServices/MockUserService.swift
  • Sources/ClerkKit/Utils/JSON+MergePatch.swift
  • Tests/Domains/User/UserServiceTests.swift
  • Tests/Domains/User/UserTests.swift
  • Tests/Utils/JSONUtilitiesTests.swift
  • Tests/Utils/URLEncodedFormEncoderTests.swift

📝 Walkthrough

Walkthrough

This PR adds a dedicated metadata update API to the Clerk iOS SDK with RFC 7396 merge-patch computation. It introduces User.UpdateMetadataParams for metadata-specific payloads and adds UserService.updateMetadata() to PATCH /v1/me/metadata. The deprecated unsafeMetadata field in User.UpdateParams is refactored to route through the new metadata API, computing deep merge patches to handle partial updates. A new JSON.mergePatch(against:) utility implements RFC 7396 semantics to generate minimal patches with null-marker-based key removal. Mocks, service tests, and user API tests validate request encoding, split-request routing for combined profile+metadata updates, and merge-patch behavior. SDK and API versions are bumped to reflect the new feature.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 15.52% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: deprecating the metadata field in the User.UpdateParams initializer and routing metadata updates through a separate API.
Description check ✅ Passed The description is related to the changeset, explaining the backwards-compatibility approach for the upcoming API change and summarizing key file modifications.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
Sources/ClerkKit/Domains/User/UserUpdateParams.swift (1)

8-99: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix CI lint blockers before merge.

Required checks are currently failing (SwiftFormat in this file and also in Sources/ClerkKit/Utils/JSON+MergePatch.swift and Tests/Utils/JSONMergePatchTests.swift), so this PR is not mergeable yet.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/ClerkKit/Domains/User/UserUpdateParams.swift` around lines 8 - 99, CI
is failing due to SwiftFormat lint violations; run the project's SwiftFormat
configuration and fix formatting in the affected files
(Sources/ClerkKit/Domains/User/UserUpdateParams.swift — symbols:
User.UpdateParams, hasAnyField, _unsafeMetadata, unsafeMetadata, CodingKeys —
plus Sources/ClerkKit/Utils/JSON+MergePatch.swift and
Tests/Utils/JSONMergePatchTests.swift), stage the formatted files, and push the
updated files so the SwiftFormat check passes; re-run the CI to confirm all lint
blockers are resolved.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Sources/ClerkKit/Domains/User/User.swift`:
- Line 251: Replace the source used to compute the merge patch from
self.unsafeMetadata to afterPatch.unsafeMetadata so deletions account for any
server-side additions; specifically, in the User update flow where you set let
current = self.unsafeMetadata ?? .object([:]) (in User.swift), change it to
derive current from afterPatch.unsafeMetadata (e.g., let current =
afterPatch.unsafeMetadata ?? .object([:])) so the generated patch includes nulls
for keys the server added and achieves true replace semantics when building the
merge patch.
- Around line 243-250: Replace the multi-statement conditional with a single
expression assignment for afterPatch and remove the redundant self and extra
blank line: change the block that declares afterPatch to a single-line
conditional assignment using rest.hasAnyField ? try await service.update(params:
rest) : self (ensuring parentheses around the try await call if needed), remove
the redundant "self." in the fallback, and delete the consecutive blank line.

---

Outside diff comments:
In `@Sources/ClerkKit/Domains/User/UserUpdateParams.swift`:
- Around line 8-99: CI is failing due to SwiftFormat lint violations; run the
project's SwiftFormat configuration and fix formatting in the affected files
(Sources/ClerkKit/Domains/User/UserUpdateParams.swift — symbols:
User.UpdateParams, hasAnyField, _unsafeMetadata, unsafeMetadata, CodingKeys —
plus Sources/ClerkKit/Utils/JSON+MergePatch.swift and
Tests/Utils/JSONMergePatchTests.swift), stage the formatted files, and push the
updated files so the SwiftFormat check passes; re-run the CI to confirm all lint
blockers are resolved.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d8551876-c9ad-4acd-bc8d-52a6a19684fe

📥 Commits

Reviewing files that changed from the base of the PR and between 5f9567a and 0949779.

📒 Files selected for processing (10)
  • Sources/ClerkKit/Domains/User/User.swift
  • Sources/ClerkKit/Domains/User/UserService.swift
  • Sources/ClerkKit/Domains/User/UserUpdateMetadataParams.swift
  • Sources/ClerkKit/Domains/User/UserUpdateParams.swift
  • Sources/ClerkKit/Mocks/MockServices/MockUserService.swift
  • Sources/ClerkKit/Utils/JSON+MergePatch.swift
  • Sources/ClerkKit/Utils/Version.swift
  • Tests/Domains/User/UserUpdateMetadataTests.swift
  • Tests/Domains/User/UserUpdateRoutingTests.swift
  • Tests/Utils/JSONMergePatchTests.swift

Comment thread Sources/ClerkKit/Domains/User/User.swift Outdated
Comment thread Sources/ClerkKit/Domains/User/User.swift Outdated
@brunol95 brunol95 force-pushed the bruno/deprecate-user-update-metadata-fields branch from 56a8cf0 to 63ef50a Compare May 19, 2026 20:57
@brunol95 brunol95 marked this pull request as draft May 20, 2026 15:15
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