Fix more PR comments

* remove server for experimental table
* add style in expression_helpers and simplify to_expression
* update pagination

Signed-off-by: Anan Zhuang <ananzh@amazon.com>

src/plugins/vis_builder/public/application/utils/use/use_saved_vis_builder_vis.ts

@@ -13,8 +13,6 @@ import {
 } from '../../../../../opensearch_dashboards_utils/public';
 import { EDIT_PATH, PLUGIN_ID } from '../../../../common';
 import { VisBuilderServices } from '../../../types';
-import { MetricOptionsDefaults } from '../../../visualizations/metric/metric_viz_type';
-import { TableOptionsDefaults } from '../../../visualizations/table/table_viz_type';
 import { getCreateBreadcrumbs, getEditBreadcrumbs } from '../breadcrumbs';
 import { getSavedVisBuilderVis } from '../get_saved_vis_builder_vis';
 import {

src/plugins/vis_builder/public/visualizations/common/expression_helpers.ts

@@ -9,8 +9,12 @@ import { ExpressionFunctionOpenSearchDashboards } from '../../../../expressions'
 import { buildExpressionFunction } from '../../../../expressions/public';
 import { VisualizationState } from '../../application/utils/state_management';
 import { getSearchService, getIndexPatterns } from '../../plugin_services';
+import { StyleState } from '../../application/utils/state_management';
 
-export const getAggExpressionFunctions = async (visualization: VisualizationState) => {
+export const getAggExpressionFunctions = async (
+ visualization: VisualizationState,
+ style?: StyleState
+) => {
  const { activeVisualization, indexPattern: indexId = '' } = visualization;
  const { aggConfigParams } = activeVisualization || {};
 
@@ -32,8 +36,8 @@ export const getAggExpressionFunctions = async (visualization: VisualizationStat
  'opensearchaggs',
  {
  index: indexId,
- metricsAtAllLevels: false,
- partialRows: false,
+ metricsAtAllLevels: style?.showMetricsAtAllLevels || false,
+ partialRows: style?.showPartialRows || false,
  aggConfigs: JSON.stringify(aggConfigs.aggs),
  includeFormatHints: false,
  }

src/plugins/vis_builder/public/visualizations/table/components/table_viz_options.tsx

@@ -20,14 +20,7 @@ import {
 import { useAggs } from '../../../../public/application/utils/use';
 import { TableOptionsDefaults } from '../table_viz_type';
 import { Option } from '../../../application/app';
-
-export enum AggTypes {
- SUM = 'sum',
- AVG = 'avg',
- MIN = 'min',
- MAX = 'max',
- COUNT = 'count',
-}
+import { AggTypes } from '../types';
 
 const { tabifyGetColumns } = search;
 
@@ -122,7 +115,6 @@ function TableVizOptions() {
  {i18n.translate('visTypeTableNewNew.params.perPageLabel', {
  defaultMessage: 'Max rows per page',
  })}
- ,
  <EuiIconTip
  content={
  <FormattedMessage

src/plugins/vis_builder/public/visualizations/table/table_viz_type.ts

@@ -9,14 +9,7 @@ import { AggGroupNames } from '../../../../data/public';
 import { TableVizOptions } from './components/table_viz_options';
 import { VisualizationTypeOptions } from '../../services/type_service';
 import { toExpression } from './to_expression';
-
-export enum AggTypes {
- SUM = 'sum',
- AVG = 'avg',
- MIN = 'min',
- MAX = 'max',
- COUNT = 'count',
-}
+import { AggTypes } from './types';
 
 export interface TableOptionsDefaults {
  perPage: number | '';

src/plugins/vis_builder/public/visualizations/table/to_expression.ts

@@ -10,7 +10,6 @@ import { buildExpression, buildExpressionFunction } from '../../../../expression
 import { RenderState } from '../../application/utils/state_management';
 import { TableOptionsDefaults } from './table_viz_type';
 import { getAggExpressionFunctions } from '../common/expression_helpers';
-import { OpenSearchaggsExpressionFunctionDefinition } from '../../../../data/public';
 
 // TODO: Update to the common getShemas from src/plugins/visualizations/public/legacy/build_pipeline.ts
 // And move to a common location accessible by all the visualizations
@@ -98,11 +97,7 @@ export interface TableRootState extends RenderState {
 }
 
 export const toExpression = async ({ style: styleState, visualization }: TableRootState) => {
- const { aggConfigs, indexPattern, expressionFns } = await getAggExpressionFunctions(
- visualization
- );
- const { id: indexId = '' } = indexPattern;
- let [opensearchaggs] = expressionFns;
+ const { aggConfigs, expressionFns } = await getAggExpressionFunctions(visualization, styleState);
 
  const {
  perPage,
@@ -141,25 +136,12 @@ export const toExpression = async ({ style: styleState, visualization }: TableRo
  ...tableData,
  };
 
- // Update buildExpressionFunction to correctly handle optional arguments
  const tableVis = buildExpressionFunction<TableVisExpressionFunctionDefinition>(
  'opensearch_dashboards_table_new',
  {
  visConfig: JSON.stringify(visConfig),
  }
  );
-
- opensearchaggs = buildExpressionFunction<OpenSearchaggsExpressionFunctionDefinition>(
- 'opensearchaggs',
- {
- index: indexId,
- metricsAtAllLevels: showMetricsAtAllLevels,
- partialRows: showPartialRows,
- aggConfigs: JSON.stringify(aggConfigs.aggs),
- includeFormatHints: false,
- }
- );
-
- const expressionFnsTableVis = [expressionFns[0], opensearchaggs];
- return buildExpression([...expressionFnsTableVis, tableVis]).toString();
+ const a = buildExpression([...expressionFns, tableVis]).toString();
+ return buildExpression([...expressionFns, tableVis]).toString();
 };

src/plugins/vis_builder/public/visualizations/table/types.ts

@@ -0,0 +1,12 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+export enum AggTypes {
+ SUM = 'sum',
+ AVG = 'avg',
+ MIN = 'min',
+ MAX = 'max',
+ COUNT = 'count',
+}

src/plugins/vis_type_table_new/opensearch_dashboards.json

@@ -1,7 +1,7 @@
 {
  "id": "visTypeTableNew",
  "version": "opensearchDashboards",
- "server": true,
+ "server": false,
  "ui": true,
  "requiredPlugins": [
  "expressions",

src/plugins/vis_type_table_new/public/components/table_vis_app.tsx

@@ -10,13 +10,13 @@ import { CoreStart } from 'opensearch-dashboards/public';
 import { I18nProvider } from '@osd/i18n/react';
 import { IInterpreterRenderHandlers } from 'src/plugins/expressions';
 import { OpenSearchDashboardsContextProvider } from '../../../opensearch_dashboards_react/public';
-
 import { TableContext } from '../table_vis_response_handler';
 import { TableVisConfig, SortColumn, ColumnWidth, TableUiState } from '../types';
 import { TableVisComponent } from './table_vis_component';
 import { TableVisComponentGroup } from './table_vis_component_group';
 
 interface TableVisAppProps {
+ services: CoreStart;
  visData: TableContext;
  visConfig: TableVisConfig;
  handlers: IInterpreterRenderHandlers;
@@ -27,7 +27,8 @@ export const TableVisApp = ({
  visData: { table, tableGroups, direction },
  visConfig,
  handlers,
-}: TableVisAppProps & { services: CoreStart }) => {
+}: TableVisAppProps) => {
+ // Rendering is asynchronous, completed by handlers.done()
  useEffect(() => {
  handlers.done();
  }, [handlers]);
@@ -39,10 +40,8 @@ export const TableVisApp = ({
 
  // TODO: remove duplicate sort and width state
  // Issue: https://github.com/opensearch-project/OpenSearch-Dashboards/issues/2704#issuecomment-1299380818
- const [sort, setSort] = useState<SortColumn>(
- handlers.uiState.get('vis.sortColumn') || { colIndex: null, direction: null }
- );
- const [width, setWidth] = useState<ColumnWidth[]>(handlers.uiState.get('vis.sortColumn') || []);
+ const [sort, setSort] = useState<SortColumn>({ colIndex: null, direction: null });
+ const [width, setWidth] = useState<ColumnWidth[]>([]);
 
  const tableUiState: TableUiState = { sort, setSort, width, setWidth };
 
@@ -70,7 +69,3 @@ export const TableVisApp = ({
  </I18nProvider>
  );
 };
-
-// default export required for React.Lazy
-// eslint-disable-next-line import/no-default-export
-export { TableVisApp as default };

src/plugins/vis_type_table_new/public/components/table_vis_component.tsx

@@ -38,7 +38,7 @@ export const TableVisComponent = ({
  const pagination = usePagination(visConfig, rows.length);
 
  const sortedRows = useMemo(() => {
- return uiState.sort && uiState.sort.colIndex !== null && uiState.sort.direction
+ return uiState.sort?.colIndex && uiState.sort.direction
  ? orderBy(rows, columns[uiState.sort.colIndex]?.id, uiState.sort.direction)
  : rows;
  }, [columns, rows, uiState]);
@@ -58,7 +58,7 @@ export const TableVisComponent = ({
  const dataGridColumns = getDataGridColumns(sortedRows, columns, table, event, uiState.width);
 
  const sortedColumns = useMemo(() => {
- return uiState.sort && uiState.sort.colIndex !== null && uiState.sort.direction
+ return uiState.sort?.colIndex && uiState.sort.direction
  ? [{ id: dataGridColumns[uiState.sort.colIndex]?.id, direction: uiState.sort.direction }]
  : [];
  }, [dataGridColumns, uiState]);

src/plugins/vis_type_table_new/public/index.ts

@@ -1,38 +1,13 @@
 /*
+ * Copyright OpenSearch Contributors
  * 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 { PluginInitializerContext } from 'opensearch-dashboards/public';
+// import { PluginInitializerContext } from 'opensearch-dashboards/public';
 import { TableVisPlugin as Plugin } from './plugin';
 
-export function plugin(initializerContext: PluginInitializerContext) {
- return new Plugin(initializerContext);
+export function plugin() {
+ return new Plugin();
 }
 /* Public Types */
 export { TableVisExpressionFunctionDefinition } from './table_vis_fn';

src/plugins/vis_type_table_new/public/plugin.ts

@@ -3,27 +3,18 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import {
- PluginInitializerContext,
- CoreSetup,
- CoreStart,
- Plugin,
-} from 'opensearch-dashboards/public';
+import { CoreSetup, CoreStart, Plugin } from 'opensearch-dashboards/public';
 import { Plugin as ExpressionsPublicPlugin } from '../../expressions/public';
-import { VisualizationsSetup } from '../../visualizations/public';
 
 import { createTableVisFn } from './table_vis_fn';
 import { DataPublicPluginStart } from '../../data/public';
 import { setFormatService } from './services';
 import { getTableVisRenderer } from './table_vis_renderer';
 
-/** @internal */
 export interface TableVisPluginSetupDependencies {
  expressions: ReturnType<ExpressionsPublicPlugin['setup']>;
- visualizations: VisualizationsSetup;
 }
 
-/** @internal */
 export interface TableVisPluginStartDependencies {
  data: DataPublicPluginStart;
 }
@@ -34,14 +25,7 @@ const setupTableVis = async (core: CoreSetup, { expressions }: TableVisPluginSet
  expressions.registerRenderer(getTableVisRenderer(coreStart));
 };
 
-/** @internal */
 export class TableVisPlugin implements Plugin<void, void> {
- initializerContext: PluginInitializerContext<ConfigSchema>;
-
- constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
- this.initializerContext = initializerContext;
- }
-
  public async setup(core: CoreSetup, dependencies: TableVisPluginSetupDependencies) {
  setupTableVis(core, dependencies);
  }

src/plugins/vis_type_table_new/public/services.ts

@@ -1,31 +1,6 @@
 /*
+ * Copyright OpenSearch Contributors
  * 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 { createGetterSetter } from '../../opensearch_dashboards_utils/public';

src/plugins/vis_type_table_new/public/utils/use_pagination.ts

@@ -9,7 +9,7 @@ import { TableVisConfig } from '../types';
 export const usePagination = (visConfig: TableVisConfig, nRow: number) => {
  const [pagination, setPagination] = useState({
  pageIndex: 0,
- pageSize: visConfig.perPage || 0,
+ pageSize: visConfig.perPage || 10,
  });
  const onChangeItemsPerPage = useCallback(
  (pageSize) => setPagination((p) => ({ ...p, pageSize, pageIndex: 0 })),
@@ -20,7 +20,7 @@ export const usePagination = (visConfig: TableVisConfig, nRow: number) => {
  ]);
 
  useEffect(() => {
- const perPage = visConfig.perPage || 0;
+ const perPage = visConfig.perPage || 10;
  const maxiPageIndex = Math.ceil(nRow / perPage) - 1;
  setPagination((p) => ({
  pageIndex: p.pageIndex > maxiPageIndex ? maxiPageIndex : p.pageIndex,