Remove unused and dangerous Exchange#eql? override

The `eql?` override has been added in very early commits but was
actually not used except in a test. It also caused performance problems
since each call to `eql?` would issue two database queries. A developer
would unknowingly trigger these when using `exchanges.uniq`. A mistake
that could have happened again in the future.

I moved the implementation to the test that was actually using it and
made a second test a bit more explicit.

app/models/exchange.rb

@@ -99,21 +99,6 @@ class Exchange < ActiveRecord::Base
     incoming? ? sender : receiver
   end
 
-  def to_h(core_only = false)
-    h = attributes.merge('variant_ids' => variant_ids.sort,
-                         'enterprise_fee_ids' => enterprise_fee_ids.sort)
-    h.reject! { |k| %w(id order_cycle_id created_at updated_at).include? k } if core_only
-    h
-  end
-
-  def eql?(e)
-    if e.respond_to? :to_h
-      to_h(true) == e.to_h(true)
-    else
-      super e
-    end
-  end
-
   def refresh_products_cache
     OpenFoodNetwork::ProductsCache.exchange_changed self
   end

spec/models/exchange_spec.rb

@@ -285,61 +285,15 @@ describe Exchange do
     ex1.update_attribute(:tag_list, "wholesale")
     ex2 = ex1.clone! new_oc
 
-    expect(ex1.eql?(ex2)).to be true
+    expect(ex1.sender_id).to eq ex2.sender_id
+    expect(ex1.receiver_id).to eq ex2.receiver_id
+    expect(ex1.pickup_time).to eq ex2.pickup_time
+    expect(ex1.pickup_instructions).to eq ex2.pickup_instructions
+    expect(ex1.incoming).to eq ex2.incoming
+    expect(ex1.receival_instructions).to eq ex2.receival_instructions
+    expect(ex1.variant_ids).to eq ex2.variant_ids
+    expect(ex1.enterprise_fee_ids).to eq ex2.enterprise_fee_ids
+
     expect(ex2.reload.tag_list).to eq ["wholesale"]
   end
-
-  describe "converting to hash" do
-    let(:oc) { create(:order_cycle) }
-    let(:exchange) do
-      exchange = oc.exchanges.last
-      exchange.save!
-      allow(exchange).to receive(:variant_ids) { [1835, 1834] } # Test id ordering
-      allow(exchange).to receive(:enterprise_fee_ids) { [1493, 1492] } # Test id ordering
-      exchange
-    end
-
-    it "converts to a hash" do
-      expect(exchange.to_h).to eq(
-        'id' => exchange.id, 'order_cycle_id' => oc.id,
-        'sender_id' => exchange.sender_id, 'receiver_id' => exchange.receiver_id,
-        'incoming' => exchange.incoming,
-        'variant_ids' => exchange.variant_ids.sort,
-        'enterprise_fee_ids' => exchange.enterprise_fee_ids.sort,
-        'pickup_time' => exchange.pickup_time, 'pickup_instructions' => exchange.pickup_instructions,
-        'receival_instructions' => exchange.receival_instructions,
-        'created_at' => exchange.created_at, 'updated_at' => exchange.updated_at
-      )
-    end
-
-    it "converts to a hash of core attributes only" do
-      expect(exchange.to_h(true)).to eq(
-        'sender_id' => exchange.sender_id, 'receiver_id' => exchange.receiver_id,
-        'incoming' => exchange.incoming,
-        'variant_ids' => exchange.variant_ids.sort,
-        'enterprise_fee_ids' => exchange.enterprise_fee_ids.sort,
-        'pickup_time' => exchange.pickup_time, 'pickup_instructions' => exchange.pickup_instructions,
-        'receival_instructions' => exchange.receival_instructions
-      )
-    end
-  end
-
-  describe "comparing equality" do
-    it "compares Exchanges using to_h" do
-      e1 = Exchange.new
-      e2 = Exchange.new
-
-      allow(e1).to receive(:to_h) { { 'sender_id' => 456 } }
-      allow(e2).to receive(:to_h) { { 'sender_id' => 456 } }
-
-      expect(e1.eql?(e2)).to be true
-    end
-
-    it "compares other objects using super" do
-      exchange = Exchange.new
-      exchange_fee = ExchangeFee.new
-
-      expect(exchange.eql?(exchange_fee)).to be false
-    end
-  end
 end

spec/models/order_cycle_spec.rb

@@ -372,14 +372,11 @@ describe OrderCycle do
     expect(occ.coordinator_fee_ids).to eq(oc.coordinator_fee_ids)
     expect(occ.preferred_product_selection_from_coordinator_inventory_only).to eq(oc.preferred_product_selection_from_coordinator_inventory_only)
 
-    # to_h gives us a unique hash for each exchange
-    # check that the clone has no additional exchanges
-    occ.exchanges.map(&:to_h).all? do |ex|
-      oc.exchanges.map(&:to_h).include? ex
-    end
-    # check that the clone has original exchanges
-    occ.exchanges.map(&:to_h).include? ex1.to_h
-    occ.exchanges.map(&:to_h).include? ex2.to_h
+    # Check that the exchanges have been cloned.
+    original_exchange_attributes = oc.exchanges.map { |ex| core_exchange_attributes(ex) }
+    cloned_exchange_attributes = occ.exchanges.map { |ex| core_exchange_attributes(ex) }
+
+    expect(cloned_exchange_attributes).to match_array original_exchange_attributes
   end
 
   describe "finding recently closed order cycles" do
@@ -467,4 +464,14 @@ describe OrderCycle do
       expect(item_with_overridden_variant.variant.on_hand).to eq(1000)
     end
   end
+
+  def core_exchange_attributes(exchange)
+    exterior_attribute_keys = %w(id order_cycle_id created_at updated_at)
+    exchange.attributes.
+      reject { |k| exterior_attribute_keys.include? k }.
+      merge(
+        'variant_ids' => exchange.variant_ids.sort,
+        'enterprise_fee_ids' => exchange.enterprise_fee_ids.sort
+      )
+  end
 end