Incorporate review

app/models/ci/build.rb

@@ -205,13 +205,13 @@ module Ci
 
     def merge_request
       return @merge_request if defined?(@merge_request)
-      
+
       @merge_request ||=
         begin
           merge_requests = MergeRequest.includes(:merge_request_diff)
             .where(source_branch: ref,
                    source_project: pipeline.project)
-            .reorder(iid: :asc)
+            .reorder(iid: :desc)
 
           merge_requests.find do |merge_request|
             merge_request.commits_sha.include?(pipeline.sha)
@@ -372,7 +372,7 @@ module Ci
     end
 
     def has_expiring_artifacts?
-      artifacts_expire_at.present?
+      artifacts_expire_at.present? && artifacts_expire_at > Time.now
     end
 
     def keep_artifacts!

app/serializers/build_artifact_entity.rb

 class BuildArtifactEntity < Grape::Entity
   include RequestAwareEntity
 
-  expose :name do |build|
-    build.name
+  expose :name do |job|
+    job.name
   end
 
-  expose :path do |build|
+  expose :artifacts_expired?, as: :expired
+  expose :artifacts_expire_at, as: :expire_at
+
+  expose :path do |job|
     download_namespace_project_job_artifacts_path(
       build.project.namespace,
       build.project,
       build)
   end
+
+  expose :keep_path, if: -> (*) { job.has_expiring_artifacts? } do |job|
+    keep_namespace_project_job_artifacts_path(
+      project.namespace,
+      project,
+      build)
+  end
+
+  expose :browse_path do |job|
+    browse_namespace_project_job_artifacts_path(
+      project.namespace,
+      project,
+      job)
+  end
+
+  private
+
+  alias_method :job, :object
+
+  def project
+    job.project
+  end
 end

app/serializers/build_details_entity.rb

@@ -2,12 +2,23 @@ class BuildDetailsEntity < BuildEntity
   expose :coverage, :erased_at, :duration
   expose :tag_list, as: :tags
 
+  expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity
+  expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :update_build, project) } do |build|
+    erase_namespace_project_build_path(project.namespace, project, build)
+  end
+
   expose :artifacts, using: BuildArtifactEntity
   expose :runner, using: RunnerEntity
   expose :pipeline, using: PipelineEntity
 
-  expose :merge_request_path, if: -> (*) { can?(current_user, :read_merge_request, project) } do |build|
-    namespace_project_merge_request_path(project.namespace, project, build.merge_request)
+  expose :merge_request, if: -> (*) { can?(current_user, :read_merge_request, project) } do
+    expose :iid do |build|
+      build.merge_request.iid
+    end
+
+    expose :path do |build|
+      namespace_project_merge_request_path(project.namespace, project, build.merge_request)
+    end
   end
 
   expose :new_issue_path, if: -> (*) { can?(request.current_user, :create_issue, project) } do |build|

app/serializers/build_serializer.rb

 class BuildSerializer < BaseSerializer
   entity BuildEntity
 
-  def represent_status(resource, opts = {}, entity_class = nil)
+  def represent_status(resource)
     data = represent(resource, { only: [:status] })
     data.fetch(:status, {})
   end

app/serializers/merge_request_entity.rb

@@ -29,7 +29,7 @@ class MergeRequestEntity < IssuableEntity
 
   expose :merge_commit_sha
   expose :merge_commit_message
-  expose :head_pipeline, with: PipelineEntity, as: :pipeline
+  expose :head_pipeline, with: PipelineDetailsEntity, as: :pipeline
 
   # Booleans
   expose :work_in_progress?, as: :work_in_progress

app/serializers/pipeline_details_entity.rb

 class PipelineDetailsEntity < PipelineEntity
-  expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? }
-  
   expose :details do
-    expose :detailed_status, as: :status, with: StatusEntity
-    expose :duration
-    expose :finished_at
     expose :stages, using: StageEntity
     expose :artifacts, using: BuildArtifactEntity
     expose :manual_actions, using: BuildActionEntity
   end
-
-  expose :flags do
-    expose :latest?, as: :latest
-    expose :triggered?, as: :triggered
-    expose :stuck?, as: :stuck
-    expose :has_yaml_errors?, as: :yaml_errors
-    expose :can_retry?, as: :retryable
-    expose :can_cancel?, as: :cancelable
-  end
 end

app/serializers/pipeline_entity.rb

@@ -15,6 +15,21 @@ class PipelineEntity < Grape::Entity
       pipeline)
   end
 
+  expose :flags do
+    expose :latest?, as: :latest
+    expose :triggered?, as: :triggered
+    expose :stuck?, as: :stuck
+    expose :has_yaml_errors?, as: :yaml_errors
+    expose :can_retry?, as: :retryable
+    expose :can_cancel?, as: :cancelable
+  end
+
+  expose :details do
+    expose :detailed_status, as: :status, with: StatusEntity
+    expose :duration
+    expose :finished_at
+  end
+
   expose :ref do
     expose :name do |pipeline|
       pipeline.ref
@@ -44,6 +59,8 @@ class PipelineEntity < Grape::Entity
                                            pipeline.id)
   end
 
+  expose :yaml_errors, if: -> (pipeline, _) { pipeline.has_yaml_errors? }
+
   private
 
   alias_method :pipeline, :object

app/serializers/runner_entity.rb

@@ -3,13 +3,15 @@ class RunnerEntity < Grape::Entity
 
   expose :id, :description
 
-  expose :edit_runner_path,
-    if: -> (*) { can?(request.current_user, :admin_build, project)  } do |runner|
+  expose :edit_path,
+    if: -> (*) { can?(request.current_user, :admin_build, project) && runner.specific? } do |runner|
     edit_namespace_project_runner_path(project.namespace, project, runner)
   end
 
   private
 
+  alias_method :runner, :object
+
   def project
     request.project
   end

spec/controllers/projects/jobs_controller_spec.rb

@@ -142,7 +142,7 @@ describe Projects::JobsController do
         expect(response).to have_http_status(:ok)
         expect(json_response['new_issue_path']).to end_with('/issues/new')
         expect(json_response['raw_path']).to match(/builds\/\d+\/raw\z/)
-        expect(json_response['merge_request_path']).to match(/merge_requests\/\d+\z/)
+        expect(json_response.dig('merge_request', 'path')).to match(/merge_requests\/\d+\z/)
       end
     end
 

spec/serializers/build_artifact_entity_spec.rb

 require 'spec_helper'
 
 describe BuildArtifactEntity do
-  let(:job) { create(:ci_build, name: 'test:job') }
+  let(:job) { create(:ci_build, name: 'test:job', artifacts_expire_at: 1.hour.from_now) }
 
   let(:entity) do
     described_class.new(job, request: double)
@@ -14,9 +14,19 @@ describe BuildArtifactEntity do
       expect(subject[:name]).to eq 'test:job'
     end
 
-    it 'contains path to the artifacts' do
+    it 'exposes information about expiration of artifacts' do
+      expect(subject).to include(:expired, :expire_at)
+    end
+
+    it 'contains paths to the artifacts' do
       expect(subject[:path])
         .to include "jobs/#{job.id}/artifacts/download"
+
+      expect(subject[:keep_path])
+        .to include "jobs/#{build.id}/artifacts/keep"
+
+      expect(subject[:browse_path])
+        .to include "jobs/#{build.id}/artifacts/browse"
     end
   end
 end

spec/serializers/build_details_entity_spec.rb

 require 'spec_helper'
 
 describe BuildDetailsEntity do
+  set(:user) { create(:admin) }
+
   it 'inherits from BuildEntity' do
     expect(described_class).to be < BuildEntity
   end
@@ -17,7 +19,6 @@ describe BuildDetailsEntity do
     end
 
     context 'when the user has access to issues and merge requests' do
-      let(:user) { create(:admin) }
       let!(:merge_request) do
         create(:merge_request, source_project: project, source_branch: build.ref)
       end
@@ -29,7 +30,27 @@ describe BuildDetailsEntity do
       it 'contains the needed key value pairs' do
         expect(subject).to include(:coverage, :erased_at, :duration)
         expect(subject).to include(:artifacts, :runner, :pipeline)
-        expect(subject).to include(:raw_path, :merge_request_path, :new_issue_path)
+        expect(subject).to include(:raw_path, :merge_request, :new_issue_path)
+      end
+
+      it 'exposes details of the merge request' do
+        expect(subject[:merge_request]).to include(:iid, :path)
+      end
+
+      context 'when the build has been erased' do
+        let!(:build) { create(:ci_build, :erasable, project: project) }
+
+        it 'exposes the user whom erased the build' do
+          expect(subject).to include(:erase_path)
+        end
+      end
+
+      context 'when the build has been erased' do
+        let!(:build) { create(:ci_build, erased_at: Time.now, project: project, erased_by: user) }
+
+        it 'exposes the user whom erased the build' do
+          expect(subject).to include(:erased_by)
+        end
       end
     end
 

spec/serializers/merge_request_entity_spec.rb

@@ -26,7 +26,7 @@ describe MergeRequestEntity do
     pipeline = build_stubbed(:ci_pipeline)
     allow(resource).to receive(:head_pipeline).and_return(pipeline)
 
-    pipeline_payload = PipelineEntity
+    pipeline_payload = PipelineDetailsEntity
       .represent(pipeline, request: req)
       .as_json
 

spec/serializers/runner_entity_spec.rb

 require 'spec_helper'
 
 describe RunnerEntity do
-  let(:runner) { create(:ci_runner) }
+  let(:runner) { create(:ci_runner, :specific) }
   let(:entity) { described_class.new(runner, request: request, current_user: user) }
   let(:request) { double('request') }
   let(:project) { create(:empty_project) }
@@ -17,7 +17,7 @@ describe RunnerEntity do
 
     it 'contains required fields' do
       expect(subject).to include(:id, :description)
-      expect(subject).to include(:edit_runner_path)
+      expect(subject).to include(:edit_path)
     end
   end
 end