From 0a33d4aed571120f34e7608a171e144df159d011 Mon Sep 17 00:00:00 2001 From: Qiwen Li Date: Thu, 14 Dec 2023 18:10:59 -0500 Subject: [PATCH 1/2] [OSCI][Fix][Discover]Prevent Adding Timefield to the Side Nav Selected Fields on Column Removal (#5537) * added in a filter for columns before rendering the panel, remove timeField if it was not previously chosen. * fix issue 5538 too --------- Signed-off-by: qiwen li Signed-off-by: Qiwen Li --- CHANGELOG.md | 1 + .../view_components/panel/index.tsx | 24 ++++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 87aa8c065eac..606355e84589 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [BUG] Fix filtering issue in data source selector ([5484](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5484)) - [BUG][Data] Support for custom filters with heterogeneous data fields ([5577](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5577)) - [BUG][Data] Fix empty suggestion history when querying in search bar [#5349](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5349) +- [BUG][Discover] Fix what is displayed in `selected fields` when removing columns from canvas [#5537](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5537) ### 🚞 Infrastructure diff --git a/src/plugins/discover/public/application/view_components/panel/index.tsx b/src/plugins/discover/public/application/view_components/panel/index.tsx index bbf5331fb16e..6b4cd2a87c91 100644 --- a/src/plugins/discover/public/application/view_components/panel/index.tsx +++ b/src/plugins/discover/public/application/view_components/panel/index.tsx @@ -3,12 +3,13 @@ * SPDX-License-Identifier: Apache-2.0 */ -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useCallback, useEffect, useRef, useState } from 'react'; import { ViewProps } from '../../../../../data_explorer/public'; import { addColumn, removeColumn, reorderColumn, + setColumns, useDispatch, useSelector, } from '../../utils/state_management'; @@ -19,6 +20,7 @@ import { IndexPatternField, opensearchFilters } from '../../../../../data/public import { useOpenSearchDashboards } from '../../../../../opensearch_dashboards_react/public'; import { DiscoverViewServices } from '../../../build_services'; import { popularizeField } from '../../helpers/popularize_field'; +import { buildColumns } from '../../utils/columns'; // eslint-disable-next-line import/no-default-export export default function DiscoverPanel(props: ViewProps) { @@ -36,7 +38,27 @@ export default function DiscoverPanel(props: ViewProps) { const { columns } = useSelector((state) => ({ columns: state.discover.columns, })); + + const prevColumns = useRef(columns); const dispatch = useDispatch(); + useEffect(() => { + const timeFieldname = indexPattern?.timeFieldName; + + if (columns !== prevColumns.current) { + let updatedColumns = buildColumns(columns); + if ( + timeFieldname && + !prevColumns.current.includes(timeFieldname) && + columns.includes(timeFieldname) + ) { + // Remove timeFieldname from columns if previously chosen columns does not include time field + updatedColumns = columns.filter((column) => column !== timeFieldname); + } + // Update the ref with the new columns + dispatch(setColumns({ columns: updatedColumns })); + prevColumns.current = columns; + } + }, [columns, dispatch, indexPattern?.timeFieldName]); useEffect(() => { const subscription = data$.subscribe((next) => { From 4d57b8af9e3aec6f8c852dda173234a687a3d167 Mon Sep 17 00:00:00 2001 From: tanupa Date: Thu, 14 Dec 2023 15:12:02 -0800 Subject: [PATCH 2/2] adding documentation on files in util in addition to a README.md with an overview of each file and its exported functions (#5540) Signed-off-by: tanupa Co-authored-by: Josh Romero --- .../src/utils/README.md | 60 +++++++++++++++++++ .../src/utils/compliance_engine.ts | 18 ++++++ .../src/utils/extract_regex.ts | 5 ++ .../src/utils/get_message.ts | 16 +++++ .../src/utils/get_rules_from_config.ts | 18 ++++++ .../src/utils/is_color_property.ts | 11 ++++ .../src/utils/is_valid_options.ts | 8 +++ .../src/utils/matches.ts | 7 +++ 8 files changed, 143 insertions(+) create mode 100644 packages/osd-stylelint-plugin-stylelint/src/utils/README.md diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/README.md b/packages/osd-stylelint-plugin-stylelint/src/utils/README.md new file mode 100644 index 000000000000..cbbe66f9288b --- /dev/null +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/README.md @@ -0,0 +1,60 @@ +# OpenSearch Dashboards-related util files + +Below is an overview of each file: + +## `compliance_engine.ts` +- **Purpose:** + - Implements a compliance engine for tracking and validating rules. + - Provides functions to check if a node is tracked, retrieve compliance rules, and more. +- **Exported Functions:** + - `isTracked(rules: ValueBasedConfig, nodeInfo: { selector: string; prop: string }): boolean` + - `getComplianceRule(rules: ValueBasedConfig, nodeInfo: { selector: string; prop: string; value: string }): ComplianceRule | undefined` + + +## `extract_regex.ts` +- **Purpose:** + - Defines a function to extract a regular expression from a string value. +- **Exported Function:** + - `extractRegex(value: string): RegExp | undefined` + + +## `get_message.ts` +- **Purpose:** + - Contains functions to generate messages related to compliance tracking. +- **Exported Functions:** + - `getUntrackedMessage(nodeInfo: { selector: string; prop: string; value: string }): string` + - `getTrackedMessage(nodeInfo: { selector: string; prop: string; value: string }): string` + - `getNotCompliantMessage(message: string, explanation?: string): string` + + +## `get_rules_from_config.ts` +- **Purpose:** + - Provides functions to retrieve rules from a configuration file. +- **Exported Functions:** + - `getRulesFromConfig(configPath: string): ValueBasedConfig` + - `getRuleFromConfig(rules: FileBasedConfig, value: string): { approved?: string[]; explanation?: string } | undefined` + + +## `is_color_property.ts` +- **Purpose:** + - Checks if a given CSS property potentially modifies a color. + - Provides a function to get the parent rule of a declaration if the declaration is a color property. +- **Exported Functions:** + - `isColorProperty(prop: string): boolean` + - `getColorPropertyParent(decl: Declaration): Rule | undefined` + + +## `is_valid_options.ts` +- **Purpose:** + - Validates options for a Stylelint rule. +- **Exported Function:** + - `isValidOptions(postcssResult: any, ruleName: string, primaryOption: Record): boolean` + + +## `matches.ts` +- **Purpose:** + - Checks if a given value matches a specified pattern. +- **Exported Function:** + - `matches(matcher: string, value: string): boolean` + +This project follows the Apache-2.0 license. Contributions must be made under this license or a compatible open-source license. diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/compliance_engine.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/compliance_engine.ts index 7b95e94e536c..edee4cd63288 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/compliance_engine.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/compliance_engine.ts @@ -18,6 +18,12 @@ export interface ComplianceRule { message: string; } +/** + * Retrieves a specific rule from the provided rules based on the node information. + * @param rules - The ValueBasedConfig containing rules. + * @param nodeInfo - The information about the node (selector and prop). + * @returns The specific rule if found, otherwise undefined. + */ const getRule = (rules: ValueBasedConfig, nodeInfo: { selector: string; prop: string }) => { const rule = rules[nodeInfo.prop]; if (!rule) { @@ -40,10 +46,22 @@ const getRule = (rules: ValueBasedConfig, nodeInfo: { selector: string; prop: st return rule[ruleKey]; }; +/** + * Checks if the specified node is tracked based on the provided rules. + * @param rules - The ValueBasedConfig containing rules. + * @param nodeInfo - The information about the node (selector and prop). + * @returns True if the node is tracked, otherwise false. + */ const isTracked = (rules: ValueBasedConfig, nodeInfo: { selector: string; prop: string }) => { return getRule(rules, nodeInfo) !== undefined; }; +/** + * Retrieves the compliance rule for the specified node and value. + * @param rules - The ValueBasedConfig containing rules. + * @param nodeInfo - The information about the node (selector, prop, and value). + * @returns The ComplianceRule object if a rule is found, otherwise undefined. + */ const getComplianceRule = ( rules: ValueBasedConfig, nodeInfo: { selector: string; prop: string; value: string } diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/extract_regex.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/extract_regex.ts index 937b486ad940..049b541879df 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/extract_regex.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/extract_regex.ts @@ -3,6 +3,11 @@ * SPDX-License-Identifier: Apache-2.0 */ +/** + * Extracts a regular expression from the provided string value. + * @param value - The string value to extract the regular expression from. + * @returns The extracted regular expression if the value is a valid regex, otherwise undefined. + */ export const extractRegex = (value: string) => { if (!value.startsWith('/')) { return undefined; diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts index b966f775e4d6..4a34793806df 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts @@ -9,12 +9,28 @@ * GitHub history for details. */ +/** + * Generates a message for untracked node information. + * @param nodeInfo - Information about the node (selector, prop, value). + * @returns The untracked message. + */ export const getUntrackedMessage = (nodeInfo: { selector: string; prop: string; value: string }) => `Untracked: "${nodeInfo.selector}.${nodeInfo.prop}: ${nodeInfo.value}"`; +/** + * Generates a message for tracked node information missing approval. + * @param nodeInfo - Information about the node (selector, prop, value). + * @returns The tracked but missing approval message. + */ export const getTrackedMessage = (nodeInfo: { selector: string; prop: string; value: string }) => `Tracked but missing approval: "${nodeInfo.selector}.${nodeInfo.prop}: ${nodeInfo.value}"`; +/** + * Generates a not compliant message with an optional explanation. + * @param message - The base not compliant message. + * @param explanation - Optional explanation for the not compliant message. + * @returns The not compliant message with or without an explanation. + */ export const getNotCompliantMessage = (message: string, explanation?: string) => { if (explanation) { return `${message} ${explanation}`; diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts index 5467915cbabb..9b51ed310d9b 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/get_rules_from_config.ts @@ -13,17 +13,35 @@ import path from 'path'; import { readFileSync } from 'fs'; import { matches } from './matches'; +/** + * configuration based on files. + */ export type FileBasedConfig = Record; + +/** + * configuration based on files. + */ export type ValueBasedConfig = Record< string, Record> >; +/** + * Retrieves rules from a configuration file. + * @param configPath - The path to the configuration file. + * @returns Parsed rules from the configuration file. + */ export const getRulesFromConfig = (configPath: string) => { const filePath = path.resolve(__dirname, configPath); return JSON.parse(readFileSync(filePath, 'utf-8')); }; +/** + * Retrieves a rule from a file-based configuration. + * @param rules - The file-based configuration rules. + * @param value - The value to match against the configuration rules. + * @returns The rule corresponding to the matched value, or undefined if no match is found. + */ export const getRuleFromConfig = (rules: FileBasedConfig, value: string) => { for (const key of Object.keys(rules)) { if (matches(key, value)) { diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts index ef7ad9118f81..7b760ea977d7 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/is_color_property.ts @@ -57,10 +57,21 @@ const COLOR_PROPERTIES = [ * each one, therefore this is to optimize the linter to * skip any property that does not impact colors. */ + +/** + * Checks if a given CSS property potentially modifies a color. + * @param prop - The CSS property to check. + * @returns A boolean indicating whether the property can impact colors. + */ export const isColorProperty = (prop: string) => { return COLOR_PROPERTIES.includes(prop); }; +/** + * Gets the parent rule of a declaration if the declaration is a color property. + * @param decl - The CSS declaration. + * @returns The parent rule if the declaration is a color property, otherwise undefined. + */ export const getColorPropertyParent = (decl: Declaration) => { if (!isColorProperty(decl.prop)) { return undefined; diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/is_valid_options.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/is_valid_options.ts index 15fae8862eaa..dd555667c273 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/is_valid_options.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/is_valid_options.ts @@ -13,6 +13,14 @@ import stylelint from 'stylelint'; const { validateOptions } = stylelint.utils; +/** + * Validates options for a Stylelint rule. + * + * @param postcssResult - The PostCSS result object. + * @param ruleName - The name of the Stylelint rule. + * @param primaryOption - The primary option for the rule. + * @returns {boolean} - A boolean indicating whether the options are valid. + */ export const isValidOptions = ( postcssResult: any, ruleName: string, diff --git a/packages/osd-stylelint-plugin-stylelint/src/utils/matches.ts b/packages/osd-stylelint-plugin-stylelint/src/utils/matches.ts index 02cc327615bd..fcf6f661bb5d 100644 --- a/packages/osd-stylelint-plugin-stylelint/src/utils/matches.ts +++ b/packages/osd-stylelint-plugin-stylelint/src/utils/matches.ts @@ -5,6 +5,13 @@ import { extractRegex } from './extract_regex'; +/** + * Checks if a given value matches a specified pattern. + * + * @param matcher - The pattern to match against (can be a string or regex). + * @param value - The value to check for a match. + * @returns {boolean} - A boolean indicating whether the value matches the pattern. + */ export const matches = (matcher: string, value: string) => { const regex = extractRegex(matcher); if (!regex) {