-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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: customFields ignored in livechat room creation #33047
fix: customFields ignored in livechat room creation #33047
Conversation
Signed-off-by: Abhinav Kumar <[email protected]>
Looks like this PR is ready to merge! 🎉 |
🦋 Changeset detectedLatest commit: 9d27458 The changes in this PR will be included in the next version bump. This PR includes changesets to release 32 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #33047 +/- ##
========================================
Coverage 59.39% 59.40%
========================================
Files 2541 2543 +2
Lines 63174 63192 +18
Branches 14219 14224 +5
========================================
+ Hits 37525 37541 +16
- Misses 22934 22935 +1
- Partials 2715 2716 +1
Flags with carried forward coverage won't be shown. Click here to find out more. |
Signed-off-by: Abhinav Kumar <[email protected]>
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.
can we provide some kind of tests here?
Signed-off-by: Abhinav Kumar <[email protected]>
apps/meteor/tests/unit/app/livechat/server/hooks/beforeNewRoom.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/ee/app/livechat-enterprise/server/hooks/beforeNewRoom.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Abhinav Kumar <[email protected]>
Signed-off-by: Abhinav Kumar <[email protected]>
apps/meteor/tests/unit/server/livechat/hooks/beforeNewRoom.spec.ts
Outdated
Show resolved
Hide resolved
apps/meteor/tests/unit/server/livechat/hooks/beforeNewRoom.spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Abhinav Kumar <[email protected]>
apps/meteor/tests/unit/server/livechat/hooks/beforeNewRoom.spec.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Abhinav Kumar <[email protected]>
@abhinavkrin just one small thing: this PR should have its own task related to 589 instead of being linked directly to it as this fix will require it's own QA (and it's not related to the issue described on 589) |
Proposed changes (including videos or screenshots)
The function
createLivechatRoom
does not take into account the customFields passed in the extraData paramRocket.Chat/apps/meteor/app/livechat/server/lib/Helper.ts
Line 60 in 05db8aa
In various places the functions is passed customFields in extraData param. This extraData is passed to
livechat.beforeRoom
callback and expects to receive extraRoomInfo. But the customField is not taken into account in this callbackRocket.Chat/apps/meteor/app/livechat/server/lib/Helper.ts
Line 85 in 05db8aa
Rocket.Chat/apps/meteor/ee/app/livechat-enterprise/server/hooks/beforeNewRoom.ts
Lines 6 to 29 in 42a176f
This PR fixes the callback to also include the customFields in the result from the callback
livechat.beforeRoom
I found this bug while investigating the issue related to on premise whatsapp app. In which, a new room was created everytime a message was sent from WA to RC. Since the custom fields was not being saved in the room data (due to the reason above), the wa app was not able to find existing room related to what whatsapp number and created a new room everytime.
I believe this bug can affect other apps as well as the rc where function is used.
Issue(s)
Steps to test or reproduce
Further comments
CORE-643