diff options
author | Ezekiel Kigbo <ekigbo@gitlab.com> | 2019-04-19 10:36:16 +0200 |
---|---|---|
committer | Ezekiel Kigbo <ekigbo@gitlab.com> | 2019-05-06 16:42:44 +0100 |
commit | 6accad69e214f44ac84f3ceac4159c712aa9af2e (patch) | |
tree | 5b07c67d5be2667a1187c13efb37424a6356af41 | |
parent | b736a9f000d1e1f6c2c94ea74a700abf5a44139e (diff) | |
download | gitlab-ce-6accad69e214f44ac84f3ceac4159c712aa9af2e.tar.gz |
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
-rw-r--r-- | app/helpers/projects_helper.rb | 4 | ||||
-rw-r--r-- | app/helpers/search_helper.rb | 2 | ||||
-rw-r--r-- | app/helpers/sorting_helper.rb | 27 | ||||
-rw-r--r-- | app/models/project.rb | 4 | ||||
-rw-r--r-- | app/views/dashboard/_projects_head.html.haml | 1 | ||||
-rw-r--r-- | app/views/dashboard/projects/_nav.html.haml | 16 | ||||
-rw-r--r-- | app/views/dashboard/projects/index.html.haml | 3 | ||||
-rw-r--r-- | app/views/explore/projects/_filter.html.haml | 3 | ||||
-rw-r--r-- | app/views/explore/projects/_nav.html.haml | 1 | ||||
-rw-r--r-- | app/views/shared/projects/_search_bar.html.haml | 1 | ||||
-rw-r--r-- | app/views/shared/projects/_sort_dropdown.html.haml | 5 | ||||
-rw-r--r-- | spec/features/dashboard/projects_spec.rb | 1 | ||||
-rw-r--r-- | spec/features/dashboard/user_filters_projects_spec.rb | 93 | ||||
-rw-r--r-- | spec/helpers/projects_helper_spec.rb | 4 |
14 files changed, 86 insertions, 79 deletions
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 8977ccaa9d8..2c43b1a2067 100644 --- a/app/helpers/projects_helper.rb +++ b/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) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index a62c00df60b..4594f5a31b9 100644 --- a/app/helpers/search_helper.rb +++ b/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, diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index df2f4468199..f2d814e6930 100644 --- a/app/helpers/sorting_helper.rb +++ b/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 diff --git a/app/models/project.rb b/app/models/project.rb index da5f2c2e96e..da72186c8a0 100644 --- a/app/models/project.rb +++ b/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 diff --git a/app/views/dashboard/_projects_head.html.haml b/app/views/dashboard/_projects_head.html.haml index 128a766374e..1d349572091 100644 --- a/app/views/dashboard/_projects_head.html.haml +++ b/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' diff --git a/app/views/dashboard/projects/_nav.html.haml b/app/views/dashboard/projects/_nav.html.haml index 2da24e37c86..fdab2bde095 100644 --- a/app/views/dashboard/projects/_nav.html.haml +++ b/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) diff --git a/app/views/dashboard/projects/index.html.haml b/app/views/dashboard/projects/index.html.haml index 9d0a4219446..0298f539b4b 100644 --- a/app/views/dashboard/projects/index.html.haml +++ b/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" diff --git a/app/views/explore/projects/_filter.html.haml b/app/views/explore/projects/_filter.html.haml index 4606f3ec674..ebc3f6b85ce 100644 --- a/app/views/explore/projects/_filter.html.haml +++ b/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? diff --git a/app/views/explore/projects/_nav.html.haml b/app/views/explore/projects/_nav.html.haml index 9ec2122d5a3..bf65c19b720 100644 --- a/app/views/explore/projects/_nav.html.haml +++ b/app/views/explore/projects/_nav.html.haml @@ -15,4 +15,3 @@ = render 'shared/projects/search_form' = render 'shared/projects/dropdown' = render 'filter' - diff --git a/app/views/shared/projects/_search_bar.html.haml b/app/views/shared/projects/_search_bar.html.haml index e44d8692eea..0620cd86db7 100644 --- a/app/views/shared/projects/_search_bar.html.haml +++ b/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 diff --git a/app/views/shared/projects/_sort_dropdown.html.haml b/app/views/shared/projects/_sort_dropdown.html.haml index 65f175efcf7..0bb2eb371dc 100644 --- a/app/views/shared/projects/_sort_dropdown.html.haml +++ b/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 diff --git a/spec/features/dashboard/projects_spec.rb b/spec/features/dashboard/projects_spec.rb index 8b5f645b2b4..d1ed64cce7f 100644 --- a/spec/features/dashboard/projects_spec.rb +++ b/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 diff --git a/spec/features/dashboard/user_filters_projects_spec.rb b/spec/features/dashboard/user_filters_projects_spec.rb index 9945bf85997..e0553086fd7 100644 --- a/spec/features/dashboard/user_filters_projects_spec.rb +++ b/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,86 +191,70 @@ 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) - expect_to_see_projects(asc) - end - it 'will update the url query' do - select_dropdown_option '#filtered-search-sorting-dropdown', 'Name' + click_sort_direction - [/sort=name_desc/, /sort=name_asc/].each do |query_param| - click_sort_direction - expect(current_url).to match(query_param) - end + expect_to_see_projects(asc) 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) - expect_to_see_projects(asc) - end - it 'will update the url query' do - select_dropdown_option '#filtered-search-sorting-dropdown', 'Last updated' + click_sort_direction - [/sort=latest_activity_asc/, /sort=latest_activity_desc/].each do |query_param| - click_sort_direction - expect(current_url).to match(query_param) - end + expect_to_see_projects(asc) 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) - expect_to_see_projects(asc) - end - it 'will update the url query' do - select_dropdown_option '#filtered-search-sorting-dropdown', 'Created date' + click_sort_direction - [/sort=created_asc/, /sort=created_desc/].each do |query_param| - click_sort_direction - expect(current_url).to match(query_param) - end + expect_to_see_projects(asc) 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) - expect_to_see_projects(asc) - end - it 'will update the url query' do - select_dropdown_option '#filtered-search-sorting-dropdown', 'Stars' + click_sort_direction - [/sort=stars_asc/, /sort=stars_desc/].each do |query_param| - click_sort_direction - expect(current_url).to match(query_param) - end + expect_to_see_projects(asc) end end end diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 37c63807c82..554cb861563 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/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 |