[mock] fix duplicate user identity issues

* updating the email to match an existing identity no-ops

lib/zendesk2/client/requests/get_user_identities.rb

@@ -1,7 +1,7 @@
 class Zendesk2::Client
   class Real
     def get_user_identities(params={})
-      user_id   = params["user_id"]
+      user_id     = require_parameters(params, "user_id")
       page_params = Zendesk2.paging_parameters(params)
 
       request(
@@ -14,9 +14,9 @@ def get_user_identities(params={})
 
   class Mock
     def get_user_identities(params={})
-      user_id = params["user_id"]
+      user_id = require_parameters(params, "user_id")
 
-      page(params, :identities, "/users/#{user_id}/identities.json", "identities", filter: lambda{|c| c.select{|a| a["user_id"] == user_id}})
+      page(params, :identities, "/users/#{user_id}/identities.json", "identities", filter: lambda { |c| c.select { |a| a["user_id"] == user_id } })
     end
   end # Mock
 end

lib/zendesk2/client/requests/update_user.rb

@@ -17,25 +17,36 @@ def update_user(params={})
       user_id = params.delete("id")
       path    = "/users/#{user_id}.json"
 
-      if (email = params["email"]) && self.data[:identities].find { |k,i| i["type"] == "email" && i["value"] == email && i["user_id"] != user_id }
-        error!(:invalid, details: {"email" => [ { "description" => "Email #{params["email"]} is already being used by another user" } ] })
+      email = params["email"]
+
+      existing_identity = self.data[:identities].values.find { |i| i["type"] == "email" && i["value"] == email }
+
+      if existing_identity && existing_identity["user_id"] != user_id
+        # email not allowed to conflict across users
+        error!(:invalid, details: { "email" => [ {
+          "description" => "Email #{params["email"]} is already being used by another user",
+        } ] })
+      elsif existing_identity && existing_identity["user_id"] == user_id
+        # no-op email already used
+      else
+        # add a new identity
+        user_identity_id = self.class.new_id # ugh
+
+        user_identity = {
+          "id"         => user_identity_id,
+          "url"        => url_for("/users/#{user_id}/identities/#{user_identity_id}.json"),
+          "created_at" => Time.now.iso8601,
+          "updated_at" => Time.now.iso8601,
+          "type"       => "email",
+          "value"      => email,
+          "verified"   => false,
+          "primary"    => false,
+          "user_id"    => user_id,
+        }
+
+        self.data[:identities][user_identity_id] = user_identity
       end
 
-      user_identity_id = self.class.new_id # ugh
-
-      user_identity = {
-        "id"         => user_identity_id,
-        "url"        => url_for("/users/#{user_id}/identities/#{user_identity_id}.json"),
-        "created_at" => Time.now.iso8601,
-        "updated_at" => Time.now.iso8601,
-        "type"       => "email",
-        "value"      => params["email"],
-        "verified"   => false,
-        "primary"    => false,
-        "user_id"    => user_id,
-      }
-
-      self.data[:identities][user_identity_id] = user_identity
       body = self.data[:users][user_id].merge!(params)
       response(
         :method => :put,
@@ -44,6 +55,7 @@ def update_user(params={})
           "user" => body
         },
       )
+
     end
   end
 end

spec/users_spec.rb

@@ -32,11 +32,7 @@
   end
 
   describe "#save" do
-    before(:each) do
-      @user = client.users.create!(email: "zendesk2+#{Zendesk2.uuid}@example.org", name: Zendesk2.uuid)
-    end
-
-    let(:user) { @user }
+    let!(:user) { client.users.create!(email: "zendesk2+#{Zendesk2.uuid}@example.org", name: Zendesk2.uuid) }
 
     it "should update organization" do
       user.organization = organization = client.organizations.create!(name: Zendesk2.uuid)
@@ -125,22 +121,29 @@
     end
 
     it "should create another identity when updating email" do
+      expect(user.identities.size).to eq(1)
+
       original_email = user.email
       user.email = (new_email = "zendesk2+#{Zendesk2.uuid}@example.org")
-      user.save!
 
-      expect((identities = user.identities.all).size).to eq(2)
-      new_identity = identities.find{|i| i.value == new_email}
-      expect(new_identity).not_to be_nil
+      expect {
+        user.save!
+      }.to change { user.identities.size }.by(1)
 
-      expect(new_identity.primary).to be_falsey
+      new_identity = user.identities.find { |i| i.value == new_email }
 
-      original_identity = identities.find{|i| i.value == original_email}
-      expect(original_identity).not_to be_nil
+      expect(new_identity).to be
+      expect(new_identity.primary).to eq(false)
 
-      expect(original_identity.primary).to be_truthy
+      original_identity = user.identities.find { |i| i.value == original_email }
 
+      expect(original_identity).to be
+      expect(original_identity.primary).to eq(true)
       expect(user.reload.email).to eq(original_email)
+
+      expect {
+        user.save!
+      }.not_to change { user.identities.size }
     end
 
     it "should form 'legacy' login url" do