Merge branch 'tomquirk/326339-dont-repeat-optional-github-integration-form' into 'master'

Add custom label GitHub status check checkbox

See merge request gitlab-org/gitlab!72566

app/assets/javascripts/integrations/edit/components/dynamic_field.vue

@@ -62,6 +62,14 @@ export default {
       required: false,
       default: null,
     },
+    /**
+     * The label that is displayed inline with the checkbox.
+     */
+    checkboxLabel: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   data() {
     return {
@@ -152,7 +160,7 @@ export default {
     <template v-if="isCheckbox">
       <input :name="fieldName" type="hidden" :value="model || false" />
       <gl-form-checkbox :id="fieldId" v-model="model" :disabled="isInheriting">
-        {{ humanizedTitle }}
+        {{ checkboxLabel || humanizedTitle }}
       </gl-form-checkbox>
     </template>
     <template v-else-if="isSelect">

app/serializers/service_field_entity.rb

@@ -4,7 +4,7 @@ class ServiceFieldEntity < Grape::Entity
   include RequestAwareEntity
   include Gitlab::Utils::StrongMemoize
 
-  expose :type, :name, :placeholder, :required, :choices
+  expose :type, :name, :placeholder, :required, :choices, :checkbox_label
 
   expose :title do |field|
     non_empty_password?(field) ? field[:non_empty_password_title] : field[:title]

doc/api/integrations.md

@@ -740,7 +740,7 @@ Parameters:
 | --------- | ---- | -------- | ----------- |
 | `token` | string | true | GitHub API token with `repo:status` OAuth scope |
 | `repository_url` | string | true | GitHub repository URL |
-| `static_context` | boolean | false | Append instance name instead of branch to [status check name](../user/project/integrations/github.md#static--dynamic-status-check-names) |
+| `static_context` | boolean | false | Append instance name instead of branch to [status check name](../user/project/integrations/github.md#static-or-dynamic-status-check-names) |
 
 ### Disable GitHub integration
 

doc/user/project/integrations/github.md

@@ -37,20 +37,20 @@ Complete these steps in GitLab:
 1. Ensure that the **Active** toggle is enabled.
 1. Paste the token you generated on GitHub.
 1. Enter the path to your project on GitHub, such as `https://github.com/username/repository`.
-1. (Optional) To disable static status check names, clear the **Static status check names** checkbox.
+1. (Optional) To disable static status check names, clear the **Enable static status check names** checkbox.
 1. Select **Save changes** or optionally select **Test settings**.
 
 After configuring the integration, see [Pipelines for external pull requests](../../../ci/ci_cd_for_external_repos/#pipelines-for-external-pull-requests)
 to configure pipelines to run for open pull requests.
 
-### Static / dynamic status check names
+### Static or dynamic status check names
 
 > - Introduced in GitLab 11.5: using static status check names as opt-in option.
 > - [In GitLab 12.4](https://gitlab.com/gitlab-org/gitlab/-/issues/9931), static status check names is default behavior for new projects.
 
 This makes it possible to mark these status checks as **Required** on GitHub.
 
-When **Static status check names** is enabled on the integration page, your
+When **Enable static status check names** is checked on the integration page, your
 GitLab instance host name is appended to a status check name.
 
-When disabled, it uses dynamic status check names and appends the branch name.
+When unchecked, it uses dynamic status check names and appends the branch name.

ee/app/models/integrations/github.rb

@@ -39,7 +39,7 @@ module Integrations
     end
 
     def fields
-      learn_more_link_url = help_page_path('user/project/integrations/github', anchor: 'static--dynamic-status-check-name')
+      learn_more_link_url = help_page_path('user/project/integrations/github', anchor: 'static-or-dynamic-status-check-names')
       learn_more_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: learn_more_link_url }
       static_context_field_help = s_('GithubIntegration|Select this if you want GitHub to mark status checks as "Required". %{learn_more_link_start}Learn more%{learn_more_link_end}.').html_safe % { learn_more_link_start: learn_more_link_start, learn_more_link_end: '</a>'.html_safe }
 
@@ -60,6 +60,7 @@ module Integrations
         { type: 'checkbox',
           name: "static_context",
           title: s_('GithubIntegration|Static status check names (optional)'),
+          checkbox_label: s_('GithubIntegration|Enable static status check names'),
           help: static_context_field_help }
       ]
     end

ee/spec/serializers/service_field_entity_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ServiceFieldEntity do
+  let(:request) { double('request') }
+
+  subject { described_class.new(field, request: request, service: integration).as_json }
+
+  before do
+    allow(request).to receive(:service).and_return(integration)
+  end
+
+  describe '#as_json' do
+    context 'GitHub Service' do
+      let(:integration) { create(:github_integration) }
+
+      context 'field with type checkbox' do
+        let(:field) { integration_field('static_context') }
+
+        it 'exposes correct attributes and casts value to Boolean' do
+          expected_hash = {
+            type: 'checkbox',
+            name: 'static_context',
+            title: 'Static status check names (optional)',
+            placeholder: nil,
+            required: nil,
+            choices: nil,
+            value: 'true',
+            checkbox_label: 'Enable static status check names'
+          }
+
+          is_expected.to include(expected_hash)
+        end
+      end
+    end
+  end
+
+  def integration_field(name)
+    integration.global_fields.find { |f| f[:name] == name }
+  end
+end

locale/gitlab.pot

@@ -15738,6 +15738,9 @@ msgstr ""
 msgid "GithubIntegration|Create a %{token_link_start}personal access token%{token_link_end} with %{status_html} access granted and paste it here."
 msgstr ""
 
+msgid "GithubIntegration|Enable static status check names"
+msgstr ""
+
 msgid "GithubIntegration|Obtain statuses for commits and pull requests."
 msgstr ""
 

spec/frontend/integrations/edit/components/dynamic_field_spec.js

@@ -35,136 +35,145 @@ describe('DynamicField', () => {
   const findGlFormTextarea = () => wrapper.findComponent(GlFormTextarea);
 
   describe('template', () => {
-    describe.each([
-      [true, 'disabled', 'readonly'],
-      [false, undefined, undefined],
-    ])('dynamic field, when isInheriting = `%p`', (isInheriting, disabled, readonly) => {
-      describe('type is checkbox', () => {
-        beforeEach(() => {
-          createComponent(
-            {
-              type: 'checkbox',
-            },
-            isInheriting,
-          );
-        });
+    describe.each`
+      isInheriting | disabled      | readonly      | checkboxLabel
+      ${true}      | ${'disabled'} | ${'readonly'} | ${undefined}
+      ${false}     | ${undefined}  | ${undefined}  | ${'Custom checkbox label'}
+    `(
+      'dynamic field, when isInheriting = `%p`',
+      ({ isInheriting, disabled, readonly, checkboxLabel }) => {
+        describe('type is checkbox', () => {
+          beforeEach(() => {
+            createComponent(
+              {
+                type: 'checkbox',
+                checkboxLabel,
+              },
+              isInheriting,
+            );
+          });
 
-        it(`renders GlFormCheckbox, which ${isInheriting ? 'is' : 'is not'} disabled`, () => {
-          expect(findGlFormCheckbox().exists()).toBe(true);
-          expect(findGlFormCheckbox().find('[type=checkbox]').attributes('disabled')).toBe(
-            disabled,
-          );
-        });
+          it(`renders GlFormCheckbox, which ${isInheriting ? 'is' : 'is not'} disabled`, () => {
+            expect(findGlFormCheckbox().exists()).toBe(true);
+            expect(findGlFormCheckbox().find('[type=checkbox]').attributes('disabled')).toBe(
+              disabled,
+            );
+          });
 
-        it('does not render other types of input', () => {
-          expect(findGlFormSelect().exists()).toBe(false);
-          expect(findGlFormTextarea().exists()).toBe(false);
-          expect(findGlFormInput().exists()).toBe(false);
-        });
-      });
+          it(`renders GlFormCheckbox with correct text content when checkboxLabel is ${checkboxLabel}`, () => {
+            expect(findGlFormCheckbox().text()).toBe(checkboxLabel ?? defaultProps.title);
+          });
 
-      describe('type is select', () => {
-        beforeEach(() => {
-          createComponent(
-            {
-              type: 'select',
-              choices: [
-                ['all', 'All details'],
-                ['standard', 'Standard'],
-              ],
-            },
-            isInheriting,
-          );
+          it('does not render other types of input', () => {
+            expect(findGlFormSelect().exists()).toBe(false);
+            expect(findGlFormTextarea().exists()).toBe(false);
+            expect(findGlFormInput().exists()).toBe(false);
+          });
         });
 
-        it(`renders GlFormSelect, which ${isInheriting ? 'is' : 'is not'} disabled`, () => {
-          expect(findGlFormSelect().exists()).toBe(true);
-          expect(findGlFormSelect().findAll('option')).toHaveLength(2);
-          expect(findGlFormSelect().find('select').attributes('disabled')).toBe(disabled);
-        });
+        describe('type is select', () => {
+          beforeEach(() => {
+            createComponent(
+              {
+                type: 'select',
+                choices: [
+                  ['all', 'All details'],
+                  ['standard', 'Standard'],
+                ],
+              },
+              isInheriting,
+            );
+          });
 
-        it('does not render other types of input', () => {
-          expect(findGlFormCheckbox().exists()).toBe(false);
-          expect(findGlFormTextarea().exists()).toBe(false);
-          expect(findGlFormInput().exists()).toBe(false);
-        });
-      });
+          it(`renders GlFormSelect, which ${isInheriting ? 'is' : 'is not'} disabled`, () => {
+            expect(findGlFormSelect().exists()).toBe(true);
+            expect(findGlFormSelect().findAll('option')).toHaveLength(2);
+            expect(findGlFormSelect().find('select').attributes('disabled')).toBe(disabled);
+          });
 
-      describe('type is textarea', () => {
-        beforeEach(() => {
-          createComponent(
-            {
-              type: 'textarea',
-            },
-            isInheriting,
-          );
+          it('does not render other types of input', () => {
+            expect(findGlFormCheckbox().exists()).toBe(false);
+            expect(findGlFormTextarea().exists()).toBe(false);
+            expect(findGlFormInput().exists()).toBe(false);
+          });
         });
 
-        it(`renders GlFormTextarea, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
-          expect(findGlFormTextarea().exists()).toBe(true);
-          expect(findGlFormTextarea().find('textarea').attributes('readonly')).toBe(readonly);
-        });
+        describe('type is textarea', () => {
+          beforeEach(() => {
+            createComponent(
+              {
+                type: 'textarea',
+              },
+              isInheriting,
+            );
+          });
 
-        it('does not render other types of input', () => {
-          expect(findGlFormCheckbox().exists()).toBe(false);
-          expect(findGlFormSelect().exists()).toBe(false);
-          expect(findGlFormInput().exists()).toBe(false);
-        });
-      });
+          it(`renders GlFormTextarea, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
+            expect(findGlFormTextarea().exists()).toBe(true);
+            expect(findGlFormTextarea().find('textarea').attributes('readonly')).toBe(readonly);
+          });
 
-      describe('type is password', () => {
-        beforeEach(() => {
-          createComponent(
-            {
-              type: 'password',
-            },
-            isInheriting,
-          );
+          it('does not render other types of input', () => {
+            expect(findGlFormCheckbox().exists()).toBe(false);
+            expect(findGlFormSelect().exists()).toBe(false);
+            expect(findGlFormInput().exists()).toBe(false);
+          });
         });
 
-        it(`renders GlFormInput, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
-          expect(findGlFormInput().exists()).toBe(true);
-          expect(findGlFormInput().attributes('type')).toBe('password');
-          expect(findGlFormInput().attributes('readonly')).toBe(readonly);
-        });
+        describe('type is password', () => {
+          beforeEach(() => {
+            createComponent(
+              {
+                type: 'password',
+              },
+              isInheriting,
+            );
+          });
 
-        it('does not render other types of input', () => {
-          expect(findGlFormCheckbox().exists()).toBe(false);
-          expect(findGlFormSelect().exists()).toBe(false);
-          expect(findGlFormTextarea().exists()).toBe(false);
-        });
-      });
+          it(`renders GlFormInput, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
+            expect(findGlFormInput().exists()).toBe(true);
+            expect(findGlFormInput().attributes('type')).toBe('password');
+            expect(findGlFormInput().attributes('readonly')).toBe(readonly);
+          });
 
-      describe('type is text', () => {
-        beforeEach(() => {
-          createComponent(
-            {
-              type: 'text',
-              required: true,
-            },
-            isInheriting,
-          );
+          it('does not render other types of input', () => {
+            expect(findGlFormCheckbox().exists()).toBe(false);
+            expect(findGlFormSelect().exists()).toBe(false);
+            expect(findGlFormTextarea().exists()).toBe(false);
+          });
         });
 
-        it(`renders GlFormInput, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
-          expect(findGlFormInput().exists()).toBe(true);
-          expect(findGlFormInput().attributes()).toMatchObject({
-            type: 'text',
-            id: 'service_project_url',
-            name: 'service[project_url]',
-            placeholder: defaultProps.placeholder,
-            required: 'required',
+        describe('type is text', () => {
+          beforeEach(() => {
+            createComponent(
+              {
+                type: 'text',
+                required: true,
+              },
+              isInheriting,
+            );
           });
-          expect(findGlFormInput().attributes('readonly')).toBe(readonly);
-        });
 
-        it('does not render other types of input', () => {
-          expect(findGlFormCheckbox().exists()).toBe(false);
-          expect(findGlFormSelect().exists()).toBe(false);
-          expect(findGlFormTextarea().exists()).toBe(false);
+          it(`renders GlFormInput, which ${isInheriting ? 'is' : 'is not'} readonly`, () => {
+            expect(findGlFormInput().exists()).toBe(true);
+            expect(findGlFormInput().attributes()).toMatchObject({
+              type: 'text',
+              id: 'service_project_url',
+              name: 'service[project_url]',
+              placeholder: defaultProps.placeholder,
+              required: 'required',
+            });
+            expect(findGlFormInput().attributes('readonly')).toBe(readonly);
+          });
+
+          it('does not render other types of input', () => {
+            expect(findGlFormCheckbox().exists()).toBe(false);
+            expect(findGlFormSelect().exists()).toBe(false);
+            expect(findGlFormTextarea().exists()).toBe(false);
+          });
         });
-      });
-    });
+      },
+    );
 
     describe('help text', () => {
       it('renders description with help text', () => {

spec/serializers/service_field_entity_spec.rb

@@ -27,7 +27,8 @@ RSpec.describe ServiceFieldEntity do
             help: 'Use a username for server version and an email for cloud version.',
             required: true,
             choices: nil,
-            value: 'jira_username'
+            value: 'jira_username',
+            checkbox_label: nil
           }
 
           is_expected.to eq(expected_hash)
@@ -46,7 +47,8 @@ RSpec.describe ServiceFieldEntity do
             help: 'Leave blank to use your current password or API token.',
             required: true,
             choices: nil,
-            value: 'true'
+            value: 'true',
+            checkbox_label: nil
           }
 
           is_expected.to eq(expected_hash)
@@ -68,7 +70,8 @@ RSpec.describe ServiceFieldEntity do
             placeholder: nil,
             required: nil,
             choices: nil,
-            value: 'true'
+            value: 'true',
+            checkbox_label: nil
           }
 
           is_expected.to include(expected_hash)
@@ -88,7 +91,8 @@ RSpec.describe ServiceFieldEntity do
             required: nil,
             choices: [['All branches', 'all'], ['Default branch', 'default'], ['Protected branches', 'protected'], ['Default branch and protected branches', 'default_and_protected']],
             help: nil,
-            value: nil
+            value: nil,
+            checkbox_label: nil
           }
 
           is_expected.to eq(expected_hash)