Add latest changes from gitlab-org/gitlab@master

8 files changed, 224 insertions, 180 deletions

diff --git a/spec/serializers/activity_pub/activity_serializer_spec.rb b/spec/serializers/activity_pub/activity_serializer_spec.rb
new file mode 100644
index 00000000000..93b52614490
--- /dev/null
+++ b/spec/serializers/activity_pub/activity_serializer_spec.rb
@@ -0,0 +1,138 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::ActivitySerializer, feature_category: :integrations do
+  let(:implementer_class) do
+    Class.new(described_class)
+  end
+
+  let(:serializer) { implementer_class.new.represent(resource) }
+
+  let(:resource) { build_stubbed(:release) }
+
+  let(:transitive_entity_class) do
+    Class.new(Grape::Entity) do
+      expose :id do |*|
+        'https://example.com/unique/url'
+      end
+
+      expose :type do |*|
+        'Follow'
+      end
+
+      expose :actor do |*|
+        'https://example.com/actor/alice'
+      end
+
+      expose :object do |*|
+        'https://example.com/actor/bob'
+      end
+    end
+  end
+
+  let(:intransitive_entity_class) do
+    Class.new(Grape::Entity) do
+      expose :id do |*|
+        'https://example.com/unique/url'
+      end
+
+      expose :type do |*|
+        'Question'
+      end
+
+      expose :actor do |*|
+        'https://example.com/actor/alice'
+      end
+
+      expose :content do |*|
+        "What's up?"
+      end
+    end
+  end
+
+  let(:entity_class) { transitive_entity_class }
+
+  shared_examples_for 'activity document' do
+    it 'belongs to the ActivityStreams namespace' do
+      expect(serializer['@context']).to eq 'https://www.w3.org/ns/activitystreams'
+    end
+
+    it 'has a unique identifier' do
+      expect(serializer).to have_key 'id'
+    end
+
+    it 'has a type' do
+      expect(serializer).to have_key 'type'
+    end
+
+    it 'has an actor' do
+      expect(serializer['actor']).to eq 'https://example.com/actor/alice'
+    end
+  end
+
+  before do
+    implementer_class.entity entity_class
+  end
+
+  context 'with a valid represented entity' do
+    it_behaves_like 'activity document'
+  end
+
+  context 'when the represented entity provides no identifier' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({ type: 'Person', actor: 'http://something/' })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingIdentifierError)
+    end
+  end
+
+  context 'when the represented entity provides no type' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({
+        id: 'http://something/',
+        actor: 'http://something-else/'
+      })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingTypeError)
+    end
+  end
+
+  context 'when the represented entity provides no actor' do
+    before do
+      allow(entity_class).to receive(:represent).and_return({ id: 'http://something/', type: 'Person' })
+    end
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingActorError)
+    end
+  end
+
+  context 'when the represented entity provides no object' do
+    let(:entity_class) { intransitive_entity_class }
+
+    context 'when the caller provides the :intransitive option' do
+      let(:serializer) { implementer_class.new.represent(resource, intransitive: true) }
+
+      it_behaves_like 'activity document'
+    end
+
+    context 'when the caller does not provide the :intransitive option' do
+      it 'raises an exception' do
+        expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::MissingObjectError)
+      end
+    end
+  end
+
+  context 'when the caller does provide the :intransitive option and an object' do
+    let(:serializer) { implementer_class.new.represent(resource, intransitive: true) }
+
+    it 'raises an exception' do
+      expect { serializer }.to raise_error(ActivityPub::ActivitySerializer::IntransitiveWithObjectError)
+    end
+  end
+end
diff --git a/spec/serializers/activity_pub/activity_streams_serializer_spec.rb b/spec/serializers/activity_pub/activity_streams_serializer_spec.rb
deleted file mode 100644
index c74beba7a81..00000000000
--- a/spec/serializers/activity_pub/activity_streams_serializer_spec.rb
+++ /dev/null
@@ -1,157 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe ActivityPub::ActivityStreamsSerializer, feature_category: :integrations do
-  let(:implementer_class) do
-    Class.new(described_class) do
-      include WithPagination
-    end
-  end
-
-  let(:entity_class) do
-    Class.new(Grape::Entity) do
-      expose :id do |*|
-        'https://example.com/unique/url'
-      end
-
-      expose :type do |*|
-        'Person'
-      end
-
-      expose :name do |*|
-        'Alice'
-      end
-    end
-  end
-
-  shared_examples_for 'ActivityStreams document' do
-    it 'belongs to the ActivityStreams namespace' do
-      expect(subject['@context']).to eq 'https://www.w3.org/ns/activitystreams'
-    end
-
-    it 'has a unique identifier' do
-      expect(subject).to have_key 'id'
-    end
-
-    it 'has a type' do
-      expect(subject).to have_key 'type'
-    end
-  end
-
-  before do
-    implementer_class.entity entity_class
-  end
-
-  context 'when the serializer is not paginated' do
-    let(:resource) { build_stubbed(:release) }
-    let(:outbox_url) { 'https://example.com/unique/url/outbox' }
-
-    context 'with a valid represented entity' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'exposes an outbox' do
-        expect(subject['outbox']).to eq 'https://example.com/unique/url/outbox'
-      end
-
-      it 'includes serialized data' do
-        expect(subject['name']).to eq 'Alice'
-      end
-    end
-
-    context 'when the represented entity provides no identifier' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      before do
-        allow(entity_class).to receive(:represent).and_return({ type: 'Person' })
-      end
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingIdentifierError)
-      end
-    end
-
-    context 'when the represented entity provides no type' do
-      subject { implementer_class.new.represent(resource, outbox: outbox_url) }
-
-      before do
-        allow(entity_class).to receive(:represent).and_return({ id: 'https://example.com/unique/url' })
-      end
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingTypeError)
-      end
-    end
-
-    context 'when the caller provides no outbox parameter' do
-      subject { implementer_class.new.represent(resource) }
-
-      it 'raises an exception' do
-        expect { subject }.to raise_error(ActivityPub::ActivityStreamsSerializer::MissingOutboxError)
-      end
-    end
-  end
-
-  context 'when the serializer is paginated' do
-    let(:resources) { build_stubbed_list(:release, 3) }
-    let(:request) { ActionDispatch::Request.new(request_data) }
-    let(:response) { ActionDispatch::Response.new }
-    let(:url) { 'https://example.com/resource/url' }
-    let(:decorated) { implementer_class.new.with_pagination(request, response) }
-
-    before do
-      allow(resources).to receive(:page).and_return(resources)
-      allow(resources).to receive(:per).and_return(resources)
-      allow(resources).to receive(:current_page).and_return(2)
-      allow(resources).to receive(:total_pages).and_return(3)
-      allow(resources).to receive(:total_count).and_return(10)
-      allow(decorated.paginator).to receive(:paginate).and_return(resources)
-    end
-
-    context 'when no page parameter is provided' do
-      subject { decorated.represent(resources) }
-
-      let(:request_data) do
-        { "rack.url_scheme" => "https", "HTTP_HOST" => "example.com", "PATH_INFO" => '/resource/url' }
-      end
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'is an index document for the pagination' do
-        expect(subject['type']).to eq 'OrderedCollection'
-      end
-
-      it 'contains the total amount of items' do
-        expect(subject['totalItems']).to eq 10
-      end
-
-      it 'contains links to first and last page' do
-        expect(subject['first']).to eq "#{url}?page=1"
-        expect(subject['last']).to eq "#{url}?page=3"
-      end
-    end
-
-    context 'when a page parameter is provided' do
-      subject { decorated.represent(resources) }
-
-      let(:request_data) do
-        { 'rack.url_scheme' => 'https', 'HTTP_HOST' => 'example.com', 'PATH_INFO' => '/resource/url',
-          'QUERY_STRING' => 'page=2&per_page=1' }
-      end
-
-      it_behaves_like 'ActivityStreams document'
-
-      it 'is a page document' do
-        expect(subject['type']).to eq 'OrderedCollectionPage'
-      end
-
-      it 'contains navigation links' do
-        expect(subject['prev']).to be_present
-        expect(subject['next']).to be_present
-        expect(subject['partOf']).to be_present
-      end
-    end
-  end
-end
diff --git a/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb b/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb
new file mode 100644
index 00000000000..287b806bb35
--- /dev/null
+++ b/spec/serializers/activity_pub/publish_release_activity_serializer_spec.rb
@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe ActivityPub::PublishReleaseActivitySerializer, feature_category: :release_orchestration do
+  let(:release) { build_stubbed(:release) }
+
+  let(:serializer) { described_class.new.represent(release) }
+
+  it 'serializes the activity attributes' do
+    expect(serializer).to include(:id, :type, :actor, :object)
+  end
+end
diff --git a/spec/serializers/activity_pub/releases_actor_serializer_spec.rb b/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
index bc754eabe5c..47a170a04f5 100644
--- a/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
+++ b/spec/serializers/activity_pub/releases_actor_serializer_spec.rb
@@ -7,7 +7,7 @@ RSpec.describe ActivityPub::ReleasesActorSerializer, feature_category: :groups_a
   let(:releases) { build_stubbed_list(:release, 3, project: project) }
 
   context 'when there is a single object provided' do
-    subject { described_class.new.represent(project, outbox: '/outbox') }
+    subject { described_class.new.represent(project, outbox: '/outbox', inbox: '/inbox') }
 
     it 'serializes the actor attributes' do
       expect(subject).to include(:id, :type, :preferredUsername, :name, :content, :context)
diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb
index ef96bf11e0b..a5f0d5b5c5a 100644
--- a/spec/services/merge_requests/request_review_service_spec.rb
+++ b/spec/services/merge_requests/request_review_service_spec.rb
@@ -71,6 +71,14 @@ RSpec.describe MergeRequests::RequestReviewService, feature_category: :code_revi
         service.execute(merge_request, user)
       end
 
+      it 'creates a sytem note' do
+        expect(SystemNoteService)
+          .to receive(:request_review)
+          .with(merge_request, project, current_user, user)
+
+        service.execute(merge_request, user)
+      end
+
       it_behaves_like 'triggers GraphQL subscription mergeRequestReviewersUpdated' do
         let(:action) { result }
       end
diff --git a/spec/services/packages/npm/create_package_service_spec.rb b/spec/services/packages/npm/create_package_service_spec.rb
index 0d46d897b5b..7a91fdfc5b9 100644
--- a/spec/services/packages/npm/create_package_service_spec.rb
+++ b/spec/services/packages/npm/create_package_service_spec.rb
@@ -25,7 +25,13 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
     let(:version_data) { params.dig('versions', version) }
     let(:lease_key) { "packages:npm:create_package_service:packages:#{project.id}_#{package_name}_#{version}" }
 
+    shared_examples 'valid service response' do
+      it { is_expected.to be_success }
+    end
+
     shared_examples 'valid package' do
+      let(:package) { subject[:package] }
+
       it 'creates a package' do
         expect { subject }
           .to change { Packages::Package.count }.by(1)
@@ -34,21 +40,27 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
           .and change { Packages::Npm::Metadatum.count }.by(1)
       end
 
-      it_behaves_like 'assigns the package creator' do
-        let(:package) { subject }
-      end
+      it_behaves_like 'assigns the package creator'
 
-      it { is_expected.to be_valid }
-      it { is_expected.to have_attributes name: package_name, version: version }
+      it 'returns a valid package' do
+        subject
 
-      it { expect(subject.npm_metadatum.package_json).to eq(version_data) }
+        expect(package).to be_valid
+          .and have_attributes name: package_name, version: version
+        expect(package.npm_metadatum.package_json).to eq(version_data)
+      end
 
       context 'with build info' do
         let_it_be(:job) { create(:ci_build, user: user) }
         let(:params) { super().merge(build: job) }
 
-        it_behaves_like 'assigns build to package'
-        it_behaves_like 'assigns status to package'
+        it_behaves_like 'assigns build to package' do
+          subject { super().payload.fetch(:package) }
+        end
+
+        it_behaves_like 'assigns status to package' do
+          subject { super().payload.fetch(:package) }
+        end
 
         it 'creates a package file build info' do
           expect { subject }.to change { Packages::PackageFileBuildInfo.count }.by(1)
@@ -154,31 +166,35 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
               .and change { Packages::Package.npm.count }.by(1)
               .and change { Packages::Tag.count }.by(1)
               .and change { Packages::Npm::Metadatum.count }.by(1)
-            expect(subject.npm_metadatum.package_json[field]).to be_blank
+            expect(package.npm_metadatum.package_json[field]).to be_blank
           end
         end
       end
     end
 
     context 'scoped package' do
+      it_behaves_like 'valid service response'
       it_behaves_like 'valid package'
     end
 
     context 'when user is no project member' do
       let_it_be(:user) { create(:user) }
 
+      it_behaves_like 'valid service response'
       it_behaves_like 'valid package'
     end
 
     context 'scoped package not following the naming convention' do
       let(:package_name) { '@any-scope/package' }
 
+      it_behaves_like 'valid service response'
       it_behaves_like 'valid package'
     end
 
     context 'unscoped package' do
       let(:package_name) { 'unscoped-package' }
 
+      it_behaves_like 'valid service response'
       it_behaves_like 'valid package'
     end
 
@@ -186,8 +202,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
       let(:package_name) { "@#{namespace.path}/my_package" }
       let!(:existing_package) { create(:npm_package, project: project, name: package_name, version: '1.0.1') }
 
-      it { expect(subject[:http_status]).to eq 403 }
-      it { expect(subject[:message]).to be 'Package already exists.' }
+      it { is_expected.to be_error }
+      it { is_expected.to have_attributes message: 'Package already exists.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_PACKAGE_EXISTS }
 
       context 'marked as pending_destruction' do
         before do
@@ -208,10 +224,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
       let(:max_file_size) { 5.bytes }
 
       shared_examples_for 'max file size validation failure' do
-        it 'returns a 400 error', :aggregate_failures do
-          expect(subject[:http_status]).to eq 400
-          expect(subject[:message]).to be 'File is too large.'
-        end
+        it { is_expected.to be_error }
+        it { is_expected.to have_attributes message: 'File is too large.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_INVALID_PARAMETER }
       end
 
       before do
@@ -271,8 +285,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
     context 'with empty versions' do
       let(:params) { super().merge!({ versions: {} }) }
 
-      it { expect(subject[:http_status]).to eq 400 }
-      it { expect(subject[:message]).to eq 'Version is empty.' }
+      it { is_expected.to be_error }
+      it { is_expected.to have_attributes message: 'Version is empty.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_INVALID_PARAMETER }
     end
 
     context 'with invalid versions' do
@@ -294,8 +308,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
     context 'with empty attachment data' do
       let(:params) { super().merge({ _attachments: { "#{package_name}-#{version}.tgz" => { data: '' } } }) }
 
-      it { expect(subject[:http_status]).to eq 400 }
-      it { expect(subject[:message]).to eq 'Attachment data is empty.' }
+      it { is_expected.to be_error }
+      it { is_expected.to have_attributes message: 'Attachment data is empty.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_INVALID_PARAMETER }
     end
 
     it 'obtains a lease to create a new package' do
@@ -309,8 +323,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
         stub_exclusive_lease_taken(lease_key, timeout: described_class::DEFAULT_LEASE_TIMEOUT)
       end
 
-      it { expect(subject[:http_status]).to eq 400 }
-      it { expect(subject[:message]).to eq 'Could not obtain package lease. Please try again.' }
+      it { is_expected.to be_error }
+      it { is_expected.to have_attributes message: 'Could not obtain package lease. Please try again.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_PACKAGE_LEASE_TAKEN }
     end
 
     context 'when feature flag :packages_protected_packages disabled' do
@@ -355,7 +369,8 @@ RSpec.describe Packages::Npm::CreatePackageService, feature_category: :package_r
       let(:service) { described_class.new(project, current_user, params) }
 
       shared_examples 'protected package' do
-        it { is_expected.to include http_status: 403, message: 'Package protected.' }
+        it { is_expected.to be_error }
+        it { is_expected.to have_attributes message: 'Package protected.', reason: ::Packages::Npm::CreatePackageService::ERROR_REASON_PACKAGE_PROTECTED }
 
         it 'does not create any npm-related package records' do
           expect { subject }
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 8c11270d6fd..2e5545b610a 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -77,6 +77,18 @@ RSpec.describe SystemNoteService, feature_category: :shared do
     end
   end
 
+  describe '.request_review' do
+    let(:reviewer) { double }
+
+    it 'calls IssuableService' do
+      expect_next_instance_of(::SystemNotes::IssuablesService) do |service|
+        expect(service).to receive(:request_review).with(reviewer)
+      end
+
+      described_class.request_review(noteable, project, author, reviewer)
+    end
+  end
+
   describe '.change_issuable_contacts' do
     let(:added_count) { 5 }
     let(:removed_count) { 3 }
diff --git a/spec/services/system_notes/issuables_service_spec.rb b/spec/services/system_notes/issuables_service_spec.rb
index 2d7cbd30db8..2b48b24b2b4 100644
--- a/spec/services/system_notes/issuables_service_spec.rb
+++ b/spec/services/system_notes/issuables_service_spec.rb
@@ -213,6 +213,21 @@ RSpec.describe ::SystemNotes::IssuablesService, feature_category: :team_planning
     end
   end
 
+  describe '#request_review' do
+    subject(:request_review) { service.request_review(reviewer) }
+
+    let_it_be(:reviewer) { create(:user) }
+    let_it_be(:noteable) { create(:merge_request, :simple, source_project: project, reviewers: [reviewer]) }
+
+    it_behaves_like 'a system note' do
+      let(:action) { 'reviewer' }
+    end
+
+    it 'builds a correct phrase when a reviewer has been requested from a reviewer' do
+      expect(request_review.note).to eq "requested review from #{reviewer.to_reference}"
+    end
+  end
+
   describe '#change_issuable_contacts' do
     subject { service.change_issuable_contacts(1, 1) }