Commit ba5e8da3 authored by Alex Kalderimis's avatar Alex Kalderimis

Add wiki_events feature flag

This ensures that the methods we use for retrieving events respect the
:wiki_events feature flag.

This flag must be applied in two places - both in the event_collection
and in the events_finder, since there exist places in the code base
where the event_collection is used without the finder (for example in
the projects or the groups controllers) so it must be applied there, but
in the events_finder there is a branch that by-passes the
event_collection, so we also need it in the finder.

To support this flag, we have a new scope `Event.not_wiki_page`, which
only returns events that are not wiki page events.

This flag is off by default.

This includes improvements to the event collection spec by asserting
identity rather than length of results.

Includes suggested changes from reviewer (@.luke)

Co-authored-by: @.luke
parent 29044d94
...@@ -52,10 +52,17 @@ class EventsFinder ...@@ -52,10 +52,17 @@ class EventsFinder
if current_user && scope == 'all' if current_user && scope == 'all'
EventCollection.new(current_user.authorized_projects).all_project_events EventCollection.new(current_user.authorized_projects).all_project_events
else else
source.events # EventCollection is responsible for applying the feature flag
apply_feature_flags(source.events)
end end
end end
def apply_feature_flags(events)
return events if ::Feature.enabled?(:wiki_events)
events.not_wiki_page
end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def by_current_user_access(events) def by_current_user_access(events)
events.merge(Project.public_or_visible_to_user(current_user)) events.merge(Project.public_or_visible_to_user(current_user))
......
...@@ -82,6 +82,7 @@ class Event < ApplicationRecord ...@@ -82,6 +82,7 @@ class Event < ApplicationRecord
scope :code_push, -> { where(action: PUSHED) } scope :code_push, -> { where(action: PUSHED) }
scope :merged, -> { where(action: MERGED) } scope :merged, -> { where(action: MERGED) }
scope :for_wiki_page, -> { where(target_type: WikiPage::Meta.name) } scope :for_wiki_page, -> { where(target_type: WikiPage::Meta.name) }
scope :not_wiki_page, -> { where('target_type IS NULL or target_type <> ?', WikiPage::Meta.name) }
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
......
...@@ -33,16 +33,23 @@ class EventCollection ...@@ -33,16 +33,23 @@ class EventCollection
project_events project_events
end end
relation = apply_feature_flags(relation)
relation = paginate_events(relation) relation = paginate_events(relation)
relation.with_associations.to_a relation.with_associations.to_a
end end
def all_project_events def all_project_events
Event.from_union([project_events]).recent apply_feature_flags(Event.from_union([project_events]).recent)
end end
private private
def apply_feature_flags(events)
return events if ::Feature.enabled?(:wiki_events)
events.not_wiki_page
end
def project_events def project_events
relation_with_join_lateral('project_id', projects) relation_with_join_lateral('project_id', projects)
end end
......
...@@ -23,6 +23,8 @@ class EventFilter ...@@ -23,6 +23,8 @@ class EventFilter
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def apply_filter(events) def apply_filter(events)
events = apply_feature_flags(events)
case filter case filter
when PUSH when PUSH
events.where(action: Event::PUSHED) events.where(action: Event::PUSHED)
...@@ -35,7 +37,7 @@ class EventFilter ...@@ -35,7 +37,7 @@ class EventFilter
when ISSUE when ISSUE
events.where(action: [Event::CREATED, Event::UPDATED, Event::CLOSED, Event::REOPENED], target_type: 'Issue') events.where(action: [Event::CREATED, Event::UPDATED, Event::CLOSED, Event::REOPENED], target_type: 'Issue')
when WIKI when WIKI
events.for_wiki_page wiki_events(events)
else else
events events
end end
...@@ -44,6 +46,18 @@ class EventFilter ...@@ -44,6 +46,18 @@ class EventFilter
private private
def apply_feature_flags(events)
return events.not_wiki_page unless Feature.enabled?(:wiki_events)
events
end
def wiki_events(events)
return events unless Feature.enabled?(:wiki_events)
events.for_wiki_page
end
def filters def filters
[ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM, WIKI] [ALL, PUSH, MERGED, ISSUE, COMMENTS, TEAM, WIKI]
end end
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
describe EventsFinder do describe EventsFinder do
let(:user) { create(:user) } let_it_be(:user) { create(:user) }
let(:other_user) { create(:user) } let(:other_user) { create(:user) }
let(:project1) { create(:project, :private, creator_id: user.id, namespace: user.namespace) } let(:project1) { create(:project, :private, creator_id: user.id, namespace: user.namespace) }
...@@ -20,7 +20,7 @@ describe EventsFinder do ...@@ -20,7 +20,7 @@ describe EventsFinder do
let(:opened_merge_request3) { create(:merge_request, source_project: project1, author: other_user) } let(:opened_merge_request3) { create(:merge_request, source_project: project1, author: other_user) }
let!(:other_developer_event) { create(:event, project: project1, author: other_user, target: opened_merge_request3, action: Event::CREATED) } let!(:other_developer_event) { create(:event, project: project1, author: other_user, target: opened_merge_request3, action: Event::CREATED) }
let(:public_project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) } let_it_be(:public_project) { create(:project, :public, creator_id: user.id, namespace: user.namespace) }
let(:confidential_issue) { create(:closed_issue, confidential: true, project: public_project, author: user) } let(:confidential_issue) { create(:closed_issue, confidential: true, project: public_project, author: user) }
let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: Event::CLOSED) } let!(:confidential_event) { create(:event, project: public_project, author: user, target: confidential_issue, action: Event::CLOSED) }
...@@ -59,6 +59,32 @@ describe EventsFinder do ...@@ -59,6 +59,32 @@ describe EventsFinder do
end end
end end
describe 'wiki events feature flag' do
let_it_be(:events) { create_list(:wiki_page_event, 3, project: public_project) }
subject(:finder) { described_class.new(source: public_project, target_type: 'wiki', current_user: user) }
context 'the wiki_events feature flag is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'omits the wiki page events' do
expect(finder.execute).to be_empty
end
end
context 'the wiki_events feature flag is enabled' do
before do
stub_feature_flags(wiki_events: true)
end
it 'can find the wiki events' do
expect(finder.execute).to match_array(events)
end
end
end
context 'dashboard events' do context 'dashboard events' do
before do before do
project1.add_developer(other_user) project1.add_developer(other_user)
......
...@@ -79,6 +79,16 @@ describe EventFilter do ...@@ -79,6 +79,16 @@ describe EventFilter do
it 'returns all events' do it 'returns all events' do
expect(filtered_events).to eq(Event.all) expect(filtered_events).to eq(Event.all)
end end
context 'the :wiki_events filter is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'does not return wiki events' do
expect(filtered_events).to eq(Event.not_wiki_page)
end
end
end end
context 'with the "wiki" filter' do context 'with the "wiki" filter' do
...@@ -87,6 +97,16 @@ describe EventFilter do ...@@ -87,6 +97,16 @@ describe EventFilter do
it 'returns only wiki page events' do it 'returns only wiki page events' do
expect(filtered_events).to contain_exactly(wiki_page_event, wiki_page_update_event) expect(filtered_events).to contain_exactly(wiki_page_event, wiki_page_update_event)
end end
context 'the :wiki_events filter is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'does not return wiki events' do
expect(filtered_events).not_to include(wiki_page_event, wiki_page_update_event)
end
end
end end
context 'with an unknown filter' do context 'with an unknown filter' do
...@@ -95,6 +115,16 @@ describe EventFilter do ...@@ -95,6 +115,16 @@ describe EventFilter do
it 'returns all events' do it 'returns all events' do
expect(filtered_events).to eq(Event.all) expect(filtered_events).to eq(Event.all)
end end
context 'the :wiki_events filter is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'does not return wiki events' do
expect(filtered_events).to eq(Event.not_wiki_page)
end
end
end end
context 'with a nil filter' do context 'with a nil filter' do
...@@ -103,6 +133,16 @@ describe EventFilter do ...@@ -103,6 +133,16 @@ describe EventFilter do
it 'returns all events' do it 'returns all events' do
expect(filtered_events).to eq(Event.all) expect(filtered_events).to eq(Event.all)
end end
context 'the :wiki_events filter is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'does not return wiki events' do
expect(filtered_events).to eq(Event.not_wiki_page)
end
end
end end
end end
......
...@@ -8,22 +8,68 @@ describe EventCollection do ...@@ -8,22 +8,68 @@ describe EventCollection do
let_it_be(:project) { create(:project_empty_repo, group: group) } let_it_be(:project) { create(:project_empty_repo, group: group) }
let_it_be(:projects) { Project.where(id: project.id) } let_it_be(:projects) { Project.where(id: project.id) }
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:merge_request) { create(:merge_request) }
context 'with project events' do context 'with project events' do
before do let_it_be(:push_event_payloads) do
20.times do Array.new(9) do
event = create(:push_event, project: project, author: user) create(:push_event_payload,
event: create(:push_event, project: project, author: user))
create(:push_event_payload, event: event)
end end
create(:closed_issue_event, project: project, author: user)
end end
it 'returns an Array of events' do let_it_be(:merge_request_events) { create_list(:event, 10, :commented, project: project, target: merge_request) }
let_it_be(:closed_issue_event) { create(:closed_issue_event, project: project, author: user) }
let_it_be(:wiki_page_event) { create(:wiki_page_event, project: project) }
let(:push_events) { push_event_payloads.map(&:event) }
it 'returns an Array of events', :aggregate_failures do
most_recent_20_events = [
wiki_page_event,
closed_issue_event,
*push_events,
*merge_request_events
].sort_by(&:id).reverse.take(20)
events = described_class.new(projects).to_a events = described_class.new(projects).to_a
expect(events).to be_an_instance_of(Array) expect(events).to be_an_instance_of(Array)
expect(events).to match_array(most_recent_20_events)
end
context 'the wiki_events feature flag is disabled' do
before do
stub_feature_flags(wiki_events: false)
end
it 'omits the wiki page events when using to_a' do
events = described_class.new(projects).to_a
expect(events).not_to include(wiki_page_event)
end
it 'omits the wiki page events when using all_project_events' do
events = described_class.new(projects).all_project_events
expect(events).not_to include(wiki_page_event)
end
end
context 'the wiki_events feature flag is enabled' do
before do
stub_feature_flags(wiki_events: true)
end
it 'includes the wiki page events when using to_a' do
events = described_class.new(projects).to_a
expect(events).to include(wiki_page_event)
end
it 'includes the wiki page events when using all_project_events' do
events = described_class.new(projects).all_project_events
expect(events).to include(wiki_page_event)
end
end end
it 'applies a limit to the number of events' do it 'applies a limit to the number of events' do
...@@ -44,12 +90,25 @@ describe EventCollection do ...@@ -44,12 +90,25 @@ describe EventCollection do
expect(events).to be_empty expect(events).to be_empty
end end
it 'allows filtering of events using an EventFilter' do it 'allows filtering of events using an EventFilter, returning single item' do
filter = EventFilter.new(EventFilter::ISSUE) filter = EventFilter.new(EventFilter::ISSUE)
events = described_class.new(projects, filter: filter).to_a events = described_class.new(projects, filter: filter).to_a
expect(events.length).to eq(1) expect(events).to contain_exactly(closed_issue_event)
expect(events[0].action).to eq(Event::CLOSED) end
it 'allows filtering of events using an EventFilter, returning several items' do
filter = EventFilter.new(EventFilter::COMMENTS)
events = described_class.new(projects, filter: filter).to_a
expect(events).to match_array(merge_request_events)
end
it 'allows filtering of events using an EventFilter, returning pushes' do
filter = EventFilter.new(EventFilter::PUSH)
events = described_class.new(projects, filter: filter).to_a
expect(events).to match_array(push_events)
end end
end end
......
...@@ -454,9 +454,10 @@ describe Event do ...@@ -454,9 +454,10 @@ describe Event do
end end
end end
describe '.for_wiki_page' do describe 'wiki_page predicate scopes' do
let_it_be(:events) do let_it_be(:events) do
[ [
create(:push_event),
create(:closed_issue_event), create(:closed_issue_event),
create(:wiki_page_event), create(:wiki_page_event),
create(:closed_issue_event), create(:closed_issue_event),
...@@ -465,10 +466,20 @@ describe Event do ...@@ -465,10 +466,20 @@ describe Event do
] ]
end end
it 'only contains the wiki page events' do describe '.for_wiki_page' do
wiki_events = events.select(&:wiki_page?) it 'only contains the wiki page events' do
wiki_events = events.select(&:wiki_page?)
expect(described_class.for_wiki_page).to match_array(wiki_events) expect(described_class.for_wiki_page).to match_array(wiki_events)
end
end
describe '.not_wiki_page' do
it 'does not contain the wiki page events' do
non_wiki_events = events.reject(&:wiki_page?)
expect(described_class.not_wiki_page).to match_array(non_wiki_events)
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