-
Notifications
You must be signed in to change notification settings - Fork 13k
fix: users.update API call erases other customFields #36578
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
fix: users.update API call erases other customFields #36578
Conversation
|
Looks like this PR is ready to merge! 🎉 |
…s-other-customfields
🦋 Changeset detectedLatest commit: 81716b7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #36578 +/- ##
===========================================
- Coverage 65.69% 65.68% -0.01%
===========================================
Files 3195 3195
Lines 106739 106739
Branches 20321 20370 +49
===========================================
- Hits 70122 70114 -8
- Misses 33979 33986 +7
- Partials 2638 2639 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
test: api test update custom fields
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.
Pull Request Overview
This PR fixes a bug where the /api/v1/users.update API call was replacing the entire customFields object instead of merging only the specified properties. The fix ensures that when updating custom fields, existing values are preserved while only specified fields are updated or added.
Key changes:
- Modified the custom fields saving logic to use field-specific updates instead of replacing the entire object
- Added comprehensive test coverage for both API endpoints and UI interactions
- Updated the field-saving function to filter out undefined values and use MongoDB's dot notation for field updates
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts |
Core fix - changes from replacing entire customFields object to updating individual fields using dot notation |
apps/meteor/tests/end-to-end/api/users.ts |
API test coverage for custom fields merging behavior |
apps/meteor/tests/e2e/admin-users-custom-fields.spec.ts |
End-to-end UI test coverage for admin user custom field management |
apps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts |
Page object updates to support custom field testing |
.changeset/wild-kiwis-cover.md |
Changeset documentation for the bug fix |
apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.ts
Show resolved
Hide resolved
…s-other-customfields
…s-other-customfields
…s-other-customfields
pierre-lehnen-rc
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.
Please include an additional test ensuring that if the admin removes a custom field's value (aka change it to an empty string), it gets successfully removed from the user too.
|
/patch |
|
Pull request #36612 added to Project: "Patch 7.9.1" |
|
/backport 7.8.4 |
Co-authored-by: Jessica Schelly Souza <jessica_schelly@hotmail.com>
|
Pull request #36613 added to Project: "Patch 7.8.4" |
|
/backport 7.7.8 |
Co-authored-by: Jessica Schelly Souza <jessica_schelly@hotmail.com>
|
Pull request #36614 added to Project: "Patch 7.7.8" |
|
/backport 7.6.5 |
|
Sorry, I couldn't do that backport because of conflicts. Could you please solve them? you can do so by running the following commands: after that just run |
|
/backport 7.5.4 |
|
Sorry, I couldn't do that backport because of conflicts. Could you please solve them? you can do so by running the following commands: after that just run |
|
/backport 7.4.5 |
|
Sorry, I couldn't do that backport because of conflicts. Could you please solve them? you can do so by running the following commands: after that just run |
Co-authored-by: Jessica Schelly Souza <jessica_schelly@hotmail.com>
|
/backport 7.6.5 |
|
Pull request #36615 added to Project: "Patch 7.6.5" |
Co-authored-by: Jessica Schelly Souza <jessica_schelly@hotmail.com>
|
/backport 7.4.5 |
|
Pull request #36616 added to Project: "Patch 7.4.5" |
Co-authored-by: Jessica Schelly Souza <jessica_schelly@hotmail.com>
|
/backport 7.5.4 |
|
Pull request #36618 added to Project: "Patch 7.5.4" |
Proposed changes (including videos or screenshots)
This PR fixes a bug where the
/api/v1/users.updateAPI call was replacing the entirecustomFieldsobject instead of merging only the specified properties. The fix ensures that when updating custom fields, existing values are preserved while only specified fields are updated or added.Key changes:
Show a summary per file
apps/meteor/app/lib/server/functions/saveCustomFieldsWithoutValidation.tsapps/meteor/tests/end-to-end/api/users.tsapps/meteor/tests/e2e/admin-users-custom-fields.spec.tsapps/meteor/tests/e2e/page-objects/fragments/admin-flextab-users.ts.changeset/wild-kiwis-cover.mdIssue(s)
https://rocketchat.atlassian.net/browse/CORE-1260
Steps to test or reproduce
Further comments