Merge branch '32773-include-test-results-within-release-evidence-4' into 'master'

Move release evidence generation to service

See merge request gitlab-org/gitlab!32927

app/models/releases/evidence.rb

@@ -6,8 +6,6 @@ class Releases::Evidence < ApplicationRecord
 
   belongs_to :release, inverse_of: :evidences
 
-  before_validation :generate_summary_and_sha
-
   default_scope { order(created_at: :asc) }
 
   sha_attribute :summary_sha
@@ -31,14 +29,4 @@ class Releases::Evidence < ApplicationRecord
 
     safe_summary
   end
-
-  private
-
-  def generate_summary_and_sha
-    summary = Evidences::EvidenceSerializer.new.represent(self) # rubocop: disable CodeReuse/Serializer
-    return unless summary
-
-    self.summary = summary
-    self.summary_sha = Gitlab::CryptoHelper.sha256(summary)
-  end
 end

app/services/releases/create_evidence_service.rb

+# frozen_string_literal: true
+
+module Releases
+  class CreateEvidenceService
+    def initialize(release)
+      @release = release
+    end
+
+    def execute
+      evidence = release.evidences.build
+
+      summary = Evidences::EvidenceSerializer.new.represent(evidence) # rubocop: disable CodeReuse/Serializer
+      evidence.summary = summary
+      # TODO: fix the sha generating https://gitlab.com/gitlab-org/gitlab/-/issues/209000
+      evidence.summary_sha = Gitlab::CryptoHelper.sha256(summary)
+
+      evidence.save!
+    end
+
+    private
+
+    attr_reader :release
+  end
+end

app/workers/create_evidence_worker.rb

@@ -10,6 +10,6 @@ class CreateEvidenceWorker # rubocop:disable Scalability/IdempotentWorker
     release = Release.find_by_id(release_id)
     return unless release
 
-    Releases::Evidence.create!(release: release)
+    ::Releases::CreateEvidenceService.new(release).execute
   end
 end

spec/controllers/projects/releases/evidences_controller_spec.rb

@@ -31,8 +31,8 @@ describe Projects::Releases::EvidencesController do
   end
 
   describe 'GET #show' do
-    let_it_be(:tag_name) { "v1.1.0-evidence" }
-    let!(:release) { create(:release, :with_evidence, project: project, tag: tag_name) }
+    let(:tag_name) { "v1.1.0-evidence" }
+    let!(:release) { create(:release, project: project, tag: tag_name) }
     let(:evidence) { release.evidences.first }
     let(:tag) { CGI.escape(release.tag) }
     let(:format) { :json }
@@ -48,6 +48,8 @@ describe Projects::Releases::EvidencesController do
     end
 
     before do
+      ::Releases::CreateEvidenceService.new(release).execute
+
       sign_in(user)
     end
 
@@ -84,14 +86,9 @@ describe Projects::Releases::EvidencesController do
     end
 
     context 'when release is associated to a milestone which includes an issue' do
-      let_it_be(:project) { create(:project, :repository, :public) }
-      let_it_be(:issue) { create(:issue, project: project) }
-      let_it_be(:milestone) { create(:milestone, project: project, issues: [issue]) }
-      let_it_be(:release) { create(:release, project: project, tag: tag_name, milestones: [milestone]) }
-
-      before do
-        create(:evidence, release: release)
-      end
+      let(:issue) { create(:issue, project: project) }
+      let(:milestone) { create(:milestone, project: project, issues: [issue]) }
+      let(:release) { create(:release, project: project, tag: tag_name, milestones: [milestone]) }
 
       shared_examples_for 'does not show the issue in evidence' do
         it do
@@ -111,7 +108,9 @@ describe Projects::Releases::EvidencesController do
         end
       end
 
-      shared_examples_for 'safely expose evidence' do
+      context 'when user is non-project member' do
+        let(:user) { create(:user) }
+
         it_behaves_like 'does not show the issue in evidence'
 
         context 'when the issue is confidential' do
@@ -127,28 +126,50 @@ describe Projects::Releases::EvidencesController do
         end
 
         context 'when project is private' do
-          let!(:project) { create(:project, :repository, :private) }
+          let(:project) { create(:project, :repository, :private) }
 
           it_behaves_like 'evidence not found'
         end
 
         context 'when project restricts the visibility of issues to project members only' do
-          let!(:project) { create(:project, :repository, :issues_private) }
+          let(:project) { create(:project, :repository, :issues_private) }
 
           it_behaves_like 'evidence not found'
         end
       end
 
-      context 'when user is non-project member' do
-        let(:user) { create(:user) }
-
-        it_behaves_like 'safely expose evidence'
-      end
-
       context 'when user is auditor', if: Gitlab.ee? do
         let(:user) { create(:user, :auditor) }
 
-        it_behaves_like 'safely expose evidence'
+        it_behaves_like 'does not show the issue in evidence'
+
+        context 'when the issue is confidential' do
+          let(:issue) { create(:issue, :confidential, project: project) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when the user is the author of the confidential issue' do
+          let(:issue) { create(:issue, :confidential, project: project, author: user) }
+
+          it_behaves_like 'does not show the issue in evidence'
+        end
+
+        context 'when project is private' do
+          let(:project) { create(:project, :repository, :private) }
+
+          it 'returns evidence ' do
+            subject
+
+            expect(json_response).to eq(evidence.summary)
+          end
+        end
+
+        context 'when project restricts the visibility of issues to project members only' do
+          let(:project) { create(:project, :repository, :issues_private) }
+
+          it_behaves_like 'evidence not found'
+        end
       end
 
       context 'when external authorization control is enabled' do

spec/features/projects/releases/user_views_releases_spec.rb

@@ -80,7 +80,7 @@ describe 'User views releases', :js do
 
     context 'with a tag containing a slash' do
       it 'sees the release' do
-        release = create :release, :with_evidence, project: project, tag: 'debian/2.4.0-1'
+        release = create :release, project: project, tag: 'debian/2.4.0-1'
         visit project_releases_path(project)
 
         expect(page).to have_content(release.name)

spec/fixtures/api/schemas/evidences/issue.json

@@ -4,7 +4,6 @@
     "id",
     "title",
     "description",
-    "author",
     "state",
     "iid",
     "confidential",

spec/fixtures/api/schemas/evidences/milestone.json

@@ -7,7 +7,8 @@
     "state",
     "iid",
     "created_at",
-    "due_date"
+    "due_date",
+    "issues"
   ],
   "properties": {
     "id": { "type": "integer" },
@@ -16,7 +17,11 @@
     "state": { "type": "string" },
     "iid": { "type": "integer" },
     "created_at": { "type": "date" },
-    "due_date": { "type": ["date", "null"] }
+    "due_date": { "type": ["date", "null"] },
+    "issues": {
+      "type": "array",
+      "items": { "$ref": "issue.json" }
+    }
   },
   "additionalProperties": false
 }

spec/lib/api/entities/release_spec.rb

@@ -4,11 +4,15 @@ require 'spec_helper'
 
 describe API::Entities::Release do
   let_it_be(:project) { create(:project) }
-  let_it_be(:release) { create(:release, :with_evidence, project: project) }
+  let(:release) { create(:release, project: project) }
   let(:evidence) { release.evidences.first }
   let(:user) { create(:user) }
   let(:entity) { described_class.new(release, current_user: user).as_json }
 
+  before do
+    ::Releases::CreateEvidenceService.new(release).execute
+  end
+
   describe 'evidences' do
     context 'when the current user can download code' do
       let(:entity_evidence) { entity[:evidences].first }

spec/models/release_spec.rb

@@ -94,14 +94,6 @@ RSpec.describe Release do
   describe 'evidence' do
     let(:release_with_evidence) { create(:release, :with_evidence, project: project) }
 
-    describe '#create_evidence!' do
-      context 'when a release is created' do
-        it 'creates one Evidence object too' do
-          expect { release_with_evidence }.to change(Releases::Evidence, :count).by(1)
-        end
-      end
-    end
-
     context 'when a release is deleted' do
       it 'also deletes the associated evidence' do
         release_with_evidence

spec/models/releases/evidence_spec.rb

@@ -5,83 +5,21 @@ require 'spec_helper'
 describe Releases::Evidence do
   let_it_be(:project) { create(:project) }
   let(:release) { create(:release, project: project) }
-  let(:schema_file) { 'evidences/evidence' }
-  let(:summary_json) { described_class.last.summary.to_json }
 
   describe 'associations' do
     it { is_expected.to belong_to(:release) }
   end
 
-  describe 'summary_sha' do
-    it 'returns nil if summary is nil' do
-      expect(build(:evidence, summary: nil).summary_sha).to be_nil
-    end
-  end
-
-  describe '#generate_summary_and_sha' do
-    before do
-      described_class.create!(release: release)
-    end
-
-    context 'when a release name is not provided' do
-      let(:release) { create(:release, project: project, name: nil) }
-
-      it 'creates a valid JSON object' do
-        expect(release.name).to eq(release.tag)
-        expect(summary_json).to match_schema(schema_file)
-      end
-    end
-
-    context 'when a release is associated to a milestone' do
-      let(:milestone) { create(:milestone, project: project) }
-      let(:release) { create(:release, project: project, milestones: [milestone]) }
-
-      context 'when a milestone has no issue associated with it' do
-        it 'creates a valid JSON object' do
-          expect(milestone.issues).to be_empty
-          expect(summary_json).to match_schema(schema_file)
-        end
-      end
-
-      context 'when a milestone has no description' do
-        let(:milestone) { create(:milestone, project: project, description: nil) }
-
-        it 'creates a valid JSON object' do
-          expect(milestone.description).to be_nil
-          expect(summary_json).to match_schema(schema_file)
-        end
-      end
-
-      context 'when a milestone has no due_date' do
-        let(:milestone) { create(:milestone, project: project, due_date: nil) }
-
-        it 'creates a valid JSON object' do
-          expect(milestone.due_date).to be_nil
-          expect(summary_json).to match_schema(schema_file)
-        end
-      end
-
-      context 'when a milestone has an issue' do
-        context 'when the issue has no description' do
-          let(:issue) { create(:issue, project: project, description: nil, state: 'closed') }
-
-          before do
-            milestone.issues << issue
-          end
+  it 'filters out issues from summary json' do
+    milestone = create(:milestone, project: project, due_date: nil)
+    issue = create(:issue, project: project, description: nil, state: 'closed')
+    milestone.issues << issue
+    release.milestones << milestone
 
-          it 'creates a valid JSON object' do
-            expect(milestone.issues.first.description).to be_nil
-            expect(summary_json).to match_schema(schema_file)
-          end
-        end
-      end
-    end
+    ::Releases::CreateEvidenceService.new(release).execute
+    evidence = release.evidences.last
 
-    context 'when a release is not associated to any milestone' do
-      it 'creates a valid JSON object' do
-        expect(release.milestones).to be_empty
-        expect(summary_json).to match_schema(schema_file)
-      end
-    end
+    expect(evidence.read_attribute(:summary)["release"]["milestones"].first["issues"].first["title"]).to be_present
+    expect(evidence.summary["release"]["milestones"].first["issues"]).to be_nil
   end
 end

spec/serializers/evidences/evidence_entity_spec.rb

@@ -3,12 +3,66 @@
 require 'spec_helper'
 
 describe Evidences::EvidenceEntity do
-  let(:evidence) { build(:evidence) }
-  let(:entity) { described_class.new(evidence) }
+  let_it_be(:project) { create(:project) }
+  let(:release) { create(:release, project: project) }
+  let(:evidence) { build(:evidence, release: release) }
+  let(:schema_file) { 'evidences/evidence' }
 
-  subject { entity.as_json }
+  subject { described_class.new(evidence).as_json }
 
   it 'exposes the expected fields' do
     expect(subject.keys).to contain_exactly(:release)
   end
+
+  context 'when a release is associated to a milestone' do
+    let(:milestone) { create(:milestone, project: project) }
+    let(:release) { create(:release, project: project, milestones: [milestone]) }
+
+    context 'when a milestone has no issue associated with it' do
+      it 'creates a valid JSON object' do
+        expect(subject[:release][:milestones].first[:issues]).to be_empty
+        expect(subject.to_json).to match_schema(schema_file)
+      end
+    end
+
+    context 'when a milestone has no description' do
+      let(:milestone) { create(:milestone, project: project, description: nil) }
+
+      it 'creates a valid JSON object' do
+        expect(subject[:release][:milestones].first[:description]).to be_nil
+        expect(subject.to_json).to match_schema(schema_file)
+      end
+    end
+
+    context 'when a milestone has no due_date' do
+      let(:milestone) { create(:milestone, project: project, due_date: nil) }
+
+      it 'creates a valid JSON object' do
+        expect(subject[:release][:milestones].first[:due_date]).to be_nil
+        expect(subject.to_json).to match_schema(schema_file)
+      end
+    end
+
+    context 'when a milestone has an issue' do
+      context 'when the issue has no description' do
+        let(:issue) { create(:issue, project: project, description: nil, state: 'closed') }
+
+        before do
+          milestone.issues << issue
+        end
+
+        it 'creates a valid JSON object' do
+          expect(subject[:release][:milestones].first[:issues].first[:title]).to be_present
+          expect(subject.to_json).to match_schema(schema_file)
+        end
+      end
+    end
+  end
+
+  context 'when a release is not associated to any milestone' do
+    it 'creates a valid JSON object' do
+      expect(subject[:release][:milestones]).to be_empty
+      expect(subject.to_json).to match_schema(schema_file)
+    end
+  end
 end

spec/services/releases/create_evidence_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Releases::CreateEvidenceService do
+  let_it_be(:project) { create(:project) }
+  let(:release) { create(:release, project: project) }
+  let(:service) { described_class.new(release) }
+
+  it 'creates evidence' do
+    expect { service.execute }.to change { release.reload.evidences.count }.by(1)
+  end
+
+  it 'saves evidence summary' do
+    service.execute
+    evidence = Releases::Evidence.last
+
+    expect(release.tag).not_to be_nil
+    expect(evidence.summary["release"]["tag_name"]).to eq(release.tag)
+  end
+
+  it 'saves sha' do
+    service.execute
+    evidence = Releases::Evidence.last
+
+    expect(evidence.summary_sha).not_to be_nil
+  end
+end

spec/workers/create_evidence_worker_spec.rb

@@ -3,9 +3,13 @@
 require 'spec_helper'
 
 describe CreateEvidenceWorker do
-  let!(:release) { create(:release) }
+  let(:release) { create(:release) }
 
   it 'creates a new Evidence record' do
+    expect_next_instance_of(::Releases::CreateEvidenceService, release) do |service|
+      expect(service).to receive(:execute).and_call_original
+    end
+
     expect { described_class.new.perform(release.id) }.to change(Releases::Evidence, :count).by(1)
   end
 end