Commit 82a0cc88 authored by Alex Kalderimis's avatar Alex Kalderimis

Remove WikiPage#meta method

this was only ever needed in tests, and having it on the actual
wiki-page model is just incurring technical debt and installing a
permanent footgun. It is removed and replaced with factories.

This also refactors the event spec to be much more declarative for
visibility tests.
parent c4fbaa06
......@@ -29,12 +29,6 @@ class WikiPage
alias_method :==, :eql?
def meta
raise 'Metadata only available for valid pages' unless valid?
@meta ||= WikiPage::Meta.find_or_create(slug, self)
end
# Sorts and groups pages by directory.
#
# pages - an array of WikiPage objects.
......@@ -242,7 +236,6 @@ class WikiPage
end
save do
@meta = nil if title_changed?
wiki.update_page(
@page,
content: content,
......
......@@ -27,10 +27,10 @@ FactoryBot.define do
action { Event::CREATED }
transient do
wiki_page { create(:wiki_page, wiki: create(:project_wiki, project: project)) }
wiki_page { create(:wiki_page, project: project) }
end
target { wiki_page.meta }
target { create(:wiki_page_meta, :for_wiki_page, wiki_page: wiki_page) }
end
end
......
......@@ -51,7 +51,14 @@ FactoryBot.define do
wiki_page { create(:wiki_page, project: project) }
end
initialize_with { wiki_page.meta }
project { @overrides[:wiki_page]&.project || create(:project) }
title { wiki_page.title }
initialize_with do
raise 'Metadata only available for valid pages' unless wiki_page.valid?
WikiPage::Meta.find_or_create(wiki_page.slug, wiki_page)
end
end
end
......
......@@ -126,7 +126,7 @@ describe Event do
let(:wiki_page) { create(:wiki_page, title: title, project: project) }
let(:event) { create(:wiki_page_event, project: project, wiki_page: wiki_page) }
it 'delegates to issue title' do
it 'delegates to wiki page title' do
expect(event.target_title).to eq(wiki_page.title)
end
end
......@@ -181,13 +181,16 @@ describe Event do
end
describe '#visible_to_user?' do
let(:project) { create(:project, :public) }
let(:non_member) { create(:user) }
let(:member) { create(:user) }
let(:guest) { create(:user) }
let(:author) { create(:author) }
let(:assignee) { create(:user) }
let(:admin) { create(:admin) }
let_it_be(:non_member) { create(:user) }
let_it_be(:member) { create(:user) }
let_it_be(:guest) { create(:user) }
let_it_be(:author) { create(:author) }
let_it_be(:assignee) { create(:user) }
let_it_be(:admin) { create(:admin) }
let_it_be(:public_project) { create(:project, :public) }
let_it_be(:private_project) { create(:project, :private) }
let(:project) { public_project }
let(:issue) { create(:issue, project: project, author: author, assignees: [assignee]) }
let(:confidential_issue) { create(:issue, :confidential, project: project, author: author, assignees: [assignee]) }
let(:project_snippet) { create(:project_snippet, :public, project: project, author: author) }
......@@ -209,29 +212,66 @@ describe Event do
project.add_guest(guest)
end
def visible_to_all
{
logged_out: true,
non_member: true,
guest: true,
member: true,
admin: true
}
end
def visible_to_none
visible_to_all.transform_values { |_| false }
end
def visible_to_none_except(*roles)
visible_to_none.merge(roles.map { |role| [role, true] }.to_h)
end
def visible_to_all_except(*roles)
visible_to_all.merge(roles.map { |role| [role, false] }.to_h)
end
shared_examples 'visibility examples' do
it 'has the correct visibility' do
expect({
logged_out: event.visible_to_user?(nil),
non_member: event.visible_to_user?(non_member),
guest: event.visible_to_user?(guest),
member: event.visible_to_user?(member),
admin: event.visible_to_user?(admin)
}).to match(visibility)
end
end
shared_examples 'visible to assignee' do |visible|
it { expect(event.visible_to_user?(assignee)).to eq(visible) }
end
shared_examples 'visible to author' do |visible|
it { expect(event.visible_to_user?(author)).to eq(visible) }
end
shared_examples 'visible to assignee and author' do |visible|
include_examples 'visible to assignee', visible
include_examples 'visible to author', visible
end
context 'commit note event' do
let(:project) { create(:project, :public, :repository) }
let(:target) { note_on_commit }
it do
aggregate_failures 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
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
context 'private project' do
let(:project) { create(:project, :private, :repository) }
it do
aggregate_failures 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 false
expect(event.visible_to_user?(admin)).to eq true
end
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:member, :admin) }
end
end
end
......@@ -240,27 +280,19 @@ describe Event do
context 'for non confidential issues' do
let(:target) { issue }
it do
expect(event.visible_to_user?(non_member)).to eq true
expect(event.visible_to_user?(author)).to eq true
expect(event.visible_to_user?(assignee)).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
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
include_examples 'visible to assignee and author', true
end
context 'for confidential issues' do
let(:target) { confidential_issue }
it do
expect(event.visible_to_user?(non_member)).to eq false
expect(event.visible_to_user?(author)).to eq true
expect(event.visible_to_user?(assignee)).to eq true
expect(event.visible_to_user?(member)).to eq true
expect(event.visible_to_user?(guest)).to eq false
expect(event.visible_to_user?(admin)).to eq true
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:member, :admin) }
end
include_examples 'visible to assignee and author', true
end
end
......@@ -268,118 +300,99 @@ describe Event do
context 'on non confidential issues' do
let(:target) { note_on_issue }
it do
expect(event.visible_to_user?(non_member)).to eq true
expect(event.visible_to_user?(author)).to eq true
expect(event.visible_to_user?(assignee)).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
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
include_examples 'visible to assignee and author', true
end
context 'on confidential issues' do
let(:target) { note_on_confidential_issue }
it do
expect(event.visible_to_user?(non_member)).to eq false
expect(event.visible_to_user?(author)).to eq true
expect(event.visible_to_user?(assignee)).to eq true
expect(event.visible_to_user?(member)).to eq true
expect(event.visible_to_user?(guest)).to eq false
expect(event.visible_to_user?(admin)).to eq true
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:member, :admin) }
end
include_examples 'visible to assignee and author', true
end
context 'private project' do
let(:project) { create(:project, :private) }
let(:project) { private_project }
let(:target) { note_on_issue }
it do
expect(event.visible_to_user?(non_member)).to eq false
expect(event.visible_to_user?(author)).to eq false
expect(event.visible_to_user?(assignee)).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
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:guest, :member, :admin) }
end
include_examples 'visible to assignee and author', false
end
end
context 'merge request diff note event' do
let(:project) { create(:project, :public) }
let(:merge_request) { create(:merge_request, source_project: project, author: author, assignees: [assignee]) }
let(:note_on_merge_request) { create(:legacy_diff_note_on_merge_request, noteable: merge_request, project: project) }
let(:target) { note_on_merge_request }
it do
expect(event.visible_to_user?(non_member)).to eq true
expect(event.visible_to_user?(author)).to eq true
expect(event.visible_to_user?(assignee)).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
context 'public project' do
let(:project) { public_project }
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
include_examples 'visible to assignee', true
end
context 'private project' do
let(:project) { create(:project, :private) }
let(:project) { private_project }
it do
expect(event.visible_to_user?(non_member)).to eq false
expect(event.visible_to_user?(author)).to eq false
expect(event.visible_to_user?(assignee)).to eq false
expect(event.visible_to_user?(member)).to eq true
expect(event.visible_to_user?(guest)).to eq false
expect(event.visible_to_user?(admin)).to eq true
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:member, :admin) }
end
include_examples 'visible to assignee', false
end
end
context 'milestone event' do
let(:target) { milestone_on_project }
it do
expect(event.visible_to_user?(nil)).to be_truthy
expect(event.visible_to_user?(non_member)).to be_truthy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
context 'on public project with private issue tracker and merge requests' do
let(:project) { create(:project, :public, :issues_private, :merge_requests_private) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all_except(:logged_out, :non_member) }
end
end
context 'on private project' do
let(:project) { create(:project, :private) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all_except(:logged_out, :non_member) }
end
end
end
context 'wiki-page event', :aggregate_failures do
let(:project) { create(:project) }
let(:event) { create(:wiki_page_event, project: project) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
context 'on private project', :aggregate_failures do
let(:project) { create(:project, :wiki_repo) }
include_examples 'visibility examples' do
let(:visibility) { visible_to_all_except(:logged_out, :non_member) }
end
end
context 'wiki-page event on public project', :aggregate_failures do
let(:project) { create(:project, :public, :wiki_repo) }
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
end
end
......@@ -387,80 +400,56 @@ describe Event do
context 'project snippet note event' do
let(:target) { note_on_project_snippet }
it do
expect(event.visible_to_user?(nil)).to be_truthy
expect(event.visible_to_user?(non_member)).to be_truthy
expect(event.visible_to_user?(author)).to be_truthy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
context 'on public project with private snippets' do
let(:project) { create(:project, :public, :snippets_private) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:guest, :member, :admin) }
end
# Normally, we'd expect the author of a comment to be able to view it.
# However, this doesn't seem to be the case for comments on snippets.
expect(event.visible_to_user?(author)).to be_falsy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
end
include_examples 'visible to author', false
end
context 'on private project' do
let(:project) { create(:project, :private) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:guest, :member, :admin) }
end
# Normally, we'd expect the author of a comment to be able to view it.
# However, this doesn't seem to be the case for comments on snippets.
expect(event.visible_to_user?(author)).to be_falsy
expect(event.visible_to_user?(member)).to be_truthy
expect(event.visible_to_user?(guest)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
end
include_examples 'visible to author', false
end
end
context 'personal snippet note event' do
let(:target) { note_on_personal_snippet }
it do
expect(event.visible_to_user?(nil)).to be_truthy
expect(event.visible_to_user?(non_member)).to be_truthy
expect(event.visible_to_user?(author)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all }
end
include_examples 'visible to author', true
context 'on internal snippet' do
let(:personal_snippet) { create(:personal_snippet, :internal, author: author) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_truthy
expect(event.visible_to_user?(author)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_all_except(:logged_out) }
end
end
context 'on private snippet' do
let(:personal_snippet) { create(:personal_snippet, :private, author: author) }
it do
expect(event.visible_to_user?(nil)).to be_falsy
expect(event.visible_to_user?(non_member)).to be_falsy
expect(event.visible_to_user?(author)).to be_truthy
expect(event.visible_to_user?(admin)).to be_truthy
include_examples 'visibility examples' do
let(:visibility) { visible_to_none_except(:admin) }
end
include_examples 'visible to author', true
end
end
end
......
......@@ -664,30 +664,6 @@ describe WikiPage do
end
end
describe '#meta' do
it 'returns an appropriate metadata object' do
wiki_page = create(:wiki_page)
expect(wiki_page.meta).to have_attributes(
valid?: true,
canonical_slug: wiki_page.slug,
title: wiki_page.title,
project: wiki_page.project,
slugs: include(have_attributes(slug: wiki_page.slug))
)
end
it 'is up-to-date after updates' do
wiki_page = create(:wiki_page)
old_title = wiki_page.title
new_title = FFaker::Lorem.sentence
expect do
wiki_page.update(title: new_title)
end.to change { wiki_page.meta.title }.from(old_title).to(new_title)
end
end
private
def remove_temp_repo(path)
......
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