Migrate the privacy settings to the generated model in both directions - #6699
Open
gpunto wants to merge 1 commit into
Open
Migrate the privacy settings to the generated model in both directions#6699gpunto wants to merge 1 commit into
gpunto wants to merge 1 commit into
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
gpunto
marked this pull request as ready for review
September 14, 2026 09:43
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (9)
💤 Files with no reviewable changes (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. WalkthroughChangesPrivacy settings model migration
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~15 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is identified in the privacy-settings model migration. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. I’m a rabbit with a tidy chart Comment |
gpunto
force-pushed
the
migrate/privacy-settings
branch
from
September 14, 2026 10:58
71ac0e6 to
f40eadf
Compare
|
andremion
approved these changes
Sep 14, 2026
andremion
left a comment
andremion
left a comment
Contributor
There was a problem hiding this comment.
Looks good. One optional nit inline.
| */ | ||
| internal fun DeliveryReceipts.toDto(): DeliveryReceiptsDto = DeliveryReceiptsDto( | ||
| enabled = enabled, | ||
| internal fun PrivacySettings.toDto(): PrivacySettingsResponse = PrivacySettingsResponse( |
Contributor
There was a problem hiding this comment.
Small thing: this is now identical to the private toResponse() below (line 287), same receiver, same return type, same body. They only differed before because toDto() returned PrivacySettingsDto. Any reason not to have toUserRequest() call toDto() and drop toResponse()? Happy to be told otherwise if you want the request path to keep its own name.
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
2 participants
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.
Goal
Parse and serialize the user's privacy settings with the generated
PrivacySettingsResponse, retiring thefour hand-written privacy DTOs.
Part of AND-1291
Implementation
privacy_settingson bothUpstreamUserDtoandDownstreamUserDtoat the generated model anddelete
PrivacySettings.kt, which held all four DTOs.toDtomappers into one and drop the fourtoDomainmappers, sincePrivacySettingsResponse.toDomain()already existed.Notes
Both directions move in one slice, which the earlier ones could not do. The spec declares
UserRequest.privacy_settingsandFullUserResponse.privacy_settingsas the samePrivacySettingsResponse, so no hand-written DTO has to stay behind to servetoDto().Field parity is exact: three nullable settings, each wrapping a required
enabled. Nothing changesrequiredness in either direction.
Devices are the obvious neighbour on
DownstreamUserDtoand are deliberately left alone.DeviceResponserequirescreated_atanduser_id, whichDeviceDtodoes not declare, and devicesarrive lean on the WebSocket, so that swap needs its own slice and its own probe.
Testing
constructions, not one line of JSON, so
UpstreamUserDtoAdapterTestserializing that object to the sameexpected JSON is the proof.
updateUserput all three settings on the wire through the newtoDto,and the response plus a fresh
queryUsersread them back, with the three values deliberately differingso a mapper crossing two of them would fail. A second run over
partialUpdateUsercovered thePATCHendpoint as well.
Summary by CodeRabbit
Bug Fixes
Tests