From 85c8605eadc2958cdf360c8844100e52a099bceb Mon Sep 17 00:00:00 2001 From: Maxim Mordasov Date: Thu, 21 Sep 2023 17:31:56 +0300 Subject: [PATCH] Unify breadcrumbs behaviour with other Grafana Apps and main core (#3003) # What this PR does Unify breadcrumbs behaviour with other Grafana Apps and main core ## Which issue(s) this PR fixes https://github.com/grafana/oncall/issues/1906 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 5 ++++ grafana-plugin/.eslintrc.js | 2 +- .../DefaultPageLayout/DefaultPageLayout.tsx | 10 +++---- .../src/models/alertgroup/alertgroup.ts | 6 +++-- .../src/pages/incident/Incident.tsx | 18 +++++++++++-- grafana-plugin/src/pages/index.tsx | 26 +++++++++++++----- .../src/pages/schedule/Schedule.tsx | 27 +++++++++++++++---- .../src/plugin/GrafanaPluginRootPage.tsx | 13 ++++++--- 8 files changed, 82 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23aac416..27ffc083 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Unify breadcrumbs behaviour with other Grafana Apps and main core ([#1906](https://github.com/grafana/oncall/issues/1906)) + ## v1.3.38 (2023-09-19) ### Fixed @@ -20,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Notify user via Slack/mobile push-notification when their shift swap request is taken by @joeyorlando ([#2992](https://github.com/grafana/oncall/pull/2992)) +- Unify breadcrumbs behaviour with other Grafana Apps and main core# ([1906](https://github.com/grafana/oncall/issues/1906)) ### Changed diff --git a/grafana-plugin/.eslintrc.js b/grafana-plugin/.eslintrc.js index 46f9f165..92831e8c 100644 --- a/grafana-plugin/.eslintrc.js +++ b/grafana-plugin/.eslintrc.js @@ -6,7 +6,7 @@ module.exports = { plugins: ['rulesdir', 'import'], settings: { 'import/internal-regex': - '^assets|^components|^containers|^icons|^models|^network|^pages|^services|^state|^utils|^plugin', + '^assets|^components|^containers|^contexts|^icons|^models|^network|^pages|^services|^state|^utils|^plugin', }, rules: { eqeqeq: 'warn', diff --git a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx index 3b2531a1..8f5155bc 100644 --- a/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx +++ b/grafana-plugin/src/containers/DefaultPageLayout/DefaultPageLayout.tsx @@ -1,14 +1,13 @@ import React, { FC } from 'react'; +import { NavModelItem } from '@grafana/data'; import { PluginPage } from 'PluginPage'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { AppRootProps } from 'types'; import Alerts from 'containers/Alerts/Alerts'; -import { pages } from 'pages'; import { isTopNavbar } from 'plugin/GrafanaPluginRootPage.helpers'; -import { DEFAULT_PAGE } from 'utils/consts'; import styles from './DefaultPageLayout.module.scss'; @@ -17,10 +16,11 @@ const cx = cn.bind(styles); interface DefaultPageLayoutProps extends AppRootProps { children?: any; page: string; + pageNav: NavModelItem; } const DefaultPageLayout: FC = observer((props) => { - const { children, page } = props; + const { children, page, pageNav } = props; if (isTopNavbar()) { return renderTopNavbar(); @@ -29,10 +29,8 @@ const DefaultPageLayout: FC = observer((props) => { return renderLegacyNavbar(); function renderTopNavbar(): JSX.Element { - const matchingPageNav = (pages[page] || pages[DEFAULT_PAGE]).getPageNav(); - return ( - +
{children}
); diff --git a/grafana-plugin/src/models/alertgroup/alertgroup.ts b/grafana-plugin/src/models/alertgroup/alertgroup.ts index 5b978afb..f0068af1 100644 --- a/grafana-plugin/src/models/alertgroup/alertgroup.ts +++ b/grafana-plugin/src/models/alertgroup/alertgroup.ts @@ -312,9 +312,11 @@ export class AlertGroupStore extends BaseStore { } @action - getAlert(pk: Alert['pk']) { - return makeRequest(`${this.path}${pk}`, {}).then((alert: Alert) => { + async getAlert(pk: Alert['pk']) { + return await makeRequest(`${this.path}${pk}`, {}).then((alert: Alert) => { this.alerts.set(pk, alert); + + return alert; }); } diff --git a/grafana-plugin/src/pages/incident/Incident.tsx b/grafana-plugin/src/pages/incident/Incident.tsx index 49d44ee9..60cb731e 100644 --- a/grafana-plugin/src/pages/incident/Incident.tsx +++ b/grafana-plugin/src/pages/incident/Incident.tsx @@ -64,7 +64,10 @@ import PagedUsers from './parts/PagedUsers'; const cx = cn.bind(styles); const INTEGRATION_NAME_LENGTH_LIMIT = 30; -interface IncidentPageProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> {} +interface IncidentPageProps extends WithStoreProps, PageProps, RouteComponentProps<{ id: string }> { + pageTitle: string; + setPageTitle: (value: string) => void; +} interface IncidentPageState extends PageBaseState { showIntegrationSettings?: boolean; @@ -89,6 +92,12 @@ class IncidentPage extends React.Component store.alertGroupStore.updateSilenceOptions(); } + componentWillUnmount(): void { + const { setPageTitle } = this.props; + + setPageTitle(undefined); + } + componentDidUpdate(prevProps: IncidentPageProps) { if (this.props.match.params.id !== prevProps.match.params.id) { this.update(); @@ -103,10 +112,14 @@ class IncidentPage extends React.Component match: { params: { id }, }, + setPageTitle, } = this.props; store.alertGroupStore .getAlert(id) + .then((alertGroup) => { + setPageTitle(`#${alertGroup.inside_organization_number} ${alertGroup.render_for_web.title}`); + }) .catch((error) => this.setState({ errorData: { ...getWrongTeamResponseInfo(error) } })); }; @@ -238,6 +251,7 @@ class IncidentPage extends React.Component match: { params: { id }, }, + pageTitle, } = this.props; const { alerts } = store.alertGroupStore; @@ -261,7 +275,7 @@ class IncidentPage extends React.Component {/* @ts-ignore*/} - #{incident.inside_organization_number} {incident.render_for_web.title} + {pageTitle} {incident.root_alert_group && ( diff --git a/grafana-plugin/src/pages/index.tsx b/grafana-plugin/src/pages/index.tsx index bea68483..280c80e7 100644 --- a/grafana-plugin/src/pages/index.tsx +++ b/grafana-plugin/src/pages/index.tsx @@ -17,7 +17,7 @@ export type PageDefinition = { action?: UserAction; hideTitle: boolean; // dont't automatically render title above page content - getPageNav(): { text: string; description: string }; + getPageNav: (pageTitle: string) => NavModelItem; }; function getPath(name = '') { @@ -34,6 +34,20 @@ export const pages: { [id: string]: PageDefinition } = [ path: getPath('alert-groups'), action: UserActions.AlertGroupsRead, }, + { + icon: 'bell', + id: 'alert-group', + text: '', + showOrgSwitcher: true, + getParentItem: (pageTitle: string) => ({ + text: pageTitle, + url: `${PLUGIN_ROOT}/alert-groups`, + }), + hideFromBreadcrumbs: true, + hideFromTabs: true, + path: getPath('alert-group/:id?'), + action: UserActions.AlertGroupsRead, + }, { icon: 'users-alt', id: 'users', @@ -72,10 +86,10 @@ export const pages: { [id: string]: PageDefinition } = [ icon: 'calendar-alt', id: 'schedule', text: '', - parentItem: { - text: 'Schedule', + getParentItem: (pageTitle: string) => ({ + text: pageTitle, url: `${PLUGIN_ROOT}/schedules`, - }, + }), hideFromBreadcrumbs: true, hideFromTabs: true, path: getPath('schedule/:id?'), @@ -139,10 +153,10 @@ export const pages: { [id: string]: PageDefinition } = [ if (!current.action || (current.action && isUserActionAllowed(current.action))) { prev[current.id] = { ...current, - getPageNav: () => + getPageNav: (pageTitle: string) => ({ text: isTopNavbar() ? '' : current.text, - parentItem: current.parentItem, + parentItem: current.getParentItem ? current.getParentItem(pageTitle) : undefined, hideFromBreadcrumbs: current.hideFromBreadcrumbs, hideFromTabs: current.hideFromTabs, } as NavModelItem), diff --git a/grafana-plugin/src/pages/schedule/Schedule.tsx b/grafana-plugin/src/pages/schedule/Schedule.tsx index 9bbb3993..11195d07 100644 --- a/grafana-plugin/src/pages/schedule/Schedule.tsx +++ b/grafana-plugin/src/pages/schedule/Schedule.tsx @@ -39,7 +39,10 @@ import styles from './Schedule.module.css'; const cx = cn.bind(styles); -interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> {} +interface SchedulePageProps extends PageProps, WithStoreProps, RouteComponentProps<{ id: string }> { + pageTitle: string; + setPageTitle: (value: string) => void; +} interface SchedulePageState extends PageBaseState { startMoment: dayjs.Dayjs; @@ -100,9 +103,11 @@ class SchedulePage extends React.Component } componentWillUnmount() { - const { store } = this.props; + const { store, setPageTitle } = this.props; store.scheduleStore.clearPreview(); + + setPageTitle(undefined); } render() { @@ -181,7 +186,7 @@ class SchedulePage extends React.Component level={2} onTextChange={this.handleNameChange} > - {schedule?.name} + {pageTitle} {schedule && } @@ -359,10 +364,14 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; + const { scheduleStore } = store; - return scheduleStore.loadItem(scheduleId); + return scheduleStore.loadItem(scheduleId).then((schedule) => { + setPageTitle(schedule?.name); + }); }; handleShowForm = async (shiftId: Shift['id'] | 'new') => { @@ -397,13 +406,17 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; const schedule = store.scheduleStore.items[scheduleId]; store.scheduleStore .update(scheduleId, { type: schedule.type, name: value }) - .then(() => store.scheduleStore.loadItem(scheduleId)); + .then(() => store.scheduleStore.loadItem(scheduleId)) + .then((schedule) => { + setPageTitle(schedule?.name); + }); }; updateEvents = () => { @@ -412,6 +425,7 @@ class SchedulePage extends React.Component match: { params: { id: scheduleId }, }, + setPageTitle, } = this.props; const { startMoment } = this.state; @@ -423,6 +437,9 @@ class SchedulePage extends React.Component store.scheduleStore .loadItem(scheduleId) // to refresh current oncall users + .then((schedule) => { + setPageTitle(schedule?.name); + }) .catch((error) => this.setState({ errorData: { ...getWrongTeamResponseInfo(error) } })); store.scheduleStore.updateRelatedUsers(scheduleId); // to refresh related users diff --git a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx index f04b8866..86494341 100644 --- a/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx +++ b/grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx @@ -38,6 +38,7 @@ import Users from 'pages/users/Users'; import { rootStore } from 'state'; import { useStore } from 'state/useStore'; import { isUserActionAllowed } from 'utils/authorization'; +import { DEFAULT_PAGE } from 'utils/consts'; dayjs.extend(utc); dayjs.extend(timezone); @@ -72,6 +73,8 @@ export const Root = observer((props: AppRootProps) => { const [basicDataLoaded, setBasicDataLoaded] = useState(false); + const [pageTitle, setPageTitle] = useState(''); + useEffect(() => { runQueuedUpdateData(0); }, []); @@ -103,8 +106,12 @@ export const Root = observer((props: AppRootProps) => { const userHasAccess = pagePermissionAction ? isUserActionAllowed(pagePermissionAction) : true; const query = getQueryParams(); + const getPageNav = () => { + return (pages[page] || pages[DEFAULT_PAGE]).getPageNav(pageTitle); + }; + return ( - + {!isTopNavbar() && ( <>
@@ -128,7 +135,7 @@ export const Root = observer((props: AppRootProps) => { - + @@ -146,7 +153,7 @@ export const Root = observer((props: AppRootProps) => { - +