diff --git a/app/models/integration.rb b/app/models/integration.rb index 727f7d2eeba0ecf15e0afde661c54df9b048537e..af8e8060d81ea6605a2451074ce984406044f58a 100644 --- a/app/models/integration.rb +++ b/app/models/integration.rb @@ -200,14 +200,21 @@ def #{arg}_was # Provide convenient boolean accessor methods for each serialized property. # Also keep track of updated properties in a similar way as ActiveModel::Dirty def self.boolean_accessor(*args) - prop_accessor(*args) - args.each do |arg| + # TODO: Allow legacy usage of `.boolean_accessor`, once all integrations + # are converted to the field DSL we can remove this and only call + # `.boolean_accessor` through `.field`. + # + # See https://gitlab.com/groups/gitlab-org/-/epics/7652 + prop_accessor(arg) unless method_defined?(arg) + class_eval <<~RUBY, __FILE__, __LINE__ + 1 - def #{arg} - return if properties.blank? + # Make the original getter available as a private method. + alias_method :#{arg}_before_type_cast, :#{arg} + private(:#{arg}_before_type_cast) - Gitlab::Utils.to_boolean(properties['#{arg}']) + def #{arg} + Gitlab::Utils.to_boolean(#{arg}_before_type_cast) end def #{arg}? diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index 63db1c9535ff17d2ac127f961a500646df926f93..39fde0c9b7799233289f0f6d8af5a8b5c9531741 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -1083,11 +1083,8 @@ def fields end it 'provides boolean accessors for checkbox fields' do - integration.boolean = 'yes' - expect(integration.boolean?).to be(true) - - integration.boolean = nil - expect(integration.boolean?).to be(false) + expect(integration).to respond_to(:boolean) + expect(integration).to respond_to(:boolean?) expect(integration).not_to respond_to(:foo?) expect(integration).not_to respond_to(:bar?) @@ -1129,11 +1126,12 @@ def fields describe 'boolean_accessor' do let(:klass) do Class.new(Integration) do + prop_accessor :test_value boolean_accessor :test_value end end - let(:integration) { klass.new(properties: { test_value: input }) } + let(:integration) { klass.new(test_value: input) } where(:input, :method_result, :predicate_method_result) do true | true | true @@ -1163,6 +1161,35 @@ def fields test_value: be(method_result), test_value?: be(predicate_method_result) ) + + # Make sure the original value is stored correctly + expect(integration.send(:test_value_before_type_cast)).to eq(input) + expect(integration.properties).to include('test_value' => input) + end + + context 'when using data fields' do + let(:klass) do + Class.new(Integration) do + field :project_url, storage: :data_fields, type: 'checkbox' + + def data_fields + issue_tracker_data || self.build_issue_tracker_data + end + end + end + + let(:integration) { klass.new(project_url: input) } + + it 'has the correct value' do + expect(integration).to have_attributes( + project_url: be(method_result), + project_url?: be(predicate_method_result) + ) + + # Make sure the original value is stored correctly + expect(integration.send(:project_url_before_type_cast)).to eq(input == false ? 'false' : input) + expect(integration.properties).not_to include('project_url') + end end end @@ -1174,6 +1201,24 @@ def fields test_value?: be(false) ) end + + context 'when getter is not defined' do + let(:input) { true } + let(:klass) do + Class.new(Integration) do + boolean_accessor :test_value + end + end + + it 'defines a prop_accessor' do + expect(integration).to have_attributes( + test_value: true, + test_value?: true + ) + + expect(integration.properties['test_value']).to be(true) + end + end end describe '#attributes' do