Merge branch 'tomquirk/339428-move-add-namespace-button-part-3' into 'master'

Move "Add Namespace" button to GlEmptyState

See merge request gitlab-org/gitlab!73714

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';
 import AddNamespaceButton from '~/jira_connect/subscriptions/components/add_namespace_button.vue';
 import SignInButton from '~/jira_connect/subscriptions/components/sign_in_button.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 { __ } from '~/locale';
+import { mockSubscription } from '../mock_data';
 
 jest.mock('~/jira_connect/subscriptions/utils', () => ({
   retrieveAlert: jest.fn().mockReturnValue({ message: 'error message' }),
@@ -20,6 +22,8 @@ describe('JiraConnectApp', () => {
   const findAlertLink = () => findAlert().findComponent(GlLink);
   const findSignInButton = () => wrapper.findComponent(SignInButton);
   const findAddNamespaceButton = () => wrapper.findComponent(AddNamespaceButton);
+  const findSubscriptionsList = () => wrapper.findComponent(SubscriptionsList);
+  const findEmptyState = () => wrapper.findComponent(GlEmptyState);
 
   const createComponent = ({ provide, mountFn = shallowMount } = {}) => {
     store = createStore();
@@ -36,91 +40,114 @@ describe('JiraConnectApp', () => {
 
   describe('template', () => {
     describe.each`
-      scenario                   | usersPath    | expectSignInButton | expectNamespaceButton
-      ${'user is not signed in'} | ${'/users'}  | ${true}            | ${false}
-      ${'user is signed in'}     | ${undefined} | ${false}           | ${true}
-    `('when $scenario', ({ usersPath, expectSignInButton, expectNamespaceButton }) => {
-      beforeEach(() => {
-        createComponent({
-          provide: {
-            usersPath,
-          },
+      scenario                                         | usersPath    | subscriptions         | expectSignInButton | expectEmptyState | expectNamespaceButton | expectSubscriptionsList
+      ${'user is not signed in with subscriptions'}    | ${'/users'}  | ${[mockSubscription]} | ${true}            | ${false}         | ${false}              | ${true}
+      ${'user is not signed in without subscriptions'} | ${'/users'}  | ${undefined}          | ${true}            | ${false}         | ${false}              | ${false}
+      ${'user is signed in with subscriptions'}        | ${undefined} | ${[mockSubscription]} | ${false}           | ${false}         | ${true}               | ${true}
+      ${'user is signed in without subscriptions'}     | ${undefined} | ${undefined}          | ${false}           | ${true}          | ${false}              | ${false}
+    `(
+      'when $scenario',
+      ({
+        usersPath,
+        expectSignInButton,
+        subscriptions,
+        expectEmptyState,
+        expectNamespaceButton,
+        expectSubscriptionsList,
+      }) => {
+        beforeEach(() => {
+          createComponent({
+            provide: {
+              usersPath,
+              subscriptions,
+            },
+          });
         });
-      });
 
-      it('renders sign in button as expected', () => {
-        expect(findSignInButton().exists()).toBe(expectSignInButton);
-      });
+        it(`${expectSignInButton ? 'renders' : 'does not render'} sign in button`, () => {
+          expect(findSignInButton().exists()).toBe(expectSignInButton);
+        });
 
-      it('renders "Add Namespace" button as expected', () => {
-        expect(findAddNamespaceButton().exists()).toBe(expectNamespaceButton);
-      });
-    });
+        it(`${expectEmptyState ? 'renders' : 'does not render'} empty state`, () => {
+          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, 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(`${
+          expectNamespaceButton ? 'renders' : 'does not render'
+        } button to add namespace`, () => {
+          expect(findAddNamespaceButton().exists()).toBe(expectNamespaceButton);
+        });
+
+        it(`${expectSubscriptionsList ? 'renders' : 'does not render'} subscriptions list`, () => {
+          expect(findSubscriptionsList().exists()).toBe(expectSubscriptionsList);
+        });
+      },
+    );
+  });
+
+  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();
+
+      findAlert().vm.$emit('dismiss');
+      await wrapper.vm.$nextTick();
+
+      expect(findAlert().exists()).toBe(false);
+    });
 
-        const alertLink = findAlertLink();
+    it('renders link when `linkUrl` is set', async () => {
+      createComponent({ mountFn: mount });
 
-        expect(alertLink.exists()).toBe(true);
-        expect(alertLink.text()).toContain('test link');
-        expect(alertLink.attributes('href')).toBe('https://gitlab.com');
+      store.commit(SET_ALERT, {
+        message: __('test message %{linkStart}test link%{linkEnd}'),
+        linkUrl: 'https://gitlab.com',
       });
+      await wrapper.vm.$nextTick();
 
-      describe('when alert is set in localStoage', () => {
-        it('renders alert on mount', () => {
-          createComponent();
+      const alertLink = findAlertLink();
 
-          const alert = findAlert();
+      expect(alertLink.exists()).toBe(true);
+      expect(alertLink.text()).toContain('test link');
+      expect(alertLink.attributes('href')).toBe('https://gitlab.com');
+    });
 
-          expect(alert.exists()).toBe(true);
-          expect(alert.html()).toContain('error message');
-        });
+    describe('when alert is set in localStoage', () => {
+      it('renders alert on mount', () => {
+        createComponent();
+
+        const alert = findAlert();
+
+        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();
     });
 

spec/views/jira_connect/subscriptions/index.html.haml_spec.rb

@@ -7,7 +7,7 @@ RSpec.describe 'jira_connect/subscriptions/index.html.haml' do
 
   before do
     allow(view).to receive(:current_user).and_return(user)
-    assign(:subscriptions, [])
+    assign(:subscriptions, create_list(:jira_connect_subscription, 1))
   end
 
   context 'when the user is signed in' do