Added blank lines to meet style guide

Un-nest title variable output

Update spec test names

Rename sort_value_most_stars -> sort_value_stars_desc

Rename sorted_by_stars -> sorted_by_stars_desc

Renname sort_value_most_stars_asc -> sort_value_stars_asc

Invert feature check, assign feature condition to a variable

Inline conditional nav bar rendering

Invert conditional label

Added follow up task

Fix filters returning 0 projects show the wrong view

Move click action out of test expectation

Use proper variable name for project in before block

Rename projects_sort_admin_options_hash

Renamed projects_sort_admin_options_has to
old_projects_sort_options_hash as its not only used
on the admin screen

Fix extra whitespace errors

Stub project_list_filter_bar in the projects_helper specs

Added follow up task for `show_projects?`

Removed url test expectations

app/helpers/projects_helper.rb

@@ -239,8 +239,10 @@ module ProjectsHelper
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
+  # TODO: Remove this method when removing the feature flag
+  # https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/11209#note_162234863
   def show_projects?(projects, params)
-    !!(params[:personal] || params[:name] || any_projects?(projects))
+    Feature.enabled?(:project_list_filter_bar) || !!(params[:personal] || params[:name] || any_projects?(projects))
   end
 
   def push_to_create_project_command(user = current_user)

app/helpers/search_helper.rb

@@ -128,7 +128,7 @@ module SearchHelper
   # rubocop: disable CodeReuse/ActiveRecord
   def projects_autocomplete(term, limit = 5)
     current_user.authorized_projects.order_id_desc.search_by_title(term)
-      .sorted_by_stars.non_archived.limit(limit).map do |p|
+      .sorted_by_stars_desc.non_archived.limit(limit).map do |p|
       {
         category: "Projects",
         id: p.id,

app/helpers/sorting_helper.rb

@@ -30,17 +30,20 @@ module SortingHelper
   end
 
   def projects_sort_options_hash
-    Feature.enabled?(:project_list_filter_bar) && !current_controller?('admin/projects') ? projects_sort_common_options_hash : projects_sort_admin_options_hash
+    Feature.enabled?(:project_list_filter_bar) && !current_controller?('admin/projects') ? projects_sort_common_options_hash : old_projects_sort_options_hash
   end
 
-  def projects_sort_admin_options_hash
+  # TODO: Simplify these sorting options
+  # https://gitlab.com/gitlab-org/gitlab-ce/issues/60798
+  # https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/11209#note_162234858
+  def old_projects_sort_options_hash
     options = {
       sort_value_latest_activity  => sort_title_latest_activity,
       sort_value_name             => sort_title_name,
       sort_value_oldest_activity  => sort_title_oldest_activity,
       sort_value_oldest_created   => sort_title_oldest_created,
       sort_value_recently_created => sort_title_recently_created,
-      sort_value_most_stars       => sort_title_most_stars
+      sort_value_stars_desc       => sort_title_most_stars
     }
 
     if current_controller?('admin/projects')
@@ -55,7 +58,7 @@ module SortingHelper
       sort_value_latest_activity  => sort_title_latest_activity,
       sort_value_recently_created => sort_title_created_date,
       sort_value_name             => sort_title_name,
-      sort_value_most_stars       => sort_title_stars
+      sort_value_stars_desc       => sort_title_stars
     }
   end
 
@@ -64,11 +67,11 @@ module SortingHelper
       sort_value_latest_activity  => sort_title_latest_activity,
       sort_value_recently_created => sort_title_created_date,
       sort_value_name             => sort_title_name,
-      sort_value_most_stars       => sort_title_stars,
+      sort_value_stars_desc       => sort_title_stars,
       sort_value_oldest_activity  => sort_title_latest_activity,
       sort_value_oldest_created   => sort_title_created_date,
       sort_value_name_desc        => sort_title_name,
-      sort_value_most_stars_asc   => sort_title_stars
+      sort_value_stars_asc        => sort_title_stars
     }
   end
 
@@ -77,11 +80,11 @@ module SortingHelper
       sort_value_latest_activity  => sort_value_oldest_activity,
       sort_value_recently_created => sort_value_oldest_created,
       sort_value_name             => sort_value_name_desc,
-      sort_value_most_stars       => sort_value_most_stars_asc,
+      sort_value_stars_desc       => sort_value_stars_asc,
       sort_value_oldest_activity  => sort_value_latest_activity,
       sort_value_oldest_created   => sort_value_recently_created,
       sort_value_name_desc        => sort_value_name,
-      sort_value_most_stars_asc   => sort_value_most_stars
+      sort_value_stars_asc        => sort_value_stars_desc
     }
   end
 
@@ -98,7 +101,7 @@ module SortingHelper
 
   def subgroups_sort_options_hash
     groups_sort_options_hash.merge(
-      sort_value_most_stars => sort_title_most_stars
+      sort_value_stars_desc => sort_title_most_stars
     )
   end
 
@@ -215,6 +218,8 @@ module SortingHelper
     end
   end
 
+  # TODO: dedupicate issuable and project sort direction
+  # https://gitlab.com/gitlab-org/gitlab-ce/issues/60798
   def issuable_sort_direction_button(sort_value)
     link_class = 'btn btn-default has-tooltip reverse-sort-btn qa-reverse-sort'
     reverse_sort = issuable_reverse_sort_order_hash[sort_value]
@@ -525,11 +530,11 @@ module SortingHelper
     'contacted_asc'
   end
 
-  def sort_value_most_stars
+  def sort_value_stars_desc
     'stars_desc'
   end
 
-  def sort_value_most_stars_asc
+  def sort_value_stars_asc
     'stars_asc'
   end
 

app/models/project.rb

@@ -357,7 +357,7 @@ class Project < ApplicationRecord
 
   # last_activity_at is throttled every minute, but last_repository_updated_at is updated with every push
   scope :sorted_by_activity, -> { reorder("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC") }
-  scope :sorted_by_stars, -> { reorder(star_count: :desc) }
+  scope :sorted_by_stars_desc, -> { reorder(star_count: :desc) }
   scope :sorted_by_stars_asc, -> { reorder(star_count: :asc) }
 
   scope :in_namespace, ->(namespace_ids) { where(namespace_id: namespace_ids) }
@@ -545,7 +545,7 @@ class Project < ApplicationRecord
       when 'latest_activity_asc'
         reorder(last_activity_at: :asc)
       when 'stars_desc'
-        sorted_by_stars
+        sorted_by_stars_desc
       when 'stars_asc'
         sorted_by_stars_asc
       else

app/views/dashboard/_projects_head.html.haml

@@ -1,5 +1,6 @@
 - project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
 - feature_project_list_filter_bar = Feature.enabled?(:project_list_filter_bar)
+
 = content_for :flash_message do
   = render 'shared/project_limit'
 

app/views/dashboard/projects/_nav.html.haml

@@ -2,14 +2,10 @@
 - active_class = 'btn p-2 active'
 - project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
 - is_explore_trending = project_tab_filter == :explore_trending
+- feature_project_list_filter_bar = Feature.enabled?(:project_list_filter_bar)
+
 .nav-block{ class: Feature.enabled?(:project_list_filter_bar) ? "w-100" : "" }
-  - if !Feature.enabled?(:project_list_filter_bar)
-    %ul.nav-links.mobile-separator.nav.nav-tabs
-      = nav_link(html_options: { class: ("active" unless params[:personal].present?) }) do
-        = link_to s_('DashboardProjects|All'), dashboard_projects_path
-      = nav_link(html_options: { class: ("active" if params[:personal].present?) }) do
-        = link_to s_('DashboardProjects|Personal'), filter_projects_path(personal: true)
-  - else
+  - if feature_project_list_filter_bar
     .btn-group.button-filter-group.d-flex.m-0.p-0
       - if project_tab_filter == :explore || is_explore_trending
         = link_to s_('DashboardProjects|Trending'), trending_explore_projects_path, class: is_explore_trending ? active_class : inactive_class
@@ -17,3 +13,9 @@
       - else
         = link_to s_('DashboardProjects|All'), dashboard_projects_path, class: params[:personal].present? ? inactive_class : active_class
         = link_to s_('DashboardProjects|Personal'), filter_projects_path(personal: true), class: params[:personal].present? ? active_class : inactive_class
+  - else
+    %ul.nav-links.mobile-separator.nav.nav-tabs
+      = nav_link(html_options: { class: ("active" unless params[:personal].present?) }) do
+        = link_to s_('DashboardProjects|All'), dashboard_projects_path
+      = nav_link(html_options: { class: ("active" if params[:personal].present?) }) do
+        = link_to s_('DashboardProjects|Personal'), filter_projects_path(personal: true)

app/views/dashboard/projects/index.html.haml

@@ -13,8 +13,7 @@
   = render "projects/last_push"
   - if show_projects?(@projects, params)
     = render 'dashboard/projects_head'
-    - unless Feature.enabled?(:project_list_filter_bar)
-      = render 'nav'
+    = render 'nav' unless Feature.enabled?(:project_list_filter_bar)
     = render 'projects'
   - else
     = render "zero_authorized_projects"

app/views/explore/projects/_filter.html.haml

@@ -1,8 +1,9 @@
 - has_label = local_assigns.fetch(:has_label, false)
+
 - if current_user
   .dropdown.js-project-filter-dropdown-wrap
     %button.dropdown-menu-toggle{ href: '#', "data-toggle" => "dropdown", 'data-display' => 'static' }
-      - if !has_label
+      - unless has_label
         = icon('globe', class: 'mt-1')
         %span.light.ml-3= _("Visibility:")
       - if params[:visibility_level].present?

app/views/explore/projects/_nav.html.haml

@@ -15,4 +15,3 @@
       = render 'shared/projects/search_form'
       = render 'shared/projects/dropdown'
     = render 'filter'
-

app/views/shared/projects/_search_bar.html.haml

@@ -1,5 +1,6 @@
 - @sort ||= sort_value_latest_activity
 - project_tab_filter = local_assigns.fetch(:project_tab_filter, "")
+
 .filtered-search-block.row-content-block.bt-0
   .filtered-search-wrapper.d-flex.flex-nowrap.flex-column.flex-sm-wrap.flex-sm-row.flex-xl-nowrap
     - unless project_tab_filter == :starred

app/views/shared/projects/_sort_dropdown.html.haml

@@ -1,5 +1,6 @@
 - @sort ||= sort_value_latest_activity
 - toggle_text = projects_sort_option_titles[@sort]
+
 .btn-group.w-100{ role: "group" }
   .btn-group.w-100.dropdown.js-project-filter-dropdown-wrap{ role: "group" }
     %button.dropdown-menu-toggle{ id: 'sort-projects-dropdown', type: 'button', data: { toggle: 'dropdown', display: 'static' }, class: 'btn btn-default' }
@@ -10,8 +11,7 @@
         = _("Sort by")
       - projects_sort_options_hash.each do |value, title|
         %li
-          = link_to filter_projects_path(sort: value), class: ("is-active" if toggle_text == title) do
-            = title
+          = link_to title, filter_projects_path(sort: value), class: ("is-active" if toggle_text == title)
 
       %li.divider
       %li
@@ -23,6 +23,7 @@
       %li
         = link_to filter_projects_path(archived: 'only'), class: ("is-active" if params[:archived] == 'only') do
           = _("Show archived projects only")
+
       - if current_user && @group && @group.shared_projects.present?
         %li.divider
         %li

spec/features/dashboard/projects_spec.rb

@@ -145,6 +145,7 @@ describe 'Dashboard Projects' do
 
     it 'does not show tabs to filter by all projects or personal' do
       visit(starred_dashboard_projects_path)
+
       expect(page).not_to have_content '.filtered-search-nav'
     end
   end

spec/features/dashboard/user_filters_projects_spec.rb

@@ -53,7 +53,7 @@ describe 'Dashboard > User filters projects' do
       visit dashboard_projects_path
     end
 
-    it 'will autocomplete searches', :js do
+    it 'autocompletes searches upon typing', :js do
       expect(page).to have_content 'Victorialand'
       expect(page).to have_content 'Treasure'
 
@@ -85,13 +85,12 @@ describe 'Dashboard > User filters projects' do
     end
 
     def expect_to_see_projects(sorted_projects)
-      click_sort_direction
       list = page.all('.projects-list .project-name').map(&:text)
       expect(list).to match(sorted_projects)
     end
 
     describe 'Search' do
-      it 'will execute when i click the search button' do
+      it 'executes when the search button is clicked' do
         expect(page).to have_content 'Victorialand'
         expect(page).to have_content 'Treasure'
 
@@ -116,30 +115,37 @@ describe 'Dashboard > User filters projects' do
 
     describe 'Filter' do
       before do
-        priv = create(:project, :private, name: 'Private project', namespace: user.namespace)
-        int = create(:project, :internal, name: 'Internal project', namespace: user.namespace)
+        private_project = create(:project, :private, name: 'Private project', namespace: user.namespace)
+        internal_project = create(:project, :internal, name: 'Internal project', namespace: user.namespace)
 
-        priv.add_maintainer(user)
-        int.add_maintainer(user)
+        private_project.add_maintainer(user)
+        internal_project.add_maintainer(user)
       end
 
-      it 'can filter for only private projects' do
+      it 'filters private projects only' do
         select_dropdown_option '#filtered-search-visibility-dropdown', 'Private'
+
         expect(current_url).to match(/visibility_level=0/)
+
         list = page.all('.projects-list .project-name').map(&:text)
+
         expect(list).to match(["Private project", "Treasure", "Victorialand"])
       end
 
-      it 'can filter for only internal projects' do
+      it 'filters internal projects only' do
         select_dropdown_option '#filtered-search-visibility-dropdown', 'Internal'
+
         expect(current_url).to match(/visibility_level=10/)
+
         list = page.all('.projects-list .project-name').map(&:text)
+
         expect(list).to match(['Internal project'])
       end
 
-      it 'can filter for any project' do
+      it 'filters any project' do
         select_dropdown_option '#filtered-search-visibility-dropdown', 'Any'
         list = page.all('.projects-list .project-name').map(&:text)
+
         expect(list).to match(["Internal project", "Private project", "Treasure", "Victorialand"])
       end
     end
@@ -151,8 +157,8 @@ describe 'Dashboard > User filters projects' do
           { name: 'Cell saga', created_at: Time.now },
           { name: 'Frieza saga', created_at: 10.days.ago }
         ].each do |item|
-          proj = create(:project, name: item[:name], namespace: user.namespace, created_at: item[:created_at])
-          proj.add_developer(user)
+          project = create(:project, name: item[:name], namespace: user.namespace, created_at: item[:created_at])
+          project.add_developer(user)
         end
 
         user.toggle_star(project)
@@ -160,12 +166,13 @@ describe 'Dashboard > User filters projects' do
         user2.toggle_star(project2)
       end
 
-      it 'will include sorting direction' do
+      it 'includes sorting direction' do
         sorting_dropdown = page.find('.filtered-search-block #filtered-search-sorting-dropdown')
+
         expect(sorting_dropdown).to have_css '.reverse-sort-btn'
       end
 
-      it 'will have all sorting options', :js do
+      it 'has all sorting options', :js do
         sorting_dropdown = page.find('.filtered-search-block #filtered-search-sorting-dropdown')
         sorting_option_labels = ['Last updated', 'Created date', 'Name', 'Stars']
 
@@ -176,7 +183,7 @@ describe 'Dashboard > User filters projects' do
         end
       end
 
-      it 'will default to Last updated', :js do
+      it 'defaults to "Last updated"', :js do
         page.find('.filtered-search-block #filtered-search-sorting-dropdown').click
         active_sorting_option = page.first('.filtered-search-block #filtered-search-sorting-dropdown .is-active')
 
@@ -184,87 +191,71 @@ describe 'Dashboard > User filters projects' do
       end
 
       context 'Sorting by name' do
-        it 'will sort the project list' do
+        it 'sorts the project list' do
           select_dropdown_option '#filtered-search-sorting-dropdown', 'Name'
 
           desc = ['Victorialand', 'Treasure', 'Red ribbon army', 'Frieza saga', 'Cell saga']
           asc = ['Cell saga', 'Frieza saga', 'Red ribbon army', 'Treasure', 'Victorialand']
 
+          click_sort_direction
+
           expect_to_see_projects(desc)
+
+          click_sort_direction
+
           expect_to_see_projects(asc)
         end
-
-        it 'will update the url query' do
-          select_dropdown_option '#filtered-search-sorting-dropdown', 'Name'
-
-          [/sort=name_desc/, /sort=name_asc/].each do |query_param|
-            click_sort_direction
-            expect(current_url).to match(query_param)
-          end
-        end
       end
 
       context 'Sorting by Last updated' do
-        it 'will sort the project list' do
+        it 'sorts the project list' do
           select_dropdown_option '#filtered-search-sorting-dropdown', 'Last updated'
 
           desc = ["Frieza saga", "Red ribbon army", "Victorialand", "Treasure", "Cell saga"]
           asc = ["Cell saga", "Treasure", "Victorialand", "Red ribbon army", "Frieza saga"]
 
+          click_sort_direction
+
           expect_to_see_projects(desc)
+
+          click_sort_direction
+
           expect_to_see_projects(asc)
         end
-
-        it 'will update the url query' do
-          select_dropdown_option '#filtered-search-sorting-dropdown', 'Last updated'
-
-          [/sort=latest_activity_asc/, /sort=latest_activity_desc/].each do |query_param|
-            click_sort_direction
-            expect(current_url).to match(query_param)
-          end
-        end
       end
 
       context 'Sorting by Created date' do
-        it 'will sort the project list' do
+        it 'sorts the project list' do
           select_dropdown_option '#filtered-search-sorting-dropdown', 'Created date'
 
           desc = ["Frieza saga", "Red ribbon army", "Victorialand", "Treasure", "Cell saga"]
           asc = ["Cell saga", "Treasure", "Victorialand", "Red ribbon army", "Frieza saga"]
 
+          click_sort_direction
+
           expect_to_see_projects(desc)
+
+          click_sort_direction
+
           expect_to_see_projects(asc)
         end
-
-        it 'will update the url query' do
-          select_dropdown_option '#filtered-search-sorting-dropdown', 'Created date'
-
-          [/sort=created_asc/, /sort=created_desc/].each do |query_param|
-            click_sort_direction
-            expect(current_url).to match(query_param)
-          end
-        end
       end
 
       context 'Sorting by Stars' do
-        it 'will sort the project list' do
+        it 'sorts the project list' do
           select_dropdown_option '#filtered-search-sorting-dropdown', 'Stars'
 
           desc = ["Red ribbon army", "Cell saga", "Frieza saga", "Victorialand", "Treasure"]
           asc = ["Treasure", "Victorialand", "Red ribbon army", "Cell saga", "Frieza saga"]
 
+          click_sort_direction
+
           expect_to_see_projects(desc)
+
+          click_sort_direction
+
           expect_to_see_projects(asc)
         end
-
-        it 'will update the url query' do
-          select_dropdown_option '#filtered-search-sorting-dropdown', 'Stars'
-
-          [/sort=stars_asc/, /sort=stars_desc/].each do |query_param|
-            click_sort_direction
-            expect(current_url).to match(query_param)
-          end
-        end
       end
     end
   end

spec/helpers/projects_helper_spec.rb

@@ -445,6 +445,10 @@ describe ProjectsHelper do
       Project.all
     end
 
+    before do
+      stub_feature_flags(project_list_filter_bar: false)
+    end
+
     it 'returns true when there are projects' do
       expect(helper.show_projects?(projects, {})).to eq(true)
     end