Skip to content

Conversation

@vkarpov15
Copy link
Collaborator

Summary

Tests and cleanup from #15725

Examples

Lex-Ashu and others added 5 commits November 7, 2025 00:05
Fixes #15724

The id() method now properly casts the id parameter based on the schema's
_id type instead of always attempting to cast to ObjectId. This fixes
issues when using custom _id types like String, Number, or Object.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…asting

Fix: cast id parameter based on schema _id type in DocumentArray.id()
@vkarpov15 vkarpov15 added this to the 8.20 milestone Nov 10, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the MongooseDocumentArray#id() method to support custom schema types for the _id field, addressing issue gh-15725. Previously, the method only attempted to cast the provided ID to an ObjectId, which failed for custom _id types like Number or String.

Key Changes:

  • Replaced hardcoded ObjectId casting with schema-based type casting that automatically detects and uses the subdocument's _id schema type
  • Added fallback logic to maintain backwards compatibility when schema information is unavailable
  • Removed the castObjectId import as it's no longer needed for the generic implementation

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/types/documentArray/methods/index.js Refactored the id() method to use schema-based casting instead of hardcoded ObjectId casting, supporting custom _id types while maintaining backwards compatibility
test/types.documentarray.test.js Updated existing test to pass required parameters to MongooseDocumentArray constructor and added new test case for custom Number _id type

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


const parentDoc = { $__: true, $__schema: new Schema({ subdocs: [schema] }) };

const a = new MongooseDocumentArray([sub1], 'test', parentDoc);
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

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

The path name mismatch between the schema definition and array constructor could cause the test to fail. The parent schema defines a field named subdocs (line 199), but the MongooseDocumentArray is instantiated with path 'test' (line 201). This mismatch means doc.$__schema.path('test') will return undefined, preventing the schema-based casting logic from working. The path should be 'subdocs' to match the schema definition.

Suggested change
const a = new MongooseDocumentArray([sub1], 'test', parentDoc);
const a = new MongooseDocumentArray([sub1], 'subdocs', parentDoc);

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM

try {
castedId = idSchemaType.cast(id);
} catch (e) {
castedId = null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldnt it still be null already?

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.

4 participants