Skip to content

Commit

Permalink
Unify breadcrumbs behaviour with other Grafana Apps and main core (#3003
Browse files Browse the repository at this point in the history
)

# What this PR does

Unify breadcrumbs behaviour with other Grafana Apps and main core

## Which issue(s) this PR fixes

#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)
  • Loading branch information
Maxim Mordasov authored Sep 21, 2023
1 parent 2848836 commit 85c8605
Show file tree
Hide file tree
Showing 8 changed files with 82 additions and 25 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion grafana-plugin/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -17,10 +16,11 @@ const cx = cn.bind(styles);
interface DefaultPageLayoutProps extends AppRootProps {
children?: any;
page: string;
pageNav: NavModelItem;
}

const DefaultPageLayout: FC<DefaultPageLayoutProps> = observer((props) => {
const { children, page } = props;
const { children, page, pageNav } = props;

if (isTopNavbar()) {
return renderTopNavbar();
Expand All @@ -29,10 +29,8 @@ const DefaultPageLayout: FC<DefaultPageLayoutProps> = observer((props) => {
return renderLegacyNavbar();

function renderTopNavbar(): JSX.Element {
const matchingPageNav = (pages[page] || pages[DEFAULT_PAGE]).getPageNav();

return (
<PluginPage page={page} pageNav={matchingPageNav}>
<PluginPage page={page} pageNav={pageNav}>
<div className={cx('root')}>{children}</div>
</PluginPage>
);
Expand Down
6 changes: 4 additions & 2 deletions grafana-plugin/src/models/alertgroup/alertgroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down
18 changes: 16 additions & 2 deletions grafana-plugin/src/pages/incident/Incident.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -89,6 +92,12 @@ class IncidentPage extends React.Component<IncidentPageProps, IncidentPageState>
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();
Expand All @@ -103,10 +112,14 @@ class IncidentPage extends React.Component<IncidentPageProps, IncidentPageState>
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) } }));
};

Expand Down Expand Up @@ -238,6 +251,7 @@ class IncidentPage extends React.Component<IncidentPageProps, IncidentPageState>
match: {
params: { id },
},
pageTitle,
} = this.props;

const { alerts } = store.alertGroupStore;
Expand All @@ -261,7 +275,7 @@ class IncidentPage extends React.Component<IncidentPageProps, IncidentPageState>
{/* @ts-ignore*/}
<HorizontalGroup align="baseline">
<Text.Title level={3} data-testid="incident-title">
#{incident.inside_organization_number} {incident.render_for_web.title}
{pageTitle}
</Text.Title>
{incident.root_alert_group && (
<Text type="secondary">
Expand Down
26 changes: 20 additions & 6 deletions grafana-plugin/src/pages/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '') {
Expand All @@ -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',
Expand Down Expand Up @@ -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?'),
Expand Down Expand Up @@ -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),
Expand Down
27 changes: 22 additions & 5 deletions grafana-plugin/src/pages/schedule/Schedule.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -100,9 +103,11 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
}

componentWillUnmount() {
const { store } = this.props;
const { store, setPageTitle } = this.props;

store.scheduleStore.clearPreview();

setPageTitle(undefined);
}

render() {
Expand Down Expand Up @@ -181,7 +186,7 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
level={2}
onTextChange={this.handleNameChange}
>
{schedule?.name}
{pageTitle}
</Text.Title>
{schedule && <ScheduleQuality schedule={schedule} lastUpdated={this.state.lastUpdated} />}
</div>
Expand Down Expand Up @@ -359,10 +364,14 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
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') => {
Expand Down Expand Up @@ -397,13 +406,17 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
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 = () => {
Expand All @@ -412,6 +425,7 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>
match: {
params: { id: scheduleId },
},
setPageTitle,
} = this.props;

const { startMoment } = this.state;
Expand All @@ -423,6 +437,9 @@ class SchedulePage extends React.Component<SchedulePageProps, SchedulePageState>

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

Expand Down
13 changes: 10 additions & 3 deletions grafana-plugin/src/plugin/GrafanaPluginRootPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -72,6 +73,8 @@ export const Root = observer((props: AppRootProps) => {

const [basicDataLoaded, setBasicDataLoaded] = useState(false);

const [pageTitle, setPageTitle] = useState('');

useEffect(() => {
runQueuedUpdateData(0);
}, []);
Expand Down Expand Up @@ -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 (
<DefaultPageLayout {...props} page={page}>
<DefaultPageLayout {...props} page={page} pageNav={getPageNav()}>
{!isTopNavbar() && (
<>
<Header />
Expand All @@ -128,7 +135,7 @@ export const Root = observer((props: AppRootProps) => {
<Incidents query={query} />
</Route>
<Route path={getRoutesForPage('alert-group')} exact>
<Incident query={query} />
<Incident query={query} pageTitle={pageTitle} setPageTitle={setPageTitle} />
</Route>
<Route path={getRoutesForPage('users')} exact>
<Users query={query} />
Expand All @@ -146,7 +153,7 @@ export const Root = observer((props: AppRootProps) => {
<Schedules query={query} />
</Route>
<Route path={getRoutesForPage('schedule')} exact>
<Schedule query={query} />
<Schedule query={query} pageTitle={pageTitle} setPageTitle={setPageTitle} />
</Route>
<Route path={getRoutesForPage('outgoing_webhooks')} exact>
<OutgoingWebhooks query={query} />
Expand Down

0 comments on commit 85c8605

Please sign in to comment.