Merge branch 'pipeline/fix-header-status-update' into 'master'

Update pipeline header status when retrying jobs

See merge request gitlab-org/gitlab!66764

app/assets/javascripts/pipelines/components/graph/graph_component_wrapper.vue

@@ -7,6 +7,7 @@ import LocalStorageSync from '~/vue_shared/components/local_storage_sync.vue';
 import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { DEFAULT, DRAW_FAILURE, LOAD_FAILURE } from '../../constants';
 import DismissPipelineGraphCallout from '../../graphql/mutations/dismiss_pipeline_notification.graphql';
+import getPipelineQuery from '../../graphql/queries/get_pipeline_header_data.query.graphql';
 import { reportToSentry, reportMessageToSentry } from '../../utils';
 import { listByLayers } from '../parsing_utils';
 import { IID_FAILURE, LAYER_VIEW, STAGE_VIEW, VIEW_TYPE_KEY } from './constants';
@@ -51,6 +52,7 @@ export default {
       alertType: null,
       callouts: [],
       currentViewType: STAGE_VIEW,
+      canRefetchHeaderPipeline: false,
       pipeline: null,
       pipelineLayers: null,
       showAlert: false,
@@ -78,6 +80,26 @@ export default {
         );
       },
     },
+    headerPipeline: {
+      query: getPipelineQuery,
+      // this query is already being called in header_component.vue, which shares the same cache as this component
+      // the skip here is to prevent sending double network requests on page load
+      skip() {
+        return !this.canRefetchHeaderPipeline;
+      },
+      variables() {
+        return {
+          fullPath: this.pipelineProjectPath,
+          iid: this.pipelineIid,
+        };
+      },
+      update(data) {
+        return data.project?.pipeline || {};
+      },
+      error() {
+        this.reportFailure({ type: LOAD_FAILURE, skipSentry: true });
+      },
+    },
     pipeline: {
       context() {
         return getQueryHeaders(this.graphqlResourceEtag);
@@ -217,6 +239,10 @@ export default {
     },
     refreshPipelineGraph() {
       this.$apollo.queries.pipeline.refetch();
+
+      // this will update the status in header_component since they share the same cache
+      this.canRefetchHeaderPipeline = true;
+      this.$apollo.queries.headerPipeline.refetch();
     },
     /* eslint-disable @gitlab/require-i18n-strings */
     reportFailure({ type, err = 'No error string passed.', skipSentry = false }) {

app/assets/javascripts/pipelines/components/header_component.vue

@@ -143,13 +143,6 @@ export default {
       return cancelable && userPermissions.updatePipeline;
     },
   },
-  watch: {
-    isFinished(finished) {
-      if (finished) {
-        this.$apollo.queries.pipeline.stopPolling();
-      }
-    },
-  },
   methods: {
     reportFailure(errorType) {
       this.failureType = errorType;
@@ -218,7 +211,7 @@ export default {
 };
 </script>
 <template>
-  <div class="pipeline-header-container">
+  <div class="js-pipeline-header-container">
     <gl-alert v-if="hasError" :variant="failure.variant">{{ failure.text }}</gl-alert>
     <ci-header
       v-if="shouldRenderContent"

spec/features/projects/pipelines/pipeline_spec.rb

@@ -240,10 +240,14 @@ RSpec.describe 'Pipeline', :js do
           end
         end
 
-        it 'is possible to retry the success job' do
+        it 'is possible to retry the success job', :sidekiq_might_not_need_inline do
           find('#ci-badge-build .ci-action-icon-container').click
+          wait_for_requests
 
           expect(page).not_to have_content('Retry job')
+          within('.js-pipeline-header-container') do
+            expect(page).to have_selector('.js-ci-status-icon-running')
+          end
         end
       end
 
@@ -282,10 +286,14 @@ RSpec.describe 'Pipeline', :js do
           end
         end
 
-        it 'is possible to retry the failed build' do
+        it 'is possible to retry the failed build', :sidekiq_might_not_need_inline do
           find('#ci-badge-test .ci-action-icon-container').click
+          wait_for_requests
 
           expect(page).not_to have_content('Retry job')
+          within('.js-pipeline-header-container') do
+            expect(page).to have_selector('.js-ci-status-icon-running')
+          end
         end
 
         it 'includes the failure reason' do
@@ -308,10 +316,14 @@ RSpec.describe 'Pipeline', :js do
           end
         end
 
-        it 'is possible to play the manual job' do
+        it 'is possible to play the manual job', :sidekiq_might_not_need_inline do
           find('#ci-badge-manual-build .ci-action-icon-container').click
+          wait_for_requests
 
           expect(page).not_to have_content('Play job')
+          within('.js-pipeline-header-container') do
+            expect(page).to have_selector('.js-ci-status-icon-running')
+          end
         end
       end
 
@@ -411,11 +423,18 @@ RSpec.describe 'Pipeline', :js do
       context 'when retrying' do
         before do
           find('[data-testid="retryPipeline"]').click
+          wait_for_requests
         end
 
         it 'does not show a "Retry" button', :sidekiq_might_not_need_inline do
           expect(page).not_to have_content('Retry')
         end
+
+        it 'shows running status in pipeline header', :sidekiq_might_not_need_inline do
+          within('.js-pipeline-header-container') do
+            expect(page).to have_selector('.js-ci-status-icon-running')
+          end
+        end
       end
     end
 
@@ -770,7 +789,7 @@ RSpec.describe 'Pipeline', :js do
         it 'shows deploy job as created' do
           subject
 
-          within('.pipeline-header-container') do
+          within('.js-pipeline-header-container') do
             expect(page).to have_content('pending')
           end
 
@@ -795,7 +814,7 @@ RSpec.describe 'Pipeline', :js do
           it 'shows deploy job as pending' do
             subject
 
-            within('.pipeline-header-container') do
+            within('.js-pipeline-header-container') do
               expect(page).to have_content('running')
             end
 
@@ -817,7 +836,7 @@ RSpec.describe 'Pipeline', :js do
       it 'shows deploy job as created' do
         subject
 
-        within('.pipeline-header-container') do
+        within('.js-pipeline-header-container') do
           expect(page).to have_content('pending')
         end
 
@@ -842,7 +861,7 @@ RSpec.describe 'Pipeline', :js do
         it 'shows deploy job as pending' do
           subject
 
-          within('.pipeline-header-container') do
+          within('.js-pipeline-header-container') do
             expect(page).to have_content('running')
           end
 
@@ -871,7 +890,7 @@ RSpec.describe 'Pipeline', :js do
         it 'shows deploy job as waiting for resource' do
           subject
 
-          within('.pipeline-header-container') do
+          within('.js-pipeline-header-container') do
             expect(page).to have_content('waiting')
           end
 
@@ -893,7 +912,7 @@ RSpec.describe 'Pipeline', :js do
           it 'shows deploy job as waiting for resource' do
             subject
 
-            within('.pipeline-header-container') do
+            within('.js-pipeline-header-container') do
               expect(page).to have_content('waiting')
             end
 
@@ -914,7 +933,7 @@ RSpec.describe 'Pipeline', :js do
           it 'shows deploy job as pending' do
             subject
 
-            within('.pipeline-header-container') do
+            within('.js-pipeline-header-container') do
               expect(page).to have_content('running')
             end
 
@@ -936,7 +955,7 @@ RSpec.describe 'Pipeline', :js do
             it 'shows deploy job as pending' do
               subject
 
-              within('.pipeline-header-container') do
+              within('.js-pipeline-header-container') do
                 expect(page).to have_content('running')
               end
 
@@ -959,7 +978,7 @@ RSpec.describe 'Pipeline', :js do
           it 'shows deploy job as waiting for resource' do
             subject
 
-            within('.pipeline-header-container') do
+            within('.js-pipeline-header-container') do
               expect(page).to have_content('waiting')
             end
 
@@ -981,7 +1000,7 @@ RSpec.describe 'Pipeline', :js do
           it 'shows deploy job as waiting for resource' do
             subject
 
-            within('.pipeline-header-container') do
+            within('.js-pipeline-header-container') do
               expect(page).to have_content('waiting')
             end
 

spec/frontend/pipelines/graph/graph_component_wrapper_spec.js

@@ -18,6 +18,8 @@ import GraphViewSelector from '~/pipelines/components/graph/graph_view_selector.
 import StageColumnComponent from '~/pipelines/components/graph/stage_column_component.vue';
 import LinksLayer from '~/pipelines/components/graph_shared/links_layer.vue';
 import * as parsingUtils from '~/pipelines/components/parsing_utils';
+import getPipelineHeaderData from '~/pipelines/graphql/queries/get_pipeline_header_data.query.graphql';
+import { mockRunningPipelineHeaderData } from '../mock_data';
 import { mapCallouts, mockCalloutsResponse, mockPipelineResponse } from './mock_data';
 
 const defaultProvide = {
@@ -72,8 +74,10 @@ describe('Pipeline graph wrapper', () => {
   } = {}) => {
     const callouts = mapCallouts(calloutsList);
     const getUserCalloutsHandler = jest.fn().mockResolvedValue(mockCalloutsResponse(callouts));
+    const getPipelineHeaderDataHandler = jest.fn().mockResolvedValue(mockRunningPipelineHeaderData);
 
     const requestHandlers = [
+      [getPipelineHeaderData, getPipelineHeaderDataHandler],
       [getPipelineDetails, getPipelineDetailsHandler],
       [getUserCallouts, getUserCalloutsHandler],
     ];
@@ -111,6 +115,11 @@ describe('Pipeline graph wrapper', () => {
       createComponentWithApollo();
       expect(getGraph().exists()).toBe(false);
     });
+
+    it('skips querying headerPipeline', () => {
+      createComponentWithApollo();
+      expect(wrapper.vm.$apollo.queries.headerPipeline.skip).toBe(true);
+    });
   });
 
   describe('when data has loaded', () => {
@@ -190,12 +199,15 @@ describe('Pipeline graph wrapper', () => {
   describe('when refresh action is emitted', () => {
     beforeEach(async () => {
       createComponentWithApollo();
+      jest.spyOn(wrapper.vm.$apollo.queries.headerPipeline, 'refetch');
       jest.spyOn(wrapper.vm.$apollo.queries.pipeline, 'refetch');
       await wrapper.vm.$nextTick();
       getGraph().vm.$emit('refreshPipelineGraph');
     });
 
     it('calls refetch', () => {
+      expect(wrapper.vm.$apollo.queries.headerPipeline.skip).toBe(false);
+      expect(wrapper.vm.$apollo.queries.headerPipeline.refetch).toHaveBeenCalled();
       expect(wrapper.vm.$apollo.queries.pipeline.refetch).toHaveBeenCalled();
     });
   });

spec/frontend/pipelines/header_component_spec.js

@@ -99,24 +99,6 @@ describe('Pipeline details header', () => {
     );
   });
 
-  describe('polling', () => {
-    it('is stopped when pipeline is finished', async () => {
-      wrapper = createComponent({ ...mockRunningPipelineHeader });
-
-      await wrapper.setData({
-        pipeline: { ...mockCancelledPipelineHeader },
-      });
-
-      expect(wrapper.vm.$apollo.queries.pipeline.stopPolling).toHaveBeenCalled();
-    });
-
-    it('is not stopped when pipeline is not finished', () => {
-      wrapper = createComponent();
-
-      expect(wrapper.vm.$apollo.queries.pipeline.stopPolling).not.toHaveBeenCalled();
-    });
-  });
-
   describe('actions', () => {
     describe('Retry action', () => {
       beforeEach(() => {

spec/frontend/pipelines/mock_data.js

@@ -127,6 +127,28 @@ export const mockSuccessfulPipelineHeader = {
   },
 };
 
+export const mockRunningPipelineHeaderData = {
+  data: {
+    project: {
+      pipeline: {
+        ...mockRunningPipelineHeader,
+        iid: '28',
+        user: {
+          name: 'Foo',
+          username: 'foobar',
+          webPath: '/foo',
+          email: 'foo@bar.com',
+          avatarUrl: 'link',
+          status: null,
+          __typename: 'UserCore',
+        },
+        __typename: 'Pipeline',
+      },
+      __typename: 'Project',
+    },
+  },
+};
+
 export const stageReply = {
   name: 'deploy',
   title: 'deploy: running',