Skip to content

fix(models): call setUpdatedAt for all docs in insertMany#39718

Merged
ggazzo merged 6 commits intodevelopfrom
fix/insert-many-updated-at
Mar 18, 2026
Merged

fix(models): call setUpdatedAt for all docs in insertMany#39718
ggazzo merged 6 commits intodevelopfrom
fix/insert-many-updated-at

Conversation

@ggazzo
Copy link
Copy Markdown
Member

@ggazzo ggazzo commented Mar 18, 2026

insertMany() only called setUpdatedAt() when a document already had a valid _id. Documents without _id (the common case) got an _id generated but skipped setUpdatedAt(), leaving them without _updatedAt in the database. This caused API response validation failures for endpoints using $ref schemas that require _updatedAt (e.g. subscriptions.getOne).

Align insertMany() with insertOne(), which already calls setUpdatedAt() unconditionally.

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Task: ARCH-2061

Summary by CodeRabbit

  • Bug Fixes

    • Batch inserts now apply timestamps to documents that receive generated IDs.
    • updatedAt is now reliably set when creating subscriptions.
  • Chores

    • Added a changeset documenting patch releases for two packages.

insertMany() only called setUpdatedAt() when a document already had a
valid _id. Documents without _id (the common case) got an _id generated
but skipped setUpdatedAt(), leaving them without _updatedAt in the
database. This caused API response validation failures for endpoints
using $ref schemas that require _updatedAt (e.g. subscriptions.getOne).

Align insertMany() with insertOne(), which already calls setUpdatedAt()
unconditionally.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot bot commented Mar 18, 2026

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

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 18, 2026

🦋 Changeset detected

Latest commit: 0000e87

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 41 packages
Name Type
@rocket.chat/meteor Patch
@rocket.chat/models Patch
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/instance-status Patch
@rocket.chat/omni-core Patch
@rocket.chat/server-fetch Patch
@rocket.chat/ui-client Patch
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/abac Patch
@rocket.chat/federation-matrix Patch
@rocket.chat/media-calls Patch
@rocket.chat/omni-core-ee Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/network-broker Patch
@rocket.chat/core-typings Patch
@rocket.chat/rest-typings Patch
@rocket.chat/uikit-playground Patch
@rocket.chat/api-client Patch
@rocket.chat/apps Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Patch
@rocket.chat/gazzodown Patch
@rocket.chat/http-router Patch
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-avatar Patch
@rocket.chat/ui-contexts Patch
@rocket.chat/ui-voip Patch
@rocket.chat/web-ui-registration Patch
@rocket.chat/license Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/mock-providers Patch
@rocket.chat/ui-video-conf Patch

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 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: b270e7ff-88bb-47eb-9f82-4525050cbdef

📥 Commits

Reviewing files that changed from the base of the PR and between 1ea00bb and 0000e87.

📒 Files selected for processing (1)
  • packages/models/src/models/BaseRaw.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build
🧰 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:

  • packages/models/src/models/BaseRaw.ts
🧠 Learnings (12)
📓 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:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-15T14:31:28.969Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:710-757
Timestamp: 2026-03-15T14:31:28.969Z
Learning: In RocketChat/Rocket.Chat, the `UserCreateParamsPOST` type in `apps/meteor/app/api/server/v1/users.ts` (migrated from `packages/rest-typings/src/v1/users/UserCreateParamsPOST.ts`) intentionally has `fields: string` (non-optional) and `settings?: IUserSettings` without a corresponding AJV schema entry. This is a pre-existing divergence carried over verbatim from the original rest-typings source (PR `#39647`). Do not flag this type/schema misalignment during the OpenAPI migration review — it is tracked as a separate follow-up fix.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-11T16:46:55.955Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39535
File: apps/meteor/app/apps/server/bridges/livechat.ts:249-249
Timestamp: 2026-03-11T16:46:55.955Z
Learning: In `apps/meteor/app/apps/server/bridges/livechat.ts`, `createVisitor()` intentionally does not propagate `externalIds` to `registerData`. This is by design: the method is deprecated (JSDoc: `deprecated Use createAndReturnVisitor instead. Note: This method does not support externalIds.`) to push callers toward `createAndReturnVisitor()`, which does support `externalIds`. Do not flag the missing `externalIds` field in `createVisitor()` as a bug.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-15T14:31:23.493Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39647
File: apps/meteor/app/api/server/v1/users.ts:891-899
Timestamp: 2026-03-15T14:31:23.493Z
Learning: In RocketChat/Rocket.Chat, `IUser.inactiveReason` in `packages/core-typings/src/IUser.ts` is typed as `'deactivated' | 'pending_approval' | 'idle_too_long'` (optional, no `null`), but the database stores `null` for newly created users. The Typia-generated `$ref: '#/components/schemas/IUser'` schema therefore correctly rejects `null` for `inactiveReason`. This causes the test "should create a new user with default roles" to fail when response validation is active (TEST_MODE). The fix is to add `| null` to `inactiveReason` in core-typings and rebuild Typia schemas in a separate PR. Do not flag this test failure as a bug introduced by the users.create OpenAPI migration (PR `#39647`). Do not suggest inlining a custom schema to work around it, as migration rules require using `$ref` when a Typia schema exists.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-09T21:20:12.687Z
Learnt from: pierre-lehnen-rc
Repo: RocketChat/Rocket.Chat PR: 39386
File: apps/meteor/server/services/push/tokenManagement/findDocumentToUpdate.ts:12-15
Timestamp: 2026-03-09T21:20:12.687Z
Learning: In `apps/meteor/server/services/push/tokenManagement/findDocumentToUpdate.ts`, the early return `if (data.voipToken) return null` (Lines 13-15) is intentionally correct. VoIP token updates always include an `_id`, so they are handled by the `_id` lookup block above (Lines 5-9) and never reach this guard. The guard is only a safety net for edge cases where `_id` is absent or no document was found, preventing an incorrect `token + appName` fallback match for VoIP-only payloads.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 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:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-11T18:15:53.272Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39425
File: apps/meteor/app/api/server/v1/rooms.ts:294-296
Timestamp: 2026-03-11T18:15:53.272Z
Learning: In Rocket.Chat's `rooms.mediaConfirm/:rid/:fileId` endpoint (apps/meteor/app/api/server/v1/rooms.ts), updating `file.name` from `bodyParams.fileName` without updating `file.path` is intentionally safe. The file path is keyed by `_id` (not by filename), so the stored path remains valid regardless of a rename. `file.name` only affects the display name in the message attachment; do not flag this as a path-divergence issue.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-16T22:56:54.500Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 39677
File: packages/models/src/helpers/omnichannel/agentStatus.ts:10-29
Timestamp: 2026-03-16T22:56:54.500Z
Learning: In `packages/models/src/helpers/omnichannel/agentStatus.ts` (PR `#39677`), the `queryStatusAgentOnline` function intentionally omits the `$or` offline-status guard for non-bot agents when `isLivechatEnabledWhenAgentIdle === true`. This is by design: the setting `Livechat_enabled_when_agent_idle` (`accept_chats_when_agent_idle`) means agents should receive chats even when idle/offline, so the offline filter must be removed in that path. Bots are always status-agnostic and are always included regardless of their online/offline status. Do not flag this as a bug.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-03-16T11:57:17.987Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 39657
File: apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts:43-45
Timestamp: 2026-03-16T11:57:17.987Z
Learning: In `apps/meteor/tests/end-to-end/apps/app-resolve-visitor.ts`, `externalIds` lookups are namespaced per `app.id`. Because `cleanupApps()` is called before each test run and a fresh app is installed (giving a new unique `app.id`), fixed `userId` strings like `'nonexistent-id'` in null-path tests are safe and cannot collide with stale visitor records from previous runs. Do not flag these as needing unique/random values.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
🔇 Additional comments (1)
packages/models/src/models/BaseRaw.ts (1)

273-281: insertMany now correctly sets _updatedAt for every document

Good fix: _id normalization is preserved, and this.setUpdatedAt(doc) now runs for all inserted docs, matching the PR objective and preventing missing _updatedAt records.


Walkthrough

The insertMany implementation in packages/models/src/models/BaseRaw.ts now calls setUpdatedAt for every inserted document, including those that previously had no _id (and received one during insert). This ensures all batch-inserted docs receive an updatedAt timestamp.

Changes

Cohort / File(s) Summary
Bug Fix: insertMany Timestamp Handling
packages/models/src/models/BaseRaw.ts
Ensure setUpdatedAt is called for documents that lacked a pre-existing _id during insertMany, so generated-id docs receive updatedAt.
Changeset
.changeset/spotty-poems-smash.md
Add changeset documenting package patches and the bug fix ensuring updatedAt is set during subscription creation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: ensuring setUpdatedAt is called for all documents in insertMany, which directly addresses the core bug being fixed.
Linked Issues check ✅ Passed The code changes directly address ARCH-2061 requirements: mutating documents to include generated ObjectIds and calling setUpdatedAt on all docs before returning, aligning insertMany with insertOne behavior.
Out of Scope Changes check ✅ Passed The changes are focused and in-scope: only the BaseRaw.ts insertMany logic was modified to fix the setUpdatedAt issue, with a corresponding changeset documenting the fix.
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.

📝 Coding Plan
  • Generate coding plan for human review comments

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.

@ggazzo ggazzo added this to the 8.3.0 milestone Mar 18, 2026
@ggazzo
Copy link
Copy Markdown
Member Author

ggazzo commented Mar 18, 2026

/jira ARCH-2021

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.97%. Comparing base (2e6441e) to head (0000e87).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #39718      +/-   ##
===========================================
- Coverage    70.98%   70.97%   -0.01%     
===========================================
  Files         3209     3209              
  Lines       113896   113896              
  Branches     20677    20660      -17     
===========================================
- Hits         80844    80835       -9     
- Misses       30997    31003       +6     
- Partials      2055     2058       +3     
Flag Coverage Δ
unit 71.61% <ø> (-0.01%) ⬇️

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.

@ggazzo ggazzo marked this pull request as ready for review March 18, 2026 18:18
@ggazzo ggazzo requested a review from a team as a code owner March 18, 2026 18:18
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/models/src/models/BaseRaw.ts">

<violation number="1" location="packages/models/src/models/BaseRaw.ts:277">
P2: This spread order preserves a non-string `_id` instead of replacing it with the generated string ID.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

ggazzo and others added 2 commits March 18, 2026 15:22
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/models/src/models/BaseRaw.ts`:
- Around line 275-279: The guard generating a string ObjectId can be defeated
because the spread puts the original doc after the generated _id; change the
assignment so the generated string _id wins (e.g., place the generated _id after
the spread) in both places where this pattern appears in BaseRaw.ts (the block
that currently calls this.setUpdatedAt(doc) and the similar logic inside
insertOne). Update the object construction so the new _id overrides any existing
non-string _id, keep calling setUpdatedAt(doc) as before, and apply the same fix
in insertOne.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f0bf0f31-5b45-4617-9c88-2926707fb9fd

📥 Commits

Reviewing files that changed from the base of the PR and between 2e6441e and 242fd59.

📒 Files selected for processing (1)
  • packages/models/src/models/BaseRaw.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: 🔎 Code Check / Code Lint
  • GitHub Check: 📦 Meteor Build (coverage)
🧰 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:

  • packages/models/src/models/BaseRaw.ts
🧠 Learnings (2)
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • packages/models/src/models/BaseRaw.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • packages/models/src/models/BaseRaw.ts

ggazzo and others added 2 commits March 18, 2026 15:24
Fixes an issue with updatedAt not being set during subscription creation.
sampaiodiego
sampaiodiego previously approved these changes Mar 18, 2026
ggazzo added a commit that referenced this pull request Mar 18, 2026
…ORE MERGE

Cherry-picked to test if the insertMany setUpdatedAt fix resolves
subscriptions.getOne validation failures.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@ggazzo ggazzo added the stat: QA assured Means it has been tested and approved by a company insider label Mar 18, 2026
@dionisio-bot dionisio-bot bot added the stat: ready to merge PR tested and approved waiting for merge label Mar 18, 2026
@ggazzo ggazzo merged commit eae3fb3 into develop Mar 18, 2026
39 of 41 checks passed
@ggazzo ggazzo deleted the fix/insert-many-updated-at branch March 18, 2026 18:57
ggazzo added a commit that referenced this pull request Mar 25, 2026
ggazzo added a commit that referenced this pull request Mar 25, 2026
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 type: bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants