[Data Explorer] Initial commit #4292

ashwin-pc · 2023-06-15T19:31:06Z

Description

Moves discover to discover_legacy
Adds toggle for discover legacy
Adds initial data explorer plugin code and routing for discover 2.0
Adds initial view service for data explorer

Issues Resolved

closes #4217
closes #4218
closes #4219
closes #4220
closes #4223

Screenshot

Screen.Recording.2023-06-15.at.12.27.22.PM.mov

Testing the changes

Click on the discover tab with both the advanced settings turned on and off for discover legacy. All discover routes should work when discover legacy is turned on.

Check List

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

codecov · 2023-06-15T21:43:56Z

Codecov Report

❗ No coverage uploaded for pull request base (feature/data-explorer@487cd36). Click here to learn what that means.
The diff coverage is n/a.

@@                   Coverage Diff                    @@
##             feature/data-explorer    #4292   +/-   ##
========================================================
  Coverage                         ?   66.55%           
========================================================
  Files                            ?     3278           
  Lines                            ?    62654           
  Branches                         ?     9760           
========================================================
  Hits                             ?    41702           
  Misses                           ?    18587           
  Partials                         ?     2365

Flag	Coverage Δ
Linux	`66.50% <0.00%> (?)`
Windows	`66.50% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

AMoo-Miki

Am i correct to assume everything under discover_legacy is just renamed discover with possibly some reference changes?

AMoo-Miki · 2023-06-21T05:19:24Z

src/plugins/data_explorer/README.md

@@ -0,0 +1,11 @@
+# dataExplorer
+
+A OpenSearch Dashboards plugin


Suggested change

A OpenSearch Dashboards plugin

An OpenSearch Dashboards plugin to ... ???

Most of these are placeholders for now. Just want to setup the initial framework

AMoo-Miki · 2023-06-21T05:21:43Z

src/plugins/data_explorer/public/components/sidebar.tsx

+        <EuiComboBox
+          placeholder="Select a datasource"
+          singleSelection={{ asPlainText: true }}
+          options={[
+            {
+              label: 'Select a datasource',


Can we i18nize these?

The whole component here will be replaced. This is just a placeholder for now

AMoo-Miki · 2023-06-21T05:23:07Z

src/plugins/data_explorer/public/index.scss

+$osdHeaderOffset: $euiHeaderHeightCompensation;
+
+.dePageTemplate {
+  height: calc(100vh - #{$osdHeaderOffset});


Does this take into account the expanded header being true or false?

Yes it does. This is how VisBuilder does it

AMoo-Miki · 2023-06-21T05:32:31Z

src/plugins/data_explorer/public/services/view_service/view_service.ts

+
+  private registerView(viewDefinition: View) {
+    if (this.views[viewDefinition.id]) {
+      throw new Error(`A view with this the id ${viewDefinition.id} already exists!`);


Should we wrap the id in "", `` or []?

throw new Error(`A view with this the id "${viewDefinition.id}" already exists!`);

We could, this is never a user facing error and mostly copied over from a similar service in the other apps. no strong opinions here :)

AMoo-Miki · 2023-06-21T05:39:52Z

src/plugins/data_explorer/public/services/view_service/view_service.ts

+       * returns all registered Views
+       */
+      all: (): View[] => {
+        return [...Object.values(this.views)];


Why would we need to spread this?

good question, I dont remember why i did this. Again something i just referenced from a similar service in VisBuilder.

AMoo-Miki · 2023-06-21T05:48:18Z

src/plugins/discover/public/application/view_components/canvas/create_canvas.tsx

+import React from 'react';
+
+export const createCanvas = () => {
+  return <div>Test Canvas</div>;


What do we expect to place instead of this Test? Does it need a ToDo: or will this stay as is?

Again, this is a placeholder. Will go away before the merge to main.

AMoo-Miki · 2023-06-21T05:48:37Z

src/plugins/discover/public/application/view_components/panel/create_panel.tsx

+import React from 'react';
+
+export const createPanel = () => {
+  return <div>Test Panel</div>;


Is this something we plan on changing?

Yes, will go away soon. It is a placeholder

ashwin-pc · 2023-06-22T21:53:48Z

Am i correct to assume everything under discover_legacy is just renamed discover with possibly some reference changes?

Mostly yes, the server component is not moved over from discover since we dont have anything react dependent there. But otherwise yes, its just a reference change.

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

ananzh

have played with it for table vis
think this is good to merge in as an initial starting point

kavilla · 2023-06-27T16:36:29Z

src/plugins/data_explorer/public/services/view_service/view_service.ts

+ */
+
+/*
+ * Licensed to Elasticsearch B.V. under one or more contributor


do we still need this one?

Ah good catch, ill update them in a followup PR

kavilla · 2023-06-27T16:45:22Z

src/plugins/discover/server/ui_settings.ts

@@ -47,6 +48,17 @@ import {
 } from '../common';

 export const uiSettings: Record<string, UiSettingsParams> = {
+  [DISCOVER_LEGACY_TOGGLE]: {
+    name: i18n.translate('discover.advancedSettings.legacyToggleTitle', {
+      defaultMessage: 'Disable new discover app',


nit: should we change this to be Disable legacy discover app? Or is there an action item to change the logic above so that it will route to the new discover app on true?

The default should be the new app but i set it to the legacy to see if the CI would pass. Looks like the legacy path is causing a problem so i will revert the change

kavilla

I like it. Seems relatively safe to get into main?

I know historically, the legacy suffix has been used in the process of migrating stuff? To which i guess can cause some changes throughout the app if we want full support (for example the routing). I wonder if we want to consider the next suffix to have the new code and a new route and then once we fully adopt it then we either delete or add the legacy suffix if it is still needed.

ashwin-pc · 2023-06-28T17:54:06Z

I want to fix the functional tests before going into main, so for now ill merge it into the feature branch for now :)

ashwin-pc added 6 commits June 13, 2023 10:59

create legacy plugin and new data explorer plugin

0ec844b

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

Adds license header

f423f72

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

adds legacy toggle

5547f2c

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

fixes nav link

8248d49

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

working data explorer routing basic

76ce0b0

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

sets the toggle to true for functional tests

66139a6

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

ashwin-pc requested review from ananzh, kavilla, seanneumann, AMoo-Miki, joshuarrrr, abbyhu2000, zengyan-amazon, kristenTian, zhongnansu, manasvinibs, ZilongX and Flyingliuhub as code owners June 15, 2023 19:31

github-actions bot added the distinguished-contributor label Jun 15, 2023

ashwin-pc added discover for discover reinvent de-angular de-angularize work data explorer Issues related to the Data Explorer project and removed distinguished-contributor labels Jun 15, 2023

ashwin-pc changed the title ~~Feature/data explorer~~ [Data Explorer] Initial commit Jun 15, 2023

AMoo-Miki reviewed Jun 21, 2023

View reviewed changes

removes components that fail tests and are not necessary

3da84d6

Signed-off-by: Ashwin P Chandran <ashwinpc@amazon.com>

ananzh approved these changes Jun 26, 2023

View reviewed changes

kavilla reviewed Jun 27, 2023

View reviewed changes

kavilla approved these changes Jun 27, 2023

View reviewed changes

ashwin-pc merged commit d4acb33 into opensearch-project:feature/data-explorer Jun 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Data Explorer] Initial commit #4292

[Data Explorer] Initial commit #4292

ashwin-pc commented Jun 15, 2023 •

edited

Loading

codecov bot commented Jun 15, 2023 •

edited

Loading

AMoo-Miki left a comment

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

AMoo-Miki Jun 21, 2023

ashwin-pc Jun 22, 2023

ashwin-pc commented Jun 22, 2023

ananzh left a comment

kavilla Jun 27, 2023

ashwin-pc Jun 28, 2023

kavilla Jun 27, 2023

ashwin-pc Jun 28, 2023

kavilla left a comment

ashwin-pc commented Jun 28, 2023

		@@ -0,0 +1,11 @@
		# dataExplorer

		A OpenSearch Dashboards plugin

	A OpenSearch Dashboards plugin
	An OpenSearch Dashboards plugin to ... ???

[Data Explorer] Initial commit #4292

[Data Explorer] Initial commit #4292

Conversation

ashwin-pc commented Jun 15, 2023 • edited Loading

Description

Issues Resolved

Screenshot

Testing the changes

Check List

codecov bot commented Jun 15, 2023 • edited Loading

Codecov Report

AMoo-Miki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashwin-pc commented Jun 22, 2023

ananzh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kavilla left a comment

Choose a reason for hiding this comment

ashwin-pc commented Jun 28, 2023

ashwin-pc commented Jun 15, 2023 •

edited

Loading

codecov bot commented Jun 15, 2023 •

edited

Loading