Просмотр исходного кода

Fixes #3121 - Object lookup by text attribute fails if symbol is given

Mantas 4 лет назад
Родитель
Сommit
543b552298

+ 1 - 1
app/models/application_model/can_lookup.rb

@@ -48,7 +48,7 @@ returns
 
     def find_and_save_to_cache_by(attr)
       record = find_by(attr)
-      return nil if string_key?(attr.keys.first) && (record&.send(attr.keys.first) != attr.values.first) # enforce case-sensitivity on MySQL
+      return nil if string_key?(attr.keys.first) && (record&.send(attr.keys.first) != attr.values.first.to_s) # enforce case-sensitivity on MySQL
       return record if ActiveRecord::Base.connection.transaction_open? # rollbacks can invalidate cache entries
 
       cache_set(attr.values.first, record)

+ 7 - 0
spec/models/application_model/can_lookup_examples.rb

@@ -64,6 +64,13 @@ RSpec.shared_examples 'ApplicationModel::CanLookup' do
             end
           end
 
+          if described_class.type_for_attribute(attribute).type == :string
+            # https://github.com/zammad/zammad/issues/3121
+            it 'retrieves results from cache with value as symbol' do
+              expect(described_class.lookup(attribute => instance.send(attribute).to_sym)).to be_present
+            end
+          end
+
           context 'when called a second time' do
             before { described_class.lookup(attribute => instance.send(attribute)) }
 

+ 54 - 0
spec/models/history_spec.rb

@@ -189,4 +189,58 @@ RSpec.describe History, type: :model do
       end
     end
   end
+
+  shared_examples 'lookup and create if needed' do |prefix|
+    let(:prefix)       { prefix }
+    let(:value_string) { Faker::Lorem.word }
+    let(:value_symbol) { value_string.to_sym }
+    let(:method_name)  { "#{prefix}_lookup" }
+    let(:cache_key)    { "#{described_class}::#{prefix.capitalize}::#{value_string}" }
+
+    context 'when object does not exist' do
+      it 'creates with a given String' do
+        expect(described_class.send(method_name, value_string)).to be_present
+      end
+
+      it 'creates with a given Symbol' do
+        expect(described_class.send(method_name, value_symbol)).to be_present
+      end
+    end
+
+    context 'when object exists' do
+      before do
+        described_class.send(method_name, value_string)
+      end
+
+      it 'retrieves object with a given String' do
+        expect(described_class.send(method_name, value_string)).to be_present
+      end
+
+      it 'hits cache with a given String' do
+        allow(Rails.cache).to receive(:read)
+        described_class.send(method_name, value_string)
+        expect(Rails.cache).to have_received(:read).with(cache_key)
+      end
+
+      it 'retrieves object with a given Symbol' do
+        expect(described_class.send(method_name, value_symbol)).to be_present
+      end
+
+      it 'hits cache with a given Symbol' do
+        allow(Rails.cache).to receive(:read)
+        described_class.send(method_name, value_symbol)
+        expect(Rails.cache).to have_received(:read).with(cache_key)
+      end
+    end
+  end
+
+  # https://github.com/zammad/zammad/issues/3121
+  describe '.type_lookup' do
+    include_examples 'lookup and create if needed', 'type'
+  end
+
+  # https://github.com/zammad/zammad/issues/3121
+  describe '.object_lookup' do
+    include_examples 'lookup and create if needed', 'object'
+  end
 end