From 40d82f2adac19723f3c6f01284d7ccf7c15c08d4 Mon Sep 17 00:00:00 2001 From: MariaAga Date: Mon, 1 Jun 2026 10:06:26 +0100 Subject: [PATCH] Fixes #39383 - add no-magic-numbers js lint rule --- .../lint_generic_config.js | 27 +++++++++++++++++++ script/lint/lint_core_config.js | 1 + webpack/assets/javascripts/bundle_novnc.js | 3 ++- webpack/assets/javascripts/dashboard/index.js | 5 ++-- .../react_app/common/rtlTestHelpers.js | 1 + .../components/Editor/EditorActions.js | 6 ++++- .../Tabs/Details/Cards/Provisioning/index.js | 13 ++++++--- .../Cards/Virtualization/VirtVmware.js | 3 ++- .../HostDetails/Tabs/ReportsTab/helpers.js | 3 ++- .../HostDetails/Tabs/ReportsTab/index.js | 7 +++-- .../components/HostDetails/Tabs/index.js | 13 ++++++--- .../HostsIndex/Columns/reportedDataColumns.js | 10 ++++--- .../Layout/components/InstanceBanner.js | 19 ++++++++----- .../MemoryAllocationInput.js | 18 ++++++------- .../__tests__/MemoryAllocationInput.test.js | 6 ++--- .../MemoryAllocationInput/constants.js | 4 ++- .../components/PF4/Bookmarks/BookmarkItems.js | 8 ++++-- .../TableIndexPage/Table/TableIndexHooks.js | 4 ++- .../TemplateGeneratorActions.js | 6 +++-- .../components/ToastsList/helpers.js | 4 ++- .../react_app/components/ToastsList/index.js | 10 ++++--- .../common/DateTimePicker/DateTimePicker.js | 3 ++- .../ResourceLoadFailedEmptyState.js | 7 +++-- .../charts/AreaChart/AreaChartHelpers.js | 12 ++++++--- .../common/charts/BarChart/index.js | 9 ++++--- .../common/charts/DonutChart/index.js | 5 +++- .../common/charts/helpers/LegendHelpers.js | 26 ++++++++++++------ .../assets/javascripts/react_app/constants.js | 16 +++++++++++ .../javascripts/react_app/mockRequests.js | 3 ++- .../react_app/redux/actions/common/forms.js | 3 ++- .../redux/actions/notifications/index.js | 3 ++- .../javascripts/react_app/redux/consts.js | 1 + .../redux/reducers/hosts/storage/vmware.js | 3 ++- .../RegistrationCommandsPage/index.js | 4 +-- .../services/charts/AreaChartService.js | 3 ++- .../services/charts/ChartService.js | 14 ++++++++-- 36 files changed, 208 insertions(+), 75 deletions(-) diff --git a/script/lint/@theforeman/eslint-plugin-foreman/lint_generic_config.js b/script/lint/@theforeman/eslint-plugin-foreman/lint_generic_config.js index 913a1e4eb8..b15812b22f 100644 --- a/script/lint/@theforeman/eslint-plugin-foreman/lint_generic_config.js +++ b/script/lint/@theforeman/eslint-plugin-foreman/lint_generic_config.js @@ -65,6 +65,33 @@ module.exports = { packageDir: packageJsonDirectories, }, ], + 'no-magic-numbers': [ + 'error', + { + ignore: [ + // Common general-purpose values + 0, + 1, + -1, + 2, + 3, + 4, + 5, + 6, + 8, + 9, + // Pagination per-page options + 10, + 15, + 20, + 25, + 50, + ], + ignoreArrayIndexes: true, + enforceConst: true, + detectObjects: false, + }, + ], '@theforeman/rules/require-ouiaid': 'error', }, }; diff --git a/script/lint/lint_core_config.js b/script/lint/lint_core_config.js index 84653e25b3..28b89c8b81 100644 --- a/script/lint/lint_core_config.js +++ b/script/lint/lint_core_config.js @@ -208,6 +208,7 @@ module.exports = { 'xml', 'xpi', 'xyz', + 'uname', 'yaml', ], minLength: 3, diff --git a/webpack/assets/javascripts/bundle_novnc.js b/webpack/assets/javascripts/bundle_novnc.js index 42a768b754..b63ca7384b 100644 --- a/webpack/assets/javascripts/bundle_novnc.js +++ b/webpack/assets/javascripts/bundle_novnc.js @@ -52,8 +52,9 @@ function connectFinished() { showStatus('normal', __('Connected')); } +const WS_ABNORMAL_CLOSURE = 1006; function onClose(e) { - if (e.code === 1006) { + if (e.code === WS_ABNORMAL_CLOSURE) { showStatus( 'failed', __( diff --git a/webpack/assets/javascripts/dashboard/index.js b/webpack/assets/javascripts/dashboard/index.js index 4ff95e8630..9e495e0548 100644 --- a/webpack/assets/javascripts/dashboard/index.js +++ b/webpack/assets/javascripts/dashboard/index.js @@ -25,6 +25,7 @@ $(document).on('ContentLoad', () => { let refreshTimeout; +const AUTO_REFRESH_INTERVAL_MS = 60000; function autoRefresh() { const element = $('.auto-refresh'); clearTimeout(refreshTimeout); @@ -37,7 +38,7 @@ function autoRefresh() { if (autoRefreshIsOn && hasFocus) { reloadPage(); } - }, 60000); + }, AUTO_REFRESH_INTERVAL_MS); } } @@ -45,7 +46,7 @@ export function startGridster() { $('.gridster>ul') .gridster({ widget_margins: [10, 10], - widget_base_dimensions: [94, 340], + widget_base_dimensions: [94, 340], // eslint-disable-line no-magic-numbers max_size_x: 12, min_cols: 12, max_cols: 12, diff --git a/webpack/assets/javascripts/react_app/common/rtlTestHelpers.js b/webpack/assets/javascripts/react_app/common/rtlTestHelpers.js index d8f6936d6a..c7a98e1bb6 100644 --- a/webpack/assets/javascripts/react_app/common/rtlTestHelpers.js +++ b/webpack/assets/javascripts/react_app/common/rtlTestHelpers.js @@ -204,6 +204,7 @@ export const rtlHelpers = { /** * Helper to wait for async operations in tests */ + // eslint-disable-next-line no-magic-numbers waitForAsync: async (callback, timeout = 1000) => new Promise(resolve => setTimeout(() => { diff --git a/webpack/assets/javascripts/react_app/components/Editor/EditorActions.js b/webpack/assets/javascripts/react_app/components/Editor/EditorActions.js index 6eee129e6c..3494682520 100644 --- a/webpack/assets/javascripts/react_app/components/Editor/EditorActions.js +++ b/webpack/assets/javascripts/react_app/components/Editor/EditorActions.js @@ -199,7 +199,11 @@ const createHostAPIRequest = async (query, array, url, dispatch, getState) => { return onResultsError(error); } }; -const debouncedCreateHostAPIRequest = debounce(createHostAPIRequest, 250); +const DEBOUNCE_DELAY_MS = 250; +const debouncedCreateHostAPIRequest = debounce( + createHostAPIRequest, + DEBOUNCE_DELAY_MS +); export const onHostSearch = e => (dispatch, getState) => { if (e.target.value === '') diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Provisioning/index.js b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Provisioning/index.js index d9c1227780..9c42b52a06 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Provisioning/index.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Provisioning/index.js @@ -13,7 +13,7 @@ import CardTemplate from '../../../../Templates/CardItem/CardTemplate'; import Slot from '../../../../../common/Slot'; import SkeletonLoader from '../../../../../common/SkeletonLoader'; import DefaultLoaderEmptyState from '../../../../DetailsCard/DefaultLoaderEmptyState'; -import { STATUS } from '../../../../../../constants'; +import { STATUS, MS_PER_SECOND } from '../../../../../../constants'; const ProvisioningCard = ({ status, hostDetails }) => { const { @@ -30,8 +30,9 @@ const ProvisioningCard = ({ status, hostDetails }) => { round: true, }; + const BUILD_OFFSET_MS = 500000; const getWordsDurations = duration => - duration > 0 && duration < 1000 + duration > 0 && duration < MS_PER_SECOND ? __('Less than a second') : humanizeDuration(duration, dateOptions); @@ -40,7 +41,9 @@ const ProvisioningCard = ({ status, hostDetails }) => { const duration = initiatedAt && installedAt && - Math.abs(installedDate.getTime() + 500000 - initiateDate.getTime()); + Math.abs( + installedDate.getTime() + BUILD_OFFSET_MS - initiateDate.getTime() + ); return ( @@ -53,7 +56,9 @@ const ProvisioningCard = ({ status, hostDetails }) => { > {(duration || duration === 0) && ( {getWordsDurations(duration)} diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Virtualization/VirtVmware.js b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Virtualization/VirtVmware.js index 2d2ff7c8a5..db17c33e8f 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Virtualization/VirtVmware.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/Details/Cards/Virtualization/VirtVmware.js @@ -7,6 +7,7 @@ import { } from '@patternfly/react-core'; import { number_to_human_size as NumberToHumanSize } from 'number_helpers'; import { translate as __ } from '../../../../../../common/I18n'; +import { BYTES_PER_KB } from '../../../../../../constants'; const VirtVmware = ({ vm }) => ( <> @@ -21,7 +22,7 @@ const VirtVmware = ({ vm }) => ( {__('Memory')} - {NumberToHumanSize(vm.memory_mb * 1024 ** 2, { + {NumberToHumanSize(vm.memory_mb * BYTES_PER_KB ** 2, { strip_insignificant_zeros: true, })} diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/helpers.js b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/helpers.js index f33f6f26e8..a82a4e4d5c 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/helpers.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/helpers.js @@ -205,7 +205,8 @@ export const getColumns = (fetchReports, origin) => { there is no need to show that origin column. */ if (!origin) { - columns.splice(-2, 0, { + const ORIGIN_COLUMN_OFFSET = -2; + columns.splice(ORIGIN_COLUMN_OFFSET, 0, { title: __('Origin'), formatter: originFormatter, }); diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/index.js b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/index.js index d6e30e523b..f4b6d69c01 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/index.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/ReportsTab/index.js @@ -15,7 +15,10 @@ import { } from '../../../../redux/API/APISelectors'; import { useForemanSettings } from '../../../../Root/Context/ForemanContext'; import ReportsTable from './ReportsTable'; -import { getControllerSearchProps } from '../../../../constants'; +import { + HTTP_STATUS_CODES, + getControllerSearchProps, +} from '../../../../constants'; import PermissionDenied from '../../../PermissionDenied'; const ReportsTab = ({ hostName, origin }) => { @@ -99,7 +102,7 @@ const ReportsTab = ({ hostName, origin }) => { }, [history] ); - if (response?.status === 403) { + if (response?.status === HTTP_STATUS_CODES.FORBIDDEN) { return ; } return ( diff --git a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/index.js b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/index.js index 91f7b4d7b1..b98c0bd31f 100644 --- a/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/index.js +++ b/webpack/assets/javascripts/react_app/components/HostDetails/Tabs/index.js @@ -7,26 +7,31 @@ import DetailTab from './Details'; import ReportsTab from './ReportsTab'; import ParametersTab from './Parameters'; +const TAB_WEIGHT_OVERVIEW = 5000; +const TAB_WEIGHT_DETAILS = 4000; +const TAB_WEIGHT_PARAMETERS = 850; +const TAB_WEIGHT_REPORTS = 477; + export const registerCoreTabs = () => { addGlobalFill( TABS_SLOT_ID, DEFAULT_TAB, , - 5000, + TAB_WEIGHT_OVERVIEW, { title: __('Overview') } ); addGlobalFill( TABS_SLOT_ID, 'Details', , - 4000, + TAB_WEIGHT_DETAILS, { title: __('Details') } ); addGlobalFill( TABS_SLOT_ID, 'Reports', , - 477, + TAB_WEIGHT_REPORTS, { title: __('Reports'), } @@ -35,7 +40,7 @@ export const registerCoreTabs = () => { TABS_SLOT_ID, 'Parameters', , - 850, + TAB_WEIGHT_PARAMETERS, { title: __('Parameters'), } diff --git a/webpack/assets/javascripts/react_app/components/HostsIndex/Columns/reportedDataColumns.js b/webpack/assets/javascripts/react_app/components/HostsIndex/Columns/reportedDataColumns.js index 70aacede84..e3e63740f0 100644 --- a/webpack/assets/javascripts/react_app/components/HostsIndex/Columns/reportedDataColumns.js +++ b/webpack/assets/javascripts/react_app/components/HostsIndex/Columns/reportedDataColumns.js @@ -1,6 +1,7 @@ /* eslint-disable camelcase */ import { number_to_human_size as NumberToHumanSize } from 'number_helpers'; import { translate as __ } from '../../../common/I18n'; +import { BYTES_PER_KB } from '../../../constants'; const reportedDataColumns = [ { @@ -30,9 +31,12 @@ const reportedDataColumns = [ title: __('RAM'), wrapper: hostDetails => { if (!hostDetails?.reported_data?.ram) return null; - return NumberToHumanSize(hostDetails.reported_data.ram * 1024 * 1024, { - strip_insignificant_zeros: true, - }); + return NumberToHumanSize( + hostDetails.reported_data.ram * BYTES_PER_KB * BYTES_PER_KB, + { + strip_insignificant_zeros: true, + } + ); }, isSorted: false, weight: 1300, diff --git a/webpack/assets/javascripts/react_app/components/Layout/components/InstanceBanner.js b/webpack/assets/javascripts/react_app/components/Layout/components/InstanceBanner.js index c0c9e67a52..447a8dc4c3 100644 --- a/webpack/assets/javascripts/react_app/components/Layout/components/InstanceBanner.js +++ b/webpack/assets/javascripts/react_app/components/Layout/components/InstanceBanner.js @@ -2,6 +2,13 @@ import React from 'react'; import PropTypes from 'prop-types'; import { Banner } from '@patternfly/react-core'; +// sRGB luminance coefficients per ITU-R BT.709 +const LUMINANCE_R = 0.2126; +const LUMINANCE_G = 0.7152; +const LUMINANCE_B = 0.0722; +const MAX_CHANNEL_VALUE = 255; +const LUMINANCE_THRESHOLD = 0.5; + const getContrastColor = backgroundColor => { const hexToRgb = hex => { const result = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i.exec(hex); @@ -14,15 +21,13 @@ const getContrastColor = backgroundColor => { : null; }; backgroundColor = hexToRgb(backgroundColor); - // Calculate the relative luminance of the background color const luminance = - (0.2126 * backgroundColor.r + - 0.7152 * backgroundColor.g + - 0.0722 * backgroundColor.b) / - 255; + (LUMINANCE_R * backgroundColor.r + + LUMINANCE_G * backgroundColor.g + + LUMINANCE_B * backgroundColor.b) / + MAX_CHANNEL_VALUE; - // Choose black or white text based on the relative luminance - return luminance > 0.5 ? 'black' : 'white'; + return luminance > LUMINANCE_THRESHOLD ? 'black' : 'white'; }; const validateHexColor = instanceColor => { // Check if the string is a valid hex color code diff --git a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/MemoryAllocationInput.js b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/MemoryAllocationInput.js index 9cf46396e0..be13e3a499 100644 --- a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/MemoryAllocationInput.js +++ b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/MemoryAllocationInput.js @@ -2,7 +2,7 @@ import RCInputNumber from 'rc-input-number'; import React, { useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import { sprintf, translate as __ } from '../../common/I18n'; -import { MB_FORMAT, MEGABYTES } from './constants'; +import { DEFAULT_MEMORY_MB, MB_FORMAT, BYTES_PER_MB } from './constants'; import '../common/forms/NumericInput.scss'; import { noop } from '../../common/helpers'; @@ -18,16 +18,16 @@ const MemoryAllocationInput = ({ setError, setWarning, }) => { - const [valueMB, setValueMB] = useState(value / MEGABYTES); + const [valueMB, setValueMB] = useState(value / BYTES_PER_MB); useEffect(() => { - const valueBytes = valueMB * MEGABYTES; + const valueBytes = valueMB * BYTES_PER_MB; if (maxValue && valueBytes > maxValue) { setWarning(null); setError( sprintf( __('Specified value is higher than maximum value %s'), - `${maxValue / MEGABYTES} ${MB_FORMAT}` + `${maxValue / BYTES_PER_MB} ${MB_FORMAT}` ) ); } else if (recommendedMaxValue && valueBytes > recommendedMaxValue) { @@ -35,7 +35,7 @@ const MemoryAllocationInput = ({ setWarning( sprintf( __('Specified value is higher than recommended maximum %s'), - `${recommendedMaxValue / MEGABYTES} ${MB_FORMAT}` + `${recommendedMaxValue / BYTES_PER_MB} ${MB_FORMAT}` ) ); } else { @@ -50,7 +50,7 @@ const MemoryAllocationInput = ({ v = Math.floor(valueMB / 2); } setValueMB(v); - onChange(v * MEGABYTES); + onChange(v * BYTES_PER_MB); }; return ( @@ -62,13 +62,13 @@ const MemoryAllocationInput = ({ parser={str => str.replace(/\D/g, '')} onChange={handleChange} disabled={disabled} - min={minValue && minValue / MEGABYTES} + min={minValue && minValue / BYTES_PER_MB} step={1} precision={0} name="" prefixCls="foreman-numeric-input" /> - + ); }; @@ -97,7 +97,7 @@ MemoryAllocationInput.propTypes = { }; MemoryAllocationInput.defaultProps = { - value: 2048 * MEGABYTES, + value: DEFAULT_MEMORY_MB * BYTES_PER_MB, onChange: noop, recommendedMaxValue: null, maxValue: null, diff --git a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/__tests__/MemoryAllocationInput.test.js b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/__tests__/MemoryAllocationInput.test.js index 9ad986357e..8c33ae7923 100644 --- a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/__tests__/MemoryAllocationInput.test.js +++ b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/__tests__/MemoryAllocationInput.test.js @@ -1,7 +1,7 @@ import React from 'react'; import { mount } from 'enzyme'; import { Provider } from 'react-redux'; -import { MEGABYTES } from '../constants'; +import { BYTES_PER_MB } from '../constants'; import MemoryAllocationInput from '../'; @@ -11,7 +11,7 @@ describe('MemoryAllocationInput', () => { const setWarning = jest.fn(); const component = mount( @@ -23,7 +23,7 @@ describe('MemoryAllocationInput', () => { it('error alert', async () => { const setError = jest.fn(); const component = mount( - + ); expect(component.find('.foreman-numeric-input-input').prop('value')).toEqual('21504 MB'); expect(setError.mock.calls.length).toBe(1); diff --git a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/constants.js b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/constants.js index 81dc96eef8..8bb86144cb 100644 --- a/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/constants.js +++ b/webpack/assets/javascripts/react_app/components/MemoryAllocationInput/constants.js @@ -1,3 +1,5 @@ export const MB_FORMAT = 'MB'; -export const MEGABYTES = 1048576; +export const BYTES_PER_MB = 1048576; + +export const DEFAULT_MEMORY_MB = 2048; diff --git a/webpack/assets/javascripts/react_app/components/PF4/Bookmarks/BookmarkItems.js b/webpack/assets/javascripts/react_app/components/PF4/Bookmarks/BookmarkItems.js index 91efcffbfa..505fe181db 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/Bookmarks/BookmarkItems.js +++ b/webpack/assets/javascripts/react_app/components/PF4/Bookmarks/BookmarkItems.js @@ -42,8 +42,12 @@ const pendingItem = ( ); +const MAX_BOOKMARK_NAME_LENGTH = 90; + const bookmarksList = ({ bookmarks, onBookmarkClick }) => { - const hasLongerName = bookmarks.some(bookmark => bookmark.name.length > 90); + const hasLongerName = bookmarks.some( + bookmark => bookmark.name.length > MAX_BOOKMARK_NAME_LENGTH + ); return ( (bookmarks.length > 0 && @@ -70,7 +74,7 @@ const errorItem = errors => ( 90 ? 'adapt-long-bookmark' : '' + errors.length > MAX_BOOKMARK_NAME_LENGTH ? 'adapt-long-bookmark' : '' }`} key="bookmarks-errors" isDisabled diff --git a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableIndexHooks.js b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableIndexHooks.js index fb7b7ba02b..526e01b31d 100644 --- a/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableIndexHooks.js +++ b/webpack/assets/javascripts/react_app/components/PF4/TableIndexPage/Table/TableIndexHooks.js @@ -3,6 +3,7 @@ import { isEqual } from 'lodash'; import URI from 'urijs'; import { useHistory } from 'react-router-dom'; import { useAPI } from '../../../../common/hooks/API/APIHooks'; +import { HTTP_STATUS_CODES } from '../../../../constants'; /** @@ -119,7 +120,8 @@ export const useCurrentUserTablePreferences = ({ tableName }) => { userTablePreferenceResponse.response?.columns; const hasPreference = !( - userTablePreferenceResponse.response?.response?.status === 404 + userTablePreferenceResponse.response?.response?.status === + HTTP_STATUS_CODES.NOT_FOUND ); return { diff --git a/webpack/assets/javascripts/react_app/components/TemplateGenerator/TemplateGeneratorActions.js b/webpack/assets/javascripts/react_app/components/TemplateGenerator/TemplateGeneratorActions.js index 121cf44b4b..09272ce3de 100644 --- a/webpack/assets/javascripts/react_app/components/TemplateGenerator/TemplateGeneratorActions.js +++ b/webpack/assets/javascripts/react_app/components/TemplateGenerator/TemplateGeneratorActions.js @@ -1,6 +1,7 @@ /* eslint-disable promise/prefer-await-to-then */ import { saveAs } from 'file-saver'; import { API } from '../../redux/API'; +import { HTTP_STATUS_CODES } from '../../constants'; import { TEMPLATE_GENERATE_REQUEST, @@ -40,7 +41,8 @@ const _downloadFile = response => { const _getErrors = errorResponse => { if (!errorResponse || !errorResponse.data) return null; - if (errorResponse.status === 422) return errorResponse.data.errors; + if (errorResponse.status === HTTP_STATUS_CODES.UNPROCESSABLE_ENTITY) + return errorResponse.data.errors; if (errorResponse.data.error) return [errorResponse.data.error]; // most of >500 return [errorResponse.data]; }; @@ -50,7 +52,7 @@ export const pollReportData = pollUrl => dispatch => { return API.get(pollUrl, { responseType: 'blob' }) .then(response => { - if (response.status === 200) { + if (response.status === HTTP_STATUS_CODES.OK) { dispatch({ type: TEMPLATE_GENERATE_SUCCESS, payload: {} }); _downloadFile(response); } else if (pollingInterval) { diff --git a/webpack/assets/javascripts/react_app/components/ToastsList/helpers.js b/webpack/assets/javascripts/react_app/components/ToastsList/helpers.js index bee8645860..19235d12a7 100644 --- a/webpack/assets/javascripts/react_app/components/ToastsList/helpers.js +++ b/webpack/assets/javascripts/react_app/components/ToastsList/helpers.js @@ -20,8 +20,10 @@ export const toastType = type => { return fallbackTypes[type] || AlertVariant.custom; }; +const MAX_TOAST_TITLE_LENGTH = 60; + export const toastTitle = (message, type) => { - if (message.length <= 60) return message; + if (message.length <= MAX_TOAST_TITLE_LENGTH) return message; return defaultTitle(type); }; diff --git a/webpack/assets/javascripts/react_app/components/ToastsList/index.js b/webpack/assets/javascripts/react_app/components/ToastsList/index.js index 42f781e645..941340afbe 100644 --- a/webpack/assets/javascripts/react_app/components/ToastsList/index.js +++ b/webpack/assets/javascripts/react_app/components/ToastsList/index.js @@ -10,9 +10,11 @@ import { } from '@patternfly/react-core'; import { addToast, deleteToast, selectToastsList } from './slice'; -import { toastType, toastTitle } from './helpers'; +import { toastType, toastTitle, MAX_TOAST_TITLE_LENGTH } from './helpers'; import './style.scss'; +const TOAST_TIMEOUT_MS = 8000; + const ToastsList = ({ railsMessages }) => { const dispatch = useDispatch(); const messages = useSelector(selectToastsList); @@ -30,7 +32,7 @@ const ToastsList = ({ railsMessages }) => { key={key} title={toastTitle(message, toastType(type))} variant={toastType(type)} - timeout={sticky ? false : 8000} + timeout={sticky ? false : TOAST_TIMEOUT_MS} onTimeout={() => dispatch(deleteToast(key))} className="foreman-toast" actionClose={ @@ -45,7 +47,9 @@ const ToastsList = ({ railsMessages }) => { } {...toastProps} > - {(message.length > 60 || React.isValidElement(message)) && message} + {(message.length > MAX_TOAST_TITLE_LENGTH || + React.isValidElement(message)) && + message} ) ); diff --git a/webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js b/webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js index 94e2a9318d..1230a4ea9d 100644 --- a/webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js +++ b/webpack/assets/javascripts/react_app/components/common/DateTimePicker/DateTimePicker.js @@ -59,7 +59,8 @@ const DateTimePicker = ({ intervalRef.current = null; } updateError(); - intervalRef.current = setInterval(updateError, 30000); // make sure the error is updated every 30 seconds so isFutureOnly is always up to date + const ERROR_UPDATE_INTERVAL = 30000; + intervalRef.current = setInterval(updateError, ERROR_UPDATE_INTERVAL); return () => { if (intervalRef.current) { clearInterval(intervalRef.current); diff --git a/webpack/assets/javascripts/react_app/components/common/EmptyState/ResourceLoadFailedEmptyState.js b/webpack/assets/javascripts/react_app/components/common/EmptyState/ResourceLoadFailedEmptyState.js index d6598dda2a..c6f7434435 100644 --- a/webpack/assets/javascripts/react_app/components/common/EmptyState/ResourceLoadFailedEmptyState.js +++ b/webpack/assets/javascripts/react_app/components/common/EmptyState/ResourceLoadFailedEmptyState.js @@ -5,6 +5,7 @@ import { LockIcon, SearchIcon } from '@patternfly/react-icons'; import EmptyStatePattern from './EmptyStatePattern'; import { resourceLoadFailedEmptyStatePropTypes } from './EmptyStatePropTypes'; import { translate as __, sprintf } from '../../../common/I18n'; +import { HTTP_STATUS_CODES } from '../../../constants'; import { usePermissions } from '../../../common/hooks/Permissions/permissionHooks'; import { useForemanPermissions } from '../../../Root/Context/ForemanContext'; @@ -19,11 +20,13 @@ const resolveFailureReason = ({ viewPermissions, hasViewPermission, }) => { - if (httpStatus === 403) return FAILURE_REASON.FORBIDDEN; + if (httpStatus === HTTP_STATUS_CODES.FORBIDDEN) + return FAILURE_REASON.FORBIDDEN; if (viewPermissions?.length > 0 && !hasViewPermission) { return FAILURE_REASON.FORBIDDEN; } - if (httpStatus === 404) return FAILURE_REASON.NOT_FOUND; + if (httpStatus === HTTP_STATUS_CODES.NOT_FOUND) + return FAILURE_REASON.NOT_FOUND; if (viewPermissions?.length > 0 && hasViewPermission) { return FAILURE_REASON.NOT_FOUND; } diff --git a/webpack/assets/javascripts/react_app/components/common/charts/AreaChart/AreaChartHelpers.js b/webpack/assets/javascripts/react_app/components/common/charts/AreaChart/AreaChartHelpers.js index db8614733f..bee54db02a 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/AreaChart/AreaChartHelpers.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/AreaChart/AreaChartHelpers.js @@ -1,3 +1,9 @@ +import { MS_PER_SECOND } from '../../../../constants'; + +const MS_THRESHOLD = 1e12; +const MIN_Y_STEP = 0.1; +const HALF_STEP = 0.5; + /** Process raw data into chart series. Backend may send Unix seconds or milliseconds. */ export const processChartData = (data, xAxisDataLabel) => { if (!data || data.length === 0) return null; @@ -6,7 +12,7 @@ export const processChartData = (data, xAxisDataLabel) => { const timestamps = timeColumn.slice(1); const toMs = val => { const n = Number(val); - return n >= 1e12 ? n : n * 1000; + return n >= MS_THRESHOLD ? n : n * MS_PER_SECOND; }; const dates = timestamps.map(t => new Date(toMs(t))); const series = data @@ -45,7 +51,7 @@ export const getYTickValues = (chartData, config, hiddenSeries) => { }); }); } - if (maxY <= 0) return [0, 0.5, 1.0]; - const step = Math.max(0.1, Math.ceil((maxY / 4) * 10) / 10); + if (maxY <= 0) return [0, HALF_STEP, 1.0]; + const step = Math.max(MIN_Y_STEP, Math.ceil((maxY / 4) * 10) / 10); return [0, 1, 2, 3, 4, 5].map(i => Math.round(i * step * 10) / 10); }; diff --git a/webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js b/webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js index 9e5e4328f0..97a5657e35 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/BarChart/index.js @@ -45,7 +45,8 @@ const getTooltipValue = datum => { /** Match native JS: use scientific notation for very large magnitudes (avoids float width bugs). */ const formatYAxisTick = t => { const num = Number(t); - if (Math.abs(num) >= 1e21) { + const SCIENTIFIC_NOTATION_THRESHOLD = 1e21; + if (Math.abs(num) >= SCIENTIFIC_NOTATION_THRESHOLD) { return num.toExponential(1); } return num.toFixed(1); @@ -62,7 +63,9 @@ const CHART_PADDING_BASE = { right: 20, top: 10, }; -const DOMAIN_PADDING = { x: [30, 25] }; +const BAR_DOMAIN_PADDING_LEFT = 30; +const DOMAIN_PADDING = { x: [BAR_DOMAIN_PADDING_LEFT, 25] }; +const TOOLTIP_BASE_WIDTH = 100; const TOOLTIP = { width: 140, height: 70, @@ -129,7 +132,7 @@ const BarChart = ({ const maxTooltipString = getTooltipValue({ y: maxY }); const dynamicTooltipWidth = Math.max( TOOLTIP.width, - 100 + maxTooltipString.length * 8 + TOOLTIP_BASE_WIDTH + maxTooltipString.length * 8 ); // Handle click events 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 056ce82416..be7fcb2fac 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 @@ -9,6 +9,7 @@ import { Icon } from '@patternfly/react-core'; import { InfoCircleIcon } from '@patternfly/react-icons'; import { getDonutChartConfig } from '../../../../../services/charts/DonutChartService'; import EmptyState from '../../EmptyState'; +import { PERCENT_MULTIPLIER } from '../../../../constants'; import { translate as __ } from '../../../../../react_app/common/I18n'; import { noop } from '../../../../common/helpers'; @@ -57,7 +58,9 @@ const DonutChart = ({ ); const percentage = total > 0 - ? ((maxItem.y / total) * 100).toFixed(title.precision ?? 1) + ? ((maxItem.y / total) * PERCENT_MULTIPLIER).toFixed( + title.precision ?? 1 + ) : 0; titleText = `${percentage}%`; subtitleText = title.secondary || maxItem.x; diff --git a/webpack/assets/javascripts/react_app/components/common/charts/helpers/LegendHelpers.js b/webpack/assets/javascripts/react_app/components/common/charts/helpers/LegendHelpers.js index 2d6088f563..e6a90e72f7 100644 --- a/webpack/assets/javascripts/react_app/components/common/charts/helpers/LegendHelpers.js +++ b/webpack/assets/javascripts/react_app/components/common/charts/helpers/LegendHelpers.js @@ -2,9 +2,17 @@ import React from 'react'; import { ChartLabel, ChartPoint } from '@patternfly/react-charts'; import { chart_color_black_500 as chartColorBlack500 } from '@patternfly/react-tokens'; import './LegendHelpers.scss'; +import { MS_PER_SECOND } from '../../../../constants'; + +const MS_THRESHOLD = 1e12; +const DIMMED_OPACITY = 0.35; +const SCIENTIFIC_NOTATION_THRESHOLD = 1e21; +const EPOCH_THRESHOLD = 1e9; +const DEFAULT_Y_AXIS_LABEL_OFFSET = -12; +const STAGGER_OFFSET = 14; export const getSeriesOpacity = isDimmedByHover => { - if (isDimmedByHover) return 0.35; + if (isDimmedByHover) return DIMMED_OPACITY; return 1; }; @@ -42,7 +50,8 @@ export const getLegendEvents = (chartName, toggleSeries) => ({ /** Format a timestamp for axis tick display (matches formatAxisTick output). */ const formatTickForDedup = t => { - const date = t instanceof Date ? t : new Date(t >= 1e12 ? t : t * 1000); + const ms = t >= MS_THRESHOLD ? t : t * MS_PER_SECOND; + const date = t instanceof Date ? t : new Date(ms); return date.toLocaleString(undefined, { month: 'short', day: 'numeric', @@ -92,7 +101,7 @@ export const XAxisTickLabel = props => { const { style, index = 0, - yAxisLabelOffset = -12, + yAxisLabelOffset = DEFAULT_Y_AXIS_LABEL_OFFSET, dy: _dyIgnored, text, ...rest @@ -101,7 +110,7 @@ export const XAxisTickLabel = props => { style && typeof style === 'object' && !Array.isArray(style) ? { ...style, verticalAnchor: undefined } : style; - const staggerOffset = index % 2 === 1 ? 14 : 0; + const staggerOffset = index % 2 === 1 ? STAGGER_OFFSET : 0; const axisSpacing = 16; const tooltipText = getTooltipText(text); return ( @@ -124,8 +133,8 @@ export const formatAxisTick = t => { let date; if (t instanceof Date) { date = t; - } else if (typeof t === 'number' && t >= 1e9) { - const ms = t >= 1e12 ? t : t * 1000; + } else if (typeof t === 'number' && t >= EPOCH_THRESHOLD) { + const ms = t >= MS_THRESHOLD ? t : t * MS_PER_SECOND; date = new Date(ms); } else { date = null; @@ -144,7 +153,7 @@ export const formatAxisTick = t => { /** Fixed decimals for normal range; scientific notation for very large magnitudes. */ export const formatYAxisTick = t => { const num = Number(t); - if (Math.abs(num) >= 1e21) { + if (Math.abs(num) >= SCIENTIFIC_NOTATION_THRESHOLD) { return num.toExponential(1); } return num.toFixed(1); @@ -153,7 +162,8 @@ export const formatYAxisTick = t => { export const formatTooltipTitle = datum => { const x = datum?.x ?? datum?._x; if (x == null) return ''; - const date = x instanceof Date ? x : new Date(x >= 1e12 ? x : x * 1000); + const ms = x >= MS_THRESHOLD ? x : x * MS_PER_SECOND; + const date = x instanceof Date ? x : new Date(ms); const dateStr = date.toLocaleDateString(undefined, { month: 'numeric', day: 'numeric', diff --git a/webpack/assets/javascripts/react_app/constants.js b/webpack/assets/javascripts/react_app/constants.js index fe6063bc5d..3b428bcd55 100644 --- a/webpack/assets/javascripts/react_app/constants.js +++ b/webpack/assets/javascripts/react_app/constants.js @@ -1,5 +1,21 @@ import { getManualURL } from './common/helpers'; +export const MS_PER_SECOND = 1000; +export const PERCENT_MULTIPLIER = 100; +export const BYTES_PER_KB = 1024; + +export const HTTP_STATUS_CODES = { + OK: 200, + CREATED: 201, + NO_CONTENT: 204, + BAD_REQUEST: 400, + UNAUTHORIZED: 401, + FORBIDDEN: 403, + NOT_FOUND: 404, + UNPROCESSABLE_ENTITY: 422, + INTERNAL_SERVER_ERROR: 500, +}; + export const STATUS = { PENDING: 'PENDING', RESOLVED: 'RESOLVED', diff --git a/webpack/assets/javascripts/react_app/mockRequests.js b/webpack/assets/javascripts/react_app/mockRequests.js index b7db03ab94..ee2ab3ee01 100644 --- a/webpack/assets/javascripts/react_app/mockRequests.js +++ b/webpack/assets/javascripts/react_app/mockRequests.js @@ -1,5 +1,6 @@ import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; +import { HTTP_STATUS_CODES } from './constants'; export const mock = new MockAdapter(axios); const methods = { @@ -13,7 +14,7 @@ export const mockRequest = ({ method = 'GET', url, data = null, - status = 200, + status = HTTP_STATUS_CODES.OK, response = null, }) => mock[methods[method]](url, data).reply(status, response); diff --git a/webpack/assets/javascripts/react_app/redux/actions/common/forms.js b/webpack/assets/javascripts/react_app/redux/actions/common/forms.js index ea4a47d04f..dd3d8523c4 100644 --- a/webpack/assets/javascripts/react_app/redux/actions/common/forms.js +++ b/webpack/assets/javascripts/react_app/redux/actions/common/forms.js @@ -1,5 +1,6 @@ import { APIActions } from '../../API'; import { sprintf, translate as __ } from '../../../../react_app/common/I18n'; +import { HTTP_STATUS_CODES } from '../../../../react_app/constants'; const getBaseErrors = ({ error: { errors, severity } }) => { let _error; @@ -29,7 +30,7 @@ export const prepareErrors = (errors, base) => export const onError = (error, actions) => { actions.setSubmitting(false); - if (error.response?.status === 422) { + if (error.response?.status === HTTP_STATUS_CODES.UNPROCESSABLE_ENTITY) { const base = getBaseErrors(error?.response?.data); actions.setErrors( diff --git a/webpack/assets/javascripts/react_app/redux/actions/notifications/index.js b/webpack/assets/javascripts/react_app/redux/actions/notifications/index.js index ac1282af4e..596c3160db 100644 --- a/webpack/assets/javascripts/react_app/redux/actions/notifications/index.js +++ b/webpack/assets/javascripts/react_app/redux/actions/notifications/index.js @@ -13,11 +13,12 @@ import { withInterval, } from '../../middlewares/IntervalMiddleware'; import { DEFAULT_INTERVAL } from './constants'; +import { HTTP_STATUS_CODES } from '../../../constants'; const interval = process.env.NOTIFICATIONS_POLLING || DEFAULT_INTERVAL; const handleNotificationPollingError = error => { - if (error.response?.status === 401) { + if (error.response?.status === HTTP_STATUS_CODES.UNAUTHORIZED) { stopNotificationsPolling(); reloadPage(); } diff --git a/webpack/assets/javascripts/react_app/redux/consts.js b/webpack/assets/javascripts/react_app/redux/consts.js index 4fb79555a1..047dd470de 100644 --- a/webpack/assets/javascripts/react_app/redux/consts.js +++ b/webpack/assets/javascripts/react_app/redux/consts.js @@ -34,3 +34,4 @@ export const PASSWORD_STRENGTH_PASSWORD_CHANGED = 'PASSWORD_STRENGTH_PASSWORD_CHANGED'; export const PASSWORD_STRENGTH_PASSWORD_MATCHED = 'PASSWORD_STRENGTH_PASSWORD_MATCHED'; +export const CONTROLLER_KEY_BASE = 1000; diff --git a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js index d9d0ca0646..c10f28ac94 100644 --- a/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js +++ b/webpack/assets/javascripts/react_app/redux/reducers/hosts/storage/vmware.js @@ -23,6 +23,7 @@ import { STORAGE_VMWARE_STORAGEPODS_REQUEST, STORAGE_VMWARE_STORAGEPODS_SUCCESS, STORAGE_VMWARE_STORAGEPODS_FAILURE, + CONTROLLER_KEY_BASE, } from '../../../consts'; const initialState = Immutable({ @@ -43,7 +44,7 @@ const renumberVolumes = volumes => const availableControllerKeys = Array.from( { length: 8 }, - (value, index) => 1000 + index + (value, index) => CONTROLLER_KEY_BASE + index ); const getAvailableKey = controllers => head( diff --git a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js b/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js index 509b7088dd..c8ab3c1353 100644 --- a/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js +++ b/webpack/assets/javascripts/react_app/routes/RegistrationCommands/RegistrationCommandsPage/index.js @@ -20,7 +20,7 @@ import { useForemanOrganization, useForemanLocation, } from '../../../Root/Context/ForemanContext'; -import { STATUS } from '../../../constants'; +import { STATUS, HTTP_STATUS_CODES } from '../../../constants'; import PageLayout from '../../common/PageLayout/PageLayout'; import Slot from '../../../components/common/Slot'; import PermissionDenied from '../../../components/PermissionDenied'; @@ -193,7 +193,7 @@ const RegistrationCommandsPage = () => { }, [dispatch, hostGroupId, operatingSystemId]); // Do not show the form if the user is not authorized to register hosts - if (apiDataResponseCode === 403) { + if (apiDataResponseCode === HTTP_STATUS_CODES.FORBIDDEN) { return ; } diff --git a/webpack/assets/javascripts/services/charts/AreaChartService.js b/webpack/assets/javascripts/services/charts/AreaChartService.js index 6efa0e0971..c75aafe182 100644 --- a/webpack/assets/javascripts/services/charts/AreaChartService.js +++ b/webpack/assets/javascripts/services/charts/AreaChartService.js @@ -1,5 +1,6 @@ import uuidV1 from 'uuid/v1'; import { getChartConfig } from './ChartService'; +import { MS_PER_SECOND } from '../../react_app/constants'; export const getAreaChartConfig = ({ data, @@ -29,7 +30,7 @@ export const getAreaChartConfig = ({ if (data) { const timestamps = data[0].slice(1); const formatedDates = timestamps.map( - epochSecs => new Date(epochSecs * 1000) + epochSecs => new Date(epochSecs * MS_PER_SECOND) ); chartConfig.data.colors = {}; chartConfig.data.columns[0] = [xAxisDataLabel].concat(formatedDates); diff --git a/webpack/assets/javascripts/services/charts/ChartService.js b/webpack/assets/javascripts/services/charts/ChartService.js index 72eff7f303..0c017f65f5 100644 --- a/webpack/assets/javascripts/services/charts/ChartService.js +++ b/webpack/assets/javascripts/services/charts/ChartService.js @@ -11,6 +11,7 @@ import { timeseriesLineChartConfig, timeseriesAreaChartConfig, } from './ChartService.consts'; +import { PERCENT_MULTIPLIER } from '../../react_app/constants'; const chartsSizeConfig = { area: { @@ -70,7 +71,12 @@ export const getChartConfig = ({ dataWithShortNames = data.map(val => { const item = Immutable.asMutable(val.slice()); const longName = item[0]; - item[0] = item[0].length > 30 ? `${val[0].substring(0, 10)}...` : item[0]; + const MAX_LABEL_LENGTH = 30; + const TRUNCATED_LABEL_LENGTH = 10; + item[0] = + item[0].length > MAX_LABEL_LENGTH + ? `${val[0].substring(0, TRUNCATED_LABEL_LENGTH)}...` + : item[0]; longNames[item[0]] = longName; return item; }); @@ -214,7 +220,11 @@ export const getDonutChartConfigPF5 = ({ // 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; + total > 0 + ? ((datum.y / total) * PERCENT_MULTIPLIER).toFixed( + title?.precision ?? 1 + ) + : 0; return `${datum.name}: ${percentage}%`; };