Merge branch 'adambuckland/gitlab-ce-19727-sort-submodules-with-files'

4 files changed, 39 insertions, 15 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 0a0e894ae64..cac61e9d1ff 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -86,6 +86,7 @@ v 8.11.0 (unreleased)
   - Update devise initializer to turn on changed password notification emails. !5648 (tombell)
   - Avoid to show the original password field when password is automatically set. !5712 (duduribeiro)
   - Fix importing GitLab projects with an invalid MR source project
+  - Sort folders with submodules in Files view !5521
 
 v 8.10.5 (unreleased)
 
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index dbedf417fa5..4a76c679bad 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -4,23 +4,11 @@ module TreeHelper
   #
   # contents - A Grit::Tree object for the current tree
   def render_tree(tree)
-    # Render Folders before Files/Submodules
+    # Sort submodules and folders together by name ahead of files
     folders, files, submodules = tree.trees, tree.blobs, tree.submodules
-
     tree = ""
-
-    # Render folders if we have any
-    tree << render(partial: 'projects/tree/tree_item', collection: folders,
-                   locals: { type: 'folder' }) if folders.present?
-
-    # Render files if we have any
-    tree << render(partial: 'projects/tree/blob_item', collection: files,
-                   locals: { type: 'file' }) if files.present?
-
-    # Render submodules if we have any
-    tree << render(partial: 'projects/tree/submodule_item',
-                   collection: submodules) if submodules.present?
-
+    items = (folders + submodules).sort_by(&:name) + files
+    tree << render(partial: "projects/tree/tree_row", collection: items) if items.present?
     tree.html_safe
   end
 
diff --git a/app/views/projects/tree/_tree_row.html.haml b/app/views/projects/tree/_tree_row.html.haml
new file mode 100644
index 00000000000..0a5c6f048f7
--- /dev/null
+++ b/app/views/projects/tree/_tree_row.html.haml
@@ -0,0 +1,6 @@
+- if tree_row.type == :tree
+  = render partial: 'projects/tree/tree_item', object: tree_row, as: 'tree_item', locals: { type: 'folder' }
+- elsif tree_row.type == :blob
+  = render partial: 'projects/tree/blob_item', object: tree_row, as: 'blob_item', locals: { type: 'file' }
+- elsif tree_row.type == :commit
+  = render partial: 'projects/tree/submodule_item', object: tree_row, as: 'submodule_item'
diff --git a/spec/features/projects/files/files_sort_submodules_with_folders_spec.rb b/spec/features/projects/files/files_sort_submodules_with_folders_spec.rb
new file mode 100644
index 00000000000..10b91d8990b
--- /dev/null
+++ b/spec/features/projects/files/files_sort_submodules_with_folders_spec.rb
@@ -0,0 +1,29 @@
+require 'spec_helper'
+
+feature 'User views files page', feature: true do
+  include WaitForAjax
+
+  let(:user) { create(:user) }
+  let(:project) { create(:forked_project_with_submodules) }
+
+  before do
+    project.team << [user, :master]
+    login_as user
+    visit namespace_project_tree_path(project.namespace, project, project.repository.root_ref)
+  end
+
+  scenario 'user sees folders and submodules sorted together, followed by files' do
+    rows = all('td.tree-item-file-name').map(&:text)
+    tree = project.repository.tree
+
+    folders = tree.trees.map(&:name)
+    files = tree.blobs.map(&:name)
+    submodules = tree.submodules.map do |submodule|
+      submodule.name + " @ " + submodule.id[0..7]
+    end
+
+    sorted_titles = (folders + submodules).sort + files
+
+    expect(rows).to eq(sorted_titles)
+  end
+end