Reorganise Jira Connect UI

The commit reorganises the UI logic for the Jira Connect
App. This is part-UI enhancement, part tech-debt cleanup.

To the user, the main change is that the Add Namespace button
is now in the empty state when the user is signed in but has no
subscriptions.

This restructuring also enables us to move some HAML
markup to Vue.

Changelog: changed

app/assets/javascripts/jira_connect/subscriptions/components/app.vue

 <script>
-import { GlAlert, GlLink, GlSprintf } from '@gitlab/ui';
+import { GlAlert, GlLink, GlSprintf, GlEmptyState } from '@gitlab/ui';
+import { isEmpty } from 'lodash';
 import { mapState, mapMutations } from 'vuex';
 import { retrieveAlert } from '~/jira_connect/subscriptions/utils';
 import { SET_ALERT } from '../store/mutation_types';
@@ -13,6 +14,7 @@ export default {
     GlAlert,
     GlLink,
     GlSprintf,
+    GlEmptyState,
     SubscriptionsList,
     AddNamespaceButton,
     SignInButton,
@@ -21,12 +23,18 @@ export default {
     usersPath: {
       default: '',
     },
+    subscriptions: {
+      default: [],
+    },
   },
   computed: {
     ...mapState(['alert']),
     shouldShowAlert() {
       return Boolean(this.alert?.message);
     },
+    hasSubscriptions() {
+      return !isEmpty(this.subscriptions);
+    },
     userSignedIn() {
       return Boolean(!this.usersPath);
     },
@@ -66,15 +74,44 @@ export default {
       </template>
     </gl-alert>
 
-    <h2 class="gl-text-center">{{ s__('JiraService|GitLab for Jira Configuration') }}</h2>
-
-    <div class="jira-connect-app-body gl-my-7 gl-px-5 gl-pb-4">
-      <div class="gl-display-flex gl-justify-content-end">
-        <sign-in-button v-if="!userSignedIn" :users-path="usersPath" />
-        <add-namespace-button v-else />
-      </div>
+    <h2 class="gl-text-center gl-mb-7">{{ s__('JiraService|GitLab for Jira Configuration') }}</h2>
+    <div class="jira-connect-app-body gl-mx-auto gl-px-5 gl-mb-7">
+      <template v-if="hasSubscriptions">
+        <div class="gl-display-flex gl-justify-content-end">
+          <sign-in-button v-if="!userSignedIn" :users-path="usersPath" />
+          <add-namespace-button v-else />
+        </div>
 
-      <subscriptions-list />
+        <subscriptions-list />
+      </template>
+      <template v-else>
+        <div v-if="!userSignedIn" class="gl-text-center">
+          <p class="gl-mb-7">{{ s__('JiraService|Sign in to GitLab.com to get started.') }}</p>
+          <sign-in-button class="gl-mb-7" :users-path="usersPath">
+            {{ __('Sign in to GitLab') }}
+          </sign-in-button>
+          <p>
+            {{
+              s__(
+                'Integrations|Note: this integration only works with accounts on GitLab.com (SaaS).',
+              )
+            }}
+          </p>
+        </div>
+        <gl-empty-state
+          v-else
+          :title="s__('Integrations|No linked namespaces')"
+          :description="
+            s__(
+              'Integrations|Namespaces are the GitLab groups and subgroups you link to this Jira instance.',
+            )
+          "
+        >
+          <template #actions>
+            <add-namespace-button />
+          </template>
+        </gl-empty-state>
+      </template>
     </div>
   </div>
 </template>

app/assets/javascripts/jira_connect/subscriptions/components/subscriptions_list.vue

 <script>
-import { GlButton, GlEmptyState, GlTable } from '@gitlab/ui';
+import { GlButton, GlTable } from '@gitlab/ui';
 import { isEmpty } from 'lodash';
 import { mapMutations } from 'vuex';
 import { removeSubscription } from '~/jira_connect/subscriptions/api';
@@ -12,7 +12,6 @@ import GroupItemName from './group_item_name.vue';
 export default {
   components: {
     GlButton,
-    GlEmptyState,
     GlTable,
     GroupItemName,
     TimeagoTooltip,
@@ -44,17 +43,15 @@ export default {
     },
   ],
   i18n: {
-    emptyTitle: s__('Integrations|No linked namespaces'),
-    emptyDescription: s__(
-      'Integrations|Namespaces are the GitLab groups and subgroups you link to this Jira instance.',
-    ),
     unlinkError: s__('Integrations|Failed to unlink namespace. Please try again.'),
   },
   methods: {
     ...mapMutations({
       setAlert: SET_ALERT,
     }),
-    isEmpty,
+    isUnlinkButtonDisabled(item) {
+      return !isEmpty(item);
+    },
     isLoadingItem(item) {
       return this.loadingItem === item;
     },
@@ -81,29 +78,22 @@ export default {
 </script>
 
 <template>
-  <div>
-    <gl-empty-state
-      v-if="isEmpty(subscriptions)"
-      :title="$options.i18n.emptyTitle"
-      :description="$options.i18n.emptyDescription"
-    />
-    <gl-table v-else :items="subscriptions" :fields="$options.fields">
-      <template #cell(name)="{ item }">
-        <group-item-name :group="item.group" />
-      </template>
-      <template #cell(created_at)="{ item }">
-        <timeago-tooltip :time="item.created_at" />
-      </template>
-      <template #cell(actions)="{ item }">
-        <gl-button
-          :class="unlinkBtnClass(item)"
-          category="secondary"
-          :loading="isLoadingItem(item)"
-          :disabled="!isEmpty(loadingItem)"
-          @click.prevent="onClick(item)"
-          >{{ __('Unlink') }}</gl-button
-        >
-      </template>
-    </gl-table>
-  </div>
+  <gl-table :items="subscriptions" :fields="$options.fields">
+    <template #cell(name)="{ item }">
+      <group-item-name :group="item.group" />
+    </template>
+    <template #cell(created_at)="{ item }">
+      <timeago-tooltip :time="item.created_at" />
+    </template>
+    <template #cell(actions)="{ item }">
+      <gl-button
+        :class="unlinkBtnClass(item)"
+        category="secondary"
+        :loading="isLoadingItem(item)"
+        :disabled="isUnlinkButtonDisabled(loadingItem)"
+        @click.prevent="onClick(item)"
+        >{{ __('Unlink') }}</gl-button
+      >
+    </template>
+  </gl-table>
 </template>

app/assets/stylesheets/page_bundles/jira_connect.scss

@@ -42,8 +42,6 @@ $header-height: 40px;
 
 .jira-connect-app-body {
   max-width: 768px;
-  margin-left: auto;
-  margin-right: auto;
 }
 
 // needed for external_link

app/views/jira_connect/subscriptions/index.html.haml

@@ -9,20 +9,9 @@
     = link_to _('Sign in to GitLab'), jira_connect_users_path, target: '_blank', rel: 'noopener noreferrer', class: 'js-jira-connect-sign-in'
 
 %main.jira-connect-app.gl-px-5.gl-pt-7.gl-mx-auto
-  - if current_user.blank? && @subscriptions.empty?
-    .jira-connect-app-body.gl-px-5.gl-text-center
-      %h2= s_('JiraService|GitLab for Jira Configuration')
-      %p= s_('JiraService|Sign in to GitLab.com to get started.')
+  .js-jira-connect-app{ data: jira_connect_app_data(@subscriptions) }
 
-      .gl-mt-7
-        = external_link _('Sign in to GitLab'), jira_connect_users_path, class: "btn gl-button btn-confirm js-jira-connect-sign-in"
-
-      .gl-mt-7
-        %p= s_('Integrations|Note: this integration only works with accounts on GitLab.com (SaaS).')
-  - else
-    .js-jira-connect-app{ data: jira_connect_app_data(@subscriptions) }
-
-  %p.jira-connect-app-body.gl-px-5.gl-mt-7.gl-font-base.gl-text-center
+  %p.jira-connect-app-body.gl-px-5.gl-font-base.gl-text-center.gl-mx-auto
     %strong= s_('Integrations|Browser limitations')
     - browser_link_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'
     - firefox_link_start = browser_link_start.html_safe % { url: 'https://www.mozilla.org/en-US/firefox/' }

spec/frontend/jira_connect/subscriptions/components/app_spec.js

-import { GlAlert, GlLink } from '@gitlab/ui';
+import { GlAlert, GlLink, GlEmptyState } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 
 import JiraConnectApp from '~/jira_connect/subscriptions/components/app.vue';
@@ -7,6 +7,7 @@ import SignInButton from '~/jira_connect/subscriptions/components/sign_in_button
 import createStore from '~/jira_connect/subscriptions/store';
 import { SET_ALERT } from '~/jira_connect/subscriptions/store/mutation_types';
 import { __ } from '~/locale';
+import { mockSubscription } from '../mock_data';
 
 jest.mock('~/jira_connect/subscriptions/utils', () => ({
   retrieveAlert: jest.fn().mockReturnValue({ message: 'error message' }),
@@ -20,6 +21,7 @@ describe('JiraConnectApp', () => {
   const findAlertLink = () => findAlert().findComponent(GlLink);
   const findSignInButton = () => wrapper.findComponent(SignInButton);
   const findAddNamespaceButton = () => wrapper.findComponent(AddNamespaceButton);
+  const findEmptyState = () => wrapper.findComponent(GlEmptyState);
 
   const createComponent = ({ provide, mountFn = shallowMount } = {}) => {
     store = createStore();
@@ -34,7 +36,7 @@ describe('JiraConnectApp', () => {
     wrapper.destroy();
   });
 
-  describe('template', () => {
+  describe('with subscriptions', () => {
     describe.each`
       scenario                   | usersPath    | expectSignInButton | expectNamespaceButton
       ${'user is not signed in'} | ${'/users'}  | ${true}            | ${false}
@@ -44,6 +46,7 @@ describe('JiraConnectApp', () => {
         createComponent({
           provide: {
             usersPath,
+            subscriptions: [mockSubscription],
           },
         });
       });
@@ -56,71 +59,96 @@ describe('JiraConnectApp', () => {
         expect(findAddNamespaceButton().exists()).toBe(expectNamespaceButton);
       });
     });
+  });
+
+  describe('with no subscriptions', () => {
+    describe.each`
+      scenario                   | usersPath    | expectSignInButton | expectEmptyState
+      ${'user is not signed in'} | ${'/users'}  | ${true}            | ${false}
+      ${'user is signed in'}     | ${undefined} | ${false}           | ${true}
+    `('when $scenario', ({ usersPath, expectSignInButton, expectEmptyState }) => {
+      beforeEach(() => {
+        createComponent({
+          provide: {
+            usersPath,
+            subscriptions: [],
+          },
+        });
+      });
+
+      it('renders sign in button as expected', () => {
+        expect(findSignInButton().exists()).toBe(expectSignInButton);
+      });
 
-    describe('alert', () => {
-      it.each`
-        message          | variant      | alertShouldRender
-        ${'Test error'}  | ${'danger'}  | ${true}
-        ${'Test notice'} | ${'info'}    | ${true}
-        ${''}            | ${undefined} | ${false}
-        ${undefined}     | ${undefined} | ${false}
-      `(
-        'renders correct alert when message is `$message` and variant is `$variant`',
-        async ({ message, alertShouldRender, variant }) => {
-          createComponent();
-
-          store.commit(SET_ALERT, { message, variant });
-          await wrapper.vm.$nextTick();
-
-          const alert = findAlert();
-
-          expect(alert.exists()).toBe(alertShouldRender);
-          if (alertShouldRender) {
-            expect(alert.isVisible()).toBe(alertShouldRender);
-            expect(alert.html()).toContain(message);
-            expect(alert.props('variant')).toBe(variant);
-            expect(findAlertLink().exists()).toBe(false);
-          }
-        },
-      );
-
-      it('hides alert on @dismiss event', async () => {
+      it('renders empty state as expected', () => {
+        expect(findEmptyState().exists()).toBe(expectEmptyState);
+      });
+    });
+  });
+
+  describe('alert', () => {
+    it.each`
+      message          | variant      | alertShouldRender
+      ${'Test error'}  | ${'danger'}  | ${true}
+      ${'Test notice'} | ${'info'}    | ${true}
+      ${''}            | ${undefined} | ${false}
+      ${undefined}     | ${undefined} | ${false}
+    `(
+      'renders correct alert when message is `$message` and variant is `$variant`',
+      async ({ message, alertShouldRender, variant }) => {
         createComponent();
 
-        store.commit(SET_ALERT, { message: 'test message' });
+        store.commit(SET_ALERT, { message, variant });
         await wrapper.vm.$nextTick();
 
-        findAlert().vm.$emit('dismiss');
-        await wrapper.vm.$nextTick();
+        const alert = findAlert();
 
-        expect(findAlert().exists()).toBe(false);
-      });
+        expect(alert.exists()).toBe(alertShouldRender);
+        if (alertShouldRender) {
+          expect(alert.isVisible()).toBe(alertShouldRender);
+          expect(alert.html()).toContain(message);
+          expect(alert.props('variant')).toBe(variant);
+          expect(findAlertLink().exists()).toBe(false);
+        }
+      },
+    );
 
-      it('renders link when `linkUrl` is set', async () => {
-        createComponent({ mountFn: mount });
+    it('hides alert on @dismiss event', async () => {
+      createComponent();
 
-        store.commit(SET_ALERT, {
-          message: __('test message %{linkStart}test link%{linkEnd}'),
-          linkUrl: 'https://gitlab.com',
-        });
-        await wrapper.vm.$nextTick();
+      store.commit(SET_ALERT, { message: 'test message' });
+      await wrapper.vm.$nextTick();
 
-        const alertLink = findAlertLink();
+      findAlert().vm.$emit('dismiss');
+      await wrapper.vm.$nextTick();
 
-        expect(alertLink.exists()).toBe(true);
-        expect(alertLink.text()).toContain('test link');
-        expect(alertLink.attributes('href')).toBe('https://gitlab.com');
+      expect(findAlert().exists()).toBe(false);
+    });
+
+    it('renders link when `linkUrl` is set', async () => {
+      createComponent({ mountFn: mount });
+
+      store.commit(SET_ALERT, {
+        message: __('test message %{linkStart}test link%{linkEnd}'),
+        linkUrl: 'https://gitlab.com',
       });
+      await wrapper.vm.$nextTick();
+
+      const alertLink = findAlertLink();
 
-      describe('when alert is set in localStoage', () => {
-        it('renders alert on mount', () => {
-          createComponent();
+      expect(alertLink.exists()).toBe(true);
+      expect(alertLink.text()).toContain('test link');
+      expect(alertLink.attributes('href')).toBe('https://gitlab.com');
+    });
+
+    describe('when alert is set in localStoage', () => {
+      it('renders alert on mount', () => {
+        createComponent();
 
-          const alert = findAlert();
+        const alert = findAlert();
 
-          expect(alert.exists()).toBe(true);
-          expect(alert.html()).toContain('error message');
-        });
+        expect(alert.exists()).toBe(true);
+        expect(alert.html()).toContain('error message');
       });
     });
   });

spec/frontend/jira_connect/subscriptions/components/subscriptions_list_spec.js

-import { GlButton, GlEmptyState, GlTable } from '@gitlab/ui';
-import { mount, shallowMount } from '@vue/test-utils';
+import { GlButton } from '@gitlab/ui';
+import { mount } from '@vue/test-utils';
 import waitForPromises from 'helpers/wait_for_promises';
 
 import * as JiraConnectApi from '~/jira_connect/subscriptions/api';
+import GroupItemName from '~/jira_connect/subscriptions/components/group_item_name.vue';
+
 import SubscriptionsList from '~/jira_connect/subscriptions/components/subscriptions_list.vue';
 import createStore from '~/jira_connect/subscriptions/store';
 import { SET_ALERT } from '~/jira_connect/subscriptions/store/mutation_types';
 import { reloadPage } from '~/jira_connect/subscriptions/utils';
+import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import { mockSubscription } from '../mock_data';
 
 jest.mock('~/jira_connect/subscriptions/utils');
@@ -15,11 +18,13 @@ describe('SubscriptionsList', () => {
   let wrapper;
   let store;
 
-  const createComponent = ({ mountFn = shallowMount, provide = {} } = {}) => {
+  const createComponent = () => {
     store = createStore();
 
-    wrapper = mountFn(SubscriptionsList, {
-      provide,
+    wrapper = mount(SubscriptionsList, {
+      provide: {
+        subscriptions: [mockSubscription],
+      },
       store,
     });
   };
@@ -28,28 +33,28 @@ describe('SubscriptionsList', () => {
     wrapper.destroy();
   });
 
-  const findGlEmptyState = () => wrapper.findComponent(GlEmptyState);
-  const findGlTable = () => wrapper.findComponent(GlTable);
-  const findUnlinkButton = () => findGlTable().findComponent(GlButton);
+  const findUnlinkButton = () => wrapper.findComponent(GlButton);
   const clickUnlinkButton = () => findUnlinkButton().trigger('click');
 
   describe('template', () => {
-    it('renders GlEmptyState when subscriptions is empty', () => {
+    beforeEach(() => {
       createComponent();
+    });
+
+    it('renders "name" cell correctly', () => {
+      const groupItemNames = wrapper.findAllComponents(GroupItemName);
+      expect(groupItemNames.wrappers).toHaveLength(1);
 
-      expect(findGlEmptyState().exists()).toBe(true);
-      expect(findGlTable().exists()).toBe(false);
+      const item = groupItemNames.at(0);
+      expect(item.props('group')).toBe(mockSubscription.group);
     });
 
-    it('renders GlTable when subscriptions are present', () => {
-      createComponent({
-        provide: {
-          subscriptions: [mockSubscription],
-        },
-      });
+    it('renders "created at" cell correctly', () => {
+      const timeAgoTooltips = wrapper.findAllComponents(TimeagoTooltip);
+      expect(timeAgoTooltips.wrappers).toHaveLength(1);
 
-      expect(findGlEmptyState().exists()).toBe(false);
-      expect(findGlTable().exists()).toBe(true);
+      const item = timeAgoTooltips.at(0);
+      expect(item.props('time')).toBe(mockSubscription.created_at);
     });
   });
 
@@ -57,12 +62,7 @@ describe('SubscriptionsList', () => {
     let removeSubscriptionSpy;
 
     beforeEach(() => {
-      createComponent({
-        mountFn: mount,
-        provide: {
-          subscriptions: [mockSubscription],
-        },
-      });
+      createComponent();
       removeSubscriptionSpy = jest.spyOn(JiraConnectApi, 'removeSubscription').mockResolvedValue();
     });