From 05738af3ee99355e30d1c652cfb0d15178c53b91 Mon Sep 17 00:00:00 2001
From: Luke Bennett <lbennett@gitlab.com>
Date: Thu, 14 Mar 2019 20:52:01 +0000
Subject: [PATCH] Improve project service desk settings

Prioritize and simplify project settings content.
---
 .../components/service_desk_root.vue          | 23 +++---
 .../components/service_desk_setting.vue       | 74 ++++++++++---------
 .../projects/_service_desk_settings.html.haml |  5 +-
 ...settings-ok-service-desk-settings-only.yml |  5 ++
 .../settings/service_desk_setting_spec.rb     |  6 +-
 .../components/service_desk_setting_spec.js   | 32 ++------
 locale/gitlab.pot                             | 30 ++++++--
 7 files changed, 91 insertions(+), 84 deletions(-)
 create mode 100644 ee/changelogs/unreleased/proj-settings-ok-service-desk-settings-only.yml

diff --git a/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_root.vue b/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_root.vue
index 2fcb5700208fe7..0bba340f11cc0d 100644
--- a/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_root.vue
+++ b/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_root.vue
@@ -1,5 +1,6 @@
 <script>
 import Flash from '~/flash';
+import { __ } from '~/locale';
 import serviceDeskSetting from './service_desk_setting.vue';
 import ServiceDeskStore from '../stores/service_desk_store';
 import ServiceDeskService from '../services/service_desk_service';
@@ -65,14 +66,14 @@ export default {
         .then(data => {
           const email = data.service_desk_address;
           if (!email) {
-            throw new Error("Response didn't include `service_desk_address`");
+            throw new Error(__("Response didn't include `service_desk_address`"));
           }
 
           this.store.setIncomingEmail(email);
         })
         .catch(() => {
-          this.flash = new Flash(
-            'An error occurred while fetching the Service Desk address.',
+          this.flash = Flash(
+            __('An error occurred while fetching the Service Desk address.'),
             'alert',
             this.$el,
           );
@@ -83,7 +84,8 @@ export default {
       this.isEnabled = isChecked;
       this.store.resetIncomingEmail();
       if (this.flash) {
-        this.flash.destroy();
+        this.flash.remove();
+        this.flash = undefined;
       }
 
       this.service
@@ -92,18 +94,17 @@ export default {
         .then(data => {
           const email = data.service_desk_address;
           if (isChecked && !email) {
-            throw new Error("Response didn't include `service_desk_address`");
+            throw new Error(__("Response didn't include `service_desk_address`"));
           }
 
           this.store.setIncomingEmail(email);
         })
         .catch(() => {
-          const verb = isChecked ? 'enabling' : 'disabling';
-          this.flash = new Flash(
-            `An error occurred while ${verb} Service Desk.`,
-            'alert',
-            this.$el,
-          );
+          const message = isChecked
+            ? __('An error occurred while enabling Service Desk.')
+            : __('An error occurred while disabling Service Desk.');
+
+          this.flash = Flash(message, 'alert', this.$el);
         });
     },
   },
diff --git a/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_setting.vue b/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_setting.vue
index 571e35896925e9..694053c1c7e239 100644
--- a/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_setting.vue
+++ b/ee/app/assets/javascripts/projects/settings_service_desk/components/service_desk_setting.vue
@@ -1,4 +1,5 @@
 <script>
+import Toggle from '~/vue_shared/components/toggle_button.vue';
 import tooltip from '~/vue_shared/directives/tooltip';
 import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 import eventHub from '../event_hub';
@@ -11,6 +12,7 @@ export default {
 
   components: {
     ClipboardButton,
+    Toggle,
   },
 
   props: {
@@ -25,8 +27,7 @@ export default {
     },
   },
   methods: {
-    onCheckboxToggle(e) {
-      const isChecked = e.target.checked;
+    onCheckboxToggle(isChecked) {
       eventHub.$emit('serviceDeskEnabledCheckboxToggled', isChecked);
     },
   },
@@ -35,44 +36,45 @@ export default {
 
 <template>
   <div>
-    <div class="form-check">
-      <label for="service-desk-enabled-checkbox">
-        <input
-          id="service-desk-enabled-checkbox"
-          ref="enabled-checkbox"
-          :checked="isEnabled"
-          type="checkbox"
-          @change="onCheckboxToggle($event)"
-        />
-        <span class="descr"> Activate Service Desk </span>
-      </label>
-    </div>
-    <div v-if="isEnabled" class="panel-slim ">
-      <div class="card-header">
-        <h3 class="card-title">Forward external support email address to:</h3>
-      </div>
-      <div class="card-body">
+    <toggle
+      id="service-desk-checkbox"
+      ref="service-desk-checkbox"
+      :value="isEnabled"
+      class="d-inline-block align-middle mr-1"
+      @change="onCheckboxToggle"
+    />
+    <label class="font-weight-bold" for="service-desk-checkbox">
+      {{ __('Activate Service Desk') }}
+    </label>
+    <div v-if="isEnabled" class="row mt-3">
+      <div class="col-md-9 mb-0">
+        <strong id="incoming-email-describer" class="d-block mb-1">
+          {{ __('Forward external support email address to') }}
+        </strong>
         <template v-if="incomingEmail">
-          <span ref="service-desk-incoming-email"> {{ incomingEmail }} </span>
-          <clipboard-button
-            :title="__('Copy incoming email address to clipboard')"
-            :text="incomingEmail"
-            css-class="btn btn-clipboard btn-transparent"
-          />
-          <a
-            href="https://docs.gitlab.com/ee/development/emails.html#email-namespace"
-            target="_blank"
-            rel="noopener"
-          >
-            <i
-              class="fa fa-question-circle"
-              :aria-label="__('Learn more about incoming email addresses')"
-            ></i>
-          </a>
+          <div class="input-group">
+            <input
+              ref="service-desk-incoming-email"
+              type="text"
+              class="form-control incoming-email h-auto"
+              placeholder="__('Incoming email')"
+              aria-label="__('Incoming email')"
+              aria-describedby="incoming-email-describer"
+              :value="incomingEmail"
+              disabled="true"
+            />
+            <div class="input-group-append">
+              <clipboard-button
+                :title="__('Copy to clipboard')"
+                :text="incomingEmail"
+                css-class="btn qa-clipboard-button"
+              />
+            </div>
+          </div>
         </template>
         <template v-else>
           <i class="fa fa-spinner fa-spin" aria-hidden="true"> </i>
-          <span class="sr-only"> Fetching incoming email </span>
+          <span class="sr-only">{{ __('Fetching incoming email') }}</span>
         </template>
       </div>
     </div>
diff --git a/ee/app/views/projects/_service_desk_settings.html.haml b/ee/app/views/projects/_service_desk_settings.html.haml
index 41dfc526a63fd2..51958620be4b90 100644
--- a/ee/app/views/projects/_service_desk_settings.html.haml
+++ b/ee/app/views/projects/_service_desk_settings.html.haml
@@ -5,9 +5,8 @@
       %h4.settings-title.js-settings-toggle.js-settings-toggle-trigger-only= _('Service Desk')
       %button.btn.js-settings-toggle
         = expanded ? _('Collapse') : _('Expand')
-      %p
-        Customize your service desk settings.
-        = link_to "Learn more about service desk.", help_page_path('user/project/service_desk'), target: '_blank'
+      - link_start = "<a href='#{help_page_path('user/project/service_desk')}' target='_blank' rel='noopener noreferrer'>".html_safe
+      %p= _('Enable/disable your service desk. %{link_start}Learn more about service desk%{link_end}.').html_safe % { link_start: link_start, link_end: '</a>'.html_safe }
     .settings-content
       - if EE::Gitlab::ServiceDesk.enabled?(project: @project)
         .js-service-desk-setting-root{ data: { endpoint: project_service_desk_path(@project),
diff --git a/ee/changelogs/unreleased/proj-settings-ok-service-desk-settings-only.yml b/ee/changelogs/unreleased/proj-settings-ok-service-desk-settings-only.yml
new file mode 100644
index 00000000000000..db9274bd8e4cb8
--- /dev/null
+++ b/ee/changelogs/unreleased/proj-settings-ok-service-desk-settings-only.yml
@@ -0,0 +1,5 @@
+---
+title: Improve project service desk settings
+merge_request: 10381
+author:
+type: other
diff --git a/ee/spec/features/projects/settings/service_desk_setting_spec.rb b/ee/spec/features/projects/settings/service_desk_setting_spec.rb
index b87253c6145d2c..cc7d6dca35cae2 100644
--- a/ee/spec/features/projects/settings/service_desk_setting_spec.rb
+++ b/ee/spec/features/projects/settings/service_desk_setting_spec.rb
@@ -18,15 +18,15 @@
   end
 
   it 'shows activation checkbox' do
-    expect(page).to have_selector("#service-desk-enabled-checkbox")
+    expect(page).to have_selector("#service-desk-checkbox")
   end
 
   it 'shows incoming email after activating' do
-    find("#service-desk-enabled-checkbox").click
+    find("#service-desk-checkbox").click
     wait_for_requests
     project.reload
     expect(project.service_desk_enabled).to be_truthy
     expect(project.service_desk_address).to be_present
-    expect(find('.js-service-desk-setting-wrapper .card-body')).to have_content(project.service_desk_address)
+    expect(find('.incoming-email').value).to eq(project.service_desk_address)
   end
 end
diff --git a/ee/spec/javascripts/projects/settings_service_desk/components/service_desk_setting_spec.js b/ee/spec/javascripts/projects/settings_service_desk/components/service_desk_setting_spec.js
index b82c514e8dd415..4612e414abed51 100644
--- a/ee/spec/javascripts/projects/settings_service_desk/components/service_desk_setting_spec.js
+++ b/ee/spec/javascripts/projects/settings_service_desk/components/service_desk_setting_spec.js
@@ -28,7 +28,9 @@ describe('ServiceDeskSetting', () => {
         });
 
         it('should see activation checkbox (not disabled)', () => {
-          expect(vm.$refs['enabled-checkbox'].getAttribute('disabled')).toEqual(null);
+          const checkbox = vm.$refs['service-desk-checkbox'].$el;
+
+          expect(checkbox.querySelector('.project-feature-toggle:not(.is-checked)')).toEqual(null);
         });
 
         it('should see main panel with the email info', () => {
@@ -56,29 +58,17 @@ describe('ServiceDeskSetting', () => {
       });
 
       it('should see email', () => {
-        expect(vm.$refs['service-desk-incoming-email'].textContent.trim()).toEqual(incomingEmail);
+        expect(vm.$refs['service-desk-incoming-email'].value.trim()).toEqual(incomingEmail);
         expect(vm.$el.querySelector('.fa-spinner')).toBeNull();
         expect(vm.$el.querySelector('.fa-exclamation-circle')).toBeNull();
       });
 
       it('renders a copy to clipboard button', () => {
-        const button = vm.$el.querySelector('.btn-clipboard');
+        const button = vm.$el.querySelector('.qa-clipboard-button');
 
         expect(button).not.toBe(null);
         expect(button.dataset.clipboardText).toBe(incomingEmail);
       });
-
-      it('renders a help question icon which links to gitlab docs', () => {
-        const link = vm.$el.querySelector('.card-body a');
-
-        expect(link).not.toBeNull();
-
-        expect(link.href).toContain('docs.gitlab.com');
-
-        const icon = link.querySelector('i.fa-question-circle');
-
-        expect(icon).not.toBeNull();
-      });
     });
   });
 
@@ -121,22 +111,14 @@ describe('ServiceDeskSetting', () => {
 
       it('when getting checked', () => {
         expect(onCheckboxToggleSpy).not.toHaveBeenCalled();
-        vm.onCheckboxToggle({
-          target: {
-            checked: true,
-          },
-        });
+        vm.onCheckboxToggle(true);
 
         expect(onCheckboxToggleSpy).toHaveBeenCalledWith(true);
       });
 
       it('when getting unchecked', () => {
         expect(onCheckboxToggleSpy).not.toHaveBeenCalled();
-        vm.onCheckboxToggle({
-          target: {
-            checked: false,
-          },
-        });
+        vm.onCheckboxToggle(false);
 
         expect(onCheckboxToggleSpy).toHaveBeenCalledWith(false);
       });
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 74ea44f9dbd81b..4a326d7ddaa743 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -485,6 +485,9 @@ msgstr ""
 msgid "Action to take when receiving an alert."
 msgstr ""
 
+msgid "Activate Service Desk"
+msgstr ""
+
 msgid "Active"
 msgstr ""
 
@@ -880,12 +883,18 @@ msgstr ""
 msgid "An error occurred while detecting host keys"
 msgstr ""
 
+msgid "An error occurred while disabling Service Desk."
+msgstr ""
+
 msgid "An error occurred while dismissing the alert. Refresh the page and try again."
 msgstr ""
 
 msgid "An error occurred while dismissing the feature highlight. Refresh the page and try dismissing again."
 msgstr ""
 
+msgid "An error occurred while enabling Service Desk."
+msgstr ""
+
 msgid "An error occurred while fetching markdown preview"
 msgstr ""
 
@@ -898,6 +907,9 @@ msgstr ""
 msgid "An error occurred while fetching stages."
 msgstr ""
 
+msgid "An error occurred while fetching the Service Desk address."
+msgstr ""
+
 msgid "An error occurred while fetching the job log."
 msgstr ""
 
@@ -2996,9 +3008,6 @@ msgstr ""
 msgid "Copy file path to clipboard"
 msgstr ""
 
-msgid "Copy incoming email address to clipboard"
-msgstr ""
-
 msgid "Copy link"
 msgstr ""
 
@@ -3784,6 +3793,9 @@ msgstr ""
 msgid "Enable usage ping to get an overview of how you are using GitLab from a feature perspective."
 msgstr ""
 
+msgid "Enable/disable your service desk. %{link_start}Learn more about service desk%{link_end}."
+msgstr ""
+
 msgid "Enabled"
 msgstr ""
 
@@ -4438,6 +4450,9 @@ msgstr ""
 msgid "February"
 msgstr ""
 
+msgid "Fetching incoming email"
+msgstr ""
+
 msgid "Fields on this page are now uneditable, you can configure"
 msgstr ""
 
@@ -4611,6 +4626,9 @@ msgstr ""
 msgid "Format"
 msgstr ""
 
+msgid "Forward external support email address to"
+msgstr ""
+
 msgid "Found errors in your %{gitlab_ci_yml}:"
 msgstr ""
 
@@ -6147,9 +6165,6 @@ msgstr ""
 msgid "Learn more about group-level project templates"
 msgstr ""
 
-msgid "Learn more about incoming email addresses"
-msgstr ""
-
 msgid "Learn more about signing commits"
 msgstr ""
 
@@ -8763,6 +8778,9 @@ msgstr ""
 msgid "Response"
 msgstr ""
 
+msgid "Response didn't include `service_desk_address`"
+msgstr ""
+
 msgid "Response metrics (AWS ELB)"
 msgstr ""
 
-- 
GitLab