Merge branch '4084-epics-username-autocomplete' into 'master'

autocomplete usernames in Epic comments/description

Closes #4084

See merge request gitlab-org/gitlab-ee!5475

app/assets/javascripts/gfm_auto_complete.js

@@ -408,7 +408,10 @@ class GfmAutoComplete {
 
   fetchData($input, at) {
     if (this.isLoadingData[at]) return;
+
     this.isLoadingData[at] = true;
+    const dataSource = this.dataSources[GfmAutoComplete.atTypeMap[at]];
+
     if (this.cachedData[at]) {
       this.loadData($input, at, this.cachedData[at]);
     } else if (GfmAutoComplete.atTypeMap[at] === 'emojis') {
@@ -418,12 +421,14 @@ class GfmAutoComplete {
           GfmAutoComplete.glEmojiTag = glEmojiTag;
         })
         .catch(() => { this.isLoadingData[at] = false; });
-    } else {
-      AjaxCache.retrieve(this.dataSources[GfmAutoComplete.atTypeMap[at]], true)
+    } else if (dataSource) {
+      AjaxCache.retrieve(dataSource, true)
         .then((data) => {
           this.loadData($input, at, data);
         })
         .catch(() => { this.isLoadingData[at] = false; });
+    } else {
+      this.isLoadingData[at] = false;
     }
   }
 

app/assets/javascripts/notes/components/comment_form.vue

@@ -99,10 +99,6 @@ export default {
         'js-note-target-reopen': !this.isOpen,
       };
     },
-    supportQuickActions() {
-      // Disable quick actions support for Epics
-      return this.noteableType !== constants.EPIC_NOTEABLE_TYPE;
-    },
     markdownDocsPath() {
       return this.getNotesData.markdownDocsPath;
     },
@@ -359,7 +355,7 @@ Please check your network connection and try again.`;
                   name="note[note]"
                   class="note-textarea js-vue-comment-form
 js-gfm-input js-autosize markdown-area js-vue-textarea"
-                  :data-supports-quick-actions="supportQuickActions"
+                  data-supports-quick-actions="true"
                   aria-label="Description"
                   v-model="note"
                   ref="textarea"

app/helpers/application_helper.rb

@@ -264,4 +264,17 @@ module ApplicationHelper
 
     _('You are on a read-only GitLab instance.')
   end
+
+  def autocomplete_data_sources(object, noteable_type)
+    return {} unless object && noteable_type
+
+    {
+      members: members_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]),
+      issues: issues_project_autocomplete_sources_path(object),
+      merge_requests: merge_requests_project_autocomplete_sources_path(object),
+      labels: labels_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id]),
+      milestones: milestones_project_autocomplete_sources_path(object),
+      commands: commands_project_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id])
+    }
+  end
 end

app/models/ability.rb

@@ -10,6 +10,14 @@ class Ability
       end
     end
 
+    # Given a list of users and a group this method returns the users that can
+    # read the given group.
+    def users_that_can_read_group(users, group)
+      DeclarativePolicy.subject_scope do
+        users.select { |u| allowed?(u, :read_group, group) }
+      end
+    end
+
     # Given a list of users and a snippet this method returns the users that can
     # read the given snippet.
     def users_that_can_read_personal_snippet(users, snippet)

app/models/concerns/participable.rb

@@ -23,6 +23,7 @@
 #     users = issue.participants
 module Participable
   extend ActiveSupport::Concern
+  prepend EE::Participable
 
   module ClassMethods
     # Adds a list of participant attributes. Attributes can either be symbols or
@@ -98,6 +99,10 @@ module Participable
 
     participants.merge(ext.users)
 
+    filter_by_ability(participants)
+  end
+
+  def filter_by_ability(participants)
     case self
     when PersonalSnippet
       Ability.users_that_can_read_personal_snippet(participants.to_a, self)

app/models/group.rb

@@ -285,6 +285,13 @@ class Group < Namespace
       .where(source_id: self_and_descendants.reorder(nil).select(:id))
   end
 
+  # Returns all members that are part of the group, it's subgroups, and ancestor groups
+  def direct_and_indirect_members
+    GroupMember
+      .active_without_invites_and_requests
+      .where(source_id: self_and_hierarchy.reorder(nil).select(:id))
+  end
+
   def users_with_parents
     User
       .where(id: members_with_parents.select(:user_id))
@@ -297,6 +304,30 @@ class Group < Namespace
       .reorder(nil)
   end
 
+  # Returns all users that are members of the group because:
+  # 1. They belong to the group
+  # 2. They belong to a project that belongs to the group
+  # 3. They belong to a sub-group or project in such sub-group
+  # 4. They belong to an ancestor group
+  def direct_and_indirect_users
+    union = Gitlab::SQL::Union.new([
+      User
+        .where(id: direct_and_indirect_members.select(:user_id))
+        .reorder(nil),
+      project_users_with_descendants
+    ])
+
+    User.from("(#{union.to_sql}) #{User.table_name}")
+  end
+
+  # Returns all users that are members of projects
+  # belonging to the current group or sub-groups
+  def project_users_with_descendants
+    User
+      .joins(projects: :group)
+      .where(namespaces: { id: self_and_descendants.select(:id) })
+  end
+
   def max_member_access_for_user(user)
     return GroupMember::OWNER if user.admin?
 

app/models/namespace.rb

@@ -172,6 +172,13 @@ class Namespace < ActiveRecord::Base
     projects.with_shared_runners.any?
   end
 
+  # Returns all ancestors, self, and descendants of the current namespace.
+  def self_and_hierarchy
+    Gitlab::GroupHierarchy
+      .new(self.class.where(id: id))
+      .all_groups
+  end
+
   # Returns all the ancestors of the current namespaces.
   def ancestors
     return self.class.none unless parent_id

app/services/concerns/users/participable_service.rb

+module Users
+  module ParticipableService
+    extend ActiveSupport::Concern
+
+    included do
+      attr_reader :noteable
+    end
+
+    def noteable_owner
+      return [] unless noteable && noteable.author.present?
+
+      [as_hash(noteable.author)]
+    end
+
+    def participants_in_noteable
+      return [] unless noteable
+
+      users = noteable.participants(current_user)
+      sorted(users)
+    end
+
+    def sorted(users)
+      users.uniq.to_a.compact.sort_by(&:username).map do |user|
+        as_hash(user)
+      end
+    end
+
+    def groups
+      current_user.authorized_groups.sort_by(&:path).map do |group|
+        count = group.users.count
+        { username: group.full_path, name: group.full_name, count: count, avatar_url: group.avatar_url }
+      end
+    end
+
+    private
+
+    def as_hash(user)
+      { username: user.username, name: user.name, avatar_url: user.avatar_url }
+    end
+  end
+end

app/services/projects/participants_service.rb

 module Projects
   class ParticipantsService < BaseService
-    attr_reader :noteable
+    include Users::ParticipableService
 
     def execute(noteable)
       @noteable = noteable
@@ -10,36 +10,6 @@ module Projects
       participants.uniq
     end
 
-    def noteable_owner
-      return [] unless noteable && noteable.author.present?
-
-      [{
-        name: noteable.author.name,
-        username: noteable.author.username,
-        avatar_url: noteable.author.avatar_url
-      }]
-    end
-
-    def participants_in_noteable
-      return [] unless noteable
-
-      users = noteable.participants(current_user)
-      sorted(users)
-    end
-
-    def sorted(users)
-      users.uniq.to_a.compact.sort_by(&:username).map do |user|
-        { username: user.username, name: user.name, avatar_url: user.avatar_url }
-      end
-    end
-
-    def groups
-      current_user.authorized_groups.sort_by(&:path).map do |group|
-        count = group.users.count
-        { username: group.full_path, name: group.full_name, count: count, avatar_url: group.avatar_url }
-      end
-    end
-
     def all_members
       count = project.team.members.flatten.count
       [{ username: "all", name: "All Project and Group Members", count: count }]

app/views/layouts/_init_auto_complete.html.haml

-- project = @target_project || @project
+- object = @target_project || @project || @group
 - noteable_type = @noteable.class if @noteable.present?
 
-- if project
+- datasources = autocomplete_data_sources(object, noteable_type)
+
+- if object
   -# haml-lint:disable InlineJavaScript
   :javascript
     gl = window.gl || {};
     gl.GfmAutoComplete = gl.GfmAutoComplete || {};
-    gl.GfmAutoComplete.dataSources = {
-      members: "#{members_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}",
-      issues: "#{issues_project_autocomplete_sources_path(project)}",
-      mergeRequests: "#{merge_requests_project_autocomplete_sources_path(project)}",
-      labels: "#{labels_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}",
-      milestones: "#{milestones_project_autocomplete_sources_path(project)}",
-      commands: "#{commands_project_autocomplete_sources_path(project, type: noteable_type, type_id: params[:id])}"
-    };
+    gl.GfmAutoComplete.dataSources = #{datasources.to_json};

config/routes/group.rb

@@ -84,6 +84,12 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
       end
     end
 
+    resources :autocomplete_sources, only: [] do
+      collection do
+        get 'members'
+      end
+    end
+
     resources :billings, only: [:index]
     resources :epics, concerns: :awardable, constraints: { id: /\d+/ } do
       member do

ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue

@@ -154,7 +154,7 @@
           :project-namespace="projectNamespace"
           :show-inline-edit-button="true"
           :show-delete-button="false"
-          :enable-autocomplete="false"
+          :enable-autocomplete="true"
         />
       </div>
       <epic-sidebar

ee/app/controllers/groups/autocomplete_sources_controller.rb

+class Groups::AutocompleteSourcesController < Groups::ApplicationController
+  def members
+    render json: ::Groups::ParticipantsService.new(@group, current_user).execute(target)
+  end
+
+  private
+
+  def target
+    case params[:type]&.downcase
+    when 'epic'
+      EpicsFinder.new(current_user, group_id: @group.id).find_by(iid: params[:type_id])
+    end
+  end
+end

ee/app/controllers/groups/epics_controller.rb

@@ -47,7 +47,7 @@ class Groups::EpicsController < Groups::ApplicationController
 
     return render_404 unless can?(current_user, :read_epic, @epic)
 
-    @epic
+    @noteable = @epic
   end
   alias_method :issuable, :epic
   alias_method :awardable, :epic

ee/app/helpers/ee/application_helper.rb

@@ -28,6 +28,17 @@ module EE
       class_names
     end
 
+    override :autocomplete_data_sources
+    def autocomplete_data_sources(object, noteable_type)
+      return {} unless object && noteable_type
+
+      return super unless object.is_a?(Group)
+
+      {
+        members: members_group_autocomplete_sources_path(object, type: noteable_type, type_id: params[:id])
+      }
+    end
+
     private
 
     def appearance

ee/app/models/concerns/ee/participable.rb

+module EE
+  module Participable
+    extend ::Gitlab::Utils::Override
+
+    override :filter_by_ability
+    def filter_by_ability(participants)
+      return super unless self.is_a?(Epic)
+
+      Ability.users_that_can_read_group(participants.to_a, self.group)
+    end
+  end
+end

ee/app/services/groups/participants_service.rb

+module Groups
+  class ParticipantsService < BaseService
+    include Users::ParticipableService
+
+    def execute(noteable)
+      @noteable = noteable
+
+      participants = noteable_owner + participants_in_noteable + all_members + groups + group_members
+      participants.uniq
+    end
+
+    def all_members
+      count = group_members.count
+      [{ username: "all", name: "All Group Members", count: count }]
+    end
+
+    def group_members
+      return [] unless noteable
+
+      @group_members ||= sorted(noteable.group.direct_and_indirect_users)
+    end
+  end
+end

ee/changelogs/unreleased/4084-epics-username-autocomplete.yml

+---
+title: Enable username autocomplete inside Epics
+merge_request: 5475
+author:
+type: added

ee/spec/features/epics/update_epic_spec.rb

@@ -83,11 +83,10 @@ feature 'Update Epic', :js do
         expect(link).to match(link_match)
       end
 
-      # Autocomplete is disabled for epics until #4084 is resolved
-      describe 'autocomplete disabled' do
-        it 'does not open atwho container' do
+      describe 'autocomplete enabled' do
+        it 'opens atwho container' do
           find('#issue-description').native.send_keys('@')
-          expect(page).not_to have_selector('.atwho-container')
+          expect(page).to have_selector('.atwho-container')
         end
       end
     end

ee/spec/helpers/application_helper_spec.rb

+require 'spec_helper'
+
+describe ApplicationHelper do
+  describe '#autocomplete_data_sources' do
+    let(:object) { create(:group) }
+    let(:noteable_type) { Epic }
+    it 'returns paths for autocomplete_sources_controller' do
+      sources = helper.autocomplete_data_sources(object, noteable_type)
+      expect(sources.keys).to match_array([:members])
+      sources.keys.each do |key|
+        expect(sources[key]).not_to be_nil
+      end
+    end
+  end
+end

ee/spec/services/groups/participants_service_spec.rb

+require 'spec_helper'
+
+describe Groups::ParticipantsService do
+  let(:group) { create(:group, avatar: fixture_file_upload(Rails.root + 'spec/fixtures/dk.png')) }
+  let(:user) { create(:user) }
+  let(:epic) { create(:epic, group: group, author: user) }
+
+  before do
+    create(:group_member, group: group, user: user)
+  end
+
+  def user_to_autocompletable(user)
+    {
+      username: user.username,
+      name: user.name,
+      avatar_url: user.avatar_url
+    }
+  end
+
+  describe '#execute' do
+    it 'should add the owner to the list' do
+      expect(described_class.new(group, user).execute(epic).first).to eq(user_to_autocompletable(user))
+    end
+  end
+
+  describe '#participants_in_noteable' do
+    before do
+      @users = []
+      5.times do
+        other_user = create(:user)
+        create(:group_member, group: group, user: other_user)
+        @users << other_user
+      end
+
+      create(:note, author: user, project: nil, noteable: epic, note: @users.map { |u| u.to_reference }.join(' '))
+    end
+
+    it 'should return all participants' do
+      service = described_class.new(group, user)
+      service.instance_variable_set(:@noteable, epic)
+      result = service.participants_in_noteable
+
+      expected_users = (@users + [user]).map(&method(:user_to_autocompletable))
+
+      expect(result).to match_array(expected_users)
+    end
+  end
+
+  describe '#group_members', :nested_groups do
+    let(:parent_group) { create(:group) }
+    let(:group) { create(:group, parent: parent_group) }
+    let(:subgroup) { create(:group_with_members, parent: group) }
+    let(:subproject) { create(:project, group: subgroup) }
+
+    it 'should return all members in parent groups, sub-groups, and sub-projects' do
+      parent_group.add_developer(create(:user))
+      subgroup.add_developer(create(:user))
+      subproject.add_developer(create(:user))
+
+      service = described_class.new(group, user)
+      service.instance_variable_set(:@noteable, epic)
+      result = service.group_members
+
+      expected_users = (group.self_and_hierarchy.map(&:users).flatten + subproject.users)
+                       .map(&method(:user_to_autocompletable))
+
+      expect(expected_users.count).to eq(5)
+      expect(result).to match_array(expected_users)
+    end
+  end
+
+  describe '#groups' do
+    describe 'avatar_url' do
+      let(:groups) { described_class.new(group, user).groups }
+
+      it 'should return an url for the avatar' do
+        expect(groups.size).to eq 1
+        expect(groups.first[:avatar_url]).to eq("/uploads/-/system/group/avatar/#{group.id}/dk.png")
+      end
+
+      it 'should return an url for the avatar with relative url' do
+        stub_config_setting(relative_url_root: '/gitlab')
+        stub_config_setting(url: Settings.send(:build_gitlab_url))
+
+        expect(groups.size).to eq 1
+        expect(groups.first[:avatar_url]).to eq("/gitlab/uploads/-/system/group/avatar/#{group.id}/dk.png")
+      end
+    end
+  end
+end

spec/helpers/application_helper_spec.rb

@@ -151,4 +151,16 @@ describe ApplicationHelper do
       end
     end
   end
+
+  describe '#autocomplete_data_sources' do
+    let(:project) { create(:project) }
+    let(:noteable_type) { Issue }
+    it 'returns paths for autocomplete_sources_controller' do
+      sources = helper.autocomplete_data_sources(project, noteable_type)
+      expect(sources.keys).to match_array([:members, :issues, :merge_requests, :labels, :milestones, :commands])
+      sources.keys.each do |key|
+        expect(sources[key]).not_to be_nil
+      end
+    end
+  end
 end

spec/models/group_spec.rb

@@ -425,6 +425,23 @@ describe Group do
     end
   end
 
+  describe '#direct_and_indirect_members', :nested_groups do
+    let!(:group) { create(:group, :nested) }
+    let!(:sub_group) { create(:group, parent: group) }
+    let!(:master) { group.parent.add_user(create(:user), GroupMember::MASTER) }
+    let!(:developer) { group.add_user(create(:user), GroupMember::DEVELOPER) }
+    let!(:other_developer) { group.add_user(create(:user), GroupMember::DEVELOPER) }
+
+    it 'returns parents members' do
+      expect(group.direct_and_indirect_members).to include(developer)
+      expect(group.direct_and_indirect_members).to include(master)
+    end
+
+    it 'returns descendant members' do
+      expect(group.direct_and_indirect_members).to include(other_developer)
+    end
+  end
+
   describe '#users_with_descendants', :nested_groups do
     let(:user_a) { create(:user) }
     let(:user_b) { create(:user) }
@@ -444,6 +461,59 @@ describe Group do
     end
   end
 
+  describe '#direct_and_indirect_users', :nested_groups do
+    let(:user_a) { create(:user) }
+    let(:user_b) { create(:user) }
+    let(:user_c) { create(:user) }
+    let(:user_d) { create(:user) }
+
+    let(:group) { create(:group) }
+    let(:nested_group) { create(:group, parent: group) }
+    let(:deep_nested_group) { create(:group, parent: nested_group) }
+    let(:project) { create(:project, namespace: group) }
+
+    before do
+      group.add_developer(user_a)
+      group.add_developer(user_c)
+      nested_group.add_developer(user_b)
+      deep_nested_group.add_developer(user_a)
+      project.add_developer(user_d)
+    end
+
+    it 'returns member users on every nest level without duplication' do
+      expect(group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c, user_d)
+      expect(nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c)
+      expect(deep_nested_group.direct_and_indirect_users).to contain_exactly(user_a, user_b, user_c)
+    end
+
+    it 'does not return members of projects belonging to ancestor groups' do
+      expect(nested_group.direct_and_indirect_users).not_to include(user_d)
+    end
+  end
+
+  describe '#project_users_with_descendants', :nested_groups do
+    let(:user_a) { create(:user) }
+    let(:user_b) { create(:user) }
+    let(:user_c) { create(:user) }
+
+    let(:group) { create(:group) }
+    let(:nested_group) { create(:group, parent: group) }
+    let(:deep_nested_group) { create(:group, parent: nested_group) }
+    let(:project_a) { create(:project, namespace: group) }
+    let(:project_b) { create(:project, namespace: nested_group) }
+    let(:project_c) { create(:project, namespace: deep_nested_group) }
+
+    it 'returns members of all projects in group and subgroups' do
+      project_a.add_developer(user_a)
+      project_b.add_developer(user_b)
+      project_c.add_developer(user_c)
+
+      expect(group.project_users_with_descendants).to contain_exactly(user_a, user_b, user_c)
+      expect(nested_group.project_users_with_descendants).to contain_exactly(user_b, user_c)
+      expect(deep_nested_group.project_users_with_descendants).to contain_exactly(user_c)
+    end
+  end
+
   describe '#user_ids_for_project_authorizations' do
     it 'returns the user IDs for which to refresh authorizations' do
       master = create(:user)

spec/models/namespace_spec.rb

@@ -411,6 +411,21 @@ describe Namespace do
     end
   end
 
+  describe '#self_and_hierarchy', :nested_groups do
+    let!(:group) { create(:group, path: 'git_lab') }
+    let!(:nested_group) { create(:group, parent: group) }
+    let!(:deep_nested_group) { create(:group, parent: nested_group) }
+    let!(:very_deep_nested_group) { create(:group, parent: deep_nested_group) }
+    let!(:another_group) { create(:group, path: 'gitllab') }
+    let!(:another_group_nested) { create(:group, path: 'foo', parent: another_group) }
+
+    it 'returns the correct tree' do
+      expect(group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group)
+      expect(nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group)
+      expect(very_deep_nested_group.self_and_hierarchy).to contain_exactly(group, nested_group, deep_nested_group, very_deep_nested_group)
+    end
+  end
+
   describe '#ancestors', :nested_groups do
     let(:group) { create(:group) }
     let(:nested_group) { create(:group, parent: group) }