diff options
Diffstat (limited to 'spec/models/concerns')
-rw-r--r-- | spec/models/concerns/awardable_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/concerns/cache_markdown_field_spec.rb | 280 | ||||
-rw-r--r-- | spec/models/concerns/discussion_on_diff_spec.rb | 24 | ||||
-rw-r--r-- | spec/models/concerns/has_status_spec.rb | 12 | ||||
-rw-r--r-- | spec/models/concerns/ignorable_column_spec.rb | 38 | ||||
-rw-r--r-- | spec/models/concerns/issuable_spec.rb | 144 | ||||
-rw-r--r-- | spec/models/concerns/mentionable_spec.rb | 49 | ||||
-rw-r--r-- | spec/models/concerns/milestoneish_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/concerns/noteable_spec.rb | 261 | ||||
-rw-r--r-- | spec/models/concerns/relative_positioning_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/resolvable_discussion_spec.rb | 548 | ||||
-rw-r--r-- | spec/models/concerns/resolvable_note_spec.rb | 329 | ||||
-rw-r--r-- | spec/models/concerns/routable_spec.rb | 179 | ||||
-rw-r--r-- | spec/models/concerns/spammable_spec.rb | 4 | ||||
-rw-r--r-- | spec/models/concerns/strip_attribute_spec.rb | 2 |
15 files changed, 1683 insertions, 199 deletions
diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index de791abdf3d..63ad3a3630b 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -1,10 +1,12 @@ require 'spec_helper' -describe Issue, "Awardable" do +describe Awardable do let!(:issue) { create(:issue) } let!(:award_emoji) { create(:award_emoji, :downvote, awardable: issue) } describe "Associations" do + subject { build(:issue) } + it { is_expected.to have_many(:award_emoji).dependent(:destroy) } end diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 6151d53cd91..40bbb10eaac 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -1,9 +1,6 @@ require 'spec_helper' describe CacheMarkdownField do - caching_classes = CacheMarkdownField::CACHING_CLASSES - CacheMarkdownField::CACHING_CLASSES = ["ThingWithMarkdownFields"].freeze - # The minimum necessary ActiveModel to test this concern class ThingWithMarkdownFields include ActiveModel::Model @@ -21,24 +18,25 @@ describe CacheMarkdownField do end extend ActiveModel::Callbacks - define_model_callbacks :save + define_model_callbacks :create, :update include CacheMarkdownField cache_markdown_field :foo cache_markdown_field :baz, pipeline: :single_line - def self.add_attr(attr_name) - self.attribute_names += [attr_name] - define_attribute_methods(attr_name) - attr_reader(attr_name) - define_method("#{attr_name}=") do |val| - send("#{attr_name}_will_change!") unless val == send(attr_name) - instance_variable_set("@#{attr_name}", val) + def self.add_attr(name) + self.attribute_names += [name] + define_attribute_methods(name) + attr_reader(name) + define_method("#{name}=") do |value| + write_attribute(name, value) end end - [:foo, :foo_html, :bar, :baz, :baz_html].each do |attr_name| - add_attr(attr_name) + add_attr :cached_markdown_version + + [:foo, :foo_html, :bar, :baz, :baz_html].each do |name| + add_attr(name) end def initialize(*) @@ -48,134 +46,258 @@ describe CacheMarkdownField do clear_changes_information end + def read_attribute(name) + instance_variable_get("@#{name}") + end + + def write_attribute(name, value) + send("#{name}_will_change!") unless value == read_attribute(name) + instance_variable_set("@#{name}", value) + end + def save - run_callbacks :save do + run_callbacks :update do changes_applied end end end - CacheMarkdownField::CACHING_CLASSES = caching_classes - def thing_subclass(new_attr) Class.new(ThingWithMarkdownFields) { add_attr(new_attr) } end - let(:markdown) { "`Foo`" } - let(:html) { "<p><code>Foo</code></p>" } + let(:markdown) { '`Foo`' } + let(:html) { '<p dir="auto"><code>Foo</code></p>' } - let(:updated_markdown) { "`Bar`" } - let(:updated_html) { "<p dir=\"auto\"><code>Bar</code></p>" } + let(:updated_markdown) { '`Bar`' } + let(:updated_html) { '<p dir="auto"><code>Bar</code></p>' } - subject { ThingWithMarkdownFields.new(foo: markdown, foo_html: html) } + let(:thing) { ThingWithMarkdownFields.new(foo: markdown, foo_html: html, cached_markdown_version: CacheMarkdownField::CACHE_VERSION) } - describe ".attributes" do - it "excludes cache attributes" do - expect(thing_subclass(:qux).new.attributes.keys.sort).to eq(%w[bar baz foo qux]) + describe '.attributes' do + it 'excludes cache attributes' do + expect(thing.attributes.keys.sort).to eq(%w[bar baz foo]) end end - describe ".cache_markdown_field" do - it "refuses to allow untracked classes" do - expect { thing_subclass(:qux).__send__(:cache_markdown_field, :qux) }.to raise_error(RuntimeError) + context 'an unchanged markdown field' do + before do + thing.foo = thing.foo + thing.save end + + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.foo_html_changed?).not_to be_truthy } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end - context "an unchanged markdown field" do + context 'a changed markdown field' do before do - subject.foo = subject.foo - subject.save + thing.foo = updated_markdown + thing.save end - it { expect(subject.foo).to eq(markdown) } - it { expect(subject.foo_html).to eq(html) } - it { expect(subject.foo_html_changed?).not_to be_truthy } + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end - context "a changed markdown field" do + context 'a non-markdown field changed' do before do - subject.foo = updated_markdown - subject.save + thing.bar = 'OK' + thing.save end - it { expect(subject.foo_html).to eq(updated_html) } + it { expect(thing.bar).to eq('OK') } + it { expect(thing.foo).to eq(markdown) } + it { expect(thing.foo_html).to eq(html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } end - context "a non-markdown field changed" do + context 'version is out of date' do + let(:thing) { ThingWithMarkdownFields.new(foo: updated_markdown, foo_html: html, cached_markdown_version: nil) } + before do - subject.bar = "OK" - subject.save + thing.save end - it { expect(subject.bar).to eq("OK") } - it { expect(subject.foo).to eq(markdown) } - it { expect(subject.foo_html).to eq(html) } + it { expect(thing.foo_html).to eq(updated_html) } + it { expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) } + end + + describe '#cached_html_up_to_date?' do + subject { thing.cached_html_up_to_date?(:foo) } + + it 'returns false when the version is absent' do + thing.cached_markdown_version = nil + + is_expected.to be_falsy + end + + it 'returns false when the version is too early' do + thing.cached_markdown_version -= 1 + + is_expected.to be_falsy + end + + it 'returns false when the version is too late' do + thing.cached_markdown_version += 1 + + is_expected.to be_falsy + end + + it 'returns true when the version is just right' do + thing.cached_markdown_version = CacheMarkdownField::CACHE_VERSION + + is_expected.to be_truthy + end + + it 'returns false if markdown has been changed but html has not' do + thing.foo = updated_html + + is_expected.to be_falsy + end + + it 'returns true if markdown has not been changed but html has' do + thing.foo_html = updated_html + + is_expected.to be_truthy + end + + it 'returns true if markdown and html have both been changed' do + thing.foo = updated_markdown + thing.foo_html = updated_html + + is_expected.to be_truthy + end + + it 'returns false if the markdown field is set but the html is not' do + thing.foo_html = nil + + is_expected.to be_falsy + end + end + + describe '#refresh_markdown_cache!' do + before do + thing.foo = updated_markdown + end + + context 'do_update: false' do + it 'fills all html fields' do + thing.refresh_markdown_cache! + + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end + + it 'does not save the result' do + expect(thing).not_to receive(:update_columns) + + thing.refresh_markdown_cache! + end + + it 'updates the markdown cache version' do + thing.cached_markdown_version = nil + thing.refresh_markdown_cache! + + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) + end + end + + context 'do_update: true' do + it 'fills all html fields' do + thing.refresh_markdown_cache!(do_update: true) + + expect(thing.foo_html).to eq(updated_html) + expect(thing.foo_html_changed?).to be_truthy + expect(thing.baz_html_changed?).to be_truthy + end + + it 'skips saving if not persisted' do + expect(thing).to receive(:persisted?).and_return(false) + expect(thing).not_to receive(:update_columns) + + thing.refresh_markdown_cache!(do_update: true) + end + + it 'saves the changes using #update_columns' do + expect(thing).to receive(:persisted?).and_return(true) + expect(thing).to receive(:update_columns) + .with("foo_html" => updated_html, "baz_html" => "", "cached_markdown_version" => CacheMarkdownField::CACHE_VERSION) + + thing.refresh_markdown_cache!(do_update: true) + end + end end describe '#banzai_render_context' do - it "sets project to nil if the object lacks a project" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:project) + subject(:context) { thing.banzai_render_context(:foo) } + + it 'sets project to nil if the object lacks a project' do + is_expected.to have_key(:project) expect(context[:project]).to be_nil end - it "excludes author if the object lacks an author" do - context = subject.banzai_render_context(:foo) - expect(context).not_to have_key(:author) + it 'excludes author if the object lacks an author' do + is_expected.not_to have_key(:author) end - it "raises if the context for an unrecognised field is requested" do - expect{subject.banzai_render_context(:not_found)}.to raise_error(ArgumentError) + it 'raises if the context for an unrecognised field is requested' do + expect { thing.banzai_render_context(:not_found) }.to raise_error(ArgumentError) end - it "includes the pipeline" do - context = subject.banzai_render_context(:baz) - expect(context[:pipeline]).to eq(:single_line) + it 'includes the pipeline' do + baz = thing.banzai_render_context(:baz) + + expect(baz[:pipeline]).to eq(:single_line) end - it "returns copies of the context template" do - template = subject.cached_markdown_fields[:baz] - copy = subject.banzai_render_context(:baz) + it 'returns copies of the context template' do + template = thing.cached_markdown_fields[:baz] + copy = thing.banzai_render_context(:baz) + expect(copy).not_to be(template) end - context "with a project" do - subject { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project) } + context 'with a project' do + let(:thing) { thing_subclass(:project).new(foo: markdown, foo_html: html, project: :project_value) } - it "sets the project in the context" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:project) - expect(context[:project]).to eq(:project) + it 'sets the project in the context' do + is_expected.to have_key(:project) + expect(context[:project]).to eq(:project_value) end - it "invalidates the cache when project changes" do - subject.project = :new_project + it 'invalidates the cache when project changes' do + thing.project = :new_project allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) - subject.save + thing.save - expect(subject.foo_html).to eq(updated_html) - expect(subject.baz_html).to eq(updated_html) + expect(thing.foo_html).to eq(updated_html) + expect(thing.baz_html).to eq(updated_html) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) end end - context "with an author" do - subject { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author) } + context 'with an author' do + let(:thing) { thing_subclass(:author).new(foo: markdown, foo_html: html, author: :author_value) } - it "sets the author in the context" do - context = subject.banzai_render_context(:foo) - expect(context).to have_key(:author) - expect(context[:author]).to eq(:author) + it 'sets the author in the context' do + is_expected.to have_key(:author) + expect(context[:author]).to eq(:author_value) end - it "invalidates the cache when author changes" do - subject.author = :new_author + it 'invalidates the cache when author changes' do + thing.author = :new_author allow(Banzai::Renderer).to receive(:cacheless_render_field).and_return(updated_html) - subject.save + thing.save - expect(subject.foo_html).to eq(updated_html) - expect(subject.baz_html).to eq(updated_html) + expect(thing.foo_html).to eq(updated_html) + expect(thing.baz_html).to eq(updated_html) + expect(thing.cached_markdown_version).to eq(CacheMarkdownField::CACHE_VERSION) end end end diff --git a/spec/models/concerns/discussion_on_diff_spec.rb b/spec/models/concerns/discussion_on_diff_spec.rb new file mode 100644 index 00000000000..8571e85627c --- /dev/null +++ b/spec/models/concerns/discussion_on_diff_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe DiscussionOnDiff, model: true do + subject { create(:diff_note_on_merge_request).to_discussion } + + describe "#truncated_diff_lines" do + let(:truncated_lines) { subject.truncated_diff_lines } + + context "when diff is greater than allowed number of truncated diff lines " do + it "returns fewer lines" do + expect(subject.diff_lines.count).to be > DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + + expect(truncated_lines.count).to be <= DiffDiscussion::NUMBER_OF_TRUNCATED_DIFF_LINES + end + end + + context "when some diff lines are meta" do + it "returns no meta lines" do + expect(subject.diff_lines).to include(be_meta) + expect(truncated_lines).not_to include(be_meta) + end + end + end +end diff --git a/spec/models/concerns/has_status_spec.rb b/spec/models/concerns/has_status_spec.rb index 82abad0e2f6..67dae7cf4c0 100644 --- a/spec/models/concerns/has_status_spec.rb +++ b/spec/models/concerns/has_status_spec.rb @@ -231,6 +231,18 @@ describe HasStatus do end end + describe '.created_or_pending' do + subject { CommitStatus.created_or_pending } + + %i[created pending].each do |status| + it_behaves_like 'containing the job', status + end + + %i[running failed success].each do |status| + it_behaves_like 'not containing the job', status + end + end + describe '.finished' do subject { CommitStatus.finished } diff --git a/spec/models/concerns/ignorable_column_spec.rb b/spec/models/concerns/ignorable_column_spec.rb new file mode 100644 index 00000000000..dba9fe43327 --- /dev/null +++ b/spec/models/concerns/ignorable_column_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' + +describe IgnorableColumn do + let :base_class do + Class.new do + def self.columns + # This method does not have access to "double" + [Struct.new(:name).new('id'), Struct.new(:name).new('title')] + end + end + end + + let :model do + Class.new(base_class) do + include IgnorableColumn + end + end + + describe '.columns' do + it 'returns the columns, excluding the ignored ones' do + model.ignore_column(:title) + + expect(model.columns.map(&:name)).to eq(%w(id)) + end + end + + describe '.ignored_columns' do + it 'returns a Set' do + expect(model.ignored_columns).to be_an_instance_of(Set) + end + + it 'returns the names of the ignored columns' do + model.ignore_column(:title) + + expect(model.ignored_columns).to eq(Set.new(%w(title))) + end + end +end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 4522206fab1..27890e33b49 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -1,13 +1,15 @@ require 'spec_helper' -describe Issue, "Issuable" do +describe Issuable do + let(:issuable_class) { Issue } let(:issue) { create(:issue) } let(:user) { create(:user) } describe "Associations" do + subject { build(:issue) } + it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:author) } - it { is_expected.to belong_to(:assignee) } it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:todos).dependent(:destroy) } @@ -23,10 +25,14 @@ describe Issue, "Issuable" do end describe 'Included modules' do + let(:described_class) { issuable_class } + it { is_expected.to include_module(Awardable) } end describe "Validation" do + subject { build(:issue) } + before do allow(subject).to receive(:set_iid).and_return(false) end @@ -39,9 +45,11 @@ describe Issue, "Issuable" do end describe "Scope" do - it { expect(described_class).to respond_to(:opened) } - it { expect(described_class).to respond_to(:closed) } - it { expect(described_class).to respond_to(:assigned) } + subject { build(:issue) } + + it { expect(issuable_class).to respond_to(:opened) } + it { expect(issuable_class).to respond_to(:closed) } + it { expect(issuable_class).to respond_to(:assigned) } end describe 'author_name' do @@ -57,74 +65,20 @@ describe Issue, "Issuable" do end end - describe 'assignee_name' do - it 'is delegated to assignee' do - issue.update!(assignee: create(:user)) - - expect(issue.assignee_name).to eq issue.assignee.name - end - - it 'returns nil when assignee is nil' do - issue.assignee_id = nil - issue.save(validate: false) - - expect(issue.assignee_name).to eq nil - end - end - - describe "before_save" do - describe "#update_cache_counts" do - context "when previous assignee exists" do - before do - assignee = create(:user) - issue.project.team << [assignee, :developer] - issue.update(assignee: assignee) - end - - it "updates cache counts for new assignee" do - user = create(:user) - - expect(user).to receive(:update_cache_counts) - - issue.update(assignee: user) - end - - it "updates cache counts for previous assignee" do - old_assignee = issue.assignee - allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee) - - expect(old_assignee).to receive(:update_cache_counts) - - issue.update(assignee: nil) - end - end - - context "when previous assignee does not exist" do - before{ issue.update(assignee: nil) } - - it "updates cache count for the new assignee" do - expect_any_instance_of(User).to receive(:update_cache_counts) - - issue.update(assignee: user) - end - end - end - end - describe ".search" do let!(:searchable_issue) { create(:issue, title: "Searchable issue") } it 'returns notes with a matching title' do - expect(described_class.search(searchable_issue.title)). + expect(issuable_class.search(searchable_issue.title)). to eq([searchable_issue]) end it 'returns notes with a partially matching title' do - expect(described_class.search('able')).to eq([searchable_issue]) + expect(issuable_class.search('able')).to eq([searchable_issue]) end it 'returns notes with a matching title regardless of the casing' do - expect(described_class.search(searchable_issue.title.upcase)). + expect(issuable_class.search(searchable_issue.title.upcase)). to eq([searchable_issue]) end end @@ -135,31 +89,31 @@ describe Issue, "Issuable" do end it 'returns notes with a matching title' do - expect(described_class.full_search(searchable_issue.title)). + expect(issuable_class.full_search(searchable_issue.title)). to eq([searchable_issue]) end it 'returns notes with a partially matching title' do - expect(described_class.full_search('able')).to eq([searchable_issue]) + expect(issuable_class.full_search('able')).to eq([searchable_issue]) end it 'returns notes with a matching title regardless of the casing' do - expect(described_class.full_search(searchable_issue.title.upcase)). + expect(issuable_class.full_search(searchable_issue.title.upcase)). to eq([searchable_issue]) end it 'returns notes with a matching description' do - expect(described_class.full_search(searchable_issue.description)). + expect(issuable_class.full_search(searchable_issue.description)). to eq([searchable_issue]) end it 'returns notes with a partially matching description' do - expect(described_class.full_search(searchable_issue.description)). + expect(issuable_class.full_search(searchable_issue.description)). to eq([searchable_issue]) end it 'returns notes with a matching description regardless of the casing' do - expect(described_class.full_search(searchable_issue.description.upcase)). + expect(issuable_class.full_search(searchable_issue.description.upcase)). to eq([searchable_issue]) end end @@ -298,7 +252,20 @@ describe Issue, "Issuable" do end context "issue is assigned" do - before { issue.update_attribute(:assignee, user) } + before { issue.assignees << user } + + it "returns correct hook data" do + expect(data[:assignees].first).to eq(user.hook_attrs) + end + end + + context "merge_request is assigned" do + let(:merge_request) { create(:merge_request) } + let(:data) { merge_request.to_hook_data(user) } + + before do + merge_request.update_attribute(:assignee, user) + end it "returns correct hook data" do expect(data[:object_attributes]['assignee_id']).to eq(user.id) @@ -320,24 +287,6 @@ describe Issue, "Issuable" do include_examples 'deprecated repository hook data' end - describe '#card_attributes' do - it 'includes the author name' do - allow(issue).to receive(:author).and_return(double(name: 'Robert')) - allow(issue).to receive(:assignee).and_return(nil) - - expect(issue.card_attributes). - to eq({ 'Author' => 'Robert', 'Assignee' => nil }) - end - - it 'includes the assignee name' do - allow(issue).to receive(:author).and_return(double(name: 'Robert')) - allow(issue).to receive(:assignee).and_return(double(name: 'Douwe')) - - expect(issue.card_attributes). - to eq({ 'Author' => 'Robert', 'Assignee' => 'Douwe' }) - end - end - describe '#labels_array' do let(:project) { create(:empty_project) } let(:bug) { create(:label, project: project, title: 'bug') } @@ -466,27 +415,6 @@ describe Issue, "Issuable" do end end - describe '#assignee_or_author?' do - let(:user) { build(:user, id: 1) } - let(:issue) { build(:issue) } - - it 'returns true for a user that is assigned to an issue' do - issue.assignee = user - - expect(issue.assignee_or_author?(user)).to eq(true) - end - - it 'returns true for a user that is the author of an issue' do - issue.author = user - - expect(issue.assignee_or_author?(user)).to eq(true) - end - - it 'returns false for a user that is not the assignee or author' do - expect(issue.assignee_or_author?(user)).to eq(false) - end - end - describe '#spend_time' do let(:user) { create(:user) } let(:issue) { create(:issue) } diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 2092576e981..e382c7120de 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -163,3 +163,52 @@ describe Issue, "Mentionable" do end end end + +describe Commit, 'Mentionable' do + let(:project) { create(:project, :public, :repository) } + let(:commit) { project.commit } + + describe '#matches_cross_reference_regex?' do + it "is false when message doesn't reference anything" do + allow(commit.raw).to receive(:message).and_return "WIP: Do something" + + expect(commit.matches_cross_reference_regex?).to be false + end + + it 'is true if issue #number mentioned in title' do + allow(commit.raw).to receive(:message).and_return "#1" + + expect(commit.matches_cross_reference_regex?).to be true + end + + it 'is true if references an MR' do + allow(commit.raw).to receive(:message).and_return "See merge request !12" + + expect(commit.matches_cross_reference_regex?).to be true + end + + it 'is true if references a commit' do + allow(commit.raw).to receive(:message).and_return "a1b2c3d4" + + expect(commit.matches_cross_reference_regex?).to be true + end + + it 'is true if issue referenced by url' do + issue = create(:issue, project: project) + + allow(commit.raw).to receive(:message).and_return Gitlab::UrlBuilder.build(issue) + + expect(commit.matches_cross_reference_regex?).to be true + end + + context 'with external issue tracker' do + let(:project) { create(:jira_project) } + + it 'is true if external issues referenced' do + allow(commit.raw).to receive(:message).and_return 'JIRA-123' + + expect(commit.matches_cross_reference_regex?).to be true + end + end + end +end diff --git a/spec/models/concerns/milestoneish_spec.rb b/spec/models/concerns/milestoneish_spec.rb index 68e4c0a522b..675b730c557 100644 --- a/spec/models/concerns/milestoneish_spec.rb +++ b/spec/models/concerns/milestoneish_spec.rb @@ -11,13 +11,13 @@ describe Milestone, 'Milestoneish' do let(:milestone) { create(:milestone, project: project) } let!(:issue) { create(:issue, project: project, milestone: milestone) } let!(:security_issue_1) { create(:issue, :confidential, project: project, author: author, milestone: milestone) } - let!(:security_issue_2) { create(:issue, :confidential, project: project, assignee: assignee, milestone: milestone) } + let!(:security_issue_2) { create(:issue, :confidential, project: project, assignees: [assignee], milestone: milestone) } let!(:closed_issue_1) { create(:issue, :closed, project: project, milestone: milestone) } let!(:closed_issue_2) { create(:issue, :closed, project: project, milestone: milestone) } let!(:closed_security_issue_1) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_security_issue_2) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } let!(:closed_security_issue_3) { create(:issue, :confidential, :closed, project: project, author: author, milestone: milestone) } - let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignee: assignee, milestone: milestone) } + let!(:closed_security_issue_4) { create(:issue, :confidential, :closed, project: project, assignees: [assignee], milestone: milestone) } let!(:merge_request) { create(:merge_request, source_project: project, target_project: project, milestone: milestone) } before do diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb new file mode 100644 index 00000000000..bdae742ff1d --- /dev/null +++ b/spec/models/concerns/noteable_spec.rb @@ -0,0 +1,261 @@ +require 'spec_helper' + +describe Noteable, model: true do + let!(:active_diff_note1) { create(:diff_note_on_merge_request) } + let(:project) { active_diff_note1.project } + subject { active_diff_note1.noteable } + let!(:active_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: subject, in_reply_to: active_diff_note1) } + let!(:active_diff_note3) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: active_position2) } + let!(:outdated_diff_note1) { create(:diff_note_on_merge_request, project: project, noteable: subject, position: outdated_position) } + let!(:outdated_diff_note2) { create(:diff_note_on_merge_request, project: project, noteable: subject, in_reply_to: outdated_diff_note1) } + let!(:discussion_note1) { create(:discussion_note_on_merge_request, project: project, noteable: subject) } + let!(:discussion_note2) { create(:discussion_note_on_merge_request, in_reply_to: discussion_note1) } + let!(:commit_diff_note1) { create(:diff_note_on_commit, project: project) } + let!(:commit_diff_note2) { create(:diff_note_on_commit, project: project, in_reply_to: commit_diff_note1) } + let!(:commit_note1) { create(:note_on_commit, project: project) } + let!(:commit_note2) { create(:note_on_commit, project: project) } + let!(:commit_discussion_note1) { create(:discussion_note_on_commit, project: project) } + let!(:commit_discussion_note2) { create(:discussion_note_on_commit, in_reply_to: commit_discussion_note1) } + let!(:commit_discussion_note3) { create(:discussion_note_on_commit, project: project) } + let!(:note1) { create(:note, project: project, noteable: subject) } + let!(:note2) { create(:note, project: project, noteable: subject) } + + let(:active_position2) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: 16, + new_line: 22, + diff_refs: subject.diff_refs + ) + end + + let(:outdated_position) do + Gitlab::Diff::Position.new( + old_path: "files/ruby/popen.rb", + new_path: "files/ruby/popen.rb", + old_line: nil, + new_line: 9, + diff_refs: project.commit("874797c3a73b60d2187ed6e2fcabd289ff75171e").diff_refs + ) + end + + describe '#discussions' do + let(:discussions) { subject.discussions } + + it 'includes discussions for diff notes, commit diff notes, commit notes, and regular notes' do + expect(discussions).to eq([ + DiffDiscussion.new([active_diff_note1, active_diff_note2], subject), + DiffDiscussion.new([active_diff_note3], subject), + DiffDiscussion.new([outdated_diff_note1, outdated_diff_note2], subject), + Discussion.new([discussion_note1, discussion_note2], subject), + DiffDiscussion.new([commit_diff_note1, commit_diff_note2], subject), + OutOfContextDiscussion.new([commit_note1, commit_note2], subject), + Discussion.new([commit_discussion_note1, commit_discussion_note2], subject), + Discussion.new([commit_discussion_note3], subject), + IndividualNoteDiscussion.new([note1], subject), + IndividualNoteDiscussion.new([note2], subject) + ]) + end + end + + describe '#grouped_diff_discussions' do + let(:grouped_diff_discussions) { subject.grouped_diff_discussions } + + it "includes active discussions" do + discussions = grouped_diff_discussions.values.flatten + + expect(discussions.count).to eq(2) + expect(discussions.map(&:id)).to eq([active_diff_note1.discussion_id, active_diff_note3.discussion_id]) + expect(discussions.all?(&:active?)).to be true + + expect(discussions.first.notes).to eq([active_diff_note1, active_diff_note2]) + expect(discussions.last.notes).to eq([active_diff_note3]) + end + + it "doesn't include outdated discussions" do + expect(grouped_diff_discussions.values.flatten.map(&:id)).not_to include(outdated_diff_note1.discussion_id) + end + + it "groups the discussions by line code" do + expect(grouped_diff_discussions[active_diff_note1.line_code].first.id).to eq(active_diff_note1.discussion_id) + expect(grouped_diff_discussions[active_diff_note3.line_code].first.id).to eq(active_diff_note3.discussion_id) + end + end + + context "discussion status" do + let(:first_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + let(:second_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + let(:third_discussion) { build_stubbed(:discussion_note_on_merge_request, noteable: subject, project: project).to_discussion } + + before do + allow(subject).to receive(:resolvable_discussions).and_return([first_discussion, second_discussion, third_discussion]) + end + + describe "#discussions_resolvable?" do + context "when all discussions are unresolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(false) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolvable?).to be false + end + end + + context "when some discussions are unresolvable and some discussions are resolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolvable?).to be true + end + end + + context "when all discussions are resolvable" do + before do + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(true) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolvable?).to be true + end + end + end + + describe "#discussions_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns true" do + expect(subject.discussions_resolved?).to be true + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_resolved?).to be false + end + end + end + end + + describe "#discussions_to_be_resolved?" do + context "when discussions are not resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when discussions are resolvable" do + before do + allow(subject).to receive(:discussions_resolvable?).and_return(true) + + allow(first_discussion).to receive(:resolvable?).and_return(true) + allow(second_discussion).to receive(:resolvable?).and_return(false) + allow(third_discussion).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable discussions are resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.discussions_to_be_resolved?).to be false + end + end + + context "when some resolvable discussions are not resolved" do + before do + allow(first_discussion).to receive(:resolved?).and_return(true) + allow(third_discussion).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.discussions_to_be_resolved?).to be true + end + end + end + end + + describe "#discussions_to_be_resolved" do + before do + allow(first_discussion).to receive(:to_be_resolved?).and_return(true) + allow(second_discussion).to receive(:to_be_resolved?).and_return(false) + allow(third_discussion).to receive(:to_be_resolved?).and_return(false) + end + + it 'includes only discussions that need to be resolved' do + expect(subject.discussions_to_be_resolved).to eq([first_discussion]) + end + end + + describe '#discussions_can_be_resolved_by?' do + let(:user) { build(:user) } + + context 'all discussions can be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(true) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(true) + end + end + + context 'one discussion cannot be resolved by the user' do + before do + allow(first_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(second_discussion).to receive(:can_resolve?).with(user).and_return(true) + allow(third_discussion).to receive(:can_resolve?).with(user).and_return(false) + end + + it 'allows a user to resolve the discussions' do + expect(subject.discussions_can_be_resolved_by?(user)).to be(false) + end + end + end + end +end diff --git a/spec/models/concerns/relative_positioning_spec.rb b/spec/models/concerns/relative_positioning_spec.rb index 255b584a85e..494e6f1b6f6 100644 --- a/spec/models/concerns/relative_positioning_spec.rb +++ b/spec/models/concerns/relative_positioning_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Issue, 'RelativePositioning' do +describe RelativePositioning do let(:project) { create(:empty_project) } let(:issue) { create(:issue, project: project) } let(:issue1) { create(:issue, project: project) } diff --git a/spec/models/concerns/resolvable_discussion_spec.rb b/spec/models/concerns/resolvable_discussion_spec.rb new file mode 100644 index 00000000000..18327fe262d --- /dev/null +++ b/spec/models/concerns/resolvable_discussion_spec.rb @@ -0,0 +1,548 @@ +require 'spec_helper' + +describe Discussion, ResolvableDiscussion, models: true do + subject { described_class.new([first_note, second_note, third_note]) } + + let(:first_note) { create(:discussion_note_on_merge_request) } + let(:merge_request) { first_note.noteable } + let(:project) { first_note.project } + let(:second_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project, in_reply_to: first_note) } + let(:third_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + describe "#resolvable?" do + context "when potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(true) + end + + context "when all notes are unresolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(false) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + + context "when some notes are unresolvable and some notes are resolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.resolvable?).to be true + end + end + + context "when all notes are resolvable" do + before do + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(true) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + it "returns true" do + expect(subject.resolvable?).to be true + end + end + end + + context "when not potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + end + + describe "#resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(true) + end + + it "returns true" do + expect(subject.resolved?).to be true + end + end + + context "when some resolvable notes are not resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(false) + end + + it "returns false" do + expect(subject.resolved?).to be false + end + end + end + end + + describe "#to_be_resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when some resolvable notes are not resolved" do + before do + allow(first_note).to receive(:resolved?).and_return(true) + allow(third_note).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.to_be_resolved?).to be true + end + end + end + end + + describe "#can_resolve?" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when not signed in" do + let(:current_user) { nil } + + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + + context "when signed in" do + context "when the signed in user is the noteable author" do + before do + subject.noteable.author = current_user + end + + it "returns true" do + expect(subject.can_resolve?(current_user)).to be true + end + end + + context "when the signed in user can push to the project" do + before do + subject.project.team << [current_user, :master] + end + + it "returns true" do + expect(subject.can_resolve?(current_user)).to be true + end + end + + context "when the signed in user is a random user" do + it "returns false" do + expect(subject.can_resolve?(current_user)).to be false + end + end + end + end + end + + describe "#resolve!" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't set resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).to be_nil + end + + it "doesn't set resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to be_nil + end + + it "doesn't mark as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + let(:user) { create(:user) } + let(:second_note) { create(:diff_note_on_commit) } # unresolvable + + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + first_note.resolve!(user) + third_note.resolve!(user) + + first_note.reload + third_note.reload + end + + it "doesn't change resolved_at on the resolved notes" do + expect(first_note.resolved_at).not_to be_nil + expect(third_note.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_at } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_at } + end + + it "doesn't change resolved_by on the resolved notes" do + expect(first_note.resolved_by).to eq(user) + expect(third_note.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved_by } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved_by } + end + + it "doesn't change the resolved state on the resolved notes" do + expect(first_note.resolved?).to be true + expect(third_note.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { first_note.resolved? } + expect { subject.resolve!(current_user) }.not_to change { third_note.resolved? } + end + + it "doesn't change resolved_at" do + expect(subject.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } + end + + it "doesn't change resolved_by" do + expect(subject.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } + end + + it "doesn't change resolved state" do + expect(subject.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } + end + end + + context "when some resolvable notes are resolved" do + before do + first_note.resolve!(user) + end + + it "doesn't change resolved_at on the resolved note" do + expect(first_note.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload.resolved_at } + end + + it "doesn't change resolved_by on the resolved note" do + expect(first_note.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload && first_note.resolved_by } + end + + it "doesn't change the resolved state on the resolved note" do + expect(first_note.resolved?).to be true + + expect { subject.resolve!(current_user) }. + not_to change { first_note.reload && first_note.resolved? } + end + + it "sets resolved_at on the unresolved note" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved_at).not_to be_nil + end + + it "sets resolved_by on the unresolved note" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved_by).to eq(current_user) + end + + it "marks the unresolved note as resolved" do + subject.resolve!(current_user) + third_note.reload + + expect(third_note.resolved?).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be true + end + end + + context "when no resolvable notes are resolved" do + it "sets resolved_at on the unresolved notes" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved_at).not_to be_nil + expect(third_note.resolved_at).not_to be_nil + end + + it "sets resolved_by on the unresolved notes" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved_by).to eq(current_user) + expect(third_note.resolved_by).to eq(current_user) + end + + it "marks the unresolved notes as resolved" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(first_note.resolved?).to be true + expect(third_note.resolved?).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + first_note.reload + third_note.reload + + expect(subject.resolved?).to be true + end + end + end + end + + describe "#unresolve!" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + + context "when resolvable" do + let(:user) { create(:user) } + + before do + allow(subject).to receive(:resolvable?).and_return(true) + + allow(first_note).to receive(:resolvable?).and_return(true) + allow(second_note).to receive(:resolvable?).and_return(false) + allow(third_note).to receive(:resolvable?).and_return(true) + end + + context "when all resolvable notes are resolved" do + before do + first_note.resolve!(user) + third_note.resolve!(user) + end + + it "unsets resolved_at on the resolved notes" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved_at).to be_nil + expect(third_note.resolved_at).to be_nil + end + + it "unsets resolved_by on the resolved notes" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved_by).to be_nil + expect(third_note.resolved_by).to be_nil + end + + it "unmarks the resolved notes as resolved" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(first_note.resolved?).to be false + expect(third_note.resolved?).to be false + end + + it "unsets resolved_at" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(subject.resolved_at).to be_nil + end + + it "unsets resolved_by" do + subject.unresolve! + first_note.reload + third_note.reload + + expect(subject.resolved_by).to be_nil + end + + it "unmarks as resolved" do + subject.unresolve! + + expect(subject.resolved?).to be false + end + end + + context "when some resolvable notes are resolved" do + before do + first_note.resolve!(user) + end + + it "unsets resolved_at on the resolved note" do + subject.unresolve! + + expect(subject.first_note.resolved_at).to be_nil + end + + it "unsets resolved_by on the resolved note" do + subject.unresolve! + + expect(subject.first_note.resolved_by).to be_nil + end + + it "unmarks the resolved note as resolved" do + subject.unresolve! + + expect(subject.first_note.resolved?).to be false + end + end + end + end + + describe "#first_note_to_resolve" do + it "returns the first note that still needs to be resolved" do + allow(first_note).to receive(:to_be_resolved?).and_return(false) + allow(second_note).to receive(:to_be_resolved?).and_return(true) + + expect(subject.first_note_to_resolve).to eq(second_note) + end + end + + describe "#last_resolved_note" do + let(:current_user) { create(:user) } + + before do + first_note.resolve!(current_user) + third_note.resolve!(current_user) + second_note.resolve!(current_user) + end + + it "returns the last note that was resolved" do + expect(subject.last_resolved_note).to eq(second_note) + end + end +end diff --git a/spec/models/concerns/resolvable_note_spec.rb b/spec/models/concerns/resolvable_note_spec.rb new file mode 100644 index 00000000000..1503ccdff11 --- /dev/null +++ b/spec/models/concerns/resolvable_note_spec.rb @@ -0,0 +1,329 @@ +require 'spec_helper' + +describe Note, ResolvableNote, models: true do + let(:project) { create(:project) } + let(:merge_request) { create(:merge_request, source_project: project) } + subject { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + context 'resolvability scopes' do + let!(:note1) { create(:note, project: project) } + let!(:note2) { create(:diff_note_on_commit, project: project) } + let!(:note3) { create(:diff_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + let!(:note4) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + let!(:note5) { create(:discussion_note_on_issue, project: project) } + let!(:note6) { create(:discussion_note_on_merge_request, :system, noteable: merge_request, project: project) } + + describe '.potentially_resolvable' do + it 'includes diff and discussion notes on merge requests' do + expect(Note.potentially_resolvable).to match_array([note3, note4, note6]) + end + end + + describe '.resolvable' do + it 'includes non-system diff and discussion notes on merge requests' do + expect(Note.resolvable).to match_array([note3, note4]) + end + end + + describe '.resolved' do + it 'includes resolved non-system diff and discussion notes on merge requests' do + expect(Note.resolved).to match_array([note3]) + end + end + + describe '.unresolved' do + it 'includes non-resolved non-system diff and discussion notes on merge requests' do + expect(Note.unresolved).to match_array([note4]) + end + end + end + + describe ".resolve!" do + let(:current_user) { create(:user) } + let!(:commit_note) { create(:diff_note_on_commit, project: project) } + let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + let!(:unresolved_note) { create(:discussion_note_on_merge_request, noteable: merge_request, project: project) } + + before do + described_class.resolve!(current_user) + + commit_note.reload + resolved_note.reload + unresolved_note.reload + end + + it 'resolves only the resolvable, not yet resolved notes' do + expect(commit_note.resolved_at).to be_nil + expect(resolved_note.resolved_by).not_to eq(current_user) + expect(unresolved_note.resolved_at).not_to be_nil + expect(unresolved_note.resolved_by).to eq(current_user) + end + end + + describe ".unresolve!" do + let!(:resolved_note) { create(:discussion_note_on_merge_request, :resolved, noteable: merge_request, project: project) } + + before do + described_class.unresolve! + + resolved_note.reload + end + + it 'unresolves the resolved notes' do + expect(resolved_note.resolved_by).to be_nil + expect(resolved_note.resolved_at).to be_nil + end + end + + describe '#resolvable?' do + context "when potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(true) + end + + context "when a system note" do + before do + subject.system = true + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + + context "when a regular note" do + it "returns true" do + expect(subject.resolvable?).to be true + end + end + end + + context "when not potentially resolvable" do + before do + allow(subject).to receive(:potentially_resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.resolvable?).to be false + end + end + end + + describe "#to_be_resolved?" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when resolved" do + before do + allow(subject).to receive(:resolved?).and_return(true) + end + + it "returns false" do + expect(subject.to_be_resolved?).to be false + end + end + + context "when not resolved" do + before do + allow(subject).to receive(:resolved?).and_return(false) + end + + it "returns true" do + expect(subject.to_be_resolved?).to be true + end + end + end + end + + describe "#resolved?" do + let(:current_user) { create(:user) } + + context 'when not resolvable' do + before do + subject.resolve!(current_user) + + allow(subject).to receive(:resolvable?).and_return(false) + end + + it 'returns false' do + expect(subject.resolved?).to be_falsey + end + end + + context 'when resolvable' do + context 'when the note has been resolved' do + before do + subject.resolve!(current_user) + end + + it 'returns true' do + expect(subject.resolved?).to be_truthy + end + end + + context 'when the note has not been resolved' do + it 'returns false' do + expect(subject.resolved?).to be_falsey + end + end + end + end + + describe "#resolve!" do + let(:current_user) { create(:user) } + + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't set resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).to be_nil + end + + it "doesn't set resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to be_nil + end + + it "doesn't mark as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be false + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when already resolved" do + let(:user) { create(:user) } + + before do + subject.resolve!(user) + end + + it "returns nil" do + expect(subject.resolve!(current_user)).to be_nil + end + + it "doesn't change resolved_at" do + expect(subject.resolved_at).not_to be_nil + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_at } + end + + it "doesn't change resolved_by" do + expect(subject.resolved_by).to eq(user) + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved_by } + end + + it "doesn't change resolved status" do + expect(subject.resolved?).to be true + + expect { subject.resolve!(current_user) }.not_to change { subject.resolved? } + end + end + + context "when not yet resolved" do + it "returns true" do + expect(subject.resolve!(current_user)).to be true + end + + it "sets resolved_at" do + subject.resolve!(current_user) + + expect(subject.resolved_at).not_to be_nil + end + + it "sets resolved_by" do + subject.resolve!(current_user) + + expect(subject.resolved_by).to eq(current_user) + end + + it "marks as resolved" do + subject.resolve!(current_user) + + expect(subject.resolved?).to be true + end + end + end + end + + describe "#unresolve!" do + context "when not resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(false) + end + + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + + context "when resolvable" do + before do + allow(subject).to receive(:resolvable?).and_return(true) + end + + context "when resolved" do + let(:user) { create(:user) } + + before do + subject.resolve!(user) + end + + it "returns true" do + expect(subject.unresolve!).to be true + end + + it "unsets resolved_at" do + subject.unresolve! + + expect(subject.resolved_at).to be_nil + end + + it "unsets resolved_by" do + subject.unresolve! + + expect(subject.resolved_by).to be_nil + end + + it "unmarks as resolved" do + subject.unresolve! + + expect(subject.resolved?).to be false + end + end + + context "when not resolved" do + it "returns nil" do + expect(subject.unresolve!).to be_nil + end + end + end + end +end diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 677e60e1282..49a4132f763 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Group, 'Routable' do - let!(:group) { create(:group) } + let!(:group) { create(:group, name: 'foo') } describe 'Validations' do it { is_expected.to validate_presence_of(:route) } @@ -9,6 +9,7 @@ describe Group, 'Routable' do describe 'Associations' do it { is_expected.to have_one(:route).dependent(:destroy) } + it { is_expected.to have_many(:redirect_routes).dependent(:destroy) } end describe 'Callbacks' do @@ -35,10 +36,53 @@ describe Group, 'Routable' do describe '.find_by_full_path' do let!(:nested_group) { create(:group, parent: group) } - it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) } - it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) } - it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) } - it { expect(described_class.find_by_full_path('unknown')).to eq(nil) } + context 'without any redirect routes' do + it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) } + it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) } + it { expect(described_class.find_by_full_path('unknown')).to eq(nil) } + end + + context 'with redirect routes' do + let!(:group_redirect_route) { group.redirect_routes.create!(path: 'bar') } + let!(:nested_group_redirect_route) { nested_group.redirect_routes.create!(path: nested_group.path.sub('foo', 'bar')) } + + context 'without follow_redirects option' do + context 'with the given path not matching any route' do + it { expect(described_class.find_by_full_path('unknown')).to eq(nil) } + end + + context 'with the given path matching the canonical route' do + it { expect(described_class.find_by_full_path(group.to_param)).to eq(group) } + it { expect(described_class.find_by_full_path(group.to_param.upcase)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group.to_param)).to eq(nested_group) } + end + + context 'with the given path matching a redirect route' do + it { expect(described_class.find_by_full_path(group_redirect_route.path)).to eq(nil) } + it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase)).to eq(nil) } + it { expect(described_class.find_by_full_path(nested_group_redirect_route.path)).to eq(nil) } + end + end + + context 'with follow_redirects option set to true' do + context 'with the given path not matching any route' do + it { expect(described_class.find_by_full_path('unknown', follow_redirects: true)).to eq(nil) } + end + + context 'with the given path matching the canonical route' do + it { expect(described_class.find_by_full_path(group.to_param, follow_redirects: true)).to eq(group) } + it { expect(described_class.find_by_full_path(group.to_param.upcase, follow_redirects: true)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group.to_param, follow_redirects: true)).to eq(nested_group) } + end + + context 'with the given path matching a redirect route' do + it { expect(described_class.find_by_full_path(group_redirect_route.path, follow_redirects: true)).to eq(group) } + it { expect(described_class.find_by_full_path(group_redirect_route.path.upcase, follow_redirects: true)).to eq(group) } + it { expect(described_class.find_by_full_path(nested_group_redirect_route.path, follow_redirects: true)).to eq(nested_group) } + end + end + end end describe '.where_full_path_in' do @@ -81,12 +125,137 @@ describe Group, 'Routable' do it { is_expected.to eq([nested_group]) } end + describe '.member_self_and_descendants' do + let!(:user) { create(:user) } + let!(:nested_group) { create(:group, parent: group) } + + before { group.add_owner(user) } + subject { described_class.member_self_and_descendants(user.id) } + + it { is_expected.to match_array [group, nested_group] } + end + + describe '.member_hierarchy' do + # foo/bar would also match foo/barbaz instead of just foo/bar and foo/bar/baz + let!(:user) { create(:user) } + + # group + # _______ (foo) _______ + # | | + # | | + # nested_group_1 nested_group_2 + # (bar) (barbaz) + # | | + # | | + # nested_group_1_1 nested_group_2_1 + # (baz) (baz) + # + let!(:nested_group_1) { create :group, parent: group, name: 'bar' } + let!(:nested_group_1_1) { create :group, parent: nested_group_1, name: 'baz' } + let!(:nested_group_2) { create :group, parent: group, name: 'barbaz' } + let!(:nested_group_2_1) { create :group, parent: nested_group_2, name: 'baz' } + + context 'user is not a member of any group' do + subject { described_class.member_hierarchy(user.id) } + + it 'returns an empty array' do + is_expected.to eq [] + end + end + + context 'user is member of all groups' do + before do + group.add_owner(user) + nested_group_1.add_owner(user) + nested_group_1_1.add_owner(user) + nested_group_2.add_owner(user) + nested_group_2_1.add_owner(user) + end + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the top group' do + before { group.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns all groups' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 1' do + before { nested_group_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + + context 'user is member of the first child (internal node), branch 2' do + before { nested_group_2.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_2, nested_group_2_1 + ] + end + end + + context 'user is member of the last child (leaf node)' do + before { nested_group_1_1.add_owner(user) } + subject { described_class.member_hierarchy(user.id) } + + it 'returns the groups in the hierarchy' do + is_expected.to match_array [ + group, + nested_group_1, nested_group_1_1 + ] + end + end + end + describe '#full_path' do let(:group) { create(:group) } let(:nested_group) { create(:group, parent: group) } it { expect(group.full_path).to eq(group.path) } it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } + + context 'with RequestStore active' do + before do + RequestStore.begin! + end + + after do + RequestStore.end! + RequestStore.clear! + end + + it 'does not load the route table more than once' do + expect(group).to receive(:uncached_full_path).once.and_call_original + + 3.times { group.full_path } + expect(group.full_path).to eq(group.path) + end + end end describe '#full_name' do diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index fd3b8307571..e698207166c 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -1,9 +1,11 @@ require 'spec_helper' -describe Issue, 'Spammable' do +describe Spammable do let(:issue) { create(:issue, description: 'Test Desc.') } describe 'Associations' do + subject { build(:issue) } + it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) } end diff --git a/spec/models/concerns/strip_attribute_spec.rb b/spec/models/concerns/strip_attribute_spec.rb index c3af7a0960f..8c945686b66 100644 --- a/spec/models/concerns/strip_attribute_spec.rb +++ b/spec/models/concerns/strip_attribute_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Milestone, "StripAttribute" do +describe StripAttribute do let(:milestone) { create(:milestone) } describe ".strip_attributes" do |