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 mongoize update all array operators 8.1 #5824

Conversation

JohnMaguir
Copy link
Contributor

@@ -198,7 +198,7 @@ def to_criteria
def value_for(operator, klass, key, value)
case operator
when "$rename" then value.to_s
when "$addToSet", "$push" then value.mongoize
when "$addToSet", "$push", '$pull', '$pop' then value.mongoize
Copy link
Contributor

Choose a reason for hiding this comment

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

$pullAll?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @johnnyshields. I looked close at this exact question when I was evaluating these PRs, and it's not necessary to specify $pullAll here. In the case of $push, $pull, etc, the value of the operation may different than the declared value of the field itself (because the field is array-valued), and so must be handled specially. For $pullAll, the value is an array, for an array-valued field, and so it is handled fine by the default case.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jamis any chances to see this changes in 8.1-stable and next release 8.1.x?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the nudge, @dem. I'm not sure how this fell through the cracks, but I'll get it merged today.

@jamis jamis merged commit feaa47d into mongodb:8.1-stable Aug 1, 2024
30 of 67 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.

4 participants