Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #37122 - Update system status chart in job invocations detail page #871

Merged
merged 1 commit into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions app/views/api/v2/job_invocations/base.json.rabl
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ node do |invocation|
:template_id => pattern_template&.template_id,
:template_name => pattern_template&.template_name,
:effective_user => pattern_template&.effective_user,
:succeeded => invocation_count(invocation, :output_key => :success_count),
:failed => invocation_count(invocation, :output_key => :failed_count),
:pending => invocation_count(invocation, :output_key => :pending_count),
:succeeded => invocation.progress_report[:success],
:failed => invocation.progress_report[:error],
:pending => invocation.progress_report[:pending],
:cancelled => invocation.progress_report[:cancelled],
:total => invocation_count(invocation, :output_key => :total_count),
:missing => invocation.missing_hosts_count,
:total_hosts => invocation.total_hosts_count,
}
end

Expand Down
10 changes: 10 additions & 0 deletions webpack/JobInvocationDetail/JobInvocationConstants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,13 @@ export const STATUS = {
SUCCEEDED: 'succeeded',
FAILED: 'failed',
};

export const DATE_OPTIONS = {
day: 'numeric',
month: 'short',
year: 'numeric',
hour: '2-digit',
minute: '2-digit',
hour12: false,
timeZoneName: 'short',
};
38 changes: 38 additions & 0 deletions webpack/JobInvocationDetail/JobInvocationDetail.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
.job-invocation-detail-page-section {
$chart_size: 105px;

.chart-donut {
height: $chart_size;
width: $chart_size;
margin-bottom: 10px;
}

.chart-legend {
height: $chart_size;
min-width: 270px;

.legend-title {
font-weight: bold;
font-size: var(--pf-global--FontSize--sm);
margin-left: 8px;
margin-bottom: 0;
}

.pf-c-description-list {
margin-left: 8px;
margin-top: 8px;

.pf-c-description-list__term .pf-c-description-list__text {
font-weight: normal;
}
}
}

.pf-c-divider {
max-height: $chart_size;
}

.job-overview {
height: $chart_size;
}
}
38 changes: 13 additions & 25 deletions webpack/JobInvocationDetail/JobInvocationOverview.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@ import {
DescriptionListGroup,
DescriptionListDescription,
} from '@patternfly/react-core';
import { translate as __ } from 'foremanReact/common/I18n';
import DefaultLoaderEmptyState from 'foremanReact/components/HostDetails/DetailsCard/DefaultLoaderEmptyState';
import { translate as __, documentLocale } from 'foremanReact/common/I18n';

const JobInvocationOverview = ({ data }) => {
const JobInvocationOverview = ({
data,
isAlreadyStarted,
formattedStartDate,
}) => {
const {
start_at: startAt,
ssh_user: sshUser,
template_id: templateId,
template_name: templateName,
Expand All @@ -22,27 +25,6 @@ const JobInvocationOverview = ({ data }) => {
const canEditJobTemplates = permissions
? permissions.edit_job_templates
: false;
const dateOptions = {
day: 'numeric',
month: 'short',
year: 'numeric',
hour: '2-digit',
minute: '2-digit',
hour12: false,
timeZoneName: 'short',
};
let formattedStartDate = __('Not yet');

if (startAt) {
// Ensures date string compatibility across browsers
const convertedDate = new Date(startAt.replace(/[-.]/g, '/'));
if (convertedDate.getTime() <= new Date().getTime()) {
formattedStartDate = convertedDate.toLocaleString(
documentLocale(),
dateOptions
);
}
}

return (
<DescriptionList
Expand All @@ -63,7 +45,7 @@ const JobInvocationOverview = ({ data }) => {
<DescriptionListGroup>
<DescriptionListTerm>{__('Started at:')}</DescriptionListTerm>
<DescriptionListDescription>
{formattedStartDate}
{isAlreadyStarted ? formattedStartDate : __('Not yet')}
</DescriptionListDescription>
</DescriptionListGroup>
<DescriptionListGroup>
Expand Down Expand Up @@ -99,6 +81,12 @@ const JobInvocationOverview = ({ data }) => {

JobInvocationOverview.propTypes = {
data: PropTypes.object.isRequired,
isAlreadyStarted: PropTypes.bool.isRequired,
formattedStartDate: PropTypes.string,
};

JobInvocationOverview.defaultProps = {
formattedStartDate: undefined,
};

export default JobInvocationOverview;
153 changes: 153 additions & 0 deletions webpack/JobInvocationDetail/JobInvocationSystemStatusChart.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
import React, { useEffect, useState } from 'react';
import PropTypes from 'prop-types';
import { translate as __, sprintf } from 'foremanReact/common/I18n';
import {
ChartDonut,
ChartLabel,
ChartLegend,
ChartTooltip,
} from '@patternfly/react-charts';
import {
DescriptionList,
DescriptionListTerm,
DescriptionListGroup,
DescriptionListDescription,
FlexItem,
Text,
} from '@patternfly/react-core';
import {
global_palette_green_500 as successedColor,
global_palette_red_100 as failedColor,
global_palette_blue_300 as inProgressColor,
global_palette_black_600 as canceledColor,
global_palette_black_500 as emptyChartDonut,
} from '@patternfly/react-tokens';
import DefaultLoaderEmptyState from 'foremanReact/components/HostDetails/DetailsCard/DefaultLoaderEmptyState';
import './JobInvocationDetail.scss';

const JobInvocationSystemStatusChart = ({
data,
isAlreadyStarted,
formattedStartDate,
}) => {
const {
succeeded,
failed,
pending,
cancelled,
total,
total_hosts: totalHosts, // includes scheduled
} = data;
const chartData = [
{ title: __('Succeeded:'), count: succeeded, color: successedColor.value },
{ title: __('Failed:'), count: failed, color: failedColor.value },
{ title: __('In Progress:'), count: pending, color: inProgressColor.value },
{ title: __('Canceled:'), count: cancelled, color: canceledColor.value },
];
const chartDonutTitle = () => {
if (total > 0) return `${succeeded.toString()}/${total}`;
if (totalHosts > 0) return `0/${totalHosts}`;
return '0';
Comment on lines +48 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between total and totalHosts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The total represents the combined count of succeeded, failed, pending, and canceled, whereas totalHosts also includes those currently scheduled in the future.

};
const chartSize = 105;
const [legendWidth, setLegendWidth] = useState(270);

// Calculates chart legend width based on its content
useEffect(() => {
const legendContainer = document.querySelector('.chart-legend');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand it today, but probably would be best to add a comment on why its needed for future us :)

if (legendContainer) {
const rectElement = legendContainer.querySelector('rect');
if (rectElement) {
const rectWidth = parseFloat(rectElement.getAttribute('width'));
setLegendWidth(rectWidth);
}
}
}, [isAlreadyStarted, data]);

return (
<>
<FlexItem className="chart-donut">
<ChartDonut
allowTooltip
constrainToVisibleArea
data={
total > 0
? chartData.map(d => ({
label: sprintf(__(`${d.title} ${d.count} hosts`)),
y: d.count,
}))
: [{ label: sprintf(__(`Scheduled: ${totalHosts} hosts`)), y: 1 }]
}
colorScale={
total > 0 ? chartData.map(d => d.color) : [emptyChartDonut.value]
}
labelComponent={
<ChartTooltip pointerLength={0} constrainToVisibleArea />
}
title={chartDonutTitle}
titleComponent={
// inline style overrides PatternFly default styling
<ChartLabel style={{ fontSize: '20px' }} />
}
subTitle={__('Systems')}
subTitleComponent={
// inline style overrides PatternFly default styling
<ChartLabel
style={{ fontSize: '12px', fill: canceledColor.value }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be in a css file

Copy link
Contributor Author

@kmalyjur kmalyjur Mar 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be in a css file

@MariaAga is the comment // inline style overrides PatternFly default styling enough? If so, could you please merge it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah! i approved it, and have to merge permissions in plugins, only @adamruzicka can merge here:)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't want to merge with outstanding comments in case this was still unresolved

/>
}
padding={{
bottom: 0,
left: 0,
right: 0,
top: 0,
}}
width={chartSize}
height={chartSize}
/>
</FlexItem>
<FlexItem className="chart-legend">
<Text ouiaId="legend-title" className="legend-title">
{__('System status')}
</Text>
{isAlreadyStarted ? (
<ChartLegend
orientation="vertical"
itemsPerRow={2}
gutter={25}
rowGutter={7}
padding={{ left: 15 }}
data={chartData.map(d => ({
name: `${d.title} ${d.count}`,
symbol: { type: 'circle' },
}))}
colorScale={chartData.map(d => d.color)}
width={legendWidth}
height={chartSize}
/>
) : (
<DescriptionList>
<DescriptionListGroup>
<DescriptionListTerm>{__('Scheduled at:')}</DescriptionListTerm>
<DescriptionListDescription>
{formattedStartDate || <DefaultLoaderEmptyState />}
</DescriptionListDescription>
</DescriptionListGroup>
</DescriptionList>
)}
</FlexItem>
</>
);
};

JobInvocationSystemStatusChart.propTypes = {
data: PropTypes.object.isRequired,
isAlreadyStarted: PropTypes.bool.isRequired,
formattedStartDate: PropTypes.string,
};

JobInvocationSystemStatusChart.defaultProps = {
formattedStartDate: undefined,
};

export default JobInvocationSystemStatusChart;
58 changes: 48 additions & 10 deletions webpack/JobInvocationDetail/index.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
import React, { useEffect } from 'react';
import { useSelector, useDispatch } from 'react-redux';
import PropTypes from 'prop-types';
import { Divider, PageSection, Flex, FlexItem } from '@patternfly/react-core';
import { translate as __ } from 'foremanReact/common/I18n';
import { Divider, PageSection, Flex } from '@patternfly/react-core';
import { translate as __, documentLocale } from 'foremanReact/common/I18n';
import PageLayout from 'foremanReact/routes/common/PageLayout/PageLayout';
import { stopInterval } from 'foremanReact/redux/middlewares/IntervalMiddleware';
import { getData } from './JobInvocationActions';
import { selectItems } from './JobInvocationSelectors';
import JobInvocationOverview from './JobInvocationOverview';
import { JOB_INVOCATION_KEY, STATUS } from './JobInvocationConstants';
import JobInvocationSystemStatusChart from './JobInvocationSystemStatusChart';
import {
JOB_INVOCATION_KEY,
STATUS,
DATE_OPTIONS,
} from './JobInvocationConstants';
import './JobInvocationDetail.scss';

const JobInvocationDetailPage = ({
match: {
Expand All @@ -17,11 +23,28 @@ const JobInvocationDetailPage = ({
}) => {
const dispatch = useDispatch();
const items = useSelector(selectItems);
const { description, status_label: statusLabel, task } = items;
const {
description,
status_label: statusLabel,
task,
start_at: startAt,
} = items;
const finished =
statusLabel === STATUS.FAILED || statusLabel === STATUS.SUCCEEDED;
const autoRefresh = task?.state === STATUS.PENDING || false;

let isAlreadyStarted = false;
let formattedStartDate;
if (startAt) {
// Ensures date string compatibility across browsers
const convertedDate = new Date(startAt.replace(/[-.]/g, '/'));
isAlreadyStarted = convertedDate.getTime() <= new Date().getTime();
formattedStartDate = convertedDate.toLocaleString(
documentLocale(),
DATE_OPTIONS
);
}

useEffect(() => {
dispatch(getData(`/api/job_invocations/${id}`));
if (finished && !autoRefresh) {
Expand All @@ -48,17 +71,32 @@ const JobInvocationDetailPage = ({
searchable={false}
>
<React.Fragment>
<PageSection isFilled variant="light">
<Flex>
<FlexItem> </FlexItem>
<PageSection
className="job-invocation-detail-page-section"
isFilled
variant="light"
>
<Flex alignItems={{ default: 'alignItemsFlexStart' }}>
<JobInvocationSystemStatusChart
data={items}
isAlreadyStarted={isAlreadyStarted}
formattedStartDate={formattedStartDate}
/>
<Divider
orientation={{
default: 'vertical',
}}
/>
<FlexItem>
<JobInvocationOverview data={items} />
</FlexItem>
<Flex
className="job-overview"
alignItems={{ default: 'alignItemsCenter' }}
>
<JobInvocationOverview
data={items}
isAlreadyStarted={isAlreadyStarted}
formattedStartDate={formattedStartDate}
/>
</Flex>
</Flex>
</PageSection>
</React.Fragment>
Expand Down
Loading