From 5ac8c453f38ce374c46dd454d425f7b38dee8f07 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Sat, 19 Jan 2013 14:38:01 +0100 Subject: [PATCH] Fix find_or_(create|initialize)_by on scoped relations. [ fix #2707 ] --- CHANGELOG.md | 4 + lib/mongoid/criterion/modifiable.rb | 43 ++++ lib/mongoid/finders.rb | 43 +--- spec/mongoid/criterion/modifiable_spec.rb | 191 ++++++++++++++++++ spec/mongoid/finders_spec.rb | 187 ----------------- .../mongoid/relations/referenced/many_spec.rb | 60 +++++- 6 files changed, 289 insertions(+), 239 deletions(-) create mode 100644 spec/mongoid/criterion/modifiable_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 90aaa2b62c..56ce5ba9c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -176,6 +176,10 @@ For instructions on upgrading to newer versions, visit * \#2714 Overriding sessions when the new session has a different database will now properly switch the database at runtime as well. +* \#2707 Calling `find_or_create_by` or `find_by_initialize_by` off a relation + with a chained criteria or scope now properly keeps the relations intact on + the new or found document. + * \#2699 Resetting a field now removes the name from the changed attributes list. (Subhash Bhushan) diff --git a/lib/mongoid/criterion/modifiable.rb b/lib/mongoid/criterion/modifiable.rb index 3a4d77cc74..0f6b2f05cf 100644 --- a/lib/mongoid/criterion/modifiable.rb +++ b/lib/mongoid/criterion/modifiable.rb @@ -55,6 +55,32 @@ def create!(attrs = {}, &block) create_document(:create!, attrs, &block) end + # Find the first +Document+ given the conditions, or creates a new document + # with the conditions that were supplied. + # + # @example Find or create the document. + # Person.find_or_create_by(:attribute => "value") + # + # @param [ Hash ] attrs The attributes to check. + # + # @return [ Document ] A matching or newly created document. + def find_or_create_by(attrs = {}, &block) + find_or(:create, attrs, &block) + end + + # Find the first +Document+ given the conditions, or initializes a new document + # with the conditions that were supplied. + # + # @example Find or initialize the document. + # Person.find_or_initialize_by(:attribute => "value") + # + # @param [ Hash ] attrs The attributes to check. + # + # @return [ Document ] A matching or newly initialized document. + def find_or_initialize_by(attrs = {}, &block) + find_or(:new, attrs, &block) + end + # Find the first +Document+, or creates a new document # with the conditions that were supplied plus attributes. # @@ -127,8 +153,25 @@ def create_document(method, attrs = nil, &block) end, &block) end + # Find the first object or create/initialize it. + # + # @api private + # + # @example Find or perform an action. + # Person.find_or(:create, :name => "Dev") + # + # @param [ Symbol ] method The method to invoke. + # @param [ Hash ] attrs The attributes to query or set. + # + # @return [ Document ] The first or new document. + def find_or(method, attrs = {}, &block) + where(attrs).first || send(method, attrs, &block) + end + # Find the first document or create/initialize it. # + # @api private + # # @example First or perform an action. # Person.first_or(:create, :name => "Dev") # diff --git a/lib/mongoid/finders.rb b/lib/mongoid/finders.rb index df13a3851a..2bd064e006 100644 --- a/lib/mongoid/finders.rb +++ b/lib/mongoid/finders.rb @@ -17,6 +17,8 @@ module Finders :each_with_index, :extras, :find_and_modify, + :find_or_create_by, + :find_or_initialize_by, :first_or_create, :first_or_create!, :first_or_initialize, @@ -81,32 +83,6 @@ def find(*args) with_default_scope.find(*args) end - # Find the first +Document+ given the conditions, or creates a new document - # with the conditions that were supplied. - # - # @example Find or create the document. - # Person.find_or_create_by(:attribute => "value") - # - # @param [ Hash ] attrs The attributes to check. - # - # @return [ Document ] A matching or newly created document. - def find_or_create_by(attrs = {}, &block) - find_or(:create, attrs, &block) - end - - # Find the first +Document+ given the conditions, or initializes a new document - # with the conditions that were supplied. - # - # @example Find or initialize the document. - # Person.find_or_initialize_by(:attribute => "value") - # - # @param [ Hash ] attrs The attributes to check. - # - # @return [ Document ] A matching or newly initialized document. - def find_or_initialize_by(attrs = {}, &block) - find_or(:new, attrs, &block) - end - # Find the first +Document+ given the conditions, or raises # Mongoid::Errors::DocumentNotFound # @@ -148,20 +124,5 @@ def first def last with_default_scope.last end - - protected - - # Find the first object or create/initialize it. - # - # @example Find or perform an action. - # Person.find_or(:create, :name => "Dev") - # - # @param [ Symbol ] method The method to invoke. - # @param [ Hash ] attrs The attributes to query or set. - # - # @return [ Document ] The first or new document. - def find_or(method, attrs = {}, &block) - where(attrs).first || send(method, attrs, &block) - end end end diff --git a/spec/mongoid/criterion/modifiable_spec.rb b/spec/mongoid/criterion/modifiable_spec.rb new file mode 100644 index 0000000000..4e818c0366 --- /dev/null +++ b/spec/mongoid/criterion/modifiable_spec.rb @@ -0,0 +1,191 @@ +require "spec_helper" + +describe Mongoid::Criterion::Modifiable do + + describe ".find_or_create_by" do + + context "when the document is found" do + + context "when providing an attribute" do + + let!(:person) do + Person.create(title: "Senior") + end + + it "returns the document" do + Person.find_or_create_by(title: "Senior").should eq(person) + end + end + + context "when providing a document" do + + context "with an owner with a BSON identity type" do + + let!(:person) do + Person.create + end + + let!(:game) do + Game.create(person: person) + end + + context "when providing the object directly" do + + let(:from_db) do + Game.find_or_create_by(person: person) + end + + it "returns the document" do + from_db.should eq(game) + end + end + + context "when providing the proxy relation" do + + let(:from_db) do + Game.find_or_create_by(person: game.person) + end + + it "returns the document" do + from_db.should eq(game) + end + end + end + + context "with an owner with an Integer identity type" do + + let!(:jar) do + Jar.create + end + + let!(:cookie) do + Cookie.create(jar: jar) + end + + let(:from_db) do + Cookie.find_or_create_by(jar: jar) + end + + it "returns the document" do + from_db.should eq(cookie) + end + end + end + end + + context "when the document is not found" do + + context "when providing a document" do + + let!(:person) do + Person.create + end + + let!(:game) do + Game.create + end + + let(:from_db) do + Game.find_or_create_by(person: person) + end + + it "returns the new document" do + from_db.person.should eq(person) + end + + it "does not return an existing false document" do + from_db.should_not eq(game) + end + end + + context "when not providing a block" do + + let!(:person) do + Person.find_or_create_by(title: "Senorita") + end + + it "creates a persisted document" do + person.should be_persisted + end + + it "sets the attributes" do + person.title.should eq("Senorita") + end + end + + context "when providing a block" do + + let!(:person) do + Person.find_or_create_by(title: "Senorita") do |person| + person.pets = true + end + end + + it "creates a persisted document" do + person.should be_persisted + end + + it "sets the attributes" do + person.title.should eq("Senorita") + end + + it "calls the block" do + person.pets.should be_true + end + end + end + end + + describe ".find_or_initialize_by" do + + context "when the document is found" do + + let!(:person) do + Person.create(title: "Senior") + end + + it "returns the document" do + Person.find_or_initialize_by(title: "Senior").should eq(person) + end + end + + context "when the document is not found" do + + context "when not providing a block" do + + let!(:person) do + Person.find_or_initialize_by(title: "Senorita") + end + + it "creates a new document" do + person.should be_new_record + end + + it "sets the attributes" do + person.title.should eq("Senorita") + end + end + + context "when providing a block" do + + let!(:person) do + Person.find_or_initialize_by(title: "Senorita") do |person| + person.pets = true + end + end + + it "creates a new document" do + person.should be_new_record + end + + it "sets the attributes" do + person.title.should eq("Senorita") + end + + it "calls the block" do + person.pets.should be_true + end + end + end + end +end diff --git a/spec/mongoid/finders_spec.rb b/spec/mongoid/finders_spec.rb index 770e32ec9e..38a6cc1cf7 100644 --- a/spec/mongoid/finders_spec.rb +++ b/spec/mongoid/finders_spec.rb @@ -39,193 +39,6 @@ end end - describe ".find_or_create_by" do - - context "when the document is found" do - - context "when providing an attribute" do - - let!(:person) do - Person.create(title: "Senior") - end - - it "returns the document" do - Person.find_or_create_by(title: "Senior").should eq(person) - end - end - - context "when providing a document" do - - context "with an owner with a BSON identity type" do - - let!(:person) do - Person.create - end - - let!(:game) do - Game.create(person: person) - end - - context "when providing the object directly" do - - let(:from_db) do - Game.find_or_create_by(person: person) - end - - it "returns the document" do - from_db.should eq(game) - end - end - - context "when providing the proxy relation" do - - let(:from_db) do - Game.find_or_create_by(person: game.person) - end - - it "returns the document" do - from_db.should eq(game) - end - end - end - - context "with an owner with an Integer identity type" do - - let!(:jar) do - Jar.create - end - - let!(:cookie) do - Cookie.create(jar: jar) - end - - let(:from_db) do - Cookie.find_or_create_by(jar: jar) - end - - it "returns the document" do - from_db.should eq(cookie) - end - end - end - end - - context "when the document is not found" do - - context "when providing a document" do - - let!(:person) do - Person.create - end - - let!(:game) do - Game.create - end - - let(:from_db) do - Game.find_or_create_by(person: person) - end - - it "returns the new document" do - from_db.person.should eq(person) - end - - it "does not return an existing false document" do - from_db.should_not eq(game) - end - end - - context "when not providing a block" do - - let!(:person) do - Person.find_or_create_by(title: "Senorita") - end - - it "creates a persisted document" do - person.should be_persisted - end - - it "sets the attributes" do - person.title.should eq("Senorita") - end - end - - context "when providing a block" do - - let!(:person) do - Person.find_or_create_by(title: "Senorita") do |person| - person.pets = true - end - end - - it "creates a persisted document" do - person.should be_persisted - end - - it "sets the attributes" do - person.title.should eq("Senorita") - end - - it "calls the block" do - person.pets.should be_true - end - end - end - end - - describe ".find_or_initialize_by" do - - context "when the document is found" do - - let!(:person) do - Person.create(title: "Senior") - end - - it "returns the document" do - Person.find_or_initialize_by(title: "Senior").should eq(person) - end - end - - context "when the document is not found" do - - context "when not providing a block" do - - let!(:person) do - Person.find_or_initialize_by(title: "Senorita") - end - - it "creates a new document" do - person.should be_new_record - end - - it "sets the attributes" do - person.title.should eq("Senorita") - end - end - - context "when providing a block" do - - let!(:person) do - Person.find_or_initialize_by(title: "Senorita") do |person| - person.pets = true - end - end - - it "creates a new document" do - person.should be_new_record - end - - it "sets the attributes" do - person.title.should eq("Senorita") - end - - it "calls the block" do - person.pets.should be_true - end - end - end - end - describe ".find_by" do context "when the document is found" do diff --git a/spec/mongoid/relations/referenced/many_spec.rb b/spec/mongoid/relations/referenced/many_spec.rb index 59388810f4..390fc42507 100644 --- a/spec/mongoid/relations/referenced/many_spec.rb +++ b/spec/mongoid/relations/referenced/many_spec.rb @@ -2480,26 +2480,56 @@ it "returns the document" do found.should eq(post) end + + it "keeps the document in the relation" do + found.person.should eq(person) + end end context "when the document does not exist" do - let(:found) do - person.posts.find_or_create_by(title: "Test") do |post| - post.content = "The Content" + context "when there is no criteria attached" do + + let(:found) do + person.posts.find_or_create_by(title: "Test") do |post| + post.content = "The Content" + end end - end - it "sets the new document attributes" do - found.title.should eq("Test") - end + it "sets the new document attributes" do + found.title.should eq("Test") + end - it "returns a newly persisted document" do - found.should be_persisted + it "returns a newly persisted document" do + found.should be_persisted + end + + it "calls the passed block" do + found.content.should eq("The Content") + end + + it "keeps the document in the relation" do + found.person.should eq(person) + end end - it "calls the passed block" do - found.content.should eq("The Content") + context "when a criteria is attached" do + + let(:found) do + person.posts.recent.find_or_create_by(title: "Test") + end + + it "sets the new document attributes" do + found.title.should eq("Test") + end + + it "returns a newly persisted document" do + found.should be_persisted + end + + it "keeps the document in the relation" do + found.person.should eq(person) + end end end end @@ -2523,6 +2553,10 @@ it "returns the document" do found.should eq(rating) end + + it "keeps the document in the relation" do + found.ratable.should eq(movie) + end end context "when the document does not exist" do @@ -2538,6 +2572,10 @@ it "returns a newly persisted document" do found.should be_persisted end + + it "keeps the document in the relation" do + found.ratable.should eq(movie) + end end end end