Commit 87259315 authored by Jan Provaznik's avatar Jan Provaznik Committed by Stan Hu

Update group activity page to show group events

* group events are now inclueded in the 'All' tab and a new 'Epic
  events' tab added to this page
* group events are not showed on other activity pages (e.g. user) yet
* this MR also removes old "non-join_lateral" query for getting
  events because this was used only for postgres <9.3 and we now
  require at least 9.6
parent 722dca57
...@@ -198,15 +198,13 @@ class GroupsController < Groups::ApplicationController ...@@ -198,15 +198,13 @@ class GroupsController < Groups::ApplicationController
def load_events def load_events
params[:sort] ||= 'latest_activity_desc' params[:sort] ||= 'latest_activity_desc'
options = {} options = { include_subgroups: true }
options[:include_subgroups] = true projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user)
.execute
@projects = GroupProjectsFinder.new(params: params, group: group, options: options, current_user: current_user) .includes(:namespace)
.execute
.includes(:namespace)
@events = EventCollection @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 .to_a
Events::RenderService Events::RenderService
...@@ -228,6 +226,14 @@ class GroupsController < Groups::ApplicationController ...@@ -228,6 +226,14 @@ class GroupsController < Groups::ApplicationController
url_for(safe_params) url_for(safe_params)
end end
private
def groups
if @group.supports_events?
@group.self_and_descendants.public_or_visible_to_user(current_user)
end
end
end end
GroupsController.prepend_if_ee('EE::GroupsController') GroupsController.prepend_if_ee('EE::GroupsController')
...@@ -77,15 +77,6 @@ class Event < ApplicationRecord ...@@ -77,15 +77,6 @@ class Event < ApplicationRecord
scope :recent, -> { reorder(id: :desc) } scope :recent, -> { reorder(id: :desc) }
scope :code_push, -> { where(action: PUSHED) } 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 scope :with_associations, -> do
# We're using preload for "push_event_payload" as otherwise the association # We're using preload for "push_event_payload" as otherwise the association
# is not always available (depending on the query being built). # is not always available (depending on the query being built).
......
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
# in a controller), it's not suitable for building queries that are used for # in a controller), it's not suitable for building queries that are used for
# building other queries. # building other queries.
class EventCollection class EventCollection
include Gitlab::Utils::StrongMemoize
# To prevent users from putting too much pressure on the database by cycling # 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. # through thousands of events we put a limit on the number of pages.
MAX_PAGE = 10 MAX_PAGE = 10
...@@ -13,57 +15,52 @@ class EventCollection ...@@ -13,57 +15,52 @@ class EventCollection
# projects - An ActiveRecord::Relation object that returns the projects for # projects - An ActiveRecord::Relation object that returns the projects for
# which to retrieve events. # which to retrieve events.
# filter - An EventFilter instance to use for filtering 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 @projects = projects
@limit = limit @limit = limit
@offset = offset @offset = offset
@filter = filter @filter = filter
@groups = groups
end end
# Returns an Array containing the events. # Returns an Array containing the events.
def to_a def to_a
return [] if current_page > MAX_PAGE return [] if current_page > MAX_PAGE
relation = if Gitlab::Database.join_lateral_supported? relation = if groups
relation_with_join_lateral project_and_group_events
else else
relation_without_join_lateral relation_with_join_lateral('project_id', projects)
end end
relation = paginate_events(relation)
relation.with_associations.to_a relation.with_associations.to_a
end end
private private
# Returns the events relation to use when JOIN LATERAL is not supported. def project_and_group_events
# project_events = relation_with_join_lateral('project_id', projects)
# This relation simply gets all the events for all authorized projects, then group_events = relation_with_join_lateral('group_id', groups)
# limits that set.
def relation_without_join_lateral
events = filtered_events.in_projects(projects)
paginate_events(events) Event.from_union([project_events, group_events]).recent
end end
# Returns the events relation to use when JOIN LATERAL is supported.
#
# This relation is built using JOIN LATERAL, producing faster queries than a # This relation is built using JOIN LATERAL, producing faster queries than a
# regular LIMIT + OFFSET approach. # regular LIMIT + OFFSET approach.
def relation_with_join_lateral def relation_with_join_lateral(parent_column, parents)
projects_for_lateral = projects.select(:id).to_sql parents_for_lateral = parents.select(:id).to_sql
lateral = filtered_events lateral = filtered_events
.limit(limit_for_join_lateral) .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 .to_sql
# The outer query does not need to re-apply the filters since the JOIN # The outer query does not need to re-apply the filters since the JOIN
# LATERAL body already takes care of this. # LATERAL body already takes care of this.
outer = base_relation base_relation
.from("(#{projects_for_lateral}) projects_for_lateral") .from("(#{parents_for_lateral}) parents_for_lateral")
.joins("JOIN LATERAL (#{lateral}) AS #{Event.table_name} ON true") .joins("JOIN LATERAL (#{lateral}) AS #{Event.table_name} ON true")
paginate_events(outer)
end end
def filtered_events def filtered_events
...@@ -97,4 +94,10 @@ class EventCollection ...@@ -97,4 +94,10 @@ class EventCollection
def projects def projects
@projects.except(:order) @projects.except(:order)
end end
def groups
strong_memoize(:groups) do
groups.except(:order) if @groups
end
end
end end
...@@ -436,6 +436,10 @@ class Group < Namespace ...@@ -436,6 +436,10 @@ class Group < Namespace
members.owners.order_recent_sign_in.limit(ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT) members.owners.order_recent_sign_in.limit(ACCESS_REQUEST_APPROVERS_TO_BE_NOTIFIED_LIMIT)
end end
def supports_events?
false
end
private private
def update_two_factor_requirement def update_two_factor_requirement
......
.nav-block.activities .nav-block.activities
= render 'shared/event_filter' = render 'shared/event_filter', show_group_events: @group.supports_events?
.controls .controls
= link_to group_path(@group, rss_url_options), class: 'btn d-none d-sm-inline-block has-tooltip' , title: 'Subscribe' do = 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 %i.fa.fa-rss
......
- show_group_events = local_assigns.fetch(:show_group_events, false)
.scrolling-tabs-container.inner-page-scroll-tabs.is-smaller.flex-fill .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller.flex-fill
.fade-left= icon('angle-left') .fade-left= icon('angle-left')
.fade-right= icon('angle-right') .fade-right= icon('angle-right')
...@@ -9,6 +11,8 @@ ...@@ -9,6 +11,8 @@
= event_filter_link EventFilter::MERGED, _('Merge events'), s_('EventFilterBy|Filter by merge events') = event_filter_link EventFilter::MERGED, _('Merge events'), s_('EventFilterBy|Filter by merge events')
- if event_filter_visible(:issues) - if event_filter_visible(:issues)
= event_filter_link EventFilter::ISSUE, _('Issue events'), s_('EventFilterBy|Filter by issue events') = 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? - if comments_visible?
= event_filter_link EventFilter::COMMENTS, _('Comments'), s_('EventFilterBy|Filter by comments') = event_filter_link EventFilter::COMMENTS, _('Comments'), s_('EventFilterBy|Filter by comments')
= event_filter_link EventFilter::TEAM, _('Team'), s_('EventFilterBy|Filter by team') = event_filter_link EventFilter::TEAM, _('Team'), s_('EventFilterBy|Filter by team')
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
module EE module EE
module Event module Event
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override
prepended do prepended do
scope :issues, -> { where(target_type: 'Issue') } scope :issues, -> { where(target_type: 'Issue') }
...@@ -12,6 +13,18 @@ module EE ...@@ -12,6 +13,18 @@ module EE
scope :merged, -> { where(action: ::Event::MERGED) } scope :merged, -> { where(action: ::Event::MERGED) }
scope :totals_by_author, -> { group(:author_id).count } scope :totals_by_author, -> { group(:author_id).count }
scope :totals_by_author_target_type_action, -> { group(:author_id, :target_type, :action).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 end
def epic_note? def epic_note?
......
...@@ -232,6 +232,11 @@ module EE ...@@ -232,6 +232,11 @@ module EE
::Gitlab.config.dependency_proxy.enabled && feature_available?(:dependency_proxy) ::Gitlab.config.dependency_proxy.enabled && feature_available?(:dependency_proxy)
end end
override :supports_events?
def supports_events?
::Feature.enabled?(:group_events) && feature_available?(:epics)
end
private private
def custom_project_templates_group_allowed def custom_project_templates_group_allowed
......
= event_filter_link EventFilter::EPIC, _('Epic events'), s_('EventFilterBy|Filter by epic events')
# 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
# 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
# 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
# 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
...@@ -9,12 +9,11 @@ class EventFilter ...@@ -9,12 +9,11 @@ class EventFilter
ISSUE = 'issue' ISSUE = 'issue'
COMMENTS = 'comments' COMMENTS = 'comments'
TEAM = 'team' TEAM = 'team'
FILTERS = [ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM].freeze
def initialize(filter) def initialize(filter)
# Split using comma to maintain backward compatibility Ex/ "filter1,filter2" # Split using comma to maintain backward compatibility Ex/ "filter1,filter2"
filter = filter.to_s.split(',')[0].to_s filter = filter.to_s.split(',')[0].to_s
@filter = FILTERS.include?(filter) ? filter : ALL @filter = filters.include?(filter) ? filter : ALL
end end
def active?(key) def active?(key)
...@@ -39,4 +38,12 @@ class EventFilter ...@@ -39,4 +38,12 @@ class EventFilter
end end
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
private
def filters
[ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM]
end
end end
EventFilter.prepend_if_ee('EE::EventFilter')
...@@ -87,10 +87,6 @@ module Gitlab ...@@ -87,10 +87,6 @@ module Gitlab
version.to_f < 10 version.to_f < 10
end end
def self.join_lateral_supported?
version.to_f >= 9.3
end
def self.replication_slots_supported? def self.replication_slots_supported?
version.to_f >= 9.4 version.to_f >= 9.4
end end
......
...@@ -6162,6 +6162,9 @@ msgstr "" ...@@ -6162,6 +6162,9 @@ msgstr ""
msgid "Epic" msgid "Epic"
msgstr "" msgstr ""
msgid "Epic events"
msgstr ""
msgid "Epics" msgid "Epics"
msgstr "" msgstr ""
...@@ -6435,6 +6438,9 @@ msgstr "" ...@@ -6435,6 +6438,9 @@ msgstr ""
msgid "EventFilterBy|Filter by comments" msgid "EventFilterBy|Filter by comments"
msgstr "" msgstr ""
msgid "EventFilterBy|Filter by epic events"
msgstr ""
msgid "EventFilterBy|Filter by issue events" msgid "EventFilterBy|Filter by issue events"
msgstr "" msgstr ""
......
...@@ -125,7 +125,7 @@ describe GroupsController do ...@@ -125,7 +125,7 @@ describe GroupsController do
end end
context 'as json' do 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 2.times do
project = create(:project, group: group) project = create(:project, group: group)
create(:event, project: project) create(:event, project: project)
......
...@@ -211,7 +211,7 @@ describe 'Edit Project Settings' do ...@@ -211,7 +211,7 @@ describe 'Edit Project Settings' do
visit activity_project_path(project) visit activity_project_path(project)
page.within(".event-filter") do 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("Push events")
expect(page).not_to have_content("Merge events") expect(page).not_to have_content("Merge events")
expect(page).not_to have_content("Comments") expect(page).not_to have_content("Comments")
......
...@@ -3,12 +3,6 @@ ...@@ -3,12 +3,6 @@
require 'spec_helper' require 'spec_helper'
describe EventFilter do 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 describe '#filter' do
it 'returns "all" if given filter is nil' do it 'returns "all" if given filter is nil' do
expect(described_class.new(nil).filter).to eq(described_class::ALL) expect(described_class.new(nil).filter).to eq(described_class::ALL)
......
...@@ -101,20 +101,6 @@ describe Gitlab::Database do ...@@ -101,20 +101,6 @@ describe Gitlab::Database do
end end
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 describe '.replication_slots_supported?' do
it 'returns false when using PostgreSQL 9.3' do it 'returns false when using PostgreSQL 9.3' do
allow(described_class).to receive(:version).and_return('9.3.1') allow(described_class).to receive(:version).and_return('9.3.1')
......
...@@ -4,50 +4,75 @@ require 'spec_helper' ...@@ -4,50 +4,75 @@ require 'spec_helper'
describe EventCollection do describe EventCollection do
describe '#to_a' do describe '#to_a' do
let(:project) { create(:project_empty_repo) } set(:group) { create(:group) }
let(:projects) { Project.where(id: project.id) } set(:project) { create(:project_empty_repo, group: group) }
let(:user) { create(:user) } set(:projects) { Project.where(id: project.id) }
set(:user) { create(:user) }
before do context 'with project events' do
20.times do before do
event = create(:push_event, project: project, author: user) 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 end
create(:closed_issue_event, project: project, author: user) it 'returns an Array of events' do
end events = described_class.new(projects).to_a
it 'returns an Array of events' do expect(events).to be_an_instance_of(Array)
events = described_class.new(projects).to_a end
expect(events).to be_an_instance_of(Array) it 'applies a limit to the number of events' do
end events = described_class.new(projects).to_a
it 'applies a limit to the number of events' do expect(events.length).to eq(20)
events = described_class.new(projects).to_a end
expect(events.length).to eq(20) it 'can paginate through events' do
end events = described_class.new(projects, offset: 20).to_a
it 'can paginate through events' do expect(events.length).to eq(1)
events = described_class.new(projects, offset: 20).to_a end
expect(events.length).to eq(1) it 'returns an empty Array when crossing the maximum page number' do
end events = described_class.new(projects, limit: 1, offset: 15).to_a
it 'returns an empty Array when crossing the maximum page number' do expect(events).to be_empty
events = described_class.new(projects, limit: 1, offset: 15).to_a 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 end
it 'allows filtering of events using an EventFilter' do context 'with group events' do
filter = EventFilter.new(EventFilter::ISSUE) let(:groups) { group.self_and_descendants.public_or_visible_to_user(user) }
events = described_class.new(projects, filter: filter).to_a 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(subject).to eq([event3, event2, event1])
expect(events[0].action).to eq(Event::CLOSED) 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 end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment