diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb index e3a5ddcda9386506421685eae647caf0a0e94ae6..5547fe1d7e403c96cc9a7132ef739a4b2924ad58 100644 --- a/app/helpers/issues_helper.rb +++ b/app/helpers/issues_helper.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true module IssuesHelper + include Issues::IssueTypeHelpers + def issue_css_classes(issue) classes = ["issue"] classes << "closed" if issue.closed? diff --git a/app/services/concerns/issues/issue_type_helpers.rb b/app/services/concerns/issues/issue_type_helpers.rb new file mode 100644 index 0000000000000000000000000000000000000000..44c20d20ff1d06c6651f9ce40164e86389700cd1 --- /dev/null +++ b/app/services/concerns/issues/issue_type_helpers.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Issues + module IssueTypeHelpers + # @param object [Issue, Project] + # @param issue_type [String, Symbol] + def create_issue_type_allowed?(object, issue_type) + WorkItem::Type.base_types.key?(issue_type.to_s) && + can?(current_user, :"create_#{issue_type}", object) + end + end +end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 6dce9fd6e73817106ab7f9244f0c8eb2c8a394fa..efb5de5b17c61f175b08c2f0b4560728ba4d370e 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -3,6 +3,7 @@ module Issues class BaseService < ::IssuableBaseService include IncidentManagement::UsageData + include IssueTypeHelpers def hook_data(issue, action, old_associations: {}) hook_data = issue.to_hook_data(current_user, old_associations: old_associations) @@ -44,7 +45,7 @@ def find_work_item_type_id(issue_type) def filter_params(issue) super - params.delete(:issue_type) unless issue_type_allowed?(issue) + params.delete(:issue_type) unless create_issue_type_allowed?(issue, params[:issue_type]) filter_incident_label(issue) if params[:issue_type] moved_issue = params.delete(:moved_issue) @@ -89,12 +90,6 @@ def delete_milestone_total_issue_counter_cache(milestone) Milestones::IssuesCountService.new(milestone).delete_cache end - # @param object [Issue, Project] - def issue_type_allowed?(object) - WorkItem::Type.base_types.key?(params[:issue_type]) && - can?(current_user, :"create_#{params[:issue_type]}", object) - end - # @param issue [Issue] def filter_incident_label(issue) return unless add_incident_label?(issue) || remove_incident_label?(issue) diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 7fdc8daf15cf2b01aa237e6f85e5abc783b51875..8fd844c4886e43079f42254ccb2163ce65057106 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -80,7 +80,7 @@ def allowed_issue_params ] allowed_params << :milestone_id if can?(current_user, :admin_issue, project) - allowed_params << :issue_type if issue_type_allowed?(project) + allowed_params << :issue_type if create_issue_type_allowed?(project, params[:issue_type]) params.slice(*allowed_params) end diff --git a/app/views/shared/issuable/form/_type_selector.html.haml b/app/views/shared/issuable/form/_type_selector.html.haml index f5f6f32d5baea2c234f4eb7892fef0126e7e9e52..ae0fe54de4f064d4f2ba67b5b9387a5f7eb542a7 100644 --- a/app/views/shared/issuable/form/_type_selector.html.haml +++ b/app/views/shared/issuable/form/_type_selector.html.haml @@ -18,17 +18,19 @@ = sprite_icon('close', size: 16, css_class: 'dropdown-menu-close-icon') .dropdown-content{ data: { testid: 'issue-type-select-dropdown' } } %ul - %li.js-filter-issuable-type - = link_to new_project_issue_path(@project), class: ("is-active" if issuable.issue?) do - #{sprite_icon(work_item_type_icon(:issue), css_class: 'gl-icon')} #{_("Issue")} - %li.js-filter-issuable-type{ data: { track: { action: "select_issue_type_incident", label: "select_issue_type_incident_dropdown_option" } } } - = link_to new_project_issue_path(@project, { issuable_template: 'incident', issue: { issue_type: 'incident' } }), class: ("is-active" if issuable.incident?) do - #{sprite_icon(work_item_type_icon(:incident), css_class: 'gl-icon')} #{_("Incident")} + - if create_issue_type_allowed?(@project, :issue) + %li.js-filter-issuable-type + = link_to new_project_issue_path(@project), class: ("is-active" if issuable.issue?) do + #{sprite_icon(work_item_type_icon(:issue), css_class: 'gl-icon')} #{_('Issue')} + - if create_issue_type_allowed?(@project, :incident) + %li.js-filter-issuable-type{ data: { track: { action: "select_issue_type_incident", label: "select_issue_type_incident_dropdown_option" } } } + = link_to new_project_issue_path(@project, { issuable_template: 'incident', issue: { issue_type: 'incident' } }), class: ("is-active" if issuable.incident?) do + #{sprite_icon(work_item_type_icon(:incident), css_class: 'gl-icon')} #{_('Incident')} #js-type-popover - if issuable.incident? %p.form-text.text-muted - incident_docs_url = help_page_path('operations/incident_management/incidents.md') - - incident_docs_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: incident_docs_url } - = _('A %{incident_docs_start}modified issue%{incident_docs_end} to guide the resolution of incidents.').html_safe % { incident_docs_start: incident_docs_start, incident_docs_end: '</a>'.html_safe } + - incident_docs_start = format('<a href="%{url}" target="_blank" rel="noopener noreferrer">', url: incident_docs_url) + = format(_('A %{incident_docs_start}modified issue%{incident_docs_end} to guide the resolution of incidents.'), incident_docs_start: incident_docs_start, incident_docs_end: '</a>').html_safe diff --git a/spec/features/issues/form_spec.rb b/spec/features/issues/form_spec.rb index 4bad67acc87f5da259ebbc1cccb99620e1c99a3f..bb68dcb614ad05511207160928d13fa6707387d8 100644 --- a/spec/features/issues/form_spec.rb +++ b/spec/features/issues/form_spec.rb @@ -4,25 +4,29 @@ RSpec.describe 'New/edit issue', :js do include ActionView::Helpers::JavaScriptHelper - include FormHelper let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user)} - let_it_be(:user2) { create(:user)} + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:label) { create(:label, project: project) } let_it_be(:label2) { create(:label, project: project) } let_it_be(:issue) { create(:issue, project: project, assignees: [user], milestone: milestone) } - before do - stub_licensed_features(multiple_issue_assignees: false, issue_weights: false) + let(:current_user) { user } + before_all do project.add_maintainer(user) project.add_maintainer(user2) - sign_in(user) end - context 'new issue' do + before do + stub_licensed_features(multiple_issue_assignees: false, issue_weights: false) + + sign_in(current_user) + end + + describe 'new issue' do before do visit new_project_issue_path(project) end @@ -235,29 +239,42 @@ end describe 'displays issue type options in the dropdown' do + shared_examples 'type option is visible' do |label:, identifier:| + it "shows #{identifier} option", :aggregate_failures do + page.within('[data-testid="issue-type-select-dropdown"]') do + expect(page).to have_selector(%([data-testid="issue-type-#{identifier}-icon"])) + expect(page).to have_content(label) + end + end + end + before do page.within('.issue-form') do click_button 'Issue' end end - it 'correctly displays the Issue type option with an icon', :aggregate_failures do - page.within('[data-testid="issue-type-select-dropdown"]') do - expect(page).to have_selector('[data-testid="issue-type-issue-icon"]') - expect(page).to have_content('Issue') - end - end + it_behaves_like 'type option is visible', label: 'Issue', identifier: :issue + it_behaves_like 'type option is visible', label: 'Incident', identifier: :incident - it 'correctly displays the Incident type option with an icon', :aggregate_failures do - page.within('[data-testid="issue-type-select-dropdown"]') do - expect(page).to have_selector('[data-testid="issue-type-incident-icon"]') - expect(page).to have_content('Incident') + context 'when user is guest' do + let_it_be(:guest) { create(:user) } + + let(:current_user) { guest } + + before_all do + project.add_guest(guest) end + + it_behaves_like 'type option is visible', label: 'Issue', identifier: :issue + it_behaves_like 'type option is visible', label: 'Incident', identifier: :incident end end describe 'milestone' do - let!(:milestone) { create(:milestone, title: '"><img src=x onerror=alert(document.domain)>', project: project) } + let!(:milestone) do + create(:milestone, title: '"><img src=x onerror=alert(document.domain)>', project: project) + end it 'escapes milestone' do click_button 'Milestone' @@ -274,7 +291,7 @@ end end - context 'edit issue' do + describe 'edit issue' do before do visit edit_project_issue_path(project, issue) end @@ -329,7 +346,7 @@ end end - context 'inline edit' do + describe 'inline edit' do before do visit project_issue_path(project, issue) end