Merge branch 'ce-to-ee-2018-03-05'

.gitlab-ci.yml

@@ -686,9 +686,10 @@ codequality:
   cache: {}
   dependencies: []
   script:
+    - apk update && apk add jq
     - ./scripts/codequality analyze -f json > raw_codeclimate.json || true
     # The following line keeps only the fields used in the MR widget, reducing the JSON artifact size
-    - cat raw_codeclimate.json | docker run -i stedolan/jq -c 'map({check_name,description,fingerprint,location})' > codeclimate.json
+    - jq -c 'map({check_name,description,fingerprint,location})' raw_codeclimate.json > codeclimate.json
   artifacts:
     paths: [codeclimate.json]
     expire_in: 1 week

app/finders/todos_finder.rb

@@ -110,10 +110,6 @@ class TodosFinder
     ids
   end
 
-  def projects(items)
-    ProjectsFinder.new(current_user: current_user, project_ids_relation: project_ids(items)).execute
-  end
-
   def type?
     type.present? && %w(Issue MergeRequest).include?(type)
   end
@@ -152,13 +148,14 @@ class TodosFinder
 
   def by_project(items)
     if project?
-      items = items.where(project: project)
+      items.where(project: project)
     else
-      item_projects = projects(items)
-      items = items.merge(item_projects).joins(:project)
-    end
+      projects = Project
+        .public_or_visible_to_user(current_user)
+        .order_id_desc
 
-    items
+      items.joins(:project).merge(projects)
+    end
   end
 
   def by_state(items)

app/helpers/notes_helper.rb

@@ -11,7 +11,7 @@ module NotesHelper
   end
 
   def note_supports_quick_actions?(note)
-    Notes::QuickActionsService.supported?(note, current_user)
+    Notes::QuickActionsService.supported?(note)
   end
 
   def noteable_json(noteable)

app/services/notes/quick_actions_service.rb

@@ -9,14 +9,12 @@ module Notes
       UPDATE_SERVICES[note.noteable_type]
     end
 
-    def self.supported?(note, current_user)
-      noteable_update_service(note) &&
-        current_user &&
-        current_user.can?(:"update_#{note.to_ability_name}", note.noteable)
+    def self.supported?(note)
+      !!noteable_update_service(note)
     end
 
     def supported?(note)
-      self.class.supported?(note, current_user)
+      self.class.supported?(note)
     end
 
     def extract_commands(note, options = {})

app/views/projects/blob/_header.html.haml

@@ -14,9 +14,11 @@
       = lock_file_link(html_options: { class: 'btn btn-sm path-lock' })
 
       = edit_blob_button
+
       -# EE-specific
       = ide_edit_button
       -# EE-specific
+
       - if current_user
         = replace_blob_link
         = delete_blob_link

changelogs/unreleased/43334-reply-by-email-did-not-pick-up-unsubscribe-quick-action.yml

+---
+title: Fix quick actions for users who cannot update issues and merge requests
+merge_request: 17482
+author:
+type: fixed

changelogs/unreleased/remove-projects-finder-from-todos-finder.yml

+---
+title: Don't use ProjectsFinder in TodosFinder
+merge_request:
+author:
+type: performance

doc/api/merge_requests.md

@@ -500,6 +500,8 @@ Parameters:
 
 ## List MR pipelines
 
+> [Introduced][ce-15454] in GitLab 10.5.0.
+
 Get a list of merge request pipelines.
 
 ```
@@ -1603,3 +1605,4 @@ Example response:
 
 [ce-13060]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/13060
 [ce-14016]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14016
+[ce-15454]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15454

doc/ssh/README.md

@@ -35,8 +35,8 @@ to clipboard step.
 If you don't see the string or would like to generate a SSH key pair with a
 custom name continue onto the next step.
 
->
-**Note:** Public SSH key may also be named as follows:
+Note that Public SSH key may also be named as follows:
+
 - `id_dsa.pub`
 - `id_ecdsa.pub`
 - `id_ed25519.pub`
@@ -73,7 +73,7 @@ custom name continue onto the next step.
    key pair, but it is not required and you can skip creating a password by
    pressing enter.
 
-     >**Note:**
+     NOTE: **Note:**
      If you want to change the password of your SSH key pair, you can use
      `ssh-keygen -p <keyname>`.
 
@@ -162,11 +162,13 @@ That's why it needs to uniquely map to a single user.
 
 ## Deploy keys
 
+### Per-repository deploy keys
+
 Deploy keys allow read-only or read-write (if enabled) access to one or
 multiple projects with a single SSH key pair.
 
 This is really useful for cloning repositories to your Continuous
-Integration (CI) server. By using deploy keys, you don't have to setup a
+Integration (CI) server. By using deploy keys, you don't have to set up a
 dummy user account.
 
 If you are a project master or owner, you can add a deploy key in the
@@ -185,6 +187,47 @@ a group.
 Deploy keys can be shared between projects, you just need to add them to each
 project.
 
+### Global shared deploy keys
+
+Global Shared Deploy keys allow read-only or read-write (if enabled) access to 
+be configured on any repository in the entire GitLab installation.
+
+This is really useful for integrating repositories to secured, shared Continuous
+Integration (CI) services or other shared services. 
+GitLab administrators can set up the Global Shared Deploy key in GitLab and 
+add the private key to any shared systems.  Individual repositories opt into
+exposing their repsitory using these keys when a project masters (or higher)
+authorizes a Global Shared Deploy key to be used with their project. 
+
+Global Shared Keys can provide greater security compared to Per-Project Deploy
+Keys since an administrator of the target integrated system is the only one
+who needs to know and configure the private key.
+
+GitLab administrators set up Global Deploy keys in the Admin area under the
+section **Deploy Keys**. Ensure keys have a meaningful title as that will be
+the primary way for project masters and owners to identify the correct Global
+Deploy key to add.  For instance, if the key gives access to a SaaS CI instance,
+use the name of that service in the key name if that is all it is used for.
+When creating Global Shared Deploy keys, give some thought to the granularity
+of keys - they could be of very narrow usage such as just a specific service or 
+of broader usage for something like "Anywhere you need to give read access to 
+your repository".
+
+Once a GitLab administrator adds the Global Deployment key, project masters 
+and owners can add it in project's **Settings > Repository** section by expanding the 
+**Deploy Key** section and clicking **Enable** next to the appropriate key listed 
+under **Public deploy keys available to any project**.
+
+NOTE: **Note:**
+The heading **Public deploy keys available to any project** only appears
+if there is at least one Global Deploy Key configured.
+
+CAUTION: **Warning:**
+Defining Global Deploy Keys does not expose any given repository via
+the key until that respository adds the Global Deploy Key to their project.
+In this way the Global Deploy Keys enable access by other systems, but do
+not implicitly give any access just by setting them up.
+
 ## Applications
 
 ### Eclipse

lib/gitlab/ci/pipeline/chain/create.rb

@@ -17,27 +17,11 @@ module Gitlab
             end
           rescue ActiveRecord::RecordInvalid => e
             error("Failed to persist the pipeline: #{e}")
-          ensure
-            if pipeline.builds.where(stage_id: nil).any?
-              invalid_builds_counter.increment(node: hostname)
-            end
           end
 
           def break?
             !pipeline.persisted?
           end
-
-          private
-
-          def invalid_builds_counter
-            @counter ||= Gitlab::Metrics
-              .counter(:gitlab_ci_invalid_builds_total,
-                       'Invalid builds without stage assigned counter')
-          end
-
-          def hostname
-            @hostname ||= Socket.gethostname
-          end
         end
       end
     end

spec/features/issues/user_uses_slash_commands_spec.rb

@@ -59,7 +59,6 @@ feature 'Issues > User uses quick actions', :js do
         it 'does not create a note, and sets the due date accordingly' do
           write_note("/due 2016-08-28")
 
-          expect(page).to have_content '/due 2016-08-28'
           expect(page).not_to have_content 'Commands applied'
 
           issue.reload
@@ -99,7 +98,6 @@ feature 'Issues > User uses quick actions', :js do
         it 'does not create a note, and sets the due date accordingly' do
           write_note("/remove_due_date")
 
-          expect(page).to have_content '/remove_due_date'
           expect(page).not_to have_content 'Commands applied'
 
           issue.reload
@@ -144,10 +142,9 @@ feature 'Issues > User uses quick actions', :js do
           visit project_issue_path(project, issue)
         end
 
-        it 'creates a note, and does not set the weight' do
+        it 'does not create a note or set the weight' do
           write_note("/weight 5")
 
-          expect(page).to have_content '/weight 5'
           expect(page).not_to have_content 'Commands applied'
 
           issue.reload
@@ -182,10 +179,9 @@ feature 'Issues > User uses quick actions', :js do
           visit project_issue_path(project, issue)
         end
 
-        it 'creates a note, and does not set the weight' do
+        it 'does create a note or set the weight' do
           write_note("/clear_weight")
 
-          expect(page).to have_content '/clear_weight'
           expect(page).not_to have_content 'Commands applied'
 
           issue.reload
@@ -223,7 +219,6 @@ feature 'Issues > User uses quick actions', :js do
         it 'does not create a note, and does not mark the issue as a duplicate' do
           write_note("/duplicate ##{original_issue.to_reference}")
 
-          expect(page).to have_content "/duplicate ##{original_issue.to_reference}"
           expect(page).not_to have_content 'Commands applied'
           expect(page).not_to have_content "marked this issue as a duplicate of #{original_issue.to_reference}"
 

spec/fixtures/emails/update_commands_only_reply.eml

+Return-Path: <jake@adventuretime.ooo>
+Received: from iceking.adventuretime.ooo ([unix socket]) by iceking (Cyrus v2.2.13-Debian-2.2.13-19+squeeze3) with LMTPA; Thu, 13 Jun 2013 17:03:50 -0400
+Received: from mail-ie0-x234.google.com (mail-ie0-x234.google.com [IPv6:2607:f8b0:4001:c03::234]) by iceking.adventuretime.ooo (8.14.3/8.14.3/Debian-9.4) with ESMTP id r5DL3nFJ016967 (version=TLSv1/SSLv3 cipher=RC4-SHA bits=128 verify=NOT) for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 17:03:50 -0400
+Received: by mail-ie0-f180.google.com with SMTP id f4so21977375iea.25 for <reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo>; Thu, 13 Jun 2013 14:03:48 -0700
+Received: by 10.0.0.1 with HTTP; Thu, 13 Jun 2013 14:03:48 -0700
+Date: Thu, 13 Jun 2013 17:03:48 -0400
+From: Jake the Dog <jake@adventuretime.ooo>
+To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
+Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
+In-Reply-To: <issue_1@localhost>
+References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
+Mime-Version: 1.0
+Content-Type: text/plain;
+ charset=ISO-8859-1
+Content-Transfer-Encoding: 7bit
+X-Sieve: CMU Sieve 2.2
+X-Received: by 10.0.0.1 with SMTP id n7mr11234144ipb.85.1371157428600; Thu,
+ 13 Jun 2013 14:03:48 -0700 (PDT)
+X-Scanned-By: MIMEDefang 2.69 on IPv6:2001:470:1d:165::1
+
+/close
+
+On Sun, Jun 9, 2013 at 1:39 PM, eviltrout via Discourse Meta
+<reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo> wrote:
+>
+>
+>
+> eviltrout posted in 'Adventure Time Sux' on Discourse Meta:
+>
+> ---
+> hey guys everyone knows adventure time sucks!
+>
+> ---
+> Please visit this link to respond: http://localhost:3000/t/adventure-time-sux/1234/3
+>
+> To unsubscribe from these emails, visit your [user preferences](http://localhost:3000/user_preferences).
+>

spec/lib/backup/repository_spec.rb

@@ -28,6 +28,23 @@ describe Backup::Repository do
   end
 
   describe '#restore' do
+    subject { described_class.new }
+
+    let(:timestamp) { Time.utc(2017, 3, 22) }
+    let(:temp_dirs) do
+      Gitlab.config.repositories.storages.map do |name, storage|
+        File.join(storage['path'], '..', 'repositories.old.' + timestamp.to_i.to_s)
+      end
+    end
+
+    around do |example|
+      Timecop.freeze(timestamp) { example.run }
+    end
+
+    after do
+      temp_dirs.each { |path| FileUtils.rm_rf(path) }
+    end
+
     describe 'command failure' do
       before do
         allow(Gitlab::Popen).to receive(:popen).and_return(['error', 1])
@@ -35,7 +52,7 @@ describe Backup::Repository do
 
       context 'hashed storage' do
         it 'shows the appropriate error' do
-          described_class.new.restore
+          subject.restore
 
           expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} (#{project.disk_path}) - error")
         end
@@ -45,7 +62,7 @@ describe Backup::Repository do
         let!(:project) { create(:project, :legacy_storage) }
 
         it 'shows the appropriate error' do
-          described_class.new.restore
+          subject.restore
 
           expect(progress).to have_received(:puts).with("Ignoring error on #{project.full_path} - error")
         end

spec/lib/gitlab/email/handler/create_note_handler_spec.rb

@@ -55,8 +55,8 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
       expect { receiver.execute }.to raise_error(Gitlab::Email::InvalidNoteError)
     end
 
-    context 'because the note was commands only' do
-      let!(:email_raw) { fixture_file("emails/commands_only_reply.eml") }
+    context 'because the note was update commands only' do
+      let!(:email_raw) { fixture_file("emails/update_commands_only_reply.eml") }
 
       context 'and current user cannot update noteable' do
         it 'raises a CommandsOnlyNoteError' do
@@ -70,13 +70,10 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
         end
 
         it 'does not raise an error' do
-          expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
-
           # One system note is created for the 'close' event
           expect { receiver.execute }.to change { noteable.notes.count }.by(1)
 
           expect(noteable.reload).to be_closed
-          expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy
         end
       end
     end
@@ -85,15 +82,13 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
   context 'when the note contains quick actions' do
     let!(:email_raw) { fixture_file("emails/commands_in_reply.eml") }
 
-    context 'and current user cannot update noteable' do
-      it 'post a note and does not update the noteable' do
-        expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
-
-        # One system note is created for the new note
-        expect { receiver.execute }.to change { noteable.notes.count }.by(1)
+    context 'and current user cannot update the noteable' do
+      it 'only executes the commands that the user can perform' do
+        expect { receiver.execute }
+          .to change { noteable.notes.user.count }.by(1)
+          .and change { user.todos_pending_count }.from(0).to(1)
 
         expect(noteable.reload).to be_open
-        expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
       end
     end
 
@@ -102,14 +97,14 @@ describe Gitlab::Email::Handler::CreateNoteHandler do
         project.add_developer(user)
       end
 
-      it 'post a note and updates the noteable' do
+      it 'posts a note and updates the noteable' do
         expect(TodoService.new.todo_exist?(noteable, user)).to be_falsy
 
-        # One system note is created for the new note, one for the 'close' event
-        expect { receiver.execute }.to change { noteable.notes.count }.by(2)
+        expect { receiver.execute }
+          .to change { noteable.notes.user.count }.by(1)
+          .and change { user.todos_pending_count }.from(0).to(1)
 
         expect(noteable.reload).to be_closed
-        expect(TodoService.new.todo_exist?(noteable, user)).to be_truthy
       end
     end
   end

spec/services/notes/create_service_spec.rb

@@ -57,32 +57,55 @@ describe Notes::CreateService do
       end
     end
 
-    describe 'note with commands' do
-      describe '/close, /label, /assign & /milestone' do
-        let(:note_text) { %(HELLO\n/close\n/assign @#{user.username}\nWORLD) }
+    context 'note with commands' do
+      context 'as a user who can update the target' do
+        context '/close, /label, /assign & /milestone' do
+          let(:note_text) { %(HELLO\n/close\n/assign @#{user.username}\nWORLD) }
 
-        it 'saves the note and does not alter the note text' do
-          expect_any_instance_of(Issues::UpdateService).to receive(:execute).and_call_original
+          it 'saves the note and does not alter the note text' do
+            expect_any_instance_of(Issues::UpdateService).to receive(:execute).and_call_original
 
-          note = described_class.new(project, user, opts.merge(note: note_text)).execute
+            note = described_class.new(project, user, opts.merge(note: note_text)).execute
 
-          expect(note.note).to eq "HELLO\nWORLD"
+            expect(note.note).to eq "HELLO\nWORLD"
+          end
+        end
+
+        context '/merge with sha option' do
+          let(:note_text) { %(HELLO\n/merge\nWORLD) }
+          let(:params) { opts.merge(note: note_text, merge_request_diff_head_sha: 'sha') }
+
+          it 'saves the note and exectues merge command' do
+            note = described_class.new(project, user, params).execute
+
+            expect(note.note).to eq "HELLO\nWORLD"
+          end
         end
       end
 
-      describe '/merge with sha option' do
-        let(:note_text) { %(HELLO\n/merge\nWORLD) }
-        let(:params) { opts.merge(note: note_text, merge_request_diff_head_sha: 'sha') }
+      context 'as a user who cannot update the target' do
+        let(:note_text) { "HELLO\n/todo\n/assign #{user.to_reference}\nWORLD" }
+        let(:note) { described_class.new(project, user, opts.merge(note: note_text)).execute }
 
-        it 'saves the note and exectues merge command' do
-          note = described_class.new(project, user, params).execute
+        before do
+          project.team.find_member(user.id).update!(access_level: Gitlab::Access::GUEST)
+        end
+
+        it 'applies commands the user can execute' do
+          expect { note }.to change { user.todos_pending_count }.from(0).to(1)
+        end
+
+        it 'does not apply commands the user cannot execute' do
+          expect { note }.not_to change { issue.assignees }
+        end
 
+        it 'saves the note' do
           expect(note.note).to eq "HELLO\nWORLD"
         end
       end
     end
 
-    describe 'personal snippet note' do
+    context 'personal snippet note' do
       subject { described_class.new(nil, user, params).execute }
 
       let(:snippet) { create(:personal_snippet) }
@@ -103,7 +126,7 @@ describe Notes::CreateService do
       end
     end
 
-    describe 'note with emoji only' do
+    context 'note with emoji only' do
       it 'creates regular note' do
         opts = {
           note: ':smile: ',

spec/services/notes/quick_actions_service_spec.rb

@@ -165,31 +165,17 @@ describe Notes::QuickActionsService do
 
     let(:note) { create(:note_on_issue, project: project) }
 
-    context 'with no current_user' do
-      it 'returns false' do
-        expect(described_class.supported?(note, nil)).to be_falsy
-      end
-    end
-
-    context 'when current_user cannot update the noteable' do
-      it 'returns false' do
-        user = create(:user)
-
-        expect(described_class.supported?(note, user)).to be_falsy
-      end
-    end
-
-    context 'when current_user can update the noteable' do
+    context 'with a note on an issue' do
       it 'returns true' do
-        expect(described_class.supported?(note, master)).to be_truthy
+        expect(described_class.supported?(note)).to be_truthy
       end
+    end
 
-      context 'with a note on a commit' do
-        let(:note) { create(:note_on_commit, project: project) }
+    context 'with a note on a commit' do
+      let(:note) { create(:note_on_commit, project: project) }
 
-        it 'returns false' do
-          expect(described_class.supported?(note, nil)).to be_falsy
-        end
+      it 'returns false' do
+        expect(described_class.supported?(note)).to be_falsy
       end
     end
   end
@@ -201,7 +187,7 @@ describe Notes::QuickActionsService do
       service = described_class.new(project, master)
       note = create(:note_on_issue, project: project)
 
-      expect(described_class).to receive(:supported?).with(note, master)
+      expect(described_class).to receive(:supported?).with(note)
 
       service.supported?(note)
     end

spec/support/features/issuable_slash_commands_shared_examples.rb

@@ -127,7 +127,6 @@ shared_examples 'issuable record that supports quick actions in its description
         it "does not close the #{issuable_type}" do
           write_note("/close")
 
-          expect(page).to have_content '/close'
           expect(page).not_to have_content 'Commands applied'
 
           expect(issuable).to be_open
@@ -165,7 +164,6 @@ shared_examples 'issuable record that supports quick actions in its description
         it "does not reopen the #{issuable_type}" do
           write_note("/reopen")
 
-          expect(page).to have_content '/reopen'
           expect(page).not_to have_content 'Commands applied'
 
           expect(issuable).to be_closed
@@ -195,10 +193,9 @@ shared_examples 'issuable record that supports quick actions in its description
           visit public_send("namespace_project_#{issuable_type}_path", project.namespace, project, issuable)
         end
 
-        it "does not reopen the #{issuable_type}" do
+        it "does not change the #{issuable_type} title" do
           write_note("/title Awesome new title")
 
-          expect(page).to have_content '/title'
           expect(page).not_to have_content 'Commands applied'
 
           expect(issuable.reload.title).not_to eq 'Awesome new title'