From c6c3afef6b8de599eb739bd2aa72c9a40471b6e1 Mon Sep 17 00:00:00 2001 From: tlabaj Date: Wed, 3 Dec 2025 08:00:53 -0500 Subject: [PATCH] Fixes #38877 - Update donut chart wrapper to PF5 --- .../components/ChartBox/ChartBox.test.js | 266 ++++++++++++-- .../__snapshots__/ChartBox.test.js.snap | 249 ------------- .../charts/DonutChart/DonutChart.fixtures.js | 84 ++--- .../charts/DonutChart/DonutChart.test.js | 218 +++++++++++- .../__snapshots__/DonutChart.test.js.snap | 96 ----- .../common/charts/DonutChart/index.js | 70 +++- .../services/charts/ChartService.consts.js | 27 ++ .../services/charts/ChartService.js | 92 +++++ .../services/charts/DonutChartService.js | 14 +- .../services/charts/DonutChartService.test.js | 328 +++++++++++++++--- .../DonutChartService.test.js.snap | 204 ----------- 11 files changed, 940 insertions(+), 708 deletions(-) delete mode 100644 webpack/assets/javascripts/react_app/components/ChartBox/__snapshots__/ChartBox.test.js.snap delete mode 100644 webpack/assets/javascripts/react_app/components/common/charts/DonutChart/__snapshots__/DonutChart.test.js.snap delete mode 100644 webpack/assets/javascripts/services/charts/__snapshots__/DonutChartService.test.js.snap diff --git a/webpack/assets/javascripts/react_app/components/ChartBox/ChartBox.test.js b/webpack/assets/javascripts/react_app/components/ChartBox/ChartBox.test.js index c26c8334c76..b28009f45d3 100644 --- a/webpack/assets/javascripts/react_app/components/ChartBox/ChartBox.test.js +++ b/webpack/assets/javascripts/react_app/components/ChartBox/ChartBox.test.js @@ -1,55 +1,245 @@ -import { shallow } from 'enzyme'; import React from 'react'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; import ChartBox from './ChartBox'; -import { classFunctionUnitTest } from '../../common/testHelpers'; -jest.unmock('../../../services/charts/DonutChartService'); -jest.unmock('./'); +// Mock DonutChart to avoid CSS import issues from MessageBox +jest.mock('../common/charts/DonutChart', () => ({ + __esModule: true, + default: ({ data, noDataMsg }) => ( +
+ {data && data.length > 0 ? ( + + Chart + + ) : ( +
{noDataMsg || 'No data available'}
+ )} +
+ ), +})); + +// Mock BarChart since it still uses C3-based patternfly-react which doesn't work in Jest +jest.mock('../common/charts/BarChart', () => ({ + __esModule: true, + default: ({ data }) => ( +
+ {data && data.length > 0 ? 'Bar Chart with data' : 'Empty bar chart'} +
+ ), +})); + +// Mock MessageBox to avoid CSS import issues in Jest +jest.mock('../common/MessageBox', () => ({ + __esModule: true, + default: ({ msg }) =>
{msg}
, +})); + +// Mock Loader to avoid CSS import issues +jest.mock('../common/Loader', () => ({ + __esModule: true, + default: ({ children, status }) => ( +
+ {status === 'PENDING' ? 'Loading...' : children} +
+ ), +})); describe('ChartBox', () => { - const setup = ({ status, chart = { id: '2' } }) => - shallow( - - ); - - it('pending', () => { - const box = setup({ status: 'PENDING' }); - - expect(box).toMatchSnapshot(); + const defaultProps = { + type: 'donut', + chart: { id: 'test-chart', data: [] }, + status: 'PENDING', + title: 'Test Chart Title', + errorText: '', + noDataMsg: 'no data', + tip: 'Click to expand', + }; + + beforeEach(() => { + jest.clearAllMocks(); }); - it('error', () => { - const box = setup({ status: 'ERROR' }); + describe('rendering states', () => { + it('renders with pending status', () => { + const { container } = render( + + ); + + // Loader component uses .loader-root class + expect(container.querySelector('.loader-root')).toBeInTheDocument(); + }); + + it('renders with error status and error message', () => { + render( + + ); + + expect(screen.getByText('Failed to load data')).toBeInTheDocument(); + }); + + it('renders with resolved status and chart data', () => { + const { container } = render( + + ); - expect(box).toMatchSnapshot(); + // Mocked DonutChart renders SVG + expect(container.querySelector('svg')).toBeInTheDocument(); + expect(container.querySelector('.donut-chart-pf')).toBeInTheDocument(); + }); + + it('renders title', () => { + const { container } = render( + + ); + + expect(container.textContent).toContain('My Custom Title'); + }); }); - it('resolved', () => { - const box = setup({ - chart: { id: '2', data: [[1, 2]] }, - status: 'RESOLVED', + describe('chart types', () => { + it('renders DonutChart when type is donut', () => { + const { container } = render( + + ); + + // DonutChart renders with donut-chart-pf class + expect(container.querySelector('.donut-chart-pf')).toBeInTheDocument(); }); - expect(box).toMatchSnapshot(); + it('renders BarChart when type is bar', () => { + const { container } = render( + + ); + + // BarChart should render (mocked) + expect(screen.getByTestId('bar-chart')).toBeInTheDocument(); + expect( + container.querySelector('.donut-chart-pf') + ).not.toBeInTheDocument(); + }); }); - it('render modal', () => { - const box = setup({ - chart: { id: '2', data: [[1, 2]] }, - status: 'RESOLVED', + describe('modal functionality', () => { + it('renders chart title in the component', () => { + const { container } = render( + + ); + + expect(container.textContent).toContain('Test Chart Title'); + }); + + it('title has onClick handler when there is data', () => { + const { container } = render( + + ); + + // Title should have tooltip attribute when there's data + const titleWithTooltip = container.querySelector( + '[data-toggle="tooltip"]' + ); + expect(titleWithTooltip).toBeInTheDocument(); + }); + + it('title does not have onClick handler when there is no data', () => { + const { container } = render( + + ); + + // When no data, the title should not have tooltip attribute + const titleWithTooltip = container.querySelector( + '[data-toggle="tooltip"]' + ); + expect(titleWithTooltip).not.toBeInTheDocument(); + }); + }); + + describe('props handling', () => { + it('applies custom className', () => { + const { container } = render( + + ); + + const card = container.querySelector('.chart-box'); + expect(card).toHaveClass('custom-chart-class'); + }); + + it('uses provided config for donut chart', () => { + const { container } = render( + + ); + + expect(container.querySelector('.donut-chart-pf')).toBeInTheDocument(); + }); + + it('renders with searchUrl in chart data', () => { + const { container } = render( + + ); + + expect(container.querySelector('.donut-chart-pf')).toBeInTheDocument(); + }); + + it('renders no data message when chart has no data', () => { + const { container } = render( + + ); + + expect(container.textContent).toContain('No data available'); }); - expect(box.find('.chart-box-modal').props().isOpen).toBeFalsy(); - box.find('.panel-title').simulate('click'); - box.update(); - expect(box.find('.chart-box-modal').props().isOpen).toBeTruthy(); }); }); diff --git a/webpack/assets/javascripts/react_app/components/ChartBox/__snapshots__/ChartBox.test.js.snap b/webpack/assets/javascripts/react_app/components/ChartBox/__snapshots__/ChartBox.test.js.snap deleted file mode 100644 index 0fffa21452a..00000000000 --- a/webpack/assets/javascripts/react_app/components/ChartBox/__snapshots__/ChartBox.test.js.snap +++ /dev/null @@ -1,249 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`ChartBox error 1`] = ` - - - - some title - - - - - - - - - - - - -`; - -exports[`ChartBox pending 1`] = ` - - - - some title - - - - - - - - - - - - -`; - -exports[`ChartBox resolved 1`] = ` - - - - some title - - - - - - - - - - - - -`; diff --git a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.fixtures.js b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.fixtures.js index ed7e6796307..0839fc1adad 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.fixtures.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.fixtures.js @@ -1,55 +1,43 @@ -export default [ - ['column1', 100], - ['column2', 50], +// Input data format (what gets passed to DonutChart component) +export const inputData = [ + ['Ubuntu 14.04', 4, '#0088ce'], + ['Fedora 21', 3, '#ec7a08'], + ['Centos 7', 2, '#3f9c35'], + ['Debian 8', 1, '#005c66'], ]; -export const mockData = { - donut: { - width: 15, - label: { - show: false, - }, - }, - data: { - columns: [ - [ - 'Fedora 21 extra extra extra extra extra extra extra extra extra extra long label', - 3, - ], - ['Ubuntu 14.04', 4], - ['Centos 7 extra extra long label', 2], - ['Debian 8', 1], - ['Fedora 27', 2], - ['Fedora 26', 1], - ], - }, - tooltip: { - show: true, - }, - legend: { - show: false, - }, - padding: { - top: 0, - left: 0, - right: 0, - bottom: 0, - }, - noDataMsg: 'No data available', - tip: 'Expand the chart', - status: 'RESOLVED', - id: 'operatingsystem', - title: 'OS Distribution', - search: '/hosts?search=os_title=~VAL~', +export const inputDataWithLongLabels = [ + [ + 'Fedora 21 extra extra extra extra extra extra extra extra extra extra long label', + 3, + '#ec7a08', + ], + ['Ubuntu 14.04', 4, '#0088ce'], + ['Centos 7 extra extra long label', 2, '#3f9c35'], + ['Debian 8', 1, '#005c66'], +]; + +// PF5 Victory chart config format (what getDonutChartConfig returns) +export const mockChartConfig = { + id: 'test-chart', + data: [ + { x: 'Ubuntu 14.04', y: 4, name: 'Ubuntu 14.04' }, + { x: 'Fedora 21', y: 3, name: 'Fedora 21' }, + { x: 'Centos 7', y: 2, name: 'Centos 7' }, + { x: 'Debian 8', y: 1, name: 'Debian 8' }, + ], + colorScale: ['#0088ce', '#ec7a08', '#3f9c35', '#005c66'], + width: 240, + height: 240, + padding: { top: 20, left: 20, right: 20, bottom: 20 }, + innerRadius: 75, + labels: jest.fn(({ datum }) => `${datum.x}: ${datum.y}`), + events: undefined, + legendData: undefined, }; -export const emptyData = { - data: { - type: 'donut', - columns: [], - names: {}, - }, - noDataMsg: 'No data available', +export const emptyChartConfig = { + data: [], }; export const zeroedData = [ diff --git a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.test.js b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.test.js index 33bda555cf8..826240f83fa 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.test.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/DonutChart.test.js @@ -1,21 +1,215 @@ -import { shallow } from 'enzyme'; import React from 'react'; -import { mockData, emptyData } from './DonutChart.fixtures'; +import { render, screen } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { + inputData, + mockChartConfig, + emptyChartConfig, +} from './DonutChart.fixtures'; import DonutChart from './'; import * as chartService from '../../../../../services/charts/DonutChartService'; -jest.unmock('./'); -describe('renders DonutChart', () => { - it('render donut chart', () => { - chartService.getDonutChartConfig = jest.fn(() => mockData); - const wrapper = shallow(); +// Mock the DonutChartService +jest.mock('../../../../../services/charts/DonutChartService'); - expect(wrapper).toMatchSnapshot(); +// Mock MessageBox to avoid CSS import issues in Jest +jest.mock('../../MessageBox', () => ({ + __esModule: true, + default: ({ msg }) =>
{msg}
, +})); + +describe('DonutChart', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + describe('with valid data', () => { + it('renders the chart with data', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render(); + + // Check that the chart container is rendered + expect(container.querySelector('.donut-chart-pf')).toBeInTheDocument(); + // Check that SVG chart is rendered + expect(container.querySelector('svg')).toBeInTheDocument(); + }); + + it('displays the percentage title for the largest segment', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render( + + ); + + // Ubuntu has 4 out of 10 total = 40% + // The title is rendered as text in the SVG + expect(container.textContent).toContain('40.0%'); + }); + + it('respects precision: 0 for whole number percentages', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render( + + ); + + // Ubuntu has 4 out of 10 total = 40% (no decimal places) + expect(container.textContent).toContain('40%'); + // Should NOT contain decimal point when precision is 0 + expect(container.textContent).not.toContain('40.0%'); + }); + + it('displays the subtitle with the largest segment name', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render( + + ); + + expect(container.textContent).toContain('Ubuntu 14.04'); + }); + + it('passes data to the service for configuration', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render(); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + data: inputData, + config: 'medium', + }) + ); + }); + + it('supports custom title with primary and secondary text', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render( + + ); + + expect(container.textContent).toContain('50'); + expect(container.textContent).toContain('Hosts'); + }); + + it('passes onclick handler to the service', () => { + const mockOnClick = jest.fn(); + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render(); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + onclick: mockOnClick, + }) + ); + }); + + it('passes searchUrl and searchFilters to the service', () => { + const searchUrl = '/hosts?search=~VAL~'; + const searchFilters = { test: 'filter' }; + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render( + + ); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + searchUrl, + searchFilters, + }) + ); + }); + + it('renders chart segments for each data item', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + const { container } = render(); + + // ChartDonut renders path elements for each segment + const paths = container.querySelectorAll('path'); + expect(paths.length).toBeGreaterThan(0); + }); + }); + + describe('with empty data', () => { + it('renders MessageBox when no data is available', () => { + chartService.getDonutChartConfig.mockReturnValue(emptyChartConfig); + + const { container } = render(); + + expect(screen.getByTestId('message-box')).toBeInTheDocument(); + expect(screen.getByText('No data available')).toBeInTheDocument(); + // Should not render chart container + expect( + container.querySelector('.donut-chart-pf') + ).not.toBeInTheDocument(); + }); + + it('renders custom noDataMsg when provided', () => { + chartService.getDonutChartConfig.mockReturnValue(emptyChartConfig); + + render(); + + expect(screen.getByTestId('message-box')).toBeInTheDocument(); + expect(screen.getByText('Custom empty message')).toBeInTheDocument(); + }); }); - it('render empty state', () => { - chartService.getDonutChartConfig = jest.fn(() => emptyData); - const wrapper = shallow(); - expect(wrapper).toMatchSnapshot(); + describe('with different config sizes', () => { + it('uses regular config by default', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render(); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + config: 'regular', + }) + ); + }); + + it('accepts medium config', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render(); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + config: 'medium', + }) + ); + }); + + it('accepts large config', () => { + chartService.getDonutChartConfig.mockReturnValue(mockChartConfig); + + render(); + + expect(chartService.getDonutChartConfig).toHaveBeenCalledWith( + expect.objectContaining({ + config: 'large', + }) + ); + }); }); }); diff --git a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/__snapshots__/DonutChart.test.js.snap b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/__snapshots__/DonutChart.test.js.snap deleted file mode 100644 index 091f9570983..00000000000 --- a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/__snapshots__/DonutChart.test.js.snap +++ /dev/null @@ -1,96 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`renders DonutChart render donut chart 1`] = ` - -`; - -exports[`renders DonutChart render empty state 1`] = ` - -`; diff --git a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/index.js b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/index.js index e701eed9a3f..78bef7fb9f2 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/index.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/DonutChart/index.js @@ -1,6 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; -import { DonutChart as PfDonutChart } from 'patternfly-react'; +import { ChartDonut, ChartThemeColor } from '@patternfly/react-charts'; import { getDonutChartConfig } from '../../../../../services/charts/DonutChartService'; import MessageBox from '../../MessageBox'; import { translate as __ } from '../../../../../react_app/common/I18n'; @@ -12,7 +12,6 @@ const DonutChart = ({ config, noDataMsg, title, - unloadData, searchUrl, searchFilters, }) => { @@ -22,15 +21,66 @@ const DonutChart = ({ onclick, searchUrl, searchFilters, + title, }); - if (chartConfig.data.columns.length > 0) { + if (chartConfig.data && chartConfig.data.length > 0) { + const { + data: chartData, + colorScale, + width, + height, + padding, + innerRadius, + labels, + events, + legendData, + } = chartConfig; + + // Calculate title values for display based on title prop + const total = chartData.reduce((sum, item) => sum + item.y, 0); + let titleText = ''; + let subtitleText = ''; + + if (title) { + if (title.type === 'percent' && chartData.length > 0) { + // Find the largest value for percentage calculation + const maxItem = chartData.reduce( + (max, item) => (item.y > max.y ? item : max), + chartData[0] + ); + const percentage = + total > 0 + ? ((maxItem.y / total) * 100).toFixed(title.precision ?? 1) + : 0; + titleText = `${percentage}%`; + subtitleText = title.secondary || maxItem.x; + } else if (title.primary) { + titleText = title.primary; + subtitleText = title.secondary || ''; + } + } + return ( - +
+ +
); } return ; @@ -41,7 +91,6 @@ DonutChart.propTypes = { config: PropTypes.oneOf(['regular', 'medium', 'large']), noDataMsg: PropTypes.string, title: PropTypes.object, - unloadData: PropTypes.bool, onclick: PropTypes.func, searchUrl: PropTypes.string, searchFilters: PropTypes.object, @@ -51,8 +100,7 @@ DonutChart.defaultProps = { data: undefined, config: 'regular', noDataMsg: __('No data available'), - title: { type: 'percent', precision: 1 }, - unloadData: false, + title: { type: 'percent', precision: 0 }, onclick: noop, searchUrl: undefined, searchFilters: undefined, diff --git a/webpack/assets/javascripts/services/charts/ChartService.consts.js b/webpack/assets/javascripts/services/charts/ChartService.consts.js index f09ee44de28..6682f362f67 100644 --- a/webpack/assets/javascripts/services/charts/ChartService.consts.js +++ b/webpack/assets/javascripts/services/charts/ChartService.consts.js @@ -50,8 +50,21 @@ export const chartConfig = { size: enums.SIZE.REGULAR, }; +// PF5 Victory Charts Configurations for Donut Charts +// Note: PF5 uses different configuration structure than C3.js export const donutChartConfig = { ...chartConfig, + // PF5-specific properties + size: enums.SIZE.REGULAR, + padding: { + top: 20, + left: 20, + right: 20, + bottom: 20, + }, + // innerRadius controls the donut hole size (higher = larger hole) + // Regular: small donut width means larger inner radius (thinner ring) + innerRadius: 75, // Approximates C3 width: 15 donut: { width: enums.WIDTH.SMALL, label: { show: false }, @@ -61,6 +74,13 @@ export const donutChartConfig = { export const donutMediumChartConfig = { ...donutChartConfig, size: enums.SIZE.MEDIUM, + padding: { + top: 20, + left: 20, + right: 20, + bottom: 20, + }, + innerRadius: 100, // Approximates C3 width: 20 legend: { show: false }, donut: { ...donutChartConfig.donut, @@ -71,6 +91,13 @@ export const donutMediumChartConfig = { export const donutLargeChartConfig = { ...donutChartConfig, size: enums.SIZE.LARGE, + padding: { + top: 20, + left: 20, + right: 20, + bottom: 80, // More space for legend at bottom + }, + innerRadius: 130, // Approximates C3 width: 25 legend: { show: true, position: 'bottom' }, donut: { ...donutChartConfig.donut, diff --git a/webpack/assets/javascripts/services/charts/ChartService.js b/webpack/assets/javascripts/services/charts/ChartService.js index 197c1c5ce2e..978bea8b30a 100644 --- a/webpack/assets/javascripts/services/charts/ChartService.js +++ b/webpack/assets/javascripts/services/charts/ChartService.js @@ -105,6 +105,98 @@ export const getChartConfig = ({ }; }; +// PF5 Victory Charts Configuration +export const getDonutChartConfigPF5 = ({ + data, + config, + onclick, + searchUrl, + searchFilters, + title, + id = uuidV1(), +}) => { + const chartConfigForType = chartsSizeConfig.donut[config]; + const dataExists = doDataExist(data); + + if (!dataExists) { + return { data: [] }; + } + + // Transform data from [label, value, color] to Victory format + const transformedData = []; + const colorScale = []; + + data.forEach(item => { + const [label, value, color] = item; + transformedData.push({ x: label, y: value, name: label }); + + if (color) { + colorScale.push(color); + } + }); + + // Handle click events for Victory charts + const isClickable = onclick || searchUrl; + const events = isClickable + ? [ + { + target: 'data', + eventHandlers: { + onClick: (event, props) => { + const { datum } = props; + const fullLabel = datum.name || datum.x; + + // Create data object compatible with onclick handler + const clickData = { + id: fullLabel, + name: datum.x, + value: datum.y, + }; + + if (onclick) { + onclick(clickData, event); + } + + if (searchUrl) { + navigateToSearch(searchUrl, searchFilters || {}, clickData); + } + }, + onMouseEnter: () => { + document.body.style.cursor = 'pointer'; + return null; + }, + onMouseLeave: () => { + document.body.style.cursor = 'default'; + return null; + }, + }, + }, + ] + : undefined; + + // Calculate total for percentage display in tooltips + const total = transformedData.reduce((sum, item) => sum + item.y, 0); + + // Configure labels to show full name and percentage in tooltip + const labels = ({ datum }) => { + const percentage = + total > 0 ? ((datum.y / total) * 100).toFixed(title?.precision ?? 1) : 0; + return `${datum.name}: ${percentage}%`; + }; + + return { + id, + data: transformedData, + colorScale: colorScale.length > 0 ? colorScale : undefined, + width: chartConfigForType.size.width, + height: chartConfigForType.size.height, + padding: chartConfigForType.padding, + innerRadius: chartConfigForType.innerRadius, + labels, + events, + }; +}; + export const navigateToSearch = (url, searchFilters, data) => { let val = searchFilters[data.id] || data.id; let setUrl; diff --git a/webpack/assets/javascripts/services/charts/DonutChartService.js b/webpack/assets/javascripts/services/charts/DonutChartService.js index 02d4d0b8db4..ca60b40ee14 100644 --- a/webpack/assets/javascripts/services/charts/DonutChartService.js +++ b/webpack/assets/javascripts/services/charts/DonutChartService.js @@ -1,5 +1,5 @@ import uuidV1 from 'uuid/v1'; -import { getChartConfig, navigateToSearch } from './ChartService'; +import { getDonutChartConfigPF5 } from './ChartService'; export const getDonutChartConfig = ({ data, @@ -7,15 +7,15 @@ export const getDonutChartConfig = ({ onclick, searchUrl, searchFilters, + title, id = uuidV1(), }) => - getChartConfig({ - type: 'donut', + getDonutChartConfigPF5({ data, config, + onclick, + searchUrl, + searchFilters, + title, id, - onclick: (d, element) => { - if (onclick) onclick(d, element); - if (searchUrl) navigateToSearch(searchUrl, searchFilters || {}, d); - }, }); diff --git a/webpack/assets/javascripts/services/charts/DonutChartService.test.js b/webpack/assets/javascripts/services/charts/DonutChartService.test.js index c112cc398ae..11b1c334b1f 100644 --- a/webpack/assets/javascripts/services/charts/DonutChartService.test.js +++ b/webpack/assets/javascripts/services/charts/DonutChartService.test.js @@ -1,58 +1,300 @@ -import Immutable from 'seamless-immutable'; import { getDonutChartConfig } from './DonutChartService'; import { - zeroedData, - mixedData, - dataWithLongLabels, + inputData, + inputDataWithLongLabels, } from '../../react_app/components/common/charts/DonutChart/DonutChart.fixtures'; jest.mock('./ChartService.consts'); describe('getDonutChartConfig', () => { - it('data should be filtered', () => { - expect( - getDonutChartConfig({ + describe('data filtering and transformation', () => { + it('returns empty data array when all values are zero', () => { + const zeroedData = [ + ['Fedora 21', 0], + ['Ubuntu 14.04', 0], + ['Centos 7', 0], + ]; + + const result = getDonutChartConfig({ data: zeroedData, onclick: jest.fn(), config: 'regular', - id: 'some-id', - }) - ).toMatchSnapshot(); + id: 'test-id', + }); + + expect(result.data).toEqual([]); + }); + + it('includes all data values, even zeros', () => { + const mixedData = [ + ['Fedora 21', 3], + ['Ubuntu 14.04', 2], + ['Centos 7', 0], + ['Debian 8', 0], + ]; + + const result = getDonutChartConfig({ + data: mixedData, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + // PF5 Victory charts include all segments, even with zero values + expect(result.data).toHaveLength(4); + expect(result.data[0]).toEqual({ + x: 'Fedora 21', + y: 3, + name: 'Fedora 21', + }); + expect(result.data[1]).toEqual({ + x: 'Ubuntu 14.04', + y: 2, + name: 'Ubuntu 14.04', + }); + // Zero values are included + expect(result.data[2].y).toBe(0); + expect(result.data[3].y).toBe(0); + }); + + it('transforms data to Victory format with x, y, and name', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + expect(result.data).toHaveLength(4); + result.data.forEach(item => { + expect(item).toHaveProperty('x'); + expect(item).toHaveProperty('y'); + expect(item).toHaveProperty('name'); + }); + }); + + it('extracts custom colors from data', () => { + const dataWithColors = [ + ['OS 1', 10, '#ff0000'], + ['OS 2', 20, '#00ff00'], + ['OS 3', 30, '#0000ff'], + ]; + + const result = getDonutChartConfig({ + data: dataWithColors, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + expect(result.colorScale).toEqual(['#ff0000', '#00ff00', '#0000ff']); + }); + + it('handles data without custom colors', () => { + const dataWithoutColors = [ + ['OS 1', 10], + ['OS 2', 20], + ]; + + const result = getDonutChartConfig({ + data: dataWithoutColors, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + expect(result.colorScale).toBeUndefined(); + }); + }); + + describe('label handling', () => { + it('preserves full labels including long ones', () => { + const result = getDonutChartConfig({ + data: inputDataWithLongLabels, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + const longLabelItem = result.data.find(item => + item.name.includes('extra extra extra extra extra extra extra') + ); + + // Both x and name should contain the full label + expect(longLabelItem.x).toBe(longLabelItem.name); + expect(longLabelItem.name).toContain( + 'extra extra extra extra extra extra extra' + ); + }); + + it('preserves all labels as-is', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + result.data.forEach(item => { + expect(item.x).toBe(item.name); + }); + }); + + it('includes full label in tooltip via labels function', () => { + const result = getDonutChartConfig({ + data: inputDataWithLongLabels, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + const longLabelItem = result.data.find(item => + item.name.includes('extra extra extra extra extra extra extra') + ); + + const tooltipText = result.labels({ datum: longLabelItem }); + + // Tooltip should contain full name and percentage + expect(tooltipText).toContain(longLabelItem.name); + expect(tooltipText).toMatch(/%$/); + }); + }); + + describe('configuration sizes', () => { + it('uses regular size configuration', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + expect(result.width).toBe(240); + expect(result.height).toBe(240); + }); + + it('uses medium size configuration', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'medium', + id: 'test-id', + }); + + expect(result.width).toBe(320); + expect(result.height).toBe(320); + }); + + it('uses large size configuration', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'large', + id: 'test-id', + }); + + expect(result.height).toBe(500); + }); }); - it('data should not be filtered with regular size donut ', () => { - expect( - getDonutChartConfig( - Immutable({ - data: mixedData, - onclick: jest.fn(), - config: 'regular', - id: 'some-id', - }) - ) - ).toMatchSnapshot(); + + describe('event handling', () => { + it('includes click events when onclick is provided', () => { + const mockOnClick = jest.fn(); + + const result = getDonutChartConfig({ + data: inputData, + onclick: mockOnClick, + config: 'regular', + id: 'test-id', + }); + + expect(result.events).toBeDefined(); + expect(result.events).toHaveLength(1); + expect(result.events[0].target).toBe('data'); + }); + + it('includes click events when searchUrl is provided', () => { + const result = getDonutChartConfig({ + data: inputData, + searchUrl: '/hosts?search=~VAL~', + config: 'regular', + id: 'test-id', + }); + + expect(result.events).toBeDefined(); + expect(result.events).toHaveLength(1); + }); + + it('does not include events when neither onclick nor searchUrl provided', () => { + const result = getDonutChartConfig({ + data: inputData, + config: 'regular', + id: 'test-id', + }); + + expect(result.events).toBeUndefined(); + }); }); - it('data should not be filtered with large size donut', () => { - expect( - getDonutChartConfig( - Immutable({ - data: mixedData, - onclick: jest.fn(), - config: 'large', - id: 'some-id', - }) - ) - ).toMatchSnapshot(); + + describe('tooltip labels', () => { + it('displays percentage in tooltip', () => { + const result = getDonutChartConfig({ + data: [ + ['Item 1', 40], + ['Item 2', 60], + ], + config: 'regular', + id: 'test-id', + }); + + const label1 = result.labels({ + datum: { x: 'Item 1', y: 40, name: 'Item 1' }, + }); + const label2 = result.labels({ + datum: { x: 'Item 2', y: 60, name: 'Item 2' }, + }); + + expect(label1).toBe('Item 1: 40.0%'); + expect(label2).toBe('Item 2: 60.0%'); + }); }); - it('data with long labels should be trimmed', () => { - expect( - getDonutChartConfig( - Immutable({ - data: dataWithLongLabels, - onclick: jest.fn(), - config: 'regular', - id: 'some-id', - }) - ) - ).toMatchSnapshot(); + + describe('return structure', () => { + it('returns required PF5 Victory chart configuration properties', () => { + const result = getDonutChartConfig({ + data: inputData, + onclick: jest.fn(), + config: 'regular', + id: 'test-id', + }); + + expect(result).toHaveProperty('id'); + expect(result).toHaveProperty('data'); + expect(result).toHaveProperty('width'); + expect(result).toHaveProperty('height'); + expect(result).toHaveProperty('padding'); + expect(result).toHaveProperty('innerRadius'); + expect(result).toHaveProperty('labels'); + }); + + it('uses provided id', () => { + const result = getDonutChartConfig({ + data: inputData, + config: 'regular', + id: 'custom-id', + }); + + expect(result.id).toBe('custom-id'); + }); + + it('generates id if not provided', () => { + const result = getDonutChartConfig({ + data: inputData, + config: 'regular', + }); + + expect(result.id).toBeDefined(); + expect(typeof result.id).toBe('string'); + }); }); }); diff --git a/webpack/assets/javascripts/services/charts/__snapshots__/DonutChartService.test.js.snap b/webpack/assets/javascripts/services/charts/__snapshots__/DonutChartService.test.js.snap deleted file mode 100644 index ca2b2eb82a9..00000000000 --- a/webpack/assets/javascripts/services/charts/__snapshots__/DonutChartService.test.js.snap +++ /dev/null @@ -1,204 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`getDonutChartConfig data should be filtered 1`] = ` -Object { - "color": Object { - "pattern": Array [], - }, - "data": Object { - "columns": Array [], - "onclick": [Function], - }, - "donut": Object { - "label": Object { - "show": false, - }, - "width": 15, - }, - "id": "some-id", - "legend": Object { - "show": false, - }, - "onrendered": [Function], - "padding": Object { - "bottom": 0, - "left": 0, - "right": 0, - "top": 0, - }, - "size": Object { - "height": 240, - "width": 240, - }, - "tooltip": Object { - "format": Object { - "name": [Function], - }, - }, -} -`; - -exports[`getDonutChartConfig data should not be filtered with large size donut 1`] = ` -Object { - "color": Object { - "pattern": Array [], - }, - "data": Object { - "columns": Array [ - Array [ - "Fedora 21", - 3, - ], - Array [ - "Ubuntu 14.04", - 2, - ], - Array [ - "Centos 7", - 0, - ], - Array [ - "Debian 8", - 0, - ], - ], - "onclick": [Function], - }, - "donut": Object { - "label": Object { - "show": false, - }, - "width": 25, - }, - "id": "some-id", - "legend": Object { - "position": "bottom", - "show": true, - }, - "onrendered": [Function], - "padding": Object { - "bottom": 0, - "left": 0, - "right": 0, - "top": 0, - }, - "size": Object { - "height": 500, - }, - "tooltip": Object { - "format": Object { - "name": [Function], - }, - }, -} -`; - -exports[`getDonutChartConfig data should not be filtered with regular size donut 1`] = ` -Object { - "color": Object { - "pattern": Array [], - }, - "data": Object { - "columns": Array [ - Array [ - "Fedora 21", - 3, - ], - Array [ - "Ubuntu 14.04", - 2, - ], - Array [ - "Centos 7", - 0, - ], - Array [ - "Debian 8", - 0, - ], - ], - "onclick": [Function], - }, - "donut": Object { - "label": Object { - "show": false, - }, - "width": 15, - }, - "id": "some-id", - "legend": Object { - "show": false, - }, - "onrendered": [Function], - "padding": Object { - "bottom": 0, - "left": 0, - "right": 0, - "top": 0, - }, - "size": Object { - "height": 240, - "width": 240, - }, - "tooltip": Object { - "format": Object { - "name": [Function], - }, - }, -} -`; - -exports[`getDonutChartConfig data with long labels should be trimmed 1`] = ` -Object { - "color": Object { - "pattern": Array [], - }, - "data": Object { - "columns": Array [ - Array [ - "Fedora 21 ...", - 3, - ], - Array [ - "Ubuntu 14....", - 2, - ], - Array [ - "Centos 7 e...", - 0, - ], - Array [ - "Debian 8 e...", - 0, - ], - ], - "onclick": [Function], - }, - "donut": Object { - "label": Object { - "show": false, - }, - "width": 15, - }, - "id": "some-id", - "legend": Object { - "show": false, - }, - "onrendered": [Function], - "padding": Object { - "bottom": 0, - "left": 0, - "right": 0, - "top": 0, - }, - "size": Object { - "height": 240, - "width": 240, - }, - "tooltip": Object { - "format": Object { - "name": [Function], - }, - }, -} -`;