Merge branch '263402-enable-review-app-button-opens-twice-the-popup' into 'master'

Avoid opening 2 modals for enabling review app

See merge request gitlab-org/gitlab!45361

app/assets/javascripts/environments/components/enable_review_app_button.vue → app/assets/javascripts/environments/components/enable_review_app_modal.vue

 <script>
-import { GlButton, GlModal, GlModalDirective, GlLink, GlSprintf } from '@gitlab/ui';
+import { GlLink, GlModal, GlSprintf } from '@gitlab/ui';
 import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue';
 import { s__ } from '~/locale';
 
 export default {
   components: {
-    GlButton,
     GlLink,
     GlModal,
     GlSprintf,
     ModalCopyButton,
   },
-  directives: {
-    'gl-modal': GlModalDirective,
+  props: {
+    modalId: {
+      type: String,
+      required: true,
+    },
   },
   instructionText: {
     step1: s__(
@@ -37,27 +39,15 @@ export default {
     - branches
   except:
     - master`,
-    id: 'enable-review-app-info',
     title: s__('ReviewApp|Enable Review App'),
   },
 };
 </script>
 <template>
-  <div>
-    <gl-button
-      v-gl-modal="$options.modalInfo.id"
-      variant="info"
-      category="secondary"
-      type="button"
-      class="gl-w-full js-enable-review-app-button"
-    >
-      {{ s__('Environments|Enable review app') }}
-    </gl-button>
   <gl-modal
-      :modal-id="$options.modalInfo.id"
+    :modal-id="modalId"
     :title="$options.modalInfo.title"
     size="lg"
-      class="text-2 ws-normal"
     ok-only
     ok-variant="light"
     :ok-title="$options.modalInfo.closeText"
@@ -84,12 +74,12 @@ export default {
           </template>
         </gl-sprintf>
       </p>
-        <div class="flex align-items-start">
-          <pre class="w-100"> {{ $options.modalInfo.copyString }} </pre>
+      <div class="gl-display-flex align-items-start">
+        <pre class="gl-w-full"> {{ $options.modalInfo.copyString }} </pre>
         <modal-copy-button
           :title="$options.modalInfo.copyToClipboardText"
           :text="$options.modalInfo.copyString"
-            :modal-id="$options.modalInfo.id"
+          :modal-id="modalId"
           css-classes="border-0"
         />
       </div>
@@ -105,5 +95,4 @@ export default {
       </gl-sprintf>
     </p>
   </gl-modal>
-  </div>
 </template>

app/assets/javascripts/environments/components/environments_app.vue

 <script>
-import { GlBadge, GlButton, GlTab, GlTabs } from '@gitlab/ui';
+import { GlBadge, GlButton, GlModalDirective, GlTab, GlTabs } from '@gitlab/ui';
 import { deprecatedCreateFlash as Flash } from '~/flash';
 import { s__ } from '~/locale';
 import emptyState from './empty_state.vue';
 import eventHub from '../event_hub';
 import environmentsMixin from '../mixins/environments_mixin';
 import CIPaginationMixin from '~/vue_shared/mixins/ci_pagination_api_mixin';
-import EnableReviewAppButton from './enable_review_app_button.vue';
+import EnableReviewAppModal from './enable_review_app_modal.vue';
 import StopEnvironmentModal from './stop_environment_modal.vue';
 import DeleteEnvironmentModal from './delete_environment_modal.vue';
 import ConfirmRollbackModal from './confirm_rollback_modal.vue';
 
 export default {
+  i18n: {
+    newEnvironmentButtonLabel: s__('Environments|New environment'),
+    reviewAppButtonLabel: s__('Environments|Enable review app'),
+  },
+  modal: {
+    id: 'enable-review-app-info',
+  },
   components: {
     ConfirmRollbackModal,
     emptyState,
-    EnableReviewAppButton,
+    EnableReviewAppModal,
     GlBadge,
     GlButton,
     GlTab,
@@ -23,9 +30,10 @@ export default {
     StopEnvironmentModal,
     DeleteEnvironmentModal,
   },
-
+  directives: {
+    'gl-modal': GlModalDirective,
+  },
   mixins: [CIPaginationMixin, environmentsMixin],
-
   props: {
     endpoint: {
       type: String,
@@ -140,17 +148,25 @@ export default {
         gl-mt-3
         gl-display-md-none!"
       >
-        <enable-review-app-button
+        <gl-button
           v-if="state.reviewAppDetails.can_setup_review_app"
+          v-gl-modal="$options.modal.id"
+          data-testid="enable-review-app"
+          variant="info"
+          category="secondary"
+          type="button"
           class="gl-mb-3 gl-flex-fill-1"
-        />
+        >
+          {{ $options.i18n.reviewAppButtonLabel }}
+        </gl-button>
         <gl-button
           v-if="canCreateEnvironment"
           :href="newEnvironmentPath"
+          data-testid="new-environment"
           category="primary"
           variant="success"
         >
-          {{ s__('Environments|New environment') }}
+          {{ $options.i18n.newEnvironmentButtonLabel }}
         </gl-button>
       </div>
       <gl-tabs content-class="gl-display-none">
@@ -176,17 +192,25 @@ export default {
             gl-lg-justify-content-end
             gl-lg-mt-0"
           >
-            <enable-review-app-button
+            <gl-button
               v-if="state.reviewAppDetails.can_setup_review_app"
+              v-gl-modal="$options.modal.id"
+              data-testid="enable-review-app"
+              variant="info"
+              category="secondary"
+              type="button"
               class="gl-mb-3 gl-lg-mr-3 gl-lg-mb-0"
-            />
+            >
+              {{ $options.i18n.reviewAppButtonLabel }}
+            </gl-button>
             <gl-button
               v-if="canCreateEnvironment"
               :href="newEnvironmentPath"
+              data-testid="new-environment"
               category="primary"
               variant="success"
             >
-              {{ s__('Environments|New environment') }}
+              {{ $options.i18n.newEnvironmentButtonLabel }}
             </gl-button>
           </div>
         </template>
@@ -208,6 +232,11 @@ export default {
           <empty-state :help-path="helpPagePath" />
         </template>
       </container>
+      <enable-review-app-modal
+        v-if="state.reviewAppDetails.can_setup_review_app"
+        :modal-id="$options.modal.id"
+        data-testid="enable-review-app-modal"
+      />
     </div>
   </div>
 </template>

changelogs/unreleased/263402-enable-review-app-button-opens-twice-the-popup.yml

+---
+title: Avooid opening 2 modals for enabling review app
+merge_request: 45361
+author:
+type: fixed

spec/frontend/environments/enable_review_app_button_spec.js → spec/frontend/environments/enable_review_app_modal_spec.js

-import { shallowMount, mount } from '@vue/test-utils';
+import { shallowMount } from '@vue/test-utils';
 import ModalCopyButton from '~/vue_shared/components/modal_copy_button.vue';
-import EnableReviewAppButton from '~/environments/components/enable_review_app_button.vue';
+import EnableReviewAppButton from '~/environments/components/enable_review_app_modal.vue';
 
 describe('Enable Review App Button', () => {
   let wrapper;
@@ -9,19 +9,13 @@ describe('Enable Review App Button', () => {
     wrapper.destroy();
   });
 
-  describe('renders button with text', () => {
-    beforeEach(() => {
-      wrapper = mount(EnableReviewAppButton);
-    });
-
-    it('renders Enable Review text', () => {
-      expect(wrapper.text()).toBe('Enable review app');
-    });
-  });
-
   describe('renders the modal', () => {
     beforeEach(() => {
-      wrapper = shallowMount(EnableReviewAppButton);
+      wrapper = shallowMount(EnableReviewAppButton, {
+        propsData: {
+          modalId: 'fake-id',
+        },
+      });
     });
 
     it('renders the copyToClipboard button', () => {

spec/frontend/environments/environments_app_spec.js

-import { GlButton } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import MockAdapter from 'axios-mock-adapter';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
+import EnableReviewAppModal from '~/environments/components/enable_review_app_modal.vue';
 import Container from '~/environments/components/container.vue';
 import EmptyState from '~/environments/components/empty_state.vue';
 import EnvironmentsApp from '~/environments/components/environments_app.vue';
@@ -35,13 +36,15 @@ describe('Environment', () => {
     });
   };
 
-  const createWrapper = (shallow = false) => {
+  const createWrapper = (shallow = false, props = {}) => {
     const fn = shallow ? shallowMount : mount;
-    wrapper = fn(EnvironmentsApp, { propsData: mockData });
+    wrapper = extendedWrapper(fn(EnvironmentsApp, { propsData: { ...mockData, ...props } }));
     return axios.waitForAll();
   };
 
-  const findNewEnvironmentButton = () => wrapper.find(GlButton);
+  const findEnableReviewAppButton = () => wrapper.findByTestId('enable-review-app');
+  const findEnableReviewAppModal = () => wrapper.findAll(EnableReviewAppModal);
+  const findNewEnvironmentButton = () => wrapper.findByTestId('new-environment');
   const findEnvironmentsTabAvailable = () => wrapper.find('.js-environments-tab-available > a');
   const findEnvironmentsTabStopped = () => wrapper.find('.js-environments-tab-stopped > a');
 
@@ -64,19 +67,6 @@ describe('Environment', () => {
       it('should render the empty state', () => {
         expect(wrapper.find(EmptyState).exists()).toBe(true);
       });
-
-      describe('when it is possible to enable a review app', () => {
-        beforeEach(() => {
-          mockRequest(200, { environments: [], review_app: { can_setup_review_app: true } });
-          return createWrapper();
-        });
-
-        it('should render the enable review app button', () => {
-          expect(wrapper.find('.js-enable-review-app-button').text()).toContain(
-            'Enable review app',
-          );
-        });
-      });
     });
 
     describe('with paginated environments', () => {
@@ -91,7 +81,7 @@ describe('Environment', () => {
         return createWrapper();
       });
 
-      it('should render a conatiner table with environments', () => {
+      it('should render a container table with environments', () => {
         const containerTable = wrapper.find(Container);
 
         expect(containerTable.exists()).toBe(true);
@@ -181,8 +171,8 @@ describe('Environment', () => {
   describe('environment button', () => {
     describe('when user can create environment', () => {
       beforeEach(() => {
-        mockRequest([environment]);
-        wrapper = shallowMount(EnvironmentsApp, { propsData: mockData });
+        mockRequest(200, { environments: [] });
+        return createWrapper(true);
       });
 
       it('should render', () => {
@@ -192,10 +182,8 @@ describe('Environment', () => {
 
     describe('when user can not create environment', () => {
       beforeEach(() => {
-        mockRequest([environment]);
-        wrapper = shallowMount(EnvironmentsApp, {
-          propsData: { ...mockData, canCreateEnvironment: false },
-        });
+        mockRequest(200, { environments: [] });
+        return createWrapper(true, { ...mockData, canCreateEnvironment: false });
       });
 
       it('should not render', () => {
@@ -203,4 +191,41 @@ describe('Environment', () => {
       });
     });
   });
+
+  describe('review app modal', () => {
+    describe('when it is not possible to enable a review app', () => {
+      beforeEach(() => {
+        mockRequest(200, { environments: [] });
+        return createWrapper(true);
+      });
+
+      it('should not render the enable review app button', () => {
+        expect(findEnableReviewAppButton().exists()).toBe(false);
+      });
+
+      it('should not render a review app modal', () => {
+        const modal = findEnableReviewAppModal();
+        expect(modal).toHaveLength(0);
+        expect(modal.exists()).toBe(false);
+      });
+    });
+
+    describe('when it is possible to enable a review app', () => {
+      beforeEach(() => {
+        mockRequest(200, { environments: [], review_app: { can_setup_review_app: true } });
+        return createWrapper(true);
+      });
+
+      it('should render the enable review app button', () => {
+        expect(findEnableReviewAppButton().exists()).toBe(true);
+        expect(findEnableReviewAppButton().text()).toContain('Enable review app');
+      });
+
+      it('should render only one review app modal', () => {
+        const modal = findEnableReviewAppModal();
+        expect(modal).toHaveLength(1);
+        expect(modal.at(0).exists()).toBe(true);
+      });
+    });
+  });
 });