From 0672258915a0cf444802ffc50ad1cd914f4f11d4 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Wed, 9 Mar 2016 16:24:02 +0100 Subject: Cleanup CiCommit and CiBuild - Remove all view related methods from Ci::Build and CommitStatus - Remove unused Ci::Commit and Ci::Build methods - Use polymorphism to render different types of CommitStatus --- spec/models/build_spec.rb | 32 ++---------------------------- spec/models/ci/commit_spec.rb | 46 +------------------------------------------ 2 files changed, 3 insertions(+), 75 deletions(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index e3d3d453653..f4f817297b9 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -312,8 +312,8 @@ describe Ci::Build, models: true do end end - describe :show_warning? do - subject { build.show_warning? } + describe :stuck? do + subject { build.stuck? } %w(pending).each do |state| context "if commit_status.status is #{state}" do @@ -343,34 +343,6 @@ describe Ci::Build, models: true do end end - describe :artifacts_download_url do - subject { build.artifacts_download_url } - - context 'artifacts file does not exist' do - before { build.update_attributes(artifacts_file: nil) } - it { is_expected.to be_nil } - end - - context 'artifacts file exists' do - let(:build) { create(:ci_build, :artifacts) } - it { is_expected.to_not be_nil } - end - end - - describe :artifacts_browse_url do - subject { build.artifacts_browse_url } - - it "should be nil if artifacts browser is unsupported" do - allow(build).to receive(:artifacts_metadata?).and_return(false) - is_expected.to be_nil - end - - it 'should not be nil if artifacts browser is supported' do - allow(build).to receive(:artifacts_metadata?).and_return(true) - is_expected.to_not be_nil - end - end - describe :artifacts? do subject { build.artifacts? } diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index 4dc309a4255..c51444f4875 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -23,7 +23,7 @@ describe Ci::Commit, models: true do let(:commit) { FactoryGirl.create :ci_commit, project: project } it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:statuses) } + it { is_expected.to have_many(:commit_statuses) } it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:builds) } it { is_expected.to validate_presence_of :sha } @@ -32,50 +32,6 @@ describe Ci::Commit, models: true do it { is_expected.to respond_to :git_author_email } it { is_expected.to respond_to :short_sha } - describe :ordered do - let(:project) { FactoryGirl.create :empty_project } - - it 'returns ordered list of commits' do - commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project - commit2 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project - expect(project.ci_commits.ordered).to eq([commit2, commit1]) - end - - it 'returns commits ordered by committed_at and id, with nulls last' do - commit1 = FactoryGirl.create :ci_commit, committed_at: 1.hour.ago, project: project - commit2 = FactoryGirl.create :ci_commit, committed_at: nil, project: project - commit3 = FactoryGirl.create :ci_commit, committed_at: 2.hours.ago, project: project - commit4 = FactoryGirl.create :ci_commit, committed_at: nil, project: project - expect(project.ci_commits.ordered).to eq([commit2, commit4, commit3, commit1]) - end - end - - describe :last_build do - subject { commit.last_build } - before do - @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday - @second = FactoryGirl.create :ci_build, commit: commit - end - - it { is_expected.to be_a(Ci::Build) } - it('returns with the most recently created build') { is_expected.to eq(@second) } - end - - describe :retry do - before do - @first = FactoryGirl.create :ci_build, commit: commit, created_at: Date.yesterday - @second = FactoryGirl.create :ci_build, commit: commit - end - - it "creates only a new build" do - expect(commit.builds.count(:all)).to eq 2 - expect(commit.statuses.count(:all)).to eq 2 - commit.retry - expect(commit.builds.count(:all)).to eq 3 - expect(commit.statuses.count(:all)).to eq 3 - end - end - describe :valid_commit_sha do context 'commit.sha can not start with 00000000' do before do -- cgit v1.2.1 From f32e28f6faab176845a780d5b4d26881c08bcfec Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Thu, 10 Mar 2016 11:06:33 +0100 Subject: Fix commit_spec: invalid validation --- spec/models/ci/commit_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models') diff --git a/spec/models/ci/commit_spec.rb b/spec/models/ci/commit_spec.rb index c51444f4875..412842337ba 100644 --- a/spec/models/ci/commit_spec.rb +++ b/spec/models/ci/commit_spec.rb @@ -23,7 +23,7 @@ describe Ci::Commit, models: true do let(:commit) { FactoryGirl.create :ci_commit, project: project } it { is_expected.to belong_to(:project) } - it { is_expected.to have_many(:commit_statuses) } + it { is_expected.to have_many(:statuses) } it { is_expected.to have_many(:trigger_requests) } it { is_expected.to have_many(:builds) } it { is_expected.to validate_presence_of :sha } -- cgit v1.2.1 From 16592e2b45d42e22f9d1d595a1f44821c7b30441 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Mon, 14 Mar 2016 13:33:26 +0100 Subject: Fix review comments - Remove unused Gitlab::Application.routes.url_helpers from Ci::Build - Remove too much logic from a view, use Ci::Commit.matrix_builds - Use ci_status_with_icon - Don't describe symbols --- spec/models/build_spec.rb | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'spec/models') diff --git a/spec/models/build_spec.rb b/spec/models/build_spec.rb index f4f817297b9..b7457808040 100644 --- a/spec/models/build_spec.rb +++ b/spec/models/build_spec.rb @@ -9,7 +9,7 @@ describe Ci::Build, models: true do it { is_expected.to respond_to :trace_html } - describe :first_pending do + describe '#first_pending' do let(:first) { FactoryGirl.create :ci_build, commit: commit, status: 'pending', created_at: Date.yesterday } let(:second) { FactoryGirl.create :ci_build, commit: commit, status: 'pending' } before { first; second } @@ -19,7 +19,7 @@ describe Ci::Build, models: true do it('returns with the first pending build') { is_expected.to eq(first) } end - describe :create_from do + describe '#create_from' do before do build.status = 'success' build.save @@ -33,7 +33,7 @@ describe Ci::Build, models: true do end end - describe :ignored? do + describe '#ignored?' do subject { build.ignored? } context 'if build is not allowed to fail' do @@ -69,7 +69,7 @@ describe Ci::Build, models: true do end end - describe :trace do + describe '#trace' do subject { build.trace_html } it { is_expected.to be_empty } @@ -101,7 +101,7 @@ describe Ci::Build, models: true do # it { is_expected.to eq(commit.project.timeout) } # end - describe :options do + describe '#options' do let(:options) do { image: "ruby:2.1", @@ -122,25 +122,25 @@ describe Ci::Build, models: true do # it { is_expected.to eq(project.allow_git_fetch) } # end - describe :project do + describe '#project' do subject { build.project } it { is_expected.to eq(commit.project) } end - describe :project_id do + describe '#project_id' do subject { build.project_id } it { is_expected.to eq(commit.project_id) } end - describe :project_name do + describe '#project_name' do subject { build.project_name } it { is_expected.to eq(project.name) } end - describe :extract_coverage do + describe '#extract_coverage' do context 'valid content & regex' do subject { build.extract_coverage('Coverage 1033 / 1051 LOC (98.29%) covered', '\(\d+.\d+\%\) covered') } @@ -172,7 +172,7 @@ describe Ci::Build, models: true do end end - describe :variables do + describe '#variables' do context 'returns variables' do subject { build.variables } @@ -242,7 +242,7 @@ describe Ci::Build, models: true do end end - describe :can_be_served? do + describe '#can_be_served?' do let(:runner) { FactoryGirl.create :ci_runner } before { build.project.runners << runner } @@ -277,7 +277,7 @@ describe Ci::Build, models: true do end end - describe :any_runners_online? do + describe '#any_runners_online?' do subject { build.any_runners_online? } context 'when no runners' do @@ -312,7 +312,7 @@ describe Ci::Build, models: true do end end - describe :stuck? do + describe '#stuck?' do subject { build.stuck? } %w(pending).each do |state| @@ -343,7 +343,7 @@ describe Ci::Build, models: true do end end - describe :artifacts? do + describe '#artifacts?' do subject { build.artifacts? } context 'artifacts archive does not exist' do @@ -358,7 +358,7 @@ describe Ci::Build, models: true do end - describe :artifacts_metadata? do + describe '#artifacts_metadata?' do subject { build.artifacts_metadata? } context 'artifacts metadata does not exist' do it { is_expected.to be_falsy } @@ -370,7 +370,7 @@ describe Ci::Build, models: true do end end - describe :repo_url do + describe '#repo_url' do let(:build) { FactoryGirl.create :ci_build } let(:project) { build.project } @@ -384,7 +384,7 @@ describe Ci::Build, models: true do it { is_expected.to include(project.web_url[7..-1]) } end - describe :depends_on_builds do + describe '#depends_on_builds' do let!(:build) { FactoryGirl.create :ci_build, commit: commit, name: 'build', stage_idx: 0, stage: 'build' } let!(:rspec_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rspec', stage_idx: 1, stage: 'test' } let!(:rubocop_test) { FactoryGirl.create :ci_build, commit: commit, name: 'rubocop', stage_idx: 1, stage: 'test' } @@ -416,7 +416,7 @@ describe Ci::Build, models: true do created_at: created_at) end - describe :merge_request do + describe '#merge_request' do context 'when a MR has a reference to the commit' do before do @merge_request = create_mr(build, commit, factory: :merge_request) -- cgit v1.2.1 From 1714883107b7b8b8f2ef8c2836acc2866362738e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 15 Mar 2016 13:20:54 +0100 Subject: Revert "Merge branch 'avatar-cropping' into 'master' " This reverts commit 01160fc06182de89c400af174861f6545ad6ceb8, reversing changes made to 4bff9daf8b6d85e9c78565e21cfaa3f6d36f0282. --- spec/models/user_spec.rb | 26 -------------------------- 1 file changed, 26 deletions(-) (limited to 'spec/models') diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 909b6796591..6290ab3ebec 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -174,32 +174,6 @@ describe User, models: true do end end end - - describe 'avatar' do - it 'only validates when avatar is present and changed' do - user = build(:user, :with_avatar) - - user.avatar_crop_x = nil - user.avatar_crop_y = nil - user.avatar_crop_size = nil - - expect(user).not_to be_valid - expect(user.errors.keys). - to match_array %i(avatar_crop_x avatar_crop_y avatar_crop_size) - end - - it 'does not validate when avatar has not changed' do - user = create(:user, :with_avatar) - - expect { user.avatar_crop_x = nil }.not_to change(user, :valid?) - end - - it 'does not validate when avatar is not present' do - user = create(:user) - - expect { user.avatar_crop_y = nil }.not_to change(user, :valid?) - end - end end describe "Respond to" do -- cgit v1.2.1 From c742760289e51117d3e76e27a626691bec631e1e Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 15 Mar 2016 16:46:17 +0100 Subject: Ignore eager loading in Project.search UNION The queries that are UNION'd together don't need any eager loading (since we really only use the resulting SQL instead of having ActiveRecord actually run the queries). By dropping any eager loaded associations queries such as the following work instead of producing a SQL error: Project.all.includes(:namespace).search('foo') --- spec/models/project_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/models') diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 59c5ffa6b9c..b8b9a455b83 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -634,6 +634,12 @@ describe Project, models: true do it 'returns projects with a matching namespace name regardless of the casing' do expect(described_class.search(project.namespace.name.upcase)).to eq([project]) end + + it 'returns projects when eager loading namespaces' do + relation = described_class.all.includes(:namespace) + + expect(relation.search(project.namespace.name)).to eq([project]) + end end describe '#rename_repo' do -- cgit v1.2.1 From 0444fa560acd07255960284f19b1de6499cd5910 Mon Sep 17 00:00:00 2001 From: Timothy Andrew Date: Fri, 12 Feb 2016 20:28:39 +0530 Subject: Original implementation to allow users to subscribe to labels 1. Allow subscribing (the current user) to a label - Refactor the `Subscription` coffeescript class - The main change is that it accepts a container, and conducts all DOM queries within its scope. We need this because the labels page has multiple instances of `Subscription` on the same page. 2. Creating an issue or MR with labels notifies users subscribed to those labels - Label `has_many` subscribers through subscriptions. 3. Adding a label to an issue or MR notifies users subscribed to those labels - This only applies to subscribers of the label that has just been added, not all labels for the issue. --- spec/models/concerns/subscribable_spec.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 spec/models/concerns/subscribable_spec.rb (limited to 'spec/models') diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb new file mode 100644 index 00000000000..9ee60426a5d --- /dev/null +++ b/spec/models/concerns/subscribable_spec.rb @@ -0,0 +1,16 @@ +require "spec_helper" + +describe Subscribable, "Subscribable" do + let(:resource) { create(:issue) } + let(:user) { create(:user) } + + describe "#subscribed?" do + it do + expect(resource.subscribed?(user)).to be_falsey + resource.toggle_subscription(user) + expect(resource.subscribed?(user)).to be_truthy + resource.toggle_subscription(user) + expect(resource.subscribed?(user)).to be_falsey + end + end +end -- cgit v1.2.1 From 54ec7e959900493b6e9174bf4dfe09ed0afd1e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 1 Mar 2016 17:33:13 +0100 Subject: Improving the original label-subscribing implementation 1. Make the "subscribed" text in Issuable sidebar reflect the labels subscription status 2. Current user mut be logged-in to toggle issue/MR/label subscription --- spec/models/concerns/issuable_spec.rb | 42 +++++++++++++++++++++++++ spec/models/concerns/subscribable_spec.rb | 51 ++++++++++++++++++++++++++++--- 2 files changed, 88 insertions(+), 5 deletions(-) (limited to 'spec/models') diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index aff384c2949..be29b6d66ff 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -113,6 +113,48 @@ describe Issue, "Issuable" do end end + describe '#subscribed?' do + context 'user is not a participant in the issue' do + before { allow(issue).to receive(:participants).with(user).and_return([]) } + + it 'returns false when no subcription exists' do + expect(issue.subscribed?(user)).to be_falsey + end + + it 'returns true when a subcription exists and subscribed is true' do + issue.subscriptions.create(user: user, subscribed: true) + + expect(issue.subscribed?(user)).to be_truthy + end + + it 'returns false when a subcription exists and subscribed is false' do + issue.subscriptions.create(user: user, subscribed: false) + + expect(issue.subscribed?(user)).to be_falsey + end + end + + context 'user is a participant in the issue' do + before { allow(issue).to receive(:participants).with(user).and_return([user]) } + + it 'returns false when no subcription exists' do + expect(issue.subscribed?(user)).to be_truthy + end + + it 'returns true when a subcription exists and subscribed is true' do + issue.subscriptions.create(user: user, subscribed: true) + + expect(issue.subscribed?(user)).to be_truthy + end + + it 'returns false when a subcription exists and subscribed is false' do + issue.subscriptions.create(user: user, subscribed: false) + + expect(issue.subscribed?(user)).to be_falsey + end + end + end + describe "#to_hook_data" do let(:data) { issue.to_hook_data(user) } let(:project) { issue.project } diff --git a/spec/models/concerns/subscribable_spec.rb b/spec/models/concerns/subscribable_spec.rb index 9ee60426a5d..e31fdb0bffb 100644 --- a/spec/models/concerns/subscribable_spec.rb +++ b/spec/models/concerns/subscribable_spec.rb @@ -1,15 +1,56 @@ -require "spec_helper" +require 'spec_helper' -describe Subscribable, "Subscribable" do +describe Subscribable, 'Subscribable' do let(:resource) { create(:issue) } let(:user) { create(:user) } - describe "#subscribed?" do - it do + describe '#subscribed?' do + it 'returns false when no subcription exists' do expect(resource.subscribed?(user)).to be_falsey - resource.toggle_subscription(user) + end + + it 'returns true when a subcription exists and subscribed is true' do + resource.subscriptions.create(user: user, subscribed: true) + expect(resource.subscribed?(user)).to be_truthy + end + + it 'returns false when a subcription exists and subscribed is false' do + resource.subscriptions.create(user: user, subscribed: false) + + expect(resource.subscribed?(user)).to be_falsey + end + end + describe '#subscribers' do + it 'returns [] when no subcribers exists' do + expect(resource.subscribers).to be_empty + end + + it 'returns the subscribed users' do + resource.subscriptions.create(user: user, subscribed: true) + resource.subscriptions.create(user: create(:user), subscribed: false) + + expect(resource.subscribers).to eq [user] + end + end + + describe '#toggle_subscription' do + it 'toggles the current subscription state for the given user' do + expect(resource.subscribed?(user)).to be_falsey + resource.toggle_subscription(user) + + expect(resource.subscribed?(user)).to be_truthy + end + end + + describe '#unsubscribe' do + it 'unsubscribes the given current user' do + resource.subscriptions.create(user: user, subscribed: true) + expect(resource.subscribed?(user)).to be_truthy + + resource.unsubscribe(user) + expect(resource.subscribed?(user)).to be_falsey end end -- cgit v1.2.1