diff --git a/app/assets/javascripts/releases/components/app_show.vue b/app/assets/javascripts/releases/components/app_show.vue index 9ef38503c10d5fd904ae1cbd5e45907acf27812e..c38e93d420b8d2d5886964bdef3283379332705a 100644 --- a/app/assets/javascripts/releases/components/app_show.vue +++ b/app/assets/javascripts/releases/components/app_show.vue @@ -1,5 +1,8 @@ <script> -import { mapState, mapActions } from 'vuex'; +import createFlash from '~/flash'; +import { s__ } from '~/locale'; +import oneReleaseQuery from '../queries/one_release.query.graphql'; +import { convertGraphQLRelease } from '../util'; import ReleaseBlock from './release_block.vue'; import ReleaseSkeletonLoader from './release_skeleton_loader.vue'; @@ -9,21 +12,58 @@ export default { ReleaseBlock, ReleaseSkeletonLoader, }, - computed: { - ...mapState('detail', ['isFetchingRelease', 'fetchError', 'release']), + inject: { + fullPath: { + default: '', + }, + tagName: { + default: '', + }, }, - created() { - this.fetchRelease(); + apollo: { + release: { + query: oneReleaseQuery, + variables() { + return { + fullPath: this.fullPath, + tagName: this.tagName, + }; + }, + update(data) { + if (data.project?.release) { + return convertGraphQLRelease(data.project.release); + } + + return null; + }, + result(result) { + // Handle the case where the query succeeded but didn't return any data + if (!result.error && !this.release) { + this.showFlash( + new Error(`No release found in project "${this.fullPath}" with tag "${this.tagName}"`), + ); + } + }, + error(error) { + this.showFlash(error); + }, + }, }, methods: { - ...mapActions('detail', ['fetchRelease']), + showFlash(error) { + createFlash({ + message: s__('Release|Something went wrong while getting the release details.'), + captureError: true, + error, + }); + }, }, }; </script> <template> <div class="gl-mt-3"> - <release-skeleton-loader v-if="isFetchingRelease" /> + <release-skeleton-loader v-if="$apollo.queries.release.loading" /> - <release-block v-else-if="!fetchError" :release="release" /> + <release-block v-else-if="release" :release="release" /> </div> </template> diff --git a/app/assets/javascripts/releases/mount_show.js b/app/assets/javascripts/releases/mount_show.js index f3ed7d6c5ff10edc61f606a887ae95b6c4c3a118..7272880197aa8c1b50d5ca84b7d33119e8efa6e8 100644 --- a/app/assets/javascripts/releases/mount_show.js +++ b/app/assets/javascripts/releases/mount_show.js @@ -1,26 +1,28 @@ import Vue from 'vue'; -import Vuex from 'vuex'; +import VueApollo from 'vue-apollo'; +import createDefaultClient from '~/lib/graphql'; import ReleaseShowApp from './components/app_show.vue'; -import createStore from './stores'; -import createDetailModule from './stores/modules/detail'; -Vue.use(Vuex); +Vue.use(VueApollo); + +const apolloProvider = new VueApollo({ + defaultClient: createDefaultClient(), +}); export default () => { const el = document.getElementById('js-show-release-page'); - const store = createStore({ - modules: { - detail: createDetailModule(el.dataset), - }, - featureFlags: { - graphqlIndividualReleasePage: Boolean(gon.features?.graphqlIndividualReleasePage), - }, - }); + if (!el) return false; + + const { projectPath, tagName } = el.dataset; return new Vue({ el, - store, + apolloProvider, + provide: { + fullPath: projectPath, + tagName, + }, render: (h) => h(ReleaseShowApp), }); }; diff --git a/app/assets/javascripts/releases/stores/modules/detail/actions.js b/app/assets/javascripts/releases/stores/modules/detail/actions.js index 5fa002706c6eddd1748a664a2f802e75ae9a8a29..8dc2083dd2bce53ffecbaae1e9e9a28c4c8b8f7a 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/actions.js +++ b/app/assets/javascripts/releases/stores/modules/detail/actions.js @@ -43,7 +43,7 @@ export const fetchRelease = ({ commit, state, rootState }) => { }) .catch((error) => { commit(types.RECEIVE_RELEASE_ERROR, error); - createFlash(s__('Release|Something went wrong while getting the release details')); + createFlash(s__('Release|Something went wrong while getting the release details.')); }); } @@ -54,7 +54,7 @@ export const fetchRelease = ({ commit, state, rootState }) => { }) .catch((error) => { commit(types.RECEIVE_RELEASE_ERROR, error); - createFlash(s__('Release|Something went wrong while getting the release details')); + createFlash(s__('Release|Something went wrong while getting the release details.')); }); }; diff --git a/app/controllers/projects/releases_controller.rb b/app/controllers/projects/releases_controller.rb index 614bada09eda81d675da975409bba0d87f6052f6..2638285676105c85dcf1e8f1d269ba89eb971256 100644 --- a/app/controllers/projects/releases_controller.rb +++ b/app/controllers/projects/releases_controller.rb @@ -12,7 +12,6 @@ class Projects::ReleasesController < Projects::ApplicationController 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) - push_frontend_feature_flag(:graphql_individual_release_page, project, default_enabled: true) end before_action :authorize_update_release!, only: %i[edit update] before_action :authorize_create_release!, only: :new diff --git a/changelogs/unreleased/nfriend-reorganize-release-detail-page-store.yml b/changelogs/unreleased/nfriend-reorganize-release-detail-page-store.yml new file mode 100644 index 0000000000000000000000000000000000000000..0b3fc4ccf1e7aa4f0e65a62c01f2d7027f19a40d --- /dev/null +++ b/changelogs/unreleased/nfriend-reorganize-release-detail-page-store.yml @@ -0,0 +1,5 @@ +--- +title: Remove graphql_individual_release_page feature flag +merge_request: 56882 +author: +type: removed diff --git a/config/feature_flags/development/graphql_individual_release_page.yml b/config/feature_flags/development/graphql_individual_release_page.yml deleted file mode 100644 index 8cf13ca4854dc78b4368db4af6a07a620a172552..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/graphql_individual_release_page.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: graphql_individual_release_page -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/44779 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/263522 -milestone: '13.5' -type: development -group: group::release -default_enabled: true diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 6b43f79455a0f3efbfce9c2c6a463d23cd92a20d..2e6d673e528bc65859318971111b4428d4e9af92 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -25348,7 +25348,7 @@ msgstr "" msgid "Release|Something went wrong while creating a new release" msgstr "" -msgid "Release|Something went wrong while getting the release details" +msgid "Release|Something went wrong while getting the release details." msgstr "" msgid "Release|Something went wrong while saving the release details" diff --git a/spec/features/projects/releases/user_views_release_spec.rb b/spec/features/projects/releases/user_views_release_spec.rb index 186122536ce351d75939cf9f2401515d2d409eb2..4410f345e5698f954d4219f5befe5e9fb454d461 100644 --- a/spec/features/projects/releases/user_views_release_spec.rb +++ b/spec/features/projects/releases/user_views_release_spec.rb @@ -5,7 +5,6 @@ RSpec.describe 'User views Release', :js do let(:project) { create(:project, :repository) } let(:user) { create(:user) } - let(:graphql_feature_flag) { true } let(:release) do create(:release, @@ -15,8 +14,6 @@ end before do - stub_feature_flags(graphql_individual_release_page: graphql_feature_flag) - project.add_developer(user) sign_in(user) @@ -26,35 +23,23 @@ it_behaves_like 'page meta description', 'Lorem ipsum dolor sit amet' - shared_examples 'release page' do - it 'renders the breadcrumbs' do - within('.breadcrumbs') do - expect(page).to have_content("#{project.creator.name} #{project.name} Releases #{release.name}") - - expect(page).to have_link(project.creator.name, href: user_path(project.creator)) - expect(page).to have_link(project.name, href: project_path(project)) - expect(page).to have_link('Releases', href: project_releases_path(project)) - expect(page).to have_link(release.name, href: project_release_path(project, release)) - end - end + it 'renders the breadcrumbs' do + within('.breadcrumbs') do + expect(page).to have_content("#{project.creator.name} #{project.name} Releases #{release.name}") - it 'renders the release details' do - within('.release-block') do - expect(page).to have_content(release.name) - expect(page).to have_content(release.tag) - expect(page).to have_content(release.commit.short_id) - expect(page).to have_content('Lorem ipsum dolor sit amet') - end + expect(page).to have_link(project.creator.name, href: user_path(project.creator)) + expect(page).to have_link(project.name, href: project_path(project)) + expect(page).to have_link('Releases', href: project_releases_path(project)) + expect(page).to have_link(release.name, href: project_release_path(project, release)) end end - describe 'when the graphql_individual_release_page feature flag is enabled' do - it_behaves_like 'release page' - end - - describe 'when the graphql_individual_release_page feature flag is disabled' do - let(:graphql_feature_flag) { false } - - it_behaves_like 'release page' + it 'renders the release details' do + within('.release-block') do + expect(page).to have_content(release.name) + expect(page).to have_content(release.tag) + expect(page).to have_content(release.commit.short_id) + expect(page).to have_content('Lorem ipsum dolor sit amet') + end end end diff --git a/spec/frontend/releases/components/app_show_spec.js b/spec/frontend/releases/components/app_show_spec.js index 5caea395f0a1d83886d464cdc606b12a0028b29d..425cb9d005926736f08f5b14feb8eeb9472e3e67 100644 --- a/spec/frontend/releases/components/app_show_spec.js +++ b/spec/frontend/releases/components/app_show_spec.js @@ -1,63 +1,176 @@ import { shallowMount } from '@vue/test-utils'; -import Vuex from 'vuex'; +import Vue from 'vue'; +import VueApollo from 'vue-apollo'; import { getJSONFixture } from 'helpers/fixtures'; -import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; +import createMockApollo from 'helpers/mock_apollo_helper'; +import createFlash from '~/flash'; import ReleaseShowApp from '~/releases/components/app_show.vue'; import ReleaseBlock from '~/releases/components/release_block.vue'; import ReleaseSkeletonLoader from '~/releases/components/release_skeleton_loader.vue'; +import oneReleaseQuery from '~/releases/queries/one_release.query.graphql'; -const originalRelease = getJSONFixture('api/releases/release.json'); +jest.mock('~/flash'); + +const oneReleaseQueryResponse = getJSONFixture( + 'graphql/releases/queries/one_release.query.graphql.json', +); + +Vue.use(VueApollo); + +const EXPECTED_ERROR_MESSAGE = 'Something went wrong while getting the release details.'; +const MOCK_FULL_PATH = 'project/full/path'; +const MOCK_TAG_NAME = 'test-tag-name'; describe('Release show component', () => { let wrapper; - let release; - let actions; - beforeEach(() => { - release = convertObjectPropsToCamelCase(originalRelease); - }); - - const factory = (state) => { - actions = { - fetchRelease: jest.fn(), - }; - - const store = new Vuex.Store({ - modules: { - detail: { - namespaced: true, - actions, - state, - }, + const createComponent = ({ apolloProvider }) => { + wrapper = shallowMount(ReleaseShowApp, { + provide: { + fullPath: MOCK_FULL_PATH, + tagName: MOCK_TAG_NAME, }, + apolloProvider, }); - - wrapper = shallowMount(ReleaseShowApp, { store }); }; + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + const findLoadingSkeleton = () => wrapper.find(ReleaseSkeletonLoader); const findReleaseBlock = () => wrapper.find(ReleaseBlock); - it('calls fetchRelease when the component is created', () => { - factory({ release }); - expect(actions.fetchRelease).toHaveBeenCalledTimes(1); + const expectLoadingIndicator = () => { + it('renders a loading indicator', () => { + expect(findLoadingSkeleton().exists()).toBe(true); + }); + }; + + const expectNoLoadingIndicator = () => { + it('does not render a loading indicator', () => { + expect(findLoadingSkeleton().exists()).toBe(false); + }); + }; + + const expectNoFlash = () => { + it('does not show a flash message', () => { + expect(createFlash).not.toHaveBeenCalled(); + }); + }; + + const expectFlashWithMessage = (message) => { + it(`shows a flash message that reads "${message}"`, () => { + expect(createFlash).toHaveBeenCalledTimes(1); + expect(createFlash).toHaveBeenCalledWith({ + message, + captureError: true, + error: expect.any(Error), + }); + }); + }; + + const expectReleaseBlock = () => { + it('renders a release block', () => { + expect(findReleaseBlock().exists()).toBe(true); + }); + }; + + const expectNoReleaseBlock = () => { + it('does not render a release block', () => { + expect(findReleaseBlock().exists()).toBe(false); + }); + }; + + describe('GraphQL query variables', () => { + const queryHandler = jest.fn().mockResolvedValueOnce(oneReleaseQueryResponse); + + beforeEach(() => { + const apolloProvider = createMockApollo([[oneReleaseQuery, queryHandler]]); + + createComponent({ apolloProvider }); + }); + + it('builds a GraphQL with the expected variables', () => { + expect(queryHandler).toHaveBeenCalledTimes(1); + expect(queryHandler).toHaveBeenCalledWith({ + fullPath: MOCK_FULL_PATH, + tagName: MOCK_TAG_NAME, + }); + }); }); - it('shows a loading skeleton and hides the release block while the API call is in progress', () => { - factory({ isFetchingRelease: true }); - expect(findLoadingSkeleton().exists()).toBe(true); - expect(findReleaseBlock().exists()).toBe(false); + describe('when the component is loading data', () => { + beforeEach(() => { + const apolloProvider = createMockApollo([ + [oneReleaseQuery, jest.fn().mockReturnValueOnce(new Promise(() => {}))], + ]); + + createComponent({ apolloProvider }); + }); + + expectLoadingIndicator(); + expectNoFlash(); + expectNoReleaseBlock(); }); - it('hides the loading skeleton and shows the release block when the API call finishes successfully', () => { - factory({ isFetchingRelease: false }); - expect(findLoadingSkeleton().exists()).toBe(false); - expect(findReleaseBlock().exists()).toBe(true); + describe('when the component has successfully loaded the release', () => { + beforeEach(() => { + const apolloProvider = createMockApollo([ + [oneReleaseQuery, jest.fn().mockResolvedValueOnce(oneReleaseQueryResponse)], + ]); + + createComponent({ apolloProvider }); + }); + + expectNoLoadingIndicator(); + expectNoFlash(); + expectReleaseBlock(); }); - it('hides both the loading skeleton and the release block when the API call fails', () => { - factory({ fetchError: new Error('Uh oh') }); - expect(findLoadingSkeleton().exists()).toBe(false); - expect(findReleaseBlock().exists()).toBe(false); + describe('when the request succeeded, but the returned "project" key was null', () => { + beforeEach(() => { + const apolloProvider = createMockApollo([ + [oneReleaseQuery, jest.fn().mockResolvedValueOnce({ data: { project: null } })], + ]); + + createComponent({ apolloProvider }); + }); + + expectNoLoadingIndicator(); + expectFlashWithMessage(EXPECTED_ERROR_MESSAGE); + expectNoReleaseBlock(); + }); + + describe('when the request succeeded, but the returned "project.release" key was null', () => { + beforeEach(() => { + const apolloProvider = createMockApollo([ + [ + oneReleaseQuery, + jest.fn().mockResolvedValueOnce({ data: { project: { release: null } } }), + ], + ]); + + createComponent({ apolloProvider }); + }); + + expectNoLoadingIndicator(); + expectFlashWithMessage(EXPECTED_ERROR_MESSAGE); + expectNoReleaseBlock(); + }); + + describe('when an error occurs while loading the release', () => { + beforeEach(() => { + const apolloProvider = createMockApollo([ + [oneReleaseQuery, jest.fn().mockRejectedValueOnce('An error occurred!')], + ]); + + createComponent({ apolloProvider }); + }); + + expectNoLoadingIndicator(); + expectFlashWithMessage(EXPECTED_ERROR_MESSAGE); + expectNoReleaseBlock(); }); }); diff --git a/spec/frontend/releases/stores/modules/detail/actions_spec.js b/spec/frontend/releases/stores/modules/detail/actions_spec.js index 9c125fbb87bd12be5076c9ee9035c841ea7e69cf..0f81869e3f97283e238bc27c06e0a5d25ff4bd97 100644 --- a/spec/frontend/releases/stores/modules/detail/actions_spec.js +++ b/spec/frontend/releases/stores/modules/detail/actions_spec.js @@ -163,7 +163,7 @@ describe('Release detail actions', () => { return actions.fetchRelease({ commit: jest.fn(), state, rootState: state }).then(() => { expect(createFlash).toHaveBeenCalledTimes(1); expect(createFlash).toHaveBeenCalledWith( - 'Something went wrong while getting the release details', + 'Something went wrong while getting the release details.', ); }); });