Skip to content

fix: use emoji _id instead of filter key for collection data lookup #40221

Open
Naetiksoni08 wants to merge 1 commit intoRocketChat:developfrom
Naetiksoni08:fix/emoji-collection-wrong-key-lookup
Open

fix: use emoji _id instead of filter key for collection data lookup #40221
Naetiksoni08 wants to merge 1 commit intoRocketChat:developfrom
Naetiksoni08:fix/emoji-collection-wrong-key-lookup

Conversation

@Naetiksoni08
Copy link
Copy Markdown
Contributor

@Naetiksoni08 Naetiksoni08 commented Apr 19, 2026

Proposed changes (including videos or screenshots)

While exploring ComposerPopupProvider.tsx during #40167, I found another bug in the same file.

 // Before (broken)                                                                                                                                              
 .map((_id) => {                                                                                                                                               
     const data = collection[key]; // key = ':sm' (filter string, not emoji id)                                                                                  
     return { _id, data };                                                     
 })           

key is the filter string (e.g. :sm), not the emoji's actual id. So every emoji in the list was getting the data of the filter string — which mostly doesn't exist — instead of its own data.

  // After (fixed)
  .map((_id) => {
      const data = collection[_id]; // use the emoji's own id
      return { _id, data };                                                                                                                                       
  })

This fix applies to both the emoji send popup (triggered by :) and the reaction add popup (triggered by +:).

Steps to test or reproduce

  1. Open message composer and type :sm
  2. Check emoji popup — emojis were receiving wrong/undefined data from collection
  3. After fix — each emoji correctly receives its own data

Further comments

Found this bug while exploring ComposerPopupProvider.tsx during #40167. Both emoji popup configs had the same bug.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed emoji suggestion retrieval in the composer popup to ensure correct emoji items are displayed when using emoji triggers.

@Naetiksoni08 Naetiksoni08 requested a review from a team as a code owner April 19, 2026 13:05
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot Bot commented Apr 19, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 19, 2026

⚠️ No Changeset found

Latest commit: 609ff6e

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b3ceae02-663b-444a-b316-c90b194fb2da

📥 Commits

Reviewing files that changed from the base of the PR and between 24b3671 and 609ff6e.

📒 Files selected for processing (1)
  • apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
🧠 Learnings (4)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-01-17T01:51:47.764Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38219
File: packages/core-typings/src/cloud/Announcement.ts:5-6
Timestamp: 2026-01-17T01:51:47.764Z
Learning: In packages/core-typings/src/cloud/Announcement.ts, the AnnouncementSchema.createdBy field intentionally overrides IBannerSchema.createdBy (object with _id and optional username) with a string enum ['cloud', 'system'] to match existing runtime behavior. This is documented as technical debt with a FIXME comment at apps/meteor/app/cloud/server/functions/syncWorkspace/handleCommsSync.ts:53 and should not be flagged as an error until the runtime behavior is corrected.

Applied to files:

  • apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
📚 Learning: 2026-03-11T22:04:20.529Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 39545
File: apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts:59-61
Timestamp: 2026-03-11T22:04:20.529Z
Learning: In `apps/meteor/client/views/room/body/hooks/useHasNewMessages.ts`, the `msg.u._id === uid` early-return in the `streamNewMessage` handler is intentional: the "New messages" indicator is designed to notify about messages from other users only. Self-sent messages — including those sent from a different session/device — are always skipped, by design. Do not flag this as a multi-session regression.

Applied to files:

  • apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.

Applied to files:

  • apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
🔇 Additional comments (1)
apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx (1)

228-232: LGTM — the lookup now uses the iterated emoji id.

Using collection[_id] correctly pairs each emoji suggestion with its own data in both popup providers and fixes the wrong/undefined data issue described in the PR.

Also applies to: 285-289


Walkthrough

Bug fix in emoji suggestion provider: changed emoji data retrieval from using a fixed key variable to using the actual iterated key (_id) when indexing the collection, correcting data retrieval for emoji popup suggestions.

Changes

Cohort / File(s) Summary
Emoji Data Retrieval Fix
apps/meteor/client/views/room/providers/ComposerPopupProvider.tsx
Fixed emoji collection indexing logic in two emoji popup providers (: trigger and +: trigger) to use individual emoji IDs (collection[_id]) instead of a shared key variable for correct data pairing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly and specifically describes the main fix: changing from using a filter key to using the emoji's _id for collection data lookup in ComposerPopupProvider.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant