-
Notifications
You must be signed in to change notification settings - Fork 2.2k
fix: DeleteSocialAccounts and AddSocialAccounts of UsersService #3922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3922 +/- ##
==========================================
+ Coverage 92.44% 92.45% +0.01%
==========================================
Files 203 203
Lines 14925 14954 +29
==========================================
+ Hits 13797 13826 +29
Misses 926 926
Partials 202 202 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gmlewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Not-Dhananjay-Mishra!
Just a couple minor tweaks, please, then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear - @zyfy29
gmlewis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, @Not-Dhananjay-Mishra and @alexandear!
LGTM.
Merging.
Fixes: #3921
Added an internal struct
socialAccountsRequestand use it to send the request body instead of passingaccountURLsdirectly, which previously caused anInvalid inputerror.Since the struct is internal and the public method signature remains unchanged, this change introduces no breaking API changes.