Improve pipelines design

app/controllers/projects/pipelines_controller.rb

@@ -4,63 +4,28 @@ class Projects::PipelinesController < Projects::ApplicationController
   before_action :authorize_read_pipeline!
   before_action :authorize_create_pipeline!, only: [:new, :create]
   before_action :authorize_update_pipeline!, only: [:retry, :cancel]
-  layout 'project'
 
   def index
     @scope = params[:scope]
     @all_pipelines = project.ci_commits
-    @pipelines = @all_pipelines.order(id: :desc)
-    @pipelines =
-      case @scope
-      when 'running'
-        @pipelines.running_or_pending
-      when 'branches'
-        @branches = project.repository.branches.map(&:name)
-        @branches_ids = @all_pipelines.where(ref: @branches).group(:ref).select('max(id)')
-        @pipelines.where(id: @branches_ids)
-      when 'tags'
-        @tags = project.repository.tags.map(&:name)
-        @tags_ids = @all_pipelines.where(ref: @tags).group(:ref).select('max(id)')
-        @pipelines.where(id: @tags_ids)
-      else
-        @pipelines
-      end
-    @pipelines = @pipelines.page(params[:page]).per(30)
+    @pipelines = PipelinesFinder.new(project).execute(@all_pipelines, @scope)
+    @pipelines = @pipelines.order(id: :desc).page(params[:page]).per(30)
   end
 
   def new
   end
 
   def create
-    ref_names = project.repository.ref_names
-    unless ref_names.include?(params[:ref])
-      @error = 'Reference not found'
-      render action: 'new'
-      return
+    begin
+      pipeline = Ci::CreatePipelineService.new(project, current_user, create_params).execute
+      redirect_to namespace_project_pipeline_path(project.namespace, project, pipeline)
+    rescue ArgumentError => e
+      @error = e.message
+      render 'new'
+    rescue => e
+      @error = 'Undefined error'
+      render 'new'
     end
-
-    commit = project.commit(params[:ref])
-    unless commit
-      @error = 'Commit not found'
-      render action: 'new'
-      return
-    end
-
-    pipeline = project.ci_commits.new(sha: commit.id, ref: params[:ref], before_sha: Gitlab::Git::BLANK_SHA)
-
-    # Skip creating ci_commit when no gitlab-ci.yml is found
-    unless pipeline.config_processor
-      @error = pipeline.yaml_errors || 'Missing .gitlab-ci.yml file'
-      render action: 'new'
-      return
-    end
-
-    Ci::Commit.transaction do
-      commit.save!
-      commit.create_builds(current_user)
-    end
-
-    redirect_to builds_namespace_project_commit_path(project.namespace, project, commit.id)
   end
 
   def show
@@ -70,19 +35,23 @@ class Projects::PipelinesController < Projects::ApplicationController
   end
 
   def retry
-    pipeline.builds.latest.failed.select(&:retryable?).each(&:retry)
+    pipeline.retry_failed
 
     redirect_back_or_default default: namespace_project_pipelines_path(project.namespace, project)
   end
 
   def cancel
-    pipeline.builds.running_or_pending.each(&:cancel)
+    pipeline.cancel_running
 
     redirect_back_or_default default: namespace_project_pipelines_path(project.namespace, project)
   end
 
   private
 
+  def create_params
+    params.permit(:ref)
+  end
+
   def pipeline
     @pipeline ||= project.ci_commits.find_by!(id: params[:id])
   end

app/finders/pipelines_finder.rb

+class PipelinesFinder
+  attr_reader :project
+
+  def initialize(project)
+    @project = project
+  end
+
+  def execute(pipelines, scope)
+    case scope
+    when 'running'
+      pipelines.running_or_pending
+    when 'branches'
+      from_ids(pipelines, ids_for_ref(pipelines, branches))
+    when 'tags'
+      from_ids(pipelines, ids_for_ref(pipelines, tags))
+    else
+      pipelines
+    end
+  end
+
+  private
+
+  def ids_for_ref(pipelines, refs)
+    pipelines.where(ref: refs).group(:ref).select('max(id)')
+  end
+
+  def from_ids(pipelines, ids)
+    pipelines.unscoped.where(id: ids)
+  end
+
+  def branches
+    project.repository.branches.map(&:name)
+  end
+
+  def tags
+    project.repository.tags.map(&:name)
+  end
+end

app/helpers/ci_status_helper.rb

@@ -50,6 +50,13 @@ module CiStatusHelper
     render_status_with_link('pipeline', pipeline.status, path, tooltip_placement)
   end
 
+  def no_runners_for_project?(project)
+    project.runners.blank? &&
+      Ci::Runner.shared.blank?
+  end
+
+  private
+
   def render_status_with_link(type, status, path, tooltip_placement)
     link_to ci_icon_for_status(status),
             path,
@@ -57,9 +64,4 @@ module CiStatusHelper
             title: "#{type.titleize}: #{ci_label_for_status(status)}",
             data: { toggle: 'tooltip', placement: tooltip_placement }
   end
-
-  def no_runners_for_project?(project)
-    project.runners.blank? &&
-      Ci::Runner.shared.blank?
-  end
 end

app/models/ci/commit.rb

@@ -89,6 +89,14 @@ module Ci
       end
     end
 
+    def cancel_running
+      builds.running_or_pending.each(&:cancel)
+    end
+
+    def retry_failed
+      builds.latest.failed.select(&:retryable?).each(&:retry)
+    end
+
     def latest?
       return false unless ref
       commit = project.commit(ref)

app/services/ci/create_pipeline_service.rb

+module Ci
+  class CreatePipelineService < BaseService
+    def execute
+      unless ref_names.include?(params[:ref])
+        raise ArgumentError, 'Reference not found'
+      end
+
+      unless commit
+        raise ArgumentError, 'Commit not found'
+      end
+
+      unless can?(current_user, :create_pipeline, project)
+        raise RuntimeError, 'Insufficient permissions to create a new pipeline'
+      end
+
+      Ci::Commit.transaction do
+        unless pipeline.config_processor
+          raise ArgumentError, pipeline.yaml_errors || 'Missing .gitlab-ci.yml file'
+        end
+
+        pipeline.save!
+        pipeline.create_builds(current_user)
+      end
+
+      pipeline
+    end
+
+    private
+
+    def ref_names
+      @ref_names ||= project.repository.ref_names
+    end
+
+    def commit
+      @commit ||= project.commit(params[:ref])
+    end
+
+    def pipeline
+      @pipeline ||= project.ci_commits.new(sha: commit.id, ref: params[:ref], before_sha: Gitlab::Git::BLANK_SHA)
+    end
+  end
+end
\ No newline at end of file

app/views/layouts/nav/_project.html.haml

@@ -39,12 +39,12 @@
           Commits
 
   - if project_nav_tab? :builds
-    = nav_link(controller: %w(pipelines)) do
-      = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-builds' do
+    = nav_link(controller: :pipelines) do
+      = link_to project_pipelines_path(@project), title: 'Pipelines', class: 'shortcuts-pipelines' do
         = icon('ship fw')
         %span
           Pipelines
-          %span.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count(:all))
+          %span.count.ci_counter= number_with_delimiter(@project.ci_commits.running_or_pending.count)
 
     = nav_link(controller: %w(builds)) do
       = link_to project_builds_path(@project), title: 'Builds', class: 'shortcuts-builds' do

app/views/projects/ci/builds/_build.html.haml

@@ -13,9 +13,9 @@
       %strong ##{build.id}
 
     - if build.stuck?
-      %i.fa.fa-warning.text-warning.has-tooltip(title="Build is stuck. Check runners.")
+      = icon('warning', class: 'text-warning has-tooltip', title: 'Build is stuck. Check runners.')
     - if defined?(retried) && retried
-      %i.fa.fa-warning.has-tooltip(title="Build was retried")
+      = icon('warning', class: 'text-warning has-tooltip', title: 'Build was retried.')
 
   - if defined?(commit_sha) && commit_sha
     %td
@@ -59,24 +59,24 @@
     %td.duration
       - if build.duration
         %p
-          %i.fa.fa-clock-o
+          = icon("clock-o")
            
           #{duration_in_words(build.finished_at, build.started_at)}
       - if build.finished_at
         %p
-          %i.fa.fa-calendar
+          = icon("calendar")
            
           #{time_ago_with_tooltip(build.finished_at)}
   - else
     %td.duration
       - if build.duration
-        %i.fa.fa-clock-o
+        = icon("clock-o")
          
         #{duration_in_words(build.finished_at, build.started_at)}
 
     %td.timestamp
       - if build.finished_at
-        %i.fa.fa-calendar
+        = icon("calendar")
          
         %span #{time_ago_with_tooltip(build.finished_at)}
 
@@ -89,11 +89,11 @@
     .pull-right
       - if can?(current_user, :read_build, build) && build.artifacts?
         = link_to download_namespace_project_build_artifacts_path(build.project.namespace, build.project, build), title: 'Download artifacts', class: 'btn btn-build' do
-          %i.fa.fa-download
+          = icon('download')
       - if can?(current_user, :update_build, build)
         - if build.active?
           = link_to cancel_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Cancel', class: 'btn btn-build' do
-            %i.fa.fa-remove.cred
+            = icon('remove', class: 'cred')
         - elsif defined?(allow_retry) && allow_retry && build.retryable?
           = link_to retry_namespace_project_build_path(build.project.namespace, build.project, build, return_to: request.original_url), method: :post, title: 'Retry', class: 'btn btn-build' do
-            %i.fa.fa-refresh
+            = icon('refresh')

app/views/projects/ci/commits/_commit.html.haml

@@ -42,12 +42,12 @@
   %td
     - if commit.started_at && commit.finished_at
       %p
-        %i.fa.fa-clock-o
+        = icon("clock-o")
          
         #{duration_in_words(commit.finished_at, commit.started_at)}
     - if commit.finished_at
       %p
-        %i.fa.fa-calendar
+        = icon("calendar")
          
         #{time_ago_with_tooltip(commit.finished_at)}
 
@@ -63,7 +63,7 @@
             - artifacts.each do |build|
               %li
                 = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, build), rel: 'nofollow' do
-                  %i.fa.fa-download
+                  = icon("download")
                   %span #{build.name}
 
       - if can?(current_user, :update_pipeline, @project)
@@ -74,4 +74,4 @@
          
         - if commit.active?
           = link_to cancel_namespace_project_pipeline_path(@project.namespace, @project, commit.id), class: 'btn btn-remove has-tooltip', title: "Cancel", method: :post do
-            = icon("remove cred")
+            = icon("remove", class: "cred")

spec/models/project_spec.rb

@@ -62,7 +62,7 @@ describe Project, models: true do
     it { is_expected.to have_one(:pushover_service).dependent(:destroy) }
     it { is_expected.to have_one(:asana_service).dependent(:destroy) }
     it { is_expected.to have_many(:commit_statuses) }
-    it { is_expected.to have_many(:pipelines) }
+    it { is_expected.to have_many(:ci_commits) }
     it { is_expected.to have_many(:builds) }
     it { is_expected.to have_many(:runner_projects) }
     it { is_expected.to have_many(:runners) }