Test all cycle analytics pre-calculation code.

All the code that pre-calculates metrics for use in the cycle analytics
page.

- Ci::Pipeline -> build start/finish
- Ci::Pipeline#merge_requests
- Issue -> record default metrics after save
- MergeRequest -> record default metrics after save
- Deployment -> Update "first_deployed_to_production_at" for MR metrics
- Git Push -> Update "first commit mention" for issue metrics
- Merge request create/update/refresh -> Update "merge requests closing issues"

app/models/ci/pipeline.rb

@@ -283,7 +283,7 @@ module Ci
     # the merge request's latest commit.
     def merge_requests
       project.merge_requests.where(source_branch: self.ref).
-        select { |merge_request| merge_request.pipeline.id == self.id }
+        select { |merge_request| merge_request.pipeline.try(:id) == self.id }
     end
     private

app/models/merge_request.rb

@@ -503,6 +503,7 @@ class MergeRequest < ActiveRecord::Base
   # running `ReferenceExtractor` on each of them separately.
   def cache_merge_request_closes_issues!(current_user = self.author)
     transaction do
+      self.merge_requests_closing_issues.destroy_all
       closes_issues(current_user).each do |issue|
         MergeRequestsClosingIssues.create!(merge_request: self, issue: issue)
       end

app/services/create_deployment_service.rb

@@ -23,17 +23,26 @@ class CreateDeploymentService < BaseService
   private
   def update_merge_request_metrics(deployment, environment)
-    # TODO: Test cases:
-    # 1. Merge request with metrics available and `first_deployed_to_production_at` is nil
-    # 2. Merge request with metrics available and `first_deployed_to_production_at` is set
-    # 3. Merge request with metrics unavailable
-    # 4. Only applies to merge requests merged AFTER the previous production deploy to this branch
     if environment.name == "production"
-      merge_requests_deployed_to_production_for_first_time = project.merge_requests.joins("LEFT OUTER JOIN merge_request_metrics ON merge_request_metrics.merge_request_id = merge_requests.id").
-                                                             where(target_branch: params[:ref], "merge_request_metrics.first_deployed_to_production_at" => nil).
-                                                             where("merge_request_metrics.merged_at < ?", deployment.created_at)
+      query = project.merge_requests.joins("LEFT OUTER JOIN merge_request_metrics ON merge_request_metrics.merge_request_id = merge_requests.id").
+              where(target_branch: params[:ref], "merge_request_metrics.first_deployed_to_production_at" => nil)
+
+      previous_deployment = previous_deployment_for_ref(deployment)
+      merge_requests_deployed_to_production_for_first_time = if previous_deployment
+                                                               query.where("merge_request_metrics.merged_at < ? AND merge_request_metrics.merged_at > ?", deployment.created_at, previous_deployment.created_at)
+                                                             else
+                                                               query.where("merge_request_metrics.merged_at < ?", deployment.created_at)
+                                                             end
       merge_requests_deployed_to_production_for_first_time.each { |merge_request| merge_request.metrics.record_production_deploy!(deployment.created_at) }
     end
   end
+
+  def previous_deployment_for_ref(current_deployment)
+    @previous_deployment_for_ref ||=
+      project.deployments.joins(:environment).
+      where("environments.name": params[:environment], ref: params[:ref]).
+      where.not(id: current_deployment.id).
+      first
+  end
 end

app/services/git_push_service.rb

@@ -188,7 +188,6 @@ class GitPushService < BaseService
     @branch_name ||= Gitlab::Git.ref_name(params[:ref])
   end
-  # TODO: What about commits created using the Web UI? Cross references are not created there.
   def update_issue_metrics(commit, authors)
     mentioned_issues = commit.all_references(authors[commit]).issues
     mentioned_issues.each { |issue| issue.metrics.record_commit_mention!(commit.committed_date) if issue.metrics.present? }

spec/models/ci/pipeline_spec.rb

@@ -169,6 +169,37 @@ describe Ci::Pipeline, models: true do
         expect(pipeline.reload.finished_at).to be_nil
       end
     end
+
+    describe "merge request metrics" do
+      let(:project) { FactoryGirl.create :project }
+      let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) }
+      let!(:merge_request) { create(:merge_request, source_project: project, source_branch: pipeline.ref) }
+
+      context 'when transitioning to running' do
+        it 'records the build start time' do
+          time = Time.now
+          Timecop.freeze(time) { build.run }
+
+          expect(merge_request.metrics.latest_build_started_at).to eq(time)
+        end
+
+        it 'clears the build end time' do
+          build.run
+
+          expect(merge_request.metrics.latest_build_finished_at).to be_nil
+        end
+      end
+
+      context 'when transitioning to success' do
+        it 'records the build end time' do
+          build.run
+          time = Time.now
+          Timecop.freeze(time) { build.success }
+
+          expect(merge_request.metrics.latest_build_finished_at).to eq(time)
+        end
+      end
+    end
   end
   describe '#branch?' do
@@ -429,4 +460,28 @@ describe Ci::Pipeline, models: true do
       create(:ci_build, :created, pipeline: pipeline, name: name)
     end
   end
+
+  describe "#merge_requests" do
+    let(:project) { FactoryGirl.create :project }
+    let(:pipeline) { FactoryGirl.create(:ci_empty_pipeline, status: 'created', project: project, ref: 'master', sha: project.repository.commit('master').id) }
+
+    it "returns merge requests whose `diff_head_sha` matches the pipeline's SHA" do
+      merge_request = create(:merge_request, source_project: project, source_branch: pipeline.ref)
+
+      expect(pipeline.merge_requests).to eq([merge_request])
+    end
+
+    it "doesn't return merge requests whose source branch doesn't match the pipeline's ref" do
+      create(:merge_request, source_project: project, source_branch: 'feature', target_branch: 'master')
+
+      expect(pipeline.merge_requests).to be_empty
+    end
+
+    it "doesn't return merge requests whose `diff_head_sha` doesn't match the pipeline's SHA" do
+      create(:merge_request, source_project: project, source_branch: pipeline.ref)
+      allow_any_instance_of(MergeRequest).to receive(:diff_head_sha) { '97de212e80737a608d939f648d959671fb0a0142b' }
+
+      expect(pipeline.merge_requests).to be_empty
+    end
+  end
 end

spec/models/issue/metrics_spec.rb

+require 'spec_helper'
+
+describe Issue::Metrics, models: true do
+  let(:project) { create(:project) }
+
+  subject { create(:issue, project: project) }
+
+  describe "when recording the default set of issue metrics on issue save" do
+    context "milestones" do
+      it "records the first time an issue is associated with a milestone" do
+        time = Time.now
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        metrics = subject.metrics
+
+        expect(metrics).to be_present
+        expect(metrics.first_associated_with_milestone_at).to eq(time)
+      end
+
+      it "does not record the second time an issue is associated with a milestone" do
+        time = Time.now
+        Timecop.freeze(time) { subject.update(milestone: create(:milestone)) }
+        Timecop.freeze(time + 2.hours) { subject.update(milestone: nil) }
+        Timecop.freeze(time + 6.hours) { subject.update(milestone: create(:milestone)) }
+        metrics = subject.metrics
+
+        expect(metrics).to be_present
+        expect(metrics.first_associated_with_milestone_at).to eq(time)
+      end
+    end
+
+    context "list labels" do
+      it "records the first time an issue is associated with a list label" do
+        list_label = create(:label, lists: [create(:list)])
+        time = Time.now
+        Timecop.freeze(time) { subject.update(label_ids: [list_label.id]) }
+        metrics = subject.metrics
+
+        expect(metrics).to be_present
+        expect(metrics.first_added_to_board_at).to eq(time)
+      end
+
+      it "does not record the second time an issue is associated with a list label" do
+        time = Time.now
+        first_list_label = create(:label, lists: [create(:list)])
+        Timecop.freeze(time) { subject.update(label_ids: [first_list_label.id]) }
+        second_list_label = create(:label, lists: [create(:list)])
+        Timecop.freeze(time + 5.hours) { subject.update(label_ids: [second_list_label.id]) }
+        metrics = subject.metrics
+
+        expect(metrics).to be_present
+        expect(metrics.first_added_to_board_at).to eq(time)
+      end
+    end
+  end
+end

spec/models/merge_request/metrics_spec.rb

+require 'spec_helper'
+
+describe MergeRequest::Metrics, models: true do
+  let(:project) { create(:project) }
+
+  subject { create(:merge_request, source_project: project) }
+
+  describe "when recording the default set of metrics on merge request save" do
+    it "records the merge time" do
+      time = Time.now
+      Timecop.freeze(time) { subject.mark_as_merged }
+      metrics = subject.metrics
+
+      expect(metrics).to be_present
+      expect(metrics.merged_at).to eq(time)
+    end
+  end
+end

spec/services/create_deployment_service_spec.rb

@@ -132,4 +132,80 @@ describe CreateDeploymentService, services: true do
       end
     end
   end
+
+  describe "merge request metrics" do
+    let(:params) do
+      {
+        environment: 'production',
+        ref: 'master',
+        tag: false,
+        sha: '97de212e80737a608d939f648d959671fb0a0142b',
+      }
+    end
+
+    let(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: project) }
+
+    context "while updating the 'first_deployed_to_production_at' time" do
+      before { merge_request.mark_as_merged }
+
+      context "for merge requests merged before the current deploy" do
+        it "sets the time if the deploy's environment is 'production'" do
+          time = Time.now
+          Timecop.freeze(time) { service.execute }
+
+          expect(merge_request.metrics.first_deployed_to_production_at).to eq(time)
+        end
+
+        it "doesn't set the time if the deploy's environment is not 'production'" do
+          staging_params = params.merge(environment: 'staging')
+          service = described_class.new(project, user, staging_params)
+          service.execute
+
+          expect(merge_request.metrics.first_deployed_to_production_at).to be_nil
+        end
+
+        it 'does not raise errors if the merge request does not have a metrics record' do
+          merge_request.metrics.destroy
+
+          expect(merge_request.reload.metrics).to be_nil
+          expect { service.execute }.not_to raise_error
+        end
+      end
+
+      context "for merge requests merged before the previous deploy" do
+        context "if the 'first_deployed_to_production_at' time is already set" do
+          it "does not overwrite the older 'first_deployed_to_production_at' time" do
+            # Previous deploy
+            time = Time.now
+            Timecop.freeze(time) { service.execute }
+
+            expect(merge_request.reload.metrics.first_deployed_to_production_at).to eq(time)
+
+            # Current deploy
+            service = described_class.new(project, user, params)
+            Timecop.freeze(time + 12.hours) { service.execute }
+
+            expect(merge_request.reload.metrics.first_deployed_to_production_at).to eq(time)
+          end
+        end
+
+        context "if the 'first_deployed_to_production_at' time is not already set" do
+          it "does not overwrite the older 'first_deployed_to_production_at' time" do
+            # Previous deploy
+            time = Time.now
+            Timecop.freeze(time) { service.execute }
+            merge_request.metrics.update(first_deployed_to_production_at: nil)
+
+            expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil
+
+            # Current deploy
+            service = described_class.new(project, user, params)
+            Timecop.freeze(time + 12.hours) { service.execute }
+
+            expect(merge_request.reload.metrics.first_deployed_to_production_at).to be_nil
+          end
+        end
+      end
+    end
+  end
 end

spec/services/git_push_service_spec.rb

@@ -324,6 +324,43 @@ describe GitPushService, services: true do
     end
   end
+  describe "issue metrics" do
+    let(:issue) { create :issue, project: project }
+    let(:commit_author) { create :user }
+    let(:commit) { project.commit }
+    let(:commit_time) { Time.now }
+
+    before do
+      project.team << [commit_author, :developer]
+      project.team << [user, :developer]
+
+      allow(commit).to receive_messages(
+        safe_message: "this commit \n mentions #{issue.to_reference}",
+        references: [issue],
+        author_name: commit_author.name,
+        author_email: commit_author.email,
+        committed_date: commit_time
+      )
+
+      allow(project.repository).to receive(:commits_between).and_return([commit])
+    end
+
+    context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do
+      it 'sets the metric for referenced issues' do
+        execute_service(project, user, @oldrev, @newrev, @ref)
+
+        expect(issue.metrics.first_mentioned_in_commit_at).to eq(commit_time)
+      end
+
+      it 'does not set the metric for non-referenced issues' do
+        non_referenced_issue = create(:issue, project: project)
+        execute_service(project, user, @oldrev, @newrev, @ref)
+
+        expect(non_referenced_issue.metrics.first_mentioned_in_commit_at).to be_nil
+      end
+    end
+  end
+
   describe "closing issues from pushed commits containing a closing reference" do
     let(:issue) { create :issue, project: project }
     let(:other_issue) { create :issue, project: project }

spec/services/merge_requests/create_service_spec.rb

@@ -83,5 +83,33 @@ describe MergeRequests::CreateService, services: true do
         }
       end
     end
+
+    context 'while saving references to issues that the created merge request closes' do
+      let(:first_issue) { create(:issue, project: project) }
+      let(:second_issue) { create(:issue, project: project) }
+
+      let(:opts) do
+        {
+          title: 'Awesome merge_request',
+          source_branch: 'feature',
+          target_branch: 'master',
+          force_remove_source_branch: '1'
+        }
+      end
+
+      before do
+        project.team << [user, :master]
+        project.team << [assignee, :developer]
+      end
+
+      it 'creates a `MergeRequestsClosingIssues` record for each issue' do
+        issue_closing_opts = opts.merge(description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}")
+        service = described_class.new(project, user, issue_closing_opts)
+        allow(service).to receive(:execute_hooks)
+        merge_request = service.execute
+
+        expect(merge_request.issues_closed).to match_array([first_issue, second_issue])
+      end
+    end
   end
 end

spec/services/merge_requests/refresh_service_spec.rb

@@ -174,6 +174,36 @@ describe MergeRequests::RefreshService, services: true do
       end
     end
+    context 'push commits closing issues' do
+      let(:issue) { create :issue, project: @project }
+      let(:commit_author) { create :user }
+      let(:commit) { project.commit }
+      let!(:merge_request) { create(:merge_request, target_branch: 'master', source_branch: 'feature', source_project: @project) }
+
+      before do
+        project.team << [commit_author, :developer]
+        project.team << [user, :developer]
+
+        allow(commit).to receive_messages(
+          safe_message: "Closes #{issue.to_reference}",
+          references: [issue],
+          author_name: commit_author.name,
+          author_email: commit_author.email,
+          committed_date: Time.now
+        )
+
+        allow_any_instance_of(MergeRequest).to receive(:commits).and_return([commit])
+      end
+
+      it 'creates a `MergeRequestsClosingIssues` record for each closed issue' do
+        refresh_service = service.new(@project, @user)
+        allow(refresh_service).to receive(:execute_hooks)
+        refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature')
+
+        expect(merge_request.reload.issues_closed).to eq([issue])
+      end
+    end
+
     def reload_mrs
       @merge_request.reload
       @fork_merge_request.reload

spec/services/merge_requests/update_service_spec.rb

@@ -263,5 +263,39 @@ describe MergeRequests::UpdateService, services: true do
         end
       end
     end
+
+    context 'while saving references to issues that the updated merge request closes' do
+      let(:first_issue) { create(:issue, project: project) }
+      let(:second_issue) { create(:issue, project: project) }
+
+      it 'creates a `MergeRequestsClosingIssues` record for each issue' do
+        issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" }
+        service = described_class.new(project, user, issue_closing_opts)
+        allow(service).to receive(:execute_hooks)
+        service.execute(merge_request)
+
+        expect(merge_request.reload.issues_closed).to match_array([first_issue, second_issue])
+      end
+    end
+
+      it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do
+        opts = {
+          title: 'Awesome merge_request',
+          description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}",
+          source_branch: 'feature',
+          target_branch: 'master',
+          force_remove_source_branch: '1'
+        }
+
+        merge_request = MergeRequests::CreateService.new(project, user, opts).execute
+
+        expect(merge_request.issues_closed).to match_array([first_issue, second_issue])
+
+        service = described_class.new(project, user, description: "not closing any issues")
+        allow(service).to receive(:execute_hooks)
+        service.execute(merge_request.reload)
+
+        expect(merge_request.reload.issues_closed).to be_empty
+      end
   end
 end