From ffe455615eef10d1719884fd131f6953976583ff Mon Sep 17 00:00:00 2001 From: Alexei Karikov Date: Mon, 17 Apr 2023 23:55:11 +0600 Subject: [PATCH 1/7] [CCI] Replace jquery usage in console plugin with native methods (#3733) * Remove jquery import and unused mock test * Removed jquery imports and replaced jquery functions and methods to native js in console plugin tests * Removed jquery imports and replaced jquery functions to native js in console plugin * Adding a changelog entry * Accept changes from new mappings * Update to template string Co-authored-by: Josh Romero Signed-off-by: Alexei Karikov --------- Signed-off-by: Alexei Karikov Co-authored-by: Josh Romero --- CHANGELOG.md | 1 + .../__tests__/input.test.js | 5 ++- .../__tests__/output_tokenization.test.js | 5 ++- .../legacy_core_editor/legacy_core_editor.ts | 31 ++++++++----------- .../__tests__/integration.test.js | 4 +-- .../__tests__/sense_editor.test.js | 5 ++- .../sense_editor/sense_editor.test.mocks.ts | 8 ----- .../ace_token_provider/token_provider.test.ts | 6 ++-- src/plugins/console/public/lib/osd/osd.js | 20 ++++++------ 9 files changed, 33 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 062c3205f947..60e36f05e775 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -183,6 +183,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Console] Replace jQuery.ajax with core.http when calling OSD APIs in console ([#3080](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3080)) - [I18n] Fix Listr type errors and error handlers ([#3629](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3629)) - [Multiple DataSource] Present the authentication type choices in a drop-down ([#3693](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3693)) +- [Console] Replace jQuery usage in console plugin with native methods ([#3733](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3733)) ### 🔩 Tests diff --git a/src/plugins/console/public/application/models/legacy_core_editor/__tests__/input.test.js b/src/plugins/console/public/application/models/legacy_core_editor/__tests__/input.test.js index 7accc948e6cd..653e34aa0073 100644 --- a/src/plugins/console/public/application/models/legacy_core_editor/__tests__/input.test.js +++ b/src/plugins/console/public/application/models/legacy_core_editor/__tests__/input.test.js @@ -31,7 +31,6 @@ import '../legacy_core_editor.test.mocks'; import RowParser from '../../../../lib/row_parser'; import { createTokenIterator } from '../../../factories'; -import $ from 'jquery'; import { create } from '../create'; describe('Input', () => { @@ -46,10 +45,10 @@ describe('Input', () => { coreEditor = create(document.querySelector('#ConAppEditor')); - $(coreEditor.getContainer()).show(); + coreEditor.getContainer().style.display = ''; }); afterEach(() => { - $(coreEditor.getContainer()).hide(); + coreEditor.getContainer().style.display = 'none'; }); describe('.getLineCount', () => { diff --git a/src/plugins/console/public/application/models/legacy_core_editor/__tests__/output_tokenization.test.js b/src/plugins/console/public/application/models/legacy_core_editor/__tests__/output_tokenization.test.js index 4973011a2aaa..d143d72e15c2 100644 --- a/src/plugins/console/public/application/models/legacy_core_editor/__tests__/output_tokenization.test.js +++ b/src/plugins/console/public/application/models/legacy_core_editor/__tests__/output_tokenization.test.js @@ -29,7 +29,6 @@ */ import '../legacy_core_editor.test.mocks'; -import $ from 'jquery'; import RowParser from '../../../../lib/row_parser'; import ace from 'brace'; import { createReadOnlyAceEditor } from '../create_readonly'; @@ -39,11 +38,11 @@ const tokenIterator = ace.acequire('ace/token_iterator'); describe('Output Tokenization', () => { beforeEach(() => { output = createReadOnlyAceEditor(document.querySelector('#ConAppOutput')); - $(output.container).show(); + output.container.style.display = ''; }); afterEach(() => { - $(output.container).hide(); + output.container.style.display = 'none'; }); function tokensAsList() { diff --git a/src/plugins/console/public/application/models/legacy_core_editor/legacy_core_editor.ts b/src/plugins/console/public/application/models/legacy_core_editor/legacy_core_editor.ts index 55ee5fe2a343..5fe93ca4e094 100644 --- a/src/plugins/console/public/application/models/legacy_core_editor/legacy_core_editor.ts +++ b/src/plugins/console/public/application/models/legacy_core_editor/legacy_core_editor.ts @@ -30,7 +30,6 @@ import ace from 'brace'; import { Editor as IAceEditor, IEditSession as IAceEditSession } from 'brace'; -import $ from 'jquery'; import { CoreEditor, Position, @@ -54,11 +53,11 @@ const rangeToAceRange = ({ start, end }: Range) => export class LegacyCoreEditor implements CoreEditor { private _aceOnPaste: any; - $actions: any; + actions: any; resize: () => void; constructor(private readonly editor: IAceEditor, actions: HTMLElement) { - this.$actions = $(actions); + this.actions = actions; this.editor.setShowPrintMargin(false); const session = this.editor.getSession(); @@ -274,20 +273,16 @@ export class LegacyCoreEditor implements CoreEditor { private setActionsBar = (value?: any, topOrBottom: 'top' | 'bottom' = 'top') => { if (value === null) { - this.$actions.css('visibility', 'hidden'); + this.actions.style.visibility = 'hidden'; } else { if (topOrBottom === 'top') { - this.$actions.css({ - bottom: 'auto', - top: value, - visibility: 'visible', - }); + this.actions.style.bottom = 'auto'; + this.actions.style.top = value; + this.actions.style.visibility = 'visible'; } else { - this.$actions.css({ - top: 'auto', - bottom: value, - visibility: 'visible', - }); + this.actions.style.top = 'auto'; + this.actions.style.bottom = value; + this.actions.style.visibility = 'visible'; } } }; @@ -318,14 +313,14 @@ export class LegacyCoreEditor implements CoreEditor { } legacyUpdateUI(range: any) { - if (!this.$actions) { + if (!this.actions) { return; } if (range) { // elements are positioned relative to the editor's container // pageY is relative to page, so subtract the offset // from pageY to get the new top value - const offsetFromPage = $(this.editor.container).offset()!.top; + const offsetFromPage = this.editor.container.offsetTop; const startLine = range.start.lineNumber; const startColumn = range.start.column; const firstLine = this.getLineValue(startLine); @@ -345,11 +340,11 @@ export class LegacyCoreEditor implements CoreEditor { let offset = 0; if (isWrapping) { // Try get the line height of the text area in pixels. - const textArea = $(this.editor.container.querySelector('textArea')!); + const textArea = this.editor.container.querySelector('textArea'); const hasRoomOnNextLine = this.getLineValue(startLine).length < maxLineLength; if (textArea && hasRoomOnNextLine) { // Line height + the number of wraps we have on a line. - offset += this.getLineValue(startLine).length * textArea.height()!; + offset += this.getLineValue(startLine).length * textArea.getBoundingClientRect().height; } else { if (startLine > 1) { this.setActionsBar(getScreenCoords(startLine - 1)); diff --git a/src/plugins/console/public/application/models/sense_editor/__tests__/integration.test.js b/src/plugins/console/public/application/models/sense_editor/__tests__/integration.test.js index cf6df4d31b06..88f9acc27e7f 100644 --- a/src/plugins/console/public/application/models/sense_editor/__tests__/integration.test.js +++ b/src/plugins/console/public/application/models/sense_editor/__tests__/integration.test.js @@ -44,11 +44,11 @@ describe('Integration', () => { '
'; senseEditor = create(document.querySelector('#ConAppEditor')); - $(senseEditor.getCoreEditor().getContainer()).show(); + senseEditor.getCoreEditor().getContainer().style.display = ''; senseEditor.autocomplete._test.removeChangeListener(); }); afterEach(() => { - $(senseEditor.getCoreEditor().getContainer()).hide(); + senseEditor.getCoreEditor().getContainer().style.display = 'none'; senseEditor.autocomplete._test.addChangeListener(); }); diff --git a/src/plugins/console/public/application/models/sense_editor/__tests__/sense_editor.test.js b/src/plugins/console/public/application/models/sense_editor/__tests__/sense_editor.test.js index 18d798c28c94..de67ae1a8908 100644 --- a/src/plugins/console/public/application/models/sense_editor/__tests__/sense_editor.test.js +++ b/src/plugins/console/public/application/models/sense_editor/__tests__/sense_editor.test.js @@ -30,7 +30,6 @@ import '../sense_editor.test.mocks'; -import $ from 'jquery'; import _ from 'lodash'; import { create } from '../create'; @@ -51,11 +50,11 @@ describe('Editor', () => {
`; input = create(document.querySelector('#ConAppEditor')); - $(input.getCoreEditor().getContainer()).show(); + input.getCoreEditor().getContainer().style.display = ''; input.autocomplete._test.removeChangeListener(); }); afterEach(function () { - $(input.getCoreEditor().getContainer()).hide(); + input.getCoreEditor().getContainer().style.display = 'none'; input.autocomplete._test.addChangeListener(); }); diff --git a/src/plugins/console/public/application/models/sense_editor/sense_editor.test.mocks.ts b/src/plugins/console/public/application/models/sense_editor/sense_editor.test.mocks.ts index 6474fcb0ec9d..92e86d60104c 100644 --- a/src/plugins/console/public/application/models/sense_editor/sense_editor.test.mocks.ts +++ b/src/plugins/console/public/application/models/sense_editor/sense_editor.test.mocks.ts @@ -31,11 +31,3 @@ /* eslint no-undef: 0 */ import '../legacy_core_editor/legacy_core_editor.test.mocks'; - -import jQuery from 'jquery'; -jest.spyOn(jQuery, 'ajax').mockImplementation( - () => - new Promise(() => { - // never resolve - }) as any -); diff --git a/src/plugins/console/public/lib/ace_token_provider/token_provider.test.ts b/src/plugins/console/public/lib/ace_token_provider/token_provider.test.ts index 55c18381cb9c..a933fdeb8010 100644 --- a/src/plugins/console/public/lib/ace_token_provider/token_provider.test.ts +++ b/src/plugins/console/public/lib/ace_token_provider/token_provider.test.ts @@ -30,8 +30,6 @@ import '../../application/models/sense_editor/sense_editor.test.mocks'; -import $ from 'jquery'; - // TODO: // We import from application models as a convenient way to bootstrap loading up of an editor using // this lib. We also need to import application specific mocks which is not ideal. @@ -59,14 +57,14 @@ describe('Ace (legacy) token provider', () => { senseEditor = create(document.querySelector('#ConAppEditor')!); - $(senseEditor.getCoreEditor().getContainer())!.show(); + senseEditor.getCoreEditor().getContainer().style.display = ''; (senseEditor as any).autocomplete._test.removeChangeListener(); tokenProvider = senseEditor.getCoreEditor().getTokenProvider(); }); afterEach(async () => { - $(senseEditor.getCoreEditor().getContainer())!.hide(); + senseEditor.getCoreEditor().getContainer().style.display = 'none'; (senseEditor as any).autocomplete._test.addChangeListener(); await senseEditor.update('', true); }); diff --git a/src/plugins/console/public/lib/osd/osd.js b/src/plugins/console/public/lib/osd/osd.js index 529fba754a93..cf8126271072 100644 --- a/src/plugins/console/public/lib/osd/osd.js +++ b/src/plugins/console/public/lib/osd/osd.js @@ -38,7 +38,6 @@ import { UsernameAutocompleteComponent, } from '../autocomplete/components'; -import $ from 'jquery'; import _ from 'lodash'; import Api from './api'; @@ -174,20 +173,19 @@ function loadApisFromJson( // like this, it looks like a minor security issue. export function setActiveApi(api) { if (!api) { - $.ajax({ - url: '../api/console/api_server', - dataType: 'json', // disable automatic guessing + fetch('../api/console/api_server', { + method: 'GET', headers: { 'osd-xsrf': 'opensearch-dashboards', }, - }).then( - function (data) { + }) + .then(function (response) { + response.json(); + }) + .then(function (data) { setActiveApi(loadApisFromJson(data)); - }, - function (jqXHR) { - console.log("failed to load API '" + api + "': " + jqXHR.responseText); - } - ); + }) + .catch((error) => console.log(`failed to load API '${api}': ${error}`)); return; } From 677dcbd039b39a662928ea6ab62a3305c97ac24e Mon Sep 17 00:00:00 2001 From: Aigerim Suleimenova Date: Tue, 18 Apr 2023 01:25:16 +0600 Subject: [PATCH 2/7] resolved get_keystore unit test (#3854) --- CHANGELOG.md | 1 + src/cli_keystore/get_keystore.test.js | 14 ++++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60e36f05e775..03bdc5998392 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -197,6 +197,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Vis Builder] Adds field unit tests ([#3211](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3211)) - [BWC Tests] Add BWC tests for 2.6.0 ([#3356](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3356)) - Prevent primitive linting limitations from being applied to unit tests found under `src/setup_node_env` ([#3403](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3403)) +- [Tests] Fix unit tests for `get_keystore` ([#3854](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3854)) - [Tests] Use `scripts/use_node` instead of `node` in functional test plugins ([#3783](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3783)) ## [2.x] diff --git a/src/cli_keystore/get_keystore.test.js b/src/cli_keystore/get_keystore.test.js index ec838a6b956f..2e05657e4ae8 100644 --- a/src/cli_keystore/get_keystore.test.js +++ b/src/cli_keystore/get_keystore.test.js @@ -32,6 +32,8 @@ import { getKeystore } from './get_keystore'; import { Logger } from '../cli_plugin/lib/logger'; import fs from 'fs'; import sinon from 'sinon'; +import { getConfigDirectory, getDataPath } from '@osd/utils'; +import { join } from 'path'; describe('get_keystore', () => { const sandbox = sinon.createSandbox(); @@ -45,15 +47,19 @@ describe('get_keystore', () => { }); it('uses the config directory if there is no pre-existing keystore', () => { + const configKeystore = join(getConfigDirectory(), 'opensearch_dashboards.keystore'); + const dataKeystore = join(getDataPath(), 'opensearch_dashboards.keystore'); sandbox.stub(fs, 'existsSync').returns(false); - expect(getKeystore()).toContain('config'); - expect(getKeystore()).not.toContain('data'); + expect(getKeystore()).toContain(configKeystore); + expect(getKeystore()).not.toContain(dataKeystore); }); it('uses the data directory if there is a pre-existing keystore in the data directory', () => { + const configKeystore = join(getConfigDirectory(), 'opensearch_dashboards.keystore'); + const dataKeystore = join(getDataPath(), 'opensearch_dashboards.keystore'); sandbox.stub(fs, 'existsSync').returns(true); - expect(getKeystore()).toContain('data'); - expect(getKeystore()).not.toContain('config'); + expect(getKeystore()).toContain(dataKeystore); + expect(getKeystore()).not.toContain(configKeystore); }); it('logs a deprecation warning if the data directory is used', () => { From e4fccfc3787d358191a9ea87b4eacdd0f6092711 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Mon, 17 Apr 2023 13:15:32 -0700 Subject: [PATCH 3/7] Add docker files and instructions for debugging functional tests (#3747) This PR introduces new Docker files to enable debugging of Selenium functional tests for Docker users. It configures a VNC viewer for real-time browser interaction monitoring during test execution. Additionally, a new section is added to the documentation detailing the process of running and debugging Selenium functional tests using Docker and a VNC viewer. Issue Resolve https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3700 Signed-off-by: Anan Zhuang --- CHANGELOG.md | 1 + TESTING.md | 10 +-- dev-tools/install-docker-dev.sh | 16 ++++- docs/docker-dev/Dockerfile.selenium | 70 +++++++++++++++++++++ docs/docker-dev/docker-compose.selenium.yml | 24 +++++++ docs/docker-dev/docker-dev-setup-manual.md | 65 +++++++++++++++---- docs/docker-dev/start-vnc.sh | 15 +++++ 7 files changed, 183 insertions(+), 18 deletions(-) create mode 100644 docs/docker-dev/Dockerfile.selenium create mode 100644 docs/docker-dev/docker-compose.selenium.yml create mode 100644 docs/docker-dev/start-vnc.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 03bdc5998392..ad516b6c049f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -158,6 +158,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Doc] Update SECURITY.md with instructions for nested dependencies and backporting ([#3497](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3497)) - [Doc] [Console] Fix/update documentation links in Dev Tools console ([#3724](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3724)) - [Doc] Update DEVELOPER_GUIDE.md with added manual bootstrap timeout solution and max virtual memory error solution with docker ([#3764](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3764)) +- [Doc] Add docker files and instructions for debugging Selenium functional tests ([#3747](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3747)) ### 🛠 Maintenance diff --git a/TESTING.md b/TESTING.md index 9cca21b7eee6..23866aede01c 100644 --- a/TESTING.md +++ b/TESTING.md @@ -18,7 +18,7 @@ OpenSearch Dashboards uses [Jest](https://jestjs.io/) for unit and integration t In general, we recommend four tiers of tests: * Unit tests: Unit tests: small and modular tests that utilize mocks for external dependencies. -* Integration tests: higher-level tests that verify interactions between systems (eg. HTTP APIs, OpenSearch API calls, calling other plugin). +* Integration tests: higher-level tests that verify interactions between systems (eg. HTTP APIs, OpenSearch API calls, calling other plugin). * End-to-end tests (e2e): functional tests that verify behavior in a web browser. * Backwards Compatibility tests: cypress tests that verify any changes are backwards compatible with previous versions. @@ -57,6 +57,8 @@ Say that you would want to debug a test in CI group 1, you can run the following This will print off an address, to which you could open your chrome browser on your instance and navigate to `chrome://inspect/#devices` and inspect the functional test runner `scripts/functional_tests.js`. +If you prefer to run functional tests using Docker, you can find instructions on how to set up and debug functional tests in a Docker environment in the [Debug Functional Tests](docs/docker-dev/docker-dev-setup-manual.md#debug-functional-tests) section of the `docker-dev-setup-manual.md` file. + ### Backwards Compatibility tests To run all the backwards compatibility tests on OpenSearch Dashboards without security: @@ -80,13 +82,13 @@ This will create an archive of the data based on the OpenSearch Dashboards versi Make sure you run lint checker before submitting a pull request. To run lint checker: `node scripts/precommit_hook.js --fix` -Please ensure that you don't introduce any broken links accidently. For any intentional broken link (e.g. dummy url in unit test), you can add it to [lycheeexclude](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.lycheeexclude) allow list specifically. +Please ensure that you don't introduce any broken links accidently. For any intentional broken link (e.g. dummy url in unit test), you can add it to [lycheeexclude](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/.lycheeexclude) allow-list specifically. # Writing Tests Conventions and best practices for writing tests can be found in [/src/core/TESTING.md](/src/core/TESTING.md) # Continuous Integration -Automated testing is provided with Jenkins for Continuous Integration. Jenkins enables developers to build, deploy, and automate projects and provides us to run groups of tests quickly. CI groups are ran from the [Jenkinsfile](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/Jenkinsfile). +Automated testing is provided by Jenkins for Continuous Integration. Jenkins enables developers to build, deploy, and automate projects, and permits us to run groups of tests quickly. CI groups are run from [Jenkinsfile](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/Jenkinsfile). # Environment misc Selenium tests are run in headless mode on CI. Locally the same tests will be executed in a real browser. You can activate headless mode by setting the environment variable: @@ -114,4 +116,4 @@ sudo apt-get install -y --allow-downgrades /tmp/chrome.deb Although Jest is the standard for this project, there are a few Mocha tests that still exist. You can run these tests by running: `yarn test:mocha` -However, these tests will eventually be migrated. Please avoid writing new Mocha tests. For further questions or to check the status please see this [issue](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/215). \ No newline at end of file +However, these tests will eventually be migrated; please avoid writing new Mocha tests. For further questions or to check the status, please see this [issue](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/215). diff --git a/dev-tools/install-docker-dev.sh b/dev-tools/install-docker-dev.sh index 0d40d12afed6..bd5c51496968 100644 --- a/dev-tools/install-docker-dev.sh +++ b/dev-tools/install-docker-dev.sh @@ -21,8 +21,22 @@ osd_do_copy_dev_docker_files(){ mkdir -p "$INSTALL_DIR" osd_download -s "$ENTRYPOINT_SRC" -o "./$INSTALL_DIR/entrypoint.sh" osd_download -s "$DOCKER_COMPOSE_SRC" -o "./$INSTALL_DIR/docker-compose.yml" + + if [ "$1" = "--ftr" ]; then + printf "run ftr" + local START_VNC_SRC + START_VNC_SRC="https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/docs/docker-dev/start-vnc.sh" + local DOCKERFILE_SELENIUM_SRC + DOCKERFILE_SELENIUM_SRC="https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/docs/docker-dev/Dockerfile.selenium" + local DOCKER_COMPOSE_SELENIUM_SRC + DOCKER_COMPOSE_SELENIUM_SRC="https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/docs/docker-dev/docker-compose.selenium.yml" + + osd_download -s "$START_VNC_SRC" -o "./$INSTALL_DIR/start-vnc.sh" + osd_download -s "$DOCKERFILE_SELENIUM_SRC" -o "./$INSTALL_DIR/Dockerfile.selenium" + osd_download -s "$DOCKER_COMPOSE_SELENIUM_SRC" -o "./$INSTALL_DIR/docker-compose.selenium.yml" + fi } -osd_do_copy_dev_docker_files +osd_do_copy_dev_docker_files $1 } # this ensures the entire script is downloaded # diff --git a/docs/docker-dev/Dockerfile.selenium b/docs/docker-dev/Dockerfile.selenium new file mode 100644 index 000000000000..059b2d74afa5 --- /dev/null +++ b/docs/docker-dev/Dockerfile.selenium @@ -0,0 +1,70 @@ +FROM abbyhu/opensearch-dashboards-dev:latest + +# Switch to root user +USER root + +# Install the locales package +# Uncomment the en_US.UTF-8 UTF-8 line in the sytstem /etc/locale.gen file +# Then generate the locales and update the system locale to en_US.UTF-8 +# Install all other requested packages +RUN apt-get update && \ + apt-get install -y locales && \ + sed -i -e 's/# en_US.UTF-8 UTF-8/en_US.UTF-8 UTF-8/' /etc/locale.gen && \ + dpkg-reconfigure --frontend=noninteractive locales && \ + update-locale LANG=en_US.UTF-8 && \ + apt-get install -y xvfb x11vnc openbox lxde-core lxterminal wget apt-transport-https sudo + +ENV LANG en_US.UTF-8 +ENV LC_ALL en_US.UTF-8 + +# Create the LXTerminal configuration directory and set the encoding +RUN mkdir -p /etc/xdg/lxterminal && \ + echo '[General]' >> /etc/xdg/lxterminal/lxterminal.conf && \ + echo 'encoding=UTF-8' >> /etc/xdg/lxterminal/lxterminal.conf + +# Specify the version of Chrome that matches the version of chromedriver in the package.json. +#ARG CHROME_VERSION=107.0.5304.121-1 + +## Install Google Chrome version 107 +#RUN curl -sSL https://dl.google.com/linux/linux_signing_key.pub | apt-key add - && \ +# echo 'deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main' > /etc/apt/sources.list.d/google-chrome.list && \ +# apt-get update && \ +# wget -O /tmp/chrome.deb https://dl.google.com/linux/chrome/deb/pool/main/g/google-chrome-stable/google-chrome-stable_${CHROME_VERSION}_amd64.deb && \ +# apt-get install -y /tmp/chrome.deb --no-install-recommends && \ +# rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* + +# Install Google Chrome +RUN wget -q -O - https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add - && \ + echo 'deb [arch=amd64] http://dl.google.com/linux/chrome/deb/ stable main' > /etc/apt/sources.list.d/google-chrome.list && \ + apt-get update && \ + apt-get install -y google-chrome-stable && \ + rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* + +# Create the directory and set the ownership for osd-dev user +RUN mkdir -p /docker-workspace/OpenSearch-Dashboards/.opensearch && \ + chown -R osd-dev /docker-workspace/OpenSearch-Dashboards/.opensearch + +COPY entrypoint.sh /entrypoint.sh +COPY start-vnc.sh /start-vnc.sh + +RUN chmod +x /entrypoint.sh /start-vnc.sh + +ENTRYPOINT ["/entrypoint.sh"] + +# Create a Google Chrome desktop file with the specified launch options. +# Currently Google Chrome is not available in the menu of your VNC Viewer session. +# To enable that, you need to open the terminal and run: +# google-chrome --no-sandbox --disable-gpu --remote-debugging-address=0.0.0.0 --remote-debugging-port=9222 +# This part is added to automate this process by creating a desktop file for Google Chrome. +RUN echo '[Desktop Entry]\n\ +Version=1.0\n\ +Name=Google Chrome\n\ +GenericName=Web Browser\n\ +Comment=Access the Internet\n\ +Exec=google-chrome --no-sandbox --disable-gpu --remote-debugging-address=0.0.0.0 --remote-debugging-port=9222 %U\n\ +Terminal=false\n\ +Icon=google-chrome\n\ +Type=Application\n\ +Categories=Network;WebBrowser;\n\ +MimeType=text/html;text/xml;application/xhtml_xml;x-scheme-handler/http;x-scheme-handler/https;'\ +> /usr/share/applications/google-chrome.desktop diff --git a/docs/docker-dev/docker-compose.selenium.yml b/docs/docker-dev/docker-compose.selenium.yml new file mode 100644 index 000000000000..c0ed2bafcaa4 --- /dev/null +++ b/docs/docker-dev/docker-compose.selenium.yml @@ -0,0 +1,24 @@ +version: '3' +services: + selenium-test: + build: + context: . + dockerfile: Dockerfile.selenium + container_name: selenium-test + depends_on: + - dev-env + environment: + - DISPLAY=:99 + volumes: + - /dev/shm:/dev/shm + - .:/workspace + networks: + - opensearch-net + ports: + - 5900:5900 + entrypoint: ["/start-vnc.sh"] +volumes: + opensearch-data: + osd-dev: +networks: + opensearch-net: diff --git a/docs/docker-dev/docker-dev-setup-manual.md b/docs/docker-dev/docker-dev-setup-manual.md index 53b80e9ff633..3263372a3d45 100644 --- a/docs/docker-dev/docker-dev-setup-manual.md +++ b/docs/docker-dev/docker-dev-setup-manual.md @@ -1,18 +1,18 @@ # Docker Development Environment Setup The following instructions demonstrate how to set up a development environment for OpenSearch Dashboards using Docker. It utilizes tools such as `Docker` and `VS Code`, and users should be familiar with the basic usages of them. Users will be able to develop and run the application inside VS Code without additional configurations. -1. Install [Docker](https://docs.docker.com/get-docker/) if not already installed. +1. Install [Docker](https://docs.docker.com/get-docker/) if not already installed. * Make sure that Docker daemon is running. (For windows and macos,you need to have [Docker Desktop](https://docs.docker.com/desktop/), or its alternatives, such as [Finch](https://github.com/runfinch/finch)) 2. In the terminal, run the command below. - * This should create a folder named `opensearch-dashboards-docker-dev` and it should contain two files: `docker-compose.yml` and `entrypoint.sh`. + * This should create a folder named `opensearch-dashboards-docker-dev` and it should contain two files: `docker-compose.yml` and `entrypoint.sh`. * Here is the link to the installer script: `https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/dev-tools/install-docker-dev.sh` if needed. ```bash curl -o- https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/dev-tools/install-docker-dev.sh | bash ``` -3. Open VS Code or [install it](https://code.visualstudio.com/download), if it's not already installed. +3. Open VS Code or [install it](https://code.visualstudio.com/download), if it's not already installed. * Make sure VS Code has the extensions `Dev Containers` and `Docker` installed. If not, go to `Extensions` tab, search and install them. 4. Under the Discover tab, click `Open Folder`, and open the `opensearch-dashboards-docker-dev` folder that we just created. @@ -21,34 +21,34 @@ curl -o- https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboa * If fork repo has not been created: Go to [OpenSearch Dashboards github page](https://github.com/opensearch-project/OpenSearch-Dashboards) and under fork, select create a new fork, and then copy the https link of the fork url and use it in the above command. The command needs to be re-run every time it re-start the docker compose file in a new terminal. ```bash export REPO_URL=[insert your fork repo url here] -``` - -6. Run the `docker-compose.yml` file in the background by typing: +``` + +6. Run the `docker-compose.yml` file in the background by typing: ```bash docker compose up -d --build ``` -7. Under the `Docker` tab in VS Code, verify that there are two containers running: `opensearchproject/opensearch:latest` and `abbyhu/opensearch-dashboards-dev:latest`. - * This can also be verified by using the command line: +7. Under the `Docker` tab in VS Code, verify that there are two containers running: `opensearchproject/opensearch:latest` and `abbyhu/opensearch-dashboards-dev:latest`. + * This can also be verified by using the command line: ```bash docker ps ``` -8. Right-click `abbyhu/opensearch-dashboards-dev:latest`, and select `Attach Visual Studio Code`. - * This will ssh into the container and you will be able to view and edit the files using VS Code as the code editor. - * If you do not wish to use VS Code as the code editor, the alternative way of ssh into the container is by using the command below: +8. Right-click `abbyhu/opensearch-dashboards-dev:latest`, and select `Attach Visual Studio Code`. + * This will ssh into the container and you will be able to view and edit the files using VS Code as the code editor. + * If you do not wish to use VS Code as the code editor, the alternative way of ssh into the container is by using the command below: ```bash docker exec -it dev-env /bin/bash ``` -9. For the new VS Code window, if it is not showing the repository code, then select `Open Folder`. Then open `/workspace-docker/OpenSearch-Dashboards`. +9. For the new VS Code window, if it is not showing the repository code, then select `Open Folder`. Then open `/docker-workspace/OpenSearch-Dashboards`. 10. In the terminal, start the OpenSearch Dashboards application by typing: ```bash yarn start:docker ``` -11. Now that OpenSearch Dashboards is running, you should be able to see a log line similar to `[info][server][OpenSearchDashboards][http] http server running at http://0.0.0.0:5603/dog`. +11. Now that OpenSearch Dashboards is running, you should be able to see a log line similar to `[info][server][OpenSearchDashboards][http] http server running at http://0.0.0.0:5603/dog`. * The last three letters `dog` are randomly generated every time we start dashboards. 12. Wait for the optimizer to run, which takes about 100s - 200s. Once the optimizer is finished running, it will show a line such as `[success][@osd/optimizer] 48 bundles compiled successfully after 204.9 sec, watching for changes`. @@ -57,3 +57,42 @@ yarn start:docker * Files are constantly watched, so when you make code changes, OpenSearch Dashboards will rebuild and restart automatically. Refresh the link in the browser and the new changes should be applied. 14. `Git` is already configured in the `entrypoint.sh` file, and the remote is already tracking the fork repository. You can start contributing by creating your branch off the main, and commit your first PR! + +# Debug Functional Tests +This section explains how to run Selenium functional tests using Docker and debug the tests through browser interaction. + +1. Install a VNC viewer, such as [RealVNC](https://www.realvnc.com/en/connect/download/viewer/), if you haven't already. You'll need to set up a free account to use RealVNC. This VNC viewer will enable you to view and interact with the browser running the Selenium tests. The subsequent steps will illustrate the setup process using RealVNC as an example. + +2. Make sure you have completed steps 1-5 in the [Docker Development Environment Setup](#docker-development-environment-setup). Now, ensure you have 5 files, `docker-compose.yml`, `docker-compose.selenium.yml`, `Dockerfile.selenium`, `entrypoint.sh` and `start-vnc.sh`. You can also download them by running the installer script. This time you need to pass a parameter `--ftr`. + +```bash +curl -o- https://raw.githubusercontent.com/opensearch-project/OpenSearch-Dashboards/main/dev-tools/install-docker-dev.sh | bash -s -- --ftr +``` + +3. In the terminal, run the following commands to stop any running containers, build the new configuration, and start the containers: + +```bash +docker-compose -f docker-compose.yml -f docker-compose.selenium.yml down +docker-compose -f docker-compose.yml -f docker-compose.selenium.yml up -d --build +``` + +4. Under the Docker tab in VS Code, you should see three containers running: `opensearchproject/opensearch:latest`, `abbyhu/opensearch-dashboards-dev:latest`, and `selenium-test`. + +5. First, right-click on `opensearch-dashboards-docker-dev-selenium-test` (which we'll refer to as `selenium-test` for simplicity) and choose `Attach Visual Studio Code`. This action mirrors [Step 8](#install-step-8) from the [Docker Development Environment Setup](#docker-development-environment-setup). By doing this, you'll be able to edit the functional test directly within the `selenium-test` container using VS Code which is located at `test/functional`. + +6. Connect to the VNC server running in the Docker container. First, launch the VNC viewer, then enter `localhost:5900` at the top and press return. You will see a popup window labeled `Unencrypted connection`; click continue. Enter the password (the default password is 123), which is configured in `start-vnc.sh`. + +7. After entering the VNC viewer, click the menu at the bottom left. Then click `Internet` to ensure Google works. Also, click `System Tools` and select `LXTerminal` to verify you can type in the terminal. The default terminal path should be `/docker-workspace/OpenSearch-Dashboards`. + +8. Run OpenSearch and OpenSearch Dashboards separately before running functional tests. This can help isolate the functional test process and confirm that any issues are caused only by the functional test. Follow these steps: + * First, start OpenSearch by executing `yarn opensearch snapshot` in one terminal. + * In a second terminal, run `yarn start --no-base-path` to start OpenSearch Dashboards on port 5601. + * Make sure OpenSearch Dashboards is running on port 5601 before running any functional tests. + + Please note that the initial startup process may take 2 to 10 minutes, as it requires all the bundles to be assembled. This only occurs the first time you run it; afterward, everything is cached, significantly reducing the startup time for OpenSearch Dashboards. The initial bundling speed is dependent on your hardware limitations and VNC settings. Feel free to make adjustments to improve the process as needed. + +9. Open a separate terminal and update Chromedriver by executing `node scripts/upgrade_chromedriver.js`, followed by `yarn osd bootstrap`. This will update the Chromedriver version within the node_modules directory, allowing you to use a compatible version with your installed Google Chrome. + +10. To run functional tests, please refer to the [Functional tests section](../../TESTING.md#functional-tests) in the `TESTING.md` file. + +11. The Selenium tests will be executed in the browser, viewable through the VNC viewer. You can monitor the test progress in real-time. diff --git a/docs/docker-dev/start-vnc.sh b/docs/docker-dev/start-vnc.sh new file mode 100644 index 000000000000..e592aa6e6590 --- /dev/null +++ b/docs/docker-dev/start-vnc.sh @@ -0,0 +1,15 @@ +#!/bin/bash + +# Start the VNC server +export DISPLAY=:99 + +mkdir -p /home/.vnc + +Xvfb :99 -screen 0 1280x1024x24 & + +x11vnc -forever -display :99 -rfbport 5900 -passwd 123 -bg -o /home/.vnc/x11vnc.log + +sudo -u osd-dev startlxde & + +# Keep the container running +tail -f /dev/null From 0762566ec143fe9a8c1d2b61d4d4c33d64f825d3 Mon Sep 17 00:00:00 2001 From: Miki Date: Mon, 17 Apr 2023 15:23:52 -0700 Subject: [PATCH 4/7] Omit adding the `osd-version` header when the Fetch request is to an external origin (#3643) * Making `fetch` requests using core/public/http/fetch, an `osd-version` header is forcefully added, even to external requests. This change examines the destination and only adds the header to relative URLs and those that are to OSD itself. * This change also adds `osd-xsrf` to calls that use `osd-version` incorrectly to satisfy XSRF protection Fixes #3277 Signed-off-by: Miki --- CHANGELOG.md | 2 ++ src/core/public/http/fetch.ts | 14 +++++++++++++- .../__snapshots__/ui_settings_api.test.ts.snap | 7 +++++++ .../integration_tests/lifecycle_handlers.test.ts | 5 ++++- src/core/server/http/lifecycle_handlers.test.ts | 9 +++++---- src/core/server/http/lifecycle_handlers.ts | 3 ++- src/plugins/bfetch/public/plugin.ts | 1 + .../public/angular/angular_config.tsx | 4 ++++ 8 files changed, 38 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad516b6c049f..c29de254733b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Vis Builder] Add metric to metric, bucket to bucket aggregation persistence ([#3495](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3495)) - Use mirrors to download Node.js binaries to escape sporadic 404 errors ([#3619](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3619)) - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544)) +- Add `osd-xsrf` header to all requests that incorrectly used `node-version` to satisfy XSRF protection ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) - [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605)) - [VisBuilder] Add UI actions handler ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [Dashboard] Indicate that IE is no longer supported ([#3641](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3641)) @@ -117,6 +118,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Region Maps] Add ui setting to configure custom vector map's size parameter([#3399](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3399)) - [Search Telemetry] Fixes search telemetry's observable object that won't be GC-ed([#3390](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3390)) - Clean up and rebuild `@osd/pm` ([#3570](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3570)) +- Omit adding the `osd-version` header when the Fetch request is to an external origin ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) - [Vega] Add Filter custom label for opensearchDashboardsAddFilter ([#3640](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3640)) - [Timeline] Fix y-axis label color in dark mode ([#3698](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3698)) - [VisBuilder] Fix multiple warnings thrown on page load ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) diff --git a/src/core/public/http/fetch.ts b/src/core/public/http/fetch.ts index cefaa353f7fa..694372c46d99 100644 --- a/src/core/public/http/fetch.ts +++ b/src/core/public/http/fetch.ts @@ -31,6 +31,7 @@ import { omitBy } from 'lodash'; import { format } from 'url'; import { BehaviorSubject } from 'rxjs'; +import { isRelativeUrl } from '@osd/std'; import { IBasePath, @@ -144,7 +145,6 @@ export class Fetch { headers: removedUndefined({ 'Content-Type': 'application/json', ...options.headers, - 'osd-version': this.params.opensearchDashboardsVersion, }), }; @@ -158,6 +158,18 @@ export class Fetch { fetchOptions.headers['osd-system-request'] = 'true'; } + /* `osd-version` is used on the server-side to make sure that an incoming request originated from a front-end + * of the same version; see core/server/http/lifecycle_handlers.ts + * `osd-xsrf` is to satisfy XSRF protection but is only meaningful to OpenSearch Dashboards. + * + * If the `url` equals `basePath`, starts with `basePath` + '/', or is relative, add `osd-version` and `osd-xsrf` headers. + */ + const basePath = this.params.basePath.get(); + if (isRelativeUrl(url) || url === basePath || url.startsWith(`${basePath}/`)) { + fetchOptions.headers['osd-version'] = this.params.opensearchDashboardsVersion; + fetchOptions.headers['osd-xsrf'] = 'osd-fetch'; + } + return new Request(url, fetchOptions as RequestInit); } diff --git a/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap b/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap index f80d88dd91ff..52d04f52ff8b 100644 --- a/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap +++ b/src/core/public/ui_settings/__snapshots__/ui_settings_api.test.ts.snap @@ -9,6 +9,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -20,6 +21,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -36,6 +38,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -47,6 +50,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -63,6 +67,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -74,6 +79,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, @@ -113,6 +119,7 @@ Array [ "accept": "application/json", "content-type": "application/json", "osd-version": "opensearchDashboardsVersion", + "osd-xsrf": "osd-fetch", }, "method": "POST", }, diff --git a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts index fc13c1ae3fbb..0742e8c08d6d 100644 --- a/src/core/server/http/integration_tests/lifecycle_handlers.test.ts +++ b/src/core/server/http/integration_tests/lifecycle_handlers.test.ts @@ -231,20 +231,23 @@ describe('core lifecycle handlers', () => { .expect(200, 'ok'); }); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection it('accepts requests with the version header', async () => { await getSupertest(method.toLowerCase(), testPath) .set(versionHeader, actualVersion) .expect(200, 'ok'); }); + // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection it('rejects requests without either an xsrf or version header', async () => { await getSupertest(method.toLowerCase(), testPath).expect(400, { statusCode: 400, error: 'Bad Request', - message: 'Request must contain a osd-xsrf header.', + message: 'Request must contain the osd-xsrf header.', }); }); + // ToDo: Rename next; `osd-version` incorrectly used for satisfying XSRF protection it('accepts whitelisted requests without either an xsrf or version header', async () => { await getSupertest(method.toLowerCase(), whitelistedTestPath).expect(200, 'ok'); }); diff --git a/src/core/server/http/lifecycle_handlers.test.ts b/src/core/server/http/lifecycle_handlers.test.ts index 6a49bbfa14fa..cd96f0ea7760 100644 --- a/src/core/server/http/lifecycle_handlers.test.ts +++ b/src/core/server/http/lifecycle_handlers.test.ts @@ -102,6 +102,7 @@ describe('xsrf post-auth handler', () => { expect(result).toEqual('next'); }); + // ToDo: Remove; `osd-version` incorrectly used for satisfying XSRF protection it('accepts requests with version header', () => { const config = createConfig({ xsrf: { whitelist: [], disableProtection: false } }); const handler = createXsrfPostAuthHandler(config); @@ -129,7 +130,7 @@ describe('xsrf post-auth handler', () => { expect(responseFactory.badRequest).toHaveBeenCalledTimes(1); expect(responseFactory.badRequest.mock.calls[0][0]).toMatchInlineSnapshot(` Object { - "body": "Request must contain a osd-xsrf header.", + "body": "Request must contain the osd-xsrf header.", } `); expect(result).toEqual('badRequest'); @@ -199,7 +200,7 @@ describe('versionCheck post-auth handler', () => { responseFactory = httpServerMock.createLifecycleResponseFactory(); }); - it('forward the request to the next interceptor if header matches', () => { + it('forward the request to the next interceptor if osd-version header matches the actual version', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: { 'osd-version': 'actual-version' } }); @@ -212,7 +213,7 @@ describe('versionCheck post-auth handler', () => { expect(result).toBe('next'); }); - it('returns a badRequest error if header does not match', () => { + it('returns a badRequest error if osd-version header exists but does not match the actual version', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: { 'osd-version': 'another-version' } }); @@ -236,7 +237,7 @@ describe('versionCheck post-auth handler', () => { expect(result).toBe('badRequest'); }); - it('forward the request to the next interceptor if header is not present', () => { + it('forward the request to the next interceptor if osd-version header is not present', () => { const handler = createVersionCheckPostAuthHandler('actual-version'); const request = forgeRequest({ headers: {} }); diff --git a/src/core/server/http/lifecycle_handlers.ts b/src/core/server/http/lifecycle_handlers.ts index 636bb8af4522..f17b07942e6a 100644 --- a/src/core/server/http/lifecycle_handlers.ts +++ b/src/core/server/http/lifecycle_handlers.ts @@ -54,8 +54,9 @@ export const createXsrfPostAuthHandler = (config: HttpConfig): OnPostAuthHandler const hasVersionHeader = VERSION_HEADER in request.headers; const hasXsrfHeader = XSRF_HEADER in request.headers; + // ToDo: Remove !hasVersionHeader; `osd-version` incorrectly used for satisfying XSRF protection if (!isSafeMethod(request.route.method) && !hasVersionHeader && !hasXsrfHeader) { - return response.badRequest({ body: `Request must contain a ${XSRF_HEADER} header.` }); + return response.badRequest({ body: `Request must contain the ${XSRF_HEADER} header.` }); } return toolkit.next(); diff --git a/src/plugins/bfetch/public/plugin.ts b/src/plugins/bfetch/public/plugin.ts index 1a3a0bffc821..e115b35a44e1 100644 --- a/src/plugins/bfetch/public/plugin.ts +++ b/src/plugins/bfetch/public/plugin.ts @@ -95,6 +95,7 @@ export class BfetchPublicPlugin url: `${basePath}/${removeLeadingSlash(params.url)}`, headers: { 'Content-Type': 'application/json', + 'osd-xsrf': 'osd-bfetch', 'osd-version': version, ...(params.headers || {}), }, diff --git a/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx b/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx index ef01d29e4b14..fbe36a289d70 100644 --- a/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx +++ b/src/plugins/opensearch_dashboards_legacy/public/angular/angular_config.tsx @@ -159,6 +159,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => { // Configure jQuery prefilter $.ajaxPrefilter(({ osdXsrfToken = true }: any, originalOptions, jqXHR) => { if (osdXsrfToken) { + jqXHR.setRequestHeader('osd-xsrf', 'osd-legacy'); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection jqXHR.setRequestHeader('osd-version', version); } }); @@ -170,6 +172,8 @@ export const $setupXsrfRequestInterceptor = (version: string) => { request(opts) { const { osdXsrfToken = true } = opts as any; if (osdXsrfToken) { + set(opts, ['headers', 'osd-xsrf'], 'osd-legacy'); + // ToDo: Remove next; `osd-version` incorrectly used for satisfying XSRF protection set(opts, ['headers', 'osd-version'], version); } return opts; From fc195ea580844dea47e21c4854ac7767f2513531 Mon Sep 17 00:00:00 2001 From: Anan Zhuang Date: Mon, 17 Apr 2023 15:54:52 -0700 Subject: [PATCH 5/7] [Table Vis] move format table, consolidate types and add unit tests (#3397) Currently, table data is formatted by a until function convertToFormattedData in TableVisComponent. In this PR, we moved the formatting data process to table_vis_response_handler.ts to combine with other data process logics. In this way, component render and data handling logics are completely isolated. This PR also solidate some types. Issue Resolved: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3395 https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2856 Signed-off-by: Anan Zhuang --- CHANGELOG.md | 1 + .../public/components/table_vis_app.scss | 16 +- .../public/components/table_vis_app.test.tsx | 98 ++++++++ .../public/components/table_vis_app.tsx | 17 +- .../public/components/table_vis_cell.test.tsx | 65 +++++ .../public/components/table_vis_cell.tsx | 38 +++ .../components/table_vis_component.test.tsx | 234 ++++++++++++++++++ .../public/components/table_vis_component.tsx | 117 ++++----- .../table_vis_component_group.test.tsx | 67 +++++ .../components/table_vis_component_group.tsx | 7 +- .../components/table_vis_grid_columns.tsx | 21 +- .../vis_type_table/public/table_vis_fn.ts | 4 +- .../public/table_vis_renderer.test.tsx | 78 ++++++ .../public/table_vis_response_handler.test.ts | 157 ++++++++++++ .../public/table_vis_response_handler.ts | 60 ++--- src/plugins/vis_type_table/public/types.ts | 7 - .../add_percentage_col.test.ts.snap | 170 +++++++++++++ .../public/utils/add_percentage_col.test.ts | 76 ++++++ .../public/utils/add_percentage_col.ts | 76 ++++++ .../public/utils/convert_to_csv_data.test.ts | 76 ++++++ .../public/utils/convert_to_csv_data.ts | 2 +- .../utils/convert_to_formatted_data.test.ts | 136 ++++++++++ .../public/utils/convert_to_formatted_data.ts | 69 ++---- .../public/utils/get_table_ui_state.test.ts | 66 +++++ .../public/utils/get_table_ui_state.ts | 40 +++ .../vis_type_table/public/utils/index.ts | 2 + .../public/utils/use_pagination.test.ts | 101 ++++++++ .../public/utils/use_pagination.ts | 23 +- 28 files changed, 1627 insertions(+), 197 deletions(-) create mode 100644 src/plugins/vis_type_table/public/components/table_vis_app.test.tsx create mode 100644 src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx create mode 100644 src/plugins/vis_type_table/public/components/table_vis_cell.tsx create mode 100644 src/plugins/vis_type_table/public/components/table_vis_component.test.tsx create mode 100644 src/plugins/vis_type_table/public/components/table_vis_component_group.test.tsx create mode 100644 src/plugins/vis_type_table/public/table_vis_renderer.test.tsx create mode 100644 src/plugins/vis_type_table/public/table_vis_response_handler.test.ts create mode 100644 src/plugins/vis_type_table/public/utils/__snapshots__/add_percentage_col.test.ts.snap create mode 100644 src/plugins/vis_type_table/public/utils/add_percentage_col.test.ts create mode 100644 src/plugins/vis_type_table/public/utils/add_percentage_col.ts create mode 100644 src/plugins/vis_type_table/public/utils/convert_to_csv_data.test.ts create mode 100644 src/plugins/vis_type_table/public/utils/convert_to_formatted_data.test.ts create mode 100644 src/plugins/vis_type_table/public/utils/get_table_ui_state.test.ts create mode 100644 src/plugins/vis_type_table/public/utils/get_table_ui_state.ts create mode 100644 src/plugins/vis_type_table/public/utils/use_pagination.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c29de254733b..001f9248f2b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple DataSource] Allow create and distinguish index pattern with same name but from different datasources ([#3571](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3571)) - [Multiple DataSource] Integrate multiple datasource with dev tool console ([#3754](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3754)) - Add satisfaction survey link to help menu ([#3676] (https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3676)) +- [Table Visualization] Move format table, consolidate types and add unit tests ([#3397](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3397)) ### 🐛 Bug Fixes diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.scss b/src/plugins/vis_type_table/public/components/table_vis_app.scss index 876847667418..aafcd40e7382 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_app.scss +++ b/src/plugins/vis_type_table/public/components/table_vis_app.scss @@ -1,21 +1,29 @@ +// Container for the Table Visualization component .visTable { display: flex; flex-direction: column; flex: 1 0 0; overflow: auto; + + @include euiScrollBar; } +// Group container for table visualization components .visTable__group { padding: $euiSizeS; margin-bottom: $euiSizeL; + display: flex; + flex-direction: column; + flex: 0 0 auto; +} - > h3 { - text-align: center; - } +// Style for table component title +.visTable__component__title { + text-align: center; } +// Modifier for visTables__group when displayed in columns .visTable__groupInColumns { - display: flex; flex-direction: row; align-items: flex-start; } diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx new file mode 100644 index 000000000000..37cb753765f8 --- /dev/null +++ b/src/plugins/vis_type_table/public/components/table_vis_app.test.tsx @@ -0,0 +1,98 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { coreMock } from '../../../../core/public/mocks'; +import { IInterpreterRenderHandlers } from 'src/plugins/expressions'; +import { TableVisApp } from './table_vis_app'; +import { TableVisConfig } from '../types'; +import { TableVisData } from '../table_vis_response_handler'; + +jest.mock('./table_vis_component_group', () => ({ + TableVisComponentGroup: () => ( +
TableVisComponentGroup
+ ), +})); + +jest.mock('./table_vis_component', () => ({ + TableVisComponent: () =>
TableVisComponent
, +})); + +describe('TableVisApp', () => { + const serviceMock = coreMock.createStart(); + const handlersMock = ({ + done: jest.fn(), + uiState: { + get: jest.fn((key) => { + switch (key) { + case 'vis.sortColumn': + return {}; + case 'vis.columnsWidth': + return []; + default: + return undefined; + } + }), + set: jest.fn(), + }, + event: 'event', + } as unknown) as IInterpreterRenderHandlers; + const visConfigMock = ({} as unknown) as TableVisConfig; + + it('should render TableVisComponent if no split table', () => { + const visDataMock = { + table: { + columns: [], + rows: [], + formattedColumns: [], + }, + tableGroups: [], + } as TableVisData; + const { getByTestId } = render( + + ); + expect(getByTestId('TableVisComponent')).toBeInTheDocument(); + }); + + it('should render TableVisComponentGroup component if split direction is column', () => { + const visDataMock = { + tableGroups: [], + direction: 'column', + } as TableVisData; + const { container, getByTestId } = render( + + ); + expect(container.outerHTML.includes('visTable visTable__groupInColumns')).toBe(true); + expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument(); + }); + + it('should render TableVisComponentGroup component if split direction is row', () => { + const visDataMock = { + tableGroups: [], + direction: 'row', + } as TableVisData; + const { container, getByTestId } = render( + + ); + expect(container.outerHTML.includes('visTable')).toBe(true); + expect(getByTestId('TableVisComponentGroup')).toBeInTheDocument(); + }); +}); diff --git a/src/plugins/vis_type_table/public/components/table_vis_app.tsx b/src/plugins/vis_type_table/public/components/table_vis_app.tsx index af10500a1a92..81f4d775f1e5 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_app.tsx +++ b/src/plugins/vis_type_table/public/components/table_vis_app.tsx @@ -4,20 +4,22 @@ */ import './table_vis_app.scss'; -import React, { useEffect, useState } from 'react'; +import React, { useEffect } from 'react'; import classNames from 'classnames'; import { CoreStart } from 'opensearch-dashboards/public'; import { I18nProvider } from '@osd/i18n/react'; import { IInterpreterRenderHandlers } from 'src/plugins/expressions'; +import { PersistedState } from '../../../visualizations/public'; import { OpenSearchDashboardsContextProvider } from '../../../opensearch_dashboards_react/public'; -import { TableContext } from '../table_vis_response_handler'; -import { TableVisConfig, ColumnSort, ColumnWidth, TableUiState } from '../types'; +import { TableVisData } from '../table_vis_response_handler'; +import { TableVisConfig } from '../types'; import { TableVisComponent } from './table_vis_component'; import { TableVisComponentGroup } from './table_vis_component_group'; +import { getTableUIState, TableUiState } from '../utils'; interface TableVisAppProps { services: CoreStart; - visData: TableContext; + visData: TableVisData; visConfig: TableVisConfig; handlers: IInterpreterRenderHandlers; } @@ -38,12 +40,7 @@ export const TableVisApp = ({ visTable__groupInColumns: direction === 'column', }); - // TODO: remove duplicate sort and width state - // Issue: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2704#issuecomment-1299380818 - const [sort, setSort] = useState({ colIndex: null, direction: null }); - const [width, setWidth] = useState([]); - - const tableUiState: TableUiState = { sort, setSort, width, setWidth }; + const tableUiState: TableUiState = getTableUIState(handlers.uiState as PersistedState); return ( diff --git a/src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx new file mode 100644 index 000000000000..9b1b1c02ac40 --- /dev/null +++ b/src/plugins/vis_type_table/public/components/table_vis_cell.test.tsx @@ -0,0 +1,65 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render } from '@testing-library/react'; + +import { OpenSearchDashboardsDatatableRow } from 'src/plugins/expressions'; +import { FormattedColumn } from '../types'; +import { getTableVisCellValue } from './table_vis_cell'; +import { FieldFormat } from 'src/plugins/data/public'; + +class MockFieldFormat extends FieldFormat { + convert = jest.fn(); +} + +describe('getTableVisCellValue', () => { + const mockFormatter = new MockFieldFormat(); + + const columns: FormattedColumn[] = [ + { + id: 'testId', + title: 'Test Column', + formatter: mockFormatter, + filterable: true, + }, + ]; + + const sortedRows: OpenSearchDashboardsDatatableRow[] = [ + { + testId: 'Test Value 1', + }, + { + testId: 'Test Value 2', + }, + ]; + + const TableCell = ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => { + const getCellValue = getTableVisCellValue(sortedRows, columns); + return getCellValue({ rowIndex, columnId }); + }; + + beforeEach(() => { + mockFormatter.convert.mockClear(); + }); + + test('should render cell value with correct formatting', () => { + mockFormatter.convert.mockReturnValueOnce('Test Value 1'); + const { getByText } = render(); + expect(mockFormatter.convert).toHaveBeenCalledWith('Test Value 1', 'html'); + expect(getByText('Test Value 1')).toBeInTheDocument(); + expect(getByText('Test Value 1').closest('strong')).toBeInTheDocument(); + }); + + test('should return null when rowIndex is out of bounds', () => { + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); + + test('should return null when no matching columnId is found', () => { + const { container } = render(); + expect(container.firstChild).toBeNull(); + }); +}); diff --git a/src/plugins/vis_type_table/public/components/table_vis_cell.tsx b/src/plugins/vis_type_table/public/components/table_vis_cell.tsx new file mode 100644 index 000000000000..30c0877df701 --- /dev/null +++ b/src/plugins/vis_type_table/public/components/table_vis_cell.tsx @@ -0,0 +1,38 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import dompurify from 'dompurify'; + +import { OpenSearchDashboardsDatatableRow } from 'src/plugins/expressions'; +import { FormattedColumn } from '../types'; + +export const getTableVisCellValue = ( + sortedRows: OpenSearchDashboardsDatatableRow[], + columns: FormattedColumn[] +) => ({ rowIndex, columnId }: { rowIndex: number; columnId: string }) => { + if (rowIndex < 0 || rowIndex >= sortedRows.length) { + return null; + } + const row = sortedRows[rowIndex]; + if (!row || !row.hasOwnProperty(columnId)) { + return null; + } + const rawContent = row[columnId]; + const colIndex = columns.findIndex((col) => col.id === columnId); + const htmlContent = columns[colIndex].formatter.convert(rawContent, 'html'); + const formattedContent = ( + /* + * Justification for dangerouslySetInnerHTML: + * This is one of the visualizations which makes use of the HTML field formatters. + * Since these formatters produce raw HTML, this visualization needs to be able to render them as-is, relying + * on the field formatter to only produce safe HTML. + * `htmlContent` is created by converting raw data via HTML field formatter, so we need to make sure this value never contains + * any unsafe HTML (e.g. by bypassing the field formatter). + */ +
// eslint-disable-line react/no-danger + ); + return formattedContent || null; +}; diff --git a/src/plugins/vis_type_table/public/components/table_vis_component.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_component.test.tsx new file mode 100644 index 000000000000..6e2d0090aa36 --- /dev/null +++ b/src/plugins/vis_type_table/public/components/table_vis_component.test.tsx @@ -0,0 +1,234 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { shallow } from 'enzyme'; +import { TableVisConfig, ColumnSort } from '../types'; +import { TableVisComponent } from './table_vis_component'; +import { FormattedColumn } from '../types'; +import { FormattedTableContext } from '../table_vis_response_handler'; +import { getTableVisCellValue } from './table_vis_cell'; +import { getDataGridColumns } from './table_vis_grid_columns'; +import { EuiDataGridColumn } from '@elastic/eui'; + +jest.mock('./table_vis_cell', () => ({ + getTableVisCellValue: jest.fn(() => () => {}), +})); + +const mockGetDataGridColumns = jest.fn(() => []); +jest.mock('./table_vis_grid_columns', () => ({ + getDataGridColumns: jest.fn(() => mockGetDataGridColumns()), +})); + +const table = { + formattedColumns: [ + { + id: 'col-0-2', + title: 'name.keyword: Descending', + formatter: {}, + filterable: true, + }, + { + id: 'col-1-1', + title: 'Count', + formatter: {}, + filterable: false, + sumTotal: 5, + formattedTotal: 5, + total: 5, + }, + ] as FormattedColumn[], + rows: [ + { 'col-0-2': 'Alice', 'col-1-1': 3 }, + { 'col-0-2': 'Anthony', 'col-1-1': 1 }, + { 'col-0-2': 'Timmy', 'col-1-1': 1 }, + ], + columns: [ + { id: 'col-0-2', name: 'Name' }, + { id: 'col-1-1', name: 'Count' }, + ], +} as FormattedTableContext; + +const visConfig = { + buckets: [ + { + accessor: 0, + aggType: 'terms', + format: { + id: 'terms', + params: { + id: 'number', + missingBucketLabel: 'Missing', + otherBucketLabel: 'Other', + parsedUrl: { + basePath: '/arf', + origin: '', + pathname: '/arf/app/home', + }, + }, + }, + label: 'age: Descending', + params: {}, + }, + ], + metrics: [ + { + accessor: 1, + aggType: 'count', + format: { + id: 'number', + }, + label: 'Count', + params: {}, + }, + ], + perPage: 10, + percentageCol: '', + showMetricsAtAllLevels: false, + showPartialRows: false, + showTotal: false, + title: '', + totalFunc: 'sum', +} as TableVisConfig; + +const uiState = { + sort: {} as ColumnSort, + setSort: jest.fn(), + colWidth: [], + setWidth: jest.fn(), +}; + +describe('TableVisComponent', function () { + const props = { + title: '', + table, + visConfig, + event: jest.fn(), + uiState, + }; + + const dataGridColumnsValue = [ + { + id: 'col-0-2', + display: 'name.keyword: Descending', + displayAsText: 'name.keyword: Descending', + actions: { + showHide: false, + showMoveLeft: false, + showMoveRight: false, + showSortAsc: {}, + showSortDesc: {}, + }, + cellActions: expect.any(Function), + }, + { + id: 'col-1-1', + display: 'Count', + displayAsText: 'Count', + actions: { + showHide: false, + showMoveLeft: false, + showMoveRight: false, + showSortAsc: {}, + showSortDesc: {}, + }, + cellActions: undefined, + }, + ] as EuiDataGridColumn[]; + + it('should render data grid', () => { + const comp = shallow(); + expect(comp.find('EuiDataGrid')).toHaveLength(1); + }); + + it('should render title when provided', () => { + const compWithTitle = shallow(); + const titleElement = compWithTitle.find('EuiTitle'); + expect(titleElement).toHaveLength(1); + expect(titleElement.find('h3').text()).toEqual('Test Title'); + }); + + it('should not render title when not provided', () => { + const compWithoutTitle = shallow(); + const titleElement = compWithoutTitle.find('EuiTitle'); + expect(titleElement).toHaveLength(0); + }); + + it('should set sort if sort column', () => { + mockGetDataGridColumns.mockReturnValueOnce(dataGridColumnsValue); + const comp = shallow(); + const { onSort } = comp.find('EuiDataGrid').prop('sorting') as any; + onSort([]); + expect(props.uiState.setSort).toHaveBeenCalledWith([]); + onSort([{ id: 'col-0-2', direction: 'asc' }]); + expect(props.uiState.setSort).toHaveBeenCalledWith({ colIndex: 0, direction: 'asc' }); + onSort([ + { id: 'col-0-2', direction: 'asc' }, + { id: 'col-1-1', direction: 'desc' }, + ]); + expect(props.uiState.setSort).toHaveBeenCalledWith({ colIndex: 1, direction: 'desc' }); + }); + + it('should set width if adjust column width', () => { + const uiStateProps = { + ...props.uiState, + width: [ + { colIndex: 0, width: 12 }, + { colIndex: 1, width: 8 }, + ], + }; + const comp = shallow(); + const onColumnResize = comp.find('EuiDataGrid').prop('onColumnResize') as any; + onColumnResize({ columnId: 'col-0-2', width: 18 }); + expect(props.uiState.setWidth).toHaveBeenCalledWith({ colIndex: 0, width: 18 }); + const updatedComp = shallow(); + const onColumnResizeUpdate = updatedComp.find('EuiDataGrid').prop('onColumnResize') as any; + onColumnResizeUpdate({ columnId: 'col-0-2', width: 18 }); + expect(props.uiState.setWidth).toHaveBeenCalledWith({ colIndex: 0, width: 18 }); + }); + + it('should create sortedRows and pass to getTableVisCellValue', () => { + const uiStateProps = { + ...props.uiState, + sort: { colIndex: 1, direction: 'asc' } as ColumnSort, + }; + const sortedRows = [ + { 'col-0-2': 'Anthony', 'col-1-1': 1 }, + { 'col-0-2': 'Timmy', 'col-1-1': 1 }, + { 'col-0-2': 'Alice', 'col-1-1': 3 }, + ]; + mockGetDataGridColumns.mockReturnValueOnce(dataGridColumnsValue); + shallow(); + expect(getTableVisCellValue).toHaveBeenCalledWith(sortedRows, table.formattedColumns); + expect(getDataGridColumns).toHaveBeenCalledWith(table, props.event, props.uiState.colWidth); + }); + + it('should return formattedTotal from footerCellValue', () => { + let comp = shallow(); + let renderFooterCellValue = comp.find('EuiDataGrid').prop('renderFooterCellValue') as any; + expect(renderFooterCellValue).toEqual(undefined); + comp = shallow(); + renderFooterCellValue = comp.find('EuiDataGrid').prop('renderFooterCellValue'); + expect(renderFooterCellValue({ columnId: 'col-1-1' })).toEqual(5); + expect(renderFooterCellValue({ columnId: 'col-0-2' })).toEqual(null); + }); + + it('should apply pagination correctly', () => { + const comp = shallow(); + const paginationProps = comp.find('EuiDataGrid').prop('pagination'); + expect(paginationProps).toMatchObject({ + pageIndex: 0, + pageSize: 3, + onChangeItemsPerPage: expect.any(Function), + onChangePage: expect.any(Function), + }); + }); + + it('should not call renderFooterCellValue when showTotal is false', () => { + const comp = shallow(); + const renderFooterCellValue = comp.find('EuiDataGrid').prop('renderFooterCellValue'); + expect(renderFooterCellValue).toBeUndefined(); + }); +}); diff --git a/src/plugins/vis_type_table/public/components/table_vis_component.tsx b/src/plugins/vis_type_table/public/components/table_vis_component.tsx index 4576e3420e22..1b16ec170a84 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_component.tsx +++ b/src/plugins/vis_type_table/public/components/table_vis_component.tsx @@ -5,20 +5,20 @@ import React, { useCallback, useMemo } from 'react'; import { orderBy } from 'lodash'; -import dompurify from 'dompurify'; import { EuiDataGridProps, EuiDataGrid, EuiDataGridSorting, EuiTitle } from '@elastic/eui'; import { IInterpreterRenderHandlers } from 'src/plugins/expressions'; -import { Table } from '../table_vis_response_handler'; -import { TableVisConfig, ColumnWidth, ColumnSort, TableUiState } from '../types'; +import { FormattedTableContext } from '../table_vis_response_handler'; +import { TableVisConfig, ColumnSort } from '../types'; import { getDataGridColumns } from './table_vis_grid_columns'; +import { getTableVisCellValue } from './table_vis_cell'; import { usePagination } from '../utils'; -import { convertToFormattedData } from '../utils/convert_to_formatted_data'; import { TableVisControl } from './table_vis_control'; +import { TableUiState } from '../utils'; interface TableVisComponentProps { title?: string; - table: Table; + table: FormattedTableContext; visConfig: TableVisConfig; event: IInterpreterRenderHandlers['event']; uiState: TableUiState; @@ -29,52 +29,44 @@ export const TableVisComponent = ({ table, visConfig, event, - uiState, + uiState: { sort, setSort, colWidth, setWidth }, }: TableVisComponentProps) => { - const { formattedRows: rows, formattedColumns: columns } = convertToFormattedData( - table, - visConfig - ); + const { rows, formattedColumns } = table; const pagination = usePagination(visConfig, rows.length); const sortedRows = useMemo(() => { - return uiState.sort.colIndex !== null && - columns[uiState.sort.colIndex].id && - uiState.sort.direction - ? orderBy(rows, columns[uiState.sort.colIndex].id, uiState.sort.direction) - : rows; - }, [columns, rows, uiState]); + const sortColumnId = + sort.colIndex !== null && sort.colIndex !== undefined + ? formattedColumns[sort.colIndex]?.id + : undefined; + + if (sortColumnId && sort.direction) { + return orderBy(rows, sortColumnId, sort.direction); + } else { + return rows; + } + }, [formattedColumns, rows, sort]); - const renderCellValue = useMemo(() => { - return (({ rowIndex, columnId }) => { - const rawContent = sortedRows[rowIndex][columnId]; - const colIndex = columns.findIndex((col) => col.id === columnId); - const htmlContent = columns[colIndex].formatter.convert(rawContent, 'html'); - const formattedContent = ( - /* - * Justification for dangerouslySetInnerHTML: - * This is one of the visualizations which makes use of the HTML field formatters. - * Since these formatters produce raw HTML, this visualization needs to be able to render them as-is, relying - * on the field formatter to only produce safe HTML. - * `htmlContent` is created by converting raw data via HTML field formatter, so we need to make sure this value never contains - * any unsafe HTML (e.g. by bypassing the field formatter). - */ -
// eslint-disable-line react/no-danger - ); - return sortedRows.hasOwnProperty(rowIndex) ? formattedContent || null : null; - }) as EuiDataGridProps['renderCellValue']; - }, [sortedRows, columns]); + const renderCellValue = useMemo(() => getTableVisCellValue(sortedRows, formattedColumns), [ + sortedRows, + formattedColumns, + ]); - const dataGridColumns = getDataGridColumns(sortedRows, columns, table, event, uiState.width); + const dataGridColumns = getDataGridColumns(table, event, colWidth); const sortedColumns = useMemo(() => { - return uiState.sort.colIndex !== null && - dataGridColumns[uiState.sort.colIndex].id && - uiState.sort.direction - ? [{ id: dataGridColumns[uiState.sort.colIndex].id, direction: uiState.sort.direction }] - : []; - }, [dataGridColumns, uiState]); + if ( + sort.colIndex !== null && + sort.colIndex !== undefined && + dataGridColumns[sort.colIndex].id && + sort.direction + ) { + return [{ id: dataGridColumns[sort.colIndex].id, direction: sort.direction }]; + } else { + return []; + } + }, [dataGridColumns, sort]); const onSort = useCallback( (sortingCols: EuiDataGridSorting['columns'] | []) => { @@ -85,47 +77,34 @@ export const TableVisComponent = ({ colIndex: dataGridColumns.findIndex((col) => col.id === nextSortValue?.id), direction: nextSortValue.direction, } - : { - colIndex: null, - direction: null, - }; - uiState.setSort(nextSort); + : []; + setSort(nextSort); return nextSort; }, - [dataGridColumns, uiState] + [dataGridColumns, setSort] ); const onColumnResize: EuiDataGridProps['onColumnResize'] = useCallback( - ({ columnId, width }) => { - const curWidth: ColumnWidth[] = uiState.width; - const nextWidth = [...curWidth]; - const nextColIndex = columns.findIndex((col) => col.id === columnId); - const curColIndex = curWidth.findIndex((col) => col.colIndex === nextColIndex); - const nextColWidth = { colIndex: nextColIndex, width }; - - // if updated column index is not found, then add it to nextWidth - // else reset it in nextWidth - if (curColIndex < 0) nextWidth.push(nextColWidth); - else nextWidth[curColIndex] = nextColWidth; - - // update uiState.width - uiState.setWidth(nextWidth); + ({ columnId, width }: { columnId: string; width: number }) => { + const colIndex = formattedColumns.findIndex((col) => col.id === columnId); + // update width in uiState + setWidth({ colIndex, width }); }, - [columns, uiState] + [formattedColumns, setWidth] ); const ariaLabel = title || visConfig.title || 'tableVis'; const footerCellValue = visConfig.showTotal ? ({ columnId }: { columnId: any }) => { - return columns.find((col) => col.id === columnId)?.formattedTotal || null; + return formattedColumns.find((col) => col.id === columnId)?.formattedTotal || null; } : undefined; return ( <> {title && ( - +

{title}

)} @@ -133,7 +112,7 @@ export const TableVisComponent = ({ aria-label={ariaLabel} columns={dataGridColumns} columnVisibility={{ - visibleColumns: columns.map(({ id }) => id), + visibleColumns: formattedColumns.map(({ id }) => id), setVisibleColumns: () => {}, }} rowCount={rows.length} @@ -153,7 +132,11 @@ export const TableVisComponent = ({ showFullScreenSelector: false, showStyleSelector: false, additionalControls: ( - + ), }} /> diff --git a/src/plugins/vis_type_table/public/components/table_vis_component_group.test.tsx b/src/plugins/vis_type_table/public/components/table_vis_component_group.test.tsx new file mode 100644 index 000000000000..570c8c0b853b --- /dev/null +++ b/src/plugins/vis_type_table/public/components/table_vis_component_group.test.tsx @@ -0,0 +1,67 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import React from 'react'; +import { render } from '@testing-library/react'; +import { TableVisComponentGroup } from './table_vis_component_group'; +import { TableVisConfig, ColumnSort } from '../types'; +import { Table, TableGroup } from '../table_vis_response_handler'; + +jest.mock('./table_vis_component', () => ({ + TableVisComponent: () =>
TableVisComponent
, +})); + +const table1 = { + table: { + columns: [], + rows: [], + formattedColumns: [], + } as Table, + title: '', +} as TableGroup; + +const table2 = { + table: { + columns: [], + rows: [], + formattedColumns: [], + } as Table, + title: '', +} as TableGroup; + +const tableUiStateMock = { + sort: { colIndex: undefined, direction: undefined } as ColumnSort, + setSort: jest.fn(), + width: [], + setWidth: jest.fn(), +}; + +describe('TableVisApp', () => { + it('should not render table or table group components if no table', () => { + const { container, queryAllByText } = render( + + ); + expect(queryAllByText('TableVisComponent')).toHaveLength(0); + expect(container.outerHTML.includes('visTable__group')).toBe(false); + }); + + it('should render table component 2 times', () => { + const { container, queryAllByText } = render( + + ); + expect(queryAllByText('TableVisComponent')).toHaveLength(2); + expect(container.outerHTML.includes('visTable__group')).toBe(true); + }); +}); diff --git a/src/plugins/vis_type_table/public/components/table_vis_component_group.tsx b/src/plugins/vis_type_table/public/components/table_vis_component_group.tsx index 633b9d2230bd..af8fd8048cbc 100644 --- a/src/plugins/vis_type_table/public/components/table_vis_component_group.tsx +++ b/src/plugins/vis_type_table/public/components/table_vis_component_group.tsx @@ -7,8 +7,9 @@ import React, { memo } from 'react'; import { IInterpreterRenderHandlers } from 'src/plugins/expressions'; import { TableGroup } from '../table_vis_response_handler'; -import { TableVisConfig, TableUiState } from '../types'; +import { TableVisConfig } from '../types'; import { TableVisComponent } from './table_vis_component'; +import { TableUiState } from '../utils'; interface TableVisGroupComponentProps { tableGroups: TableGroup[]; @@ -21,11 +22,11 @@ export const TableVisComponentGroup = memo( ({ tableGroups, visConfig, event, uiState }: TableVisGroupComponentProps) => { return ( <> - {tableGroups.map(({ tables, title }) => ( + {tableGroups.map(({ table, title }) => (
{ const filterBucket = (rowIndex: number, columnIndex: number, negate: boolean) => { - const foramttedColumnId = cols[columnIndex].id; - const rawColumnIndex = table.columns.findIndex((col) => col.id === foramttedColumnId); event({ name: 'filterBucket', data: { @@ -28,10 +23,10 @@ export const getDataGridColumns = ( { table: { columns: table.columns, - rows, + rows: table.rows, }, row: rowIndex, - column: rawColumnIndex, + column: columnIndex, }, ], negate, @@ -39,11 +34,11 @@ export const getDataGridColumns = ( }); }; - return cols.map((col, colIndex) => { + return table.formattedColumns.map((col, colIndex) => { const cellActions = col.filterable ? [ ({ rowIndex, columnId, Component, closePopover }: EuiDataGridColumnCellActionProps) => { - const filterValue = rows[rowIndex][columnId]; + const filterValue = table.rows[rowIndex][columnId]; const filterContent = col.formatter?.convert(filterValue); const filterForValueText = i18n.translate( @@ -79,7 +74,7 @@ export const getDataGridColumns = ( ); }, ({ rowIndex, columnId, Component, closePopover }: EuiDataGridColumnCellActionProps) => { - const filterValue = rows[rowIndex][columnId]; + const filterValue = table.rows[rowIndex][columnId]; const filterContent = col.formatter?.convert(filterValue); const filterOutValueText = i18n.translate( diff --git a/src/plugins/vis_type_table/public/table_vis_fn.ts b/src/plugins/vis_type_table/public/table_vis_fn.ts index 4f0fb2c0ba1f..556cfaf24e00 100644 --- a/src/plugins/vis_type_table/public/table_vis_fn.ts +++ b/src/plugins/vis_type_table/public/table_vis_fn.ts @@ -4,7 +4,7 @@ */ import { i18n } from '@osd/i18n'; -import { tableVisResponseHandler, TableContext } from './table_vis_response_handler'; +import { tableVisResponseHandler, TableVisData } from './table_vis_response_handler'; import { ExpressionFunctionDefinition, OpenSearchDashboardsDatatable, @@ -19,7 +19,7 @@ interface Arguments { } export interface TableVisRenderValue { - visData: TableContext; + visData: TableVisData; visType: 'table'; visConfig: TableVisConfig; } diff --git a/src/plugins/vis_type_table/public/table_vis_renderer.test.tsx b/src/plugins/vis_type_table/public/table_vis_renderer.test.tsx new file mode 100644 index 000000000000..ee18bcfaf734 --- /dev/null +++ b/src/plugins/vis_type_table/public/table_vis_renderer.test.tsx @@ -0,0 +1,78 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { unmountComponentAtNode } from 'react-dom'; +import { act } from '@testing-library/react'; + +import { CoreStart } from 'opensearch-dashboards/public'; +import { getTableVisRenderer } from './table_vis_renderer'; +import { TableVisData } from './table_vis_response_handler'; +import { TableVisConfig } from './types'; +import { TableVisRenderValue } from './table_vis_fn'; + +const mockVisData = { + tableGroups: [], + direction: 'row', +} as TableVisData; + +const mockVisConfig = { + title: 'My Table', + metrics: [] as any, + buckets: [] as any, +} as TableVisConfig; + +const mockHandlers = { + done: jest.fn(), + reload: jest.fn(), + update: jest.fn(), + event: jest.fn(), + onDestroy: jest.fn(), +}; + +const mockCoreStart = {} as CoreStart; + +describe('getTableVisRenderer', () => { + let container: any = null; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + }); + + afterEach(() => { + unmountComponentAtNode(container); + container.remove(); + container = null; + }); + + it('should render table visualization', async () => { + const renderer = getTableVisRenderer(mockCoreStart); + const mockTableVisRenderValue = { + visData: mockVisData, + visType: 'table', + visConfig: mockVisConfig, + } as TableVisRenderValue; + await act(async () => { + renderer.render(container, mockTableVisRenderValue, mockHandlers); + }); + expect(container.querySelector('.tableVis')).toBeTruthy(); + }); + + it('should destroy table on unmount', async () => { + const renderer = getTableVisRenderer(mockCoreStart); + const mockTableVisRenderValue = { + visData: mockVisData, + visType: 'table', + visConfig: mockVisConfig, + } as TableVisRenderValue; + await act(async () => { + renderer.render(container, mockTableVisRenderValue, mockHandlers); + }); + await act(async () => { + unmountComponentAtNode(container); + }); + expect(mockHandlers.onDestroy).toHaveBeenCalled(); + }); +}); diff --git a/src/plugins/vis_type_table/public/table_vis_response_handler.test.ts b/src/plugins/vis_type_table/public/table_vis_response_handler.test.ts new file mode 100644 index 000000000000..89627854b449 --- /dev/null +++ b/src/plugins/vis_type_table/public/table_vis_response_handler.test.ts @@ -0,0 +1,157 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { tableVisResponseHandler } from './table_vis_response_handler'; + +jest.mock('./services', () => { + const formatService = { + deserialize: jest.fn(() => ({ + convert: jest.fn((value) => value), + })), + }; + + return { + getFormatService: () => formatService, + }; +}); + +const createTableGroup = (title, rows) => ({ + title, + table: { + columns: [ + { id: 'col-0', meta: { type: 'string' }, name: 'Column 1' }, + { id: 'col-1', meta: { type: 'number' }, name: 'Column 2' }, + ], + formattedColumns: [ + { + id: 'col-0', + title: 'Column 1', + formatter: { convert: expect.any(Function) }, + filterable: true, + }, + { + id: 'col-1', + title: 'Column 2', + formatter: { convert: expect.any(Function) }, + filterable: false, + }, + ], + rows, + }, +}); + +describe('tableVisResponseHandler', () => { + const input = { + type: 'datatable', + columns: [ + { id: 'col-0', name: 'Column 1', meta: { type: 'string' } }, + { id: 'col-1', name: 'Column 2', meta: { type: 'number' } }, + ], + rows: [ + { 'col-0': 'Group 1', 'col-1': 100 }, + { 'col-0': 'Group 2', 'col-1': 200 }, + ], + }; + + const baseVisConfig = { + title: 'My Table', + buckets: [ + { + accessor: 0, + label: 'Column 1', + format: { + id: 'string', + params: {}, + }, + params: {}, + aggType: 'terms', + }, + ], + metrics: [ + { + accessor: 1, + label: 'Count', + format: { + id: 'number', + }, + params: {}, + aggType: 'count', + }, + ], + }; + + const splitConfig = { + accessor: 0, + label: 'Column 1', + format: { + id: 'string', + params: {}, + }, + params: {}, + aggType: 'terms', + }; + + it('should correctly format data with splitRow', () => { + const visConfig = { ...baseVisConfig, splitRow: [splitConfig] }; + + const expected = { + table: undefined, + tableGroups: [ + createTableGroup('Group 1: Column 1', [{ 'col-0': 'Group 1', 'col-1': 100 }]), + createTableGroup('Group 2: Column 1', [{ 'col-0': 'Group 2', 'col-1': 200 }]), + ], + direction: 'row', + }; + + const result = tableVisResponseHandler(input, visConfig); + expect(result).toEqual(expected); + }); + + it('should correctly format data with splitColumn', () => { + const visConfig = { ...baseVisConfig, splitColumn: [splitConfig] }; + + const expected = { + table: undefined, + tableGroups: [ + createTableGroup('Group 1: Column 1', [{ 'col-0': 'Group 1', 'col-1': 100 }]), + createTableGroup('Group 2: Column 1', [{ 'col-0': 'Group 2', 'col-1': 200 }]), + ], + direction: 'column', + }; + + const result = tableVisResponseHandler(input, visConfig); + expect(result).toEqual(expected); + }); + + it('should correctly format data with no split', () => { + const visConfig = baseVisConfig; + + const expected = { + table: { + columns: input.columns, + formattedColumns: [ + { + id: 'col-0', + title: 'Column 1', + formatter: { convert: expect.any(Function) }, + filterable: true, + }, + { + id: 'col-1', + title: 'Column 2', + formatter: { convert: expect.any(Function) }, + filterable: false, + }, + ], + rows: input.rows, + }, + tableGroups: [], + direction: undefined, + }; + + const result = tableVisResponseHandler(input, visConfig); + expect(result).toEqual(expected); + }); +}); diff --git a/src/plugins/vis_type_table/public/table_vis_response_handler.ts b/src/plugins/vis_type_table/public/table_vis_response_handler.ts index b1d41edfff8b..975038c4c11f 100644 --- a/src/plugins/vis_type_table/public/table_vis_response_handler.ts +++ b/src/plugins/vis_type_table/public/table_vis_response_handler.ts @@ -30,25 +30,25 @@ import { getFormatService } from './services'; import { OpenSearchDashboardsDatatable } from '../../expressions/public'; -import { TableVisConfig } from './types'; +import { FormattedColumn, TableVisConfig } from './types'; +import { convertToFormattedData } from './utils/convert_to_formatted_data'; export interface Table { columns: OpenSearchDashboardsDatatable['columns']; rows: OpenSearchDashboardsDatatable['rows']; } +export interface FormattedTableContext extends Table { + formattedColumns: FormattedColumn[]; +} + export interface TableGroup { - table: OpenSearchDashboardsDatatable; - tables: Table[]; + table: FormattedTableContext; title: string; - name: string; - key: any; - column: number; - row: number; } -export interface TableContext { - table?: Table; +export interface TableVisData { + table?: FormattedTableContext; tableGroups: TableGroup[]; direction?: 'row' | 'column'; } @@ -56,10 +56,10 @@ export interface TableContext { export function tableVisResponseHandler( input: OpenSearchDashboardsDatatable, config: TableVisConfig -): TableContext { - let table: Table | undefined; +): TableVisData { + let table: FormattedTableContext | undefined; const tableGroups: TableGroup[] = []; - let direction: TableContext['direction']; + let direction: TableVisData['direction']; const split = config.splitColumn || config.splitRow; @@ -78,30 +78,32 @@ export function tableVisResponseHandler( (splitMap as any)[splitValue] = splitIndex++; const tableGroup: TableGroup = { title: `${splitColumnFormatter.convert(splitValue)}: ${splitColumn.name}`, - name: splitColumn.name, - key: splitValue, - column: splitColumnIndex, - row: rowIndex, - table: input, - tables: [], + table: { + formattedColumns: [], + rows: [], + columns: input.columns, + }, }; - tableGroup.tables.push({ - columns: input.columns, - rows: [], - }); - tableGroups.push(tableGroup); } - const tableIndex = (splitMap as any)[splitValue]; - (tableGroups[tableIndex] as any).tables[0].rows.push(row); + const tableIndex = splitMap[splitValue]; + tableGroups[tableIndex].table.rows.push(row); + }); + + // format tables + tableGroups.forEach((tableGroup) => { + tableGroup.table = convertToFormattedData(tableGroup.table, config); }); } else { - table = { - columns: input.columns, - rows: input.rows, - }; + table = convertToFormattedData( + { + columns: input.columns, + rows: input.rows, + }, + config + ); } return { diff --git a/src/plugins/vis_type_table/public/types.ts b/src/plugins/vis_type_table/public/types.ts index 814a86f5ac69..a14767f96302 100644 --- a/src/plugins/vis_type_table/public/types.ts +++ b/src/plugins/vis_type_table/public/types.ts @@ -75,10 +75,3 @@ export interface ColumnSort { colIndex?: number; direction?: 'asc' | 'desc'; } - -export interface TableUiState { - sort: ColumnSort; - setSort: (sort: ColumnSort) => void; - width: ColumnWidth[]; - setWidth: (columnWidths: ColumnWidth[]) => void; -} diff --git a/src/plugins/vis_type_table/public/utils/__snapshots__/add_percentage_col.test.ts.snap b/src/plugins/vis_type_table/public/utils/__snapshots__/add_percentage_col.test.ts.snap new file mode 100644 index 000000000000..41f4f24ecffb --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/__snapshots__/add_percentage_col.test.ts.snap @@ -0,0 +1,170 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`addPercentageCol should add new percentage column 1`] = ` +Object { + "cols": Array [ + Object { + "filterable": true, + "formatter": Object {}, + "id": "col-0-2", + "title": "name.keyword: Descending", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1", + "sumTotal": 5, + "title": "Count", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1-percents", + "title": "count percentages", + }, + ], + "rows": Array [ + Object { + "col-0-2": "Alice", + "col-1-1": 3, + "col-1-1-percents": 0.6, + }, + Object { + "col-0-2": "Anthony", + "col-1-1": 1, + "col-1-1-percents": 0.2, + }, + Object { + "col-0-2": "Timmy", + "col-1-1": 1, + "col-1-1-percents": 0.2, + }, + ], +} +`; + +exports[`addPercentageCol should handle empty input data 1`] = ` +Object { + "cols": Array [], + "rows": Array [], +} +`; + +exports[`addPercentageCol should handle input data with null values 1`] = ` +Object { + "cols": Array [ + Object { + "filterable": true, + "formatter": Object {}, + "id": "col-0-2", + "title": "name.keyword: Descending", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1", + "sumTotal": 5, + "title": "Count", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1-percents", + "title": "count percentages", + }, + ], + "rows": Array [ + Object { + "col-0-2": "Alice", + "col-1-1": null, + "col-1-1-percents": 0, + }, + Object { + "col-0-2": "Anthony", + "col-1-1": null, + "col-1-1-percents": 0, + }, + Object { + "col-0-2": "Timmy", + "col-1-1": null, + "col-1-1-percents": 0, + }, + ], +} +`; + +exports[`addPercentageCol should handle input data with one row 1`] = ` +Object { + "cols": Array [ + Object { + "filterable": true, + "formatter": Object {}, + "id": "col-0-2", + "title": "name.keyword: Descending", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1", + "sumTotal": 5, + "title": "Count", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1-percents", + "title": "count percentages", + }, + ], + "rows": Array [ + Object { + "col-0-2": "Alice", + "col-1-1": 3, + "col-1-1-percents": 0.6, + }, + ], +} +`; + +exports[`addPercentageCol should handle sumTotal being 0 1`] = ` +Object { + "cols": Array [ + Object { + "filterable": true, + "formatter": Object {}, + "id": "col-0-2", + "title": "name.keyword: Descending", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1", + "sumTotal": 0, + "title": "Count", + }, + Object { + "filterable": false, + "formatter": Object {}, + "id": "col-1-1-percents", + "title": "count percentages", + }, + ], + "rows": Array [ + Object { + "col-0-2": "Alice", + "col-1-1": 3, + "col-1-1-percents": 0, + }, + Object { + "col-0-2": "Anthony", + "col-1-1": 1, + "col-1-1-percents": 0, + }, + Object { + "col-0-2": "Timmy", + "col-1-1": 1, + "col-1-1-percents": 0, + }, + ], +} +`; diff --git a/src/plugins/vis_type_table/public/utils/add_percentage_col.test.ts b/src/plugins/vis_type_table/public/utils/add_percentage_col.test.ts new file mode 100644 index 000000000000..5f27cb5e49ea --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/add_percentage_col.test.ts @@ -0,0 +1,76 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { addPercentageCol } from './add_percentage_col'; +import { FormattedColumn } from '../types'; +import { Table } from '../table_vis_response_handler'; + +const mockDeserialize = jest.fn(() => ({})); +jest.mock('../services', () => ({ + getFormatService: jest.fn(() => ({ + deserialize: mockDeserialize, + })), +})); + +let formattedColumns: FormattedColumn[]; +const rows = [ + { 'col-0-2': 'Alice', 'col-1-1': 3 }, + { 'col-0-2': 'Anthony', 'col-1-1': 1 }, + { 'col-0-2': 'Timmy', 'col-1-1': 1 }, +] as Table['rows']; + +beforeEach(() => { + formattedColumns = [ + { + id: 'col-0-2', + title: 'name.keyword: Descending', + formatter: {}, + filterable: true, + }, + { + id: 'col-1-1', + title: 'Count', + formatter: {}, + filterable: false, + sumTotal: 5, + }, + ] as FormattedColumn[]; +}); + +describe('addPercentageCol', () => { + it('should add new percentage column', () => { + const result = addPercentageCol(formattedColumns, 'count', rows, 1); + expect(result).toMatchSnapshot(); + }); + + it('should handle sumTotal being 0', () => { + formattedColumns[1].sumTotal = 0; + const result = addPercentageCol(formattedColumns, 'count', rows, 1); + expect(result).toMatchSnapshot(); + }); + + it('should handle empty input data', () => { + const emptyFormattedColumns: FormattedColumn[] = []; + const emptyRows: Table['rows'] = []; + const result = addPercentageCol(emptyFormattedColumns, 'count', emptyRows, 1); + expect(result).toMatchSnapshot(); + }); + + it('should handle input data with one row', () => { + const oneRow = [{ 'col-0-2': 'Alice', 'col-1-1': 3 }] as Table['rows']; + const result = addPercentageCol(formattedColumns, 'count', oneRow, 1); + expect(result).toMatchSnapshot(); + }); + + it('should handle input data with null values', () => { + const nullValueRows = [ + { 'col-0-2': 'Alice', 'col-1-1': null }, + { 'col-0-2': 'Anthony', 'col-1-1': null }, + { 'col-0-2': 'Timmy', 'col-1-1': null }, + ] as Table['rows']; + const result = addPercentageCol(formattedColumns, 'count', nullValueRows, 1); + expect(result).toMatchSnapshot(); + }); +}); diff --git a/src/plugins/vis_type_table/public/utils/add_percentage_col.ts b/src/plugins/vis_type_table/public/utils/add_percentage_col.ts new file mode 100644 index 000000000000..8c300f29d06a --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/add_percentage_col.ts @@ -0,0 +1,76 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Any modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { i18n } from '@osd/i18n'; +import { Table } from '../table_vis_response_handler'; +import { getFormatService } from '../services'; +import { FormattedColumn } from '../types'; + +function insert(arr: FormattedColumn[], index: number, col: FormattedColumn) { + const newArray = [...arr]; + newArray.splice(index + 1, 0, col); + return newArray; +} +/** + * @param columns - the formatted columns that will be displayed + * @param title - the title of the column to add to + * @param rows - the row data for the columns + * @param insertAtIndex - the index to insert the percentage column at + * @returns cols and rows for the table to render now included percentage column(s) + */ +export function addPercentageCol( + columns: FormattedColumn[], + title: string, + rows: Table['rows'], + insertAtIndex: number +) { + if (columns.length === 0) { + return { cols: columns, rows }; + } + const { id, sumTotal } = columns[insertAtIndex]; + const newId = `${id}-percents`; + const formatter = getFormatService().deserialize({ id: 'percent' }); + const i18nTitle = i18n.translate('visTypeTable.params.percentageTableColumnName', { + defaultMessage: '{title} percentages', + values: { title }, + }); + const newCols = insert(columns, insertAtIndex, { + title: i18nTitle, + id: newId, + formatter, + filterable: false, + }); + const newRows = rows.map((row) => ({ + [newId]: sumTotal === 0 ? 0 : (row[id] as number) / (sumTotal as number), + ...row, + })); + + return { cols: newCols, rows: newRows }; +} diff --git a/src/plugins/vis_type_table/public/utils/convert_to_csv_data.test.ts b/src/plugins/vis_type_table/public/utils/convert_to_csv_data.test.ts new file mode 100644 index 000000000000..591dbe5454ce --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/convert_to_csv_data.test.ts @@ -0,0 +1,76 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { IUiSettingsClient } from 'opensearch-dashboards/public'; +import { FormattedColumn } from '../types'; +import { toCsv } from './convert_to_csv_data'; +import { IFieldFormat } from 'src/plugins/data/common'; + +const mockConvert = jest.fn((x) => x); +const defaultFormatter = { convert: (x) => mockConvert(x) } as IFieldFormat; + +function implementConvert(nRow: number) { + for (let i = 0; i < nRow; i++) { + mockConvert.mockImplementationOnce((x) => x); + mockConvert.mockImplementationOnce((x) => x); + mockConvert.mockImplementationOnce((x) => { + return parseFloat(x) * 100 + '%'; + }); + } +} + +const columns = [ + { + id: 'col-0-2', + title: 'name.keyword: Descending', + formatter: defaultFormatter, + filterable: true, + }, + { + id: 'col-1-1', + title: 'Count', + formatter: defaultFormatter, + filterable: false, + sumTotal: 5, + formattedTotal: 5, + total: 5, + }, + { + id: 'col-1-1-percents', + title: 'Count percentages', + formatter: defaultFormatter, + filterable: false, + }, +] as FormattedColumn[]; + +const rows = [ + { 'col-1-1-percents': 0.6, 'col-0-2': 'Alice', 'col-1-1': 3 }, + { 'col-1-1-percents': 0.2, 'col-0-2': 'Anthony', 'col-1-1': 1 }, + { 'col-1-1-percents': 0.2, 'col-0-2': 'Timmy', 'col-1-1': 1 }, +]; + +const uiSettings = { + get: (key: string) => { + if (key === 'csv:separator') return ','; + else if (key === 'csv:quoteValues') return true; + }, +} as IUiSettingsClient; + +describe('toCsv', () => { + it('should create csv rows if not formatted', () => { + const result = toCsv(false, { rows, columns, uiSettings }); + expect(result).toEqual( + '"name.keyword: Descending",Count,"Count percentages"\r\nAlice,3,"0.6"\r\nAnthony,1,"0.2"\r\nTimmy,1,"0.2"\r\n' + ); + }); + + it('should create csv rows if formatted', () => { + implementConvert(3); + const result = toCsv(true, { rows, columns, uiSettings }); + expect(result).toEqual( + '"name.keyword: Descending",Count,"Count percentages"\r\nAlice,3,"60%"\r\nAnthony,1,"20%"\r\nTimmy,1,"20%"\r\n' + ); + }); +}); diff --git a/src/plugins/vis_type_table/public/utils/convert_to_csv_data.ts b/src/plugins/vis_type_table/public/utils/convert_to_csv_data.ts index 2c37df1aa3d5..3d4781736689 100644 --- a/src/plugins/vis_type_table/public/utils/convert_to_csv_data.ts +++ b/src/plugins/vis_type_table/public/utils/convert_to_csv_data.ts @@ -46,7 +46,7 @@ interface CSVDataProps { uiSettings: CoreStart['uiSettings']; } -const toCsv = function (formatted: boolean, { rows, columns, uiSettings }: CSVDataProps) { +export const toCsv = function (formatted: boolean, { rows, columns, uiSettings }: CSVDataProps) { const separator = uiSettings.get(CSV_SEPARATOR_SETTING); const quoteValues = uiSettings.get(CSV_QUOTE_VALUES_SETTING); diff --git a/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.test.ts b/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.test.ts new file mode 100644 index 000000000000..34085b70a278 --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.test.ts @@ -0,0 +1,136 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { convertToFormattedData } from './convert_to_formatted_data'; +import { TableVisConfig } from '../types'; +import { Table } from '../table_vis_response_handler'; +import { AggTypes } from '../types'; + +const mockDeserialize = jest.fn(() => ({})); +jest.mock('../services', () => ({ + getFormatService: jest.fn(() => ({ + deserialize: mockDeserialize, + })), +})); + +const table = { + type: 'opensearch_dashboards_datatable', + columns: [ + { id: 'col-0-2', name: 'name.keyword: Descending', meta: { type: 'terms' } }, + { id: 'col-1-1', name: 'Count', meta: { type: 'count' } }, + ], + rows: [ + { 'col-0-2': 'Alice', 'col-1-1': 3 }, + { 'col-0-2': 'Anthony', 'col-1-1': 1 }, + { 'col-0-2': 'Timmy', 'col-1-1': 1 }, + ], +} as Table; + +let visConfig = {} as TableVisConfig; + +function implementDeserialize() { + mockDeserialize.mockImplementationOnce(() => ({})); + mockDeserialize.mockImplementationOnce(() => ({ + allowsNumericalAggregations: true, + convert: jest.fn((x: number) => x), + })); +} + +describe('convertToFormattedData', () => { + beforeEach(() => { + visConfig = { + buckets: [ + { + accessor: 0, + aggType: 'terms', + format: { + id: 'terms', + params: { + id: 'string', + missingBucketLabel: 'Missing', + otherBucketLabel: 'Other', + parsedUrl: { + basePath: '/arf', + origin: '', + pathname: '/arf/app/home', + }, + }, + }, + label: 'name.keyword: Descending', + params: {}, + }, + ], + metrics: [ + { + accessor: 1, + aggType: 'count', + format: { + id: 'number', + }, + label: 'Count', + params: {}, + }, + ], + perPage: 10, + percentageCol: '', + showMetricsAtAllLevels: false, + showPartialRows: false, + showTotal: false, + title: '', + totalFunc: 'sum', + } as TableVisConfig; + }); + + it('should create formatted data', () => { + const result = convertToFormattedData(table, visConfig); + expect(result.rows).toEqual(table.rows); + expect(result.formattedColumns).toEqual([ + { + id: 'col-0-2', + title: 'name.keyword: Descending', + formatter: {}, + filterable: true, + }, + { id: 'col-1-1', title: 'Count', formatter: {}, filterable: false }, + ]); + }); + + describe.each([ + [AggTypes.SUM, 5], + [AggTypes.AVG, 1.6666666666666667], + [AggTypes.MIN, 1], + [AggTypes.MAX, 3], + [AggTypes.COUNT, 3], + ])('with totalFunc as %s', (totalFunc, expectedTotal) => { + beforeEach(() => { + implementDeserialize(); + visConfig.showTotal = true; + visConfig.totalFunc = totalFunc; + }); + + it(`should add ${totalFunc} total`, () => { + const result = convertToFormattedData(table, visConfig); + const expectedFormattedColumns = [ + { + id: 'col-0-2', + title: 'name.keyword: Descending', + formatter: {}, + filterable: true, + ...(totalFunc === AggTypes.COUNT ? { sumTotal: 0, formattedTotal: 3, total: 3 } : {}), + }, + { + id: 'col-1-1', + title: 'Count', + formatter: { allowsNumericalAggregations: true, convert: expect.any(Function) }, + filterable: false, + sumTotal: 5, + formattedTotal: expectedTotal, + total: expectedTotal, + }, + ]; + expect(result.formattedColumns).toEqual(expectedFormattedColumns); + }); + }); +}); diff --git a/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.ts b/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.ts index 2ab67e3b0a67..afb2906af8a1 100644 --- a/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.ts +++ b/src/plugins/vis_type_table/public/utils/convert_to_formatted_data.ts @@ -28,56 +28,20 @@ * under the License. */ -import { i18n } from '@osd/i18n'; import { chain } from 'lodash'; -import { OpenSearchDashboardsDatatableRow } from 'src/plugins/expressions'; +import { + OpenSearchDashboardsDatatableRow, + OpenSearchDashboardsDatatableColumn, +} from 'src/plugins/expressions'; import { Table } from '../table_vis_response_handler'; import { AggTypes, TableVisConfig } from '../types'; import { getFormatService } from '../services'; import { FormattedColumn } from '../types'; - -function insert(arr: FormattedColumn[], index: number, col: FormattedColumn) { - const newArray = [...arr]; - newArray.splice(index + 1, 0, col); - return newArray; -} - -/** - * @param columns - the formatted columns that will be displayed - * @param title - the title of the column to add to - * @param rows - the row data for the columns - * @param insertAtIndex - the index to insert the percentage column at - * @returns cols and rows for the table to render now included percentage column(s) - */ -function addPercentageCol( - columns: FormattedColumn[], - title: string, - rows: Table['rows'], - insertAtIndex: number -) { - const { id, sumTotal } = columns[insertAtIndex]; - const newId = `${id}-percents`; - const formatter = getFormatService().deserialize({ id: 'percent' }); - const i18nTitle = i18n.translate('visTypeTable.params.percentageTableColumnName', { - defaultMessage: '{title} percentages', - values: { title }, - }); - const newCols = insert(columns, insertAtIndex, { - title: i18nTitle, - id: newId, - formatter, - filterable: false, - }); - const newRows = rows.map((row) => ({ - [newId]: (row[id] as number) / (sumTotal as number), - ...row, - })); - - return { cols: newCols, rows: newRows }; -} +import { addPercentageCol } from './add_percentage_col'; export interface FormattedDataProps { - formattedRows: OpenSearchDashboardsDatatableRow[]; + rows: OpenSearchDashboardsDatatableRow[]; + columns: OpenSearchDashboardsDatatableColumn[]; formattedColumns: FormattedColumn[]; } @@ -107,12 +71,15 @@ export const convertToFormattedData = ( const allowsNumericalAggregations = formatter?.allowsNumericalAggregations; if (allowsNumericalAggregations || isDate || visConfig.totalFunc === AggTypes.COUNT) { - const sum = table.rows.reduce((prev, curr) => { - // some metrics return undefined for some of the values - // derivative is an example of this as it returns undefined in the first row - if (curr[col.id] === undefined) return prev; - return prev + (curr[col.id] as number); - }, 0); + // only calculate the sumTotal for numerical columns + const sum = isBucket + ? 0 + : table.rows.reduce((prev, curr) => { + // some metrics return undefined for some of the values + // derivative is an example of this as it returns undefined in the first row + if (curr[col.id] === undefined) return prev; + return prev + (curr[col.id] as number); + }, 0); formattedColumn.sumTotal = sum; switch (visConfig.totalFunc) { @@ -164,7 +131,7 @@ export const convertToFormattedData = ( ); // column to show percentage was removed - if (insertAtIndex < 0) return { formattedRows, formattedColumns }; + if (insertAtIndex < 0) return { rows: table.rows, columns: table.columns, formattedColumns }; const { cols, rows } = addPercentageCol( formattedColumns, @@ -175,5 +142,5 @@ export const convertToFormattedData = ( formattedRows = rows; formattedColumns = cols; } - return { formattedRows, formattedColumns }; + return { rows: formattedRows, columns: table.columns, formattedColumns }; }; diff --git a/src/plugins/vis_type_table/public/utils/get_table_ui_state.test.ts b/src/plugins/vis_type_table/public/utils/get_table_ui_state.test.ts new file mode 100644 index 000000000000..64488d9275eb --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/get_table_ui_state.test.ts @@ -0,0 +1,66 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { PersistedState } from '../../../visualizations/public'; +import { TableUiState, getTableUIState } from './get_table_ui_state'; +import { ColumnWidth, ColumnSort } from '../types'; + +describe('getTableUIState', () => { + let uiState: PersistedState; + let tableUiState: TableUiState; + + beforeEach(() => { + uiState = ({ + get: jest.fn(), + set: jest.fn(), + emit: jest.fn(), + } as unknown) as PersistedState; + tableUiState = getTableUIState(uiState); + }); + + it('should get initial sort and width values from uiState', () => { + const initialSort: ColumnSort = { colIndex: 1, direction: 'asc' }; + const initialWidth: ColumnWidth[] = [{ colIndex: 0, width: 100 }]; + + (uiState.get as jest.Mock).mockImplementation((key: string) => { + if (key === 'vis.sortColumn') return initialSort; + if (key === 'vis.columnsWidth') return initialWidth; + }); + + const newTableUiState = getTableUIState(uiState); + expect(newTableUiState.sort).toEqual(initialSort); + expect(newTableUiState.colWidth).toEqual(initialWidth); + }); + + it('should set and emit sort values', () => { + const newSort: ColumnSort = { colIndex: 2, direction: 'desc' }; + tableUiState.setSort(newSort); + + expect(uiState.set).toHaveBeenCalledWith('vis.sortColumn', newSort); + expect(uiState.emit).toHaveBeenCalledWith('reload'); + }); + + it('should set and emit width values for a new column', () => { + const newWidth: ColumnWidth = { colIndex: 1, width: 150 }; + tableUiState.setWidth(newWidth); + + expect(uiState.set).toHaveBeenCalledWith('vis.columnsWidth', [newWidth]); + expect(uiState.emit).toHaveBeenCalledWith('reload'); + }); + + it('should update and emit width values for an existing column', () => { + const initialWidth: ColumnWidth[] = [{ colIndex: 0, width: 100 }]; + (uiState.get as jest.Mock).mockReturnValue(initialWidth); + + const updatedTableUiState = getTableUIState(uiState); + + const updatedWidth: ColumnWidth = { colIndex: 0, width: 150 }; + updatedTableUiState.setWidth(updatedWidth); + + const expectedWidths = [{ colIndex: 0, width: 150 }]; + expect(uiState.set).toHaveBeenCalledWith('vis.columnsWidth', expectedWidths); + expect(uiState.emit).toHaveBeenCalledWith('reload'); + }); +}); diff --git a/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts b/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts new file mode 100644 index 000000000000..58fc6b472a40 --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/get_table_ui_state.ts @@ -0,0 +1,40 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { PersistedState } from '../../../visualizations/public'; +import { ColumnSort, ColumnWidth } from '../types'; + +export interface TableUiState { + sort: ColumnSort; + setSort: (sort: ColumnSort) => void; + colWidth: ColumnWidth[]; + setWidth: (columnWidths: ColumnWidth) => void; +} + +export function getTableUIState(uiState: PersistedState): TableUiState { + const sort: ColumnSort = uiState.get('vis.sortColumn') || {}; + const colWidth: ColumnWidth[] = uiState.get('vis.columnsWidth') || []; + + const setSort = (newSort: ColumnSort) => { + uiState.set('vis.sortColumn', newSort); + uiState.emit('reload'); + }; + + const setWidth = (columnWidth: ColumnWidth) => { + const nextState = [...colWidth]; + const curColIndex = colWidth.findIndex((col) => col.colIndex === columnWidth.colIndex); + + if (curColIndex < 0) { + nextState.push(columnWidth); + } else { + nextState[curColIndex] = columnWidth; + } + + uiState.set('vis.columnsWidth', nextState); + uiState.emit('reload'); + }; + + return { sort, setSort, colWidth, setWidth }; +} diff --git a/src/plugins/vis_type_table/public/utils/index.ts b/src/plugins/vis_type_table/public/utils/index.ts index 1fd0e3f1e0fd..3277d92efc71 100644 --- a/src/plugins/vis_type_table/public/utils/index.ts +++ b/src/plugins/vis_type_table/public/utils/index.ts @@ -6,3 +6,5 @@ export * from './convert_to_csv_data'; export * from './convert_to_formatted_data'; export * from './use_pagination'; +export * from './add_percentage_col'; +export * from './get_table_ui_state'; diff --git a/src/plugins/vis_type_table/public/utils/use_pagination.test.ts b/src/plugins/vis_type_table/public/utils/use_pagination.test.ts new file mode 100644 index 000000000000..d8e7a02a0799 --- /dev/null +++ b/src/plugins/vis_type_table/public/utils/use_pagination.test.ts @@ -0,0 +1,101 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { act, renderHook } from '@testing-library/react-hooks'; +import { AggTypes, TableVisParams } from '../types'; +import { usePagination } from './use_pagination'; + +describe('usePagination', () => { + const visParams = { + perPage: 10, + showPartialRows: false, + showMetricsAtAllLevels: false, + showTotal: false, + totalFunc: AggTypes.SUM, + percentageCol: '', + } as TableVisParams; + + it('should not set pagination if perPage is empty string', () => { + const params = { + ...visParams, + perPage: '', + }; + const { result } = renderHook(() => usePagination(params, 20)); + expect(result.current).toEqual(undefined); + }); + + it('should init pagination', () => { + const { result } = renderHook(() => usePagination(visParams, 20)); + expect(result.current).toEqual({ + pageIndex: 0, + pageSize: 10, + onChangeItemsPerPage: expect.any(Function), + onChangePage: expect.any(Function), + }); + }); + + it('should init pagination with pageSize as the minimum of perPage and nRow', () => { + const { result } = renderHook(() => usePagination(visParams, 8)); + expect(result.current).toEqual({ + pageIndex: 0, + pageSize: 8, + onChangeItemsPerPage: expect.any(Function), + onChangePage: expect.any(Function), + }); + }); + + it('should set pageSize to the lesser of perPage and nRow when nRow is less than perPage', () => { + const { result } = renderHook(() => usePagination(visParams, 5)); + expect(result.current).toEqual({ + pageIndex: 0, + pageSize: 5, + onChangeItemsPerPage: expect.any(Function), + onChangePage: expect.any(Function), + }); + }); + + it('should set page index via onChangePage', () => { + const { result } = renderHook(() => usePagination(visParams, 50)); + act(() => { + // set page index to 3 + result.current?.onChangePage(3); + }); + expect(result.current?.pageIndex).toEqual(3); + }); + + it('should set to max page index via onChangePage if exceed maxiPageIndex', () => { + const { result, rerender } = renderHook((props) => usePagination(props.visParams, props.nRow), { + initialProps: { + visParams, + nRow: 55, + }, + }); + + act(() => { + // set page index to the last page + result.current?.onChangePage(5); + }); + + rerender({ visParams, nRow: 15 }); + // when the number of rows decreases, page index should + // be set to maxiPageIndex + expect(result.current).toEqual({ + pageIndex: 1, + pageSize: 10, + onChangeItemsPerPage: expect.any(Function), + onChangePage: expect.any(Function), + }); + }); + + it('should pagination via onChangeItemsPerPage', () => { + const { result } = renderHook(() => usePagination(visParams, 20)); + act(() => { + // set page size to 5 + result.current?.onChangeItemsPerPage(5); + }); + + expect(result.current?.pageSize).toEqual(5); + }); +}); diff --git a/src/plugins/vis_type_table/public/utils/use_pagination.ts b/src/plugins/vis_type_table/public/utils/use_pagination.ts index e3993f1c0868..97ecfd6b85e6 100644 --- a/src/plugins/vis_type_table/public/utils/use_pagination.ts +++ b/src/plugins/vis_type_table/public/utils/use_pagination.ts @@ -4,12 +4,12 @@ */ import { useCallback, useEffect, useMemo, useState } from 'react'; -import { TableVisConfig } from '../types'; +import { TableVisParams } from '../types'; -export const usePagination = (visConfig: TableVisConfig, nRow: number) => { +export const usePagination = (visParams: TableVisParams, nRow: number) => { const [pagination, setPagination] = useState({ pageIndex: 0, - pageSize: Math.min(visConfig.perPage || 10, nRow), + pageSize: Math.min(visParams.perPage || 0, nRow), }); const onChangeItemsPerPage = useCallback( (pageSize) => setPagination((p) => ({ ...p, pageSize, pageIndex: 0 })), @@ -20,20 +20,23 @@ export const usePagination = (visConfig: TableVisConfig, nRow: number) => { ]); useEffect(() => { - const perPage = Math.min(visConfig.perPage || 10, nRow); + const perPage = Math.min(visParams.perPage || 0, nRow); const maxiPageIndex = Math.ceil(nRow / perPage) - 1; setPagination((p) => ({ pageIndex: p.pageIndex > maxiPageIndex ? maxiPageIndex : p.pageIndex, pageSize: perPage, })); - }, [nRow, visConfig.perPage]); + }, [nRow, visParams.perPage]); return useMemo( - () => ({ - ...pagination, - onChangeItemsPerPage, - onChangePage, - }), + () => + pagination.pageSize + ? { + ...pagination, + onChangeItemsPerPage, + onChangePage, + } + : undefined, [pagination, onChangeItemsPerPage, onChangePage] ); }; From 14bde2bba9b338183f117a8e852cbd4ce02c0694 Mon Sep 17 00:00:00 2001 From: Ashwin P Chandran Date: Mon, 17 Apr 2023 16:00:28 -0700 Subject: [PATCH 6/7] Adds toast ID to toast api (#3752) * Currently sending the same toast multiple times results in multiple toasts being rendered on the screen. This change allows the toast api to additionally accept an id parameter that * Update changelog * Update src/core/public/notifications/toasts/toasts_api.tsx Issue Resolved: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2643 Signed-off-by: Ashwin P Chandran Co-authored-by: Josh Romero --------- Signed-off-by: Ashwin P Chandran Co-authored-by: Sean Neumann <1413295+seanneumann@users.noreply.github.com> Co-authored-by: Josh Romero --- CHANGELOG.md | 1 + .../notifications/toasts/toasts_api.test.ts | 23 +++++++++++++++++++ .../notifications/toasts/toasts_api.tsx | 14 ++++++++--- .../application/components/workspace.tsx | 6 ++++- .../utils/state_management/store.ts | 4 ++-- src/plugins/vis_builder/public/plugin.ts | 3 ++- 6 files changed, 44 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 001f9248f2b7..92adeabb33a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [Multiple DataSource] Refactor dev tool console to use opensearch-js client to send requests ([#3544](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3544)) - Add `osd-xsrf` header to all requests that incorrectly used `node-version` to satisfy XSRF protection ([#3643](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3643)) - [Data] Add geo shape filter field ([#3605](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3605)) +- [Notifications] Adds id to toast api for deduplication ([#3752](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3752)) - [VisBuilder] Add UI actions handler ([#3732](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3732)) - [Dashboard] Indicate that IE is no longer supported ([#3641](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3641)) - [UI] Add support for comma delimiters in the global filter bar ([#3686](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3686)) diff --git a/src/core/public/notifications/toasts/toasts_api.test.ts b/src/core/public/notifications/toasts/toasts_api.test.ts index ef4f469194d7..dacfa98b623a 100644 --- a/src/core/public/notifications/toasts/toasts_api.test.ts +++ b/src/core/public/notifications/toasts/toasts_api.test.ts @@ -105,6 +105,24 @@ describe('#get$()', () => { toasts.remove('bar'); expect(onToasts).not.toHaveBeenCalled(); }); + + it('does not emit a new toast list when a toast with the same id is passed to add()', () => { + const toasts = new ToastsApi(toastDeps()); + const onToasts = jest.fn(); + + toasts.get$().subscribe(onToasts); + toasts.add({ + id: 'foo', + title: 'foo', + }); + onToasts.mockClear(); + + toasts.add({ + id: 'foo', + title: 'bar', + }); + expect(onToasts).not.toHaveBeenCalled(); + }); }); describe('#add()', () => { @@ -135,6 +153,11 @@ describe('#add()', () => { const toasts = new ToastsApi(toastDeps()); expect(toasts.add('foo')).toHaveProperty('title', 'foo'); }); + + it('accepts an id and does not auto increment', async () => { + const toasts = new ToastsApi(toastDeps()); + expect(toasts.add({ id: 'foo', title: 'not foo' })).toHaveProperty('id', 'foo'); + }); }); describe('#remove()', () => { diff --git a/src/core/public/notifications/toasts/toasts_api.tsx b/src/core/public/notifications/toasts/toasts_api.tsx index 008c4fb3c507..76d0bf9cf9b2 100644 --- a/src/core/public/notifications/toasts/toasts_api.tsx +++ b/src/core/public/notifications/toasts/toasts_api.tsx @@ -42,12 +42,10 @@ import { I18nStart } from '../../i18n'; /** * Allowed fields for {@link ToastInput}. * - * @remarks - * `id` cannot be specified. - * * @public */ export type ToastInputFields = Pick> & { + id?: string; title?: string | MountPoint; text?: string | MountPoint; }; @@ -143,6 +141,16 @@ export class ToastsApi implements IToasts { * @returns a {@link Toast} */ public add(toastOrTitle: ToastInput) { + if (typeof toastOrTitle !== 'string') { + const toastObject = toastOrTitle; + const list = this.toasts$.getValue(); + const existingToast = list.find((toast) => toast.id === toastObject.id); + + if (existingToast) { + return existingToast; + } + } + const toast: Toast = { id: String(this.idCounter++), toastLifeTimeMs: this.uiSettings.get('notifications:lifetime:info'), diff --git a/src/plugins/vis_builder/public/application/components/workspace.tsx b/src/plugins/vis_builder/public/application/components/workspace.tsx index 214a735f6ba8..baccd4faf342 100644 --- a/src/plugins/vis_builder/public/application/components/workspace.tsx +++ b/src/plugins/vis_builder/public/application/components/workspace.tsx @@ -56,7 +56,11 @@ export const WorkspaceUI = () => { const err = schemaValidation.errorMsg || aggValidation.errorMsg; - if (err) toasts.addWarning(err); + if (err) + toasts.addWarning({ + id: 'vb_expression_validation', + title: err, + }); return; } diff --git a/src/plugins/vis_builder/public/application/utils/state_management/store.ts b/src/plugins/vis_builder/public/application/utils/state_management/store.ts index f1b1c0eeae2a..588c221a50fd 100644 --- a/src/plugins/vis_builder/public/application/utils/state_management/store.ts +++ b/src/plugins/vis_builder/public/application/utils/state_management/store.ts @@ -47,9 +47,9 @@ export const getPreloadedStore = async (services: VisBuilderServices) => { }; // the store subscriber will automatically detect changes and call handleChange function - store.subscribe(handleChange); + const unsubscribe = store.subscribe(handleChange); - return store; + return { store, unsubscribe }; }; // Infer the `RootState` and `AppDispatch` types from the store itself diff --git a/src/plugins/vis_builder/public/plugin.ts b/src/plugins/vis_builder/public/plugin.ts index 0c1d569f6bed..5744341ab3cf 100644 --- a/src/plugins/vis_builder/public/plugin.ts +++ b/src/plugins/vis_builder/public/plugin.ts @@ -163,7 +163,7 @@ export class VisBuilderPlugin }; // Instantiate the store - const store = await getPreloadedStore(services); + const { store, unsubscribe: unsubscribeStore } = await getPreloadedStore(services); const unmount = renderApp(params, services, store); // Render the application @@ -171,6 +171,7 @@ export class VisBuilderPlugin unlistenParentHistory(); unmount(); appUnMounted(); + unsubscribeStore(); }; }, }); From 8b0587e7aa683ebe9a25c4a454ddb117c68d7335 Mon Sep 17 00:00:00 2001 From: sayuree <52072402+sayuree@users.noreply.github.com> Date: Tue, 18 Apr 2023 05:11:43 +0600 Subject: [PATCH 7/7] Add tooltip to help icon (#3626) * Add tooltip to help icon Signed-off-by: sabina.zaripova Add tooltip to menu icon Signed-off-by: sabina.zaripova --- CHANGELOG.md | 1 + .../header/__snapshots__/header.test.tsx.snap | 46 +++++++++++++++++++ .../header_help_menu.test.tsx.snap | 40 ++++++++++++++++ src/core/public/chrome/ui/header/header.tsx | 8 +++- .../chrome/ui/header/header_help_menu.tsx | 8 ++++ 5 files changed, 102 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92adeabb33a7..f2c52847d8b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -245,6 +245,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - [TSVB] Fixes undefined serial diff aggregation documentation link ([#3503](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3503)) - [Console] Fix dev tool console autocomplete not loading issue ([#3775](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3775)) - [Console] Fix dev tool console run command with query parameter error ([#3813](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3813)) +- Add clarifying tooltips to header navigation ([#3573](https://github.com/opensearch-project/OpenSearch-Dashboards/issues/3573)) ### 🚞 Infrastructure diff --git a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap index c7e883d9b00b..6486e207ed37 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header.test.tsx.snap @@ -2798,6 +2798,7 @@ exports[`Header handles visibility and lock changes 1`] = ` > @@ -2842,11 +2843,13 @@ exports[`Header handles visibility and lock changes 1`] = ` > @@ -5416,6 +5419,11 @@ exports[`Header handles visibility and lock changes 1`] = ` size="m" type="questionInCircle" /> + } closePopover={[Function]} @@ -5467,6 +5475,10 @@ exports[`Header handles visibility and lock changes 1`] = ` + @@ -5514,6 +5526,17 @@ exports[`Header handles visibility and lock changes 1`] = ` size="m" /> + + + @@ -8117,6 +8140,7 @@ exports[`Header renders condensed header 1`] = ` > @@ -8161,11 +8185,13 @@ exports[`Header renders condensed header 1`] = ` > @@ -10652,6 +10678,11 @@ exports[`Header renders condensed header 1`] = ` size="m" type="questionInCircle" /> + } closePopover={[Function]} @@ -10703,6 +10734,10 @@ exports[`Header renders condensed header 1`] = ` + @@ -10750,6 +10785,17 @@ exports[`Header renders condensed header 1`] = ` size="m" /> + + + diff --git a/src/core/public/chrome/ui/header/__snapshots__/header_help_menu.test.tsx.snap b/src/core/public/chrome/ui/header/__snapshots__/header_help_menu.test.tsx.snap index 835c2d8d4a4e..5406fbf6abaa 100644 --- a/src/core/public/chrome/ui/header/__snapshots__/header_help_menu.test.tsx.snap +++ b/src/core/public/chrome/ui/header/__snapshots__/header_help_menu.test.tsx.snap @@ -1626,6 +1626,11 @@ exports[`Header help menu hides survey link 1`] = ` size="m" type="questionInCircle" /> + } closePopover={[Function]} @@ -1677,6 +1682,10 @@ exports[`Header help menu hides survey link 1`] = ` + @@ -1724,6 +1733,17 @@ exports[`Header help menu hides survey link 1`] = ` size="m" /> + + + @@ -3782,6 +3802,11 @@ exports[`Header help menu renders survey link 1`] = ` size="m" type="questionInCircle" /> + } closePopover={[Function]} @@ -3833,6 +3858,10 @@ exports[`Header help menu renders survey link 1`] = ` + @@ -3880,6 +3909,17 @@ exports[`Header help menu renders survey link 1`] = ` size="m" /> + + + diff --git a/src/core/public/chrome/ui/header/header.tsx b/src/core/public/chrome/ui/header/header.tsx index 917d1ebd4c46..a78371f4f264 100644 --- a/src/core/public/chrome/ui/header/header.tsx +++ b/src/core/public/chrome/ui/header/header.tsx @@ -173,7 +173,13 @@ export function Header({ aria-controls={navId} ref={toggleCollapsibleNavRef} > - + diff --git a/src/core/public/chrome/ui/header/header_help_menu.tsx b/src/core/public/chrome/ui/header/header_help_menu.tsx index 6f536184edc1..f0be769937f5 100644 --- a/src/core/public/chrome/ui/header/header_help_menu.tsx +++ b/src/core/public/chrome/ui/header/header_help_menu.tsx @@ -331,6 +331,14 @@ class HeaderHelpMenuUI extends Component { onClick={this.onMenuButtonClick} > + );