Merge branch 'rc/ce-to-ee-wednesday' into 'master'

CE Upstream - Wednesday

Closes #1765

See merge request !1281

.rubocop_todo.yml

@@ -180,7 +180,7 @@ Security/JSONLoad:
 Style/AlignParameters:
   Enabled: false
 
-# Offense count: 53
+# Offense count: 54
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyle, SupportedStyles.
 # SupportedStyles: percent_q, bare_percent

Gemfile

 source 'https://rubygems.org'
 
-gem 'rails', '4.2.7.1'
+gem 'rails', '4.2.8'
 gem 'rails-deprecated_sanitizer', '~> 1.0.3'
 
 # Responders respond_to and respond_with
@@ -342,7 +342,7 @@ gem 'newrelic_rpm', '~> 3.16'
 
 gem 'octokit', '~> 4.6.2'
 
-gem 'mail_room', '~> 0.9.0'
+gem 'mail_room', '~> 0.9.1'
 
 gem 'email_reply_trimmer', '~> 0.1'
 gem 'html2text'

Gemfile.lock

@@ -3,40 +3,39 @@ GEM
   specs:
     RedCloth (4.3.2)
     ace-rails-ap (4.1.0)
-    actionmailer (4.2.7.1)
-      actionpack (= 4.2.7.1)
-      actionview (= 4.2.7.1)
-      activejob (= 4.2.7.1)
+    actionmailer (4.2.8)
+      actionpack (= 4.2.8)
+      actionview (= 4.2.8)
+      activejob (= 4.2.8)
       mail (~> 2.5, >= 2.5.4)
       rails-dom-testing (~> 1.0, >= 1.0.5)
-    actionpack (4.2.7.1)
-      actionview (= 4.2.7.1)
-      activesupport (= 4.2.7.1)
+    actionpack (4.2.8)
+      actionview (= 4.2.8)
+      activesupport (= 4.2.8)
       rack (~> 1.6)
       rack-test (~> 0.6.2)
       rails-dom-testing (~> 1.0, >= 1.0.5)
       rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    actionview (4.2.7.1)
-      activesupport (= 4.2.7.1)
+    actionview (4.2.8)
+      activesupport (= 4.2.8)
       builder (~> 3.1)
       erubis (~> 2.7.0)
       rails-dom-testing (~> 1.0, >= 1.0.5)
-      rails-html-sanitizer (~> 1.0, >= 1.0.2)
-    activejob (4.2.7.1)
-      activesupport (= 4.2.7.1)
+      rails-html-sanitizer (~> 1.0, >= 1.0.3)
+    activejob (4.2.8)
+      activesupport (= 4.2.8)
       globalid (>= 0.3.0)
-    activemodel (4.2.7.1)
-      activesupport (= 4.2.7.1)
+    activemodel (4.2.8)
+      activesupport (= 4.2.8)
       builder (~> 3.1)
-    activerecord (4.2.7.1)
-      activemodel (= 4.2.7.1)
-      activesupport (= 4.2.7.1)
+    activerecord (4.2.8)
+      activemodel (= 4.2.8)
+      activesupport (= 4.2.8)
       arel (~> 6.0)
     activerecord_sane_schema_dumper (0.2)
       rails (>= 4, < 5)
-    activesupport (4.2.7.1)
+    activesupport (4.2.8)
       i18n (~> 0.7)
-      json (~> 1.7, >= 1.7.7)
       minitest (~> 5.1)
       thread_safe (~> 0.3, >= 0.3.4)
       tzinfo (~> 1.1)
@@ -47,7 +46,7 @@ GEM
       activerecord (>= 3.0)
     akismet (2.0.0)
     allocations (1.0.5)
-    arel (6.0.3)
+    arel (6.0.4)
     asana (0.4.0)
       faraday (~> 0.9)
       faraday_middleware (~> 0.9)
@@ -127,7 +126,7 @@ GEM
       execjs
     coffee-script-source (1.10.0)
     colorize (0.7.7)
-    concurrent-ruby (1.0.2)
+    concurrent-ruby (1.0.4)
     connection_pool (2.2.1)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
@@ -378,7 +377,7 @@ GEM
       json (~> 1.8)
       multi_xml (>= 0.5.2)
     httpclient (2.8.2)
-    i18n (0.7.0)
+    i18n (0.8.0)
     ice_nine (0.11.1)
     influxdb (0.2.3)
       cause
@@ -394,7 +393,7 @@ GEM
       thor (>= 0.14, < 2.0)
     jquery-ui-rails (5.0.5)
       railties (>= 3.2.16)
-    json (1.8.3)
+    json (1.8.6)
     json-schema (2.6.2)
       addressable (~> 2.3.8)
     jwt (1.5.6)
@@ -433,7 +432,7 @@ GEM
       nokogiri (>= 1.5.9)
     mail (2.6.4)
       mime-types (>= 1.16, < 4)
-    mail_room (0.9.0)
+    mail_room (0.9.1)
     memoist (0.15.0)
     method_source (0.8.2)
     mime-types (2.99.3)
@@ -453,9 +452,8 @@ GEM
     net-ssh (3.0.1)
     netrc (0.11.0)
     newrelic_rpm (3.16.0.318)
-    nokogiri (1.6.8)
+    nokogiri (1.6.8.1)
       mini_portile2 (~> 2.1.0)
-      pkg-config (~> 1.1.7)
     numerizer (0.1.1)
     oauth (0.5.1)
     oauth2 (1.2.0)
@@ -530,7 +528,6 @@ GEM
     parser (2.3.1.4)
       ast (~> 2.2)
     pg (0.18.4)
-    pkg-config (1.1.7)
     poltergeist (1.9.0)
       capybara (~> 2.1)
       cliver (~> 0.3.1)
@@ -572,28 +569,28 @@ GEM
       rack
     rack-test (0.6.3)
       rack (>= 1.0)
-    rails (4.2.7.1)
-      actionmailer (= 4.2.7.1)
-      actionpack (= 4.2.7.1)
-      actionview (= 4.2.7.1)
-      activejob (= 4.2.7.1)
-      activemodel (= 4.2.7.1)
-      activerecord (= 4.2.7.1)
-      activesupport (= 4.2.7.1)
+    rails (4.2.8)
+      actionmailer (= 4.2.8)
+      actionpack (= 4.2.8)
+      actionview (= 4.2.8)
+      activejob (= 4.2.8)
+      activemodel (= 4.2.8)
+      activerecord (= 4.2.8)
+      activesupport (= 4.2.8)
       bundler (>= 1.3.0, < 2.0)
-      railties (= 4.2.7.1)
+      railties (= 4.2.8)
       sprockets-rails
     rails-deprecated_sanitizer (1.0.3)
       activesupport (>= 4.2.0.alpha)
-    rails-dom-testing (1.0.7)
+    rails-dom-testing (1.0.8)
       activesupport (>= 4.2.0.beta, < 5.0)
-      nokogiri (~> 1.6.0)
+      nokogiri (~> 1.6)
       rails-deprecated_sanitizer (>= 1.0.1)
     rails-html-sanitizer (1.0.3)
       loofah (~> 2.0)
-    railties (4.2.7.1)
-      actionpack (= 4.2.7.1)
-      activesupport (= 4.2.7.1)
+    railties (4.2.8)
+      actionpack (= 4.2.8)
+      activesupport (= 4.2.8)
       rake (>= 0.8.7)
       thor (>= 0.18.1, < 2.0)
     rainbow (2.1.0)
@@ -757,10 +754,10 @@ GEM
       spring (>= 0.9.1)
     spring-commands-spinach (1.1.0)
       spring (>= 0.9.1)
-    sprockets (3.7.0)
+    sprockets (3.7.1)
       concurrent-ruby (~> 1.0)
       rack (> 1, < 3)
-    sprockets-rails (3.1.1)
+    sprockets-rails (3.2.0)
       actionpack (>= 4.0)
       activesupport (>= 4.0)
       sprockets (>= 3.0.0)
@@ -784,7 +781,7 @@ GEM
       daemons (~> 1.0, >= 1.0.9)
       eventmachine (~> 1.0, >= 1.0.4)
       rack (>= 1, < 3)
-    thor (0.19.1)
+    thor (0.19.4)
     thread_safe (0.3.5)
     tilt (2.0.5)
     timecop (0.8.1)
@@ -942,7 +939,7 @@ DEPENDENCIES
   license_finder (~> 2.1.0)
   licensee (~> 8.0.0)
   loofah (~> 2.0.3)
-  mail_room (~> 0.9.0)
+  mail_room (~> 0.9.1)
   method_source (~> 0.8)
   minitest (~> 5.7.0)
   mousetrap-rails (~> 1.4.6)
@@ -980,7 +977,7 @@ DEPENDENCIES
   rack-cors (~> 0.4.0)
   rack-oauth2 (~> 1.2.1)
   rack-proxy (~> 0.6.0)
-  rails (= 4.2.7.1)
+  rails (= 4.2.8)
   rails-deprecated_sanitizer (~> 1.0.3)
   rainbow (~> 2.1.0)
   rblineprof (~> 0.3.6)

README.md

@@ -59,7 +59,7 @@ We're hiring developers, support people, and production engineers all the time,
 There are two editions of GitLab:
 
 - GitLab Community Edition (CE) is available freely under the MIT Expat license.
-- GitLab Enterprise Edition (EE) includes [extra features](https://about.gitlab.com/features/#compare) that are more useful for organizations with more than 100 users. To use EE and get official support please [become a subscriber](https://about.gitlab.com/pricing/).
+- GitLab Enterprise Edition (EE) includes [extra features](https://about.gitlab.com/products/#compare-options) that are more useful for organizations with more than 100 users. To use EE and get official support please [become a subscriber](https://about.gitlab.com/products/).
 
 ## Website
 

app/assets/javascripts/create_label.js.es6

@@ -107,9 +107,9 @@
           if (typeof label.message === 'string') {
             errors = label.message;
           } else {
-            errors = label.message.map(function (value, key) {
-              return key + " " + value[0];
-            }).join("<br/>");
+            errors = Object.keys(label.message).map(key =>
+              `${gl.text.humanize(key)} ${label.message[key].join(', ')}`
+            ).join("<br/>");
           }
 
           this.$newLabelError

app/assets/javascripts/filtered_search/filtered_search_dropdown.js.es6

@@ -101,6 +101,7 @@
 
     resetFilters() {
       const hook = this.getCurrentHook();
+
       if (hook) {
         const data = hook.list.data;
         const results = data.map((o) => {

app/assets/javascripts/filtered_search/filtered_search_manager.js.es6

@@ -122,7 +122,7 @@
         const keyParam = decodeURIComponent(split[0]);
         const value = split[1];
 
-        // Check if it matches edge conditions listed in gl.FilteredSearchTokenKeys
+        // Check if it matches edge conditions listed in this.filteredSearchTokenKeys
         const condition = this.filteredSearchTokenKeys.searchByConditionUrl(p);
 
         if (condition) {

app/assets/javascripts/gl_dropdown.js

@@ -47,9 +47,10 @@
           }
           // Only filter asynchronously only if option remote is set
           if (this.options.remote) {
-            $inputContainer.parent().addClass('is-loading');
             clearTimeout(timeout);
             return timeout = setTimeout(function() {
+              $inputContainer.parent().addClass('is-loading');
+
               return this.options.query(this.input.val(), function(data) {
                 $inputContainer.parent().removeClass('is-loading');
                 return this.options.callback(data);

app/assets/javascripts/header.js

@@ -2,7 +2,7 @@
 (function() {
   $(document).on('todo:toggle', function(e, count) {
     var $todoPendingCount = $('.todos-pending-count');
-    $todoPendingCount.text(gl.text.addDelimiter(count));
+    $todoPendingCount.text(gl.text.highCountTrim(count));
     $todoPendingCount.toggleClass('hidden', count === 0);
   });
 })();

app/assets/javascripts/lib/utils/text_utility.js

@@ -14,6 +14,9 @@ require('vendor/latinise');
     gl.text.addDelimiter = function(text) {
       return text ? text.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ",") : text;
     };
+    gl.text.highCountTrim = function(count) {
+      return count > 99 ? '99+' : count;
+    };
     gl.text.randomString = function() {
       return Math.random().toString(36).substring(7);
     };

app/assets/stylesheets/pages/todos.scss

@@ -6,6 +6,8 @@
 .navbar-nav {
   li {
     .badge.todos-pending-count {
+      position: inherit;
+      top: -6px;
       margin-top: -5px;
       font-weight: normal;
       background: $todo-alert-blue;
@@ -43,6 +45,12 @@
     }
   }
 
+  .todo-avatar,
+  .todo-actions {
+    -webkit-flex: 0 0 auto;
+    flex: 0 0 auto;
+  }
+
   .todo-actions {
     display: -webkit-flex;
     display: flex;
@@ -55,8 +63,9 @@
   }
 
   .todo-item {
-    -webkit-flex: auto;
-    flex: auto;
+    -webkit-flex: 0 1 100%;
+    flex: 0 1 100%;
+    min-width: 0;
   }
 }
 
@@ -74,8 +83,29 @@
 
 .todo-item {
   .todo-title {
-    @include str-truncated(calc(100% - 174px));
-    overflow: visible;
+    display: flex;
+
+    & > .title-item {
+      -webkit-flex: 0 0 auto;
+      flex: 0 0 auto;
+      margin: 0 2px;
+
+      &:first-child {
+        margin-left: 0;
+      }
+
+      &:last-child {
+        margin-right: 0;
+      }
+    }
+
+    .todo-label {
+      -webkit-flex: 0 1 auto;
+      flex: 0 1 auto;
+      overflow: hidden;
+      text-overflow: ellipsis;
+      white-space: nowrap;
+    }
   }
 
   .status-box {
@@ -154,10 +184,12 @@
 
   .todo-item {
     .todo-title {
-      white-space: normal;
-      overflow: visible;
-      max-width: 100%;
+      flex-flow: row wrap;
       margin-bottom: 10px;
+
+      .todo-label {
+        white-space: normal;
+      }
     }
 
     .todo-body {

app/controllers/dashboard/todos_controller.rb

@@ -35,6 +35,11 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     render json: todos_counts
   end
 
+  # Used in TodosHelper also
+  def self.todos_count_format(count)
+    count >= 100 ? '99+' : count
+  end
+
   private
 
   def find_todos

app/controllers/projects/merge_requests_controller.rb

@@ -58,6 +58,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
       assignee = User.find_by_id(params[:assignee_id])
       @users.push(assignee) if assignee
     end
+
     if params[:author_id].present?
       author = User.find_by_id(params[:author_id])
       @users.push(author) if author

app/helpers/namespaces_helper.rb

 module NamespacesHelper
+  def namespace_id_from(params)
+    params.dig(:project, :namespace_id) || params[:namespace_id]
+  end
+
   def namespaces_options(selected = :current_user, display_path: false, extra_group: nil)
     groups = current_user.owned_groups + current_user.masters_groups
 

app/helpers/todos_helper.rb

@@ -3,6 +3,10 @@ module TodosHelper
     @todos_pending_count ||= current_user.todos_pending_count
   end
 
+  def todos_count_format(count)
+    count > 99 ? '99+' : count
+  end
+
   def todos_done_count
     @todos_done_count ||= current_user.todos_done_count
   end

app/services/users/destroy_service.rb

@@ -7,6 +7,10 @@ module Users
     end
 
     def execute(user, options = {})
+      unless current_user.admin? || current_user == user
+        raise Gitlab::Access::AccessDeniedError, "#{current_user} tried to destroy user #{user}!"
+      end
+
       if !options[:delete_solo_owned_groups] && user.solo_owned_groups.present?
         user.errors[:base] << 'You must transfer ownership or delete groups before you can remove user'
         return user

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

 %li{ class: "todo todo-#{todo.done? ? 'done' : 'pending'}", id: dom_id(todo), data: { url: todo_target_path(todo) } }
-  = author_avatar(todo, size: 40)
+  .todo-avatar
+    = author_avatar(todo, size: 40)
 
   .todo-item.todo-block
     .todo-title.title
       - unless todo.build_failed? || todo.unmergeable?
         = todo_target_state_pill(todo)
 
-        %span.author-name
+        .title-item.author-name
           - if todo.author
             = link_to_author(todo)
           - else
             (removed)
 
-      %span.action-name
+      .title-item.action-name
         = todo_action_name(todo)
 
-      %span.todo-label
+      .title-item.todo-label
         - if todo.target
           = todo_target_link(todo)
         - else
           (removed)
 
-      · #{time_ago_with_tooltip(todo.created_at)}
-      = todo_due_date(todo)
+      .title-item
+        ·
+
+      .title-item
+        #{time_ago_with_tooltip(todo.created_at)}
+        = todo_due_date(todo)
 
     .todo-body
       .todo-note

app/views/layouts/header/_default.html.haml

@@ -35,11 +35,13 @@
               = link_to dashboard_todos_path, title: 'Todos', aria: { label: "Todos" }, data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
                 = icon('bell fw')
                 %span.badge.todos-pending-count{ class: ("hidden" if todos_pending_count == 0) }
-                  = todos_pending_count
+                  = todos_count_format(todos_pending_count)
+
             - if Gitlab::Geo.secondary?
               %li
                 = link_to Gitlab::Geo.primary_node.url, title: 'Go to primary node', data: {toggle: 'tooltip', placement: 'bottom', container: 'body'} do
                   = icon('globe fw')
+
             - if Gitlab::Sherlock.enabled?
               %li
                 = link_to sherlock_transactions_path, title: 'Sherlock Transactions',

app/views/projects/commit/_pipeline.html.haml

@@ -3,7 +3,7 @@
     .pull-right
       - if can?(current_user, :update_pipeline, pipeline.project)
         - if pipeline.builds.latest.failed.any?(&:retryable?)
-          = link_to "Retry failed", retry_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id), class: 'btn btn-grouped btn-primary', method: :post
+          = link_to "Retry", retry_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id), class: 'js-retry-button btn btn-grouped btn-primary', method: :post
 
         - if pipeline.builds.running_or_pending.any?
           = link_to "Cancel running", cancel_namespace_project_pipeline_path(pipeline.project.namespace, pipeline.project, pipeline.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-grouped btn-danger', method: :post

app/views/projects/new.html.haml

@@ -22,7 +22,7 @@
                 - if current_user.can_select_namespace?
                   .input-group-addon
                     = root_url
-                  = f.select :namespace_id, namespaces_options(params[:namespace_id] || :current_user, display_path: true), {}, {class: 'select2 js-select-namespace', tabindex: 1}
+                  = f.select :namespace_id, namespaces_options(namespace_id_from(params) || :current_user, display_path: true), {}, {class: 'select2 js-select-namespace', tabindex: 1}
 
                 - else
                   .input-group-addon.static-namespace

app/views/projects/pipelines/_info.html.haml

@@ -8,7 +8,7 @@
   .header-action-buttons
     - if can?(current_user, :update_pipeline, @pipeline.project)
       - if @pipeline.retryable?
-        = link_to "Retry failed", retry_namespace_project_pipeline_path(@pipeline.project.namespace, @pipeline.project, @pipeline.id), class: 'btn btn-inverted-secondary', method: :post
+        = link_to "Retry", retry_namespace_project_pipeline_path(@pipeline.project.namespace, @pipeline.project, @pipeline.id), class: 'js-retry-button btn btn-inverted-secondary', method: :post
       - if @pipeline.cancelable?
         = link_to "Cancel running", cancel_namespace_project_pipeline_path(@pipeline.project.namespace, @pipeline.project, @pipeline.id), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post
 

app/views/projects/pipelines_settings/_show.html.haml

@@ -60,7 +60,7 @@
           = f.label :build_coverage_regex, "Test coverage parsing", class: 'label-light'
           .input-group
             %span.input-group-addon /
-            = f.text_field :build_coverage_regex, class: 'form-control', placeholder: '\(\d+.\d+\%\) covered'
+            = f.text_field :build_coverage_regex, class: 'form-control', placeholder: 'Regular expression'
             %span.input-group-addon /
           %p.help-block
             A regular expression that will be used to find the test coverage

app/views/search/_results.html.haml

@@ -11,7 +11,7 @@
 
   .results.prepend-top-10
     - if @scope == 'commits'
-      %ul.list-unstyled
+      %ul.content-list.commit-list.table-list.table-wide
         = render partial: "search/results/commit", collection: @search_objects
     - else
       .search-results

app/views/search/results/_snippet_blob.html.haml

@@ -12,41 +12,34 @@
     %span.light= time_ago_with_tooltip(snippet.created_at)
   %h4.snippet-title
   - snippet_path = reliable_snippet_path(snippet)
-  = link_to snippet_path do
-    .file-holder
-      .js-file-title.file-title
+  .file-holder
+    .js-file-title.file-title
+      = link_to snippet_path do
         %i.fa.fa-file
         %strong= snippet.file_name
-      - if markup?(snippet.file_name)
-        .file-content.wiki
+    - if markup?(snippet.file_name)
+      .file-content.wiki
+        - snippet_chunks.each do |chunk|
+          - unless chunk[:data].empty?
+            = render_markup(snippet.file_name, chunk[:data])
+          - else
+            .file-content.code
+              .nothing-here-block Empty file
+    - else
+      .file-content.code.js-syntax-highlight
+        .line-numbers
           - snippet_chunks.each do |chunk|
             - unless chunk[:data].empty?
-              = render_markup(snippet.file_name, chunk[:data])
+              - Gitlab::Git::Util.count_lines(chunk[:data]).times do |index|
+                - offset = defined?(chunk[:start_line]) ? chunk[:start_line] : 1
+                - i = index + offset
+                = link_to snippet_path+"#L#{i}", id: "L#{i}", rel: "#L#{i}", class: "diff-line-num" do
+                  %i.fa.fa-link
+                  = i
+        .blob-content
+          - snippet_chunks.each do |chunk|
+            - unless chunk[:data].empty?
+              = highlight(snippet.file_name, chunk[:data], repository: nil, plain: snippet.no_highlighting?)
             - else
               .file-content.code
                 .nothing-here-block Empty file
-      - else
-        .file-content.code.js-syntax-highlight
-          .line-numbers
-            - snippet_chunks.each do |chunk|
-              - unless chunk[:data].empty?
-                - Gitlab::Git::Util.count_lines(chunk[:data]).times do |index|
-                  - offset = defined?(chunk[:start_line]) ? chunk[:start_line] : 1
-                  - i = index + offset
-                  = link_to snippet_path+"#L#{i}", id: "L#{i}", rel: "#L#{i}", class: "diff-line-num" do
-                    %i.fa.fa-link
-                    = i
-                - unless snippet == snippet_chunks.last
-                  %a.diff-line-num
-                    = "."
-          %pre.code
-            %code
-              - snippet_chunks.each do |chunk|
-                - unless chunk[:data].empty?
-                  = chunk[:data]
-                  - unless chunk == snippet_chunks.last
-                    %a
-                      = "..."
-                - else
-                  .file-content.code
-                    .nothing-here-block Empty file

app/views/shared/members/_member.html.haml

@@ -65,7 +65,7 @@
                 = dropdown_title("Change permissions")
                 .dropdown-content
                   %ul
-                    - Gitlab::Access.options.each do |role, role_id|
+                    - member.class.access_level_roles.each do |role, role_id|
                       %li
                         = link_to role, "javascript:void(0)",
                           class: ("is-active" if member.access_level == role_id),

app/workers/delete_user_worker.rb

@@ -7,5 +7,7 @@ class DeleteUserWorker
     current_user = User.find(current_user_id)
 
     Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys)
+  rescue Gitlab::Access::AccessDeniedError => e
+    Rails.logger.warn("User could not be destroyed: #{e}")
   end
 end

changelogs/unreleased/1363-redo-mailroom-support.yml

+---
+title: Redo internals of Incoming Mail Support
+merge_request: 9385
+author:

changelogs/unreleased/26703-todos-count.yml

+---
+title: show 99+ for large count in todos notification bell
+merge_request: 9171
+author: mhasbini

changelogs/unreleased/26881-backup-fails-if-data-changes.yml

+---
+title: Add `copy` backup strategy to combat file changed errors
+merge_request: 8728
+author:

changelogs/unreleased/27287-label-dropdown-error-messages.yml

+---
+title: Fix displaying error messages for create label dropdown
+merge_request: 9058
+author: Tom Koole

changelogs/unreleased/28186-long-group-names-overflow-out-of-todos-view.yml

+---
+title: Truncate long Todo titles for non-mobile screens
+merge_request: 9311
+author:

changelogs/unreleased/28389-ux-problem-with-pipeline-coverage-placeholder.yml

+---
+title: Changed coverage reg expression placeholder text to be more like a placeholder
+merge_request:
+author:

changelogs/unreleased/add-filtered-search-to-mr.yml

+---
+title: Add filtered search to MR page
+merge_request:
+author:

changelogs/unreleased/api-todos-restful.yml

+---
+title: 'API: Use POST requests to mark todos as done'
+merge_request: 9410
+author: Robert Schilling

changelogs/unreleased/commit-search-ui-fix.yml

+---
+title: Fixed commit search UI
+merge_request:
+author:

changelogs/unreleased/group-memebrs-owner-level.yml

+---
+title: Added option to update to owner for group members
+merge_request:
+author:

changelogs/unreleased/rename-retry-failed-pipeline-to-retry.yml

+---
+title: Rename retry failed button on pipeline page to just retry
+merge_request:
+author:

changelogs/unreleased/sh-delete-user-permission-check.yml

+---
+title: Add user deletion permission check in `Users::DestroyService`
+merge_request:
+author:

changelogs/unreleased/snippets-search.yml

+---
+title: Fix snippets search result spacing
+merge_request:
+author:

config/mail_room.yml

-# If you change this file in a Merge Request, please also create
-# a Merge Request on https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests
-#
 :mailboxes:
   <%
-    require_relative "lib/gitlab/mail_room" unless defined?(Gitlab::MailRoom)
+    require_relative "../lib/gitlab/mail_room" unless defined?(Gitlab::MailRoom)
     config = Gitlab::MailRoom.config
 
     if Gitlab::MailRoom.enabled?

doc/api/pipelines.md

@@ -163,7 +163,7 @@ Example of response
 }
 ```
 
-## Retry failed builds in a pipeline
+## Retry builds in a pipeline
 
 > [Introduced][ce-5837] in GitLab 8.11
 

doc/api/todos.md

@@ -184,7 +184,7 @@ Marks a single pending todo given by its ID for the current user as done. The
 todo marked as done is returned in the response.
 
 ```
-DELETE /todos/:id
+POST /todos/:id/mark_as_done
 ```
 
 Parameters:
@@ -194,7 +194,7 @@ Parameters:
 | `id` | integer | yes | The ID of a todo |
 
 ```bash
-curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/130
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/130/mark_as_done
 ```
 
 Example Response:
@@ -277,20 +277,15 @@ Example Response:
 
 ## Mark all todos as done
 
-Marks all pending todos for the current user as done. It returns the number of marked todos.
+Marks all pending todos for the current user as done. It returns the HTTP status code `204` with an empty response.
 
 ```
-DELETE /todos
+POST /todos/mark_as_done
 ```
 
 ```bash
-curl --request DELETE --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos
+curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v3/todos/donmark_as_donee
 ```
 
-Example Response:
-
-```json
-3
-```
 
 [ce-3188]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3188

doc/api/v3_to_v4.md

@@ -26,6 +26,7 @@ changes are in V4:
   - `/gitignores/:key`
   - `/gitlab_ci_ymls/:key`
   - `/dockerfiles/:key`
+- Moved `DELETE /todos` to `POST /todos/mark_as_done` and `DELETE /todos/:todo_id` to `POST /todos/:todo_id/mark_as_done`
 - Moved `/projects/fork/:id` to `/projects/:id/fork` [!8940](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8940)
 - Endpoints `/projects/owned`, `/projects/visible`, `/projects/starred` & `/projects/all` are consolidated into `/projects` using query parameters [!8962](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8962)
 - Return pagination headers for all endpoints that return an array [!8606](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8606)

doc/ci/docker/using_docker_images.md

@@ -39,13 +39,15 @@ accessible during the build process.
 
 ## What is an image
 
-The `image` keyword is the name of the docker image that is present in the
-local Docker Engine (list all images with `docker images`) or any image that
-can be found at [Docker Hub][hub]. For more information about images and Docker
-Hub please read the [Docker Fundamentals][] documentation.
+The `image` keyword is the name of the docker image the docker executor
+will run to perform the CI tasks.  
 
-In short, with `image` we refer to the docker image, which will be used to
-create a container on which your job will run.
+By default the executor will only pull images from [Docker Hub][hub],
+but this can be configured in the `gitlab-runner/config.toml` by setting
+the [docker pull policy][] to allow using local images.
+
+For more information about images and Docker Hub please read
+the [Docker Fundamentals][] documentation.
 
 ## What is a service
 
@@ -271,6 +273,7 @@ containers as well as all volumes (`-v`) that were created with the container
 creation.
 
 [Docker Fundamentals]: https://docs.docker.com/engine/understanding-docker/
+[docker pull policy]: https://docs.gitlab.com/runner/executors/docker.html#how-pull-policies-work
 [hub]: https://hub.docker.com/
 [linking-containers]: https://docs.docker.com/engine/userguide/networking/default_network/dockerlinks/
 [tutum/wordpress]: https://hub.docker.com/r/tutum/wordpress/

doc/raketasks/backup_restore.md

@@ -84,6 +84,28 @@ Deleting tmp directories...[DONE]
 Deleting old backups... [SKIPPING]
 ```
 
+## Backup Strategy Option
+
+> **Note:** Introduced as an option in 8.17
+
+The default backup strategy is to essentially stream data from the respective
+data locations to the backup using the Linux command `tar` and `gzip`. This works
+fine in most cases, but can cause problems when data is rapidly changing.
+
+When data changes while `tar` is reading it, the error `file changed as we read
+it` may occur, and will cause the backup process to fail. To combat this, 8.17
+introduces a new backup strategy called `copy`. The strategy copies data files
+to a temporary location before calling `tar` and `gzip`, avoiding the error.
+
+A side-effect is that the backup process with take up to an additional 1X disk
+space. The process does its best to clean up the temporary files at each stage
+so the problem doesn't compound, but it could be a considerable change for large
+installations. This is why the `copy` strategy is not the default in 8.17.
+
+To use the `copy` strategy instead of the default streaming strategy, specify
+`STRATEGY=copy` in the Rake task command. For example,
+`sudo gitlab-rake gitlab:backup:create STRATEGY=copy`.
+
 ## Exclude specific directories from the backup
 
 You can choose what should be backed up by adding the environment variable `SKIP`.

doc/workflow/todos.md

@@ -16,7 +16,8 @@ in a simple dashboard.
 
 You can quickly access the Todos dashboard using the bell icon next to the
 search bar in the upper right corner. The number in blue is the number of Todos
-you still have open.
+you still have open if the count is < 100, else it's 99+. The exact number
+will still be shown in the body of the _To do_ tab.
 
 ![Todos icon](img/todos_icon.png)
 

lib/api/api.rb

@@ -23,6 +23,7 @@ module API
       mount ::API::V3::Subscriptions
       mount ::API::V3::SystemHooks
       mount ::API::V3::Tags
+      mount ::API::V3::Todos
       mount ::API::V3::Templates
       mount ::API::V3::Users
     end

lib/api/pipelines.rb

@@ -23,7 +23,7 @@ module API
         pipelines = PipelinesFinder.new(user_project).execute(scope: params[:scope])
         present paginate(pipelines), with: Entities::Pipeline
       end
-      
+
       desc 'Create a new pipeline' do
         detail 'This feature was introduced in GitLab 8.14'
         success Entities::Pipeline
@@ -58,7 +58,7 @@ module API
         present pipeline, with: Entities::Pipeline
       end
 
-      desc 'Retry failed builds in the pipeline' do
+      desc 'Retry builds in the pipeline' do
         detail 'This feature was introduced in GitLab 8.11.'
         success Entities::Pipeline
       end

lib/api/todos.rb

@@ -58,7 +58,7 @@ module API
       params do
         requires :id, type: Integer, desc: 'The ID of the todo being marked as done'
       end
-      delete ':id' do
+      post ':id/mark_as_done' do
         todo = current_user.todos.find(params[:id])
         TodoService.new.mark_todos_as_done([todo], current_user)
 
@@ -66,9 +66,11 @@ module API
       end
 
       desc 'Mark all todos as done'
-      delete do
+      post '/mark_as_done' do
         todos = find_todos
         TodoService.new.mark_todos_as_done(todos, current_user)
+
+        no_content!
       end
     end
   end

lib/api/v3/todos.rb

+module API
+  module V3
+    class Todos < Grape::API
+      before { authenticate! }
+
+      resource :todos do
+        desc 'Mark a todo as done' do
+          success ::API::Entities::Todo
+        end
+        params do
+          requires :id, type: Integer, desc: 'The ID of the todo being marked as done'
+        end
+        delete ':id' do
+          todo = current_user.todos.find(params[:id])
+          TodoService.new.mark_todos_as_done([todo], current_user)
+
+          present todo.reload, with: ::API::Entities::Todo, current_user: current_user
+        end
+
+        desc 'Mark all todos as done'
+        delete do
+          todos = TodosFinder.new(current_user, params).execute
+          TodoService.new.mark_todos_as_done(todos, current_user)
+        end
+      end
+    end
+  end
+end

lib/backup/files.rb

@@ -8,6 +8,7 @@ module Backup
       @name = name
       @app_files_dir = File.realpath(app_files_dir)
       @files_parent_dir = File.realpath(File.join(@app_files_dir, '..'))
+      @backup_files_dir = File.join(Gitlab.config.backup.path, File.basename(@app_files_dir) )
       @backup_tarball = File.join(Gitlab.config.backup.path, name + '.tar.gz')
     end
 
@@ -15,7 +16,21 @@ module Backup
     def dump
       FileUtils.mkdir_p(Gitlab.config.backup.path)
       FileUtils.rm_f(backup_tarball)
-      run_pipeline!([%W(tar -C #{app_files_dir} -cf - .), %W(gzip -c -1)], out: [backup_tarball, 'w', 0600])
+
+      if ENV['STRATEGY'] == 'copy'
+        cmd = %W(cp -a #{app_files_dir} #{Gitlab.config.backup.path})
+        output, status = Gitlab::Popen.popen(cmd)
+
+        unless status.zero?
+          puts output
+          abort 'Backup failed'
+        end
+
+        run_pipeline!([%W(tar -C #{@backup_files_dir} -cf - .), %W(gzip -c -1)], out: [backup_tarball, 'w', 0600])
+        FileUtils.rm_rf(@backup_files_dir)
+      else
+        run_pipeline!([%W(tar -C #{app_files_dir} -cf - .), %W(gzip -c -1)], out: [backup_tarball, 'w', 0600])
+      end
     end
 
     def restore

lib/gitlab/database/migration_helpers.rb

@@ -54,7 +54,7 @@ module Gitlab
 
         disable_statement_timeout
 
-        key_name = "fk_#{source}_#{target}_#{column}"
+        key_name = concurrent_foreign_key_name(source, column)
 
         # Using NOT VALID allows us to create a key without immediately
         # validating it. This means we keep the ALTER TABLE lock only for a
@@ -74,6 +74,15 @@ module Gitlab
         execute("ALTER TABLE #{source} VALIDATE CONSTRAINT #{key_name};")
       end
 
+      # Returns the name for a concurrent foreign key.
+      #
+      # PostgreSQL constraint names have a limit of 63 bytes. The logic used
+      # here is based on Rails' foreign_key_name() method, which unfortunately
+      # is private so we can't rely on it directly.
+      def concurrent_foreign_key_name(table, column)
+        "fk_#{Digest::SHA256.hexdigest("#{table}_#{column}_fk").first(10)}"
+      end
+
       # Long-running migrations may take more than the timeout allowed by
       # the database. Disable the session's statement timeout to ensure
       # migrations don't get killed prematurely. (PostgreSQL only)

lib/tasks/gitlab/check.rake

@@ -725,8 +725,11 @@ namespace :gitlab do
     def check_imap_authentication
       print "IMAP server credentials are correct? ... "
 
-      config_path = Rails.root.join('config', 'mail_room.yml')
-      config_file = YAML.load(ERB.new(File.read(config_path)).result)
+      config_path = Rails.root.join('config', 'mail_room.yml').to_s
+      erb = ERB.new(File.read(config_path))
+      erb.filename = config_path
+      config_file = YAML.load(erb.result)
+
       config = config_file[:mailboxes].first
 
       if config

spec/config/mail_room_spec.rb

@@ -8,7 +8,7 @@ describe 'mail_room.yml' do
 
   context 'when incoming email is disabled' do
     before do
-      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/mail_room_disabled.yml').to_s
+      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/config/mail_room_disabled.yml').to_s
       Gitlab::MailRoom.reset_config!
     end
 
@@ -26,7 +26,7 @@ describe 'mail_room.yml' do
     let(:gitlab_redis) { Gitlab::Redis.new(Rails.env) }
 
     before do
-      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/mail_room_enabled.yml').to_s
+      ENV['MAIL_ROOM_GITLAB_CONFIG_FILE'] = Rails.root.join('spec/fixtures/config/mail_room_enabled.yml').to_s
       Gitlab::MailRoom.reset_config!
     end
 

spec/features/commits_spec.rb

@@ -153,7 +153,7 @@ describe 'Commits' do
           expect(page).to have_content pipeline.git_author_name
           expect(page).to have_link('Download artifacts')
           expect(page).not_to have_link('Cancel running')
-          expect(page).not_to have_link('Retry failed')
+          expect(page).not_to have_link('Retry')
         end
       end
 
@@ -172,7 +172,7 @@ describe 'Commits' do
           expect(page).to have_content pipeline.git_author_name
           expect(page).not_to have_link('Download artifacts')
           expect(page).not_to have_link('Cancel running')
-          expect(page).not_to have_link('Retry failed')
+          expect(page).not_to have_link('Retry')
         end
       end
     end

spec/features/groups/members/list_spec.rb

@@ -30,6 +30,21 @@ feature 'Groups members list', feature: true do
     expect(second_row).to be_blank
   end
 
+  it 'updates user to owner level', :js do
+    group.add_owner(user1)
+    group.add_developer(user2)
+
+    visit group_group_members_path(group)
+
+    page.within(second_row) do
+      click_button('Developer')
+
+      click_link('Owner')
+
+      expect(page).to have_button('Owner')
+    end
+  end
+
   def first_row
     page.all('ul.content-list > li')[0]
   end

spec/features/issues/issue_sidebar_spec.rb

@@ -7,9 +7,9 @@ feature 'Issue Sidebar', feature: true do
   let(:project) { create(:project, :public) }
   let(:issue) { create(:issue, project: project) }
   let!(:user) { create(:user)}
+  let!(:label) { create(:label, project: project, title: 'bug') }
 
   before do
-    create(:label, project: project, title: 'bug')
     login_as(user)
   end
 
@@ -50,16 +50,6 @@ feature 'Issue Sidebar', feature: true do
       visit_issue(project, issue)
     end
 
-    describe 'when clicking on edit labels', js: true do
-      it 'shows dropdown option to create a new label' do
-        find('.block.labels .edit-link').click
-
-        page.within('.block.labels') do
-          expect(page).to have_content 'Create new'
-        end
-      end
-    end
-
     context 'sidebar', js: true do
       it 'changes size when the screen size is smaller' do
         sidebar_selector = 'aside.right-sidebar.right-sidebar-collapsed'
@@ -77,36 +67,53 @@ feature 'Issue Sidebar', feature: true do
       end
     end
 
-    context 'creating a new label', js: true do
-      it 'shows option to crate a new label is present' do
+    context 'editing issue labels', js: true do
+      before do
         page.within('.block.labels') do
           find('.edit-link').click
+        end
+      end
 
+      it 'shows option to create a new label' do
+        page.within('.block.labels') do
           expect(page).to have_content 'Create new'
         end
       end
 
-      it 'shows dropdown switches to "create label" section' do
-        page.within('.block.labels') do
-          find('.edit-link').click
-          click_link 'Create new'
+      context 'creating a new label', js: true do
+        before do
+          page.within('.block.labels') do
+            click_link 'Create new'
+          end
+        end
 
-          expect(page).to have_content 'Create new label'
+        it 'shows dropdown switches to "create label" section' do
+          page.within('.block.labels') do
+            expect(page).to have_content 'Create new label'
+          end
         end
-      end
 
-      it 'adds new label' do
-        page.within('.block.labels') do
-          find('.edit-link').click
-          sleep 1
-          click_link 'Create new'
+        it 'adds new label' do
+          page.within('.block.labels') do
+            fill_in 'new_label_name', with: 'wontfix'
+            page.find(".suggest-colors a", match: :first).click
+            click_button 'Create'
+
+            page.within('.dropdown-page-one') do
+              expect(page).to have_content 'wontfix'
+            end
+          end
+        end
 
-          fill_in 'new_label_name', with: 'wontfix'
-          page.find(".suggest-colors a", match: :first).click
-          click_button 'Create'
+        it 'shows error message if label title is taken' do
+          page.within('.block.labels') do
+            fill_in 'new_label_name', with: label.title
+            page.find('.suggest-colors a', match: :first).click
+            click_button 'Create'
 
-          page.within('.dropdown-page-one') do
-            expect(page).to have_content 'wontfix'
+            page.within('.dropdown-page-two') do
+              expect(page).to have_content 'Title has already been taken'
+            end
           end
         end
       end

spec/features/projects/new_project_spec.rb

@@ -19,6 +19,51 @@ feature "New project", feature: true do
     end
   end
 
+  context "Namespace selector" do
+    context "with user namespace" do
+      before do
+        visit new_project_path
+      end
+
+      it "selects the user namespace" do
+        namespace = find("#project_namespace_id")
+
+        expect(namespace.text).to eq user.username
+      end
+    end
+
+    context "with group namespace" do
+      let(:group) { create(:group, :private, owner: user) }
+
+      before do
+        group.add_owner(user)
+        visit new_project_path(namespace_id: group.id)
+      end
+
+      it "selects the group namespace" do
+        namespace = find("#project_namespace_id option[selected]")
+
+        expect(namespace.text).to eq group.name
+      end
+
+      context "on validation error" do
+        before do
+          fill_in('project_path', with: 'private-group-project')
+          choose('Internal')
+          click_button('Create project')
+
+          expect(page).to have_css '.project-edit-errors .alert.alert-danger'
+        end
+
+        it "selects the group namespace" do
+          namespace = find("#project_namespace_id option[selected]")
+
+          expect(namespace.text).to eq group.name
+        end
+      end
+    end
+  end
+
   context 'Import project options' do
     before do
       visit new_project_path

spec/features/projects/pipelines/pipeline_spec.rb

@@ -54,7 +54,7 @@ describe 'Pipeline', :feature, :js do
       expect(page).to have_content('Build')
       expect(page).to have_content('Test')
       expect(page).to have_content('Deploy')
-      expect(page).to have_content('Retry failed')
+      expect(page).to have_content('Retry')
       expect(page).to have_content('Cancel running')
     end
 
@@ -164,9 +164,9 @@ describe 'Pipeline', :feature, :js do
       it { expect(page).not_to have_content('retried') }
 
       context 'when retrying' do
-        before { click_on 'Retry failed' }
+        before { find('.js-retry-button').trigger('click') }
 
-        it { expect(page).not_to have_content('Retry failed') }
+        it { expect(page).not_to have_content('Retry') }
       end
     end
 
@@ -198,7 +198,7 @@ describe 'Pipeline', :feature, :js do
       expect(page).to have_content(build_failed.id)
       expect(page).to have_content(build_running.id)
       expect(page).to have_content(build_external.id)
-      expect(page).to have_content('Retry failed')
+      expect(page).to have_content('Retry')
       expect(page).to have_content('Cancel running')
       expect(page).to have_link('Play')
     end
@@ -226,9 +226,9 @@ describe 'Pipeline', :feature, :js do
       it { expect(page).not_to have_content('retried') }
 
       context 'when retrying' do
-        before { click_on 'Retry failed' }
+        before { find('.js-retry-button').trigger('click') }
 
-        it { expect(page).not_to have_content('Retry failed') }
+        it { expect(page).not_to have_content('Retry') }
         it { expect(page).to have_selector('.retried') }
       end
     end

spec/features/projects/ref_switcher_spec.rb

@@ -20,6 +20,8 @@ feature 'Ref switcher', feature: true, js: true do
       input.set 'binary'
       wait_for_ajax
 
+      expect(find('.dropdown-content ul')).to have_selector('li', count: 6)
+
       page.within '.dropdown-content ul' do
         input.native.send_keys :enter
       end

spec/features/todos/todos_spec.rb

@@ -171,6 +171,29 @@ describe 'Dashboard Todos', feature: true do
       end
     end
 
+    context 'User have large number of todos' do
+      before do
+        create_list(:todo, 101, :mentioned, user: user, project: project, target: issue, author: author)
+
+        login_as(user)
+        visit dashboard_todos_path
+      end
+
+      it 'shows 99+ for count >= 100 in notification' do
+        expect(page).to have_selector('.todos-pending-count', text: '99+')
+      end
+
+      it 'shows exact number in To do tab' do
+        expect(page).to have_selector('.todos-pending .badge', text: '101')
+      end
+
+      it 'shows exact number for count < 100' do
+        3.times { first('.js-done-todo').click }
+
+        expect(page).to have_selector('.todos-pending-count', text: '98')
+      end
+    end
+
     context 'User has a Build Failed todo' do
       let!(:todo) { create(:todo, :build_failed, user: user, project: project, author: author) }
 

spec/javascripts/header_spec.js

@@ -45,8 +45,8 @@ require('~/lib/utils/text_utility');
         expect(isTodosCountHidden()).toEqual(false);
       });
 
-      it('should add delimiter to todos-pending-count', function() {
-        expect($(todosPendingCount).text()).toEqual('1,000');
+      it('should show 99+ for todos-pending-count', function() {
+        expect($(todosPendingCount).text()).toEqual('99+');
       });
     });
   });

spec/javascripts/lib/utils/text_utility_spec.js.es6

@@ -35,5 +35,16 @@ require('~/lib/utils/text_utility');
         expect(gl.text.pluralize('test', 1)).toBe('test');
       });
     });
+
+    describe('gl.text.highCountTrim', () => {
+      it('returns 99+ for count >= 100', () => {
+        expect(gl.text.highCountTrim(105)).toBe('99+');
+        expect(gl.text.highCountTrim(100)).toBe('99+');
+      });
+
+      it('returns exact number for count < 100', () => {
+        expect(gl.text.highCountTrim(45)).toBe(45);
+      });
+    });
   });
 })();

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -101,6 +101,16 @@ describe Gitlab::Database::MigrationHelpers, lib: true do
     end
   end
 
+  describe '#concurrent_foreign_key_name' do
+    it 'returns the name for a foreign key' do
+      name = model.concurrent_foreign_key_name(:this_is_a_very_long_table_name,
+                                               :with_a_very_long_column_name)
+
+      expect(name).to be_an_instance_of(String)
+      expect(name.length).to eq(13)
+    end
+  end
+
   describe '#disable_statement_timeout' do
     context 'using PostgreSQL' do
       it 'disables statement timeouts' do

spec/requests/api/todos_spec.rb

@@ -107,46 +107,47 @@ describe API::Todos, api: true do
     end
   end
 
-  describe 'DELETE /todos/:id' do
+  describe 'POST /todos/:id/mark_as_done' do
     context 'when unauthenticated' do
       it 'returns authentication error' do
-        delete api("/todos/#{pending_1.id}")
+        post api("/todos/#{pending_1.id}/mark_as_done")
 
-        expect(response.status).to eq(401)
+        expect(response).to have_http_status(401)
       end
     end
 
     context 'when authenticated' do
       it 'marks a todo as done' do
-        delete api("/todos/#{pending_1.id}", john_doe)
+        post api("/todos/#{pending_1.id}/mark_as_done", john_doe)
 
-        expect(response.status).to eq(200)
+        expect(response).to have_http_status(201)
+        expect(json_response['id']).to eq(pending_1.id)
+        expect(json_response['state']).to eq('done')
         expect(pending_1.reload).to be_done
       end
 
       it 'updates todos cache' do
         expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original
 
-        delete api("/todos/#{pending_1.id}", john_doe)
+        post api("/todos/#{pending_1.id}/mark_as_done", john_doe)
       end
     end
   end
 
-  describe 'DELETE /todos' do
+  describe 'POST /mark_as_done' do
     context 'when unauthenticated' do
       it 'returns authentication error' do
-        delete api('/todos')
+        post api('/todos/mark_as_done')
 
-        expect(response.status).to eq(401)
+        expect(response).to have_http_status(401)
       end
     end
 
     context 'when authenticated' do
       it 'marks all todos as done' do
-        delete api('/todos', john_doe)
+        post api('/todos/mark_as_done', john_doe)
 
-        expect(response.status).to eq(200)
-        expect(response.body).to eq('3')
+        expect(response).to have_http_status(204)
         expect(pending_1.reload).to be_done
         expect(pending_2.reload).to be_done
         expect(pending_3.reload).to be_done
@@ -155,7 +156,7 @@ describe API::Todos, api: true do
       it 'updates todos cache' do
         expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original
 
-        delete api("/todos", john_doe)
+        post api("/todos/mark_as_done", john_doe)
       end
     end
   end

spec/requests/api/v3/todos_spec.rb

+require 'spec_helper'
+
+describe API::V3::Todos, api: true do
+  include ApiHelpers
+
+  let(:project_1) { create(:empty_project) }
+  let(:project_2) { create(:empty_project) }
+  let(:author_1) { create(:user) }
+  let(:author_2) { create(:user) }
+  let(:john_doe) { create(:user, username: 'john_doe') }
+  let!(:pending_1) { create(:todo, :mentioned, project: project_1, author: author_1, user: john_doe) }
+  let!(:pending_2) { create(:todo, project: project_2, author: author_2, user: john_doe) }
+  let!(:pending_3) { create(:todo, project: project_1, author: author_2, user: john_doe) }
+  let!(:done) { create(:todo, :done, project: project_1, author: author_1, user: john_doe) }
+
+  before do
+    project_1.team << [john_doe, :developer]
+    project_2.team << [john_doe, :developer]
+  end
+
+  describe 'DELETE /todos/:id' do
+    context 'when unauthenticated' do
+      it 'returns authentication error' do
+        delete v3_api("/todos/#{pending_1.id}")
+
+        expect(response.status).to eq(401)
+      end
+    end
+
+    context 'when authenticated' do
+      it 'marks a todo as done' do
+        delete v3_api("/todos/#{pending_1.id}", john_doe)
+
+        expect(response.status).to eq(200)
+        expect(pending_1.reload).to be_done
+      end
+
+      it 'updates todos cache' do
+        expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original
+
+        delete v3_api("/todos/#{pending_1.id}", john_doe)
+      end
+    end
+  end
+
+  describe 'DELETE /todos' do
+    context 'when unauthenticated' do
+      it 'returns authentication error' do
+        delete v3_api('/todos')
+
+        expect(response.status).to eq(401)
+      end
+    end
+
+    context 'when authenticated' do
+      it 'marks all todos as done' do
+        delete v3_api('/todos', john_doe)
+
+        expect(response.status).to eq(200)
+        expect(response.body).to eq('3')
+        expect(pending_1.reload).to be_done
+        expect(pending_2.reload).to be_done
+        expect(pending_3.reload).to be_done
+      end
+
+      it 'updates todos cache' do
+        expect_any_instance_of(User).to receive(:update_todos_count_cache).and_call_original
+
+        delete v3_api("/todos", john_doe)
+      end
+    end
+  end
+end

spec/services/users/destroy_spec.rb

@@ -2,11 +2,11 @@ require 'spec_helper'
 
 describe Users::DestroyService, services: true do
   describe "Deletes a user and all their personal projects" do
-    let!(:user)         { create(:user) }
-    let!(:current_user) { create(:user) }
-    let!(:namespace)    { create(:namespace, owner: user) }
-    let!(:project)      { create(:project, namespace: namespace) }
-    let(:service)       { described_class.new(current_user) }
+    let!(:user)      { create(:user) }
+    let!(:admin)     { create(:admin) }
+    let!(:namespace) { create(:namespace, owner: user) }
+    let!(:project)   { create(:project, namespace: namespace) }
+    let(:service)    { described_class.new(admin) }
 
     context 'no options are given' do
       it 'deletes the user' do
@@ -57,5 +57,26 @@ describe Users::DestroyService, services: true do
         expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
       end
     end
+
+    context "deletion permission checks" do
+      it 'does not delete the user when user is not an admin' do
+        other_user = create(:user)
+
+        expect { described_class.new(other_user).execute(user) }.to raise_error(Gitlab::Access::AccessDeniedError)
+        expect(User.exists?(user.id)).to be(true)
+      end
+
+      it 'allows admins to delete anyone' do
+        described_class.new(admin).execute(user)
+
+        expect(User.exists?(user.id)).to be(false)
+      end
+
+      it 'allows users to delete their own account' do
+        described_class.new(user).execute(user)
+
+        expect(User.exists?(user.id)).to be(false)
+      end
+    end
   end
 end