Merge remote-tracking branch 'origin/master' into add-sentry-js-again-with-vue

.gitlab-ci.yml

@@ -68,6 +68,13 @@ stages:
     - //@gitlab-org/gitlab-ee
     - //@gitlab/gitlab-ee
 
+# Skip all jobs except the ones that begin with 'docs/'.
+# Used for commits including ONLY documentation changes.
+# https://docs.gitlab.com/ce/development/writing_documentation.html#testing
+.except-docs: &except-docs
+  except:
+    - /^docs\/.*/
+
 .rspec-knapsack: &rspec-knapsack
   stage: test
   <<: *dedicated-runner
@@ -91,11 +98,13 @@ stages:
 .rspec-knapsack-pg: &rspec-knapsack-pg
   <<: *rspec-knapsack
   <<: *use-pg
+  <<: *except-docs
 
 .rspec-knapsack-mysql: &rspec-knapsack-mysql
   <<: *rspec-knapsack
   <<: *use-mysql
   <<: *only-master-and-ee-or-mysql
+  <<: *except-docs
 
 .spinach-knapsack: &spinach-knapsack
   stage: test
@@ -120,16 +129,19 @@ stages:
 .spinach-knapsack-pg: &spinach-knapsack-pg
   <<: *spinach-knapsack
   <<: *use-pg
+  <<: *except-docs
 
 .spinach-knapsack-mysql: &spinach-knapsack-mysql
   <<: *spinach-knapsack
   <<: *use-mysql
   <<: *only-master-and-ee-or-mysql
+  <<: *except-docs
 
 # Prepare and merge knapsack tests
 knapsack:
   <<: *knapsack-state
   <<: *dedicated-runner
+  <<: *except-docs
   stage: prepare
   script:
     - mkdir -p knapsack/${CI_PROJECT_NAME}/
@@ -156,6 +168,7 @@ update-knapsack:
 setup-test-env:
   <<: *use-pg
   <<: *dedicated-runner
+  <<: *except-docs
   stage: prepare
   script:
     - node --version
@@ -243,6 +256,7 @@ spinach mysql 9 10: *spinach-knapsack-mysql
 .exec: &exec
   <<: *ruby-static-analysis
   <<: *dedicated-runner
+  <<: *except-docs
   stage: test
   script:
     - bundle exec $CI_JOB_NAME
@@ -250,6 +264,7 @@ spinach mysql 9 10: *spinach-knapsack-mysql
 rubocop:
   <<: *ruby-static-analysis
   <<: *dedicated-runner
+  <<: *except-docs
   stage: test
   script:
     - bundle exec "rubocop --require rubocop-rspec"
@@ -266,6 +281,7 @@ rake downtime_check:
     - master
     - tags
     - /^[\d-]+-stable(-ee)?$/
+    - /^docs\/*/
 
 rake ee_compat_check:
   <<: *exec
@@ -296,10 +312,12 @@ rake ee_compat_check:
 rake pg db:migrate:reset:
   <<: *db-migrate-reset
   <<: *use-pg
+  <<: *except-docs
 
 rake mysql db:migrate:reset:
   <<: *db-migrate-reset
   <<: *use-mysql
+  <<: *except-docs
 
 .db-rollback: &db-rollback
   stage: test
@@ -311,10 +329,12 @@ rake mysql db:migrate:reset:
 rake pg db:rollback:
   <<: *db-rollback
   <<: *use-pg
+  <<: *except-docs
 
 rake mysql db:rollback:
   <<: *db-rollback
   <<: *use-mysql
+  <<: *except-docs
 
 .db-seed_fu: &db-seed_fu
   stage: test
@@ -336,14 +356,17 @@ rake mysql db:rollback:
 rake pg db:seed_fu:
   <<: *db-seed_fu
   <<: *use-pg
+  <<: *except-docs
 
 rake mysql db:seed_fu:
   <<: *db-seed_fu
   <<: *use-mysql
+  <<: *except-docs
 
 rake gitlab:assets:compile:
   stage: test
   <<: *dedicated-runner
+  <<: *except-docs
   dependencies: []
   variables:
     NODE_ENV: "production"
@@ -367,6 +390,7 @@ rake karma:
   stage: test
   <<: *use-pg
   <<: *dedicated-runner
+  <<: *except-docs
   variables:
     BABEL_ENV: "coverage"
   script:
@@ -447,6 +471,7 @@ coverage:
   stage: post-test
   services: []
   <<: *dedicated-runner
+  <<: *except-docs
   variables:
     SETUP_DB: "false"
     USE_BUNDLE_INSTALL: "true"
@@ -462,6 +487,7 @@ coverage:
 
 lint:javascript:
   <<: *dedicated-runner
+  <<: *except-docs
   stage: test
   before_script: []
   script:
@@ -469,6 +495,7 @@ lint:javascript:
 
 lint:javascript:report:
   <<: *dedicated-runner
+  <<: *except-docs
   stage: post-test
   before_script: []
   script:

GITLAB_PAGES_VERSION

-0.4.0
+0.4.1

app/controllers/concerns/service_params.rb

@@ -38,6 +38,7 @@ module ServiceParams
     :new_issue_url,
     :notify,
     :notify_only_broken_pipelines,
+    :notify_only_default_branch,
     :password,
     :priority,
     :project_key,

app/controllers/unicorn_test_controller.rb

+if Rails.env.test?
+  class UnicornTestController < ActionController::Base
+    def pid
+      render plain: Process.pid.to_s
+    end
+  
+    def kill
+      Process.kill(params[:signal], Process.pid)
+      render plain: 'Bye!'
+    end
+  end
+end

app/helpers/events_helper.rb

@@ -10,11 +10,12 @@ module EventsHelper
     'deleted' => 'icon_trash_o'
   }.freeze
 
-  def link_to_author(event)
+  def link_to_author(event, self_added: false)
     author = event.author
 
     if author
-      link_to author.name, user_path(author.username), title: author.name
+      name = self_added ? 'You' : author.name
+      link_to name, user_path(author.username), title: name
     else
       event.author_name
     end

app/helpers/todos_helper.rb

@@ -13,13 +13,13 @@ module TodosHelper
 
   def todo_action_name(todo)
     case todo.action
-    when Todo::ASSIGNED then 'assigned you'
-    when Todo::MENTIONED then 'mentioned you on'
+    when Todo::ASSIGNED then todo.self_added? ? 'assigned' : 'assigned you'
+    when Todo::MENTIONED then "mentioned #{todo_action_subject(todo)} on"
     when Todo::BUILD_FAILED then 'The build failed for'
     when Todo::MARKED then 'added a todo for'
-    when Todo::APPROVAL_REQUIRED then 'set you as an approver for'
+    when Todo::APPROVAL_REQUIRED then "set #{todo_action_subject(todo)} as an approver for"
     when Todo::UNMERGEABLE then 'Could not merge'
-    when Todo::DIRECTLY_ADDRESSED then 'directly addressed you on'
+    when Todo::DIRECTLY_ADDRESSED then "directly addressed #{todo_action_subject(todo)} on"
     end
   end
 
@@ -148,6 +148,10 @@ module TodosHelper
 
   private
 
+  def todo_action_subject(todo)
+    todo.self_added? ? 'yourself' : 'you'
+  end
+
   def show_todo_state?(todo)
     (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state)
   end

app/models/member.rb

@@ -154,6 +154,11 @@ class Member < ActiveRecord::Base
     def add_users(source, users, access_level, current_user: nil, expires_at: nil)
       return [] unless users.present?
 
+      # Collect all user ids into separate array
+      # so we can use single sql query to get user objects
+      user_ids = users.select { |user| user =~ /\A\d+\Z/ }
+      users = users - user_ids + User.where(id: user_ids)
+
       self.transaction do
         users.map do |user|
           add_user(

app/models/todo.rb

@@ -84,6 +84,10 @@ class Todo < ActiveRecord::Base
     action == BUILD_FAILED
   end
 
+  def assigned?
+    action == ASSIGNED
+  end
+
   def action_name
     ACTION_NAMES[action]
   end
@@ -117,6 +121,14 @@ class Todo < ActiveRecord::Base
     end
   end
 
+  def self_added?
+    author == user
+  end
+
+  def self_assigned?
+    assigned? && self_added?
+  end
+
   private
 
   def keep_around_commit

app/views/dashboard/todos/_todo.html.haml

@@ -9,7 +9,7 @@
 
         .title-item.author-name
           - if todo.author
-            = link_to_author(todo)
+            = link_to_author(todo, self_added: todo.self_added?)
           - else
             (removed)
 
@@ -22,6 +22,10 @@
         - else
           (removed)
 
+      - if todo.self_assigned?
+        .title-item.action-name
+          to yourself
+
       .title-item
         ·
 

app/views/shared/milestones/_sidebar.html.haml

@@ -64,7 +64,7 @@
             %span.remaining-days= remaining_days
 
     - if !project || can?(current_user, :read_issue, project)
-      .block
+      .block.issues
         .sidebar-collapsed-icon
           %strong
             = icon('hashtag', 'aria-hidden': 'true')
@@ -85,7 +85,7 @@
               Closed:
               = milestone.issues_visible_to_user(current_user).closed.count
 
-    .block
+    .block.merge-requests
       .sidebar-collapsed-icon
         %strong
           = icon('exclamation', 'aria-hidden': 'true')

changelogs/unreleased/28020-improve-todo-list-when-comes-from-yourself.yml

+---
+title: Improve text on todo list when the todo action comes from yourself
+merge_request: 10594
+author: Jacopo Beschi @jacopo-beschi

changelogs/unreleased/pages-0-4-1.yml

+---
+title: Use GitLab Pages v0.4.1
+merge_request:
+author:

changelogs/unreleased/zj-accept-default-branch-param.yml

+---
+title: Ensure the chat notifications service properly saves the "Notify only default branch" setting
+merge_request: 10959
+author:

config/routes.rb

@@ -99,5 +99,7 @@ Rails.application.routes.draw do
     end
   end
 
+  draw :test if Rails.env.test?
+
   get '*unmatched_route', to: 'application#route_not_found'
 end

config/routes/test.rb

+get '/unicorn_test/pid' => 'unicorn_test#pid'
+post '/unicorn_test/kill' => 'unicorn_test#kill'

doc/development/writing_documentation.md

@@ -70,3 +70,27 @@ All the docs follow the same [styleguide](doc_styleguide.md).
 ### Markdown
 
 Currently GitLab docs use Redcarpet as [markdown](../user/markdown.md) engine, but there's an [open discussion](https://gitlab.com/gitlab-com/gitlab-docs/issues/50) for implementing Kramdown in the near future.
+
+## Testing
+
+We try to treat documentation as code, thus have implemented some testing.
+Currently, the following tests are in place:
+
+1. `docs:check:links`: Check that all internal (relative) links work correctly
+1. `docs:check:apilint`: Check that the API docs follow some conventions
+
+If your contribution contains **only** documentation changes, you can speed up
+the CI process by prepending to the name of your branch: `docs/`. For example,
+a valid name would be `docs/update-api-issues` and it will run only the docs
+tests. If the name is `docs-update-api-issues`, the whole test suite will run
+(including docs).
+
+---
+
+When you submit a merge request to GitLab Community Edition (CE), there is an
+additional job called `rake ee_compat_check` that runs against Enterprise
+Edition (EE) and checks if your changes can apply cleanly to the EE codebase.
+If that job fails, read the instructions in the job log for what to do next.
+Contributors do not need to submit their changes to EE, GitLab Inc. employees
+on the other hand need to make sure that their changes apply cleanly to both
+CE and EE.

lib/tasks/brakeman.rake

@@ -2,7 +2,7 @@ desc 'Security check via brakeman'
 task :brakeman do
   # We get 0 warnings at level 'w3' but we would like to reach 'w2'. Merge
   # requests are welcome!
-  if system(*%w(brakeman --no-progress --skip-files lib/backup/repository.rb -w3 -z))
+  if system(*%w(brakeman --no-progress --skip-files lib/backup/repository.rb,app/controllers/unicorn_test_controller.rb -w3 -z))
     puts 'Security check succeed'
   else
     puts 'Security check failed'

spec/controllers/oauth/authorizations_controller_spec.rb

+require 'spec_helper'
+
+describe Oauth::AuthorizationsController do
+  let(:user) { create(:user) }
+
+  let(:doorkeeper) do
+    Doorkeeper::Application.create(
+      name: "MyApp",
+      redirect_uri: 'http://example.com',
+      scopes: "")
+  end
+
+  let(:params) do
+    {
+      response_type: "code",
+      client_id: doorkeeper.uid,
+      redirect_uri: doorkeeper.redirect_uri,
+      state: 'state'
+    }
+  end
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET #new' do
+    context 'without valid params' do
+      it 'returns 200 code and renders error view' do
+        get :new
+
+        expect(response).to have_http_status(200)
+        expect(response).to render_template('doorkeeper/authorizations/error')
+      end
+    end
+
+    context 'with valid params' do
+      it 'returns 200 code and renders view' do
+        get :new, params
+
+        expect(response).to have_http_status(200)
+        expect(response).to render_template('doorkeeper/authorizations/new')
+      end
+
+      it 'deletes session.user_return_to and redirects when skip authorization' do
+        request.session['user_return_to'] = 'http://example.com'
+        allow(controller).to receive(:skip_authorization?).and_return(true)
+
+        get :new, params
+
+        expect(request.session['user_return_to']).to be_nil
+        expect(response).to have_http_status(302)
+      end
+    end
+  end
+end

spec/features/projects/milestones/milestone_spec.rb

@@ -78,11 +78,10 @@ feature 'Project milestone', :feature do
 
       it 'shows the total MR and issue counts' do
         find('.milestone-sidebar .block', match: :first)
-        blocks = all('.milestone-sidebar .block')
 
         aggregate_failures 'MR and issue blocks' do
-          expect(blocks[3]).to have_content 1
-          expect(blocks[4]).to have_content 0
+          expect(find('.milestone-sidebar .block.issues')).to have_content 1
+          expect(find('.milestone-sidebar .block.merge-requests')).to have_content 0
         end
       end
     end

spec/features/todos/todos_filtering_spec.rb

@@ -45,8 +45,8 @@ describe 'Dashboard > User filters todos', feature: true, js: true do
 
       wait_for_ajax
 
-      expect(find('.todos-list')).to     have_content user_1.name
-      expect(find('.todos-list')).not_to have_content user_2.name
+      expect(find('.todos-list')).to     have_content 'merge request'
+      expect(find('.todos-list')).not_to have_content 'issue'
     end
 
     it "shows only authors of existing todos" do

spec/features/todos/todos_spec.rb

@@ -99,6 +99,83 @@ describe 'Dashboard Todos', feature: true do
       end
     end
 
+    context 'User created todos for themself' do
+      before do
+        login_as(user)
+      end
+
+      context 'issue assigned todo' do
+        before do
+          create(:todo, :assigned, user: user, project: project, target: issue, author: user)
+          visit dashboard_todos_path
+        end
+
+        it 'shows issue assigned to yourself message' do
+          page.within('.js-todos-all')  do
+            expect(page).to have_content("You assigned issue #{issue.to_reference(full: true)} to yourself")
+          end
+        end
+      end
+
+      context 'marked todo' do
+        before do
+          create(:todo, :marked, user: user, project: project, target: issue, author: user)
+          visit dashboard_todos_path
+        end
+
+        it 'shows you added a todo message' do
+          page.within('.js-todos-all')  do
+            expect(page).to have_content("You added a todo for issue #{issue.to_reference(full: true)}")
+            expect(page).not_to have_content('to yourself')
+          end
+        end
+      end
+
+      context 'mentioned todo' do
+        before do
+          create(:todo, :mentioned, user: user, project: project, target: issue, author: user)
+          visit dashboard_todos_path
+        end
+
+        it 'shows you mentioned yourself message' do
+          page.within('.js-todos-all')  do
+            expect(page).to have_content("You mentioned yourself on issue #{issue.to_reference(full: true)}")
+            expect(page).not_to have_content('to yourself')
+          end
+        end
+      end
+
+      context 'directly_addressed todo' do
+        before do
+          create(:todo, :directly_addressed, user: user, project: project, target: issue, author: user)
+          visit dashboard_todos_path
+        end
+
+        it 'shows you directly addressed yourself message' do
+          page.within('.js-todos-all')  do
+            expect(page).to have_content("You directly addressed yourself on issue #{issue.to_reference(full: true)}")
+            expect(page).not_to have_content('to yourself')
+          end
+        end
+      end
+
+      context 'approval todo' do
+        let(:merge_request) { create(:merge_request) }
+
+        before do
+          create(:todo, :approval_required, user: user, project: project, target: merge_request, author: user)
+          visit dashboard_todos_path
+        end
+
+        it 'shows you set yourself as an approver message' do
+          page.within('.js-todos-all')  do
+            expect(page).to have_content("You set yourself as an approver for merge request #{merge_request.to_reference(full: true)}")
+            expect(page).not_to have_content('to yourself')
+          end
+        end
+      end
+    end
+
     context 'User has done todos', js: true do
       before do
         create(:todo, :mentioned, :done, user: user, project: project, target: issue, author: author)

spec/models/member_spec.rb

@@ -390,13 +390,15 @@ describe Member, models: true do
     %w[project group].each do |source_type|
       context "when source is a #{source_type}" do
         let!(:source) { create(source_type, :public, :access_requestable) }
-        let!(:user) { create(:user) }
         let!(:admin) { create(:admin) }
+        let(:user1) { create(:user) }
+        let(:user2) { create(:user) }
 
         it 'returns a <Source>Member objects' do
-          members = described_class.add_users(source, [user], :master)
+          members = described_class.add_users(source, [user1, user2], :master)
 
           expect(members).to be_a Array
+          expect(members.size).to eq(2)
           expect(members.first).to be_a "#{source_type.classify}Member".constantize
           expect(members.first).to be_persisted
         end

spec/models/todo_spec.rb

@@ -125,4 +125,50 @@ describe Todo, models: true do
       expect(subject.target_reference).to eq issue.to_reference(full: true)
     end
   end
+
+  describe '#self_added?' do
+    let(:user_1) { build(:user) }
+
+    before do
+      subject.user = user_1
+    end
+
+    it 'is true when the user is the author' do
+      subject.author = user_1
+
+      expect(subject).to be_self_added
+    end
+
+    it 'is false when the user is not the author' do
+      subject.author = build(:user)
+
+      expect(subject).not_to be_self_added
+    end
+  end
+
+  describe '#self_assigned?' do
+    let(:user_1) { build(:user) }
+
+    before do
+      subject.user = user_1
+      subject.author = user_1
+      subject.action = Todo::ASSIGNED
+    end
+
+    it 'is true when todo is ASSIGNED and self_added' do
+      expect(subject).to be_self_assigned
+    end
+
+    it 'is false when the todo is not ASSIGNED' do
+      subject.action = Todo::MENTIONED
+
+      expect(subject).not_to be_self_assigned
+    end
+
+    it 'is false when todo is not self_added' do
+      subject.author = build(:user)
+
+      expect(subject).not_to be_self_assigned
+    end
+  end
 end

spec/unicorn/unicorn_spec.rb

+require 'fileutils'
+
+require 'excon'
+
+require 'spec_helper'
+
+describe 'Unicorn' do
+  before(:all) do
+    config_lines = File.read('config/unicorn.rb.example').split("\n")
+
+    # Remove these because they make setup harder.
+    config_lines = config_lines.reject do |line|
+      %w[
+        working_directory
+        worker_processes
+        listen
+        pid
+        stderr_path
+        stdout_path
+      ].any? { |prefix| line.start_with?(prefix) }
+    end
+
+    config_lines << "working_directory '#{Rails.root}'"
+
+    # We want to have exactly 1 worker process because that makes it
+    # predictable which process will handle our requests.
+    config_lines << 'worker_processes 1'
+
+    @socket_path = File.join(Dir.pwd, 'tmp/tests/unicorn.socket')
+    config_lines << "listen '#{@socket_path}'"
+
+    ready_file = 'tmp/tests/unicorn-worker-ready'
+    FileUtils.rm_f(ready_file)
+    after_fork_index = config_lines.index { |l| l.start_with?('after_fork') }
+    config_lines.insert(after_fork_index + 1, "File.write('#{ready_file}', Process.pid)")
+
+    config_path = 'tmp/tests/unicorn.rb'
+    File.write(config_path, config_lines.join("\n") + "\n")
+
+    cmd = %W[unicorn -E test -c #{config_path} #{Rails.root.join('config.ru')}]
+    @unicorn_master_pid = spawn(*cmd)
+    wait_unicorn_boot!(@unicorn_master_pid, ready_file)
+    WebMock.allow_net_connect!
+  end
+
+  %w[SIGQUIT SIGTERM SIGKILL].each do |signal|
+    it "has a worker that self-terminates on signal #{signal}" do
+      response = Excon.get('unix:///unicorn_test/pid', socket: @socket_path)
+      expect(response.status).to eq(200)
+
+      worker_pid = response.body.to_i
+      expect(worker_pid).to be > 0
+
+      begin
+        Excon.post('unix:///unicorn_test/kill', socket: @socket_path, body: "signal=#{signal}")
+      rescue Excon::Error::Socket
+        # The connection may be closed abruptly
+      end
+
+      expect(pid_gone?(worker_pid)).to eq(true)
+    end
+  end
+
+  after(:all) do
+    WebMock.disable_net_connect!(allow_localhost: true)
+    Process.kill('TERM', @unicorn_master_pid)
+  end
+
+  def wait_unicorn_boot!(master_pid, ready_file)
+    # Unicorn should boot in under 60 seconds so 120 seconds seems like a good timeout.
+    timeout = 120
+    timeout.times do
+      return if File.exist?(ready_file)
+      pid = Process.waitpid(master_pid, Process::WNOHANG)
+      raise "unicorn failed to boot: #{$?}" unless pid.nil?
+
+      sleep 1
+    end
+
+    raise "unicorn boot timed out after #{timeout} seconds"
+  end
+
+  def pid_gone?(pid)
+    # Worker termination should take less than a second. That makes 10
+    # seconds a generous timeout.
+    10.times do
+      begin
+        Process.kill(0, pid)
+      rescue Errno::ESRCH
+        return true
+      end
+
+      sleep 1
+    end
+
+    false
+  end
+end