More improvements to presenters23563-document-presenters

Signed-off-by: Rémy Coutable <remy@rymai.me>

8 files changed, 71 insertions, 91 deletions

diff --git a/spec/lib/gitlab/view/presenter/base_spec.rb b/spec/lib/gitlab/view/presenter/base_spec.rb
index 57b98276622..f2c152cdcd4 100644
--- a/spec/lib/gitlab/view/presenter/base_spec.rb
+++ b/spec/lib/gitlab/view/presenter/base_spec.rb
@@ -6,32 +6,45 @@ describe Gitlab::View::Presenter::Base do
     Struct.new(:subject).include(described_class)
   end
 
-  subject do
-    presenter_class.new(project)
+  describe '.presenter?' do
+    it 'returns true' do
+      presenter = presenter_class.new(project)
+
+      expect(presenter.class).to be_presenter
+    end
   end
 
   describe '.presents' do
     it 'exposes #subject with the given keyword' do
       presenter_class.presents(:foo)
+      presenter = presenter_class.new(project)
 
-      expect(subject.foo).to eq(project)
+      expect(presenter.foo).to eq(project)
     end
   end
 
   describe '#can?' do
-    let(:project) { create(:empty_project) }
-
     context 'user is not allowed' do
       it 'returns false' do
-        expect(subject.can?(nil, :read_project)).to be_falsy
+        presenter = presenter_class.new(build_stubbed(:empty_project))
+
+        expect(presenter.can?(nil, :read_project)).to be_falsy
       end
     end
 
     context 'user is allowed' do
-      let(:project) { create(:empty_project, :public) }
+      it 'returns true' do
+        presenter = presenter_class.new(build_stubbed(:empty_project, :public))
 
+        expect(presenter.can?(nil, :read_project)).to be_truthy
+      end
+    end
+
+    context 'subject is overriden' do
       it 'returns true' do
-        expect(subject.can?(nil, :read_project)).to be_truthy
+        presenter = presenter_class.new(build_stubbed(:empty_project, :public))
+
+        expect(presenter.can?(nil, :read_project, build_stubbed(:empty_project))).to be_falsy
       end
     end
   end
diff --git a/spec/lib/gitlab/view/presenter/delegated_spec.rb b/spec/lib/gitlab/view/presenter/delegated_spec.rb
index 816d6b7c6d4..888ab80cad5 100644
--- a/spec/lib/gitlab/view/presenter/delegated_spec.rb
+++ b/spec/lib/gitlab/view/presenter/delegated_spec.rb
@@ -1,33 +1,29 @@
 require 'spec_helper'
 
 describe Gitlab::View::Presenter::Delegated do
-  let(:project) { double(:project, foo: 'bar') }
+  let(:project) { double(:project, bar: 'baz') }
   let(:presenter_class) do
     Class.new(described_class)
   end
 
-  subject do
-    presenter_class.new(project)
-  end
-
   it 'includes Gitlab::View::Presenter::Base' do
     expect(described_class).to include(Gitlab::View::Presenter::Base)
   end
 
   describe '#initialize' do
-    subject do
-      presenter_class.new(project, user: 'user', foo: 'bar')
-    end
-
     it 'takes arbitrary key/values and exposes them' do
-      expect(subject.user).to eq('user')
-      expect(subject.foo).to eq('bar')
+      presenter = presenter_class.new(project, user: 'user', foo: 'bar')
+
+      expect(presenter.user).to eq('user')
+      expect(presenter.foo).to eq('bar')
     end
   end
 
   describe 'delegation' do
-    it 'does not forward missing methods to subject' do
-      expect(subject.foo).to eq('bar')
+    it 'forwards missing methods to subject' do
+      presenter = presenter_class.new(project)
+
+      expect(presenter.bar).to eq('baz')
     end
   end
 end
diff --git a/spec/lib/gitlab/view/presenter/factory_spec.rb b/spec/lib/gitlab/view/presenter/factory_spec.rb
index 7a65429b500..55c5ecbf92f 100644
--- a/spec/lib/gitlab/view/presenter/factory_spec.rb
+++ b/spec/lib/gitlab/view/presenter/factory_spec.rb
@@ -1,42 +1,38 @@
 require 'spec_helper'
 
 describe Gitlab::View::Presenter::Factory do
-  let(:variable) { create(:ci_variable) }
+  let(:build) { Ci::Build.new }
 
   describe '#initialize' do
     context 'without optional parameters' do
-      subject do
-        described_class.new(variable)
-      end
-
       it 'takes a subject and optional params' do
-        expect { subject }.not_to raise_error
+        presenter = described_class.new(build)
+
+        expect { presenter }.not_to raise_error
       end
     end
 
     context 'with optional parameters' do
-      subject do
-        described_class.new(variable, user: 'user')
-      end
-
       it 'takes a subject and optional params' do
-        expect { subject }.not_to raise_error
+        presenter = described_class.new(build, user: 'user')
+
+        expect { presenter }.not_to raise_error
       end
     end
   end
 
   describe '#fabricate!' do
-    subject do
-      described_class.new(variable, user: 'user', foo: 'bar').fabricate!
-    end
-
     it 'exposes given params' do
-      expect(subject.user).to eq('user')
-      expect(subject.foo).to eq('bar')
+      presenter = described_class.new(build, user: 'user', foo: 'bar').fabricate!
+
+      expect(presenter.user).to eq('user')
+      expect(presenter.foo).to eq('bar')
     end
 
     it 'detects the presenter based on the given subject' do
-      expect(subject).to be_a(Ci::Variable::Presenter)
+      presenter = described_class.new(build).fabricate!
+
+      expect(presenter).to be_a(Ci::BuildPresenter)
     end
   end
 end
diff --git a/spec/lib/gitlab/view/presenter/simple_spec.rb b/spec/lib/gitlab/view/presenter/simple_spec.rb
index baf074019ec..b489bdf1981 100644
--- a/spec/lib/gitlab/view/presenter/simple_spec.rb
+++ b/spec/lib/gitlab/view/presenter/simple_spec.rb
@@ -6,28 +6,24 @@ describe Gitlab::View::Presenter::Simple do
     Class.new(described_class)
   end
 
-  subject do
-    presenter_class.new(project)
-  end
-
   it 'includes Gitlab::View::Presenter::Base' do
     expect(described_class).to include(Gitlab::View::Presenter::Base)
   end
 
   describe '#initialize' do
-    subject do
-      presenter_class.new(project, user: 'user', foo: 'bar')
-    end
-
     it 'takes arbitrary key/values and exposes them' do
-      expect(subject.user).to eq('user')
-      expect(subject.foo).to eq('bar')
+      presenter = presenter_class.new(project, user: 'user', foo: 'bar')
+
+      expect(presenter.user).to eq('user')
+      expect(presenter.foo).to eq('bar')
     end
   end
 
   describe 'delegation' do
     it 'does not forward missing methods to subject' do
-      expect { subject.foo }.to raise_error(NoMethodError)
+      presenter = presenter_class.new(project)
+
+      expect { presenter.foo }.to raise_error(NoMethodError)
     end
   end
 end
diff --git a/spec/models/concerns/presentable_spec.rb b/spec/models/concerns/presentable_spec.rb
index 6640a5e1377..941647a79fb 100644
--- a/spec/models/concerns/presentable_spec.rb
+++ b/spec/models/concerns/presentable_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
 describe Presentable do
-  let(:build) { create(:ci_build) }
+  let(:build) { Ci::Build.new }
 
   describe '#present' do
     it 'returns a presenter' do
-      expect(build.present).to be_a(Ci::Build::Presenter)
+      expect(build.present).to be_a(Ci::BuildPresenter)
     end
 
     it 'takes optional attributes' do
diff --git a/spec/policies/base_policy_spec.rb b/spec/policies/base_policy_spec.rb
index 439d6436f34..63acc0b68cd 100644
--- a/spec/policies/base_policy_spec.rb
+++ b/spec/policies/base_policy_spec.rb
@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe BasePolicy, models: true do
-  let(:build) { create(:ci_build) }
+  let(:build) { Ci::Build.new }
 
   describe '.class_for' do
     it 'detects policy class based on the subject ancestors' do
@@ -9,7 +9,7 @@ describe BasePolicy, models: true do
     end
 
     it 'detects policy class for a presented subject' do
-      presentee = Ci::Build::Presenter.new(build)
+      presentee = Ci::BuildPresenter.new(build)
 
       expect(described_class.class_for(presentee)).to eq(Ci::BuildPolicy)
     end
diff --git a/spec/presenters/ci/build/presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index ecab84dcbc9..7a35da38b2b 100644
--- a/spec/presenters/ci/build/presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
-describe Ci::Build::Presenter do
+describe Ci::BuildPresenter do
   let(:project) { create(:empty_project) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
   let(:build) { create(:ci_build, pipeline: pipeline) }
 
-  subject do
+  subject(:presenter) do
     described_class.new(build)
   end
 
@@ -15,60 +15,62 @@ describe Ci::Build::Presenter do
 
   describe '#initialize' do
     it 'takes a build and optional params' do
-      expect { subject }.not_to raise_error
+      expect { presenter }.not_to raise_error
     end
 
     it 'exposes build' do
-      expect(subject.build).to eq(build)
+      expect(presenter.build).to eq(build)
     end
 
     it 'forwards missing methods to build' do
-      expect(subject.ref).to eq('master')
+      expect(presenter.ref).to eq('master')
     end
   end
 
   describe '#erased_by_user?' do
     it 'takes a build and optional params' do
-      expect(subject).not_to be_erased_by_user
+      expect(presenter).not_to be_erased_by_user
     end
   end
 
   describe '#erased_by_name' do
     context 'when build is not erased' do
       before do
-        expect(build).to receive(:erased_by).and_return(nil)
+        expect(presenter).to receive(:erased_by_user?).and_return(false)
       end
 
       it 'returns nil' do
-        expect(subject.erased_by_name).to be_nil
+        expect(presenter.erased_by_name).to be_nil
       end
     end
+
     context 'when build is erased' do
       before do
-        expect(build).to receive(:erased_by).twice.
+        expect(presenter).to receive(:erased_by_user?).and_return(true)
+        expect(build).to receive(:erased_by).
           and_return(double(:user, name: 'John Doe'))
       end
 
       it 'returns the name of the eraser' do
-        expect(subject.erased_by_name).to eq('John Doe')
+        expect(presenter.erased_by_name).to eq('John Doe')
       end
     end
   end
 
   describe 'quack like a Ci::Build permission-wise' do
     context 'user is not allowed' do
-      let(:project) { create(:empty_project, public_builds: false) }
+      let(:project) { build_stubbed(:empty_project, public_builds: false) }
 
       it 'returns false' do
-        expect(subject.can?(nil, :read_build)).to be_falsy
+        expect(presenter.can?(nil, :read_build)).to be_falsy
       end
     end
 
     context 'user is allowed' do
-      let(:project) { create(:empty_project, :public) }
+      let(:project) { build_stubbed(:empty_project, :public) }
 
       it 'returns true' do
-        expect(subject.can?(nil, :read_build)).to be_truthy
+        expect(presenter.can?(nil, :read_build)).to be_truthy
       end
     end
   end
diff --git a/spec/presenters/ci/variable/presenter_spec.rb b/spec/presenters/ci/variable/presenter_spec.rb
deleted file mode 100644
index b3afb2e2e33..00000000000
--- a/spec/presenters/ci/variable/presenter_spec.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-require 'spec_helper'
-
-describe Ci::Variable::Presenter do
-  let(:variable) { double(:variable) }
-
-  subject do
-    described_class.new(variable)
-  end
-
-  it 'inherits from Gitlab::View::Presenter::Simple' do
-    expect(described_class.superclass).to eq(Gitlab::View::Presenter::Simple)
-  end
-
-  describe '#initialize' do
-    it 'takes a variable and optional params' do
-      expect { subject }.not_to raise_error
-    end
-
-    it 'exposes variable' do
-      expect(subject.variable).to eq(variable)
-    end
-  end
-end