fix: always pass a string as a value to ace editor (#13563)

* always pass a string as a value to ace editor

* fix some syntax

(cherry picked from commit 67ffea8248)

superset-frontend/spec/javascripts/sqllab/TemplateParamsEditor_spec.tsx

@@ -0,0 +1,57 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF 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 React, { ReactNode } from 'react';
+import {
+  render,
+  fireEvent,
+  getByText,
+  waitFor,
+} from 'spec/helpers/testing-library';
+import brace from 'brace';
+import { ThemeProvider, supersetTheme } from '@superset-ui/core';
+
+import TemplateParamsEditor from 'src/SqlLab/components/TemplateParamsEditor';
+
+const ThemeWrapper = ({ children }: { children: ReactNode }) => (
+  <ThemeProvider theme={supersetTheme}>{children}</ThemeProvider>
+);
+
+describe('TemplateParamsEditor', () => {
+  it('should render with a title', () => {
+    const { container } = render(
+      <TemplateParamsEditor code="FOO" language="json" onChange={() => {}} />,
+      { wrapper: ThemeWrapper },
+    );
+    expect(container.querySelector('div[role="button"]')).toBeInTheDocument();
+  });
+
+  it('should open a modal with the ace editor', async () => {
+    const { container, baseElement } = render(
+      <TemplateParamsEditor code="FOO" language="json" onChange={() => {}} />,
+      { wrapper: ThemeWrapper },
+    );
+    fireEvent.click(getByText(container, 'Parameters'));
+    const spy = jest.spyOn(brace, 'acequire');
+    spy.mockReturnValue({ setCompleters: () => 'foo' });
+    await waitFor(() => {
+      expect(baseElement.querySelector('#brace-editor')).toBeInTheDocument();
+    });
+  });
+});

superset-frontend/src/SqlLab/components/TemplateParamsEditor.tsx

@@ -17,7 +17,6 @@
  * under the License.
  */
 import React, { useState, useEffect } from 'react';
-import PropTypes from 'prop-types';
 import Badge from 'src/common/components/Badge';
 import { t, styled } from '@superset-ui/core';
 import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls';
@@ -26,17 +25,7 @@ import { debounce } from 'lodash';
 import ModalTrigger from 'src/components/ModalTrigger';
 import { ConfigEditor } from 'src/components/AsyncAceEditor';
 import { FAST_DEBOUNCE } from 'src/constants';
-
+import { Tooltip } from 'src/common/components/Tooltip';
-const propTypes = {
-  onChange: PropTypes.func,
-  code: PropTypes.string,
-  language: PropTypes.oneOf(['yaml', 'json']),
-};
-
-const defaultProps = {
-  onChange: () => {},
-  code: '{}',
-};
 
 const StyledConfigEditor = styled(ConfigEditor)`
   &.ace_editor {
@@ -44,8 +33,16 @@ const StyledConfigEditor = styled(ConfigEditor)`
   }
 `;
 
-function TemplateParamsEditor({ code, language, onChange }) {
+function TemplateParamsEditor({
-  const [parsedJSON, setParsedJSON] = useState();
+  code = '{}',
+  language,
+  onChange = () => {},
+}: {
+  code: string;
+  language: 'yaml' | 'json';
+  onChange: () => void;
+}) {
+  const [parsedJSON, setParsedJSON] = useState({});
   const [isValid, setIsValid] = useState(true);
 
   useEffect(() => {
@@ -53,7 +50,7 @@ function TemplateParamsEditor({ code, language, onChange }) {
       setParsedJSON(JSON.parse(code));
       setIsValid(true);
     } catch {
-      setParsedJSON({});
+      setParsedJSON({} as any);
       setIsValid(false);
     }
   }, [code]);
@@ -96,25 +93,29 @@ function TemplateParamsEditor({ code, language, onChange }) {
     <ModalTrigger
       modalTitle={t('Template parameters')}
       triggerNode={
-        <div tooltip={t('Edit template parameters')} buttonSize="small">
+        <Tooltip
-          {`${t('Parameters')} `}
+          id="parameters-tooltip"
-          <Badge count={paramCount} />
+          placement="top"
-          {!isValid && (
+          title={t('Edit template parameters')}
-            <InfoTooltipWithTrigger
+          trigger={['hover']}
-              icon="exclamation-triangle"
+        >
-              bsStyle="danger"
+          <div role="button">
-              tooltip={t('Invalid JSON')}
+            {`${t('Parameters')} `}
-              label="invalid-json"
+            <Badge count={paramCount} />
-            />
+            {!isValid && (
-          )}
+              <InfoTooltipWithTrigger
-        </div>
+                icon="exclamation-triangle"
+                bsStyle="danger"
+                tooltip={t('Invalid JSON')}
+                label="invalid-json"
+              />
+            )}
+          </div>
+        </Tooltip>
       }
       modalBody={modalBody}
     />
   );
 }
 
-TemplateParamsEditor.propTypes = propTypes;
-TemplateParamsEditor.defaultProps = defaultProps;
-
 export default TemplateParamsEditor;

superset-frontend/src/SqlLab/reducers/getInitialState.js

@@ -74,13 +74,13 @@ export default function getInitialState({
         id: id.toString(),
         loaded: true,
         title: activeTab.label,
-        sql: activeTab.sql,
+        sql: activeTab.sql || undefined,
-        selectedText: null,
+        selectedText: undefined,
         latestQueryId: activeTab.latest_query
           ? activeTab.latest_query.id
           : null,
         autorun: activeTab.autorun,
-        templateParams: activeTab.template_params,
+        templateParams: activeTab.template_params || undefined,
         dbId: activeTab.database_id,
         functionNames: [],
         schema: activeTab.schema,

superset-frontend/src/SqlLab/reducers/getInitialState.test.ts

@@ -36,8 +36,19 @@ const apiData = {
     username: 'some name',
   },
 };
+const apiDataWithTabState = {
+  ...apiData,
+  tab_state_ids: [{ id: 1 }],
+  active_tab: { id: 1, table_schemas: [] },
+};
 describe('getInitialState', () => {
   it('should output the user that is passed in', () => {
     expect(getInitialState(apiData).sqlLab.user.userId).toEqual(1);
   });
+  it('should return undefined instead of null for templateParams', () => {
+    expect(
+      getInitialState(apiDataWithTabState).sqlLab.queryEditors[0]
+        .templateParams,
+    ).toBeUndefined();
+  });
 });

superset-frontend/src/components/AsyncAceEditor/index.tsx

@@ -119,6 +119,8 @@ export default function AsyncAceEditor(
           mode = inferredMode,
           theme = inferredTheme,
           tabSize = defaultTabSize,
+          defaultValue = '',
+          value = '',
           ...props
         },
         ref,
@@ -150,6 +152,8 @@ export default function AsyncAceEditor(
             mode={mode}
             theme={theme}
             tabSize={tabSize}
+            defaultValue={defaultValue}
+            value={value || ''}
             {...props}
           />
         );

superset/migrations/versions/67da9ef1ef9c_add_hide_left_bar_to_tabstate.py

@@ -29,12 +29,18 @@ down_revision = "1412ec1e5a7b"
 import sqlalchemy as sa
 from alembic import op
 from sqlalchemy.dialects import mysql
+from sqlalchemy.sql import expression
 
 
 def upgrade():
     with op.batch_alter_table("tab_state") as batch_op:
         batch_op.add_column(
-            sa.Column("hide_left_bar", sa.Boolean(), nullable=False, default=False)
+            sa.Column(
+                "hide_left_bar",
+                sa.Boolean(),
+                nullable=False,
+                server_default=expression.false(),
+            )
         )