Merge remote-tracking branch 'upstream/master' into pipeline-emails

* upstream/master: (372 commits)
  Enable Lint/StringConversionInInterpolation cop and autocorrect offenses
  resolve duplicated changelog entry
  credit myself :smile:
  change determine conditions
  override subject method in devise mailer
  follow the styleguide: Don't use parentheses around a literal
  wrap subject with method subject
  move spec back into shared example `an email sent from GitLab`
  stub config settings in spec
  remove empty line at block body end
  remove extra entry
  create new test in `spec/mailers/notify_spec.rb`
  move changelog to 8.13
  add configurable email subject suffix
  Fixes sidebar navigation.
  Convert "SSH Keys" Spinach features to RSpec
  Enable import/export back for non-admins
  Update gitlab-shell to 3.6.3
  Updated artwork of empty group state.
  Better empty state for Groups view.
  ...

26 files changed, 303 insertions, 167 deletions

diff --git a/app/models/board.rb b/app/models/board.rb
index 3240c4bede3..c56422914a9 100644
--- a/app/models/board.rb
+++ b/app/models/board.rb
@@ -4,4 +4,12 @@ class Board < ActiveRecord::Base
   has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all
 
   validates :project, presence: true
+
+  def backlog_list
+    lists.merge(List.backlog).take
+  end
+
+  def done_list
+    lists.merge(List.done).take
+  end
 end
diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 416ea820e0c..2911dcaa6a2 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -91,7 +91,7 @@ module Ci
             sha: build.sha,
             ref: build.ref,
             tag: build.tag,
-            options: build.options[:environment],
+            options: build.options.to_h[:environment],
             variables: build.variables)
           service.execute(build)
         end
@@ -378,7 +378,7 @@ module Ci
     end
 
     def artifacts?
-      !artifacts_expired? && self[:artifacts_file].present?
+      !artifacts_expired? && artifacts_file.exists?
     end
 
     def artifacts_metadata?
@@ -498,8 +498,11 @@ module Ci
     end
 
     def hide_secrets(trace)
-      trace = Ci::MaskSecret.mask(trace, project.runners_token) if project
-      trace = Ci::MaskSecret.mask(trace, token)
+      return unless trace
+
+      trace = trace.dup
+      Ci::MaskSecret.mask!(trace, project.runners_token) if project
+      Ci::MaskSecret.mask!(trace, token)
       trace
     end
   end
diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb
index 656a242c265..ac2477fd973 100644
--- a/app/models/commit_range.rb
+++ b/app/models/commit_range.rb
@@ -80,7 +80,7 @@ class CommitRange
   end
 
   def inspect
-    %(#<#{self.class}:#{object_id} #{to_s}>)
+    %(#<#{self.class}:#{object_id} #{self}>)
   end
 
   def to_s
diff --git a/app/models/concerns/access_requestable.rb b/app/models/concerns/access_requestable.rb
index eedd32a729f..62bc6b809f4 100644
--- a/app/models/concerns/access_requestable.rb
+++ b/app/models/concerns/access_requestable.rb
@@ -8,9 +8,6 @@ module AccessRequestable
   extend ActiveSupport::Concern
 
   def request_access(user)
-    members.create(
-      access_level: Gitlab::Access::DEVELOPER,
-      user: user,
-      requested_at: Time.now.utc)
+    Members::RequestAccessService.new(self, user).execute
   end
 end
diff --git a/app/models/concerns/awardable.rb b/app/models/concerns/awardable.rb
index d8d4575bb4d..073ac4c1b65 100644
--- a/app/models/concerns/awardable.rb
+++ b/app/models/concerns/awardable.rb
@@ -71,6 +71,12 @@ module Awardable
     end
   end
 
+  def user_authored?(current_user)
+    author = self.respond_to?(:author) ? self.author : self.user
+
+    author == current_user
+  end
+
   def awarded_emoji?(emoji_name, current_user)
     award_emoji.where(name: emoji_name, user: current_user).exists?
   end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 1650ac9fcbe..ff465d2c745 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -200,10 +200,6 @@ module Issuable
     end
   end
 
-  def user_authored?(user)
-    user == author
-  end
-
   def subscribed_without_subscriptions?(user)
     participants(user).include?(user)
   end
diff --git a/app/models/cycle_analytics/summary.rb b/app/models/cycle_analytics/summary.rb
index 53b2cacb131..b46db449bf3 100644
--- a/app/models/cycle_analytics/summary.rb
+++ b/app/models/cycle_analytics/summary.rb
@@ -10,15 +10,33 @@ class CycleAnalytics
     end
 
     def commits
-      repository = @project.repository.raw_repository
-
-      if @project.default_branch
-        repository.log(ref: @project.default_branch, after: @from).count
-      end
+      ref = @project.default_branch.presence
+      count_commits_for(ref)
     end
 
     def deploys
       @project.deployments.where("created_at > ?", @from).count
     end
+
+    private
+
+    # Don't use the `Gitlab::Git::Repository#log` method, because it enforces
+    # a limit. Since we need a commit count, we _can't_ enforce a limit, so
+    # the easiest way forward is to replicate the relevant portions of the
+    # `log` function here.
+    def count_commits_for(ref)
+      return unless ref
+
+      repository = @project.repository.raw_repository
+      sha = @project.repository.commit(ref).sha
+
+      cmd = %W(git --git-dir=#{repository.path} log)
+      cmd << '--format=%H'
+      cmd << "--after=#{@from.iso8601}"
+      cmd << sha
+
+      raw_output = IO.popen(cmd) { |io| io.read }
+      raw_output.lines.count
+    end
   end
 end
diff --git a/app/models/global_milestone.rb b/app/models/global_milestone.rb
index da7c265a371..bda2b5c5d5d 100644
--- a/app/models/global_milestone.rb
+++ b/app/models/global_milestone.rb
@@ -8,7 +8,8 @@ class GlobalMilestone
     milestones = milestones.group_by(&:title)
 
     milestones.map do |title, milestones|
-      new(title, milestones)
+      milestones_relation = Milestone.where(id: milestones.map(&:id))
+      new(title, milestones_relation)
     end
   end
 
@@ -31,7 +32,7 @@ class GlobalMilestone
   end
 
   def projects
-    @projects ||= Project.for_milestones(milestones.map(&:id))
+    @projects ||= Project.for_milestones(milestones.select(:id))
   end
 
   def state
@@ -53,19 +54,19 @@ class GlobalMilestone
   end
 
   def issues
-    @issues ||= Issue.of_milestones(milestones.map(&:id)).includes(:project)
+    @issues ||= Issue.of_milestones(milestones.select(:id)).includes(:project, :assignee, :labels)
   end
 
   def merge_requests
-    @merge_requests ||= MergeRequest.of_milestones(milestones.map(&:id)).includes(:target_project)
+    @merge_requests ||= MergeRequest.of_milestones(milestones.select(:id)).includes(:target_project, :assignee, :labels)
   end
 
   def participants
-    @participants ||= milestones.map(&:participants).flatten.compact.uniq
+    @participants ||= milestones.includes(:participants).map(&:participants).flatten.compact.uniq
   end
 
   def labels
-    @labels ||= GlobalLabel.build_collection(milestones.map(&:labels).flatten)
+    @labels ||= GlobalLabel.build_collection(milestones.includes(:labels).map(&:labels).flatten)
                            .sort_by!(&:title)
   end
 
diff --git a/app/models/group.rb b/app/models/group.rb
index aefb94b2ada..a2f88cca828 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -102,40 +102,44 @@ class Group < Namespace
     self[:lfs_enabled]
   end
 
-  def add_users(user_ids, access_level, current_user: nil, expires_at: nil)
-    user_ids.each do |user_id|
-      Member.add_user(
-        self.group_members,
-        user_id,
-        access_level,
-        current_user: current_user,
-        expires_at: expires_at
-      )
-    end
+  def add_users(users, access_level, current_user: nil, expires_at: nil)
+    GroupMember.add_users_to_group(
+      self,
+      users,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at
+    )
   end
 
   def add_user(user, access_level, current_user: nil, expires_at: nil)
-    add_users([user], access_level, current_user: current_user, expires_at: expires_at)
+    GroupMember.add_user(
+      self,
+      user,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at
+    )
   end
 
   def add_guest(user, current_user = nil)
-    add_user(user, Gitlab::Access::GUEST, current_user: current_user)
+    add_user(user, :guest, current_user: current_user)
   end
 
   def add_reporter(user, current_user = nil)
-    add_user(user, Gitlab::Access::REPORTER, current_user: current_user)
+    add_user(user, :reporter, current_user: current_user)
   end
 
   def add_developer(user, current_user = nil)
-    add_user(user, Gitlab::Access::DEVELOPER, current_user: current_user)
+    add_user(user, :developer, current_user: current_user)
   end
 
   def add_master(user, current_user = nil)
-    add_user(user, Gitlab::Access::MASTER, current_user: current_user)
+    add_user(user, :master, current_user: current_user)
   end
 
   def add_owner(user, current_user = nil)
-    add_user(user, Gitlab::Access::OWNER, current_user: current_user)
+    add_user(user, :owner, current_user: current_user)
   end
 
   def has_owner?(user)
diff --git a/app/models/member.rb b/app/models/member.rb
index 69406379948..38a278ea559 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -80,49 +80,70 @@ class Member < ActiveRecord::Base
       find_by(invite_token: invite_token)
     end
 
-    # This method is used to find users that have been entered into the "Add members" field.
-    # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
-    def user_for_id(user_id)
-      return user_id if user_id.is_a?(User)
-
-      user = User.find_by(id: user_id)
-      user ||= User.find_by(email: user_id)
-      user ||= user_id
-      user
-    end
-
-    def add_user(members, user_id, access_level, current_user: nil, expires_at: nil)
-      user = user_for_id(user_id)
+    def add_user(source, user, access_level, current_user: nil, expires_at: nil)
+      user = retrieve_user(user)
+      access_level = retrieve_access_level(access_level)
 
       # `user` can be either a User object or an email to be invited
-      if user.is_a?(User)
-        member = members.find_or_initialize_by(user_id: user.id)
+      member =
+        if user.is_a?(User)
+          source.members.find_by(user_id: user.id) ||
+          source.requesters.find_by(user_id: user.id) ||
+          source.members.build(user_id: user.id)
+        else
+          source.members.build(invite_email: user)
+        end
+
+      return member unless can_update_member?(current_user, member)
+
+      member.attributes = {
+        created_by: member.created_by || current_user,
+        access_level: access_level,
+        expires_at: expires_at
+      }
+
+      if member.request?
+        ::Members::ApproveAccessRequestService.new(source, current_user, id: member.id).execute
       else
-        member = members.build
-        member.invite_email = user
+        member.save
       end
 
-      if can_update_member?(current_user, member) || project_creator?(member, access_level)
-        member.created_by ||= current_user
-        member.access_level = access_level
-        member.expires_at = expires_at
+      member
+    end
 
-        member.save
-      end
+    def access_levels
+      Gitlab::Access.sym_options
     end
 
     private
 
+    # This method is used to find users that have been entered into the "Add members" field.
+    # These can be the User objects directly, their IDs, their emails, or new emails to be invited.
+    def retrieve_user(user)
+      return user if user.is_a?(User)
+
+      User.find_by(id: user) || User.find_by(email: user) || user
+    end
+
+    def retrieve_access_level(access_level)
+      access_levels.fetch(access_level) { access_level.to_i }
+    end
+
     def can_update_member?(current_user, member)
       # There is no current user for bulk actions, in which case anything is allowed
-      !current_user ||
-        current_user.can?(:update_group_member, member) ||
-        current_user.can?(:update_project_member, member)
+      !current_user || current_user.can?(:"update_#{member.type.underscore}", member)
     end
 
-    def project_creator?(member, access_level)
-      member.new_record? && member.owner? &&
-        access_level.to_i == ProjectMember::MASTER
+    def add_users_to_source(source, users, access_level, current_user: nil, expires_at: nil)
+      users.each do |user|
+        add_user(
+          source,
+          user,
+          access_level,
+          current_user: current_user,
+          expires_at: expires_at
+        )
+      end
     end
   end
 
diff --git a/app/models/members/group_member.rb b/app/models/members/group_member.rb
index 2f13d339c89..1b54a85d064 100644
--- a/app/models/members/group_member.rb
+++ b/app/models/members/group_member.rb
@@ -12,6 +12,22 @@ class GroupMember < Member
     Gitlab::Access.options_with_owner
   end
 
+  def self.access_levels
+    Gitlab::Access.sym_options_with_owner
+  end
+
+  def self.add_users_to_group(group, users, access_level, current_user: nil, expires_at: nil)
+    self.transaction do
+      add_users_to_source(
+        group,
+        users,
+        access_level,
+        current_user: current_user,
+        expires_at: expires_at
+      )
+    end
+  end
+
   def group
     source
   end
diff --git a/app/models/members/project_member.rb b/app/models/members/project_member.rb
index ec2d40eb11c..125f26369d7 100644
--- a/app/models/members/project_member.rb
+++ b/app/models/members/project_member.rb
@@ -34,36 +34,20 @@ class ProjectMember < Member
     #     :master
     #   )
     #
-    def add_users_to_projects(project_ids, user_ids, access, current_user: nil, expires_at: nil)
-      access_level = if roles_hash.has_key?(access)
-                       roles_hash[access]
-                     elsif roles_hash.values.include?(access.to_i)
-                       access
-                     else
-                       raise "Non valid access"
-                     end
-
-      users = user_ids.map { |user_id| Member.user_for_id(user_id) }
-
-      ProjectMember.transaction do
+    def add_users_to_projects(project_ids, users, access_level, current_user: nil, expires_at: nil)
+      self.transaction do
         project_ids.each do |project_id|
           project = Project.find(project_id)
 
-          users.each do |user|
-            Member.add_user(
-              project.project_members,
-              user,
-              access_level,
-              current_user: current_user,
-              expires_at: expires_at
-            )
-          end
+          add_users_to_source(
+            project,
+            users,
+            access_level,
+            current_user: current_user,
+            expires_at: expires_at
+          )
         end
       end
-
-      true
-    rescue
-      false
     end
 
     def truncate_teams(project_ids)
@@ -84,13 +68,15 @@ class ProjectMember < Member
       truncate_teams [project.id]
     end
 
-    def roles_hash
-      Gitlab::Access.sym_options
-    end
-
     def access_level_roles
       Gitlab::Access.options
     end
+
+    private
+
+    def can_update_member?(current_user, member)
+      super || (member.owner? && member.new_record?)
+    end
   end
 
   def access_field
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 616efaf3c42..a431d46cc9e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -155,6 +155,20 @@ class MergeRequest < ActiveRecord::Base
     where("merge_requests.id IN (#{union.to_sql})")
   end
 
+  WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
+
+  def self.work_in_progress?(title)
+    !!(title =~ WIP_REGEX)
+  end
+
+  def self.wipless_title(title)
+    title.sub(WIP_REGEX, "")
+  end
+
+  def self.wip_title(title)
+    work_in_progress?(title) ? title : "WIP: #{title}"
+  end
+
   def to_reference(from_project = nil)
     reference = "#{self.class.reference_prefix}#{iid}"
 
@@ -389,14 +403,16 @@ class MergeRequest < ActiveRecord::Base
     @closed_event ||= target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
   end
 
-  WIP_REGEX = /\A\s*(\[WIP\]\s*|WIP:\s*|WIP\s+)+\s*/i.freeze
-
   def work_in_progress?
-    !!(title =~ WIP_REGEX)
+    self.class.work_in_progress?(title)
   end
 
   def wipless_title
-    self.title.sub(WIP_REGEX, "")
+    self.class.wipless_title(self.title)
+  end
+
+  def wip_title
+    self.class.wip_title(self.title)
   end
 
   def mergeable?(skip_ci_check: false)
@@ -590,13 +606,11 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def merge_commit_message
-    message = "Merge branch '#{source_branch}' into '#{target_branch}'"
-    message << "\n\n"
-    message << title.to_s
-    message << "\n\n"
-    message << description.to_s
-    message << "\n\n"
-    message << "See merge request !#{iid}"
+    message = "Merge branch '#{source_branch}' into '#{target_branch}'\n\n"
+    message << "#{title}\n\n"
+    message << "#{description}\n\n" if description.present?
+    message << "See merge request #{to_reference}"
+
     message
   end
 
@@ -670,9 +684,12 @@ class MergeRequest < ActiveRecord::Base
   def environments
     return [] unless diff_head_commit
 
-    target_project.environments.select do |environment|
-      environment.includes_commit?(diff_head_commit)
-    end
+    environments = source_project.environments_for(
+      source_branch, diff_head_commit)
+    environments += target_project.environments_for(
+      target_branch, diff_head_commit, with_tags: true)
+
+    environments.uniq
   end
 
   def state_human_name
@@ -761,10 +778,23 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def all_pipelines
-    @all_pipelines ||=
-      if diff_head_sha && source_project
-        source_project.pipelines.order(id: :desc).where(sha: commits_sha, ref: source_branch)
-      end
+    return unless source_project
+
+    @all_pipelines ||= begin
+      sha = if persisted?
+              all_commits_sha
+            else
+              diff_head_sha
+            end
+
+      source_project.pipelines.order(id: :desc).
+        where(sha: sha, ref: source_branch)
+    end
+  end
+
+  # Note that this could also return SHA from now dangling commits
+  def all_commits_sha
+    merge_request_diffs.flat_map(&:commits_sha).uniq
   end
 
   def merge_commit
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 18c583add88..36b8b70870b 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -30,6 +30,10 @@ class MergeRequestDiff < ActiveRecord::Base
     select(column_names - ['st_diffs'])
   end
 
+  def st_commits
+    super || []
+  end
+
   # Collect information about commits and diff from repository
   # and save it to the database as serialized data
   def save_git_content
@@ -83,7 +87,7 @@ class MergeRequestDiff < ActiveRecord::Base
   end
 
   def commits
-    @commits ||= load_commits(st_commits || [])
+    @commits ||= load_commits(st_commits)
   end
 
   def reload_commits
@@ -117,6 +121,14 @@ class MergeRequestDiff < ActiveRecord::Base
     project.commit(head_commit_sha)
   end
 
+  def commits_sha
+    if @commits
+      commits.map(&:sha)
+    else
+      st_commits.map { |commit| commit[:id] }
+    end
+  end
+
   def diff_refs
     return unless start_commit_sha || base_commit_sha
 
diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 2bd7f198030..44c3cbb2c73 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -158,7 +158,7 @@ class Milestone < ActiveRecord::Base
   end
 
   def title=(value)
-    write_attribute(:title, Sanitize.clean(value.to_s)) if value.present?
+    write_attribute(:title, sanitize_title(value)) if value.present?
   end
 
   # Sorts the issues for the given IDs.
@@ -204,4 +204,8 @@ class Milestone < ActiveRecord::Base
       iid
     end
   end
+
+  def sanitize_title(value)
+    CGI.unescape_html(Sanitize.clean(value.to_s))
+  end
 end
diff --git a/app/models/note.rb b/app/models/note.rb
index b94e3cff2ce..f2656df028b 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -223,10 +223,6 @@ class Note < ActiveRecord::Base
     end
   end
 
-  def user_authored?(user)
-    user == author
-  end
-
   def award_emoji?
     can_be_award_emoji? && contains_emoji_only?
   end
diff --git a/app/models/project.rb b/app/models/project.rb
index e4b8e708884..b34e320cd34 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -147,6 +147,7 @@ class Project < ActiveRecord::Base
 
   delegate :name, to: :owner, allow_nil: true, prefix: true
   delegate :members, to: :team, prefix: true
+  delegate :add_user, to: :team
 
   # Validations
   validates :creator, presence: true, on: :create
@@ -1017,10 +1018,6 @@ class Project < ActiveRecord::Base
     project_members.find_by(user_id: user)
   end
 
-  def add_user(user, access_level, current_user: nil, expires_at: nil)
-    team.add_user(user, access_level, current_user: current_user, expires_at: expires_at)
-  end
-
   def default_branch
     @default_branch ||= repository.root_ref if repository.exists?
   end
@@ -1294,6 +1291,22 @@ class Project < ActiveRecord::Base
     Gitlab::Redis.with { |redis| redis.del(pushes_since_gc_redis_key) }
   end
 
+  def environments_for(ref, commit, with_tags: false)
+    environment_ids = deployments.group(:environment_id).
+      select(:environment_id)
+
+    environment_ids =
+      if with_tags
+        environment_ids.where('ref=? OR tag IS TRUE', ref)
+      else
+        environment_ids.where(ref: ref)
+      end
+
+    environments.where(id: environment_ids).select do |environment|
+      environment.includes_commit?(commit)
+    end
+  end
+
   private
 
   def pushes_since_gc_redis_key
diff --git a/app/models/project_feature.rb b/app/models/project_feature.rb
index 9c602c582bd..8c9534c3565 100644
--- a/app/models/project_feature.rb
+++ b/app/models/project_feature.rb
@@ -22,6 +22,12 @@ class ProjectFeature < ActiveRecord::Base
 
   belongs_to :project
 
+  default_value_for :builds_access_level,         value: ENABLED, allows_nil: false
+  default_value_for :issues_access_level,         value: ENABLED, allows_nil: false
+  default_value_for :merge_requests_access_level, value: ENABLED, allows_nil: false
+  default_value_for :snippets_access_level,       value: ENABLED, allows_nil: false
+  default_value_for :wiki_access_level,           value: ENABLED, allows_nil: false
+
   def feature_available?(feature, user)
     raise ArgumentError, 'invalid project feature' unless FEATURES.include?(feature)
 
diff --git a/app/models/project_services/custom_issue_tracker_service.rb b/app/models/project_services/custom_issue_tracker_service.rb
index 63a5ed14484..d9fba3d4a41 100644
--- a/app/models/project_services/custom_issue_tracker_service.rb
+++ b/app/models/project_services/custom_issue_tracker_service.rb
@@ -9,6 +9,10 @@ class CustomIssueTrackerService < IssueTrackerService
     end
   end
 
+  def title=(value)
+    self.properties['title'] = value if self.properties
+  end
+
   def description
     if self.properties && self.properties['description'].present?
       self.properties['description']
diff --git a/app/models/project_services/slack_service/issue_message.rb b/app/models/project_services/slack_service/issue_message.rb
index 88e053ec192..cd87a79d0c6 100644
--- a/app/models/project_services/slack_service/issue_message.rb
+++ b/app/models/project_services/slack_service/issue_message.rb
@@ -11,7 +11,7 @@ class SlackService
     attr_reader :description
 
     def initialize(params)
-      @user_name = params[:user][:name]
+      @user_name = params[:user][:username]
       @project_name = params[:project_name]
       @project_url = params[:project_url]
 
diff --git a/app/models/project_services/slack_service/merge_message.rb b/app/models/project_services/slack_service/merge_message.rb
index 11fc691022b..b7615c96068 100644
--- a/app/models/project_services/slack_service/merge_message.rb
+++ b/app/models/project_services/slack_service/merge_message.rb
@@ -10,7 +10,7 @@ class SlackService
     attr_reader :title
 
     def initialize(params)
-      @user_name = params[:user][:name]
+      @user_name = params[:user][:username]
       @project_name = params[:project_name]
       @project_url = params[:project_url]
 
diff --git a/app/models/project_services/slack_service/note_message.rb b/app/models/project_services/slack_service/note_message.rb
index 89ba51cb662..9e84e90f38c 100644
--- a/app/models/project_services/slack_service/note_message.rb
+++ b/app/models/project_services/slack_service/note_message.rb
@@ -10,7 +10,7 @@ class SlackService
 
     def initialize(params)
       params = HashWithIndifferentAccess.new(params)
-      @user_name = params[:user][:name]
+      @user_name = params[:user][:username]
       @project_name = params[:project_name]
       @project_url = params[:project_url]
 
diff --git a/app/models/project_services/slack_service/wiki_page_message.rb b/app/models/project_services/slack_service/wiki_page_message.rb
index f336d9e7691..160ca3ac115 100644
--- a/app/models/project_services/slack_service/wiki_page_message.rb
+++ b/app/models/project_services/slack_service/wiki_page_message.rb
@@ -9,7 +9,7 @@ class SlackService
     attr_reader :description
 
     def initialize(params)
-      @user_name = params[:user][:name]
+      @user_name = params[:user][:username]
       @project_name = params[:project_name]
       @project_url = params[:project_url]
 
diff --git a/app/models/project_team.rb b/app/models/project_team.rb
index ab6ea2aae36..79d041d2775 100644
--- a/app/models/project_team.rb
+++ b/app/models/project_team.rb
@@ -33,18 +33,24 @@ class ProjectTeam
     member
   end
 
-  def add_users(users, access, current_user: nil, expires_at: nil)
+  def add_users(users, access_level, current_user: nil, expires_at: nil)
     ProjectMember.add_users_to_projects(
       [project.id],
       users,
-      access,
+      access_level,
       current_user: current_user,
       expires_at: expires_at
     )
   end
 
-  def add_user(user, access, current_user: nil, expires_at: nil)
-    add_users([user], access, current_user: current_user, expires_at: expires_at)
+  def add_user(user, access_level, current_user: nil, expires_at: nil)
+    ProjectMember.add_user(
+      project,
+      user,
+      access_level,
+      current_user: current_user,
+      expires_at: expires_at
+    )
   end
 
   # Remove all users from project team
@@ -163,7 +169,7 @@ class ProjectTeam
 
       # Each group produces a list of maximum access level per user. We take the
       # max of the values produced by each group.
-      if project.invited_groups.any? && project.allowed_to_share_with_group?
+      if project_shared_with_group?
         project.project_group_links.each do |group_link|
           invited_access = max_invited_level_for_users(group_link, user_ids)
           merge_max!(access, invited_access)
@@ -200,43 +206,17 @@ class ProjectTeam
   def fetch_members(level = nil)
     project_members = project.members
     group_members = group ? group.members : []
-    invited_members = []
-
-    if project.invited_groups.any? && project.allowed_to_share_with_group?
-      project.project_group_links.includes(group: [:group_members]).each do |group_link|
-        invited_group = group_link.group
-        im = invited_group.members
-
-        if level
-          int_level = GroupMember.access_level_roles[level.to_s.singularize.titleize]
-
-          # Skip group members if we ask for masters
-          # but max group access is developers
-          next if int_level > group_link.group_access
-
-          # If we ask for developers and max
-          # group access is developers we need to provide
-          # both group master, developers as devs
-          if int_level == group_link.group_access
-            im.where("access_level >= ?)", group_link.group_access)
-          else
-            im.send(level)
-          end
-        end
-
-        invited_members << im
-      end
-
-      invited_members = invited_members.flatten.compact
-    end
 
     if level
-      project_members = project_members.send(level)
-      group_members = group_members.send(level) if group
+      project_members = project_members.public_send(level)
+      group_members = group_members.public_send(level) if group
     end
 
     user_ids = project_members.pluck(:user_id)
+
+    invited_members = fetch_invited_members(level)
     user_ids.push(*invited_members.map(&:user_id)) if invited_members.any?
+
     user_ids.push(*group_members.pluck(:user_id)) if group
 
     User.where(id: user_ids)
@@ -249,4 +229,38 @@ class ProjectTeam
   def merge_max!(first_hash, second_hash)
     first_hash.merge!(second_hash) { |_key, old, new| old > new ? old : new }
   end
+
+  def project_shared_with_group?
+    project.invited_groups.any? && project.allowed_to_share_with_group?
+  end
+
+  def fetch_invited_members(level = nil)
+    invited_members = []
+
+    return invited_members unless project_shared_with_group?
+
+    project.project_group_links.includes(group: [:group_members]).each do |link|
+      invited_group_members = link.group.members
+
+      if level
+        numeric_level = GroupMember.access_level_roles[level.to_s.singularize.titleize]
+
+        # If we're asked for a level that's higher than the group's access,
+        # there's nothing left to do
+        next if numeric_level > link.group_access
+
+        # Make sure we include everyone _above_ the requested level as well
+        invited_group_members =
+          if numeric_level == link.group_access
+            invited_group_members.where("access_level >= ?", link.group_access)
+          else
+            invited_group_members.public_send(level)
+          end
+      end
+
+      invited_members << invited_group_members
+    end
+
+    invited_members.flatten.compact
+  end
 end
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 772c62a4124..51557228ab9 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -840,7 +840,7 @@ class Repository
 
   def get_committer_and_author(user, email: nil, name: nil)
     committer = user_to_committer(user)
-    author = name && email ? Gitlab::Git::committer_hash(email: email, name: name) : committer
+    author = Gitlab::Git::committer_hash(email: email, name: name) || committer
 
     {
       author: author,
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 5ec933601ac..8a1730f3f36 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -4,6 +4,7 @@ class Snippet < ActiveRecord::Base
   include Participable
   include Referable
   include Sortable
+  include Awardable
 
   default_value_for :visibility_level, Snippet::PRIVATE