Merge branch '26943-fix-user-in-build-presenter' into 'master'

Don't override Ci::Build#user when presenting a build

Closes #26943

See merge request !8668

app/controllers/projects/builds_controller.rb

@@ -94,7 +94,7 @@ class Projects::BuildsController < Projects::ApplicationController
   private
 
   def build
-    @build ||= project.builds.find_by!(id: params[:id]).present(user: current_user)
+    @build ||= project.builds.find_by!(id: params[:id]).present(current_user: current_user)
   end
 
   def build_path(build)

app/presenters/README.md

@@ -113,7 +113,7 @@ detects the presenter based on the presented subject's class.
 class Projects::LabelsController < Projects::ApplicationController
   def edit
     @label = Gitlab::View::Presenter::Factory
-      .new(@label, user: current_user)
+      .new(@label, current_user: current_user)
       .fabricate!
   end
 end
@@ -132,7 +132,7 @@ and then in the controller:
 ```ruby
 class Projects::LabelsController < Projects::ApplicationController
   def edit
-    @label = @label.present(user: current_user)
+    @label = @label.present(current_user: current_user)
   end
 end
 ```
@@ -147,7 +147,7 @@ end
 You can also present the model in the view:
 
 ```ruby
-- label = @label.present(current_user)
+- label = @label.present(current_user: current_user)
 
 %div{ class: label.text_color }
   = render partial: label, label: label

lib/gitlab/view/presenter/base.rb

@@ -1,6 +1,8 @@
 module Gitlab
   module View
     module Presenter
+      CannotOverrideMethodError = Class.new(StandardError)
+
       module Base
         extend ActiveSupport::Concern
 

lib/gitlab/view/presenter/delegated.rb

@@ -8,6 +8,10 @@ module Gitlab
           @subject = subject
 
           attributes.each do |key, value|
+            if subject.respond_to?(key)
+              raise CannotOverrideMethodError.new("#{subject} already respond to #{key}!")
+            end
+
             define_singleton_method(key) { value }
           end
 

spec/lib/gitlab/view/presenter/delegated_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::View::Presenter::Delegated do
-  let(:project) { double(:project, bar: 'baz') }
+  let(:project) { double(:project, user: 'John Doe') }
   let(:presenter_class) do
     Class.new(described_class)
   end
@@ -12,10 +12,14 @@ describe Gitlab::View::Presenter::Delegated do
 
   describe '#initialize' do
     it 'takes arbitrary key/values and exposes them' do
-      presenter = presenter_class.new(project, user: 'user', foo: 'bar')
+      presenter = presenter_class.new(project, current_user: 'Jane Doe')
 
-      expect(presenter.user).to eq('user')
-      expect(presenter.foo).to eq('bar')
+      expect(presenter.current_user).to eq('Jane Doe')
+    end
+
+    it 'raise an error if the presentee already respond to method' do
+      expect { presenter_class.new(project, user: 'Jane Doe') }.
+        to raise_error Gitlab::View::Presenter::CannotOverrideMethodError
     end
   end
 
@@ -23,7 +27,7 @@ describe Gitlab::View::Presenter::Delegated do
     it 'forwards missing methods to subject' do
       presenter = presenter_class.new(project)
 
-      expect(presenter.bar).to eq('baz')
+      expect(presenter.user).to eq('John Doe')
     end
   end
 end

spec/lib/gitlab/view/presenter/factory_spec.rb

@@ -22,13 +22,6 @@ describe Gitlab::View::Presenter::Factory do
   end
 
   describe '#fabricate!' do
-    it 'exposes given params' do
-      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
       presenter = described_class.new(build).fabricate!
 

spec/lib/gitlab/view/presenter/simple_spec.rb

@@ -1,7 +1,7 @@
 require 'spec_helper'
 
 describe Gitlab::View::Presenter::Simple do
-  let(:project) { double(:project) }
+  let(:project) { double(:project, user: 'John Doe') }
   let(:presenter_class) do
     Class.new(described_class)
   end
@@ -12,10 +12,15 @@ describe Gitlab::View::Presenter::Simple do
 
   describe '#initialize' do
     it 'takes arbitrary key/values and exposes them' do
-      presenter = presenter_class.new(project, user: 'user', foo: 'bar')
+      presenter = presenter_class.new(project, current_user: 'Jane Doe')
 
-      expect(presenter.user).to eq('user')
-      expect(presenter.foo).to eq('bar')
+      expect(presenter.current_user).to eq('Jane Doe')
+    end
+
+    it 'override the presentee attributes' do
+      presenter = presenter_class.new(project, user: 'Jane Doe')
+
+      expect(presenter.user).to eq('Jane Doe')
     end
   end
 
@@ -23,7 +28,7 @@ describe Gitlab::View::Presenter::Simple do
     it 'does not forward missing methods to subject' do
       presenter = presenter_class.new(project)
 
-      expect { presenter.foo }.to raise_error(NoMethodError)
+      expect { presenter.user }.to raise_error(NoMethodError)
     end
   end
 end