PoC of using a presenter for the `projects#show` view23563-presenters-poc

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

21 files changed, 179 insertions, 90 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 517ad4f03f3..c8175e97ee9 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -102,6 +102,8 @@ class ApplicationController < ActionController::Base
   end
 
   def can?(object, action, subject)
+    subject = subject.object if subject.try(:presenter?)
+
     Ability.allowed?(object, action, subject)
   end
 
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index a8a18b4fa16..2b9d9109dff 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -292,8 +292,11 @@ class ProjectsController < Projects::ApplicationController
   # pages list order: repository readme, wiki home, issues list, customize workflow
   def render_landing_page
     if @project.feature_available?(:repository, current_user)
-      return render 'projects/no_repo' unless @project.repository_exists?
-      render 'projects/empty' if @project.empty_repo?
+      if !@project.repository_exists?
+        present_and_render 'projects/no_repo'
+      elsif @project.empty_repo?
+        present_and_render 'projects/empty'
+      end
     else
       if @project.wiki_enabled?
         @project_wiki = @project.wiki
@@ -302,9 +305,13 @@ class ProjectsController < Projects::ApplicationController
         @issues = issues_collection
         @issues = @issues.page(params[:page])
       end
-
-      render :show
     end
+    present_and_render :show
+  end
+
+  def present_and_render(view)
+    @project = ProjectPresenter.new(@project, current_user)
+    render view
   end
 
   def determine_layout
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index c816b616631..b43b9d90bea 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -32,14 +32,7 @@ module ApplicationHelper
     args.any? { |v| v.to_s.downcase == action_name }
   end
 
-  def project_icon(project_id, options = {})
-    project =
-      if project_id.is_a?(Project)
-        project_id
-      else
-        Project.find_with_namespace(project_id)
-      end
-
+  def project_icon(project, options = {})
     if project.avatar_url
       image_tag project.avatar_url, options
     else # generated icon
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 45a567a1eba..fac45b16da5 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -27,7 +27,7 @@ module ApplicationSettingsHelper
     current_application_settings.enabled_git_access_protocol.present?
   end
 
-  def enabled_protocol
+  def enabled_git_access_protocol
     case current_application_settings.enabled_git_access_protocol
     when 'http'
       gitlab_config.protocol
@@ -36,15 +36,6 @@ module ApplicationSettingsHelper
     end
   end
 
-  def enabled_project_button(project, protocol)
-    case protocol
-    when 'ssh'
-      ssh_clone_button(project, 'bottom', append_link: false)
-    else
-      http_clone_button(project, 'bottom', append_link: false)
-    end
-  end
-
   # Return a group of checkboxes that use Bootstrap's button plugin for a
   # toggle button effect.
   def restricted_level_checkboxes(help_block_id)
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 3fc85dc6b2b..f76351b375f 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -23,6 +23,7 @@ module BranchesHelper
   def can_push_branch?(project, branch_name)
     return false unless project.repository.branch_exists?(branch_name)
 
+    project = project.object if project.try(:presenter?)
     ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch_name)
   end
 
diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb
index dee3c78df47..8daf0e694a4 100644
--- a/app/helpers/button_helper.rb
+++ b/app/helpers/button_helper.rb
@@ -26,6 +26,15 @@ module ButtonHelper
       title: title
   end
 
+  def clone_button(project)
+    case enabled_git_access_protocol
+    when 'ssh'
+      ssh_clone_button(project, 'bottom', append_link: false)
+    else
+      http_clone_button(project, 'bottom', append_link: false)
+    end
+  end
+
   def http_clone_button(project, placement = 'right', append_link: true)
     klass = 'http-selector'
     klass << ' has-tooltip' if current_user.try(:require_password?)
diff --git a/app/helpers/members_helper.rb b/app/helpers/members_helper.rb
index 877c77050be..d648cf7e00d 100644
--- a/app/helpers/members_helper.rb
+++ b/app/helpers/members_helper.rb
@@ -34,6 +34,6 @@ module MembersHelper
 
   def leave_confirmation_message(member_source)
     "Are you sure you want to leave the " \
-    "\"#{member_source.human_name}\" #{member_source.class.to_s.humanize(capitalize: false)}?"
+    "\"#{member_source.human_name}\" #{member_source.model_name.humanize(capitalize: false)}?"
   end
 end
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 42c00ec3cd5..a91717cd9a2 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -111,9 +111,9 @@ module ProjectsHelper
   end
 
   def license_short_name(project)
-    return 'LICENSE' if project.repository.license_key.nil?
+    return 'LICENSE' if project.license_key.nil?
 
-    license = Licensee::License.new(project.repository.license_key)
+    license = Licensee::License.new(project.license_key)
 
     license.nickname || license.name
   end
@@ -254,7 +254,7 @@ module ProjectsHelper
     number_to_human_size(size_in_bytes, delimiter: ',', precision: 2)
   end
 
-  def default_url_to_repo(project = @project)
+  def default_url_to_repo(project)
     case default_clone_protocol
     when 'ssh'
       project.ssh_url_to_repo
@@ -265,7 +265,7 @@ module ProjectsHelper
 
   def default_clone_protocol
     if allowed_protocols_present?
-      enabled_protocol
+      enabled_git_access_protocol
     else
       if !current_user || current_user.require_ssh_key?
         gitlab_config.protocol
diff --git a/app/presenters/base_presenter.rb b/app/presenters/base_presenter.rb
new file mode 100644
index 00000000000..bb97bca55b1
--- /dev/null
+++ b/app/presenters/base_presenter.rb
@@ -0,0 +1,18 @@
+require 'forwardable'
+
+module BasePresenter
+  extend Forwardable
+
+  attr_reader :object, :current_user
+
+  def_delegators :@object, :persisted?, :id, :model_name, :to_param
+
+  def initialize(object, current_user = nil)
+    @object = object
+    @current_user = current_user
+  end
+
+  def presenter?
+    true
+  end
+end
diff --git a/app/presenters/project_presenter.rb b/app/presenters/project_presenter.rb
new file mode 100644
index 00000000000..93a0b30c72a
--- /dev/null
+++ b/app/presenters/project_presenter.rb
@@ -0,0 +1,69 @@
+require_relative 'base_presenter'
+
+class ProjectPresenter
+  extend Forwardable
+  include BasePresenter
+
+  def_delegators :@object, :path, :name, :name_with_namespace
+  def_delegators :@object, :namespace, :group, :members, :issues, :merge_requests
+  def_delegators :@object, :repository, :pipelines
+  def_delegators :@object, :repository_size, :avatar_url
+  def_delegators :@object, :visibility_level, :default_branch, :description
+  def_delegators :@object, :repo_exists?, :empty_repo?, :default_issues_tracker?
+  def_delegators :@object, :archived?, :protected_branch?
+  def_delegators :@object, :allowed_to_share_with_group?
+  def_delegators :@object, :forked_from_project, :external_wiki
+  def_delegators :@object, :forks_count, :star_count, :commit_count
+  def_delegators :@object, :http_url_to_repo, :ssh_url_to_repo
+
+  def feature_available?(feature, _ = nil)
+    object.feature_available?(:repository, current_user)
+  end
+
+  def download_code_possible?
+    Ability.allowed?(current_user, :download_code, object)
+  end
+
+  def push_code_possible?
+    Ability.allowed?(current_user, :push_code, object)
+  end
+
+  def fork_possible?
+    Ability.allowed?(current_user, :fork_project, object)
+  end
+
+  def remove_possible?
+    Ability.allowed?(current_user, :remove_project, object)
+  end
+
+  def destroy_current_member_possible?
+    # We don't use @project.team.find_member because it searches for group members too...
+    current_member = object.members.find_by(user_id: current_user.id)
+
+    current_member && Ability.allowed?(current_user, :destroy_project_member, current_member)
+  end
+
+  def admin_possible?
+    Ability.allowed?(current_user, :admin_project, object)
+  end
+
+  def forked_from_project?
+    @forked_from_project ||= object.forked_from_project.present?
+  end
+
+  def already_forked_by_current_user?
+    current_user.already_forked?(object)
+  end
+
+  def current_user_fork
+    current_user.fork_of(object)
+  end
+
+  def starred?
+    current_user.starred?(object)
+  end
+
+  def license_key
+    object.repository.license_key
+  end
+end
diff --git a/app/views/layouts/nav/_project.html.haml b/app/views/layouts/nav/_project.html.haml
index 99a58bbb676..b4e0c10da2f 100644
--- a/app/views/layouts/nav/_project.html.haml
+++ b/app/views/layouts/nav/_project.html.haml
@@ -5,10 +5,8 @@
         = icon('cog')
         = icon('caret-down')
       %ul.dropdown-menu.dropdown-menu-align-right
-        - can_edit = can?(current_user, :admin_project, @project)
-        -# We don't use @project.team.find_member because it searches for group members too...
-        - member = @project.members.find_by(user_id: current_user.id)
-        - can_leave = member && can?(current_user, :destroy_project_member, member)
+        - can_edit = @project.admin_possible?
+        - can_leave = @project.destroy_current_member_possible?
 
         = render 'layouts/nav/project_settings', can_edit: can_edit
 
diff --git a/app/views/layouts/project.html.haml b/app/views/layouts/project.html.haml
index 277eb71ea73..eb2c12acb6b 100644
--- a/app/views/layouts/project.html.haml
+++ b/app/views/layouts/project.html.haml
@@ -11,7 +11,7 @@
     - preview_markdown_path = preview_markdown_namespace_project_path(project.namespace, project)
   - if current_user
     :javascript
-      window.project_uploads_path = "#{namespace_project_uploads_path project.namespace,project}";
+      window.project_uploads_path = "#{namespace_project_uploads_path(project.namespace, project)}";
       window.preview_markdown_path = "#{preview_markdown_path}";
 
 - content_for :header_content do
diff --git a/app/views/projects/_home_panel.html.haml b/app/views/projects/_home_panel.html.haml
index 5a04c3318cf..555706e36ac 100644
--- a/app/views/projects/_home_panel.html.haml
+++ b/app/views/projects/_home_panel.html.haml
@@ -1,28 +1,29 @@
-- empty_repo = @project.empty_repo?
-.project-home-panel.text-center{ class: ("empty-project" if empty_repo) }
+- project = local_assigns.fetch(:project)
+
+.project-home-panel.text-center{ class: ("empty-project" if project.empty_repo?) }
   %div{ class: container_class }
     .avatar-container.s70.project-avatar
-      = project_icon(@project, alt: @project.name, class: 'avatar s70 avatar-tile')
+      = project_icon(project, alt: project.name, class: 'avatar s70 avatar-tile')
     %h1.project-title
       = @project.name
-      %span.visibility-icon.has-tooltip{data: { container: 'body' }, title: visibility_icon_description(@project)}
-        = visibility_level_icon(@project.visibility_level, fw: false)
+      %span.visibility-icon.has-tooltip{data: { container: 'body' }, title: visibility_icon_description(project)}
+        = visibility_level_icon(project.visibility_level, fw: false)
 
     .project-home-desc
-      - if @project.description.present?
-        = markdown_field(@project, :description)
+      - if project.description.present?
+        = markdown_field(project.object, :description)
 
-      - if forked_from_project = @project.forked_from_project
+      - if project.forked_from_project?
         %p
           Forked from
-          = link_to project_path(forked_from_project) do
-            = forked_from_project.namespace.try(:name)
+          = link_to project_path(project.forked_from_project) do
+            = project.forked_from_project.namespace.try(:name)
 
     .project-repo-buttons.project-action-buttons
       .count-buttons
-        = render 'projects/buttons/star'
-        = render 'projects/buttons/fork'
+        = render 'projects/buttons/star', project: @project
+        = render 'projects/buttons/fork', project: @project
 
-      - if @project.feature_available?(:repository, current_user)
+      - if project.feature_available?(:repository)
         .project-clone-holder
-          = render "shared/clone_panel"
+          = render 'shared/clone_panel', project: @project
diff --git a/app/views/projects/buttons/_fork.html.haml b/app/views/projects/buttons/_fork.html.haml
index 27da86b9efe..bf753e63d51 100644
--- a/app/views/projects/buttons/_fork.html.haml
+++ b/app/views/projects/buttons/_fork.html.haml
@@ -1,14 +1,16 @@
-- unless @project.empty_repo?
-  - if current_user && can?(current_user, :fork_project, @project)
-    - if current_user.already_forked?(@project) && current_user.manageable_namespaces.size < 2
-      = link_to namespace_project_path(current_user, current_user.fork_of(@project)), title: 'Go to your fork', class: 'btn has-tooltip' do
-        = custom_icon('icon_fork')
-        %span Fork
-    - else
-      = link_to new_namespace_project_fork_path(@project.namespace, @project), title: 'Fork project', class: 'btn' do
-        = custom_icon('icon_fork')
-        %span Fork
-    %div.count-with-arrow
-      %span.arrow
-      = link_to namespace_project_forks_path(@project.namespace, @project), title: 'Forks', class: 'count' do
-        = @project.forks_count
+- project = local_assigns.fetch(:project)
+- return if project.empty_repo? || !project.fork_possible?
+
+- if project.already_forked_by_current_user? && current_user.manageable_namespaces.one?
+  = link_to namespace_project_path(current_user, project.current_user_fork), title: 'Go to your fork', class: 'btn has-tooltip' do
+    = custom_icon('icon_fork')
+    %span Fork
+- else
+  = link_to new_namespace_project_fork_path(project.namespace, project), title: 'Fork project', class: 'btn' do
+    = custom_icon('icon_fork')
+    %span Fork
+
+%div.count-with-arrow
+  %span.arrow
+  = link_to namespace_project_forks_path(project.namespace, project), title: 'Forks', class: 'count' do
+    = project.forks_count
diff --git a/app/views/projects/buttons/_star.html.haml b/app/views/projects/buttons/_star.html.haml
index 12d35101770..d3b0679fb2d 100644
--- a/app/views/projects/buttons/_star.html.haml
+++ b/app/views/projects/buttons/_star.html.haml
@@ -1,21 +1,19 @@
+- project = local_assigns.fetch(:project)
+
 - if current_user
-  = link_to toggle_star_namespace_project_path(@project.namespace, @project), { class: 'btn star-btn toggle-star', method: :post, remote: true } do
-    - if current_user.starred?(@project)
+  = link_to toggle_star_namespace_project_path(project.namespace, project), { class: 'btn star-btn toggle-star', method: :post, remote: true } do
+    - if project.starred?
       = icon('star')
       %span.starred Unstar
     - else
       = icon('star-o')
       %span Star
-  %div.count-with-arrow
-    %span.arrow
-    %span.count.star-count
-      = @project.star_count
-
 - else
   = link_to new_user_session_path, class: 'btn has-tooltip star-btn', title: 'You must sign in to star a project' do
     = icon('star')
     Star
-  %div.count-with-arrow
-    %span.arrow
-    %span.count
-      = @project.star_count
+
+%div.count-with-arrow
+  %span.arrow
+  %span.count.star-count
+    = project.star_count
diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml
index 0aa8801c2d8..4dc3caa1bcf 100644
--- a/app/views/projects/edit.html.haml
+++ b/app/views/projects/edit.html.haml
@@ -119,7 +119,7 @@
           .form-group
             - if @project.avatar?
               .avatar-container.s160
-                = project_icon("#{@project.namespace.to_param}/#{@project.to_param}", alt: '', class: 'avatar project-avatar s160')
+                = project_icon(@project, alt: '', class: 'avatar project-avatar s160')
             %p.light
               - if @project.avatar_in_git
                 Project avatar in repository: #{ @project.avatar_in_git }
diff --git a/app/views/projects/empty.html.haml b/app/views/projects/empty.html.haml
index 7a39064adc5..a80c3edb64a 100644
--- a/app/views/projects/empty.html.haml
+++ b/app/views/projects/empty.html.haml
@@ -1,16 +1,16 @@
 - @no_container = true
 
 = content_for :flash_message do
-  - if current_user && can?(current_user, :download_code, @project)
+  - if @project.download_code_possible?
     = render 'shared/no_ssh'
     = render 'shared/no_password'
 
-= render "home_panel"
+= render "home_panel", project: @project
 
 .row-content-block.second-block.center
   %h3.page-title
     The repository for this project is empty
-  - if can?(current_user, :push_code, @project)
+  - if @project.push_code_possible?
     %p
       If you already have files you can push them using command line instructions below.
     %p
@@ -26,7 +26,7 @@
     %p
       You will need to be owner or have the master permission level for the initial push, as the master branch is automatically protected.
 
-- if can?(current_user, :push_code, @project)
+- if @project.push_code_possible?
   %div{ class: container_class }
     .prepend-top-20
     .empty_wrapper
@@ -44,7 +44,7 @@
           %h5 Create a new repository
           %pre.light-well
             :preserve
-              git clone #{ content_tag(:span, default_url_to_repo, class: 'clone')}
+              git clone #{content_tag(:span, default_url_to_repo(@project), class: 'clone')}
               cd #{h @project.path}
               touch README.md
               git add README.md
@@ -57,11 +57,13 @@
             :preserve
               cd existing_folder
               git init
-              git remote add origin #{ content_tag(:span, default_url_to_repo, class: 'clone')}
+              git remote add origin #{content_tag(:span, default_url_to_repo(@project), class: 'clone')}
               git add .
               git commit
               git push -u origin master
 
-          - if can? current_user, :remove_project, @project
+          - if @project.remove_possible?
             .prepend-top-20
-              = link_to 'Remove project', [@project.namespace.becomes(Namespace), @project], data: { confirm: remove_project_message(@project)}, method: :delete, class: "btn btn-remove pull-right"
+              = link_to 'Remove project', project_url(@project),
+                data: { confirm: remove_project_message(@project)},
+                method: :delete, class: "btn btn-remove pull-right"
diff --git a/app/views/projects/show.html.haml b/app/views/projects/show.html.haml
index 4de95036eef..210da101dc5 100644
--- a/app/views/projects/show.html.haml
+++ b/app/views/projects/show.html.haml
@@ -10,7 +10,7 @@
     = render 'shared/no_password'
 
 = render 'projects/last_push'
-= render "home_panel"
+= render 'home_panel', project: @project
 
 - if current_user && can?(current_user, :download_code, @project)
   %nav.project-stats{ class: container_class }
diff --git a/app/views/shared/_clone_panel.html.haml b/app/views/shared/_clone_panel.html.haml
index 3b82d8e686f..2a67dfcc23a 100644
--- a/app/views/shared/_clone_panel.html.haml
+++ b/app/views/shared/_clone_panel.html.haml
@@ -1,11 +1,11 @@
-- project = project || @project
+- project = local_assigns.fetch(:project)
 
 .git-clone-holder.input-group
   .input-group-btn
     -if allowed_protocols_present?
       .clone-dropdown-btn.btn.btn-static
         %span
-          = enabled_project_button(project, enabled_protocol)
+          = clone_button(project)
     - else
       %a#clone-dropdown.clone-dropdown-btn.btn{href: '#', data: { toggle: 'dropdown' }}
         %span
diff --git a/spec/helpers/application_helper_spec.rb b/spec/helpers/application_helper_spec.rb
index c706e418d26..408274a77bb 100644
--- a/spec/helpers/application_helper_spec.rb
+++ b/spec/helpers/application_helper_spec.rb
@@ -58,7 +58,7 @@ describe ApplicationHelper do
       project = create(:project, avatar: File.open(avatar_file_path))
 
       avatar_url = "http://localhost/uploads/project/avatar/#{project.id}/banana_sample.gif"
-      expect(helper.project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).
+      expect(helper.project_icon(project).to_s).
         to eq "<img src=\"#{avatar_url}\" alt=\"Banana sample\" />"
     end
 
@@ -68,7 +68,7 @@ describe ApplicationHelper do
       allow_any_instance_of(Project).to receive(:avatar_in_git).and_return(true)
 
       avatar_url = 'http://localhost' + namespace_project_avatar_path(project.namespace, project)
-      expect(helper.project_icon("#{project.namespace.to_param}/#{project.to_param}").to_s).to match(
+      expect(helper.project_icon(project).to_s).to match(
         image_tag(avatar_url))
     end
   end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 8113742923b..94f688700b7 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -97,17 +97,15 @@ describe ProjectsHelper do
   end
 
   describe '#license_short_name' do
-    let(:project) { create(:project) }
-
     context 'when project.repository has a license_key' do
       it 'returns the nickname of the license if present' do
-        allow(project.repository).to receive(:license_key).and_return('agpl-3.0')
+        project = double(:project, license_key: 'agpl-3.0')
 
         expect(helper.license_short_name(project)).to eq('GNU AGPLv3')
       end
 
       it 'returns the name of the license if nickname is not present' do
-        allow(project.repository).to receive(:license_key).and_return('mit')
+        project = double(:project, license_key: 'mit')
 
         expect(helper.license_short_name(project)).to eq('MIT License')
       end
@@ -115,7 +113,7 @@ describe ProjectsHelper do
 
     context 'when project.repository has no license_key but a license_blob' do
       it 'returns LICENSE' do
-        allow(project.repository).to receive(:license_key).and_return(nil)
+        project = double(:project, license_key: nil)
 
         expect(helper.license_short_name(project)).to eq('LICENSE')
       end