Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq

4 files changed, 225 insertions, 37 deletions

diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
index ab06c1a1209..51fb43907a6 100644
--- a/spec/services/application_settings/update_service_spec.rb
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -110,6 +110,39 @@ describe ApplicationSettings::UpdateService do
     end
   end
 
+  describe 'markdown cache invalidators' do
+    shared_examples 'invalidates markdown cache' do |attribute|
+      let(:params) { attribute }
+
+      it 'increments cache' do
+        expect { subject.execute }.to change(application_settings, :local_markdown_version).by(1)
+      end
+    end
+
+    it_behaves_like 'invalidates markdown cache', { asset_proxy_enabled: true }
+    it_behaves_like 'invalidates markdown cache', { asset_proxy_url: 'http://test.com' }
+    it_behaves_like 'invalidates markdown cache', { asset_proxy_secret_key: 'another secret' }
+    it_behaves_like 'invalidates markdown cache', { asset_proxy_whitelist: ['domain.com'] }
+
+    context 'when also setting the local_markdown_version' do
+      let(:params) { { asset_proxy_enabled: true, local_markdown_version: 12 } }
+
+      it 'does not increment' do
+        expect { subject.execute }.to change(application_settings, :local_markdown_version).to(12)
+      end
+    end
+
+    context 'do not invalidate if value does not change' do
+      let(:params) { { asset_proxy_enabled: true, asset_proxy_secret_key: 'secret', asset_proxy_url: 'http://test.com' } }
+
+      it 'does not increment' do
+        described_class.new(application_settings, admin, params).execute
+
+        expect { described_class.new(application_settings, admin, params).execute }.not_to change(application_settings, :local_markdown_version)
+      end
+    end
+  end
+
   describe 'performance bar settings' do
     using RSpec::Parameterized::TableSyntax
 
diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb
index 6874a8a0929..642a49d57d5 100644
--- a/spec/services/issues/close_service_spec.rb
+++ b/spec/services/issues/close_service_spec.rb
@@ -60,35 +60,63 @@ describe Issues::CloseService do
 
   describe '#close_issue' do
     context "closed by a merge request" do
-      before do
+      it 'mentions closure via a merge request' do
         perform_enqueued_jobs do
           described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request)
         end
-      end
 
-      it 'mentions closure via a merge request' do
         email = ActionMailer::Base.deliveries.last
 
         expect(email.to.first).to eq(user2.email)
         expect(email.subject).to include(issue.title)
         expect(email.body.parts.map(&:body)).to all(include(closing_merge_request.to_reference))
       end
+
+      context 'when user cannot read merge request' do
+        it 'does not mention merge request' do
+          project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
+          perform_enqueued_jobs do
+            described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request)
+          end
+
+          email = ActionMailer::Base.deliveries.last
+          body_text = email.body.parts.map(&:body).join(" ")
+
+          expect(email.to.first).to eq(user2.email)
+          expect(email.subject).to include(issue.title)
+          expect(body_text).not_to include(closing_merge_request.to_reference)
+        end
+      end
     end
 
     context "closed by a commit" do
-      before do
+      it 'mentions closure via a commit' do
         perform_enqueued_jobs do
           described_class.new(project, user).close_issue(issue, closed_via: closing_commit)
         end
-      end
 
-      it 'mentions closure via a commit' do
         email = ActionMailer::Base.deliveries.last
 
         expect(email.to.first).to eq(user2.email)
         expect(email.subject).to include(issue.title)
         expect(email.body.parts.map(&:body)).to all(include(closing_commit.id))
       end
+
+      context 'when user cannot read the commit' do
+        it 'does not mention the commit id' do
+          project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED)
+          perform_enqueued_jobs do
+            described_class.new(project, user).close_issue(issue, closed_via: closing_commit)
+          end
+
+          email = ActionMailer::Base.deliveries.last
+          body_text = email.body.parts.map(&:body).join(" ")
+
+          expect(email.to.first).to eq(user2.email)
+          expect(email.subject).to include(issue.title)
+          expect(body_text).not_to include(closing_commit.id)
+        end
+      end
     end
 
     context "valid params" do
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index d4fa62fa85d..8178b7d2ba2 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -183,27 +183,65 @@ describe Projects::CreateService, '#execute' do
   context 'restricted visibility level' do
     before do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])
+    end
 
-      opts.merge!(
-        visibility_level: Gitlab::VisibilityLevel::PUBLIC
-      )
+    shared_examples 'restricted visibility' do
+      it 'does not allow a restricted visibility level for non-admins' do
+        project = create_project(user, opts)
+
+        expect(project).to respond_to(:errors)
+        expect(project.errors.messages).to have_key(:visibility_level)
+        expect(project.errors.messages[:visibility_level].first).to(
+          match('restricted by your GitLab administrator')
+        )
+      end
+
+      it 'allows a restricted visibility level for admins' do
+        admin = create(:admin)
+        project = create_project(admin, opts)
+
+        expect(project.errors.any?).to be(false)
+        expect(project.saved?).to be(true)
+      end
     end
 
-    it 'does not allow a restricted visibility level for non-admins' do
-      project = create_project(user, opts)
-      expect(project).to respond_to(:errors)
-      expect(project.errors.messages).to have_key(:visibility_level)
-      expect(project.errors.messages[:visibility_level].first).to(
-        match('restricted by your GitLab administrator')
-      )
+    context 'when visibility is project based' do
+      before do
+        opts.merge!(
+          visibility_level: Gitlab::VisibilityLevel::PUBLIC
+        )
+      end
+
+      include_examples 'restricted visibility'
     end
 
-    it 'allows a restricted visibility level for admins' do
-      admin = create(:admin)
-      project = create_project(admin, opts)
+    context 'when visibility is overridden' do
+      let(:visibility) { 'public' }
 
-      expect(project.errors.any?).to be(false)
-      expect(project.saved?).to be(true)
+      before do
+        opts.merge!(
+          import_data: {
+            data: {
+              override_params: {
+                visibility: visibility
+              }
+            }
+          }
+        )
+      end
+
+      include_examples 'restricted visibility'
+
+      context 'when visibility is misspelled' do
+        let(:visibility) { 'publik' }
+
+        it 'does not restrict project creation' do
+          project = create_project(user, opts)
+
+          expect(project.errors.any?).to be(false)
+          expect(project.saved?).to be(true)
+        end
+      end
     end
   end
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index 9ee23f3eb48..bdf2f59704c 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -436,25 +436,114 @@ describe TodoService do
         should_create_todo(user: john_doe, target: confidential_issue, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_confidential_issue)
       end
 
-      context 'on commit' do
-        let(:project) { create(:project, :repository) }
-
-        it 'creates a todo for each valid mentioned user when leaving a note on commit' do
-          service.new_note(note_on_commit, john_doe)
-
-          should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
-          should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
-          should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
-          should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: note_on_commit.commit_id, author: john_doe, action: Todo::MENTIONED, note: note_on_commit)
+      context 'commits' do
+        let(:base_commit_todo_attrs) { { target_id: nil, target_type: 'Commit', author: john_doe } }
+
+        context 'leaving a note on a commit in a public project' do
+          let(:project) { create(:project, :repository, :public) }
+          it 'creates a todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::MENTIONED,
+              note: note_on_commit,
+              commit_id: note_on_commit.commit_id
+            )
+
+            service.new_note(note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_create_todo(expected_todo.merge(user: guest))
+            should_create_todo(expected_todo.merge(user: non_member))
+          end
+
+          it 'creates a directly addressed todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::DIRECTLY_ADDRESSED,
+              note: addressed_note_on_commit,
+              commit_id: addressed_note_on_commit.commit_id
+            )
+
+            service.new_note(addressed_note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_create_todo(expected_todo.merge(user: guest))
+            should_create_todo(expected_todo.merge(user: non_member))
+          end
         end
 
-        it 'creates a directly addressed todo for each valid mentioned user when leaving a note on commit' do
-          service.new_note(addressed_note_on_commit, john_doe)
+        context 'leaving a note on a commit in a public project with private code' do
+          let(:project) { create(:project, :repository, :public, :repository_private) }
+
+          it 'creates a todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::MENTIONED,
+              note: note_on_commit,
+              commit_id: note_on_commit.commit_id
+            )
+
+            service.new_note(note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_create_todo(expected_todo.merge(user: guest))
+            should_not_create_todo(expected_todo.merge(user: non_member))
+          end
+
+          it 'creates a directly addressed todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::DIRECTLY_ADDRESSED,
+              note: addressed_note_on_commit,
+              commit_id: addressed_note_on_commit.commit_id
+            )
+
+            service.new_note(addressed_note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_create_todo(expected_todo.merge(user: guest))
+            should_not_create_todo(expected_todo.merge(user: non_member))
+          end
+        end
 
-          should_create_todo(user: member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
-          should_create_todo(user: author, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
-          should_create_todo(user: john_doe, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
-          should_not_create_todo(user: non_member, target_id: nil, target_type: 'Commit', commit_id: addressed_note_on_commit.commit_id, author: john_doe, action: Todo::DIRECTLY_ADDRESSED, note: addressed_note_on_commit)
+        context 'leaving a note on a commit in a private project' do
+          let(:project) { create(:project, :repository, :private) }
+
+          it 'creates a todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::MENTIONED,
+              note: note_on_commit,
+              commit_id: note_on_commit.commit_id
+            )
+
+            service.new_note(note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_not_create_todo(expected_todo.merge(user: guest))
+            should_not_create_todo(expected_todo.merge(user: non_member))
+          end
+
+          it 'creates a directly addressed todo for each valid mentioned user' do
+            expected_todo = base_commit_todo_attrs.merge(
+              action: Todo::DIRECTLY_ADDRESSED,
+              note: addressed_note_on_commit,
+              commit_id: addressed_note_on_commit.commit_id
+            )
+
+            service.new_note(addressed_note_on_commit, john_doe)
+
+            should_create_todo(expected_todo.merge(user: member))
+            should_create_todo(expected_todo.merge(user: author))
+            should_create_todo(expected_todo.merge(user: john_doe))
+            should_not_create_todo(expected_todo.merge(user: guest))
+            should_not_create_todo(expected_todo.merge(user: non_member))
+          end
         end
       end