Merge branch 'pl-work-item-prep-work' into 'master'

Move helper methods related to issue types into a helper module

See merge request gitlab-org/gitlab!73823

app/helpers/issues_helper.rb

 # frozen_string_literal: true
 
 module IssuesHelper
+  include Issues::IssueTypeHelpers
+
   def issue_css_classes(issue)
     classes = ["issue"]
     classes << "closed" if issue.closed?

app/services/concerns/issues/issue_type_helpers.rb

+# 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

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)

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

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

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