Merge branch 'main' into fix-5268-2

Signed-off-by: Anan Zhuang <[email protected]>

CHANGELOG.md

@@ -36,7 +36,8 @@ 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)
-- [Discover] Fix advanced setting `discover:modifyColumnsOnSwitch` ([#5508](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5508))
+- [BUG][Discover] Fix what is displayed in `selected fields` when removing columns from canvas [#5537](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5537)
+- [BUG][Discover] Fix advanced setting `discover:modifyColumnsOnSwitch` ([#5508](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5508))
 
 ### 🚞 Infrastructure
 

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<string, any>): 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.

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 }

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;

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}`;

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<string, { approved?: string[]; explanation?: string }>;
+
+/**
+ * configuration based on files.
+ */
 export type ValueBasedConfig = Record<
   string,
   Record<string, Array<{ approved?: string; rejected?: string[] }>>
 >;
 
+/**
+ * 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)) {

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;

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,

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) {

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) => {