Merge branch 'ci_projects_order' into 'master'

Fix: ci projects order

https://gitlab.com/gitlab-org/gitlab-ce/issues/2553

See merge request !1333

8 files changed, 93 insertions, 95 deletions

diff --git a/app/controllers/ci/projects_controller.rb b/app/controllers/ci/projects_controller.rb
index 6483a84ee91..653384b7178 100644
--- a/app/controllers/ci/projects_controller.rb
+++ b/app/controllers/ci/projects_controller.rb
@@ -5,38 +5,32 @@ module Ci
     before_action :authenticate_user!, except: [:build, :badge, :index, :show]
     before_action :authenticate_public_page!, only: :show
     before_action :project, only: [:build, :integration, :show, :badge, :edit, :update, :destroy, :toggle_shared_runners, :dumped_yaml]
-    before_action :authorize_access_project!, except: [:build, :gitlab, :badge, :index, :show, :new, :create]
+    before_action :authorize_access_project!, except: [:build, :badge, :index, :show, :new, :create]
     before_action :authorize_manage_project!, only: [:edit, :integration, :update, :destroy, :toggle_shared_runners, :dumped_yaml]
     before_action :authenticate_token!, only: [:build]
     before_action :no_cache, only: [:badge]
     protect_from_forgery except: :build
 
-    layout 'ci/project', except: [:index, :gitlab]
+    layout 'ci/project', except: :index
 
     def index
-      @projects = Ci::Project.ordered_by_last_commit_date.public_only.page(params[:page]) unless current_user
-    end
-
-    def gitlab
       @limit, @offset = (params[:limit] || PROJECTS_BATCH).to_i, (params[:offset] || 0).to_i
       @page = @offset == 0 ? 1 : (@offset / @limit + 1)
 
-      @gl_projects = current_user.authorized_projects
-      @gl_projects = @gl_projects.where("name LIKE ?", "%#{params[:search]}%") if params[:search]
-      @gl_projects = @gl_projects.page(@page).per(@limit)
+      if current_user
+        @projects = ProjectListBuilder.new.execute(current_user, params[:search])
 
-      @projects = Ci::Project.where(gitlab_id: @gl_projects.map(&:id)).ordered_by_last_commit_date
-      @total_count = @gl_projects.size
+        @projects = @projects.page(@page).per(@limit)
 
-      @gl_projects = @gl_projects.where.not(id: @projects.map(&:gitlab_id))
+        @total_count = @projects.size
+      end
 
       respond_to do |format|
         format.json do
-          pager_json("ci/projects/gitlab", @total_count)
+          pager_json("ci/projects/index", @total_count)
         end
+        format.html
       end
-    rescue
-      @error = 'Failed to fetch GitLab projects'
     end
 
     def show
diff --git a/app/views/ci/projects/_gl_projects.html.haml b/app/views/ci/projects/_gl_projects.html.haml
deleted file mode 100644
index 7bd30b37caf..00000000000
--- a/app/views/ci/projects/_gl_projects.html.haml
+++ /dev/null
@@ -1,15 +0,0 @@
-- @gl_projects.sort_by(&:name_with_namespace).each do |project|
-  %tr.light
-    %td
-      = project.name_with_namespace
-    %td
-      %small Not added to CI
-    %td
-    %td
-      - if Ci::Project.already_added?(project)
-        %strong.cgreen
-          Added
-      - else
-        = form_tag ci_projects_path do
-          = hidden_field_tag :project, project.to_json(methods: [:name_with_namespace, :path_with_namespace, :ssh_url_to_repo])
-          = submit_tag 'Add project to CI', class: 'btn btn-default btn-sm'
diff --git a/app/views/ci/projects/_project.html.haml b/app/views/ci/projects/_project.html.haml
index e4a811119e1..869747b6eb1 100644
--- a/app/views/ci/projects/_project.html.haml
+++ b/app/views/ci/projects/_project.html.haml
@@ -1,22 +1,36 @@
-- last_commit = project.last_commit
-%tr.alert{class: commit_status_alert_class(last_commit) }
-  %td
-    = link_to [:ci, project] do
-      = project.name
-  %td
-    - if last_commit
-      #{last_commit.status} (#{commit_link(last_commit)})
-      - if project.last_commit_date
-        = time_ago_in_words project.last_commit_date
-        ago
-    - else
-      No builds yet
-  %td
-    - if project.public
-      %i.fa.fa-globe
-      Public
-    - else
-      %i.fa.fa-lock
-      Private
-  %td
-    = project.commits.count
+- if project.gitlab_ci_project
+  - ci_project = project.gitlab_ci_project
+  - last_commit = ci_project.last_commit
+  %tr.alert{class: commit_status_alert_class(last_commit) }
+    %td
+      = link_to [:ci, ci_project] do
+        = ci_project.name
+    %td
+      - if last_commit
+        #{last_commit.status} (#{commit_link(last_commit)})
+        - if ci_project.last_commit_date
+          = time_ago_in_words ci_project.last_commit_date
+          ago
+      - else
+        No builds yet
+    %td
+      - if ci_project.public
+        %i.fa.fa-globe
+        Public
+      - else
+        %i.fa.fa-lock
+        Private
+    %td
+      = ci_project.commits.count
+- else
+  %tr.light
+    %td
+      = project.name_with_namespace
+    %td
+      %small Not added to CI
+    %td
+    %td
+      = form_tag ci_projects_path do
+        = hidden_field_tag :project, project.to_json(methods: [:name_with_namespace, :path_with_namespace, :ssh_url_to_repo])
+        = submit_tag 'Add project to CI', class: 'btn btn-default btn-sm'
+    
\ No newline at end of file
diff --git a/app/views/ci/projects/_search.html.haml b/app/views/ci/projects/_search.html.haml
index 6d84b25a6af..4ab43a403f7 100644
--- a/app/views/ci/projects/_search.html.haml
+++ b/app/views/ci/projects/_search.html.haml
@@ -5,13 +5,7 @@
       .input-group-addon
         %i.fa.fa-search
 
-
 :coffeescript
   $('.ci-search-form').submit ->
-    NProgress.start()
-    query = $('.ci-search-form .search-input').val()
-    $.get '#{gitlab_ci_projects_path}', { search: query }, (data) ->
-      $(".projects").html data.html
-      NProgress.done()
-    CiPager.init "#{gitlab_ci_projects_path}" + "?search=" + query, #{Ci::ProjectsController::PROJECTS_BATCH}, false
+    CiPager.init "#{ci_projects_path}" + "?search=" + query, #{Ci::ProjectsController::PROJECTS_BATCH}, false
     false
diff --git a/app/views/ci/projects/gitlab.html.haml b/app/views/ci/projects/gitlab.html.haml
deleted file mode 100644
index 2101aa932a4..00000000000
--- a/app/views/ci/projects/gitlab.html.haml
+++ /dev/null
@@ -1,27 +0,0 @@
-- if @offset == 0
-  .gray-content-block.clearfix.light.second-block
-    .pull-left.fetch-status
-      - if params[:search].present?
-        by keyword: "#{params[:search]}",
-      #{@total_count} projects, #{@projects.size} of them added to CI
-
-  .wide-table-holder
-    %table.table.projects-table.content-list
-      %thead
-        %tr
-          %th Project Name
-          %th Last commit
-          %th Access
-          %th Commits
-
-      = render @projects
-
-      = render "gl_projects"
-
-  %p.text-center.hide.loading
-    %i.fa.fa-refresh.fa-spin
-
-- else
-  = render @projects
-
-  = render "gl_projects"
diff --git a/app/views/ci/projects/index.html.haml b/app/views/ci/projects/index.html.haml
index 60ab29a66cf..8de205d57aa 100644
--- a/app/views/ci/projects/index.html.haml
+++ b/app/views/ci/projects/index.html.haml
@@ -1,13 +1,30 @@
 - if current_user
-  .gray-content-block.top-block
-    = render "search"
-  .projects
-    %p.fetch-status.light
-      %i.fa.fa-refresh.fa-spin
+  - if @offset > 0
+    = render @projects
+  - else
+    .gray-content-block.top-block
+      = render "search"
+    .projects
+      .gray-content-block.clearfix.light.second-block
+        .pull-left.fetch-status
+          - if params[:search].present?
+            by keyword: "#{params[:search]}",
+          #{@total_count} projects
+
+        .wide-table-holder
+          %table.table.projects-table.content-list
+            %thead
+              %tr
+                %th Project Name
+                %th Last commit
+                %th Access
+                %th Commits
+
+            = render @projects
+  %p.text-center.hide.loading
+    %i.fa.fa-refresh.fa-spin
   :coffeescript
-    $.get '#{gitlab_ci_projects_path}', (data) ->
-      $(".projects").html data.html
-    CiPager.init "#{gitlab_ci_projects_path}", #{Ci::ProjectsController::PROJECTS_BATCH}, false
+    CiPager.init "#{ci_projects_path}", #{Ci::ProjectsController::PROJECTS_BATCH}, false
 
 - else
   = render 'public'
diff --git a/lib/ci/project_list_builder.rb b/lib/ci/project_list_builder.rb
new file mode 100644
index 00000000000..da26f9a9f47
--- /dev/null
+++ b/lib/ci/project_list_builder.rb
@@ -0,0 +1,21 @@
+module Ci
+  class ProjectListBuilder
+    def execute(current_user, search = nil)
+      projects = current_user.authorized_projects
+      projects = projects.search(search) if search
+      
+      projects.
+        joins("LEFT JOIN ci_projects ON projects.id = ci_projects.gitlab_id
+          LEFT JOIN #{last_commit_subquery} AS last_commit ON #{Ci::Project.table_name}.id = last_commit.project_id").
+        reorder("ci_projects.id is NULL ASC,
+          CASE WHEN last_commit.committed_at IS NULL THEN 1 ELSE 0 END,
+          last_commit.committed_at DESC")
+    end
+
+    private
+
+    def last_commit_subquery
+      "(SELECT project_id, MAX(committed_at) committed_at FROM #{Ci::Commit.table_name} GROUP BY project_id)"
+    end
+  end
+end
diff --git a/spec/controllers/ci/projects_controller_spec.rb b/spec/controllers/ci/projects_controller_spec.rb
index 015788a05e1..c7a3cd50c20 100644
--- a/spec/controllers/ci/projects_controller_spec.rb
+++ b/spec/controllers/ci/projects_controller_spec.rb
@@ -84,7 +84,7 @@ describe Ci::ProjectsController do
     end
 
     it "searches projects" do
-      xhr :get, :gitlab, { search: "str", format: "js" }.with_indifferent_access
+      xhr :get, :index, { search: "str", format: "json" }.with_indifferent_access
 
       expect(response).to be_success
       expect(response.code).to eq('200')