Merge branch '244943-incidents-navigation' into 'master'

Show incident details page via `issues/incident/:id`

Closes #244943

See merge request gitlab-org/gitlab!43459

app/assets/javascripts/incidents/components/incidents_list.vue

@@ -19,6 +19,7 @@ import Api from '~/api';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import FilteredSearchBar from '~/vue_shared/components/filtered_search_bar/filtered_search_bar_root.vue';
 import AuthorToken from '~/vue_shared/components/filtered_search_bar/tokens/author_token.vue';
+import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { convertToSnakeCase } from '~/lib/utils/text_utility';
 import { s__, __ } from '~/locale';
 import { urlParamsToObject } from '~/lib/utils/common_utils';
@@ -40,6 +41,7 @@ import {
   TH_CREATED_AT_TEST_ID,
   TH_SEVERITY_TEST_ID,
   TH_PUBLISHED_TEST_ID,
+  INCIDENT_DETAILS_PATH,
 } from '../constants';
 
 const tdClass =
@@ -111,6 +113,7 @@ export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
+  mixins: [glFeatureFlagsMixin()],
   inject: [
     'projectPath',
     'newIssuePath',
@@ -332,7 +335,10 @@ export default {
       return Boolean(assignees.nodes?.length);
     },
     navigateToIncidentDetails({ iid }) {
-      return visitUrl(joinPaths(this.issuePath, iid));
+      const path = this.glFeatures.issuesIncidentDetails
+        ? joinPaths(this.issuePath, INCIDENT_DETAILS_PATH)
+        : this.issuePath;
+      return visitUrl(joinPaths(path, iid));
     },
     handlePageChange(page) {
       const { startCursor, endCursor } = this.incidents.pageInfo;

app/assets/javascripts/incidents/constants.js

@@ -38,3 +38,4 @@ export const DEFAULT_PAGE_SIZE = 20;
 export const TH_CREATED_AT_TEST_ID = { 'data-testid': 'incident-management-created-at-sort' };
 export const TH_SEVERITY_TEST_ID = { 'data-testid': 'incident-management-severity-sort' };
 export const TH_PUBLISHED_TEST_ID = { 'data-testid': 'incident-management-published-sort' };
+export const INCIDENT_DETAILS_PATH = 'incident';

app/assets/javascripts/lib/utils/common_utils.js

@@ -44,6 +44,7 @@ export const checkPageAndAction = (page, action) => {
   return pagePath === page && actionPath === action;
 };
 
+export const isInIncidentPage = () => checkPageAndAction('issues', 'incident');
 export const isInIssuePage = () => checkPageAndAction('issues', 'show');
 export const isInMRPage = () => checkPageAndAction('merge_requests', 'show');
 export const isInEpicPage = () => checkPageAndAction('epics', 'show');

app/assets/javascripts/pages/projects/incidents/show/index.js

+import initRelatedIssues from '~/related_issues';
+import initShow from '../../issues/show';
+
+document.addEventListener('DOMContentLoaded', () => {
+  initShow();
+  initRelatedIssues();
+});

app/assets/javascripts/sidebar/mount_sidebar.js

@@ -14,7 +14,7 @@ import sidebarSubscriptions from './components/subscriptions/sidebar_subscriptio
 import SidebarSeverity from './components/severity/sidebar_severity.vue';
 import Translate from '../vue_shared/translate';
 import createDefaultClient from '~/lib/graphql';
-import { isInIssuePage, parseBoolean } from '~/lib/utils/common_utils';
+import { isInIssuePage, isInIncidentPage, parseBoolean } from '~/lib/utils/common_utils';
 import createFlash from '~/flash';
 import { __ } from '~/locale';
 import labelsSelectModule from '~/vue_shared/components/sidebar/labels_select_vue/store';
@@ -51,7 +51,7 @@ function mountAssigneesComponent(mediator) {
           projectPath: fullPath,
           field: el.dataset.field,
           signedIn: el.hasAttribute('data-signed-in'),
-          issuableType: isInIssuePage() ? 'issue' : 'merge_request',
+          issuableType: isInIssuePage() || isInIncidentPage() ? 'issue' : 'merge_request',
         },
       }),
   });
@@ -158,7 +158,7 @@ function mountLockComponent() {
   const initialData = JSON.parse(dataNode.innerHTML);
 
   let importStore;
-  if (isInIssuePage()) {
+  if (isInIssuePage() || isInIncidentPage()) {
     importStore = import(/* webpackChunkName: 'notesStore' */ '~/notes/stores').then(
       ({ store }) => store,
     );

app/controllers/projects/incidents_controller.rb

 # frozen_string_literal: true
 
 class Projects::IncidentsController < Projects::ApplicationController
+  include IssuableActions
+  include Gitlab::Utils::StrongMemoize
+
   before_action :authorize_read_issue!
+  before_action :check_feature_flag, only: [:show]
+  before_action :load_incident, only: [:show]
+
+  before_action do
+    push_frontend_feature_flag(:issues_incident_details, @project)
+  end
 
   def index
   end
+
+  private
+
+  def incident
+    strong_memoize(:incident) do
+      incident_finder
+        .execute
+        .inc_relations_for_view
+        .iid_in(params[:id])
+        .without_order
+        .first
+    end
+  end
+
+  def load_incident
+    @issue = incident # needed by rendered view
+    return render_404 unless can?(current_user, :read_issue, incident)
+
+    @noteable = incident
+    @note = incident.project.notes.new(noteable: issuable)
+  end
+
+  alias_method :issuable, :incident
+
+  def incident_finder
+    IssuesFinder.new(current_user, project_id: @project.id, issue_types: :incident)
+  end
+
+  def serializer
+    IssueSerializer.new(current_user: current_user, project: incident.project)
+  end
+
+  def check_feature_flag
+    render_404 unless Feature.enabled?(:issues_incident_details, @project)
+  end
 end

app/controllers/projects/issues_controller.rb

@@ -239,7 +239,7 @@ class Projects::IssuesController < Projects::ApplicationController
     return @issue if defined?(@issue)
 
     # The Sortable default scope causes performance issues when used with find_by
-    @issuable = @noteable = @issue ||= @project.issues.includes(author: :status).where(iid: params[:id]).reorder(nil).take!
+    @issuable = @noteable = @issue ||= @project.issues.inc_relations_for_view.iid_in(params[:id]).without_order.take!
     @note = @project.notes.new(noteable: @issuable)
 
     return render_404 unless can?(current_user, :read_issue, @issue)

app/helpers/nav_helper.rb

@@ -55,7 +55,8 @@ module NavHelper
       current_path?('projects/merge_requests/conflicts#show') ||
       current_path?('issues#show') ||
       current_path?('milestones#show') ||
-      current_path?('issues#designs')
+      current_path?('issues#designs') ||
+      current_path?('incidents#show')
   end
 
   def admin_monitoring_nav_links

app/models/issue.rb

@@ -126,6 +126,7 @@ class Issue < ApplicationRecord
   scope :counts_by_state, -> { reorder(nil).group(:state_id).count }
 
   scope :service_desk, -> { where(author: ::User.support_bot) }
+  scope :inc_relations_for_view, -> { includes(author: :status) }
 
   # An issue can be uniquely identified by project_id and iid
   # Takes one or more sets of composite IDs, expressed as hash-like records of

app/views/projects/incidents/_new_branch.html.haml

+= render 'projects/issues/new_branch'

app/views/projects/incidents/show.html.haml

+= render template: 'projects/issues/show'

config/feature_flags/development/issues_incident_details.yml

+---
+name: issues_incident_details
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/43459
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/257842
+type: development
+group: group::health
+default_enabled: false

config/routes/project.rb

@@ -311,6 +311,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do
 
         resources :incidents, only: [:index]
 
+        get 'issues/incident/:id' => 'incidents#show', as: :issues_incident
+
         namespace :error_tracking do
           resources :projects, only: :index
         end

spec/controllers/projects/incidents_controller_spec.rb

@@ -3,42 +3,127 @@
 require 'spec_helper'
 
 RSpec.describe Projects::IncidentsController do
-  let_it_be(:project) { create(:project) }
+  let_it_be_with_refind(:project) { create(:project) }
   let_it_be(:developer) { create(:user) }
   let_it_be(:guest) { create(:user) }
+  let_it_be(:anonymous) { nil }
 
   before_all do
     project.add_guest(guest)
     project.add_developer(developer)
   end
 
+  before do
+    sign_in(user) if user
+  end
+
+  subject { make_request }
+
+  shared_examples 'not found' do
+    include_examples 'returning response status', :not_found
+  end
+
+  shared_examples 'login required' do
+    it 'redirects to the login page' do
+      subject
+
+      expect(response).to redirect_to(new_user_session_path)
+    end
+  end
+
   describe 'GET #index' do
     def make_request
-      get :index, params: { namespace_id: project.namespace, project_id: project }
+      get :index, params: project_params
     end
 
-    it 'shows the page for users with guest role' do
-      sign_in(guest)
-      make_request
+    let(:user) { developer }
+
+    it 'shows the page' do
+      subject
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to render_template(:index)
     end
 
-    it 'shows the page for users with developer role' do
-      sign_in(developer)
-      make_request
+    context 'when user is unauthorized' do
+      let(:user) { anonymous }
+
+      it_behaves_like 'login required'
+    end
+
+    context 'when user is a guest' do
+      let(:user) { guest }
+
+      it 'shows the page' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template(:index)
+      end
+    end
+  end
+
+  describe 'GET #show' do
+    def make_request
+      get :show, params: project_params(id: resource)
+    end
+
+    let_it_be(:resource) { create(:incident, project: project) }
+    let(:user) { developer }
+
+    it 'renders incident page' do
+      subject
 
       expect(response).to have_gitlab_http_status(:ok)
-      expect(response).to render_template(:index)
+      expect(response).to render_template(:show)
+
+      expect(assigns(:incident)).to be_present
+      expect(assigns(:incident).author.association(:status)).to be_loaded
+      expect(assigns(:issue)).to be_present
+      expect(assigns(:noteable)).to eq(assigns(:incident))
     end
 
-    context 'when user is unauthorized' do
-      it 'redirects to the login page' do
-        make_request
+    context 'with feature flag disabled' do
+      before do
+        stub_feature_flags(issues_incident_details: false)
+      end
+
+      it_behaves_like 'not found'
+    end
+
+    context 'with non existing id' do
+      let(:resource) { non_existing_record_id }
+
+      it_behaves_like 'not found'
+    end
 
-        expect(response).to redirect_to(new_user_session_path)
+    context 'for issue' do
+      let_it_be(:resource) { create(:issue, project: project) }
+
+      it_behaves_like 'not found'
+    end
+
+    context 'when user is a guest' do
+      let(:user) { guest }
+
+      it 'shows the page' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template(:show)
       end
     end
+
+    context 'when unauthorized' do
+      let(:user) { anonymous }
+
+      it_behaves_like 'login required'
+    end
+  end
+
+  private
+
+  def project_params(opts = {})
+    opts.reverse_merge(namespace_id: project.namespace, project_id: project)
   end
 end

spec/features/incidents/incident_details_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Incident details', :js do
+  let_it_be(:project) { create(:project) }
+  let_it_be(:developer) { create(:user) }
+  let_it_be(:incident) { create(:incident, project: project, author: developer) }
+
+  before_all do
+    project.add_developer(developer)
+  end
+
+  before do
+    sign_in(developer)
+
+    visit project_issues_incident_path(project, incident)
+    wait_for_requests
+  end
+
+  context 'when a developer+ displays the incident' do
+    it 'shows the incident' do
+      page.within('.issuable-details') do
+        expect(find('h2')).to have_content(incident.title)
+      end
+    end
+
+    it 'does not show design management' do
+      expect(page).not_to have_selector('.js-design-management')
+    end
+  end
+end

spec/frontend/incidents/components/incidents_list_spec.js

@@ -28,10 +28,10 @@ import mockFilters from '../mocks/incidents_filter.json';
 
 jest.mock('~/lib/utils/url_utility', () => ({
   visitUrl: jest.fn().mockName('visitUrlMock'),
-  joinPaths: jest.fn().mockName('joinPaths'),
-  mergeUrlParams: jest.fn().mockName('mergeUrlParams'),
-  setUrlParams: jest.fn().mockName('setUrlParams'),
-  updateHistory: jest.fn().mockName('updateHistory'),
+  joinPaths: jest.fn(),
+  mergeUrlParams: jest.fn(),
+  setUrlParams: jest.fn(),
+  updateHistory: jest.fn(),
 }));
 
 describe('Incidents List', () => {
@@ -81,12 +81,13 @@ describe('Incidents List', () => {
         newIssuePath,
         incidentTemplateName,
         incidentType,
-        issuePath: '/project/isssues',
+        issuePath: '/project/issues',
         publishedAvailable: true,
         emptyListSvgPath,
         textQuery: '',
         authorUsernamesQuery: '',
         assigneeUsernamesQuery: '',
+        issuesIncidentDetails: false,
       },
       stubs: {
         GlButton: true,
@@ -182,13 +183,6 @@ describe('Incidents List', () => {
         expect(src).toBe(avatarUrl);
       });
 
-      it('contains a link to the issue details', () => {
-        findTableRows()
-          .at(0)
-          .trigger('click');
-        expect(visitUrl).toHaveBeenCalledWith(joinPaths(`/project/isssues/`, mockIncidents[0].iid));
-      });
-
       it('renders a closed icon for closed incidents', () => {
         expect(findClosedIcon().length).toBe(
           mockIncidents.filter(({ state }) => state === 'closed').length,
@@ -199,6 +193,30 @@ describe('Incidents List', () => {
     it('renders severity per row', () => {
       expect(findSeverity().length).toBe(mockIncidents.length);
     });
+
+    it('contains a link to the issue details page', () => {
+      findTableRows()
+        .at(0)
+        .trigger('click');
+      expect(visitUrl).toHaveBeenCalledWith(joinPaths(`/project/issues/`, mockIncidents[0].iid));
+    });
+
+    it('contains a link to the incident details page', async () => {
+      beforeEach(() =>
+        mountComponent({
+          data: { incidents: { list: mockIncidents }, incidentsCount: {} },
+          loading: false,
+          provide: { glFeatures: { issuesIncidentDetails: true } },
+        }),
+      );
+
+      findTableRows()
+        .at(0)
+        .trigger('click');
+      expect(visitUrl).toHaveBeenCalledWith(
+        joinPaths(`/project/issues/incident`, mockIncidents[0].iid),
+      );
+    });
   });
 
   describe('Create Incident', () => {
@@ -218,11 +236,10 @@ describe('Incidents List', () => {
       );
     });
 
-    it('sets button loading on click', () => {
+    it('sets button loading on click', async () => {
       findCreateIncidentBtn().vm.$emit('click');
-      return wrapper.vm.$nextTick().then(() => {
-        expect(findCreateIncidentBtn().attributes('loading')).toBe('true');
-      });
+      await wrapper.vm.$nextTick();
+      expect(findCreateIncidentBtn().attributes('loading')).toBe('true');
     });
 
     it("doesn't show the button when list is empty", () => {
@@ -254,51 +271,47 @@ describe('Incidents List', () => {
     });
 
     describe('prevPage', () => {
-      it('returns prevPage button', () => {
+      it('returns prevPage button', async () => {
         findPagination().vm.$emit('input', 3);
 
-        return wrapper.vm.$nextTick(() => {
-          expect(
-            findPagination()
-              .findAll('.page-item')
-              .at(0)
-              .text(),
-          ).toBe('Prev');
-        });
+        await wrapper.vm.$nextTick();
+        expect(
+          findPagination()
+            .findAll('.page-item')
+            .at(0)
+            .text(),
+        ).toBe('Prev');
       });
 
-      it('returns prevPage number', () => {
+      it('returns prevPage number', async () => {
         findPagination().vm.$emit('input', 3);
 
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.vm.prevPage).toBe(2);
-        });
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.prevPage).toBe(2);
       });
 
-      it('returns 0 when it is the first page', () => {
+      it('returns 0 when it is the first page', async () => {
         findPagination().vm.$emit('input', 1);
 
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.vm.prevPage).toBe(0);
-        });
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.prevPage).toBe(0);
       });
     });
 
     describe('nextPage', () => {
-      it('returns nextPage button', () => {
+      it('returns nextPage button', async () => {
         findPagination().vm.$emit('input', 3);
 
-        return wrapper.vm.$nextTick(() => {
-          expect(
-            findPagination()
-              .findAll('.page-item')
-              .at(1)
-              .text(),
-          ).toBe('Next');
-        });
+        await wrapper.vm.$nextTick();
+        expect(
+          findPagination()
+            .findAll('.page-item')
+            .at(1)
+            .text(),
+        ).toBe('Next');
       });
 
-      it('returns nextPage number', () => {
+      it('returns nextPage number', async () => {
         mountComponent({
           data: {
             incidents: {
@@ -312,17 +325,15 @@ describe('Incidents List', () => {
         });
         findPagination().vm.$emit('input', 1);
 
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.vm.nextPage).toBe(2);
-        });
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.nextPage).toBe(2);
       });
 
-      it('returns `null` when currentPage is already last page', () => {
+      it('returns `null` when currentPage is already last page', async () => {
         findStatusTabs().vm.$emit('input', 1);
         findPagination().vm.$emit('input', 1);
-        return wrapper.vm.$nextTick(() => {
-          expect(wrapper.vm.nextPage).toBeNull();
-        });
+        await wrapper.vm.$nextTick();
+        expect(wrapper.vm.nextPage).toBeNull();
       });
     });