Skip to content

Conversation

@aleksandernsilva
Copy link
Contributor

@aleksandernsilva aleksandernsilva commented Dec 9, 2025

Proposed changes (including videos or screenshots)

extracted from #37612

Issue(s)

FB-65

Steps to test or reproduce

Further comments

Summary by CodeRabbit

  • Tests
    • Improved test infrastructure stability by ensuring proper initialization of subscription objects in test fixtures.

✏️ Tip: You can customize this high-level summary in your review settings.

@dionisio-bot
Copy link
Contributor

dionisio-bot bot commented Dec 9, 2025

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

changeset-bot bot commented Dec 9, 2025

⚠️ No Changeset found

Latest commit: 872da49

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
Contributor

coderabbitai bot commented Dec 9, 2025

Walkthrough

The change modifies a test mock file to ensure FakeRoomProvider always creates and provides a subscription object in its context value, removing a conditional that previously allowed the subscription to be undefined.

Changes

Cohort / File(s) Change Summary
Test Mock Subscription Handling
apps/meteor/tests/mocks/client/FakeRoomProvider.tsx
Always create subscription object instead of conditionally returning undefined; normalizes context value shape

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested labels

stat: ready to merge

Poem

🐰 A subscription now guaranteed to be,
No more undefined uncertainties!
Mock or real, it hops along,
Our tests now skip a notch more strong. ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive The linked issue FB-65 lacks specific coding requirements. The PR's objective to remove subscription randomness cannot be validated against vague requirements. Provide detailed coding requirements in the linked issue to validate whether this subscription change fulfills the stated objectives for the invited empty state feature.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: removing randomness from FakeRoomProvider's subscription, which matches the code modification that ensures subscription is always defined.
Out of Scope Changes check ✅ Passed The PR modifies only FakeRoomProvider.tsx to ensure subscription is always defined, which appears focused on removing non-deterministic behavior from test mocks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/fake-provider-subscription

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9b3fa8d and 5056f03.

📒 Files selected for processing (1)
  • apps/meteor/tests/mocks/client/FakeRoomProvider.tsx (1 hunks)
🧰 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/tests/mocks/client/FakeRoomProvider.tsx
🧠 Learnings (5)
📓 Common learnings
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings (mapping subscription documents to room IDs), never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/tests/mocks/client/FakeRoomProvider.tsx
📚 Learning: 2025-09-25T09:59:26.461Z
Learnt from: Dnouv
Repo: RocketChat/Rocket.Chat PR: 37057
File: packages/apps-engine/src/definition/accessors/IUserRead.ts:23-27
Timestamp: 2025-09-25T09:59:26.461Z
Learning: AppUserBridge.getUserRoomIds in apps/meteor/app/apps/server/bridges/users.ts always returns an array of strings by mapping subscription documents to room IDs, never undefined, even when user has no room subscriptions.

Applied to files:

  • apps/meteor/tests/mocks/client/FakeRoomProvider.tsx
📚 Learning: 2025-11-27T17:56:26.050Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37557
File: apps/meteor/client/views/admin/ABAC/AdminABACRooms.tsx:115-116
Timestamp: 2025-11-27T17:56:26.050Z
Learning: In Rocket.Chat, the GET /v1/abac/rooms endpoint (implemented in ee/packages/abac/src/index.ts) only returns rooms where abacAttributes exists and is not an empty array (query: { abacAttributes: { $exists: true, $ne: [] } }). Therefore, in components consuming this endpoint (like AdminABACRooms.tsx), room.abacAttributes is guaranteed to be defined for all returned rooms, and optional chaining before calling array methods like .join() is sufficient without additional null coalescing.

Applied to files:

  • apps/meteor/tests/mocks/client/FakeRoomProvider.tsx
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.

Applied to files:

  • apps/meteor/tests/mocks/client/FakeRoomProvider.tsx
🧬 Code graph analysis (1)
apps/meteor/tests/mocks/client/FakeRoomProvider.tsx (1)
apps/meteor/tests/mocks/data.ts (1)
  • createFakeSubscription (63-90)
🔇 Additional comments (1)
apps/meteor/tests/mocks/client/FakeRoomProvider.tsx (1)

20-20: LGTM! Improved test determinism.

Removing the randomness and ensuring subscription is always present makes tests more predictable and reproducible, which is a best practice for test mocks.


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.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 9, 2025

📦 Docker Image Size Report

📈 Changes

Service Current Baseline Change Percent
sum of all images 1.2GiB 1.2GiB +12MiB
rocketchat 360MiB 349MiB +12MiB
omnichannel-transcript-service 132MiB 132MiB -2.0KiB
queue-worker-service 132MiB 132MiB +22B
ddp-streamer-service 126MiB 126MiB +228B
account-service 113MiB 113MiB +242B
stream-hub-service 111MiB 111MiB +89B
authorization-service 111MiB 111MiB -348B
presence-service 111MiB 111MiB +573B

📊 Historical Trend

---
config:
  theme: "dark"
  xyChart:
    width: 900
    height: 400
---
xychart
  title "Image Size Evolution by Service (Last 30 Days + This PR)"
  x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 15:01", "12/09 17:07 (PR)"]
  y-axis "Size (GB)" 0 --> 0.5
  line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
  line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
  line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13]
  line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.35]
  line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Loading

Statistics (last 16 days):

  • 📊 Average: 1.5GiB
  • ⬇️ Minimum: 1.2GiB
  • ⬆️ Maximum: 1.6GiB
  • 🎯 Current PR: 1.2GiB
ℹ️ About this report

This report compares Docker image sizes from this build against the develop baseline.

  • Tag: pr-37747
  • Baseline: develop
  • Timestamp: 2025-12-09 17:07:19 UTC
  • Historical data points: 16

Updated: Tue, 09 Dec 2025 17:07:20 GMT

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.72%. Comparing base (9b3fa8d) to head (5056f03).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #37747      +/-   ##
===========================================
- Coverage    67.80%   67.72%   -0.09%     
===========================================
  Files         3449     3449              
  Lines       113927   113927              
  Branches     20915    20915              
===========================================
- Hits         77250    77152      -98     
- Misses       34550    34653     +103     
+ Partials      2127     2122       -5     
Flag Coverage Δ
e2e 57.34% <ø> (+0.02%) ⬆️
e2e-api 42.21% <ø> (-1.02%) ⬇️

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.

@aleksandernsilva aleksandernsilva marked this pull request as ready for review December 9, 2025 21:29
@aleksandernsilva aleksandernsilva force-pushed the chore/fake-provider-subscription branch from 5056f03 to 872da49 Compare December 10, 2025 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants