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

Open
wants to merge 2 commits into
base: 8.1-stable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/mongoid/extensions/hash.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def to_criteria

# Get the value for the provided operator, klass, key and value.
#
# This is necessary for special cases like $rename, $addToSet and $push.
# This is necessary for special cases like $rename, $addToSet, $push, $pull and $pop.
#
# @param [ String ] operator The operator.
# @param [ Class ] klass The model class.
Expand All @@ -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.

else mongoize_for(operator, klass, key, value)
end
end
Expand Down
69 changes: 69 additions & 0 deletions spec/mongoid/contextual/mongo_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3746,6 +3746,75 @@
expect(new_order.reload.genres).to eq(["electronic"])
end
end

context "when operation is $pull" do
context "when pulling single element" do

before do
depeche_mode.update_attribute(:genres, ["electronic", "pop"])
new_order.update_attribute(:genres, ["electronic", "pop"])
context.update_all("$pull" => { genres: "electronic" })
end

it "updates the first matching document" do
expect(depeche_mode.reload.genres).to eq(["pop"])
end

it "updates the last matching document" do
expect(new_order.reload.genres).to eq(["pop"])
end
end

context "when pulling based on condition" do
before do
depeche_mode.update_attribute(:genres, ["electronic", "pop", "dance"])
new_order.update_attribute(:genres, ["electronic", "pop", "dance"])
context.update_all("$pull" => { genres: { '$in' => ["electronic", "pop"] } })
end

it "updates the first matching document" do
expect(depeche_mode.reload.genres).to eq(["dance"])
end

it "updates the last matching document" do
expect(new_order.reload.genres).to eq(["dance"])
end
end
end

context "when operation is $pop" do

before do
depeche_mode.update_attribute(:genres, ["pop", "electronic"])
end

it "removes first element in array" do
context.update_all("$pop" => { genres: -1 })
expect(depeche_mode.reload.genres).to eq(["electronic"])
end

it "removes last element in array" do
context.update_all("$pop" => { genres: 1 })
expect(depeche_mode.reload.genres).to eq(["pop"])
end
end

context "when operation is $pullAll" do

before do
depeche_mode.update_attribute(:genres, ["pop", "electronic", "dance", "pop" ])
new_order.update_attribute(:genres, ["electronic", "pop", "electronic", "dance"])
context.update_all("$pullAll" => { genres: ["pop", "electronic"] })
end

it "updates the first matching document" do
expect(depeche_mode.reload.genres).to eq(["dance"])
end

it "updates the last matching document" do
expect(new_order.reload.genres).to eq(["dance"])
end
end
end

context 'when using aliased field names' do
Expand Down
Loading