Merge branch '52618-incorrect-stage-being-shown-in-side-bar-of-job-view-api' into 'master'

Add stage name in job.json response

Closes #52618

See merge request gitlab-org/gitlab-ce!22317

app/assets/javascripts/jobs/components/sidebar.vue

@@ -36,7 +36,7 @@ export default {
     },
   },
   computed: {
-    ...mapState(['job', 'isLoading', 'stages', 'jobs']),
+    ...mapState(['job', 'isLoading', 'stages', 'jobs', 'selectedStage']),
     coverage() {
       return `${this.job.coverage}%`;
     },
@@ -276,6 +276,7 @@ export default {
           <stages-dropdown
             :stages="stages"
             :pipeline="job.pipeline"
+            :selected-stage="selectedStage"
             @requestSidebarStageDropdown="fetchJobsForStage"
           />
 

app/assets/javascripts/jobs/components/stages_dropdown.vue

@@ -2,7 +2,6 @@
 import _ from 'underscore';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
 import Icon from '~/vue_shared/components/icon.vue';
-import { __ } from '~/locale';
 
 export default {
   components: {
@@ -18,30 +17,20 @@ export default {
       type: Array,
       required: true,
     },
+    selectedStage: {
+      type: String,
+      required: true,
+    },
   },
-  data() {
-    return {
-      selectedStage: this.stages.length > 0 ? this.stages[0].name : __('More'),
-    };
-  },
+
   computed: {
     hasRef() {
       return !_.isEmpty(this.pipeline.ref);
     },
   },
-  watch: {
-    // When the component is initially mounted it may start with an empty stages array.
-    // Once the prop is updated, we set the first stage as the selected one
-    stages(newVal) {
-      if (newVal.length) {
-        this.selectedStage = newVal[0].name;
-      }
-    },
-  },
   methods: {
     onStageClick(stage) {
       this.$emit('requestSidebarStageDropdown', stage);
-      this.selectedStage = stage.name;
     },
   },
 };

app/assets/javascripts/jobs/store/actions.js

@@ -141,8 +141,10 @@ export const fetchStages = ({ state, dispatch }) => {
   axios
     .get(`${state.job.pipeline.path}.json`)
     .then(({ data }) => {
+      // Set selected stage
       dispatch('receiveStagesSuccess', data.details.stages);
-      dispatch('fetchJobsForStage', data.details.stages[0]);
+      const selectedStage = data.details.stages.find(stage => stage.name === state.selectedStage);
+      dispatch('fetchJobsForStage', selectedStage);
     })
     .catch(() => dispatch('receiveStagesError'));
 };
@@ -156,11 +158,12 @@ export const receiveStagesError = ({ commit }) => {
 /**
  * Jobs list on sidebar - depend on stages dropdown
  */
-export const requestJobsForStage = ({ commit }) => commit(types.REQUEST_JOBS_FOR_STAGE);
+export const requestJobsForStage = ({ commit }, stage) =>
+  commit(types.REQUEST_JOBS_FOR_STAGE, stage);
 
 // On stage click, set selected stage + fetch job
 export const fetchJobsForStage = ({ dispatch }, stage) => {
-  dispatch('requestJobsForStage');
+  dispatch('requestJobsForStage', stage);
 
   axios
     .get(stage.dropdown_path, {

app/assets/javascripts/jobs/store/mutations.js

@@ -53,6 +53,16 @@ export default {
     state.isLoading = false;
     state.hasError = false;
     state.job = job;
+
+    /**
+     * We only update it on the first request
+     * The dropdown can be changed by the user
+     * after the first request,
+     * and we do not want to hijack that
+     */
+    if (state.selectedStage === 'More' && job.stage) {
+      state.selectedStage = job.stage;
+    }
   },
   [types.RECEIVE_JOB_ERROR](state) {
     state.isLoading = false;
@@ -81,8 +91,9 @@ export default {
     state.stages = [];
   },
 
-  [types.REQUEST_JOBS_FOR_STAGE](state) {
+  [types.REQUEST_JOBS_FOR_STAGE](state, stage) {
     state.isLoadingJobs = true;
+    state.selectedStage = stage.name;
   },
   [types.RECEIVE_JOBS_FOR_STAGE_SUCCESS](state, jobs) {
     state.isLoadingJobs = false;

app/assets/javascripts/jobs/store/state.js

+import { __ } from '~/locale';
+
 export default () => ({
   jobEndpoint: null,
   traceEndpoint: null,
@@ -34,7 +36,7 @@ export default () => ({
   // sidebar dropdown
   isLoadingStages: false,
   isLoadingJobs: false,
-  selectedStage: null,
+  selectedStage: __('More'),
   stages: [],
   jobs: [],
 });

app/serializers/build_details_entity.rb

@@ -4,6 +4,7 @@ class BuildDetailsEntity < JobEntity
   expose :coverage, :erased_at, :duration
   expose :tag_list, as: :tags
   expose :has_trace?, as: :has_trace
+  expose :stage
   expose :user, using: UserEntity
   expose :runner, using: RunnerEntity
   expose :pipeline, using: PipelineEntity

changelogs/unreleased/52618-incorrect-stage-being-shown-in-side-bar-of-job-view-api.yml

+---
+title: Load correct stage in the stages dropdown
+merge_request: 22317
+author:
+type: fixed

spec/controllers/projects/jobs_controller_spec.rb

@@ -352,6 +352,10 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
           expect(json_response['has_trace']).to be true
         end
       end
+
+      it 'exposes the stage the job belongs to' do
+        expect(json_response['stage']).to eq('test')
+      end
     end
 
     context 'when requesting JSON job is triggered' do

spec/fixtures/api/schemas/job/job_details.json

@@ -7,7 +7,8 @@
     "artifact",
     "runner",
     "runners",
-    "has_trace"
+    "has_trace",
+    "stage"
   ],
   "properties": {
     "artifact": { "$ref": "artifact.json" },
@@ -16,6 +17,7 @@
     "deployment_status": { "$ref": "deployment_status.json" },
     "runner": { "$ref": "runner.json" },
     "runners": { "$ref": "runners.json" },
-    "has_trace": { "type": "boolean" }
+    "has_trace": { "type": "boolean" },
+    "stage": { "type": "string" }
   }
 }

spec/javascripts/jobs/components/sidebar_spec.js

@@ -161,9 +161,9 @@ describe('Sidebar details block', () => {
         vm = mountComponentWithStore(SidebarComponent, { store });
       });
 
-      it('renders first stage as selected', () => {
+      it('renders value provided as selectedStage as selected', () => {
         expect(vm.$el.querySelector('.js-selected-stage').textContent.trim()).toEqual(
-          stages[0].name,
+          vm.selectedStage,
         );
       });
     });

spec/javascripts/jobs/components/stages_dropdown_spec.js

@@ -2,7 +2,7 @@ import Vue from 'vue';
 import component from '~/jobs/components/stages_dropdown.vue';
 import mountComponent from '../../helpers/vue_mount_component_helper';
 
-describe('Artifacts block', () => {
+describe('Stages Dropdown', () => {
   const Component = Vue.extend(component);
   let vm;
 
@@ -23,10 +23,6 @@ describe('Artifacts block', () => {
         },
         path: 'pipeline/28029444',
       },
-      ref: {
-        path: 'commits/50101-truncated-job-information',
-        name: '50101-truncated-job-information',
-      },
       stages: [
         {
           name: 'build',
@@ -35,6 +31,7 @@ describe('Artifacts block', () => {
           name: 'test',
         },
       ],
+      selectedStage: 'deploy'
     });
   });
 
@@ -53,17 +50,10 @@ describe('Artifacts block', () => {
   });
 
   it('renders dropdown with stages', () => {
-    expect(vm.$el.querySelector('.dropdown button').textContent).toContain('build');
+    expect(vm.$el.querySelector('.dropdown .js-stage-item').textContent).toContain('build');
   });
 
-  it('updates selected stage on click', done => {
-    vm.$el.querySelectorAll('.stage-item')[1].click();
-    vm
-      .$nextTick()
-      .then(() => {
-        expect(vm.$el.querySelector('.dropdown button').textContent).toContain('test');
-      })
-      .then(done)
-      .catch(done.fail);
+  it('rendes selected stage', () => {
+    expect(vm.$el.querySelector('.dropdown .js-selected-stage').textContent).toContain('deploy');
   });
 });

spec/javascripts/jobs/store/actions_spec.js

@@ -424,6 +424,7 @@ describe('Job State actions', () => {
       mockedState.job.pipeline = {
         path: `${TEST_HOST}/endpoint`,
       };
+      mockedState.selectedStage = 'deploy'
       mock = new MockAdapter(axios);
     });
 
@@ -435,7 +436,7 @@ describe('Job State actions', () => {
       it('dispatches requestStages and receiveStagesSuccess, fetchJobsForStage ', done => {
         mock
           .onGet(`${TEST_HOST}/endpoint.json`)
-          .replyOnce(200, { details: { stages: [{ id: 121212, name: 'build' }] } });
+          .replyOnce(200, { details: { stages: [{ name: 'build' }, { name: 'deploy' }] } });
 
         testAction(
           fetchStages,
@@ -447,11 +448,11 @@ describe('Job State actions', () => {
               type: 'requestStages',
             },
             {
-              payload: [{ id: 121212, name: 'build' }],
+              payload: [{ name: 'build' }, { name: 'deploy' }],
               type: 'receiveStagesSuccess',
             },
             {
-              payload: { id: 121212, name: 'build' },
+              payload: { name: 'deploy' },
               type: 'fetchJobsForStage',
             },
           ],
@@ -515,9 +516,9 @@ describe('Job State actions', () => {
     it('should commit REQUEST_JOBS_FOR_STAGE mutation ', done => {
       testAction(
         requestJobsForStage,
-        null,
+        { name: 'deploy' },
         mockedState,
-        [{ type: types.REQUEST_JOBS_FOR_STAGE }],
+        [{ type: types.REQUEST_JOBS_FOR_STAGE, payload: { name: 'deploy' } }],
         [],
         done,
       );
@@ -549,6 +550,7 @@ describe('Job State actions', () => {
           [
             {
               type: 'requestJobsForStage',
+              payload: { dropdown_path: `${TEST_HOST}/jobs.json` },
             },
             {
               payload: [{ id: 121212, name: 'build' }],
@@ -574,6 +576,7 @@ describe('Job State actions', () => {
           [
             {
               type: 'requestJobsForStage',
+              payload: { dropdown_path: `${TEST_HOST}/jobs.json` },
             },
             {
               type: 'receiveJobsForStageError',

spec/javascripts/jobs/store/mutations_spec.js

@@ -108,21 +108,33 @@ describe('Jobs Store Mutations', () => {
   });
 
   describe('RECEIVE_JOB_SUCCESS', () => {
-    beforeEach(() => {
-      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321 });
-    });
-
     it('sets is loading to false', () => {
+      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321 });
       expect(stateCopy.isLoading).toEqual(false);
     });
 
     it('sets hasError to false', () => {
+      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321 });
       expect(stateCopy.hasError).toEqual(false);
     });
 
     it('sets job data', () => {
+      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321 });
       expect(stateCopy.job).toEqual({ id: 1312321 });
     });
+
+    it('sets selectedStage when the selectedStage is More', () => {
+      expect(stateCopy.selectedStage).toEqual('More');
+      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321, stage: 'deploy' });
+      expect(stateCopy.selectedStage).toEqual('deploy');
+    });
+
+    it('does not set selectedStage when the selectedStage is not More', () => {
+      stateCopy.selectedStage = 'notify'
+      expect(stateCopy.selectedStage).toEqual('notify');
+      mutations[types.RECEIVE_JOB_SUCCESS](stateCopy, { id: 1312321, stage: 'deploy' });
+      expect(stateCopy.selectedStage).toEqual('notify');
+    });
   });
 
   describe('RECEIVE_JOB_ERROR', () => {
@@ -200,9 +212,14 @@ describe('Jobs Store Mutations', () => {
 
   describe('REQUEST_JOBS_FOR_STAGE', () => {
     it('sets isLoadingStages to true', () => {
-      mutations[types.REQUEST_JOBS_FOR_STAGE](stateCopy);
+      mutations[types.REQUEST_JOBS_FOR_STAGE](stateCopy, { name: 'deploy' });
       expect(stateCopy.isLoadingJobs).toEqual(true);
     });
+
+    it('sets selectedStage', () => {
+      mutations[types.REQUEST_JOBS_FOR_STAGE](stateCopy, { name: 'deploy' });
+      expect(stateCopy.selectedStage).toEqual('deploy');
+    })
   });
 
   describe('RECEIVE_JOBS_FOR_STAGE_SUCCESS', () => {