Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: cast error when there is an elemMatch in the and clause #14171

Merged
merged 3 commits into from Dec 21, 2023

Conversation

tosaka-n
Copy link
Contributor

Summary

Examples
query:

{
  users: {
    $elemMatch: {
      $and: [
        { name: "test" },
        $or: [
          { is_deleted: true },
          { is_deleted: null } 
        ]
      ]
    }
  }
}

@@ -637,15 +637,15 @@ handle.$or = createLogicalQueryOperatorHandler('$or');
handle.$and = createLogicalQueryOperatorHandler('$and');
handle.$nor = createLogicalQueryOperatorHandler('$nor');

function createLogicalQueryOperatorHandler(op) {
return function logicalQueryOperatorHandler(val) {
function createLogicalQueryOperatorHandler(op, context) {

Choose a reason for hiding this comment

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

It doesn't seem necessary to take context as an argument in this scope.

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Code is LGTM, but can you please add a test case?

@tosaka-n tosaka-n force-pushed the featos/fix_cast_and_elemmatch branch from 130dcec to b052e65 Compare December 14, 2023 09:57
@@ -791,6 +791,38 @@ describe('model query casting', function() {
assert.ok(res);
assert.deepStrictEqual(res.map(doc => doc.arr[1].id), ['two', 'three']);
});

it('should not throw a cast error when dealing with an array of objects in combination with $elemMatch and nested $and', async function() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

nits: I couldn't think of an appropriate test name, so I'm open to suggestions.

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@vkarpov15 vkarpov15 added this to the 8.0.4 milestone Dec 21, 2023
@vkarpov15 vkarpov15 merged commit 1ec00bb into Automattic:master Dec 21, 2023
16 of 18 checks passed
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.

None yet

3 participants