Skip to content

Conversation

@KevLehman
Copy link
Member

@KevLehman KevLehman commented Apr 25, 2025

Proposed changes (including videos or screenshots)

Issue(s)

https://rocketchat.atlassian.net/browse/CTZ-86

Steps to test or reproduce

When an app (like WhatsApp) adds custom fields to the room, it issues an update via bridge with just the necessary data (in this case, the _id and customFields props).

When the apps engine receives this, it triest to convert that into a rocketchat room object, assuming some properties are there. Since, in this case, the name was not in the original update, then it defaulted to undefined, leaving the update statement with something like:

{ _id: 'xxx', customFields: {}, name: undefined }

Previously, the driver would ignore those undefined fields, but now we're passing them into the query. This caused the room to be name: undefined in the database (apart from other fields, like the room's type!)

Then, when another update or another query to the room was made, the app wasn't able to find the room it creatd (as it lacked the type l for livechat) and it attempted to create a new one. Then, when it added the custom fields, the same would happen, but this time, it would conflict with the previous room as both would have name: undefined on the database (and name should be unique).

This would throw a Duplicated Index Key error, which the apps engine would fail to parse, causing the whole operation to fail and the gateway to reschedule the message...

Further comments

Caused by #31497
(Or at least, this one made it surface :p )

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Apr 25, 2025

Looks like this PR is ready to merge! 🎉
If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link

changeset-bot bot commented Apr 25, 2025

⚠️ No Changeset found

Latest commit: d252bf3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 2025

PR Preview Action v1.6.1

🚀 View preview at
https://RocketChat.github.io/Rocket.Chat/pr-preview/pr-35878/

Built to branch gh-pages at 2025-04-28 14:32 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@codecov
Copy link

codecov bot commented Apr 25, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (release-7.6.0@01cd736). Learn more about missing BASE report.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##             release-7.6.0   #35878   +/-   ##
================================================
  Coverage                 ?   61.09%           
================================================
  Files                    ?     3005           
  Lines                    ?    71433           
  Branches                 ?    16369           
================================================
  Hits                     ?    43643           
  Misses                   ?    24824           
  Partials                 ?     2966           
Flag Coverage Δ
e2e 57.87% <ø> (?)
unit 74.98% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@KevLehman KevLehman marked this pull request as ready for review April 25, 2025 21:02
@KevLehman KevLehman requested review from a team as code owners April 25, 2025 21:02
@KevLehman KevLehman requested a review from sampaiodiego April 28, 2025 16:20
@sampaiodiego sampaiodiego added this to the 7.6.0 milestone Apr 28, 2025
@sampaiodiego sampaiodiego added the stat: QA assured Means it has been tested and approved by a company insider label Apr 28, 2025
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Apr 28, 2025
@kodiakhq kodiakhq bot merged commit d4eef32 into release-7.6.0 Apr 28, 2025
50 checks passed
@kodiakhq kodiakhq bot deleted the fix/apps-ignore-undef branch April 28, 2025 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stat: QA assured Means it has been tested and approved by a company insider stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants