From a7349560b2d13ea7894a462738b3a7687fdbae72 Mon Sep 17 00:00:00 2001 From: blackst0ne Date: Tue, 30 May 2017 15:35:49 +1100 Subject: [PATCH] 'New issue'/'New merge request' dropdowns should show only projects with issues/merge requests feature enabled --- app/assets/javascripts/project_select.js | 9 +- app/helpers/selects_helper.rb | 8 ++ app/models/project.rb | 1 + app/views/dashboard/issues.html.haml | 2 +- app/views/dashboard/merge_requests.html.haml | 2 +- .../shared/_new_project_item_select.html.haml | 2 +- ...e-for-project-that-has-issues-disabled.yml | 4 + doc/api/projects.md | 2 + lib/api/projects.rb | 8 +- spec/features/dashboard/issues_spec.rb | 101 ++++++++++-------- .../features/dashboard/merge_requests_spec.rb | 22 +++- 11 files changed, 105 insertions(+), 56 deletions(-) create mode 100644 changelogs/unreleased/19107-404-when-creating-new-milestone-or-issue-for-project-that-has-issues-disabled.yml diff --git a/app/assets/javascripts/project_select.js b/app/assets/javascripts/project_select.js index 0ff0a3b6cc4..9896b88d487 100644 --- a/app/assets/javascripts/project_select.js +++ b/app/assets/javascripts/project_select.js @@ -51,6 +51,9 @@ import Api from './api'; this.groupId = $(select).data('group-id'); this.includeGroups = $(select).data('include-groups'); this.orderBy = $(select).data('order-by') || 'id'; + this.withIssuesEnabled = $(select).data('with-issues-enabled'); + this.withMergeRequestsEnabled = $(select).data('with-merge-requests-enabled'); + placeholder = "Search for project"; if (this.includeGroups) { placeholder += " or group"; @@ -84,7 +87,11 @@ import Api from './api'; if (_this.groupId) { return Api.groupProjects(_this.groupId, query.term, projectsCallback); } else { - return Api.projects(query.term, { order_by: _this.orderBy }, projectsCallback); + return Api.projects(query.term, { + order_by: _this.orderBy, + with_issues_enabled: _this.withIssuesEnabled, + with_merge_requests_enabled: _this.withMergeRequestsEnabled + }, projectsCallback); } }; })(this), diff --git a/app/helpers/selects_helper.rb b/app/helpers/selects_helper.rb index a7d1fe4aa47..1a4f1431bdc 100644 --- a/app/helpers/selects_helper.rb +++ b/app/helpers/selects_helper.rb @@ -45,6 +45,14 @@ module SelectsHelper end end + with_feature_enabled_data_attribute = + case opts.delete(:with_feature_enabled) + when 'issues' then 'data-with-issues-enabled' + when 'merge_requests' then 'data-with-merge-requests-enabled' + end + + opts[with_feature_enabled_data_attribute] = true + hidden_field_tag(id, opts[:selected], opts) end diff --git a/app/models/project.rb b/app/models/project.rb index 29af57d7664..84070290743 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -271,6 +271,7 @@ class Project < ActiveRecord::Base scope :with_builds_enabled, -> { with_feature_enabled(:builds) } scope :with_issues_enabled, -> { with_feature_enabled(:issues) } + scope :with_merge_requests_enabled, -> { with_feature_enabled(:merge_requests) } enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 } diff --git a/app/views/dashboard/issues.html.haml b/app/views/dashboard/issues.html.haml index faa68468043..d6b46dee0e4 100644 --- a/app/views/dashboard/issues.html.haml +++ b/app/views/dashboard/issues.html.haml @@ -8,7 +8,7 @@ .nav-controls = link_to params.merge(rss_url_options), class: 'btn has-tooltip', title: 'Subscribe' do = icon('rss') - = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue" + = render 'shared/new_project_item_select', path: 'issues/new', label: "New issue", with_feature_enabled: 'issues' = render 'shared/issuable/filter', type: :issues = render 'shared/issues' diff --git a/app/views/dashboard/merge_requests.html.haml b/app/views/dashboard/merge_requests.html.haml index 12966c01950..6f6afe161d1 100644 --- a/app/views/dashboard/merge_requests.html.haml +++ b/app/views/dashboard/merge_requests.html.haml @@ -4,7 +4,7 @@ .top-area = render 'shared/issuable/nav', type: :merge_requests .nav-controls - = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request" + = render 'shared/new_project_item_select', path: 'merge_requests/new', label: "New merge request", with_feature_enabled: 'merge_requests' = render 'shared/issuable/filter', type: :merge_requests = render 'shared/merge_requests' diff --git a/app/views/shared/_new_project_item_select.html.haml b/app/views/shared/_new_project_item_select.html.haml index fbbf6f358c5..9ed844cf5e7 100644 --- a/app/views/shared/_new_project_item_select.html.haml +++ b/app/views/shared/_new_project_item_select.html.haml @@ -1,6 +1,6 @@ - if @projects.any? .project-item-select-holder - = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at' } + = project_select_tag :project_path, class: "project-item-select", data: { include_groups: local_assigns[:include_groups], order_by: 'last_activity_at' }, with_feature_enabled: local_assigns[:with_feature_enabled] %a.btn.btn-new.new-project-item-select-button = local_assigns[:label] = icon('caret-down') diff --git a/changelogs/unreleased/19107-404-when-creating-new-milestone-or-issue-for-project-that-has-issues-disabled.yml b/changelogs/unreleased/19107-404-when-creating-new-milestone-or-issue-for-project-that-has-issues-disabled.yml new file mode 100644 index 00000000000..bec9aa34761 --- /dev/null +++ b/changelogs/unreleased/19107-404-when-creating-new-milestone-or-issue-for-project-that-has-issues-disabled.yml @@ -0,0 +1,4 @@ +--- +title: 'New issue'/'New merge request' dropdowns should show only projects with issues/merge requests feature enabled +merge_request: 19107 +author: blackst0ne diff --git a/doc/api/projects.md b/doc/api/projects.md index 6b919f71792..345f93a6017 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -38,6 +38,8 @@ Parameters: | `membership` | boolean | no | Limit by projects that the current user is a member of | | `starred` | boolean | no | Limit by projects starred by the current user | | `statistics` | boolean | no | Include project statistics | +| `with_issues_enabled` | boolean | no | Limit by enabled issues feature | +| `with_merge_requests_enabled` | boolean | no | Limit by enabled merge requests feature | ```json [ diff --git a/lib/api/projects.rb b/lib/api/projects.rb index ed5004e8d1a..d4fe5c023bf 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -58,6 +58,8 @@ module API optional :owned, type: Boolean, default: false, desc: 'Limit by owned by authenticated user' optional :starred, type: Boolean, default: false, desc: 'Limit by starred status' optional :membership, type: Boolean, default: false, desc: 'Limit by projects that the current user is a member of' + optional :with_issues_enabled, type: Boolean, default: false, desc: 'Limit by enabled issues feature' + optional :with_merge_requests_enabled, type: Boolean, default: false, desc: 'Limit by enabled merge requests feature' end params :create_params do @@ -69,11 +71,15 @@ module API options = options.reverse_merge( with: Entities::Project, current_user: current_user, - simple: params[:simple] + simple: params[:simple], + with_issues_enabled: params[:with_issues_enabled], + with_merge_requests_enabled: params[:with_merge_requests_enabled] ) projects = filter_projects(projects) projects = projects.with_statistics if options[:statistics] + projects = projects.with_issues_enabled if options[:with_issues_enabled] + projects = projects.with_merge_requests_enabled if options[:with_merge_requests_enabled] options[:with] = Entities::BasicProjectDetails if options[:simple] present paginate(projects), options diff --git a/spec/features/dashboard/issues_spec.rb b/spec/features/dashboard/issues_spec.rb index 2346a9ec2ed..2cea6b1563e 100644 --- a/spec/features/dashboard/issues_spec.rb +++ b/spec/features/dashboard/issues_spec.rb @@ -2,66 +2,75 @@ require 'spec_helper' RSpec.describe 'Dashboard Issues', feature: true do let(:current_user) { create :user } - let(:public_project) { create(:empty_project, :public) } - let(:project) do - create(:empty_project) do |project| - project.team << [current_user, :master] - end - end - + let!(:public_project) { create(:empty_project, :public) } + let(:project) { create(:empty_project) } + let(:project_with_issues_disabled) { create(:empty_project, :issues_disabled) } let!(:authored_issue) { create :issue, author: current_user, project: project } let!(:authored_issue_on_public_project) { create :issue, author: current_user, project: public_project } let!(:assigned_issue) { create :issue, assignees: [current_user], project: project } let!(:other_issue) { create :issue, project: project } before do + [project, project_with_issues_disabled].each { |project| project.team << [current_user, :master] } login_as(current_user) - visit issues_dashboard_path(assignee_id: current_user.id) end - it 'shows issues assigned to current user' do - expect(page).to have_content(assigned_issue.title) - expect(page).not_to have_content(authored_issue.title) - expect(page).not_to have_content(other_issue.title) - end - - it 'shows checkmark when unassigned is selected for assignee', js: true do - find('.js-assignee-search').click - find('li', text: 'Unassigned').click - find('.js-assignee-search').click - - expect(find('li[data-user-id="0"] a.is-active')).to be_visible - end - - it 'shows issues when current user is author', js: true do - find('#assignee_id', visible: false).set('') - find('.js-author-search', match: :first).click - - expect(find('li[data-user-id="null"] a.is-active')).to be_visible - - find('.dropdown-menu-author li a', match: :first, text: current_user.to_reference).click - find('.js-author-search', match: :first).click - - page.within '.dropdown-menu-user' do - expect(find('.dropdown-menu-author li a.is-active', match: :first, text: current_user.to_reference)).to be_visible + describe 'issues' do + it 'shows issues assigned to current user' do + expect(page).to have_content(assigned_issue.title) + expect(page).not_to have_content(authored_issue.title) + expect(page).not_to have_content(other_issue.title) end - expect(page).to have_content(authored_issue.title) - expect(page).to have_content(authored_issue_on_public_project.title) - expect(page).not_to have_content(assigned_issue.title) - expect(page).not_to have_content(other_issue.title) + it 'shows checkmark when unassigned is selected for assignee', js: true do + find('.js-assignee-search').click + find('li', text: 'Unassigned').click + find('.js-assignee-search').click + + expect(find('li[data-user-id="0"] a.is-active')).to be_visible + end + + it 'shows issues when current user is author', js: true do + find('#assignee_id', visible: false).set('') + find('.js-author-search', match: :first).click + + expect(find('li[data-user-id="null"] a.is-active')).to be_visible + + find('.dropdown-menu-author li a', match: :first, text: current_user.to_reference).click + find('.js-author-search', match: :first).click + + page.within '.dropdown-menu-user' do + expect(find('.dropdown-menu-author li a.is-active', match: :first, text: current_user.to_reference)).to be_visible + end + + expect(page).to have_content(authored_issue.title) + expect(page).to have_content(authored_issue_on_public_project.title) + expect(page).not_to have_content(assigned_issue.title) + expect(page).not_to have_content(other_issue.title) + end + + it 'shows all issues' do + click_link('Reset filters') + + expect(page).to have_content(authored_issue.title) + expect(page).to have_content(authored_issue_on_public_project.title) + expect(page).to have_content(assigned_issue.title) + expect(page).to have_content(other_issue.title) + end + + it_behaves_like "it has an RSS button with current_user's RSS token" + it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end - it 'shows all issues' do - click_link('Reset filters') + describe 'new issue dropdown' do + it 'shows projects only with issues feature enabled', js: true do + find('.new-project-item-select-button').trigger('click') - expect(page).to have_content(authored_issue.title) - expect(page).to have_content(authored_issue_on_public_project.title) - expect(page).to have_content(assigned_issue.title) - expect(page).to have_content(other_issue.title) + page.within('.select2-results') do + expect(page).to have_content(project.name_with_namespace) + expect(page).not_to have_content(project_with_issues_disabled.name_with_namespace) + end + end end - - it_behaves_like "it has an RSS button with current_user's RSS token" - it_behaves_like "an autodiscoverable RSS feed with current_user's RSS token" end diff --git a/spec/features/dashboard/merge_requests_spec.rb b/spec/features/dashboard/merge_requests_spec.rb index 508ca38d7e5..9cebe52c444 100644 --- a/spec/features/dashboard/merge_requests_spec.rb +++ b/spec/features/dashboard/merge_requests_spec.rb @@ -2,16 +2,28 @@ require 'spec_helper' describe 'Dashboard Merge Requests' do let(:current_user) { create :user } - let(:project) do - create(:empty_project) do |project| - project.add_master(current_user) - end - end + let(:project) { create(:empty_project) } + let(:project_with_merge_requests_disabled) { create(:empty_project, :merge_requests_disabled) } before do + [project, project_with_merge_requests_disabled].each { |project| project.team << [current_user, :master] } + login_as(current_user) end + describe 'new merge request dropdown' do + before { visit merge_requests_dashboard_path } + + it 'shows projects only with merge requests feature enabled', js: true do + find('.new-project-item-select-button').trigger('click') + + page.within('.select2-results') do + expect(page).to have_content(project.name_with_namespace) + expect(page).not_to have_content(project_with_merge_requests_disabled.name_with_namespace) + end + end + end + it 'should show an empty state' do visit merge_requests_dashboard_path(assignee_id: current_user.id)