Use field DSL in CI integrations

Replace the `prop_accessor` and `#fields` definition with the `field`
DSL.

No functional changes, except for some reordering in the Buildkite and
DroneCi integrations so the URL field comes first.

Changelog: changed

app/models/integration.rb

@@ -161,7 +161,7 @@ def self.fields
   end
 
   def fields
-    self.class.fields
+    self.class.fields.dup
   end
 
   # Provide convenient accessor methods for each serialized property.

app/models/integrations/bamboo.rb

@@ -5,7 +5,26 @@ class Bamboo < BaseCi
     include ReactivelyCached
     prepend EnableSslVerification
 
-    prop_accessor :bamboo_url, :build_key, :username, :password
+    field :bamboo_url,
+      title: s_('BambooService|Bamboo URL'),
+      placeholder: s_('https://bamboo.example.com'),
+      help: s_('BambooService|Bamboo service root URL.'),
+      required: true
+
+    field :build_key,
+      help: s_('BambooService|Bamboo build plan key.'),
+      non_empty_password_title: s_('BambooService|Enter new build key'),
+      non_empty_password_help: s_('BambooService|Leave blank to use your current build key.'),
+      placeholder: s_('KEY'),
+      required: true
+
+    field :username,
+      help: s_('BambooService|The user with API access to the Bamboo server.')
+
+    field :password,
+      type: 'password',
+      non_empty_password_title: s_('ProjectService|Enter new password'),
+      non_empty_password_help: s_('ProjectService|Leave blank to use your current password')
 
     validates :bamboo_url, presence: true, public_url: true, if: :activated?
     validates :build_key, presence: true, if: :activated?
@@ -43,39 +62,6 @@ def self.to_param
       'bamboo'
     end
 
-    def fields
-      [
-          {
-            type: 'text',
-            name: 'bamboo_url',
-            title: s_('BambooService|Bamboo URL'),
-            placeholder: s_('https://bamboo.example.com'),
-            help: s_('BambooService|Bamboo service root URL.'),
-            required: true
-          },
-          {
-            type: 'password',
-            name: 'build_key',
-            help: s_('BambooService|Bamboo build plan key.'),
-            non_empty_password_title: s_('BambooService|Enter new build key'),
-            non_empty_password_help: s_('BambooService|Leave blank to use your current build key.'),
-            placeholder: s_('KEY'),
-            required: true
-          },
-          {
-            type: 'text',
-            name: 'username',
-            help: s_('BambooService|The user with API access to the Bamboo server.')
-          },
-          {
-            type: 'password',
-            name: 'password',
-            non_empty_password_title: s_('ProjectService|Enter new password'),
-            non_empty_password_help: s_('ProjectService|Leave blank to use your current password')
-          }
-      ]
-    end
-
     def build_page(sha, ref)
       with_reactive_cache(sha, ref) {|cached| cached[:build_page] }
     end

app/models/integrations/buildkite.rb

@@ -10,7 +10,18 @@ class Buildkite < BaseCi
 
     ENDPOINT = "https://buildkite.com"
 
-    prop_accessor :project_url, :token
+    field :project_url,
+      title: _('Pipeline URL'),
+      placeholder: "#{ENDPOINT}/example-org/test-pipeline",
+      required: true
+
+    field :token,
+      type: 'password',
+      title: _('Token'),
+      help: s_('ProjectService|The token you get after you create a Buildkite pipeline with a GitLab repository.'),
+      non_empty_password_title: s_('ProjectService|Enter new token'),
+      non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
+      required: true
 
     validates :project_url, presence: true, public_url: true, if: :activated?
     validates :token, presence: true, if: :activated?
@@ -74,24 +85,6 @@ def help
       s_('ProjectService|Run CI/CD pipelines with Buildkite.')
     end
 
-    def fields
-      [
-        { type: 'password',
-          name: 'token',
-          title: _('Token'),
-          help: s_('ProjectService|The token you get after you create a Buildkite pipeline with a GitLab repository.'),
-          non_empty_password_title: s_('ProjectService|Enter new token'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
-          required: true },
-
-        { type: 'text',
-          name: 'project_url',
-          title: _('Pipeline URL'),
-          placeholder: "#{ENDPOINT}/example-org/test-pipeline",
-          required: true }
-      ]
-    end
-
     def calculate_reactive_cache(sha, ref)
       response = Gitlab::HTTP.try_get(commit_status_path(sha), request_options)
 

app/models/integrations/drone_ci.rb

@@ -10,7 +10,17 @@ class DroneCi < BaseCi
 
     DRONE_SAAS_HOSTNAME = 'cloud.drone.io'
 
-    prop_accessor :drone_url, :token
+    field :drone_url,
+      title: s_('ProjectService|Drone server URL'),
+      placeholder: 'http://drone.example.com',
+      required: true
+
+    field :token,
+      type: 'password',
+      help: s_('ProjectService|Token for the Drone project.'),
+      non_empty_password_title: s_('ProjectService|Enter new token'),
+      non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
+      required: true
 
     validates :drone_url, presence: true, public_url: true, if: :activated?
     validates :token, presence: true, if: :activated?
@@ -94,26 +104,6 @@ def help
       s_('ProjectService|Run CI/CD pipelines with Drone.')
     end
 
-    def fields
-      [
-        {
-          type: 'password',
-          name: 'token',
-          help: s_('ProjectService|Token for the Drone project.'),
-          non_empty_password_title: s_('ProjectService|Enter new token'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'),
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'drone_url',
-          title: s_('ProjectService|Drone server URL'),
-          placeholder: 'http://drone.example.com',
-          required: true
-        }
-      ]
-    end
-
     override :hook_url
     def hook_url
       [drone_url, "/hook", "?owner=#{project.namespace.full_path}", "&name=#{project.path}", "&access_token=#{token}"].join

app/models/integrations/jenkins.rb

@@ -7,7 +7,25 @@ class Jenkins < BaseCi
     prepend EnableSslVerification
     extend Gitlab::Utils::Override
 
-    prop_accessor :jenkins_url, :project_name, :username, :password
+    field :jenkins_url,
+      title: s_('ProjectService|Jenkins server URL'),
+      required: true,
+      placeholder: 'http://jenkins.example.com',
+      help: s_('The URL of the Jenkins server.')
+
+    field :project_name,
+      required: true,
+      placeholder: 'my_project_name',
+      help: s_('The name of the Jenkins project. Copy the name from the end of the URL to the project.')
+
+    field :username,
+      help: s_('The username for the Jenkins server.')
+
+    field :password,
+      type: 'password',
+      help: s_('The password for the Jenkins server.'),
+      non_empty_password_title: s_('ProjectService|Enter new password.'),
+      non_empty_password_help: s_('ProjectService|Leave blank to use your current password.')
 
     before_validation :reset_password
 
@@ -71,37 +89,5 @@ def help
     def self.to_param
       'jenkins'
     end
-
-    def fields
-      [
-        {
-          type: 'text',
-          name: 'jenkins_url',
-          title: s_('ProjectService|Jenkins server URL'),
-          required: true,
-          placeholder: 'http://jenkins.example.com',
-          help: s_('The URL of the Jenkins server.')
-        },
-        {
-          type: 'text',
-          name: 'project_name',
-          required: true,
-          placeholder: 'my_project_name',
-          help: s_('The name of the Jenkins project. Copy the name from the end of the URL to the project.')
-        },
-        {
-          type: 'text',
-          name: 'username',
-          help: s_('The username for the Jenkins server.')
-        },
-        {
-          type: 'password',
-          name: 'password',
-          help: s_('The password for the Jenkins server.'),
-          non_empty_password_title: s_('ProjectService|Enter new password.'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current password.')
-        }
-      ]
-    end
   end
 end

app/models/integrations/mock_ci.rb

@@ -7,7 +7,11 @@ class MockCi < BaseCi
 
     ALLOWED_STATES = %w[failed canceled running pending success success-with-warnings skipped not_found].freeze
 
-    prop_accessor :mock_service_url
+    field :mock_service_url,
+      title: s_('ProjectService|Mock service URL'),
+      placeholder: 'http://localhost:4004',
+      required: true
+
     validates :mock_service_url, presence: true, public_url: true, if: :activated?
 
     def title
@@ -22,18 +26,6 @@ def self.to_param
       'mock_ci'
     end
 
-    def fields
-      [
-        {
-          type: 'text',
-          name: 'mock_service_url',
-          title: s_('ProjectService|Mock service URL'),
-          placeholder: 'http://localhost:4004',
-          required: true
-        }
-      ]
-    end
-
     # Return complete url to build page
     #
     # Ex.

app/models/integrations/teamcity.rb

@@ -8,7 +8,22 @@ class Teamcity < BaseCi
 
     TEAMCITY_SAAS_HOSTNAME = /\A[^\.]+\.teamcity\.com\z/i.freeze
 
-    prop_accessor :teamcity_url, :build_type, :username, :password
+    field :teamcity_url,
+      title: s_('ProjectService|TeamCity server URL'),
+      placeholder: 'https://teamcity.example.com',
+      required: true
+
+    field :build_type,
+      help: s_('ProjectService|The build configuration ID of the TeamCity project.'),
+      required: true
+
+    field :username,
+      help: s_('ProjectService|Must have permission to trigger a manual build in TeamCity.')
+
+    field :password,
+      type: 'password',
+      non_empty_password_title: s_('ProjectService|Enter new password'),
+      non_empty_password_help: s_('ProjectService|Leave blank to use your current password')
 
     validates :teamcity_url, presence: true, public_url: true, if: :activated?
     validates :build_type, presence: true, if: :activated?
@@ -51,35 +66,6 @@ def help
       s_('To run CI/CD pipelines with JetBrains TeamCity, input the GitLab project details in the TeamCity project Version Control Settings.')
     end
 
-    def fields
-      [
-        {
-          type: 'text',
-          name: 'teamcity_url',
-          title: s_('ProjectService|TeamCity server URL'),
-          placeholder: 'https://teamcity.example.com',
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'build_type',
-          help: s_('ProjectService|The build configuration ID of the TeamCity project.'),
-          required: true
-        },
-        {
-          type: 'text',
-          name: 'username',
-          help: s_('ProjectService|Must have permission to trigger a manual build in TeamCity.')
-        },
-        {
-          type: 'password',
-          name: 'password',
-          non_empty_password_title: s_('ProjectService|Enter new password'),
-          non_empty_password_help: s_('ProjectService|Leave blank to use your current password')
-        }
-      ]
-    end
-
     def build_page(sha, ref)
       with_reactive_cache(sha, ref) {|cached| cached[:build_page] }
     end

spec/models/integration_spec.rb

@@ -782,8 +782,16 @@
     end
   end
 
-  describe '#api_field_names' do
-    shared_examples 'api field names' do
+  describe 'field definitions' do
+    shared_examples '#fields' do
+      it 'does not return the same array' do
+        integration = fake_integration.new
+
+        expect(integration.fields).not_to be(integration.fields)
+      end
+    end
+
+    shared_examples '#api_field_names' do
       it 'filters out secret fields' do
         safe_fields = %w[some_safe_field safe_field url trojan_gift]
 
@@ -816,7 +824,8 @@ def fields
         end
       end
 
-      it_behaves_like 'api field names'
+      it_behaves_like '#fields'
+      it_behaves_like '#api_field_names'
     end
 
     context 'when the class uses the field DSL' do
@@ -839,7 +848,8 @@ def fields
         end
       end
 
-      it_behaves_like 'api field names'
+      it_behaves_like '#fields'
+      it_behaves_like '#api_field_names'
     end
   end
 

spec/support/shared_contexts/models/concerns/integrations/enable_ssl_verification_shared_context.rb

@@ -43,5 +43,9 @@
 
       expect(names.index('enable_ssl_verification')).to eq insert_index
     end
+
+    it 'does not insert the field repeatedly' do
+      expect(integration.fields.pluck(:name)).to eq(integration.fields.pluck(:name))
+    end
   end
 end