fix: schema definition for livechat/custom-fields.save #38376
+362
−5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: e80eb4f The changes in this PR will be included in the next version bump. This PR includes changesets to release 40 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 |
Contributor
|
Looks like this PR is ready to merge! 🎉 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR fixes validation issues in the livechat/custom-fields.save endpoint by making the customFieldId parameter nullable across the type system and REST schema, introducing stricter field name validation, and adding comprehensive end-to-end tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ 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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38376 +/- ##
========================================
Coverage 70.35% 70.36%
========================================
Files 3162 3162
Lines 110705 110705
Branches 19923 19924 +1
========================================
+ Hits 77892 77901 +9
+ Misses 30788 30774 -14
- Partials 2025 2030 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Contributor
📦 Docker Image Size Report➡️ Changes
📊 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/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 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "02/10 20:46 (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, 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.00]
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, 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.00]
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, 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.00]
line "omnichannel-transcript-service" [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, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
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, 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.00]
line "queue-worker-service" [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, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [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.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Tue, 10 Feb 2026 20:46:48 GMT |
KevLehman
reviewed
Jan 27, 2026
d-gubert
force-pushed
the
chore/improve-livechat-customfield-endpoint
branch
from
e8e1db7 to
08e89d1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/models/src/models/LivechatCustomField.ts (1)
52-70: Use explicit null/undefined check for_idparameter.With
_id: string | null, the truthy checkif (_id)treats empty string as "create," which could cause unintended inserts if an invalid ID slips through. Useif (_id != null)instead.Suggested fix
- if (_id) { + if (_id != null) { await this.updateOne({ _id }, { $set: record }); } else { record._id = field; await this.insertOne(record); }
🧹 Nitpick comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
4502-4504: Align type optionality with schema.The schema doesn’t require
customFieldId, but the compiled type does. Consider making it optional to keep the type guard sound.✅ Suggested fix
-export const isPOSTLivechatSaveCustomFieldsParams = ajv.compile<{ - customFieldId: string | null; +export const isPOSTLivechatSaveCustomFieldsParams = ajv.compile<{ + customFieldId?: string | null; customFieldData: Omit<ILivechatCustomField, '_id' | '_updatedAt'> & { field: string }; }>(POSTLivechatSaveCustomFieldsSchema);
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4446-4449: The JSON Schema for the property customFieldId
currently only allows a string but the TypeScript type permits string | null;
update the schema for customFieldId (the property in the omnichannel schema) to
accept null as well — for example by adding "nullable: true" to the
customFieldId schema object (or change type to ["string","null"]) while keeping
the pattern validation for non-null strings.
Member
Author
|
@KevLehman and I found out there is bug in the endpoint validation, as it was incorrectly migrated from its meteor method counterpart The method implementation validated the Moved back to draft so I can take a better look at this |
d-gubert
force-pushed
the
chore/improve-livechat-customfield-endpoint
branch
from
cf1c382 to
b3502ef
Compare
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/rest-typings/src/v1/omnichannel.ts">
<violation number="1" location="packages/rest-typings/src/v1/omnichannel.ts:4454">
P2: The `customFieldData` schema is missing a `required` array, effectively making all properties (including `field`) optional in validation, despite the TypeScript type requiring them.
The TypeScript type expects `{ field: string }` and `Omit<ILivechatCustomField...>` (which requires `label`, `scope`, `visibility`). However, the schema allows an empty `customFieldData` object or one missing these critical fields.
Since this PR adds validation to `field`, it should also ensure `field` (and other mandatory properties like `label`, `scope`, `visibility`) are required in the schema.</violation>
<violation number="2" location="packages/rest-typings/src/v1/omnichannel.ts:4501">
P0: The schema for `customFieldId` is missing `nullable: true`, which will cause validation failures when `null` is passed.
The TypeScript type was updated to `string | null`, and the PR description states "custom field IDs may be null", but the AJV schema `type: 'string'` rejects `null` values by default.
To fix this, add `nullable: true` to the `customFieldId` property in `POSTLivechatSaveCustomFieldsSchema`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
d-gubert
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/rest-typings/src/v1/omnichannel.ts`:
- Around line 4452-4456: The schema for customFieldData.field currently allows
null via "nullable: true" but the TypeScript type and runtime expectation
require a non-null string; update the JSON schema for customFieldData.field to
disallow null (remove or set nullable to false) and ensure the field is marked
required in the surrounding schema so validation enforces a string value; target
the customFieldData.field schema entry in the omnichannel types file and adjust
the schema properties accordingly.
a6f5358 to
1c92a3a
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/rest-typings/src/v1/omnichannel.ts (1)
4443-4500:⚠️ Potential issue | 🟠 MajorMissing top-level
requiredarray in schema.The schema lacks a
requiredarray at the root level, making bothcustomFieldIdandcustomFieldDataoptional (can be omitted entirely). However, the TypeScript type at line 4503-4504 declarescustomFieldDataas non-optional. This mismatch allows empty payloads{}to pass AJV validation but likely fail at runtime.🐛 Proposed fix to add required constraint
required: ['field', 'label', 'scope', 'visibility'], }, }, + required: ['customFieldData'], additionalProperties: false, };
KevLehman
previously approved these changes
Feb 5, 2026
KevLehman
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Member
|
Pls create the Jira task :) |
d-gubert
force-pushed
the
chore/improve-livechat-customfield-endpoint
branch
from
1c92a3a to
9fe7257
Compare
dionisio-bot
bot
removed
the
stat: ready to merge
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts`:
- Around line 176-207: Remove the inline comments inside the test case "should
fail when trying to create a custom field with a field name that already
exists": delete the lines containing "// Create the first custom field" and "//
Try to create another with the same field name" and keep the surrounding code
as-is (references: fieldName, customFieldId,
request.post(api('livechat/custom-fields.save'))). The test name already
describes intent, so no replacement comments are needed—just remove the comment
lines to comply with the "no inline comments in tests" guideline.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/silver-clocks-help.mdapps/meteor/tests/data/livechat/custom-fields.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.tsapps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tspackages/model-typings/src/models/ILivechatCustomFieldModel.tspackages/models/src/models/LivechatCustomField.tspackages/rest-typings/src/v1/omnichannel.ts
🧰 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/model-typings/src/models/ILivechatCustomFieldModel.tsapps/meteor/tests/data/livechat/custom-fields.tsapps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.tspackages/rest-typings/src/v1/omnichannel.tspackages/models/src/models/LivechatCustomField.ts
🧠 Learnings (16)
📓 Common learnings
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.
📚 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/model-typings/src/models/ILivechatCustomFieldModel.tsapps/meteor/tests/data/livechat/custom-fields.tspackages/rest-typings/src/v1/omnichannel.ts.changeset/silver-clocks-help.mdpackages/models/src/models/LivechatCustomField.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.tsapps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
📚 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/end-to-end/api/livechat/00-rooms.ts
📚 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/end-to-end/api/livechat/00-rooms.ts
📚 Learning: 2025-10-06T20:32:23.658Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37152
File: packages/apps-engine/tests/test-data/utilities.ts:557-573
Timestamp: 2025-10-06T20:32:23.658Z
Learning: In packages/apps-engine/tests/test-data/utilities.ts, the field name `isSubscripbedViaBundle` in the `IMarketplaceSubscriptionInfo` type should not be flagged as a typo, as it may match the upstream API's field name.
Applied to files:
packages/rest-typings/src/v1/omnichannel.ts
📚 Learning: 2025-11-17T14:30:36.271Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 37491
File: packages/desktop-api/src/index.ts:17-27
Timestamp: 2025-11-17T14:30:36.271Z
Learning: In the Rocket.Chat desktop API (`packages/desktop-api/src/index.ts`), the `CustomNotificationOptions` type has an optional `id` field by design. Custom notifications dispatched without an ID cannot be closed programmatically using `closeCustomNotification`, and this is intentional.
Applied to files:
packages/rest-typings/src/v1/omnichannel.ts
🧬 Code graph analysis (1)
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts (2)
apps/meteor/tests/data/livechat/custom-fields.ts (2)
deleteCustomField(35-49)createCustomField(8-33)packages/core-services/src/index.ts (1)
api(55-55)
🪛 Biome (2.3.13)
apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts
[error] 20-23: Duplicate before hook found.
Remove this duplicate hook or consolidate the logic into a single hook.
(lint/suspicious/noDuplicateTestHooks)
⏰ 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: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (10)
packages/model-typings/src/models/ILivechatCustomFieldModel.ts (1)
28-35: Nullable_idin the model signature looks right.This aligns the model typings with the insert-vs-update behavior.
packages/models/src/models/LivechatCustomField.ts (1)
52-75: Signature update matches existing insert/update branching.The null-aware
_idaligns with the existing truthy check.packages/rest-typings/src/v1/omnichannel.ts (1)
4443-4505: Schema/type alignment looks solid.Nullable
customFieldId, stricterfieldvalidation, and required inner fields align the contract with the intended payload..changeset/silver-clocks-help.md (1)
1-8: Changeset content matches the fix scope.Patch bumps and summary are aligned with the validation change.
apps/meteor/tests/data/livechat/custom-fields.ts (1)
6-33: Return type tightening is consistent with the API response.apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts (1)
3002-3051: Nice coverage for invalid room/visitor IDs and malformed livechatData.apps/meteor/tests/end-to-end/api/livechat/custom-fields-save.ts (4)
38-75: Auth/authorization coverage looks solid.
85-174: Create scenarios cover minimal, room-scope, and full payloads well.
218-295: Update scenarios validate both failure and success paths.
18-23: This review comment is based on an incorrect assumption. Biome'snoDuplicateTestHooksrule targets Jest/Vitest hook names (beforeAll,afterAll,beforeEach,afterEach), not Mocha'sbefore/afterhooks. Since this file uses Mocha, the duplicate hooks do not violate any lint rule and consolidation is unnecessary. The same pattern is used throughout the codebase without issues.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
KevLehman
added
the
stat: ready to merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Livechat portion extracted from #38227
Issue(s)
CORE-1806
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Bug Fixes
Tests