diff options
author | Robert Speicher <robert@gitlab.com> | 2017-08-07 19:22:41 +0000 |
---|---|---|
committer | Robert Speicher <robert@gitlab.com> | 2017-08-07 19:22:41 +0000 |
commit | 4e2b630df716174ea9b07319a0fd5eda7b60cf8e (patch) | |
tree | c3d1a509818b86bc8bbc96cb84bae8113cfe7868 /spec | |
parent | bc648ae5d225b3b76cc3105681e2f404a5918f85 (diff) | |
parent | df00ebded67e871dfab226db3ffadc3dd6807d79 (diff) | |
download | gitlab-ce-4e2b630df716174ea9b07319a0fd5eda7b60cf8e.tar.gz |
Merge branch 'group-milestone-references-system-notes' into 'master'
Support group milestone references
Closes #34778
See merge request !13289
Diffstat (limited to 'spec')
-rw-r--r-- | spec/fixtures/markdown.md.erb | 5 | ||||
-rw-r--r-- | spec/helpers/gitlab_routing_helper_spec.rb | 40 | ||||
-rw-r--r-- | spec/helpers/milestones_routing_helper_spec.rb | 46 | ||||
-rw-r--r-- | spec/lib/banzai/filter/milestone_reference_filter_spec.rb | 196 | ||||
-rw-r--r-- | spec/models/milestone_spec.rb | 38 | ||||
-rw-r--r-- | spec/services/projects/autocomplete_service_spec.rb | 27 | ||||
-rw-r--r-- | spec/services/system_note_service_spec.rb | 53 | ||||
-rw-r--r-- | spec/support/issuable_shared_examples.rb | 6 | ||||
-rw-r--r-- | spec/support/markdown_feature.rb | 6 | ||||
-rw-r--r-- | spec/support/matchers/markdown_matchers.rb | 2 |
10 files changed, 286 insertions, 133 deletions
diff --git a/spec/fixtures/markdown.md.erb b/spec/fixtures/markdown.md.erb index 58b43805705..4f46e40ce7a 100644 --- a/spec/fixtures/markdown.md.erb +++ b/spec/fixtures/markdown.md.erb @@ -227,8 +227,11 @@ References should be parseable even inside _<%= merge_request.to_reference %>_ e - Milestone in another project: <%= xmilestone.to_reference(project) %> - Ignored in code: `<%= simple_milestone.to_reference %>` - Ignored in links: [Link to <%= simple_milestone.to_reference %>](#milestone-link) -- Milestone by URL: <%= urls.project_milestone_url(milestone.project, milestone) %> +- Milestone by URL: <%= urls.milestone_url(milestone) %> - Link to milestone by URL: [Milestone](<%= milestone.to_reference %>) +- Group milestone by name: <%= Milestone.reference_prefix %><%= group_milestone.name %> +- Group milestone by name in quotes: <%= group_milestone.to_reference(format: :name) %> +- Group milestone by URL is ignore: <%= urls.milestone_url(group_milestone) %> ### Task Lists diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index 537e457513f..a44b200c5da 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -63,44 +63,4 @@ describe GitlabRoutingHelper do it { expect(resend_invite_group_member_path(group_member)).to eq resend_invite_group_group_member_path(group_member.source, group_member) } end end - - describe '#milestone_path' do - context 'for a group milestone' do - let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) } - - it 'links to the group milestone page' do - expect(milestone_path(milestone)) - .to eq(group_milestone_path(group, milestone)) - end - end - - context 'for a project milestone' do - let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) } - - it 'links to the project milestone page' do - expect(milestone_path(milestone)) - .to eq(project_milestone_path(project, milestone)) - end - end - end - - describe '#milestone_url' do - context 'for a group milestone' do - let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) } - - it 'links to the group milestone page' do - expect(milestone_url(milestone)) - .to eq(group_milestone_url(group, milestone)) - end - end - - context 'for a project milestone' do - let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) } - - it 'links to the project milestone page' do - expect(milestone_url(milestone)) - .to eq(project_milestone_url(project, milestone)) - end - end - end end diff --git a/spec/helpers/milestones_routing_helper_spec.rb b/spec/helpers/milestones_routing_helper_spec.rb new file mode 100644 index 00000000000..dc13a43c2ab --- /dev/null +++ b/spec/helpers/milestones_routing_helper_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +describe MilestonesRoutingHelper do + let(:project) { build_stubbed(:project) } + let(:group) { build_stubbed(:group) } + + describe '#milestone_path' do + context 'for a group milestone' do + let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) } + + it 'links to the group milestone page' do + expect(milestone_path(milestone)) + .to eq(group_milestone_path(group, milestone)) + end + end + + context 'for a project milestone' do + let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) } + + it 'links to the project milestone page' do + expect(milestone_path(milestone)) + .to eq(project_milestone_path(project, milestone)) + end + end + end + + describe '#milestone_url' do + context 'for a group milestone' do + let(:milestone) { build_stubbed(:milestone, group: group, iid: 1) } + + it 'links to the group milestone page' do + expect(milestone_url(milestone)) + .to eq(group_milestone_url(group, milestone)) + end + end + + context 'for a project milestone' do + let(:milestone) { build_stubbed(:milestone, project: project, iid: 1) } + + it 'links to the project milestone page' do + expect(milestone_url(milestone)) + .to eq(project_milestone_url(project, milestone)) + end + end + end +end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 5db77566513..ebd6c79077e 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -3,57 +3,57 @@ require 'spec_helper' describe Banzai::Filter::MilestoneReferenceFilter do include FilterSpecHelper - let(:project) { create(:project, :public) } - let(:milestone) { create(:milestone, project: project) } - let(:reference) { milestone.to_reference } + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } it 'requires project context' do expect { described_class.call('') }.to raise_error(ArgumentError, /:project/) end - %w(pre code a style).each do |elem| - it "ignores valid references contained inside '#{elem}' element" do - exp = act = "<#{elem}>milestone #{milestone.to_reference}</#{elem}>" - expect(reference_filter(act).to_html).to eq exp + shared_examples 'reference parsing' do + %w(pre code a style).each do |elem| + it "ignores valid references contained inside '#{elem}' element" do + exp = act = "<#{elem}>milestone #{reference}</#{elem}>" + expect(reference_filter(act).to_html).to eq exp + end end - end - it 'includes default classes' do - doc = reference_filter("Milestone #{reference}") - expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone has-tooltip' - end + it 'includes default classes' do + doc = reference_filter("Milestone #{reference}") - it 'includes a data-project attribute' do - doc = reference_filter("Milestone #{reference}") - link = doc.css('a').first + expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-milestone has-tooltip' + end - expect(link).to have_attribute('data-project') - expect(link.attr('data-project')).to eq project.id.to_s - end + it 'includes a data-project attribute' do + doc = reference_filter("Milestone #{reference}") + link = doc.css('a').first - it 'includes a data-milestone attribute' do - doc = reference_filter("See #{reference}") - link = doc.css('a').first + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end - expect(link).to have_attribute('data-milestone') - expect(link.attr('data-milestone')).to eq milestone.id.to_s - end + it 'includes a data-milestone attribute' do + doc = reference_filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-milestone') + expect(link.attr('data-milestone')).to eq milestone.id.to_s + end - it 'supports an :only_path context' do - doc = reference_filter("Milestone #{reference}", only_path: true) - link = doc.css('a').first.attr('href') + it 'supports an :only_path context' do + doc = reference_filter("Milestone #{reference}", only_path: true) + link = doc.css('a').first.attr('href') - expect(link).not_to match %r(https?://) - expect(link).to eq urls - .project_milestone_path(project, milestone) + expect(link).not_to match %r(https?://) + expect(link).to eq urls.milestone_path(milestone) + end end - context 'Integer-based references' do + shared_examples 'Integer-based references' do it 'links to a valid reference' do doc = reference_filter("See #{reference}") - expect(doc.css('a').first.attr('href')).to eq urls - .project_milestone_url(project, milestone) + expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) end it 'links with adjacent text' do @@ -68,15 +68,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - context 'String-based single-word references' do - let(:milestone) { create(:milestone, name: 'gfm', project: project) } + shared_examples 'String-based single-word references' do let(:reference) { "#{Milestone.reference_prefix}#{milestone.name}" } + before do + milestone.update!(name: 'gfm') + end + it 'links to a valid reference' do doc = reference_filter("See #{reference}") - expect(doc.css('a').first.attr('href')).to eq urls - .project_milestone_url(project, milestone) + expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) expect(doc.text).to eq 'See gfm' end @@ -92,15 +94,17 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - context 'String-based multi-word references in quotes' do - let(:milestone) { create(:milestone, name: 'gfm references', project: project) } + shared_examples 'String-based multi-word references in quotes' do let(:reference) { milestone.to_reference(format: :name) } + before do + milestone.update!(name: 'gfm references') + end + it 'links to a valid reference' do doc = reference_filter("See #{reference}") - expect(doc.css('a').first.attr('href')).to eq urls - .project_milestone_url(project, milestone) + expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) expect(doc.text).to eq 'See gfm references' end @@ -116,23 +120,27 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - describe 'referencing a milestone in a link href' do - let(:reference) { %Q{<a href="#{milestone.to_reference}">Milestone</a>} } + shared_examples 'referencing a milestone in a link href' do + let(:unquoted_reference) { "#{Milestone.reference_prefix}#{milestone.name}" } + let(:link_reference) { %Q{<a href="#{unquoted_reference}">Milestone</a>} } + + before do + milestone.update!(name: 'gfm') + end it 'links to a valid reference' do - doc = reference_filter("See #{reference}") + doc = reference_filter("See #{link_reference}") - expect(doc.css('a').first.attr('href')).to eq urls - .project_milestone_url(project, milestone) + expect(doc.css('a').first.attr('href')).to eq urls.milestone_url(milestone) end it 'links with adjacent text' do - doc = reference_filter("Milestone (#{reference}.)") + doc = reference_filter("Milestone (#{link_reference}.)") expect(doc.to_html).to match(%r(\(<a.+>Milestone</a>\.\))) end it 'includes a data-project attribute' do - doc = reference_filter("Milestone #{reference}") + doc = reference_filter("Milestone #{link_reference}") link = doc.css('a').first expect(link).to have_attribute('data-project') @@ -140,7 +148,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do end it 'includes a data-milestone attribute' do - doc = reference_filter("See #{reference}") + doc = reference_filter("See #{link_reference}") link = doc.css('a').first expect(link).to have_attribute('data-milestone') @@ -148,7 +156,35 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - describe 'cross-project / cross-namespace complete reference' do + shared_examples 'linking to a milestone as the entire link' do + let(:unquoted_reference) { "#{Milestone.reference_prefix}#{milestone.name}" } + let(:link) { urls.milestone_url(milestone) } + let(:link_reference) { %Q{<a href="#{link}">#{link}</a>} } + + it 'replaces the link text with the milestone reference' do + doc = reference_filter("See #{link}") + + expect(doc.css('a').first.text).to eq(unquoted_reference) + end + + it 'includes a data-project attribute' do + doc = reference_filter("Milestone #{link_reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-project') + expect(link.attr('data-project')).to eq project.id.to_s + end + + it 'includes a data-milestone attribute' do + doc = reference_filter("See #{link_reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-milestone') + expect(link.attr('data-milestone')).to eq milestone.id.to_s + end + end + + shared_examples 'cross-project / cross-namespace complete reference' do let(:namespace) { create(:namespace) } let(:another_project) { create(:project, :public, namespace: namespace) } let(:milestone) { create(:milestone, project: another_project) } @@ -184,7 +220,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - describe 'cross-project / same-namespace complete reference' do + shared_examples 'cross-project / same-namespace complete reference' do let(:namespace) { create(:namespace) } let(:project) { create(:project, :public, namespace: namespace) } let(:another_project) { create(:project, :public, namespace: namespace) } @@ -221,7 +257,7 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - describe 'cross project shorthand reference' do + shared_examples 'cross project shorthand reference' do let(:namespace) { create(:namespace) } let(:project) { create(:project, :public, namespace: namespace) } let(:another_project) { create(:project, :public, namespace: namespace) } @@ -258,27 +294,53 @@ describe Banzai::Filter::MilestoneReferenceFilter do end end - describe 'cross project milestone references' do - let(:another_project) { create(:project, :public) } - let(:project_path) { another_project.full_path } - let(:milestone) { create(:milestone, project: another_project) } - let(:reference) { milestone.to_reference(project) } + context 'project milestones' do + let(:milestone) { create(:milestone, project: project) } + let(:reference) { milestone.to_reference } - let!(:result) { reference_filter("See #{reference}") } + include_examples 'reference parsing' - it 'points to referenced project milestone page' do - expect(result.css('a').first.attr('href')).to eq urls - .project_milestone_url(another_project, milestone) + it_behaves_like 'Integer-based references' + it_behaves_like 'String-based single-word references' + it_behaves_like 'String-based multi-word references in quotes' + it_behaves_like 'referencing a milestone in a link href' + it_behaves_like 'cross-project / cross-namespace complete reference' + it_behaves_like 'cross-project / same-namespace complete reference' + it_behaves_like 'cross project shorthand reference' + end + + context 'group milestones' do + let(:milestone) { create(:milestone, group: group) } + let(:reference) { milestone.to_reference(format: :name) } + + include_examples 'reference parsing' + + it_behaves_like 'String-based single-word references' + it_behaves_like 'String-based multi-word references in quotes' + it_behaves_like 'referencing a milestone in a link href' + + it 'does not support references by IID' do + doc = reference_filter("See #{Milestone.reference_prefix}#{milestone.iid}") + + expect(doc.css('a')).to be_empty end - it 'contains cross project content' do - expect(result.css('a').first.text).to eq "#{milestone.name} in #{project_path}" + it 'does not support references by link' do + doc = reference_filter("See #{urls.milestone_url(milestone)}") + + expect(doc.css('a').first.text).to eq(urls.milestone_url(milestone)) end - it 'escapes the name attribute' do - allow_any_instance_of(Milestone).to receive(:title).and_return(%{"></a>whatever<a title="}) - doc = reference_filter("See #{reference}") - expect(doc.css('a').first.text).to eq "#{milestone.name} in #{project_path}" + it 'does not support cross-project references' do + another_group = create(:group) + another_project = create(:project, :public, group: group) + project_reference = another_project.to_reference(project) + + milestone.update!(group: another_group) + + doc = reference_filter("See #{project_reference}#{reference}") + + expect(doc.css('a')).to be_empty end end end diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index b48aa9558d5..d3da0107d5c 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -230,16 +230,40 @@ describe Milestone do end describe '#to_reference' do - let(:project) { build(:project, name: 'sample-project') } - let(:milestone) { build(:milestone, iid: 1, project: project) } + let(:group) { build_stubbed(:group) } + let(:project) { build_stubbed(:project, name: 'sample-project') } + let(:another_project) { build_stubbed(:project, name: 'another-project', namespace: project.namespace) } + + context 'for a project milestone' do + let(:milestone) { build_stubbed(:milestone, iid: 1, project: project, name: 'milestone') } + + it 'returns a String reference to the object' do + expect(milestone.to_reference).to eq '%1' + end + + it 'returns a reference by name when the format is set to :name' do + expect(milestone.to_reference(format: :name)).to eq '%"milestone"' + end - it 'returns a String reference to the object' do - expect(milestone.to_reference).to eq "%1" + it 'supports a cross-project reference' do + expect(milestone.to_reference(another_project)).to eq 'sample-project%1' + end end - it 'supports a cross-project reference' do - another_project = build(:project, name: 'another-project', namespace: project.namespace) - expect(milestone.to_reference(another_project)).to eq "sample-project%1" + context 'for a group milestone' do + let(:milestone) { build_stubbed(:milestone, iid: 1, group: group, name: 'milestone') } + + it 'returns nil with the default format' do + expect(milestone.to_reference).to be_nil + end + + it 'returns a reference by name when the format is set to :name' do + expect(milestone.to_reference(format: :name)).to eq '%"milestone"' + end + + it 'does not supports cross-project references' do + expect(milestone.to_reference(another_project, format: :name)).to eq '%"milestone"' + end end end diff --git a/spec/services/projects/autocomplete_service_spec.rb b/spec/services/projects/autocomplete_service_spec.rb index c1f098530bf..426593be428 100644 --- a/spec/services/projects/autocomplete_service_spec.rb +++ b/spec/services/projects/autocomplete_service_spec.rb @@ -88,4 +88,31 @@ describe Projects::AutocompleteService do end end end + + describe '#milestones' do + let(:user) { create(:user) } + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } + let!(:group_milestone) { create(:milestone, group: group) } + let!(:project_milestone) { create(:milestone, project: project) } + + let(:milestone_titles) { described_class.new(project, user).milestones.map(&:title) } + + it 'includes project and group milestones' do + expect(milestone_titles).to eq([group_milestone.title, project_milestone.title]) + end + + it 'does not include closed milestones' do + group_milestone.close + + expect(milestone_titles).to eq([project_milestone.title]) + end + + it 'does not include milestones from other projects in the group' do + other_project = create(:project, group: group) + project_milestone.update!(project: other_project) + + expect(milestone_titles).to eq([group_milestone.title]) + end + end end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index e3805160b04..8f1eb4863d9 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' describe SystemNoteService do include Gitlab::Routing - let(:project) { create(:project) } + let(:group) { create(:group) } + let(:project) { create(:project, group: group) } let(:author) { create(:user) } let(:noteable) { create(:issue, project: project) } let(:issue) { noteable } @@ -242,25 +243,51 @@ describe SystemNoteService do end describe '.change_milestone' do - subject { described_class.change_milestone(noteable, project, author, milestone) } + context 'for a project milestone' do + subject { described_class.change_milestone(noteable, project, author, milestone) } - let(:milestone) { create(:milestone, project: project) } + let(:milestone) { create(:milestone, project: project) } - it_behaves_like 'a system note' do - let(:action) { 'milestone' } - end + it_behaves_like 'a system note' do + let(:action) { 'milestone' } + end - context 'when milestone added' do - it 'sets the note text' do - expect(subject.note).to eq "changed milestone to #{milestone.to_reference}" + context 'when milestone added' do + it 'sets the note text' do + expect(subject.note).to eq "changed milestone to #{milestone.to_reference}" + end + end + + context 'when milestone removed' do + let(:milestone) { nil } + + it 'sets the note text' do + expect(subject.note).to eq 'removed milestone' + end end end - context 'when milestone removed' do - let(:milestone) { nil } + context 'for a group milestone' do + subject { described_class.change_milestone(noteable, project, author, milestone) } - it 'sets the note text' do - expect(subject.note).to eq 'removed milestone' + let(:milestone) { create(:milestone, group: group) } + + it_behaves_like 'a system note' do + let(:action) { 'milestone' } + end + + context 'when milestone added' do + it 'sets the note text to use the milestone name' do + expect(subject.note).to eq "changed milestone to #{milestone.to_reference(format: :name)}" + end + end + + context 'when milestone removed' do + let(:milestone) { nil } + + it 'sets the note text' do + expect(subject.note).to eq 'removed milestone' + end end end end diff --git a/spec/support/issuable_shared_examples.rb b/spec/support/issuable_shared_examples.rb index 970fe10db2b..42f3b4db23c 100644 --- a/spec/support/issuable_shared_examples.rb +++ b/spec/support/issuable_shared_examples.rb @@ -21,15 +21,15 @@ shared_examples 'system notes for milestones' do create(:group_member, group: group, user: user) end - it 'does not create system note' do + it 'creates a system note' do expect do update_issuable(milestone: group_milestone) - end.not_to change { Note.system.count } + end.to change { Note.system.count }.by(1) end end context 'project milestones' do - it 'creates system note' do + it 'creates a system note' do expect do update_issuable(milestone: create(:milestone)) end.to change { Note.system.count }.by(1) diff --git a/spec/support/markdown_feature.rb b/spec/support/markdown_feature.rb index 21a054af4e1..c90359d7cfa 100644 --- a/spec/support/markdown_feature.rb +++ b/spec/support/markdown_feature.rb @@ -23,7 +23,7 @@ class MarkdownFeature # Direct references ---------------------------------------------------------- def project - @project ||= create(:project, :repository).tap do |project| + @project ||= create(:project, :repository, group: group).tap do |project| project.team << [user, :master] end end @@ -75,6 +75,10 @@ class MarkdownFeature @milestone ||= create(:milestone, name: 'next goal', project: project) end + def group_milestone + @group_milestone ||= create(:milestone, name: 'group-milestone', group: group) + end + # Cross-references ----------------------------------------------------------- def xproject diff --git a/spec/support/matchers/markdown_matchers.rb b/spec/support/matchers/markdown_matchers.rb index 7afa57fb76b..d12b2757427 100644 --- a/spec/support/matchers/markdown_matchers.rb +++ b/spec/support/matchers/markdown_matchers.rb @@ -155,7 +155,7 @@ module MarkdownMatchers set_default_markdown_messages match do |actual| - expect(actual).to have_selector('a.gfm.gfm-milestone', count: 6) + expect(actual).to have_selector('a.gfm.gfm-milestone', count: 8) end end |