Automatic merge of gitlab-org/gitlab-ce master

app/models/notification_recipient.rb

@@ -101,6 +101,7 @@ class NotificationRecipient
   end
 
   def excluded_watcher_action?
+    return false unless @type == :watch
     return false unless @custom_action
 
     NotificationSetting::EXCLUDED_WATCHER_EVENTS.include?(@custom_action)
@@ -140,7 +141,7 @@ class NotificationRecipient
 
     return project_setting unless project_setting.nil? || project_setting.global?
 
-    group_setting = closest_non_global_group_notification_settting
+    group_setting = closest_non_global_group_notification_setting
 
     return group_setting unless group_setting.nil?
 
@@ -148,7 +149,7 @@ class NotificationRecipient
   end
 
   # Returns the notification_setting of the lowest group in hierarchy with non global level
-  def closest_non_global_group_notification_settting
+  def closest_non_global_group_notification_setting
     return unless @group
 
     @group

changelogs/archive.md

@@ -274,7 +274,7 @@
 - Pass variables from deployment project services to CI runner. !8107
 - New Gitea importer. !8116
 - Introduce "Set up autodeploy" button to help configure GitLab CI for deployment. !8135
-- Prevent enviroment table to overflow when name has underscores. !8142
+- Prevent environment table to overflow when name has underscores. !8142
 - Fix missing service error importing from EE to CE. !8144
 - Milestoneish SQL performance partially improved and memoized. !8146
 - Allow unauthenticated access to Repositories API GET endpoints. !8148
@@ -562,7 +562,7 @@
 - Fix broken issue/merge request links in JIRA comments. !6143 (Brian Kintz)
 - Fix filtering of milestones with quotes in title (airatshigapov)
 - Fix issue boards dragging bug in Safari
-- Refactor less readable existance checking code from CoffeeScript !6289 (jlogandavison)
+- Refactor less readable existence checking code from CoffeeScript !6289 (jlogandavison)
 - Update mail_room and enable sentinel support to Reply By Email (!7101)
 - Add task completion status in Issues and Merge Requests tabs: "X of Y tasks completed" (!6527, @gmesalazar)
 - Simpler arguments passed to named_route on toggle_award_url helper method
@@ -956,7 +956,7 @@
 
 ## 8.12.0 (2016-09-22)
 
-  - Removes inconsistency regarding tagging immediatelly as merged once you create a new branch. !6408
+  - Removes inconsistency regarding tagging immediately as merged once you create a new branch. !6408
   - Update the rouge gem to 2.0.6, which adds highlighting support for JSX, Prometheus, and others. !6251
   - Only check :can_resolve permission if the note is resolvable
   - Bump fog-aws to v0.11.0 to support ap-south-1 region
@@ -1389,7 +1389,7 @@
   - Fix notification_service argument error of declined invitation emails
   - Fix a memory leak caused by Banzai::Filter::SanitizationFilter
   - Speed up todos queries by limiting the projects set we join with
-  - Ensure file editing in UI does not overwrite commited changes without warning user
+  - Ensure file editing in UI does not overwrite committed changes without warning user
   - Eliminate unneeded calls to Repository#blob_at when listing commits with no path
   - Update gitlab_git gem to 10.4.7
   - Simplify SQL queries of marking a todo as done
@@ -2076,7 +2076,7 @@
   - Don't show forks button when user can't view forks
   - Fix atom feed links and rendering
   - Files over 5MB can only be viewed in their raw form, files over 1MB without highlighting !3718
-  - Add support for supressing text diffs using .gitattributes on the default branch (Matt Oakes)
+  - Add support for suppressing text diffs using .gitattributes on the default branch (Matt Oakes)
   - Add eager load paths to help prevent dependency load issues in Sidekiq workers. !3724
   - Added multiple colors for labels in dropdowns when dups happen.
   - Show commits in the same order as `git log`
@@ -2194,7 +2194,7 @@
   - All images in discussions and wikis now link to their source files !3464 (Connor Shea).
   - Return status code 303 after a branch DELETE operation to avoid project deletion (Stan Hu)
   - Add setting for customizing the list of trusted proxies !3524
-  - Allow projects to be transfered to a lower visibility level group
+  - Allow projects to be transferred to a lower visibility level group
   - Fix `signed_in_ip` being set to 127.0.0.1 when using a reverse proxy !3524
   - Improved Markdown rendering performance !3389
   - Make shared runners text in box configurable
@@ -2209,7 +2209,7 @@
   - API: Ability to update a group (Robert Schilling)
   - API: Ability to move issues (Robert Schilling)
   - Fix Error 500 after renaming a project path (Stan Hu)
-  - Fix a bug whith trailing slash in teamcity_url (Charles May)
+  - Fix a bug with trailing slash in teamcity_url (Charles May)
   - Allow back dating on issues when created or updated through the API
   - Allow back dating on issue notes when created through the API
   - Propose license template when creating a new LICENSE file
@@ -2219,7 +2219,7 @@
   - Allow SAML to handle external users based on user's information !3530
   - Allow Omniauth providers to be marked as `external` !3657
   - Add endpoints to archive or unarchive a project !3372
-  - Fix a bug whith trailing slash in bamboo_url
+  - Fix a bug with trailing slash in bamboo_url
   - Add links to CI setup documentation from project settings and builds pages
   - Display project members page to all members
   - Handle nil descriptions in Slack issue messages (Stan Hu)
@@ -2610,7 +2610,7 @@
   - Fixed logo animation on Safari (Roman Rott)
   - Fix Merge When Succeeded when multiple stages
   - Hide remove source branch button when the MR is merged but new commits are pushed (Zeger-Jan van de Weg)
-  - In seach autocomplete show only groups and projects you are member of
+  - In search autocomplete show only groups and projects you are member of
   - Don't process cross-reference notes from forks
   - Fix: init.d script not working on OS X
   - Faster snippet search
@@ -2870,7 +2870,7 @@
   - Bump Redis requirement to 2.8 for Sidekiq 4 (Stan Hu)
   - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera)
   - WIP identifier on merge requests no longer requires trailing space
-  - Add rake tasks for git repository maintainance (Zeger-Jan van de Weg)
+  - Add rake tasks for git repository maintenance (Zeger-Jan van de Weg)
   - Fix 500 error when update group member permission
   - Fix: As an admin, cannot add oneself as a member to a group/project
   - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera)
@@ -3199,7 +3199,7 @@
   - Sort issues by creation date in Bitbucket importer (Stan Hu)
   - Prevent too many redirects upon login when home page URL is set to external_url (Stan Hu)
   - Improve dropdown positioning on the project home page (Hannes Rosenögger)
-  - Upgrade browser gem to 1.0.0 to avoid warning in IE11 compatibilty mode (Stan Hu)
+  - Upgrade browser gem to 1.0.0 to avoid warning in IE11 compatibility mode (Stan Hu)
   - Remove user OAuth tokens from the database and request new tokens each session (Stan Hu)
   - Restrict users API endpoints to use integer IDs (Stan Hu)
   - Only show recent push event if the branch still exists or a recent merge request has not been created (Stan Hu)
@@ -3209,7 +3209,7 @@
   - Ability to fetch merge requests from refs/merge-requests/:id
   - Allow displaying of archived projects in the admin interface (Artem Sidorenko)
   - Allow configuration of import sources for new projects (Artem Sidorenko)
-  - Search for comments should be case insensetive
+  - Search for comments should be case insensitive
   - Create cross-reference for closing references on commits pushed to non-default branches (Maël Valais)
   - Ability to search milestones
   - Gracefully handle SMTP user input errors (e.g. incorrect email addresses) to prevent Sidekiq retries (Stan Hu)
@@ -3477,7 +3477,7 @@
 - Update Asciidoctor gem to version 1.5.2. (Jakub Jirutka)
 - Fix resolving of relative links to repository files in AsciiDoc documents. (Jakub Jirutka)
 - Use the user list from the target project in a merge request (Stan Hu)
-- Default extention for wiki pages is now .md instead of .markdown (Jeroen van Baarsen)
+- Default extension for wiki pages is now .md instead of .markdown (Jeroen van Baarsen)
 - Add validation to wiki page creation (only [a-zA-Z0-9/_-] are allowed) (Jeroen van Baarsen)
 - Fix new/empty milestones showing 100% completion value (Jonah Bishop)
 - Add a note when an Issue or Merge Request's title changes
@@ -3613,7 +3613,7 @@
 - Set EmailsOnPush reply-to address to committer email when enabled.
 - Fix broken file browsing with a submodule that contains a relative link (Stan Hu)
 - Fix persistent XSS vulnerability around profile website URLs.
-- Fix project import URL regex to prevent arbitary local repos from being imported.
+- Fix project import URL regex to prevent arbitrary local repos from being imported.
 - Fix directory traversal vulnerability around uploads routes.
 - Fix directory traversal vulnerability around help pages.
 - Don't leak existence of project via search autocomplete.
@@ -3676,7 +3676,7 @@
 - Don't show commit comment button when user is not signed in.
 - Fix admin user projects lists.
 - Don't leak private group existence by redirecting from namespace controller to group controller.
-- Ability to skip some items from backup (database, respositories or uploads)
+- Ability to skip some items from backup (database, repositories or uploads)
 - Archive repositories in background worker.
 - Import GitHub, Bitbucket or GitLab.com projects owned by authenticated user into current namespace.
 - Project labels are now available over the API under the "tag_list" field (Cristian Medina)
@@ -3701,7 +3701,7 @@
 
 ## 7.9.4
 
-- Security: Fix project import URL regex to prevent arbitary local repos from being imported
+- Security: Fix project import URL regex to prevent arbitrary local repos from being imported
 - Fixed issue where only 25 commits would load in file listings
 - Fix LDAP identities  after config update
 
@@ -4199,7 +4199,7 @@
 - Only masters can rewrite/remove git tags
 - Add X-Frame-Options SAMEORIGIN to Nginx config so Sidekiq admin is visible
 - UI improvements
-- Case-insensetive search for issues
+- Case-insensitive search for issues
 - Update to rails 4.1
 - Improve performance of application for projects and groups with a lot of members
 - Formally support Ruby 2.1
@@ -4358,7 +4358,7 @@
 - Group avatar
 - Pygments.rb replaced with highlight.js
 - Improve Merge request diff store logic
-- Improve render performnace for MR show page
+- Improve render performances for MR show page
 - Fixed Assembla hardcoded project name
 - Jira integration documentation
 - Refactored app/services

changelogs/unreleased/58433-email-notifications-do-not-work-properly-issue-due-date.yml

+---
+title: Fix email notifications for user excluded actions
+merge_request: 28835
+author:
+type: fixed

config/initializers/rack_timeout.rb

@@ -18,6 +18,6 @@ if defined?(::Puma) && !Rails.env.test?
                                     wait_timeout: 90)
   end
 
-  observer = Gitlab::RackTimeoutObserver.new
+  observer = Gitlab::Cluster::RackTimeoutObserver.new
   Rack::Timeout.register_state_change_observer(:gitlab_rack_timeout, &observer.callback)
 end

doc/development/gitaly.md

@@ -245,7 +245,7 @@ Here are the steps to gate a new feature in Gitaly behind a feature flag.
      // go implementation
    } else {
    	findAllTagsRequests.WithLabelValues("ruby").Inc()
-     // ruby impelmentation
+     // ruby implementation
    }
    ```
 

doc/development/testing_guide/best_practices.md

@@ -17,7 +17,7 @@ our test design. We can find some helpful heuristics documented in the Handbook
 
 ## Run tests against MySQL
 
-By default, tests are only run againts PostgreSQL, but you can run them on
+By default, tests are only run against PostgreSQL, but you can run them on
 demand against MySQL by following one of the following conventions:
 
 | Convention           | Valid example                |

doc/development/testing_guide/end_to_end/quick_start_guide.md

@@ -557,7 +557,7 @@ The code should look like this: `filter_output = search_field_tag search_id, nil
 
 > Classes starting with `qa-` are used for testing purposes only, and by defining such classes in the elements we add **testability** in the application.
 
-> When defining a class like `qa-labels-block`, it is transformed into `:labels_block` for usage in the Page Objects. So, `qa-edit-link-labels` is tranformed into `:edit_link_labels`, `qa-dropdown-menu-labels` is transformed into `:dropdown_menu_labels`, and `qa-dropdown-input-field` is transformed into `:dropdown_input_field`. Also, we use a [sanity test](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/page#how-did-we-solve-fragile-tests-problem) to check that defined elements have their respective `qa-` selectors in the specified views.
+> When defining a class like `qa-labels-block`, it is transformed into `:labels_block` for usage in the Page Objects. So, `qa-edit-link-labels` is transformed into `:edit_link_labels`, `qa-dropdown-menu-labels` is transformed into `:dropdown_menu_labels`, and `qa-dropdown-input-field` is transformed into `:dropdown_input_field`. Also, we use a [sanity test](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/qa/qa/page#how-did-we-solve-fragile-tests-problem) to check that defined elements have their respective `qa-` selectors in the specified views.
 
 > We did not define the `qa-labels-block` class in the `app/views/shared/issuable/_sidebar.html.haml` file because it was already there to be used.
 

doc/gitlab-basics/README.md

@@ -24,6 +24,7 @@ The following are guides to basic GitLab functionality:
 - [Add an image](add-image.md), to add new images to a project's repository.
 - [Create an issue](../user/project/issues/create_new_issue.md), to start collaborating within a project.
 - [Create a merge request](add-merge-request.md), to request changes made in a branch be merged into a project's repository.
+- See how these features come together in the [GitLab Flow introduction video](https://youtu.be/InKNIvky2KE) and [GitLab Flow page](../workflow/gitlab_flow.md).
 
 ## Git basics
 

doc/user/group/index.md

@@ -232,8 +232,8 @@ to explore data such as:
 From GitLab 10.5, you can transfer groups in the following ways:
 
 - Transfer a subgroup to a new parent group.
-- Convert a top-level group into a subgroup by transfering it to the desired group.
-- Convert a subgroup into a top-level group by transfering it out of its current group.
+- Convert a top-level group into a subgroup by transferring it to the desired group.
+- Convert a subgroup into a top-level group by transferring it out of its current group.
 
 When transferring groups, note:
 

doc/workflow/gitlab_flow.md

@@ -27,6 +27,8 @@ People have a hard time figuring out which branch has the latest code, or which
 Frequently, the reaction to this problem is to adopt a standardized pattern such as [Git flow](https://nvie.com/posts/a-successful-git-branching-model/) and [GitHub flow](http://scottchacon.com/2011/08/31/github-flow.html).
 We think there is still room for improvement. In this document, we describe a set of practices we call GitLab flow.
 
+For a video introduction of how this works in GitLab, see [GitLab Flow](https://youtu.be/InKNIvky2KE).
+
 ## Git flow and its problems
 
 ![Git Flow timeline by Vincent Driessen, used with permission](gitdashflow.png)

lib/gitlab/cluster/rack_timeout_observer.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Cluster
+    class RackTimeoutObserver
+      TRANSITION_STATES = %i(ready active).freeze
+
+      def initialize
+        @counter = Gitlab::Metrics.counter(:rack_requests_total, 'Number of requests in a given rack state')
+      end
+
+      # returns the Proc to be used as the observer callback block
+      def callback
+        method(:log_timeout_exception)
+      end
+
+      private
+
+      def log_timeout_exception(env)
+        info = env[::Rack::Timeout::ENV_INFO_KEY]
+        return unless info
+        return if TRANSITION_STATES.include?(info.state)
+
+        @counter.increment(labels(info, env))
+      end
+
+      def labels(info, env)
+        params = controller_params(env) || grape_params(env) || {}
+
+        {
+          controller: params['controller'],
+          action: params['action'],
+          route: params['route'],
+          state: info.state
+        }
+      end
+
+      def controller_params(env)
+        env['action_dispatch.request.parameters']
+      end
+
+      def grape_params(env)
+        endpoint = env[Grape::Env::API_ENDPOINT]
+        route = endpoint&.route&.pattern&.origin
+        return unless route
+
+        { 'route' => route }
+      end
+    end
+  end
+end

lib/gitlab/rack_timeout_observer.rb

-# frozen_string_literal: true
-
-module Gitlab
-  class RackTimeoutObserver
-    def initialize
-      @counter = Gitlab::Metrics.counter(:rack_state_total, 'Number of requests in a given rack state')
-    end
-
-    # returns the Proc to be used as the observer callback block
-    def callback
-      method(:log_timeout_exception)
-    end
-
-    private
-
-    def log_timeout_exception(env)
-      info = env[::Rack::Timeout::ENV_INFO_KEY]
-      return unless info
-
-      @counter.increment(labels(info, env))
-    end
-
-    def labels(info, env)
-      params = controller_params(env) || grape_params(env) || {}
-
-      {
-        controller: params['controller'],
-        action: params['action'],
-        route: params['route'],
-        state: info.state
-      }
-    end
-
-    def controller_params(env)
-      env['action_dispatch.request.parameters']
-    end
-
-    def grape_params(env)
-      endpoint = env[Grape::Env::API_ENDPOINT]
-      route = endpoint&.route&.pattern&.origin
-      return unless route
-
-      { 'route' => route }
-    end
-  end
-end

spec/lib/gitlab/rack_timeout_observer_spec.rb → spec/lib/gitlab/cluster/rack_timeout_observer_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-describe Gitlab::RackTimeoutObserver do
+describe Gitlab::Cluster::RackTimeoutObserver do
   let(:counter) { Gitlab::Metrics::NullMetric.instance }
 
   before do
@@ -25,7 +25,7 @@ describe Gitlab::RackTimeoutObserver do
 
       subject { described_class.new }
 
-      it 'increments timeout counter' do
+      it 'increments counter' do
         expect(counter)
           .to receive(:increment)
           .with({ controller: 'foo', action: 'bar', route: nil, state: :timed_out })
@@ -45,7 +45,7 @@ describe Gitlab::RackTimeoutObserver do
 
       subject { described_class.new }
 
-      it 'increments timeout counter' do
+      it 'increments counter' do
         allow(endpoint).to receive_message_chain('route.pattern.origin') { 'foobar' }
         expect(counter)
           .to receive(:increment)
@@ -54,5 +54,24 @@ describe Gitlab::RackTimeoutObserver do
         subject.callback.call(env)
       end
     end
+
+    context 'when request is being processed' do
+      let(:endpoint) { double }
+      let(:env) do
+        {
+          ::Rack::Timeout::ENV_INFO_KEY => double(state: :active),
+          Grape::Env::API_ENDPOINT => endpoint
+        }
+      end
+
+      subject { described_class.new }
+
+      it 'does not increment counter' do
+        allow(endpoint).to receive_message_chain('route.pattern.origin') { 'foobar' }
+        expect(counter).not_to receive(:increment)
+
+        subject.callback.call(env)
+      end
+    end
   end
 end

spec/models/notification_recipient_spec.rb

@@ -91,4 +91,237 @@ describe NotificationRecipient do
       end
     end
   end
+
+  describe '#suitable_notification_level?' do
+    context 'when notification level is mention' do
+      before do
+        user.notification_settings_for(project).mention!
+      end
+
+      context 'when type is mention' do
+        let(:recipient) { described_class.new(user, :mention, target: target, project: project) }
+
+        it 'returns true' do
+          expect(recipient.suitable_notification_level?).to eq true
+        end
+      end
+
+      context 'when type is not mention' do
+        it 'returns false' do
+          expect(recipient.suitable_notification_level?).to eq false
+        end
+      end
+    end
+
+    context 'when notification level is participating' do
+      let(:notification_setting) { user.notification_settings_for(project) }
+
+      context 'when type is participating' do
+        let(:recipient) { described_class.new(user, :participating, target: target, project: project) }
+
+        it 'returns true' do
+          expect(recipient.suitable_notification_level?).to eq true
+        end
+      end
+
+      context 'when type is mention' do
+        let(:recipient) { described_class.new(user, :mention, target: target, project: project) }
+
+        it 'returns true' do
+          expect(recipient.suitable_notification_level?).to eq true
+        end
+      end
+
+      context 'with custom action' do
+        context "when action is failed_pipeline" do
+          let(:recipient) do
+            described_class.new(
+              user,
+              :watch,
+              custom_action: :failed_pipeline,
+              target: target,
+              project: project
+            )
+          end
+
+          before do
+            notification_setting.update!(failed_pipeline: true)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context "when action is not failed_pipeline" do
+          let(:recipient) do
+            described_class.new(
+              user,
+              :watch,
+              custom_action: :success_pipeline,
+              target: target,
+              project: project
+            )
+          end
+
+          before do
+            notification_setting.update!(success_pipeline: true)
+          end
+
+          it 'returns false' do
+            expect(recipient.suitable_notification_level?).to eq false
+          end
+        end
+      end
+    end
+
+    context 'when notification level is custom' do
+      before do
+        user.notification_settings_for(project).custom!
+      end
+
+      context 'when type is participating' do
+        let(:notification_setting) { user.notification_settings_for(project) }
+        let(:recipient) do
+          described_class.new(
+            user,
+            :participating,
+            custom_action: :new_note,
+            target: target,
+            project: project
+          )
+        end
+
+        context 'with custom event enabled' do
+          before do
+            notification_setting.update!(new_note: true)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context 'without custom event enabled' do
+          before do
+            notification_setting.update!(new_note: false)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+      end
+
+      context 'when type is mention' do
+        let(:notification_setting) { user.notification_settings_for(project) }
+        let(:recipient) do
+          described_class.new(
+            user,
+            :mention,
+            custom_action: :new_issue,
+            target: target,
+            project: project
+          )
+        end
+
+        context 'with custom event enabled' do
+          before do
+            notification_setting.update!(new_issue: true)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context 'without custom event enabled' do
+          before do
+            notification_setting.update!(new_issue: false)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+      end
+
+      context 'when type is watch' do
+        let(:notification_setting) { user.notification_settings_for(project) }
+        let(:recipient) do
+          described_class.new(
+            user,
+            :watch,
+            custom_action: :failed_pipeline,
+            target: target,
+            project: project
+          )
+        end
+
+        context 'with custom event enabled' do
+          before do
+            notification_setting.update!(failed_pipeline: true)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context 'without custom event enabled' do
+          before do
+            notification_setting.update!(failed_pipeline: false)
+          end
+
+          it 'returns false' do
+            expect(recipient.suitable_notification_level?).to eq false
+          end
+        end
+      end
+    end
+
+    context 'when notification level is watch' do
+      before do
+        user.notification_settings_for(project).watch!
+      end
+
+      context 'when type is watch' do
+        context 'without excluded watcher events' do
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context 'with excluded watcher events' do
+          let(:recipient) do
+            described_class.new(user, :watch, custom_action: :issue_due, target: target, project: project)
+          end
+
+          it 'returns false' do
+            expect(recipient.suitable_notification_level?).to eq false
+          end
+        end
+      end
+
+      context 'when type is not watch' do
+        context 'without excluded watcher events' do
+          let(:recipient) { described_class.new(user, :participating, target: target, project: project) }
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+
+        context 'with excluded watcher events' do
+          let(:recipient) do
+            described_class.new(user, :participating, custom_action: :issue_due, target: target, project: project)
+          end
+
+          it 'returns true' do
+            expect(recipient.suitable_notification_level?).to eq true
+          end
+        end
+      end
+    end
+  end
 end