Move polling solution

Revert FE solution and
return no content if job
has no trace or started.

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

@@ -21,7 +21,8 @@ export const init = ({ dispatch }, { endpoint, logState, pagePath }) => {
     logState,
     pagePath,
   });
-  dispatch('fetchJob');
+
+  return Promise.all([dispatch('fetchJob'), dispatch('fetchTrace')]);
 };
 
 export const setJobEndpoint = ({ commit }, endpoint) => commit(types.SET_JOB_ENDPOINT, endpoint);
@@ -71,14 +72,7 @@ export const fetchJob = ({ state, dispatch }) => {
   });
 
   if (!Visibility.hidden()) {
-    // eslint-disable-next-line promise/catch-or-return
-    eTagPoll.makeRequest().then(() => {
-      // if a job is canceled we still need to dispatch
-      // fetchTrace to get the trace so we check for has_trace
-      if (state.job.started || state.job.has_trace) {
-        dispatch('fetchTrace');
-      }
-    });
+    eTagPoll.makeRequest();
   } else {
     axios
       .get(state.jobEndpoint)
@@ -90,7 +84,7 @@ export const fetchJob = ({ state, dispatch }) => {
     if (!Visibility.hidden()) {
       // This check is needed to ensure the loading icon
       // is not shown for a finished job during a visibility change
-      if (!isTraceReadyForRender && state.job.started) {
+      if (!isTraceReadyForRender) {
         dispatch('startPollingTrace');
       }
       dispatch('restartPolling');
@@ -258,7 +252,7 @@ export const receiveJobsForStageError = ({ commit }) => {
   flash(__('An error occurred while fetching the jobs.'));
 };
 
-export const triggerManualJob = ({ state, dispatch }, variables) => {
+export const triggerManualJob = ({ state }, variables) => {
   const parsedVariables = variables.map((variable) => {
     const copyVar = { ...variable };
     delete copyVar.id;
@@ -269,6 +263,5 @@ export const triggerManualJob = ({ state, dispatch }, variables) => {
     .post(state.job.status.action.path, {
       job_variables_attributes: parsedVariables,
     })
-    .then(() => dispatch('fetchTrace'))
     .catch(() => flash(__('An error occurred while triggering the job.')));
 };

app/controllers/projects/jobs_controller.rb

@@ -40,9 +40,7 @@ class Projects::JobsController < Projects::ApplicationController
       format.json do
         Gitlab::PollingInterval.set_header(response, interval: 10_000)
 
-        render json: BuildSerializer
-          .new(project: @project, current_user: @current_user)
-          .represent(@build, {}, BuildDetailsEntity)
+        render json: build_details
       end
     end
   end
@@ -59,9 +57,13 @@ class Projects::JobsController < Projects::ApplicationController
             stream: stream,
             state: params[:state])
 
-          render json: BuildTraceSerializer
-            .new(project: @project, current_user: @current_user)
-            .represent(build_trace)
+          if build_details[:has_trace]
+            render json: BuildTraceSerializer
+              .new(project: @project, current_user: @current_user)
+              .represent(build_trace)
+          else
+            head :no_content
+          end
         end
       end
     end
@@ -255,4 +257,10 @@ class Projects::JobsController < Projects::ApplicationController
 
     ::Gitlab::Workhorse.channel_websocket(service)
   end
+
+  def build_details
+    BuildSerializer
+      .new(project: @project, current_user: @current_user)
+      .represent(@build, {}, BuildDetailsEntity)
+  end
 end

spec/controllers/projects/jobs_controller_spec.rb

@@ -711,11 +711,7 @@ RSpec.describe Projects::JobsController, :clean_gitlab_redis_shared_state do
       let(:job) { create(:ci_build, pipeline: pipeline) }
 
       it 'returns no traces' do
-        expect(response).to have_gitlab_http_status(:ok)
-        expect(response).to match_response_schema('job/build_trace')
-        expect(json_response['id']).to eq job.id
-        expect(json_response['status']).to eq job.status
-        expect(json_response['lines']).to be_nil
+        expect(response).to have_gitlab_http_status(:no_content)
       end
     end
 

spec/features/projects/jobs_spec.rb

@@ -1212,9 +1212,11 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
   end
 
   describe "GET /:project/jobs/:id/trace.json" do
+    let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
+
     context "Job from project" do
       before do
-        visit trace_project_job_path(project, job, format: :json)
+        visit trace_project_job_path(project, build, format: :json)
       end
 
       it { expect(page.status_code).to eq(200) }

spec/features/security/project/internal_access_spec.rb

@@ -430,7 +430,7 @@ RSpec.describe "Internal Project Access" do
 
   describe 'GET /:project_path/builds/:id/trace' do
     let(:pipeline) { create(:ci_pipeline, project: project) }
-    let(:build) { create(:ci_build, pipeline: pipeline) }
+    let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
 
     subject { trace_project_job_path(project, build.id) }
 

spec/features/security/project/private_access_spec.rb

@@ -423,7 +423,7 @@ RSpec.describe "Private Project Access" do
 
   describe 'GET /:project_path/builds/:id/trace' do
     let(:pipeline) { create(:ci_pipeline, project: project) }
-    let(:build) { create(:ci_build, pipeline: pipeline) }
+    let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
 
     subject { trace_project_job_path(project, build.id) }
 

spec/features/security/project/public_access_spec.rb

@@ -238,7 +238,7 @@ RSpec.describe "Public Project Access" do
 
   describe 'GET /:project_path/builds/:id/trace' do
     let(:pipeline) { create(:ci_pipeline, project: project) }
-    let(:build) { create(:ci_build, pipeline: pipeline) }
+    let(:build) { create(:ci_build, :trace_artifact, pipeline: pipeline) }
 
     subject { trace_project_job_path(project, build.id) }
 

spec/frontend/jobs/store/actions_spec.js

@@ -27,7 +27,6 @@ import {
   hideSidebar,
   showSidebar,
   toggleSidebar,
-  triggerManualJob,
 } from '~/jobs/store/actions';
 import state from '~/jobs/store/state';
 import * as types from '~/jobs/store/mutation_types';
@@ -159,32 +158,6 @@ describe('Job State actions', () => {
         );
       });
     });
-
-    it('fetchTrace is called only if the job has started or has a trace', (done) => {
-      mock.onGet(`${TEST_HOST}/endpoint.json`).replyOnce(200, { id: 121212, name: 'karma' });
-
-      mockedState.job.started = true;
-
-      testAction(
-        fetchJob,
-        null,
-        mockedState,
-        [],
-        [
-          {
-            type: 'requestJob',
-          },
-          {
-            payload: { id: 121212, name: 'karma' },
-            type: 'receiveJobSuccess',
-          },
-          {
-            type: 'fetchTrace',
-          },
-        ],
-        done,
-      );
-    });
   });
 
   describe('receiveJobSuccess', () => {
@@ -536,43 +509,4 @@ describe('Job State actions', () => {
       );
     });
   });
-
-  describe('triggerManualJob', () => {
-    let mock;
-
-    beforeEach(() => {
-      mock = new MockAdapter(axios);
-    });
-
-    afterEach(() => {
-      mock.restore();
-    });
-
-    it('should dispatch fetchTrace', (done) => {
-      const playManualJobEndpoint = `${TEST_HOST}/manual-job/jobs/1000/play`;
-
-      mock.onPost(playManualJobEndpoint).reply(200);
-
-      mockedState.job = {
-        status: {
-          action: {
-            path: playManualJobEndpoint,
-          },
-        },
-      };
-
-      testAction(
-        triggerManualJob,
-        [{ id: '1', key: 'test_var', secret_value: 'test_value' }],
-        mockedState,
-        [],
-        [
-          {
-            type: 'fetchTrace',
-          },
-        ],
-        done,
-      );
-    });
-  });
 });