fix(firestore): respect ignoreUndefinedProperties in subpipelines#8089
fix(firestore): respect ignoreUndefinedProperties in subpipelines#8089
ignoreUndefinedProperties in subpipelines#8089Conversation
If `ignoreUndefiendProperties` is set to true, a sub-pipeline with an undefined value should result in an error being thrown. This is not currently the case, since the `_validateUserData` method on `PipelineValueExpression` does not pass down the `ignoreUndefinedProperties` config. This fixes the issue by correctly passing down the config. This is verified by adding a test that uses an undefined value in a sub-pipeline. This test fails if we don't make the change to `_validateUserData`.
product-auto-label
Bot
added
the
api: firestore
dlarocque
changed the title
ignoreUndefinedProperties for pplignoreUndefinedProperties in subpipelines
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the PipelineValueExpression class to correctly propagate the ignoreUndefinedProperties flag during user data validation and adds a system test to verify this behavior. Feedback was provided to improve the test's isolation by using a local Firestore instance instead of reassigning a shared variable and to ensure all potential validation errors are captured within the try-catch block.
diff --git c/handwritten/firestore/dev/system-test/pipeline.ts i/handwritten/firestore/dev/system-test/pipeline.ts index 38a7e06..f3da0b4 100644 --- c/handwritten/firestore/dev/system-test/pipeline.ts +++ i/handwritten/firestore/dev/system-test/pipeline.ts @@ -6217,15 +6217,15 @@ describe.skipClassic('Pipeline class', () => { }); it('PipelineValueExpression respects ignoreUndefinedProperties', async () => { - firestore = getTestDb({ignoreUndefinedProperties: false}); + const firestore_2 = getTestDb({ignoreUndefinedProperties: false}); - const subWithUndefined = firestore + const subWithUndefined = firestore_2 .pipeline() .collection('test') .where(equal(field('title'), {title: undefined})); try { - const results = await firestore + await firestore_2 .pipeline() .collection(randomCol.path) .addFields(subWithUndefined.toArrayExpression().as('reviewsData'))
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates the PipelineValueExpression class to correctly utilize the ignoreUndefinedProperties flag during user data validation, replacing a hardcoded string with the actual boolean parameter. Additionally, a new system test has been added to verify that the pipeline correctly respects this configuration by throwing an error when undefined values are encountered. I have no feedback to provide.
1 participant
If
ignoreUndefiendPropertiesis set to true, a sub-pipeline with an undefined value should result in an error being thrown. This is not currently the case, since the_validateUserDatamethod onPipelineValueExpressiondoes not pass down theignoreUndefinedPropertiesconfig.This fixes the issue by correctly passing down the config. This is verified by adding a test that uses an undefined value in a sub-pipeline. This test fails if we don't make the change to
_validateUserData.