Skip to content

Commit

Permalink
Merge branch 'main' into feature/chrome-service-support-custom-menu-h…
Browse files Browse the repository at this point in the history
…eader

Signed-off-by: Manasvini B Suryanarayana <[email protected]>
  • Loading branch information
manasvinibs authored Dec 18, 2023
2 parents e973515 + d8cbc17 commit e2fc4d6
Show file tree
Hide file tree
Showing 24 changed files with 5,360 additions and 708 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,22 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [Decouple] Add new cross compatibility check core service which export functionality for plugins to verify if their OpenSearch plugin counterpart is installed on the cluster or has incompatible version to configure the plugin behavior([#4710](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4710))
- [Discover] Display inner properties in the left navigation bar [#5429](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5429)
- [Chrome] Introduce registerCollapsibleNavHeader to allow plugins to customize the rendering of nav menu header ([#5244](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5244))
- [Custom Branding] Relative URL should be allowed for logos ([#5572](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5572))

### 🐛 Bug Fixes

- [Chore] Update deprecated url methods (url.parse(), url.format()) ([#2910](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/2910))
- Cleanup unused url ([#3847](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3847))
- [TSVB, Dashboards] Fix inconsistent dark mode code editor themes ([#4609](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4609))
- Fix `maps.proxyOpenSearchMapsServiceInMaps` config definition so it can be set ([#5170](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5170))
- [Discover] Fix inactive state on 'Discover' tab in side navigation menu ([#5432](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5432))
- [BUG] Add platform "darwin-arm64" to unit test ([#5290](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5290))
- [BUG][Dev Tool] Add dev tool documentation link to dev tool's help menu [#5166](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5166)
- Fix missing border for header navigation control on right ([#5450](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5450))
- [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

Expand Down Expand Up @@ -70,6 +74,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)

### 🔩 Tests

- [Home] Add more unit tests for other complications of overview ([#5418](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5418))

## [2.11.1 - 2023-11-21](https://github.com/opensearch-project/OpenSearch-Dashboards/releases/tag/2.11.1)

### 🛡 Security
Expand Down Expand Up @@ -511,6 +517,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- [VisBuilder] Fix Firefox legend selection issue ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [VisBuilder] Fix type errors ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732))
- [VisBuilder] Fix indexpattern selection in filter bar ([#3751](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3751))
- [Console] Fix dev tool console autocomplete not loading issue for aliases ([#5568](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/5568))

### 🚞 Infrastructure

Expand Down
60 changes: 60 additions & 0 deletions packages/osd-stylelint-plugin-stylelint/src/utils/README.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 16 additions & 0 deletions packages/osd-stylelint-plugin-stylelint/src/utils/get_message.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions packages/osd-stylelint-plugin-stylelint/src/utils/matches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
63 changes: 63 additions & 0 deletions src/core/public/chrome/ui/header/nav_link.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Copyright OpenSearch Contributors
* SPDX-License-Identifier: Apache-2.0
*/

import { isActiveNavLink, createEuiListItem } from './nav_link';
import { ChromeNavLink } from '../../..';
import { httpServiceMock } from '../../../http/http_service.mock';

describe('isActiveNavLink', () => {
it('should return true if the appId is "discover" and linkId is "discover"', () => {
expect(isActiveNavLink('discover', 'discover')).toBe(true);
});

it('should return true if the appId is "data-explorer" and linkId is "data-explorer"', () => {
expect(isActiveNavLink('data-explorer', 'data-explorer')).toBe(true);
});

it('should return true if the appId is "data-explorer" and linkId is "discover"', () => {
expect(isActiveNavLink('data-explorer', 'discover')).toBe(true);
});

it('should return false if the appId and linkId do not match', () => {
expect(isActiveNavLink('dashboard', 'discover')).toBe(false);
});
});

const mockBasePath = httpServiceMock.createSetupContract({ basePath: '/test' }).basePath;

describe('createEuiListItem', () => {
const mockLink: Partial<ChromeNavLink> = {
href: 'test',
id: 'test',
title: 'Test App',
disabled: false,
euiIconType: 'inputOutput',
icon: 'testIcon',
tooltip: 'Test App Tooltip',
};

const mockProps = {
link: mockLink as ChromeNavLink,
appId: 'test',
basePath: mockBasePath,
dataTestSubj: 'test-subj',
onClick: jest.fn(),
navigateToApp: jest.fn(),
externalLink: false,
};

it('creates a list item with the correct properties', () => {
const listItem = createEuiListItem(mockProps);
expect(listItem).toHaveProperty('label', mockProps.link.tooltip);
expect(listItem).toHaveProperty('href', mockProps.link.href);
expect(listItem).toHaveProperty('data-test-subj', mockProps.dataTestSubj);
expect(listItem).toHaveProperty('onClick');
expect(listItem).toHaveProperty(
'isActive',
isActiveNavLink(mockProps.appId, mockProps.link.id)
);
expect(listItem).toHaveProperty('isDisabled', mockProps.link.disabled);
});
});
10 changes: 9 additions & 1 deletion src/core/public/chrome/ui/header/nav_link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ import { relativeToAbsolute } from '../../nav_links/to_nav_link';
export const isModifiedOrPrevented = (event: React.MouseEvent<HTMLButtonElement, MouseEvent>) =>
event.metaKey || event.altKey || event.ctrlKey || event.shiftKey || event.defaultPrevented;

// TODO: replace hard-coded values with a registration function, so that apps can control active nav links similar to breadcrumbs
const aliasedApps: { [key: string]: string[] } = {
discover: ['data-explorer'],
};

export const isActiveNavLink = (appId: string | undefined, linkId: string): boolean =>
!!(appId === linkId || aliasedApps[linkId]?.includes(appId || ''));

interface Props {
link: ChromeNavLink;
appId?: string;
Expand Down Expand Up @@ -82,7 +90,7 @@ export function createEuiListItem({
navigateToApp(id);
}
},
isActive: appId === id,
isActive: isActiveNavLink(appId, id),
isDisabled: disabled,
'data-test-subj': dataTestSubj,
...(basePath && {
Expand Down
5 changes: 5 additions & 0 deletions src/core/server/rendering/rendering_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ describe('RenderingService', () => {
const result = await service.isUrlValid('/', 'config');
expect(result).toEqual(false);
});

it('checks relative URL returns true', async () => {
const result = await service.isUrlValid('/demo/opensearch_mark_default.png', 'config');
expect(result).toEqual(true);
});
});

describe('isTitleValid()', () => {
Expand Down
3 changes: 3 additions & 0 deletions src/core/server/rendering/rendering_service.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,9 @@ export class RenderingService {
this.logger.get('branding').error(`${configName} config is invalid. Using default branding.`);
return false;
}
if (url.startsWith('/')) {
return true;
}
return await Axios.get(url, {
httpsAgent: this.httpsAgent,
adapter: 'http',
Expand Down
Loading

0 comments on commit e2fc4d6

Please sign in to comment.