Merge branch 'afontaine/update-rspec-environments' into 'master'

Remove new environments table feature flag

See merge request gitlab-org/gitlab!80948

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

@@ -19,6 +19,10 @@ export default {
       type: Object,
       required: true,
     },
+    scope: {
+      type: String,
+      required: true,
+    },
   },
   data() {
     return { visible: false, interval: undefined };
@@ -27,7 +31,7 @@ export default {
     folder: {
       query: folderQuery,
       variables() {
-        return { environment: this.nestedEnvironment.latest };
+        return { environment: this.nestedEnvironment.latest, scope: this.scope };
       },
       pollInterval() {
         return this.interval;
@@ -52,7 +56,7 @@ export default {
       return this.visible ? this.$options.i18n.collapse : this.$options.i18n.expand;
     },
     count() {
-      return this.folder?.availableCount ?? 0;
+      return this.folder?.[`${this.scope}Count`] ?? 0;
     },
     folderClass() {
       return { 'gl-font-weight-bold': this.visible };

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

@@ -302,7 +302,11 @@ export default {
             class="gl-pl-4"
           />
         </div>
-        <div v-if="upcomingDeployment" :class="$options.deploymentClasses">
+        <div
+          v-if="upcomingDeployment"
+          :class="$options.deploymentClasses"
+          data-testid="upcoming-deployment-content"
+        >
           <deployment
             :deployment="upcomingDeployment"
             :class="{ 'gl-ml-7': inFolder }"

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

@@ -16,12 +16,14 @@ import EnvironmentItem from './new_environment_item.vue';
 import ConfirmRollbackModal from './confirm_rollback_modal.vue';
 import DeleteEnvironmentModal from './delete_environment_modal.vue';
 import CanaryUpdateModal from './canary_update_modal.vue';
+import EmptyState from './empty_state.vue';
 
 export default {
   components: {
     DeleteEnvironmentModal,
     CanaryUpdateModal,
     ConfirmRollbackModal,
+    EmptyState,
     EnvironmentFolder,
     EnableReviewAppModal,
     EnvironmentItem,
@@ -66,7 +68,7 @@ export default {
       query: environmentToChangeCanaryQuery,
     },
   },
-  inject: ['newEnvironmentPath', 'canCreateEnvironment'],
+  inject: ['newEnvironmentPath', 'canCreateEnvironment', 'helpPagePath'],
   i18n: {
     newEnvironmentButtonLabel: s__('Environments|New environment'),
     reviewAppButtonLabel: s__('Environments|Enable review app'),
@@ -103,6 +105,9 @@ export default {
     environments() {
       return this.environmentApp?.environments?.filter((e) => e.size === 1) ?? [];
     },
+    hasEnvironments() {
+      return this.environments.length > 0 || this.folders.length > 0;
+    },
     availableCount() {
       return this.environmentApp?.availableCount;
     },
@@ -221,19 +226,23 @@ export default {
         </template>
       </gl-tab>
     </gl-tabs>
-    <environment-folder
-      v-for="folder in folders"
-      :key="folder.name"
-      class="gl-mb-3"
-      :nested-environment="folder"
-    />
-    <environment-item
-      v-for="environment in environments"
-      :key="environment.name"
-      class="gl-mb-3 gl-border-gray-100 gl-border-1 gl-border-b-solid"
-      :environment="environment.latest"
-      @change="resetPolling"
-    />
+    <template v-if="hasEnvironments">
+      <environment-folder
+        v-for="folder in folders"
+        :key="folder.name"
+        class="gl-mb-3"
+        :scope="scope"
+        :nested-environment="folder"
+      />
+      <environment-item
+        v-for="environment in environments"
+        :key="environment.name"
+        class="gl-mb-3 gl-border-gray-100 gl-border-1 gl-border-b-solid"
+        :environment="environment.latest"
+        @change="resetPolling"
+      />
+    </template>
+    <empty-state v-else :help-path="helpPagePath" />
     <gl-pagination
       align="center"
       :total-items="totalItems"

app/assets/javascripts/environments/graphql/queries/folder.query.graphql

-query getEnvironmentFolder($environment: NestedLocalEnvironment) {
-  folder(environment: $environment) @client {
+query getEnvironmentFolder($environment: NestedLocalEnvironment, $scope: String) {
+  folder(environment: $environment, scope: $scope) @client {
     availableCount
     environments
     stoppedCount

app/assets/javascripts/environments/graphql/resolvers.js

@@ -59,8 +59,8 @@ export const resolvers = (endpoint) => ({
         };
       });
     },
-    folder(_, { environment: { folderPath } }) {
-      return axios.get(folderPath, { params: { per_page: 3 } }).then((res) => ({
+    folder(_, { environment: { folderPath }, scope }) {
+      return axios.get(folderPath, { params: { scope, per_page: 3 } }).then((res) => ({
         availableCount: res.data.available_count,
         environments: res.data.environments.map(mapEnvironment),
         stoppedCount: res.data.stopped_count,

app/views/projects/environments/index.html.haml

 - page_title _("Environments")
 - add_page_specific_style 'page_bundles/environments'
 
-- if Feature.enabled?(:new_environments_table)
-  #environments-table{ data: { endpoint: project_environments_path(@project, format: :json),
-    "can-read-environment" => can?(current_user, :read_environment, @project).to_s,
-    "can-create-environment" => can?(current_user, :create_environment, @project).to_s,
-    "new-environment-path" => new_project_environment_path(@project),
-    "help-page-path" => help_page_path("ci/environments/index.md"),
-    "project-path" => @project.full_path,
-    "project-id" => @project.id,
-    "default-branch-name" => @project.default_branch_or_main } }
-- else
-  #environments-list-view{ data: { environments_data: environments_list_data,
-    "can-read-environment" => can?(current_user, :read_environment, @project).to_s,
-    "can-create-environment" => can?(current_user, :create_environment, @project).to_s,
-    "new-environment-path" => new_project_environment_path(@project),
-    "help-page-path" => help_page_path("ci/environments/index.md"),
-    "project-path" => @project.full_path,
-    "project-id" => @project.id,
-    "default-branch-name" => @project.default_branch_or_main } }
+#environments-table{ data: { endpoint: project_environments_path(@project, format: :json),
+  "can-read-environment" => can?(current_user, :read_environment, @project).to_s,
+  "can-create-environment" => can?(current_user, :create_environment, @project).to_s,
+  "new-environment-path" => new_project_environment_path(@project),
+  "help-page-path" => help_page_path("ci/environments/index.md"),
+  "project-path" => @project.full_path,
+  "project-id" => @project.id,
+  "default-branch-name" => @project.default_branch_or_main } }

config/feature_flags/development/new_environments_table.yml

----
-name: new_environments_table
-introduced_by_url:
-rollout_issue_url:
-milestone: '14.4'
-type: development
-group: group::release
-default_enabled: false

ee/spec/features/projects/environments/environments_spec.rb

@@ -216,6 +216,8 @@ RSpec.describe 'Environments page', :js do
     it 'shows the open alert for the environment row' do
       visit project_environments_path(project)
 
+      page.click_button _('Expand')
+
       within(find('div[data-testid="alert"]')) do
         expect(page).to have_content('Critical')
         expect(page).to have_content('HTTP Error Rate exceeded 1.0%')

spec/features/projects/environments/environments_spec.rb

@@ -10,7 +10,6 @@ RSpec.describe 'Environments page', :js do
   let(:role) { :developer }
 
   before do
-    stub_feature_flags(new_environments_table: false)
     project.add_role(user, role)
     sign_in(user)
   end
@@ -35,24 +34,18 @@ RSpec.describe 'Environments page', :js do
     it 'shows "Available" and "Stopped" tab with links' do
       visit_environments(project)
 
-      expect(page).to have_selector('.js-environments-tab-available')
-      expect(page).to have_content('Available')
-      expect(page).to have_selector('.js-environments-tab-stopped')
-      expect(page).to have_content('Stopped')
+      expect(page).to have_link(_('Available'))
+      expect(page).to have_link(_('Stopped'))
     end
 
     describe 'with one available environment' do
-      before do
-        create(:environment, project: project, state: :available)
-      end
+      let!(:environment) { create(:environment, project: project, state: :available) }
 
       describe 'in available tab page' do
         it 'shows one environment' do
           visit_environments(project, scope: 'available')
 
-          expect(page).to have_css('.environments-container')
-          expect(page.all('.environment-name').length).to eq(1)
-          expect(page.all('[data-testid="stop-icon"]').length).to eq(1)
+          expect(page).to have_link(environment.name, href: project_environment_path(project, environment))
         end
       end
 
@@ -77,7 +70,6 @@ RSpec.describe 'Environments page', :js do
         it 'shows no environments' do
           visit_environments(project, scope: 'stopped')
 
-          expect(page).to have_css('.environments-container')
           expect(page).to have_content('You don\'t have any environments right now')
         end
       end
@@ -95,22 +87,18 @@ RSpec.describe 'Environments page', :js do
         it 'shows one environment without error' do
           visit_environments(project, scope: 'available')
 
-          expect(page).to have_css('.environments-container')
-          expect(page.all('.environment-name').length).to eq(1)
+          expect(page).to have_link(environment.name, href: project_environment_path(project, environment))
         end
       end
     end
 
     describe 'with one stopped environment' do
-      before do
-        create(:environment, project: project, state: :stopped)
-      end
+      let!(:environment) { create(:environment, project: project, state: :stopped) }
 
       describe 'in available tab page' do
         it 'shows no environments' do
           visit_environments(project, scope: 'available')
 
-          expect(page).to have_css('.environments-container')
           expect(page).to have_content('You don\'t have any environments right now')
         end
       end
@@ -119,8 +107,7 @@ RSpec.describe 'Environments page', :js do
         it 'shows one environment' do
           visit_environments(project, scope: 'stopped')
 
-          expect(page).to have_css('.environments-container')
-          expect(page.all('.environment-name').length).to eq(1)
+          expect(page).to have_link(environment.name, href: project_environment_path(project, environment))
           expect(page.all('[data-testid="stop-icon"]').length).to eq(0)
         end
       end
@@ -135,8 +122,8 @@ RSpec.describe 'Environments page', :js do
     it 'does not show environments and counters are set to zero' do
       expect(page).to have_content('You don\'t have any environments right now')
 
-      expect(page.find('.js-environments-tab-available .badge').text).to eq('0')
-      expect(page.find('.js-environments-tab-stopped .badge').text).to eq('0')
+      expect(page).to have_link("#{_('Available')} 0")
+      expect(page).to have_link("#{_('Stopped')} 0")
     end
   end
 
@@ -150,21 +137,23 @@ RSpec.describe 'Environments page', :js do
     context 'when there are no deployments' do
       before do
         visit_environments(project)
+
+        page.click_button _('Expand')
       end
 
       it 'shows environments names and counters' do
-        expect(page).to have_link(environment.name)
+        expect(page).to have_link(environment.name, href: project_environment_path(project, environment))
 
-        expect(page.find('.js-environments-tab-available .badge').text).to eq('1')
-        expect(page.find('.js-environments-tab-stopped .badge').text).to eq('0')
+        expect(page).to have_link("#{_('Available')} 1")
+        expect(page).to have_link("#{_('Stopped')} 0")
       end
 
       it 'does not show deployments' do
-        expect(page).to have_content('No deployments yet')
+        expect(page).to have_content(s_('Environments|There are no deployments for this environment yet. Learn more about setting up deployments.'))
       end
 
       it 'shows stop button when environment is not stoppable' do
-        expect(page).to have_selector(stop_button_selector)
+        expect(page).to have_button('Stop')
       end
     end
 
@@ -179,8 +168,10 @@ RSpec.describe 'Environments page', :js do
 
       it 'shows deployment SHA and internal ID' do
         visit_environments(project)
+        page.click_button _('Expand')
 
-        expect(page).to have_link(deployment.short_sha)
+        expect(page).to have_text(deployment.short_sha)
+        expect(page).to have_link(deployment.commit.full_title)
         expect(page).to have_content(deployment.iid)
       end
 
@@ -218,10 +209,6 @@ RSpec.describe 'Environments page', :js do
             .not_to change { Ci::Pipeline.count }
         end
 
-        it 'shows build name and id' do
-          expect(page).to have_link("#{build.name} ##{build.id}")
-        end
-
         it 'shows a stop button' do
           expect(page).to have_selector(stop_button_selector)
         end
@@ -373,7 +360,8 @@ RSpec.describe 'Environments page', :js do
       it 'does not show deployments' do
         visit_environments(project)
 
-        expect(page).to have_content('No deployments yet')
+        page.click_button _('Expand')
+        expect(page).to have_content(s_('Environments|There are no deployments for this environment yet. Learn more about setting up deployments.'))
       end
     end
 
@@ -389,9 +377,10 @@ RSpec.describe 'Environments page', :js do
       it "renders the upcoming deployment", :aggregate_failures do
         visit_environments(project)
 
+        page.click_button _('Expand')
+
         within(upcoming_deployment_content_selector) do
           expect(page).to have_content("##{deployment.iid}")
-          expect(page).to have_selector("a[href=\"#{project_job_path(project, deployment.deployable)}\"]")
           expect(page).to have_link(href: /#{deployment.user.username}/)
         end
       end
@@ -413,15 +402,15 @@ RSpec.describe 'Environments page', :js do
       let(:role) { :developer }
 
       it 'developer creates a new environment with a valid name' do
-        within(".environments-section") { click_link 'New environment' }
+        click_link 'New environment'
         fill_in('Name', with: 'production')
         click_on 'Save'
 
         expect(page).to have_content('production')
       end
 
-      it 'developer creates a new environmetn with invalid name' do
-        within(".environments-section") { click_link 'New environment' }
+      it 'developer creates a new environment with invalid name' do
+        click_link 'New environment'
         fill_in('Name', with: 'name,with,commas')
         click_on 'Save'
 
@@ -458,20 +447,11 @@ RSpec.describe 'Environments page', :js do
         expect(page).not_to have_content 'review-2'
         expect(page).to have_content 'staging 2'
 
-        within('.folder-row') do
-          find('.folder-name', text: 'staging').click
-        end
+        page.click_button _('Expand')
 
         expect(page).to have_content 'review-1'
         expect(page).to have_content 'review-2'
-        within('.ci-table') do
-          within('[data-qa-selector="environment_item"]', text: 'review-1') do # rubocop:disable QA/SelectorUsage
-            expect(find('.js-auto-stop').text).not_to be_empty
-          end
-          within('[data-qa-selector="environment_item"]', text: 'review-2') do # rubocop:disable QA/SelectorUsage
-            expect(find('.js-auto-stop').text).not_to be_empty
-          end
-        end
+        expect(page).to have_content 'Auto stop in'
       end
     end
 
@@ -494,9 +474,7 @@ RSpec.describe 'Environments page', :js do
         expect(page).not_to have_content 'review-2'
         expect(page).to have_content 'staging 2'
 
-        within('.folder-row') do
-          find('.folder-name', text: 'staging').click
-        end
+        page.click_button _('Expand')
 
         expect(page).to have_content 'review-1'
         expect(page).to have_content 'review-2'

spec/frontend/environments/graphql/resolvers_spec.js

@@ -124,10 +124,11 @@ describe('~/frontend/environments/graphql/resolvers', () => {
   });
   describe('folder', () => {
     it('should fetch the folder url passed to it', async () => {
-      mock.onGet(ENDPOINT, { params: { per_page: 3 } }).reply(200, folder);
+      mock.onGet(ENDPOINT, { params: { per_page: 3, scope: 'available' } }).reply(200, folder);
 
       const environmentFolder = await mockResolvers.Query.folder(null, {
         environment: { folderPath: ENDPOINT },
+        scope: 'available',
       });
 
       expect(environmentFolder).toEqual(resolvedFolder);

spec/frontend/environments/new_environment_folder_spec.js

@@ -16,8 +16,6 @@ describe('~/environments/components/new_environments_folder.vue', () => {
   let wrapper;
   let environmentFolderMock;
   let nestedEnvironment;
-  let folderName;
-  let button;
 
   const findLink = () => wrapper.findByRole('link', { name: s__('Environments|Show all') });
 
@@ -30,7 +28,10 @@ describe('~/environments/components/new_environments_folder.vue', () => {
   const createWrapper = (propsData, apolloProvider) =>
     mountExtended(EnvironmentsFolder, {
       apolloProvider,
-      propsData,
+      propsData: {
+        scope: 'available',
+        ...propsData,
+      },
       stubs: { transition: stubTransition() },
       provide: { helpPagePath: '/help' },
     });
@@ -39,62 +40,93 @@ describe('~/environments/components/new_environments_folder.vue', () => {
     environmentFolderMock = jest.fn();
     [nestedEnvironment] = resolvedEnvironmentsApp.environments;
     environmentFolderMock.mockReturnValue(resolvedFolder);
-    wrapper = createWrapper({ nestedEnvironment }, createApolloProvider());
-
-    await nextTick();
-    await waitForPromises();
-    folderName = wrapper.findByText(nestedEnvironment.name);
-    button = wrapper.findByRole('button', { name: __('Expand') });
   });
 
   afterEach(() => {
     wrapper?.destroy();
   });
 
-  it('displays the name of the folder', () => {
-    expect(folderName.text()).toBe(nestedEnvironment.name);
-  });
+  describe('default', () => {
+    let folderName;
+    let button;
 
-  describe('collapse', () => {
-    let icons;
-    let collapse;
+    beforeEach(async () => {
+      wrapper = createWrapper({ nestedEnvironment }, createApolloProvider());
 
-    beforeEach(() => {
-      collapse = wrapper.findComponent(GlCollapse);
-      icons = wrapper.findAllComponents(GlIcon);
+      await nextTick();
+      await waitForPromises();
+      folderName = wrapper.findByText(nestedEnvironment.name);
+      button = wrapper.findByRole('button', { name: __('Expand') });
     });
 
-    it('is collapsed by default', () => {
-      const link = findLink();
-
-      expect(collapse.attributes('visible')).toBeUndefined();
-      const iconNames = icons.wrappers.map((i) => i.props('name')).slice(0, 2);
-      expect(iconNames).toEqual(['angle-right', 'folder-o']);
-      expect(folderName.classes('gl-font-weight-bold')).toBe(false);
-      expect(link.exists()).toBe(false);
+    it('displays the name of the folder', () => {
+      expect(folderName.text()).toBe(nestedEnvironment.name);
     });
 
-    it('opens on click', async () => {
-      await button.trigger('click');
-
-      const link = findLink();
-
-      expect(button.attributes('aria-label')).toBe(__('Collapse'));
-      expect(collapse.attributes('visible')).toBe('visible');
-      const iconNames = icons.wrappers.map((i) => i.props('name')).slice(0, 2);
-      expect(iconNames).toEqual(['angle-down', 'folder-open']);
-      expect(folderName.classes('gl-font-weight-bold')).toBe(true);
-      expect(link.attributes('href')).toBe(nestedEnvironment.latest.folderPath);
+    describe('collapse', () => {
+      let icons;
+      let collapse;
+
+      beforeEach(() => {
+        collapse = wrapper.findComponent(GlCollapse);
+        icons = wrapper.findAllComponents(GlIcon);
+      });
+
+      it('is collapsed by default', () => {
+        const link = findLink();
+
+        expect(collapse.attributes('visible')).toBeUndefined();
+        const iconNames = icons.wrappers.map((i) => i.props('name')).slice(0, 2);
+        expect(iconNames).toEqual(['angle-right', 'folder-o']);
+        expect(folderName.classes('gl-font-weight-bold')).toBe(false);
+        expect(link.exists()).toBe(false);
+      });
+
+      it('opens on click', async () => {
+        await button.trigger('click');
+
+        const link = findLink();
+
+        expect(button.attributes('aria-label')).toBe(__('Collapse'));
+        expect(collapse.attributes('visible')).toBe('visible');
+        const iconNames = icons.wrappers.map((i) => i.props('name')).slice(0, 2);
+        expect(iconNames).toEqual(['angle-down', 'folder-open']);
+        expect(folderName.classes('gl-font-weight-bold')).toBe(true);
+        expect(link.attributes('href')).toBe(nestedEnvironment.latest.folderPath);
+      });
+
+      it('displays all environments when opened', async () => {
+        await button.trigger('click');
+
+        const names = resolvedFolder.environments.map((e) =>
+          expect.stringMatching(e.nameWithoutType),
+        );
+        const environments = wrapper
+          .findAllComponents(EnvironmentItem)
+          .wrappers.map((w) => w.text());
+        expect(environments).toEqual(expect.arrayContaining(names));
+      });
     });
+  });
 
-    it('displays all environments when opened', async () => {
-      await button.trigger('click');
-
-      const names = resolvedFolder.environments.map((e) =>
-        expect.stringMatching(e.nameWithoutType),
+  it.each(['available', 'stopped'])(
+    'with scope=%s, fetches environments with scope',
+    async (scope) => {
+      wrapper = createWrapper({ nestedEnvironment, scope }, createApolloProvider());
+
+      await nextTick();
+      await waitForPromises();
+
+      expect(environmentFolderMock).toHaveBeenCalledTimes(1);
+      expect(environmentFolderMock).toHaveBeenCalledWith(
+        {},
+        {
+          environment: nestedEnvironment.latest,
+          scope,
+        },
+        expect.anything(),
+        expect.anything(),
       );
-      const environments = wrapper.findAllComponents(EnvironmentItem).wrappers.map((w) => w.text());
-      expect(environments).toEqual(expect.arrayContaining(names));
-    });
-  });
+    },
+  );
 });

spec/frontend/environments/new_environments_app_spec.js

@@ -9,6 +9,7 @@ import { sprintf, __, s__ } from '~/locale';
 import EnvironmentsApp from '~/environments/components/new_environments_app.vue';
 import EnvironmentsFolder from '~/environments/components/new_environment_folder.vue';
 import EnvironmentsItem from '~/environments/components/new_environment_item.vue';
+import EmptyState from '~/environments/components/empty_state.vue';
 import StopEnvironmentModal from '~/environments/components/stop_environment_modal.vue';
 import CanaryUpdateModal from '~/environments/components/canary_update_modal.vue';
 import { resolvedEnvironmentsApp, resolvedFolder, resolvedEnvironment } from './graphql/mock_data';
@@ -121,6 +122,14 @@ describe('~/environments/components/new_environments_app.vue', () => {
     expect(text).toContainEqual(expect.stringMatching('production'));
   });
 
+  it('should show an empty state with no environments', async () => {
+    await createWrapperWithMocked({
+      environmentsApp: { ...resolvedEnvironmentsApp, environments: [] },
+    });
+
+    expect(wrapper.findComponent(EmptyState).exists()).toBe(true);
+  });
+
   it('should show a button to create a new environment', async () => {
     await createWrapperWithMocked({
       environmentsApp: resolvedEnvironmentsApp,