Merge branch 'epic-events-list-ee' into 'master'

[EE] Show epic events on group activity page

See merge request gitlab-org/gitlab!16108

app/controllers/groups_controller.rb

@@ -198,15 +198,13 @@ class GroupsController < Groups::ApplicationController
   def load_events
     params[:sort] ||= 'latest_activity_desc'
 
-    options = {}
-    options[:include_subgroups] = true
-
-    @projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user)
-                  .execute
-                  .includes(:namespace)
+    options = { include_subgroups: true }
+    projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user)
+                 .execute
+                 .includes(:namespace)
 
     @events = EventCollection
-                .new(@projects, offset: params[:offset].to_i, filter: event_filter)
+                .new(projects, offset: params[:offset].to_i, filter: event_filter, groups: groups)
                 .to_a
 
     Events::RenderService
@@ -228,6 +226,14 @@ class GroupsController < Groups::ApplicationController
 
     url_for(safe_params)
   end
+
+  private
+
+  def groups
+    if @group.supports_events?
+      @group.self_and_descendants.public_or_visible_to_user(current_user)
+    end
+  end
 end
 
 GroupsController.prepend_if_ee('EE::GroupsController')

app/models/event.rb

@@ -77,15 +77,6 @@ class Event < ApplicationRecord
   scope :recent, -> { reorder(id: :desc) }
   scope :code_push, -> { where(action: PUSHED) }
 
-  scope :in_projects, -> (projects) do
-    sub_query = projects
-      .except(:order)
-      .select(1)
-      .where('projects.id = events.project_id')
-
-    where('EXISTS (?)', sub_query).recent
-  end
-
   scope :with_associations, -> do
     # We're using preload for "push_event_payload" as otherwise the association
     # is not always available (depending on the query being built).

app/models/event_collection.rb

@@ -6,6 +6,8 @@
 # in a controller), it's not suitable for building queries that are used for
 # building other queries.
 class EventCollection
+  include Gitlab::Utils::StrongMemoize
+
   # To prevent users from putting too much pressure on the database by cycling
   # through thousands of events we put a limit on the number of pages.
   MAX_PAGE = 10
@@ -13,57 +15,52 @@ class EventCollection
   # projects - An ActiveRecord::Relation object that returns the projects for
   #            which to retrieve events.
   # filter - An EventFilter instance to use for filtering events.
-  def initialize(projects, limit: 20, offset: 0, filter: nil)
+  def initialize(projects, limit: 20, offset: 0, filter: nil, groups: nil)
     @projects = projects
     @limit = limit
     @offset = offset
     @filter = filter
+    @groups = groups
   end
 
   # Returns an Array containing the events.
   def to_a
     return [] if current_page > MAX_PAGE
 
-    relation = if Gitlab::Database.join_lateral_supported?
-                 relation_with_join_lateral
+    relation = if groups
+                 project_and_group_events
                else
-                 relation_without_join_lateral
+                 relation_with_join_lateral('project_id', projects)
                end
 
+    relation = paginate_events(relation)
     relation.with_associations.to_a
   end
 
   private
 
-  # Returns the events relation to use when JOIN LATERAL is not supported.
-  #
-  # This relation simply gets all the events for all authorized projects, then
-  # limits that set.
-  def relation_without_join_lateral
-    events = filtered_events.in_projects(projects)
+  def project_and_group_events
+    project_events = relation_with_join_lateral('project_id', projects)
+    group_events = relation_with_join_lateral('group_id', groups)
 
-    paginate_events(events)
+    Event.from_union([project_events, group_events]).recent
   end
 
-  # Returns the events relation to use when JOIN LATERAL is supported.
-  #
   # This relation is built using JOIN LATERAL, producing faster queries than a
   # regular LIMIT + OFFSET approach.
-  def relation_with_join_lateral
-    projects_for_lateral = projects.select(:id).to_sql
+  def relation_with_join_lateral(parent_column, parents)
+    parents_for_lateral = parents.select(:id).to_sql
 
     lateral = filtered_events
       .limit(limit_for_join_lateral)
-      .where('events.project_id = projects_for_lateral.id')
+      .where("events.#{parent_column} = parents_for_lateral.id") # rubocop:disable GitlabSecurity/SqlInjection
       .to_sql
 
     # The outer query does not need to re-apply the filters since the JOIN
     # LATERAL body already takes care of this.
-    outer = base_relation
-      .from("(#{projects_for_lateral}) projects_for_lateral")
+    base_relation
+      .from("(#{parents_for_lateral}) parents_for_lateral")
       .joins("JOIN LATERAL (#{lateral}) AS #{Event.table_name} ON true")
-
-    paginate_events(outer)
   end
 
   def filtered_events
@@ -97,4 +94,10 @@ class EventCollection
   def projects
     @projects.except(:order)
   end
+
+  def groups
+    strong_memoize(:groups) do
+      groups.except(:order) if @groups
+    end
+  end
 end

app/models/group.rb

@@ -436,6 +436,10 @@ class Group < Namespace
     members.owners.order_recent_sign_in.limit(ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT)
   end
 
+  def supports_events?
+    false
+  end
+
   private
 
   def update_two_factor_requirement

app/views/groups/_activities.html.haml

 .nav-block.activities
-  = render 'shared/event_filter'
+  = render 'shared/event_filter', show_group_events: @group.supports_events?
   .controls
     = link_to group_path(@group, rss_url_options), class: 'btn d-none d-sm-inline-block has-tooltip' , title: 'Subscribe' do
       %i.fa.fa-rss

app/views/shared/_event_filter.html.haml

+- show_group_events = local_assigns.fetch(:show_group_events, false)
+
 .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller.flex-fill
   .fade-left= icon('angle-left')
   .fade-right= icon('angle-right')
@@ -9,6 +11,8 @@
       = event_filter_link EventFilter::MERGED, _('Merge events'), s_('EventFilterBy|Filter by merge events')
     - if event_filter_visible(:issues)
       = event_filter_link EventFilter::ISSUE, _('Issue events'), s_('EventFilterBy|Filter by issue events')
+    - if show_group_events
+      = render_if_exists 'events/epics_filter'
     - if comments_visible?
       = event_filter_link EventFilter::COMMENTS, _('Comments'), s_('EventFilterBy|Filter by comments')
     = event_filter_link EventFilter::TEAM, _('Team'), s_('EventFilterBy|Filter by team')

ee/app/models/ee/event.rb

@@ -3,6 +3,7 @@
 module EE
   module Event
     extend ActiveSupport::Concern
+    extend ::Gitlab::Utils::Override
 
     prepended do
       scope :issues, -> { where(target_type: 'Issue') }
@@ -12,6 +13,18 @@ module EE
       scope :merged, -> { where(action: ::Event::MERGED) }
       scope :totals_by_author, -> { group(:author_id).count }
       scope :totals_by_author_target_type_action, -> { group(:author_id, :target_type, :action).count }
+      scope :epics, -> { where(target_type: 'Epic') }
+    end
+
+    override :visible_to_user?
+    def visible_to_user?(user = nil)
+      if epic?
+        Ability.allowed?(user, :read_epic, target)
+      elsif epic_note?
+        Ability.allowed?(user, :read_epic, note_target)
+      else
+        super
+      end
     end
 
     def epic_note?

ee/app/models/ee/group.rb

@@ -232,6 +232,11 @@ module EE
       ::Gitlab.config.dependency_proxy.enabled && feature_available?(:dependency_proxy)
     end
 
+    override :supports_events?
+    def supports_events?
+      ::Feature.enabled?(:group_events) && feature_available?(:epics)
+    end
+
     private
 
     def custom_project_templates_group_allowed

ee/app/views/events/_epics_filter.html.haml

+= event_filter_link EventFilter::EPIC, _('Epic events'), s_('EventFilterBy|Filter by epic events')

ee/lib/ee/event_filter.rb

+# frozen_string_literal: true
+
+module EE
+  module EventFilter
+    extend ::Gitlab::Utils::Override
+
+    EPIC = 'epic'
+
+    override :apply_filter
+    def apply_filter(events)
+      case filter
+      when EPIC
+        events.epics
+      else
+        super
+      end
+    end
+
+    private
+
+    override :filters
+    def filters
+      super << EPIC
+    end
+  end
+end

ee/spec/controllers/ee/groups_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe GroupsController do
+  include ExternalAuthorizationServiceHelpers
+
+  set(:user) { create(:user) }
+  set(:group) { create(:group, :public) }
+  set(:project) { create(:project, :public, namespace: group) }
+  set(:subgroup) { create(:group, :private, parent: group) }
+  set(:subgroup2) { create(:group, :private, parent: subgroup) }
+
+  describe 'GET #activity' do
+    render_views
+
+    set(:event1) { create(:event, project: project) }
+    set(:event2) { create(:event, project: nil, group: group) }
+    set(:event3) { create(:event, project: nil, group: subgroup) }
+    set(:event4) { create(:event, project: nil, group: subgroup2) }
+
+    context 'when authorized' do
+      before do
+        group.add_owner(user)
+        subgroup.add_owner(user)
+        subgroup2.add_owner(user)
+        sign_in(user)
+      end
+
+      context 'when group events are available' do
+        before do
+          stub_licensed_features(epics: true)
+        end
+
+        it 'includes events from group and subgroups' do
+          get :activity, params: { id: group.to_param }, format: :json
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['count']).to eq(4)
+        end
+      end
+
+      context 'when group events are not available' do
+        before do
+          stub_licensed_features(epics: false)
+        end
+
+        it 'does not include events from group and subgroups' do
+          get :activity, params: { id: group.to_param }, format: :json
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response['count']).to eq(1)
+        end
+      end
+    end
+
+    context 'when unauthorized' do
+      before do
+        stub_licensed_features(epics: true)
+      end
+
+      it 'includes only events visible to user' do
+        get :activity, params: { id: group.to_param }, format: :json
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response['count']).to eq(2)
+      end
+    end
+  end
+end

ee/spec/lib/ee/event_filter_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe EventFilter do
+  describe '#apply_filter' do
+    set(:group) { create(:group, :public) }
+    set(:project) { create(:project, :public) }
+    set(:epic_event) { create(:event, :created, group: group, target: create(:epic, group: group)) }
+    set(:issue_event) { create(:event, :created, project: project, target: create(:issue, project: project)) }
+    let(:filtered_events) { described_class.new(filter).apply_filter(Event.all) }
+
+    context 'with the "epic" filter' do
+      let(:filter) { described_class::EPIC }
+
+      it 'filters issue events only' do
+        expect(filtered_events).to contain_exactly(epic_event)
+      end
+    end
+  end
+end

ee/spec/models/ee/event_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Event do
+  describe '#visible_to_user?' do
+    set(:non_member) { create(:user) }
+    set(:member) { create(:user) }
+    set(:guest) { create(:user) }
+    set(:author) { create(:author) }
+    set(:admin) { create(:admin) }
+    let(:epic) { create(:epic, group: group, author: author) }
+    let(:note_on_epic) { create(:note, :on_epic, noteable: epic) }
+    let(:event) { described_class.new(group: group, target: target, author: author) }
+
+    before do
+      stub_licensed_features(epics: true)
+      group.add_developer(member)
+      group.add_guest(guest)
+    end
+
+    shared_examples 'visible to group members only' do
+      it 'is not visible to other users' do
+        expect(event.visible_to_user?(non_member)).to eq false
+        expect(event.visible_to_user?(member)).to eq true
+        expect(event.visible_to_user?(guest)).to eq true
+        expect(event.visible_to_user?(admin)).to eq true
+      end
+    end
+
+    shared_examples 'visible to everybody' do
+      it 'is visible to other users' do
+        expect(event.visible_to_user?(non_member)).to eq true
+        expect(event.visible_to_user?(member)).to eq true
+        expect(event.visible_to_user?(guest)).to eq true
+        expect(event.visible_to_user?(admin)).to eq true
+      end
+    end
+
+    context 'epic event' do
+      let(:target) { epic }
+
+      context 'on public group' do
+        let(:group) { create(:group, :public) }
+
+        it_behaves_like 'visible to everybody'
+      end
+
+      context 'on private group' do
+        let(:group) { create(:group, :private) }
+
+        it_behaves_like 'visible to group members only'
+      end
+    end
+
+    context 'epic note event' do
+      let(:target) { note_on_epic }
+
+      context 'on public group' do
+        let(:group) { create(:group, :public) }
+
+        it_behaves_like 'visible to everybody'
+      end
+
+      context 'private group' do
+        let(:group) { create(:group, :private) }
+
+        it_behaves_like 'visible to group members only'
+      end
+    end
+  end
+end

lib/event_filter.rb

@@ -9,12 +9,11 @@ class EventFilter
   ISSUE = 'issue'
   COMMENTS = 'comments'
   TEAM = 'team'
-  FILTERS = [ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM].freeze
 
   def initialize(filter)
     # Split using comma to maintain backward compatibility Ex/ "filter1,filter2"
     filter = filter.to_s.split(',')[0].to_s
-    @filter = FILTERS.include?(filter) ? filter : ALL
+    @filter = filters.include?(filter) ? filter : ALL
   end
 
   def active?(key)
@@ -39,4 +38,12 @@ class EventFilter
     end
   end
   # rubocop: enable CodeReuse/ActiveRecord
+
+  private
+
+  def filters
+    [ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM]
+  end
 end
+
+EventFilter.prepend_if_ee('EE::EventFilter')

lib/gitlab/database.rb

@@ -87,10 +87,6 @@ module Gitlab
       version.to_f < 10
     end
 
-    def self.join_lateral_supported?
-      version.to_f >= 9.3
-    end
-
     def self.replication_slots_supported?
       version.to_f >= 9.4
     end

locale/gitlab.pot

@@ -6162,6 +6162,9 @@ msgstr ""
 msgid "Epic"
 msgstr ""
 
+msgid "Epic events"
+msgstr ""
+
 msgid "Epics"
 msgstr ""
 
@@ -6435,6 +6438,9 @@ msgstr ""
 msgid "EventFilterBy|Filter by comments"
 msgstr ""
 
+msgid "EventFilterBy|Filter by epic events"
+msgstr ""
+
 msgid "EventFilterBy|Filter by issue events"
 msgstr ""
 

spec/controllers/groups_controller_spec.rb

@@ -125,7 +125,7 @@ describe GroupsController do
     end
 
     context 'as json' do
-      it 'includes all projects from groups and subgroups in event feed' do
+      it 'includes events from all projects in group and subgroups' do
         2.times do
           project = create(:project, group: group)
           create(:event, project: project)

spec/features/projects/features_visibility_spec.rb

@@ -211,7 +211,7 @@ describe 'Edit Project Settings' do
       visit activity_project_path(project)
 
       page.within(".event-filter") do
-        expect(page).to have_selector("a", count: 2)
+        expect(page).to have_content("All")
         expect(page).not_to have_content("Push events")
         expect(page).not_to have_content("Merge events")
         expect(page).not_to have_content("Comments")

spec/lib/event_filter_spec.rb

@@ -3,12 +3,6 @@
 require 'spec_helper'
 
 describe EventFilter do
-  describe 'FILTERS' do
-    it 'returns a definite list of filters' do
-      expect(described_class::FILTERS).to eq(%w[all push merged issue comments team])
-    end
-  end
-
   describe '#filter' do
     it 'returns "all" if given filter is nil' do
       expect(described_class.new(nil).filter).to eq(described_class::ALL)

spec/lib/gitlab/database_spec.rb

@@ -101,20 +101,6 @@ describe Gitlab::Database do
     end
   end
 
-  describe '.join_lateral_supported?' do
-    it 'returns false when using PostgreSQL 9.2' do
-      allow(described_class).to receive(:version).and_return('9.2.1')
-
-      expect(described_class.join_lateral_supported?).to eq(false)
-    end
-
-    it 'returns true when using PostgreSQL 9.3.0 or newer' do
-      allow(described_class).to receive(:version).and_return('9.3.0')
-
-      expect(described_class.join_lateral_supported?).to eq(true)
-    end
-  end
-
   describe '.replication_slots_supported?' do
     it 'returns false when using PostgreSQL 9.3' do
       allow(described_class).to receive(:version).and_return('9.3.1')

spec/models/event_collection_spec.rb

@@ -4,50 +4,75 @@ require 'spec_helper'
 
 describe EventCollection do
   describe '#to_a' do
-    let(:project) { create(:project_empty_repo) }
-    let(:projects) { Project.where(id: project.id) }
-    let(:user) { create(:user) }
+    set(:group) { create(:group) }
+    set(:project) { create(:project_empty_repo, group: group) }
+    set(:projects) { Project.where(id: project.id) }
+    set(:user) { create(:user) }
 
-    before do
-      20.times do
-        event = create(:push_event, project: project, author: user)
+    context 'with project events' do
+      before do
+        20.times do
+          event = create(:push_event, project: project, author: user)
 
-        create(:push_event_payload, event: event)
+          create(:push_event_payload, event: event)
+        end
+
+        create(:closed_issue_event, project: project, author: user)
       end
 
-      create(:closed_issue_event, project: project, author: user)
-    end
+      it 'returns an Array of events' do
+        events = described_class.new(projects).to_a
 
-    it 'returns an Array of events' do
-      events = described_class.new(projects).to_a
+        expect(events).to be_an_instance_of(Array)
+      end
 
-      expect(events).to be_an_instance_of(Array)
-    end
+      it 'applies a limit to the number of events' do
+        events = described_class.new(projects).to_a
 
-    it 'applies a limit to the number of events' do
-      events = described_class.new(projects).to_a
+        expect(events.length).to eq(20)
+      end
 
-      expect(events.length).to eq(20)
-    end
+      it 'can paginate through events' do
+        events = described_class.new(projects, offset: 20).to_a
 
-    it 'can paginate through events' do
-      events = described_class.new(projects, offset: 20).to_a
+        expect(events.length).to eq(1)
+      end
 
-      expect(events.length).to eq(1)
-    end
+      it 'returns an empty Array when crossing the maximum page number' do
+        events = described_class.new(projects, limit: 1, offset: 15).to_a
 
-    it 'returns an empty Array when crossing the maximum page number' do
-      events = described_class.new(projects, limit: 1, offset: 15).to_a
+        expect(events).to be_empty
+      end
+
+      it 'allows filtering of events using an EventFilter' do
+        filter = EventFilter.new(EventFilter::ISSUE)
+        events = described_class.new(projects, filter: filter).to_a
 
-      expect(events).to be_empty
+        expect(events.length).to eq(1)
+        expect(events[0].action).to eq(Event::CLOSED)
+      end
     end
 
-    it 'allows filtering of events using an EventFilter' do
-      filter = EventFilter.new(EventFilter::ISSUE)
-      events = described_class.new(projects, filter: filter).to_a
+    context 'with group events' do
+      let(:groups) { group.self_and_descendants.public_or_visible_to_user(user) }
+      let(:subject) { described_class.new(projects, groups: groups).to_a }
+
+      it 'includes also group events' do
+        subgroup = create(:group, parent: group)
+        event1 = create(:event, project: project, author: user)
+        event2 = create(:event, project: nil, group: group, author: user)
+        event3 = create(:event, project: nil, group: subgroup, author: user)
 
-      expect(events.length).to eq(1)
-      expect(events[0].action).to eq(Event::CLOSED)
+        expect(subject).to eq([event3, event2, event1])
+      end
+
+      it 'does not include events from inaccessible groups' do
+        subgroup = create(:group, :private, parent: group)
+        event1 = create(:event, project: nil, group: group, author: user)
+        create(:event, project: nil, group: subgroup, author: user)
+
+        expect(subject).to eq([event1])
+      end
     end
   end
 end