Merge branch 'remove-graphql-release-page-feature-flag' into 'master'

Remove 3 GraphQL/release-related feature flags [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60390

app/assets/javascripts/releases/components/app_index.vue

@@ -51,12 +51,8 @@ export default {
     }),
     fetchReleases() {
       this.fetchReleasesStoreAction({
-        // these two parameters are only used in "GraphQL mode"
         before: getParameterByName('before'),
         after: getParameterByName('after'),
-
-        // this parameter is only used when in "REST mode"
-        page: getParameterByName('page'),
       });
     },
   },
@@ -73,17 +69,17 @@ export default {
         :aria-describedby="shouldRenderEmptyState && 'releases-description'"
         category="primary"
         variant="success"
-        class="js-new-release-btn"
+        data-testid="new-release-button"
       >
         {{ __('New release') }}
       </gl-button>
     </div>
 
-    <release-skeleton-loader v-if="isLoading" class="js-loading" />
+    <release-skeleton-loader v-if="isLoading" />
 
     <gl-empty-state
       v-else-if="shouldRenderEmptyState"
-      class="js-empty-state"
+      data-testid="empty-state"
       :title="__('Getting started with releases')"
       :svg-path="illustrationPath"
     >
@@ -101,7 +97,7 @@ export default {
       </template>
     </gl-empty-state>
 
-    <div v-else-if="shouldRenderSuccessState" class="js-success-state">
+    <div v-else-if="shouldRenderSuccessState" data-testid="success-state">
       <release-block
         v-for="(release, index) in releases"
         :key="index"

app/assets/javascripts/releases/components/releases_pagination.vue

 <script>
-import { mapGetters } from 'vuex';
 import ReleasesPaginationGraphql from './releases_pagination_graphql.vue';
 import ReleasesPaginationRest from './releases_pagination_rest.vue';
 
@@ -7,7 +6,12 @@ export default {
   name: 'ReleasesPagination',
   components: { ReleasesPaginationGraphql, ReleasesPaginationRest },
   computed: {
-    ...mapGetters(['useGraphQLEndpoint']),
+    // TODO: Remove the ReleasesPaginationRest component since
+    // it is never shown.
+    // https://gitlab.com/gitlab-org/gitlab/-/issues/329267
+    useGraphQLEndpoint() {
+      return true;
+    },
   },
 };
 </script>

app/assets/javascripts/releases/mount_index.js

@@ -15,11 +15,6 @@ export default () => {
       modules: {
         index: createIndexModule(el.dataset),
       },
-      featureFlags: {
-        graphqlReleaseData: Boolean(gon.features?.graphqlReleaseData),
-        graphqlReleasesPage: Boolean(gon.features?.graphqlReleasesPage),
-        graphqlMilestoneStats: Boolean(gon.features?.graphqlMilestoneStats),
-      },
     }),
     render: (h) => h(ReleaseIndexApp),
   });

app/assets/javascripts/releases/stores/getters.js

-/**
- * @returns {Boolean} `true` if all the feature flags
- * required to enable the GraphQL endpoint are enabled
- */
-export const useGraphQLEndpoint = (rootState) => {
-  return Boolean(
-    rootState.featureFlags.graphqlReleaseData &&
-      rootState.featureFlags.graphqlReleasesPage &&
-      rootState.featureFlags.graphqlMilestoneStats,
-  );
-};

app/assets/javascripts/releases/stores/index.js

 import Vuex from 'vuex';
-import * as getters from './getters';
 
 export default ({ modules, featureFlags }) =>
   new Vuex.Store({
     modules,
     state: { featureFlags },
-    getters,
   });

app/assets/javascripts/releases/stores/modules/index/actions.js

-import api from '~/api';
 import { deprecatedCreateFlash as createFlash } from '~/flash';
-import {
-  normalizeHeaders,
-  parseIntPagination,
-  convertObjectPropsToCamelCase,
-} from '~/lib/utils/common_utils';
 import { __ } from '~/locale';
+import { PAGE_SIZE } from '~/releases/constants';
 import allReleasesQuery from '~/releases/queries/all_releases.query.graphql';
-import { PAGE_SIZE } from '../../../constants';
-import { gqClient, convertAllReleasesGraphQLResponse } from '../../../util';
+import { gqClient, convertAllReleasesGraphQLResponse } from '~/releases/util';
 import * as types from './mutation_types';
 
 /**
- * Gets a paginated list of releases from the server
+ * Gets a paginated list of releases from the GraphQL endpoint
  *
  * @param {Object} vuexParams
  * @param {Object} actionParams
- * @param {Number} [actionParams.page] The page number of results to fetch
- * (this parameter is only used when fetching results from the REST API)
  * @param {String} [actionParams.before] A GraphQL cursor. If provided,
- * the items returned will proceed the provided cursor (this parameter is only
- * used when fetching results from the GraphQL API).
+ * the items returned will proceed the provided cursor.
  * @param {String} [actionParams.after] A GraphQL cursor. If provided,
- * the items returned will follow the provided cursor (this parameter is only
- * used when fetching results from the GraphQL API).
+ * the items returned will follow the provided cursor.
  */
-export const fetchReleases = ({ dispatch, rootGetters }, { page = 1, before, after }) => {
-  if (rootGetters.useGraphQLEndpoint) {
-    dispatch('fetchReleasesGraphQl', { before, after });
-  } else {
-    dispatch('fetchReleasesRest', { page });
-  }
-};
-
-/**
- * Gets a paginated list of releases from the GraphQL endpoint
- */
-export const fetchReleasesGraphQl = (
-  { dispatch, commit, state },
-  { before = null, after = null },
-) => {
+export const fetchReleases = ({ dispatch, commit, state }, { before, after }) => {
   commit(types.REQUEST_RELEASES);
 
   const { sort, orderBy } = state.sorting;
@@ -55,7 +31,7 @@ export const fetchReleasesGraphQl = (
     paginationParams = { first: PAGE_SIZE, after };
   } else {
     throw new Error(
-      'Both a `before` and an `after` parameter were provided to fetchReleasesGraphQl. These parameters cannot be used together.',
+      'Both a `before` and an `after` parameter were provided to fetchReleases. These parameters cannot be used together.',
     );
   }
 
@@ -79,28 +55,6 @@ export const fetchReleasesGraphQl = (
     .catch(() => dispatch('receiveReleasesError'));
 };
 
-/**
- * Gets a paginated list of releases from the REST endpoint
- */
-export const fetchReleasesRest = ({ dispatch, commit, state }, { page }) => {
-  commit(types.REQUEST_RELEASES);
-
-  const { sort, orderBy } = state.sorting;
-
-  api
-    .releases(state.projectId, { page, sort, order_by: orderBy })
-    .then(({ data, headers }) => {
-      const restPageInfo = parseIntPagination(normalizeHeaders(headers));
-      const camelCasedReleases = convertObjectPropsToCamelCase(data, { deep: true });
-
-      commit(types.RECEIVE_RELEASES_SUCCESS, {
-        data: camelCasedReleases,
-        restPageInfo,
-      });
-    })
-    .catch(() => dispatch('receiveReleasesError'));
-};
-
 export const receiveReleasesError = ({ commit }) => {
   commit(types.RECEIVE_RELEASES_ERROR);
   createFlash(__('An error occurred while fetching the releases. Please try again.'));

app/controllers/projects/releases_controller.rb

@@ -8,11 +8,6 @@ class Projects::ReleasesController < Projects::ApplicationController
   # We have to check `download_code` permission because detail URL path
   # contains git-tag name.
   before_action :authorize_download_code!, except: [:index]
-  before_action do
-    push_frontend_feature_flag(:graphql_release_data, project, default_enabled: true)
-    push_frontend_feature_flag(:graphql_milestone_stats, project, default_enabled: true)
-    push_frontend_feature_flag(:graphql_releases_page, project, default_enabled: true)
-  end
   before_action :authorize_update_release!, only: %i[edit update]
   before_action :authorize_create_release!, only: :new
 

app/graphql/resolvers/release_resolver.rb

@@ -15,8 +15,6 @@ module Resolvers
     end
 
     def resolve(tag_name:)
-      return unless Feature.enabled?(:graphql_release_data, project, default_enabled: true)
-
       ReleasesFinder.new(
         project,
         current_user,

app/graphql/resolvers/releases_resolver.rb

@@ -23,8 +23,6 @@ module Resolvers
     }.freeze
 
     def resolve(sort:)
-      return unless Feature.enabled?(:graphql_release_data, project, default_enabled: true)
-
       ReleasesFinder.new(
         project,
         current_user,

app/graphql/types/milestone_type.rb

@@ -57,8 +57,6 @@ module Types
           description: 'Milestone statistics.'
 
     def stats
-      return unless Feature.enabled?(:graphql_milestone_stats, milestone.project || milestone.group, default_enabled: true)
-
       milestone
     end
   end

changelogs/unreleased/remove-graphql-release-page-feature-flag.yml

+---
+title: Remove graphql_release_data, graphql_milestone_stats, and graphql_releases_page
+  feature flags
+merge_request: 60390
+author:
+type: other

config/feature_flags/development/graphql_milestone_stats.yml

----
-name: graphql_milestone_stats
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/35066
-rollout_issue_url: 
-milestone: '13.2'
-type: development
-group: group::release
-default_enabled: true

config/feature_flags/development/graphql_release_data.yml

----
-name: graphql_release_data
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/30753
-rollout_issue_url: 
-milestone: '13.0'
-type: development
-group: group::release
-default_enabled: true

config/feature_flags/development/graphql_releases_page.yml

----
-name: graphql_releases_page
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/33095
-rollout_issue_url: 
-milestone: '13.4'
-type: development
-group: group::release
-default_enabled: true

spec/features/projects/releases/user_views_releases_spec.rb

@@ -147,15 +147,5 @@ RSpec.describe 'User views releases', :js do
     end
   end
 
-  context 'when the graphql_releases_page feature flag is enabled' do
-    it_behaves_like 'releases page'
-  end
-
-  context 'when the graphql_releases_page feature flag is disabled' do
-    before do
-      stub_feature_flags(graphql_releases_page: false)
-    end
-
-    it_behaves_like 'releases page'
-  end
+  it_behaves_like 'releases page'
 end

spec/frontend/releases/components/releases_pagination_spec.js

@@ -20,6 +20,10 @@ describe('~/releases/components/releases_pagination.vue', () => {
     wrapper = shallowMount(ReleasesPagination, { store, localVue });
   };
 
+  beforeEach(() => {
+    createComponent(true);
+  });
+
   afterEach(() => {
     wrapper.destroy();
     wrapper = null;
@@ -28,25 +32,8 @@ describe('~/releases/components/releases_pagination.vue', () => {
   const findRestPagination = () => wrapper.find(ReleasesPaginationRest);
   const findGraphQlPagination = () => wrapper.find(ReleasesPaginationGraphql);
 
-  describe('when one of necessary feature flags is disabled', () => {
-    beforeEach(() => {
-      createComponent(false);
-    });
-
-    it('renders the REST pagination component', () => {
-      expect(findRestPagination().exists()).toBe(true);
-      expect(findGraphQlPagination().exists()).toBe(false);
-    });
-  });
-
-  describe('when all the necessary feature flags are enabled', () => {
-    beforeEach(() => {
-      createComponent(true);
-    });
-
-    it('renders the GraphQL pagination component', () => {
-      expect(findGraphQlPagination().exists()).toBe(true);
-      expect(findRestPagination().exists()).toBe(false);
-    });
+  it('renders the GraphQL pagination component', () => {
+    expect(findGraphQlPagination().exists()).toBe(true);
+    expect(findRestPagination().exists()).toBe(false);
   });
 });

spec/frontend/releases/stores/getters_spec.js

-import * as getters from '~/releases/stores/getters';
-
-describe('~/releases/stores/getters.js', () => {
-  it.each`
-    graphqlReleaseData | graphqlReleasesPage | graphqlMilestoneStats | result
-    ${false}           | ${false}            | ${false}              | ${false}
-    ${false}           | ${false}            | ${true}               | ${false}
-    ${false}           | ${true}             | ${false}              | ${false}
-    ${false}           | ${true}             | ${true}               | ${false}
-    ${true}            | ${false}            | ${false}              | ${false}
-    ${true}            | ${false}            | ${true}               | ${false}
-    ${true}            | ${true}             | ${false}              | ${false}
-    ${true}            | ${true}             | ${true}               | ${true}
-  `(
-    'returns $result with feature flag values graphqlReleaseData=$graphqlReleaseData, graphqlReleasesPage=$graphqlReleasesPage, and graphqlMilestoneStats=$graphqlMilestoneStats',
-    ({ result: expectedResult, ...featureFlags }) => {
-      const actualResult = getters.useGraphQLEndpoint({ featureFlags });
-
-      expect(actualResult).toBe(expectedResult);
-    },
-  );
-});

spec/frontend/releases/stores/modules/list/actions_spec.js

 import { cloneDeep } from 'lodash';
 import { getJSONFixture } from 'helpers/fixtures';
 import testAction from 'helpers/vuex_action_helper';
-import api from '~/api';
-import {
-  normalizeHeaders,
-  parseIntPagination,
-  convertObjectPropsToCamelCase,
-} from '~/lib/utils/common_utils';
 import { PAGE_SIZE } from '~/releases/constants';
 import allReleasesQuery from '~/releases/queries/all_releases.query.graphql';
 import {
   fetchReleases,
-  fetchReleasesGraphQl,
-  fetchReleasesRest,
   receiveReleasesError,
   setSorting,
 } from '~/releases/stores/modules/index/actions';
 import * as types from '~/releases/stores/modules/index/mutation_types';
 import createState from '~/releases/stores/modules/index/state';
 import { gqClient, convertAllReleasesGraphQLResponse } from '~/releases/util';
-import { pageInfoHeadersWithoutPagination } from '../../../mock_data';
-
-const originalRelease = getJSONFixture('api/releases/release.json');
-const originalReleases = [originalRelease];
 
 const originalGraphqlReleasesResponse = getJSONFixture(
   'graphql/releases/queries/all_releases.query.graphql.json',
@@ -30,14 +18,12 @@ const originalGraphqlReleasesResponse = getJSONFixture(
 
 describe('Releases State actions', () => {
   let mockedState;
-  let releases;
   let graphqlReleasesResponse;
 
   const projectPath = 'root/test-project';
   const projectId = 19;
   const before = 'testBeforeCursor';
   const after = 'testAfterCursor';
-  const page = 2;
 
   beforeEach(() => {
     mockedState = {
@@ -47,57 +33,10 @@ describe('Releases State actions', () => {
       }),
     };
 
-    releases = convertObjectPropsToCamelCase(originalReleases, { deep: true });
     graphqlReleasesResponse = cloneDeep(originalGraphqlReleasesResponse);
   });
 
-  describe('when all the necessary GraphQL feature flags are enabled', () => {
-    beforeEach(() => {
-      mockedState.useGraphQLEndpoint = true;
-    });
-
-    describe('fetchReleases', () => {
-      it('dispatches fetchReleasesGraphQl with before and after parameters', () => {
-        return testAction(
-          fetchReleases,
-          { before, after, page },
-          mockedState,
-          [],
-          [
-            {
-              type: 'fetchReleasesGraphQl',
-              payload: { before, after },
-            },
-          ],
-        );
-      });
-    });
-  });
-
-  describe('when at least one of the GraphQL feature flags is disabled', () => {
-    beforeEach(() => {
-      mockedState.useGraphQLEndpoint = false;
-    });
-
-    describe('fetchReleases', () => {
-      it('dispatches fetchReleasesRest with a page parameter', () => {
-        return testAction(
-          fetchReleases,
-          { before, after, page },
-          mockedState,
-          [],
-          [
-            {
-              type: 'fetchReleasesRest',
-              payload: { page },
-            },
-          ],
-        );
-      });
-    });
-  });
-
-  describe('fetchReleasesGraphQl', () => {
+  describe('fetchReleases', () => {
     describe('GraphQL query variables', () => {
       let vuexParams;
 
@@ -109,7 +48,7 @@ describe('Releases State actions', () => {
 
       describe('when neither a before nor an after parameter is provided', () => {
         beforeEach(() => {
-          fetchReleasesGraphQl(vuexParams, { before: undefined, after: undefined });
+          fetchReleases(vuexParams, { before: undefined, after: undefined });
         });
 
         it('makes a GraphQl query with a first variable', () => {
@@ -122,7 +61,7 @@ describe('Releases State actions', () => {
 
       describe('when only a before parameter is provided', () => {
         beforeEach(() => {
-          fetchReleasesGraphQl(vuexParams, { before, after: undefined });
+          fetchReleases(vuexParams, { before, after: undefined });
         });
 
         it('makes a GraphQl query with last and before variables', () => {
@@ -135,7 +74,7 @@ describe('Releases State actions', () => {
 
       describe('when only an after parameter is provided', () => {
         beforeEach(() => {
-          fetchReleasesGraphQl(vuexParams, { before: undefined, after });
+          fetchReleases(vuexParams, { before: undefined, after });
         });
 
         it('makes a GraphQl query with first and after variables', () => {
@@ -148,12 +87,12 @@ describe('Releases State actions', () => {
 
       describe('when both before and after parameters are provided', () => {
         it('throws an error', () => {
-          const callFetchReleasesGraphQl = () => {
-            fetchReleasesGraphQl(vuexParams, { before, after });
+          const callFetchReleases = () => {
+            fetchReleases(vuexParams, { before, after });
           };
 
-          expect(callFetchReleasesGraphQl).toThrowError(
-            'Both a `before` and an `after` parameter were provided to fetchReleasesGraphQl. These parameters cannot be used together.',
+          expect(callFetchReleases).toThrowError(
+            'Both a `before` and an `after` parameter were provided to fetchReleases. These parameters cannot be used together.',
           );
         });
       });
@@ -171,7 +110,7 @@ describe('Releases State actions', () => {
             mockedState.sorting.sort = sort;
             mockedState.sorting.orderBy = orderBy;
 
-            fetchReleasesGraphQl(vuexParams, { before: undefined, after: undefined });
+            fetchReleases(vuexParams, { before: undefined, after: undefined });
 
             expect(gqClient.query).toHaveBeenCalledWith({
               query: allReleasesQuery,
@@ -191,7 +130,7 @@ describe('Releases State actions', () => {
         const convertedResponse = convertAllReleasesGraphQLResponse(graphqlReleasesResponse);
 
         return testAction(
-          fetchReleasesGraphQl,
+          fetchReleases,
           {},
           mockedState,
           [
@@ -218,90 +157,7 @@ describe('Releases State actions', () => {
 
       it(`commits ${types.REQUEST_RELEASES} and dispatch receiveReleasesError`, () => {
         return testAction(
-          fetchReleasesGraphQl,
-          {},
-          mockedState,
-          [
-            {
-              type: types.REQUEST_RELEASES,
-            },
-          ],
-          [
-            {
-              type: 'receiveReleasesError',
-            },
-          ],
-        );
-      });
-    });
-  });
-
-  describe('fetchReleasesRest', () => {
-    describe('REST query parameters', () => {
-      let vuexParams;
-
-      beforeEach(() => {
-        jest
-          .spyOn(api, 'releases')
-          .mockResolvedValue({ data: releases, headers: pageInfoHeadersWithoutPagination });
-
-        vuexParams = { dispatch: jest.fn(), commit: jest.fn(), state: mockedState };
-      });
-
-      describe('when a page parameter is provided', () => {
-        beforeEach(() => {
-          fetchReleasesRest(vuexParams, { page: 2 });
-        });
-
-        it('makes a REST query with a page query parameter', () => {
-          expect(api.releases).toHaveBeenCalledWith(projectId, {
-            page,
-            order_by: 'released_at',
-            sort: 'desc',
-          });
-        });
-      });
-    });
-
-    describe('when the request is successful', () => {
-      beforeEach(() => {
-        jest
-          .spyOn(api, 'releases')
-          .mockResolvedValue({ data: releases, headers: pageInfoHeadersWithoutPagination });
-      });
-
-      it(`commits ${types.REQUEST_RELEASES} and ${types.RECEIVE_RELEASES_SUCCESS}`, () => {
-        return testAction(
-          fetchReleasesRest,
-          {},
-          mockedState,
-          [
-            {
-              type: types.REQUEST_RELEASES,
-            },
-            {
-              type: types.RECEIVE_RELEASES_SUCCESS,
-              payload: {
-                data: convertObjectPropsToCamelCase(releases, { deep: true }),
-                restPageInfo: parseIntPagination(
-                  normalizeHeaders(pageInfoHeadersWithoutPagination),
-                ),
-              },
-            },
-          ],
-          [],
-        );
-      });
-    });
-
-    describe('when the request fails', () => {
-      beforeEach(() => {
-        jest.spyOn(api, 'releases').mockRejectedValue(new Error('Something went wrong!'));
-      });
-
-      it(`commits ${types.REQUEST_RELEASES} and dispatch receiveReleasesError`, () => {
-        return testAction(
-          fetchReleasesRest,
+          fetchReleases,
           {},
           mockedState,
           [

spec/requests/api/graphql/group/milestones_spec.rb

@@ -197,18 +197,6 @@ RSpec.describe 'Milestones through GroupQuery' do
           }
         })
       end
-
-      context 'when the graphql_milestone_stats feature flag is disabled' do
-        before do
-          stub_feature_flags(graphql_milestone_stats: false)
-        end
-
-        it 'returns nil for the stats field' do
-          expect(post_query).to eq({
-            'stats' => nil
-          })
-        end
-      end
     end
   end
 end

spec/requests/api/graphql/project/release_spec.rb

@@ -370,23 +370,6 @@ RSpec.describe 'Query.project(fullPath).release(tagName)' do
     end
   end
 
-  describe 'ensures that the release data can be contolled by a feature flag' do
-    context 'when the graphql_release_data feature flag is disabled' do
-      let_it_be(:project) { create(:project, :repository, :public) }
-      let_it_be(:release) { create(:release, project: project) }
-
-      let(:current_user) { developer }
-
-      before do
-        stub_feature_flags(graphql_release_data: false)
-
-        project.add_developer(developer)
-      end
-
-      it_behaves_like 'no access to the release field'
-    end
-  end
-
   describe 'upcoming release' do
     let(:path) { path_prefix }
     let(:project) { create(:project, :repository, :private) }

spec/requests/api/graphql/project/releases_spec.rb

@@ -295,23 +295,6 @@ RSpec.describe 'Query.project(fullPath).releases()' do
     end
   end
 
-  describe 'ensures that the release data can be contolled by a feature flag' do
-    context 'when the graphql_release_data feature flag is disabled' do
-      let_it_be(:project) { create(:project, :repository, :public) }
-      let_it_be(:release) { create(:release, project: project) }
-
-      let(:current_user) { developer }
-
-      before do
-        stub_feature_flags(graphql_release_data: false)
-
-        project.add_developer(developer)
-      end
-
-      it_behaves_like 'no access to any release data'
-    end
-  end
-
   describe 'sorting behavior' do
     let_it_be(:today) { Time.now }
     let_it_be(:yesterday) { today - 1.day }