From 47b5d3f8eb34f71f6a795b16aa7356ae7a8143e3 Mon Sep 17 00:00:00 2001 From: cgu Date: Wed, 21 Dec 2022 17:21:17 -0800 Subject: [PATCH 1/5] temp --- querybook/server/lib/data_doc/doc_types.py | 11 ++++++ querybook/server/lib/data_doc/meta.py | 42 ++++++++++++++++++++++ querybook/server/logic/datadoc.py | 7 +++- querybook/server/models/datadoc.py | 20 ++++++++++- querybook/webapp/const/datadoc.ts | 12 ++++++- 5 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 querybook/server/lib/data_doc/doc_types.py create mode 100644 querybook/server/lib/data_doc/meta.py diff --git a/querybook/server/lib/data_doc/doc_types.py b/querybook/server/lib/data_doc/doc_types.py new file mode 100644 index 000000000..4db1e144c --- /dev/null +++ b/querybook/server/lib/data_doc/doc_types.py @@ -0,0 +1,11 @@ +from typing import TypedDict, Union, Literal + + +class DataDocMetaVarConfig(TypedDict): + name: str + value: Union[str, int, float, bool] + type: Literal["boolean", "number", "string"] + + +class DataDocMeta(TypedDict): + variables: list[DataDocMetaVarConfig] diff --git a/querybook/server/lib/data_doc/meta.py b/querybook/server/lib/data_doc/meta.py new file mode 100644 index 000000000..aac0a3f62 --- /dev/null +++ b/querybook/server/lib/data_doc/meta.py @@ -0,0 +1,42 @@ +from typing import Dict, Any +from .doc_types import DataDocMeta + + +def check_variable_type(val: Any): + if isinstance(val, (int, float)): + return "number" + elif isinstance(val, bool): + return "boolean" + elif isinstance(val, str): + return "string" + + # this shouldn't happen, just in case + return "string" + + +def convert_if_legacy_datadoc_meta_v0(datadoc_meta: Dict) -> DataDocMeta: + if isinstance(datadoc_meta.get("variables"), list): + return datadoc_meta + + new_meta = {"variables": []} + + for name, value in datadoc_meta.items(): + new_meta["variables"].append( + {"name": name, "value": value, "type": check_variable_type(value)} + ) + + return new_meta + + +def convert_if_legacy_datadoc_meta(datadoc_meta: Dict) -> DataDocMeta: + datadoc_meta = convert_if_legacy_datadoc_meta_v0(datadoc_meta) + return datadoc_meta + + +def get_datadoc_meta_variables_dict(datadoc_meta: DataDocMeta) -> Dict: + variables = {} + + for config in datadoc_meta.get("variables", []): + variables[config["name"]] = config["value"] + + return variables diff --git a/querybook/server/logic/datadoc.py b/querybook/server/logic/datadoc.py index 541b3d8ff..900ad32da 100644 --- a/querybook/server/logic/datadoc.py +++ b/querybook/server/logic/datadoc.py @@ -150,7 +150,12 @@ def update_data_doc(id, commit=True, session=None, **fields): if commit: session.commit() - update_es_data_doc_by_id(data_doc.id) + + if any( + field_name in fields + for field_name in ["public", "archived", "owner_uid", "title"] + ): + update_es_data_doc_by_id(data_doc.id) # update es queries if doc is switched between public/private if "public" in fields: diff --git a/querybook/server/models/datadoc.py b/querybook/server/models/datadoc.py index 4dc373904..8614d7ab3 100644 --- a/querybook/server/models/datadoc.py +++ b/querybook/server/models/datadoc.py @@ -1,10 +1,15 @@ import sqlalchemy as sql from sqlalchemy.orm import backref, relationship +from sqlalchemy.ext.hybrid import hybrid_property from app import db from const.db import name_length, now, description_length, mediumtext_length from const.data_doc import DataCellType from lib.sqlalchemy import CRUDMixin +from lib.data_doc.meta import ( + convert_if_legacy_datadoc_meta, + get_datadoc_meta_variables_dict, +) Base = db.Base @@ -31,7 +36,7 @@ class DataDoc(Base, CRUDMixin): updated_at = sql.Column(sql.DateTime, default=now, nullable=False) title = sql.Column(sql.String(length=name_length), default="", nullable=False) - meta = sql.Column(sql.JSON, default={}, nullable=False) + _meta = sql.Column("meta", sql.JSON, default={}, nullable=False) cells = relationship( "DataCell", @@ -48,6 +53,18 @@ class DataDoc(Base, CRUDMixin): backref=backref("data_docs", cascade="all, delete", passive_deletes=True), ) + @hybrid_property + def meta(self): + return convert_if_legacy_datadoc_meta(self._meta) + + @meta.setter + def meta(self, new_meta): + self._meta = convert_if_legacy_datadoc_meta(new_meta) + + @property + def meta_variables(self): + return get_datadoc_meta_variables_dict(self.meta) + def to_dict(self, with_cells=False): data_doc_dict = { "id": self.id, @@ -58,6 +75,7 @@ def to_dict(self, with_cells=False): "created_at": self.created_at, "updated_at": self.updated_at, "meta": self.meta, + "meta_variables": self.meta_variables, "title": self.title, } diff --git a/querybook/webapp/const/datadoc.ts b/querybook/webapp/const/datadoc.ts index 3955efdbc..036a30df4 100644 --- a/querybook/webapp/const/datadoc.ts +++ b/querybook/webapp/const/datadoc.ts @@ -53,6 +53,15 @@ export interface IDataChartCell extends IDataCellBase { export type IDataCell = IDataQueryCell | IDataTextCell | IDataChartCell; export type DataCellUpdateFields = Partial>; +export type TDataDocMetaVariableType = 'string' | 'boolean' | 'number'; +export interface IDataDocMetaVariable { + name: string; + value: any; + type: TDataDocMetaVariableType; +} +export interface IDataDocMeta { + variables: IDataDocMetaVariable[]; +} export interface IDataDoc { dataDocCells: IDataCell[]; id: number; @@ -64,7 +73,8 @@ export interface IDataDoc { created_at: number; updated_at: number; - meta: Record; + meta: IDataDocMeta; + meta_variables: Record; title: string; cells?: number[]; From 116c27efbcac324dccb5b4a82a26f52edea5d1ec Mon Sep 17 00:00:00 2001 From: cgu Date: Thu, 22 Dec 2022 16:06:32 -0800 Subject: [PATCH 2/5] feat: add drag and drop for templated variables --- package.json | 4 +- .../server/lib/dag_exporter/export_dag.py | 2 +- querybook/server/lib/data_doc/meta.py | 27 ++ querybook/server/logic/datadoc.py | 20 +- querybook/server/models/datadoc.py | 9 +- querybook/server/tasks/run_datadoc.py | 5 +- .../webapp/components/DataDoc/DataDoc.tsx | 5 +- .../components/DataDocCell/DataDocCell.tsx | 8 +- .../DataDocRightSidebar.tsx | 4 +- .../DataDocTemplateButton.tsx | 10 +- .../DataDocTemplateCell.tsx | 23 +- .../DataDocTemplateVarForm.tsx | 250 ++++++++++-------- .../DataDocTemplateButton/helpers.ts | 32 +-- querybook/webapp/const/datadoc.ts | 12 +- querybook/webapp/redux/dataDoc/types.ts | 2 +- .../dataDocWebsocket/dataDocWebsocket.ts | 33 +++ .../ui/DraggableList/DraggableIcon.scss | 3 + .../webapp/ui/DraggableList/DraggableIcon.tsx | 21 ++ querybook/webapp/ui/Icon/LucideIcons.ts | 2 + yarn.lock | 17 +- 20 files changed, 306 insertions(+), 183 deletions(-) create mode 100644 querybook/webapp/ui/DraggableList/DraggableIcon.scss create mode 100644 querybook/webapp/ui/DraggableList/DraggableIcon.tsx diff --git a/package.json b/package.json index eeb60bdc6..991973cab 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "querybook", - "version": "3.15.4", + "version": "3.15.5", "description": "A Big Data Webapp", "private": true, "scripts": { @@ -54,7 +54,7 @@ "draft-js-export-html": "^1.4.1", "draft-js-import-html": "^1.4.1", "fast-json-stable-stringify": "2.0.0", - "formik": "2.2.6", + "formik": "2.2.9", "history": "^4.10.1", "html-webpack-plugin": "5.3.1", "immer": "8.0.1", diff --git a/querybook/server/lib/dag_exporter/export_dag.py b/querybook/server/lib/dag_exporter/export_dag.py index fcb251bfa..bd633f4c1 100644 --- a/querybook/server/lib/dag_exporter/export_dag.py +++ b/querybook/server/lib/dag_exporter/export_dag.py @@ -21,7 +21,7 @@ def export_dag(data_doc_id, dag_exporter_name, session=None): edges=dag["edges"], meta={ **dag_export["meta"]["exporter_meta"][dag_exporter_name], - "variables": doc.meta, + "variables": doc.meta_variables, }, cell_by_id=cell_by_id, ) diff --git a/querybook/server/lib/data_doc/meta.py b/querybook/server/lib/data_doc/meta.py index aac0a3f62..b8afa8ee2 100644 --- a/querybook/server/lib/data_doc/meta.py +++ b/querybook/server/lib/data_doc/meta.py @@ -40,3 +40,30 @@ def get_datadoc_meta_variables_dict(datadoc_meta: DataDocMeta) -> Dict: variables[config["name"]] = config["value"] return variables + + +valid_meta_keys = ["variables"] + + +def validate_datadoc_meta(datadoc_meta: DataDocMeta) -> bool: + for key in datadoc_meta.keys(): + if key not in valid_meta_keys: + return False + + if "variables" in datadoc_meta: + variables = datadoc_meta["variables"] + if not isinstance(variables, list): + return False + + for variable_config in variables: + var_type = variable_config["type"] + var_val = variable_config["value"] + + if var_type == "string" and not isinstance(var_val, str): + return False + if var_type == "boolean" and not isinstance(var_val, bool): + return False + if var_type == "number" and not isinstance(var_val, (float, int)): + return False + + return True diff --git a/querybook/server/logic/datadoc.py b/querybook/server/logic/datadoc.py index 900ad32da..37ff2ddbf 100644 --- a/querybook/server/logic/datadoc.py +++ b/querybook/server/logic/datadoc.py @@ -49,16 +49,18 @@ def create_data_doc( commit=True, session=None, ): - data_doc = DataDoc( - public=public, - archived=archived, - owner_uid=owner_uid, - environment_id=environment_id, - title=title, - meta=meta, + data_doc = DataDoc.create( + fields={ + "public": public, + "archived": archived, + "owner_uid": owner_uid, + "environment_id": environment_id, + "title": title, + "meta": meta, + }, + commit=False, + session=session, ) - session.add(data_doc) - session.flush() for index, cell in enumerate(cells): data_cell = create_data_cell( diff --git a/querybook/server/models/datadoc.py b/querybook/server/models/datadoc.py index 8614d7ab3..6b64bd4a6 100644 --- a/querybook/server/models/datadoc.py +++ b/querybook/server/models/datadoc.py @@ -9,6 +9,7 @@ from lib.data_doc.meta import ( convert_if_legacy_datadoc_meta, get_datadoc_meta_variables_dict, + validate_datadoc_meta, ) Base = db.Base @@ -55,11 +56,15 @@ class DataDoc(Base, CRUDMixin): @hybrid_property def meta(self): - return convert_if_legacy_datadoc_meta(self._meta) + return convert_if_legacy_datadoc_meta(self._meta or {}) @meta.setter def meta(self, new_meta): - self._meta = convert_if_legacy_datadoc_meta(new_meta) + is_valid = validate_datadoc_meta(new_meta) + if not is_valid: + raise ValueError("Invalid DataDoc.meta") + + self._meta = new_meta @property def meta_variables(self): diff --git a/querybook/server/tasks/run_datadoc.py b/querybook/server/tasks/run_datadoc.py index dadd1dfcd..6d1d11f73 100644 --- a/querybook/server/tasks/run_datadoc.py +++ b/querybook/server/tasks/run_datadoc.py @@ -73,7 +73,10 @@ def run_datadoc_with_config( try: query = render_templated_query( - query_cell.context, data_doc.meta, engine_id, session=session + query_cell.context, + data_doc.meta_variables, + engine_id, + session=session, ) except Exception as e: on_datadoc_completion( diff --git a/querybook/webapp/components/DataDoc/DataDoc.tsx b/querybook/webapp/components/DataDoc/DataDoc.tsx index 515103f30..8ec064906 100644 --- a/querybook/webapp/components/DataDoc/DataDoc.tsx +++ b/querybook/webapp/components/DataDoc/DataDoc.tsx @@ -22,6 +22,7 @@ import { IDataCell, IDataCellMeta, IDataDoc, + IDataDocMeta, IDataQueryCell, } from 'const/datadoc'; import { ISearchOptions, ISearchResult } from 'const/searchAndReplace'; @@ -632,7 +633,7 @@ class DataDocComponent extends React.PureComponent { key={cell.id} docId={dataDoc.id} numberOfCells={dataDoc.dataDocCells.length} - templatedVariables={dataDoc.meta} + templatedVariables={dataDoc.meta_variables} cell={cell} index={index} queryIndexInDoc={queryIndexInDoc} @@ -907,7 +908,7 @@ function mapDispatchToProps(dispatch: Dispatch) { ); }, - changeDataDocMeta: (docId: number, meta: IDataCellMeta) => + changeDataDocMeta: (docId: number, meta: IDataDocMeta) => dispatch(dataDocActions.updateDataDocField(docId, 'meta', meta)), cloneDataDoc: (docId: number) => diff --git a/querybook/webapp/components/DataDocCell/DataDocCell.tsx b/querybook/webapp/components/DataDocCell/DataDocCell.tsx index 21d9f9fa0..0224d7d9c 100644 --- a/querybook/webapp/components/DataDocCell/DataDocCell.tsx +++ b/querybook/webapp/components/DataDocCell/DataDocCell.tsx @@ -8,7 +8,11 @@ import { DataDocChartCell } from 'components/DataDocChartCell/DataDocChartCell'; import { DataDocQueryCell } from 'components/DataDocQueryCell/DataDocQueryCell'; import { DataDocTextCell } from 'components/DataDocTextCell/DataDocTextCell'; import { UserAvatar } from 'components/UserBadge/UserAvatar'; -import { DataCellUpdateFields, IDataCell } from 'const/datadoc'; +import { + DataCellUpdateFields, + IDataCell, + TDataDocMetaVariableDict, +} from 'const/datadoc'; import { DataDocContext } from 'context/DataDoc'; import { useMakeSelector } from 'hooks/redux/useMakeSelector'; import { useBoundFunc } from 'hooks/useBoundFunction'; @@ -22,7 +26,7 @@ import './DataDocCell.scss'; interface IDataDocCellProps { docId: number; numberOfCells: number; - templatedVariables: Record; + templatedVariables: TDataDocMetaVariableDict; cell: IDataCell; index: number; diff --git a/querybook/webapp/components/DataDocRightSidebar/DataDocRightSidebar.tsx b/querybook/webapp/components/DataDocRightSidebar/DataDocRightSidebar.tsx index 4e2a88f75..0d6f2ccb4 100644 --- a/querybook/webapp/components/DataDocRightSidebar/DataDocRightSidebar.tsx +++ b/querybook/webapp/components/DataDocRightSidebar/DataDocRightSidebar.tsx @@ -5,7 +5,7 @@ import { DataDocBoardsButton } from 'components/DataDocBoardsButton/DataDocBoard import { DataDocDAGExporterButton } from 'components/DataDocDAGExporter/DataDocDAGExporterButton'; import { DataDocTemplateButton } from 'components/DataDocTemplateButton/DataDocTemplateButton'; import { DataDocUIGuide } from 'components/UIGuide/DataDocUIGuide'; -import { IDataDoc } from 'const/datadoc'; +import { IDataDoc, IDataDocMeta } from 'const/datadoc'; import { useAnnouncements } from 'hooks/redux/useAnnouncements'; import { useScrollToTop } from 'hooks/ui/useScrollToTop'; import { fetchDAGExporters } from 'redux/dataDoc/action'; @@ -24,7 +24,7 @@ interface IProps { isEditable: boolean; isConnected: boolean; - changeDataDocMeta: (docId: number, meta: Record) => any; + changeDataDocMeta: (docId: number, meta: IDataDocMeta) => Promise; onClone: () => any; onCollapse: () => any; diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx index 0d71a1964..1a6e845fb 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx @@ -1,15 +1,14 @@ import React from 'react'; -import toast from 'react-hot-toast'; import { DataDocTemplateVarForm } from 'components/DataDocTemplateButton/DataDocTemplateVarForm'; -import { IDataDoc } from 'const/datadoc'; +import { IDataDoc, IDataDocMeta } from 'const/datadoc'; import { IconButton } from 'ui/Button/IconButton'; import { Modal } from 'ui/Modal/Modal'; import { DataDocTemplateInfoButton } from './DataDocTemplateInfoButton'; interface IProps { - changeDataDocMeta: (docId: number, meta: Record) => any; + changeDataDocMeta: (docId: number, meta: IDataDocMeta) => Promise; dataDoc: IDataDoc; isEditable?: boolean; } @@ -31,11 +30,10 @@ export const DataDocTemplateButton: React.FunctionComponent = ({ > { - changeDataDocMeta(dataDoc.id, meta); setShowTemplateForm(false); - toast.success('Variables saved'); + return changeDataDocMeta(dataDoc.id, meta); }} /> diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx index 0e1871af1..8dc480caa 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx @@ -1,16 +1,14 @@ -import { isEmpty } from 'lodash'; import React, { useEffect, useMemo, useState } from 'react'; -import toast from 'react-hot-toast'; import { DataDocTemplateVarForm } from 'components/DataDocTemplateButton/DataDocTemplateVarForm'; -import { IDataDoc } from 'const/datadoc'; +import { IDataDoc, IDataDocMeta } from 'const/datadoc'; import { TextButton } from 'ui/Button/Button'; import { AccentText } from 'ui/StyledText/StyledText'; import { DataDocTemplateInfoButton } from './DataDocTemplateInfoButton'; interface IProps { - changeDataDocMeta: (docId: number, meta: Record) => any; + changeDataDocMeta: (docId: number, meta: IDataDocMeta) => Promise; dataDoc: IDataDoc; isEditable?: boolean; } @@ -21,12 +19,12 @@ export const DataDocTemplateCell: React.FunctionComponent = ({ isEditable, }) => { const hasMeta = useMemo( - () => dataDoc.meta && !isEmpty(dataDoc.meta), + () => dataDoc.meta?.variables?.length > 0, [dataDoc.meta] ); - const [showFacade, setShowFacde] = useState(!hasMeta && isEditable); + const [showFacade, setShowFacade] = useState(!hasMeta && isEditable); useEffect(() => { - setShowFacde(!hasMeta && isEditable); + setShowFacade(!hasMeta && isEditable); // eslint-disable-next-line react-hooks/exhaustive-deps }, [dataDoc.id]); @@ -42,7 +40,7 @@ export const DataDocTemplateCell: React.FunctionComponent = ({ icon="Plus" className="mr4" title="New Variable" - onClick={() => setShowFacde(false)} + onClick={() => setShowFacade(false)} /> @@ -63,13 +61,12 @@ export const DataDocTemplateCell: React.FunctionComponent = ({ { - changeDataDocMeta(dataDoc.id, meta); - if (isEmpty(meta)) { - setShowFacde(true); + if (meta.variables.length === 0) { + setShowFacade(true); } - toast.success('Variables saved'); + return changeDataDocMeta(dataDoc.id, meta); }} /> diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx index 8ea8e8271..a188830dc 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx @@ -1,25 +1,28 @@ import { FieldArray, Form, Formik } from 'formik'; -import { isEmpty } from 'lodash'; -import React, { useMemo } from 'react'; +import { uniqueId } from 'lodash'; +import React, { useCallback, useMemo } from 'react'; +import toast from 'react-hot-toast'; import * as Yup from 'yup'; +import { + IDataDocMeta, + IDataDocMetaVariable, + TEMPLATED_VAR_SUPPORTED_TYPES, +} from 'const/datadoc'; +import { stopPropagationAndDefault } from 'lib/utils/noop'; import { Button, TextButton } from 'ui/Button/Button'; import { IconButton } from 'ui/Button/IconButton'; +import { DraggableIcon } from 'ui/DraggableList/DraggableIcon'; +import { DraggableList } from 'ui/DraggableList/DraggableList'; import { SimpleField } from 'ui/FormikField/SimpleField'; -import { - getVariableValueByType, - detectVariableType, - SUPPORTED_TYPES, - TTemplateVariableDict, -} from './helpers'; +import { typeCastVariables } from './helpers'; import './DataDocTemplateVarForm.scss'; export interface IDataDocTemplateVarFormProps { - onSave: (vars: TTemplateVariableDict) => any; - templatedVariables: TTemplateVariableDict; - defaultTemplatedVariables?: TTemplateVariableDict; + onSave: (meta: IDataDocMeta) => Promise; + meta: IDataDocMeta; isEditable: boolean; } @@ -27,10 +30,10 @@ const templatedVarSchema = Yup.object().shape({ variables: Yup.array().of( Yup.object({ name: Yup.string().required('Variable name must not be empty'), - valueType: Yup.string(), + type: Yup.string(), value: Yup.mixed() .required('Must not be empty') - .when('valueType', { + .when('type', { is: 'number', then: Yup.number() .typeError('Must be a number') @@ -44,32 +47,53 @@ const templatedVarSchema = Yup.object().shape({ ), }); -const defaultTemplatedVariablesValue = { '': '' }; +/** + * This interface is used for drag and drop purposes + */ +interface IDataDocMetaVariableWithId extends IDataDocMetaVariable { + id: string; +} +const templatedVarUniqueIdPrefix = '_tvar'; + +const defaultTemplatedVariables: IDataDocMetaVariableWithId[] = [ + { + name: '', + value: '', + type: 'string', + id: uniqueId(templatedVarUniqueIdPrefix), + }, +]; export const DataDocTemplateVarForm: React.FunctionComponent< IDataDocTemplateVarFormProps -> = ({ - onSave, - templatedVariables, - defaultTemplatedVariables = defaultTemplatedVariablesValue, - isEditable, -}) => { +> = ({ onSave, meta, isEditable }) => { const initialValue = useMemo( () => ({ - variables: Object.entries( - isEmpty(templatedVariables) - ? defaultTemplatedVariables - : templatedVariables - ).map( - ([key, value]) => - ({ - name: key, - valueType: detectVariableType(value), - value, - } as const) - ), + variables: meta.variables?.length + ? meta.variables.map((varConfig) => ({ + ...varConfig, + id: uniqueId(templatedVarUniqueIdPrefix), + })) + : defaultTemplatedVariables, }), - [defaultTemplatedVariables, templatedVariables] + + [meta?.variables] + ); + + const handleSaveMeta = useCallback( + (values: typeof initialValue) => { + const newMeta: IDataDocMeta = { + ...meta, + // This also gets rid of the id field + variables: typeCastVariables(values.variables), + }; + return toast.promise(onSave(newMeta), { + loading: 'Saving variables', + success: 'Variables saved!', + error: 'Failed to save variables', + }); + }, + [onSave, meta] ); return ( @@ -77,89 +101,95 @@ export const DataDocTemplateVarForm: React.FunctionComponent< enableReinitialize validationSchema={templatedVarSchema} initialValues={initialValue} - onSubmit={({ variables }) => - onSave( - variables.reduce((hash, { name, valueType, value }) => { - hash[name] = getVariableValueByType(value, valueType); - return hash; - }, {}) - ) - } + onSubmit={handleSaveMeta} > {({ handleSubmit, isSubmitting, isValid, values, dirty }) => { const variablesField = ( { - const fields = values.variables.length - ? values.variables.map( - ({ valueType }, index) => ( -
-
- null} - type="input" - name={`variables.${index}.name`} - inputProps={{ - placeholder: - 'variable name', - }} - /> - null} - type="react-select" - name={`variables.${index}.valueType`} - options={ - SUPPORTED_TYPES as any as string[] - } - isDisabled={!isEditable} - /> - {valueType === 'boolean' ? ( - null} - type="react-select" - name={`variables.${index}.value`} - options={[ - { - label: 'True', - value: true, - }, - { - label: 'False', - value: false, - }, - ]} - /> - ) : ( - null} - type={'input'} - name={`variables.${index}.value`} - inputProps={{ - placeholder: - 'variable value', - }} - /> - )} -
+ const renderVariableConfigRow = ( + index: number, + { type }: IDataDocMetaVariableWithId + ) => ( +
+ +
+ null} + type="input" + name={`variables.${index}.name`} + inputProps={{ + placeholder: 'variable name', + }} + /> + null} + type="react-select" + name={`variables.${index}.type`} + options={ + TEMPLATED_VAR_SUPPORTED_TYPES as any as string[] + } + isDisabled={!isEditable} + /> + {type === 'boolean' ? ( + null} + type="react-select" + name={`variables.${index}.value`} + options={[ + { + label: 'True', + value: true, + }, + { + label: 'False', + value: false, + }, + ]} + /> + ) : ( + null} + type={'input'} + name={`variables.${index}.value`} + inputProps={{ + placeholder: + 'variable value', + }} + /> + )} +
+ {isEditable && ( + + arrayHelpers.remove(index) + } + /> + )} +
+ ); - {isEditable && ( - - arrayHelpers.remove( - index - ) - } - /> - )} -
- ) - ) - : null; + const fields = values.variables.length ? ( + + renderVariableConfigRow( + index, + varConfig as IDataDocMetaVariableWithId + ) + } + onMove={arrayHelpers.move} + /> + ) : null; const controlDOM = isEditable && (
@@ -169,7 +199,7 @@ export const DataDocTemplateVarForm: React.FunctionComponent< onClick={() => arrayHelpers.push({ name: '', - valueType: 'string', + type: 'string', value: '', }) } diff --git a/querybook/webapp/components/DataDocTemplateButton/helpers.ts b/querybook/webapp/components/DataDocTemplateButton/helpers.ts index 26cbc146b..0dc0e8b60 100644 --- a/querybook/webapp/components/DataDocTemplateButton/helpers.ts +++ b/querybook/webapp/components/DataDocTemplateButton/helpers.ts @@ -1,24 +1,8 @@ -import { isBoolean, isNumber } from 'lodash'; +import { IDataDocMetaVariable, TDataDocMetaVariableType } from 'const/datadoc'; -export const SUPPORTED_TYPES = ['boolean', 'number', 'string'] as const; -export type TSupportedTypes = typeof SUPPORTED_TYPES[number]; - -type TTemplateVariableType = boolean | number | string; -export type TTemplateVariableDict = Record; - -export function detectVariableType(value: any): TSupportedTypes { - if (isBoolean(value)) { - return 'boolean'; - } - if (isNumber(value)) { - return 'number'; - } - return 'string'; -} - -export function getVariableValueByType( +function getVariableValueByType( value: any, - valueType: TSupportedTypes + valueType: TDataDocMetaVariableType ): any { if (value !== null) { if (valueType === 'number') { @@ -29,3 +13,13 @@ export function getVariableValueByType( } return value; } + +export function typeCastVariables( + variables: IDataDocMetaVariable[] +): IDataDocMetaVariable[] { + return variables.map(({ name, type, value }) => ({ + name, + type, + value: getVariableValueByType(value, type), + })); +} diff --git a/querybook/webapp/const/datadoc.ts b/querybook/webapp/const/datadoc.ts index 036a30df4..d8124d008 100644 --- a/querybook/webapp/const/datadoc.ts +++ b/querybook/webapp/const/datadoc.ts @@ -53,7 +53,15 @@ export interface IDataChartCell extends IDataCellBase { export type IDataCell = IDataQueryCell | IDataTextCell | IDataChartCell; export type DataCellUpdateFields = Partial>; -export type TDataDocMetaVariableType = 'string' | 'boolean' | 'number'; +export const TEMPLATED_VAR_SUPPORTED_TYPES = [ + 'boolean', + 'number', + 'string', +] as const; +export type TDataDocMetaVariableType = + typeof TEMPLATED_VAR_SUPPORTED_TYPES[number]; +export type TTemplateVariableType = boolean | number | string; +export type TDataDocMetaVariableDict = Record; export interface IDataDocMetaVariable { name: string; value: any; @@ -74,7 +82,7 @@ export interface IDataDoc { updated_at: number; meta: IDataDocMeta; - meta_variables: Record; + meta_variables: TDataDocMetaVariableDict; title: string; cells?: number[]; diff --git a/querybook/webapp/redux/dataDoc/types.ts b/querybook/webapp/redux/dataDoc/types.ts index 83010a07d..b63dc1483 100644 --- a/querybook/webapp/redux/dataDoc/types.ts +++ b/querybook/webapp/redux/dataDoc/types.ts @@ -47,7 +47,7 @@ export interface IReceiveDataDocAction extends Action { export interface IReceiveDataDocUpdateAction extends Action { type: '@@dataDoc/RECEIVE_DATA_DOC_UPDATE'; payload: { - dataDoc: IDataDoc; + dataDoc: Partial & Pick; dataDocCellById?: Record; }; } diff --git a/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts b/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts index e22c73831..b84ed1a79 100644 --- a/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts +++ b/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts @@ -3,6 +3,7 @@ import { IDataDocEditor } from 'const/datadoc'; import dataDocSocket, { IDataDocSocketEvent, } from 'lib/data-doc/datadoc-socketio'; +import { isEmpty, isEqual } from 'lodash'; import { deserializeCell, fetchDataDoc, @@ -58,6 +59,38 @@ export function openDataDoc(docId: number): ThunkResult> { dataDoc, }, }); + } else { + // Even if it is sameOrigin, dervied fields also needs to be updated + const derviedFields = ['meta_variables']; + const docId = rawDataDoc.id; + + dispatch((thunkDispatch, getState) => { + const state = getState(); + const oldDoc = + state.dataDoc.dataDocById[docId] ?? {}; + + // make a partial dataDoc with all the dervied fields + const newDocFields = {}; + for (const field of derviedFields) { + if ( + !isEqual(oldDoc[field], rawDataDoc[field]) + ) { + newDocFields[field] = rawDataDoc[field]; + } + } + + if (!isEmpty(newDocFields)) { + thunkDispatch({ + type: '@@dataDoc/RECEIVE_DATA_DOC_UPDATE', + payload: { + dataDoc: { + id: docId, + ...newDocFields, + }, + }, + }); + } + }); } }, }, diff --git a/querybook/webapp/ui/DraggableList/DraggableIcon.scss b/querybook/webapp/ui/DraggableList/DraggableIcon.scss new file mode 100644 index 000000000..a109dfaec --- /dev/null +++ b/querybook/webapp/ui/DraggableList/DraggableIcon.scss @@ -0,0 +1,3 @@ +.DraggableIcon { + cursor: grab; +} diff --git a/querybook/webapp/ui/DraggableList/DraggableIcon.tsx b/querybook/webapp/ui/DraggableList/DraggableIcon.tsx new file mode 100644 index 000000000..ab29ff5d4 --- /dev/null +++ b/querybook/webapp/ui/DraggableList/DraggableIcon.tsx @@ -0,0 +1,21 @@ +import clsx from 'clsx'; +import React from 'react'; + +import { Icon } from 'ui/Icon/Icon'; + +import './DraggableIcon.scss'; + +export interface IDraggableIconProps { + size?: number; + className?: string; +} +export const DraggableIcon: React.FC = ({ + size = 20, + className = '', +}) => ( + +); diff --git a/querybook/webapp/ui/Icon/LucideIcons.ts b/querybook/webapp/ui/Icon/LucideIcons.ts index 73054bec5..158206cf9 100644 --- a/querybook/webapp/ui/Icon/LucideIcons.ts +++ b/querybook/webapp/ui/Icon/LucideIcons.ts @@ -51,6 +51,7 @@ import { FileText, Filter, FormInput, + GripVertical, HelpCircle, Home, Info, @@ -156,6 +157,7 @@ const AllLucideIcons = { FileText, Filter, FormInput, + GripVertical, HelpCircle, Home, Info, diff --git a/yarn.lock b/yarn.lock index db135bae4..56c0cc0ea 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10613,15 +10613,15 @@ form-data@~2.3.2: combined-stream "1.0.6" mime-types "^2.1.12" -formik@2.2.6: - version "2.2.6" - resolved "https://registry.yarnpkg.com/formik/-/formik-2.2.6.tgz#378a4bafe4b95caf6acf6db01f81f3fe5147559d" - integrity sha512-Kxk2zQRafy56zhLmrzcbryUpMBvT0tal5IvcifK5+4YNGelKsnrODFJ0sZQRMQboblWNym4lAW3bt+tf2vApSA== +formik@2.2.9: + version "2.2.9" + resolved "https://registry.yarnpkg.com/formik/-/formik-2.2.9.tgz#8594ba9c5e2e5cf1f42c5704128e119fc46232d0" + integrity sha512-LQLcISMmf1r5at4/gyJigGn0gOwFbeEAlji+N9InZF6LIMXnFNkO42sCI8Jt84YZggpD4cPWObAZaxpEFtSzNA== dependencies: deepmerge "^2.1.1" hoist-non-react-statics "^3.3.0" - lodash "^4.17.14" - lodash-es "^4.17.14" + lodash "^4.17.21" + lodash-es "^4.17.21" react-fast-compare "^2.0.1" tiny-warning "^1.0.2" tslib "^1.10.0" @@ -13197,11 +13197,6 @@ lodash-decorators@6.0.1: dependencies: tslib "^1.9.2" -lodash-es@^4.17.14: - version "4.17.14" - resolved "https://registry.yarnpkg.com/lodash-es/-/lodash-es-4.17.14.tgz#12a95a963cc5955683cee3b74e85458954f37ecc" - integrity sha512-7zchRrGa8UZXjD/4ivUWP1867jDkhzTG2c/uj739utSd7O/pFFdxspCemIFKEEjErbcqRzn8nKnGsi7mvTgRPA== - lodash-es@^4.17.21: version "4.17.21" resolved "https://registry.yarnpkg.com/lodash-es/-/lodash-es-4.17.21.tgz#43e626c46e6591b7750beb2b50117390c609e3ee" From 203d3e68fbd9a31da127e0efc8c27cfc180b0ae3 Mon Sep 17 00:00:00 2001 From: cgu Date: Thu, 22 Dec 2022 17:36:45 -0800 Subject: [PATCH 3/5] resolved comments --- querybook/server/lib/data_doc/meta.py | 14 ++++++++++++++ querybook/server/models/datadoc.py | 7 ++++++- .../DataDocTemplateVarForm.tsx | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/querybook/server/lib/data_doc/meta.py b/querybook/server/lib/data_doc/meta.py index b8afa8ee2..8d090d3bf 100644 --- a/querybook/server/lib/data_doc/meta.py +++ b/querybook/server/lib/data_doc/meta.py @@ -15,6 +15,20 @@ def check_variable_type(val: Any): def convert_if_legacy_datadoc_meta_v0(datadoc_meta: Dict) -> DataDocMeta: + """Converts the old meta format which is only a dictionary of templated variables + to a more general format that has templated vars as array plus other fields + + Old meta: `{ "foo": "bar" }` + New meta: `{ "variables": ["name": "foo", "type": "string", "value": "bar", ...] }` + + If the new meta is passed in, no change would be made. + + Args: + datadoc_meta (Dict): Old/New meta format + + Returns: + DataDocMeta: New meta format + """ if isinstance(datadoc_meta.get("variables"), list): return datadoc_meta diff --git a/querybook/server/models/datadoc.py b/querybook/server/models/datadoc.py index 6b64bd4a6..26c15cfdd 100644 --- a/querybook/server/models/datadoc.py +++ b/querybook/server/models/datadoc.py @@ -67,7 +67,12 @@ def meta(self, new_meta): self._meta = new_meta @property - def meta_variables(self): + def meta_variables(self) -> dict: + """ + The field is used to generate a dictionary of templated variables. + It is used in scheduled data docs and passed to frontend to generate + templated queries + """ return get_datadoc_meta_variables_dict(self.meta) def to_dict(self, with_cells=False): diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx index a188830dc..417d64317 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx @@ -53,7 +53,7 @@ const templatedVarSchema = Yup.object().shape({ interface IDataDocMetaVariableWithId extends IDataDocMetaVariable { id: string; } -const templatedVarUniqueIdPrefix = '_tvar'; +const templatedVarUniqueIdPrefix = 'tvar_'; const defaultTemplatedVariables: IDataDocMetaVariableWithId[] = [ { From 74371a832ed6905c0ec0a86c5fdb079657454896 Mon Sep 17 00:00:00 2001 From: cgu Date: Thu, 22 Dec 2022 21:45:22 -0800 Subject: [PATCH 4/5] make templated variables in the frontend array based --- .../server/datasources/query_execution.py | 12 ++++--- querybook/server/lib/data_doc/meta.py | 12 +++---- querybook/server/models/datadoc.py | 10 +++--- .../webapp/components/DataDoc/DataDoc.tsx | 2 +- .../components/DataDocCell/DataDocCell.tsx | 4 +-- .../DataDocQueryCell/DataDocQueryCell.tsx | 6 ++-- .../DataDocTemplateButton.tsx | 9 +++-- .../DataDocTemplateCell.tsx | 11 +++--- .../DataDocTemplateVarForm.tsx | 26 ++++++-------- .../DataDocTemplateButton/helpers.ts | 11 ++++++ .../QueryComposer/QueryComposer.tsx | 36 ++++++++++++++----- .../components/QueryComposer/RunQuery.tsx | 5 +-- .../QuerySnippetInsertionModal.tsx | 5 +-- .../QuerySnippetView.tsx | 4 ++- .../TemplateQueryView/TemplatedQueryView.tsx | 5 +-- querybook/webapp/const/adhocQuery.ts | 4 ++- querybook/webapp/const/datadoc.ts | 6 +++- querybook/webapp/lib/templated-query/index.ts | 3 +- .../dataDocWebsocket/dataDocWebsocket.ts | 33 ----------------- querybook/webapp/resource/queryExecution.ts | 5 +-- 20 files changed, 110 insertions(+), 99 deletions(-) diff --git a/querybook/server/datasources/query_execution.py b/querybook/server/datasources/query_execution.py index 864f37143..027d8f204 100644 --- a/querybook/server/datasources/query_execution.py +++ b/querybook/server/datasources/query_execution.py @@ -1,5 +1,3 @@ -from typing import Dict - from flask import abort, Response, redirect from flask_login import current_user @@ -20,6 +18,8 @@ render_templated_query, ) from lib.form import validate_form +from lib.data_doc.meta import var_config_to_var_dict +from lib.data_doc.doc_types import DataDocMetaVarConfig from const.query_execution import QueryExecutionExportStatus, QueryExecutionStatus from const.datasources import RESOURCE_NOT_FOUND_STATUS_CODE from logic import ( @@ -450,9 +450,13 @@ def poll_export_statement_execution_result(task_id): @register("/query_execution/templated_query/", methods=["POST"]) -def get_templated_query(query: str, variables: Dict[str, str], engine_id: int): +def get_templated_query( + query: str, var_config: list[DataDocMetaVarConfig], engine_id: int +): try: - return render_templated_query(query, variables, engine_id) + return render_templated_query( + query, var_config_to_var_dict(var_config), engine_id + ) except QueryTemplatingError as e: raise RequestException(e) diff --git a/querybook/server/lib/data_doc/meta.py b/querybook/server/lib/data_doc/meta.py index 8d090d3bf..2a6fc4281 100644 --- a/querybook/server/lib/data_doc/meta.py +++ b/querybook/server/lib/data_doc/meta.py @@ -1,5 +1,5 @@ from typing import Dict, Any -from .doc_types import DataDocMeta +from .doc_types import DataDocMeta, DataDocMetaVarConfig def check_variable_type(val: Any): @@ -47,13 +47,13 @@ def convert_if_legacy_datadoc_meta(datadoc_meta: Dict) -> DataDocMeta: return datadoc_meta -def get_datadoc_meta_variables_dict(datadoc_meta: DataDocMeta) -> Dict: - variables = {} +def var_config_to_var_dict(variables: list[DataDocMetaVarConfig]) -> Dict: + var_dict = {} - for config in datadoc_meta.get("variables", []): - variables[config["name"]] = config["value"] + for config in variables: + var_dict[config["name"]] = config["value"] - return variables + return var_dict valid_meta_keys = ["variables"] diff --git a/querybook/server/models/datadoc.py b/querybook/server/models/datadoc.py index 26c15cfdd..6667d183c 100644 --- a/querybook/server/models/datadoc.py +++ b/querybook/server/models/datadoc.py @@ -8,9 +8,10 @@ from lib.sqlalchemy import CRUDMixin from lib.data_doc.meta import ( convert_if_legacy_datadoc_meta, - get_datadoc_meta_variables_dict, + var_config_to_var_dict, validate_datadoc_meta, ) +from lib.data_doc.doc_types import DataDocMeta Base = db.Base @@ -55,11 +56,11 @@ class DataDoc(Base, CRUDMixin): ) @hybrid_property - def meta(self): + def meta(self) -> DataDocMeta: return convert_if_legacy_datadoc_meta(self._meta or {}) @meta.setter - def meta(self, new_meta): + def meta(self, new_meta: DataDocMeta): is_valid = validate_datadoc_meta(new_meta) if not is_valid: raise ValueError("Invalid DataDoc.meta") @@ -73,7 +74,7 @@ def meta_variables(self) -> dict: It is used in scheduled data docs and passed to frontend to generate templated queries """ - return get_datadoc_meta_variables_dict(self.meta) + return var_config_to_var_dict(self.meta.get("variables", [])) def to_dict(self, with_cells=False): data_doc_dict = { @@ -85,7 +86,6 @@ def to_dict(self, with_cells=False): "created_at": self.created_at, "updated_at": self.updated_at, "meta": self.meta, - "meta_variables": self.meta_variables, "title": self.title, } diff --git a/querybook/webapp/components/DataDoc/DataDoc.tsx b/querybook/webapp/components/DataDoc/DataDoc.tsx index 8ec064906..913df9036 100644 --- a/querybook/webapp/components/DataDoc/DataDoc.tsx +++ b/querybook/webapp/components/DataDoc/DataDoc.tsx @@ -633,7 +633,7 @@ class DataDocComponent extends React.PureComponent { key={cell.id} docId={dataDoc.id} numberOfCells={dataDoc.dataDocCells.length} - templatedVariables={dataDoc.meta_variables} + templatedVariables={dataDoc.meta?.variables ?? []} cell={cell} index={index} queryIndexInDoc={queryIndexInDoc} diff --git a/querybook/webapp/components/DataDocCell/DataDocCell.tsx b/querybook/webapp/components/DataDocCell/DataDocCell.tsx index 0224d7d9c..9a25aa8a8 100644 --- a/querybook/webapp/components/DataDocCell/DataDocCell.tsx +++ b/querybook/webapp/components/DataDocCell/DataDocCell.tsx @@ -11,7 +11,7 @@ import { UserAvatar } from 'components/UserBadge/UserAvatar'; import { DataCellUpdateFields, IDataCell, - TDataDocMetaVariableDict, + TDataDocMetaVariables, } from 'const/datadoc'; import { DataDocContext } from 'context/DataDoc'; import { useMakeSelector } from 'hooks/redux/useMakeSelector'; @@ -26,7 +26,7 @@ import './DataDocCell.scss'; interface IDataDocCellProps { docId: number; numberOfCells: number; - templatedVariables: TDataDocMetaVariableDict; + templatedVariables: TDataDocMetaVariables; cell: IDataCell; index: number; diff --git a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx index 0e80904ed..9e7383214 100644 --- a/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx +++ b/querybook/webapp/components/DataDocQueryCell/DataDocQueryCell.tsx @@ -21,7 +21,7 @@ import { QuerySnippetInsertionModal } from 'components/QuerySnippetInsertionModa import { TemplatedQueryView } from 'components/TemplateQueryView/TemplatedQueryView'; import { TranspileQueryModal } from 'components/TranspileQueryModal/TranspileQueryModal'; import { UDFForm } from 'components/UDFForm/UDFForm'; -import { IDataQueryCellMeta } from 'const/datadoc'; +import { IDataQueryCellMeta, TDataDocMetaVariables } from 'const/datadoc'; import type { IQueryEngine, IQueryTranspiler } from 'const/queryEngine'; import CodeMirror from 'lib/codemirror'; import { createSQLLinter } from 'lib/codemirror/codemirror-lint'; @@ -73,7 +73,7 @@ interface IOwnProps { cellId: number; queryIndexInDoc: number; - templatedVariables: Record; + templatedVariables: TDataDocMetaVariables; shouldFocus: boolean; isFullScreen: boolean; @@ -364,7 +364,7 @@ class DataDocQueryCellComponent extends React.PureComponent { @bind public async getTransformedQuery() { - const { templatedVariables = {} } = this.props; + const { templatedVariables = [] } = this.props; const { query } = this.state; const selectedRange = this.queryEditorRef.current && diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx index 1a6e845fb..36214bfd4 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx @@ -30,10 +30,13 @@ export const DataDocTemplateButton: React.FunctionComponent = ({ > { + variables={dataDoc.meta?.variables} + onSave={(variables) => { setShowTemplateForm(false); - return changeDataDocMeta(dataDoc.id, meta); + return changeDataDocMeta(dataDoc.id, { + ...dataDoc.meta, + variables, + }); }} /> diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx index 8dc480caa..23ba3116e 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx @@ -61,12 +61,15 @@ export const DataDocTemplateCell: React.FunctionComponent = ({
{ - if (meta.variables.length === 0) { + variables={dataDoc.meta?.variables} + onSave={(newVariables) => { + if (newVariables.length === 0) { setShowFacade(true); } - return changeDataDocMeta(dataDoc.id, meta); + return changeDataDocMeta(dataDoc.id, { + ...dataDoc.meta, + variables: newVariables, + }); }} /> diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx index 417d64317..c08b30376 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateVarForm.tsx @@ -21,8 +21,8 @@ import { typeCastVariables } from './helpers'; import './DataDocTemplateVarForm.scss'; export interface IDataDocTemplateVarFormProps { - onSave: (meta: IDataDocMeta) => Promise; - meta: IDataDocMeta; + onSave: (variables: IDataDocMeta['variables']) => Promise; + variables: IDataDocMeta['variables']; isEditable: boolean; } @@ -66,34 +66,28 @@ const defaultTemplatedVariables: IDataDocMetaVariableWithId[] = [ export const DataDocTemplateVarForm: React.FunctionComponent< IDataDocTemplateVarFormProps -> = ({ onSave, meta, isEditable }) => { +> = ({ onSave, variables, isEditable }) => { const initialValue = useMemo( () => ({ - variables: meta.variables?.length - ? meta.variables.map((varConfig) => ({ + variables: variables?.length + ? variables.map((varConfig) => ({ ...varConfig, id: uniqueId(templatedVarUniqueIdPrefix), })) : defaultTemplatedVariables, }), - [meta?.variables] + [variables] ); const handleSaveMeta = useCallback( - (values: typeof initialValue) => { - const newMeta: IDataDocMeta = { - ...meta, - // This also gets rid of the id field - variables: typeCastVariables(values.variables), - }; - return toast.promise(onSave(newMeta), { + (values: typeof initialValue) => + toast.promise(onSave(typeCastVariables(values.variables)), { loading: 'Saving variables', success: 'Variables saved!', error: 'Failed to save variables', - }); - }, - [onSave, meta] + }), + [onSave] ); return ( diff --git a/querybook/webapp/components/DataDocTemplateButton/helpers.ts b/querybook/webapp/components/DataDocTemplateButton/helpers.ts index 0dc0e8b60..9d91a5466 100644 --- a/querybook/webapp/components/DataDocTemplateButton/helpers.ts +++ b/querybook/webapp/components/DataDocTemplateButton/helpers.ts @@ -1,4 +1,15 @@ import { IDataDocMetaVariable, TDataDocMetaVariableType } from 'const/datadoc'; +import { isBoolean, isNumber } from 'lodash'; + +export function detectVariableType(value: any): TDataDocMetaVariableType { + if (isBoolean(value)) { + return 'boolean'; + } + if (isNumber(value)) { + return 'number'; + } + return 'string'; +} function getVariableValueByType( value: any, diff --git a/querybook/webapp/components/QueryComposer/QueryComposer.tsx b/querybook/webapp/components/QueryComposer/QueryComposer.tsx index 60a502464..7ddae9b72 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposer.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposer.tsx @@ -12,6 +12,7 @@ import { useDispatch, useSelector } from 'react-redux'; import { DataDocTemplateInfoButton } from 'components/DataDocTemplateButton/DataDocTemplateInfoButton'; import { DataDocTemplateVarForm } from 'components/DataDocTemplateButton/DataDocTemplateVarForm'; +import { detectVariableType } from 'components/DataDocTemplateButton/helpers'; import { BoundQueryEditor } from 'components/QueryEditor/BoundQueryEditor'; import { IQueryEditorHandles } from 'components/QueryEditor/QueryEditor'; import { @@ -26,6 +27,7 @@ import { import { TemplatedQueryView } from 'components/TemplateQueryView/TemplatedQueryView'; import { TranspileQueryModal } from 'components/TranspileQueryModal/TranspileQueryModal'; import { UDFForm } from 'components/UDFForm/UDFForm'; +import { IDataDocMetaVariable } from 'const/datadoc'; import KeyMap from 'const/keyMap'; import { IQueryEngine } from 'const/queryEngine'; import { ISearchOptions, ISearchResult } from 'const/searchAndReplace'; @@ -155,12 +157,29 @@ const useRowLimit = (dispatch: Dispatch, environmentId: number) => { }; const useTemplatedVariables = (dispatch: Dispatch, environmentId: number) => { - const templatedVariables = useSelector( - (state: IStoreState) => - state.adhocQuery[environmentId]?.templatedVariables ?? {} - ); + const templatedVariables = useSelector((state: IStoreState) => { + const templatedVariablesInState = + state.adhocQuery[environmentId]?.templatedVariables ?? []; + if (Array.isArray(templatedVariablesInState)) { + return templatedVariablesInState; + } else { + // In the older version, we are storing it as a dictionary + // so we need to convert to the new format + const oldTemplatedVarConfig: Record = + templatedVariablesInState; + const newConfig: IDataDocMetaVariable[] = []; + Object.entries(oldTemplatedVarConfig).forEach(([key, value]) => { + newConfig.push({ + name: key, + value, + type: detectVariableType(value), + }); + }); + return newConfig; + } + }); const setTemplatedVariables = useCallback( - (newVariables: Record) => + (newVariables: IDataDocMetaVariable[]) => dispatch( adhocQueryActions.receiveAdhocQuery( { templatedVariables: newVariables }, @@ -373,6 +392,7 @@ const QueryComposer: React.FC = () => { dispatch, environmentId ); + const [showRenderedTemplateModal, setShowRenderedTemplateModal] = useState(false); @@ -597,9 +617,9 @@ const QueryComposer: React.FC = () => { > { - setTemplatedVariables(meta); + variables={templatedVariables} + onSave={async (newVariables) => { + setTemplatedVariables(newVariables); setShowTemplateForm(false); toast.success('Variables saved!'); }} diff --git a/querybook/webapp/components/QueryComposer/RunQuery.tsx b/querybook/webapp/components/QueryComposer/RunQuery.tsx index 619a6f960..2adb261db 100644 --- a/querybook/webapp/components/QueryComposer/RunQuery.tsx +++ b/querybook/webapp/components/QueryComposer/RunQuery.tsx @@ -1,6 +1,7 @@ import React from 'react'; import toast from 'react-hot-toast'; +import { TDataDocMetaVariables } from 'const/datadoc'; import { IQueryEngine } from 'const/queryEngine'; import { sendConfirm } from 'lib/querybookUI'; import { getDroppedTables } from 'lib/sql-helper/sql-checker'; @@ -16,7 +17,7 @@ import { ShowMoreText } from 'ui/ShowMoreText/ShowMoreText'; export async function transformQuery( query: string, - templatedVariables: Record, + templatedVariables: TDataDocMetaVariables, engine: IQueryEngine, rowLimit: Nullable ): Promise { @@ -57,7 +58,7 @@ export async function runQuery( async function transformTemplatedQuery( query: string, - templatedVariables: Record, + templatedVariables: TDataDocMetaVariables, engineId: number ) { try { diff --git a/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetInsertionModal.tsx b/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetInsertionModal.tsx index d71901f4e..49aa50a09 100644 --- a/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetInsertionModal.tsx +++ b/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetInsertionModal.tsx @@ -20,7 +20,6 @@ import './QuerySnippetInsertionModal.scss'; interface IOwnProps { onInsert: (query: string) => any; - onDismiss: () => any; onHide: () => void; } type StateProps = ReturnType; @@ -71,13 +70,11 @@ class QuerySnippetInsertionModalComponent extends React.PureComponent< @bind public onInsertSnippet(query: string) { - const { onInsert, onDismiss } = this.props; + const { onInsert } = this.props; if (onInsert) { onInsert(query); } - - onDismiss(); } @bind diff --git a/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetView.tsx b/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetView.tsx index 8cb08f3d0..396554a5c 100644 --- a/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetView.tsx +++ b/querybook/webapp/components/QuerySnippetInsertionModal/QuerySnippetView.tsx @@ -68,7 +68,9 @@ export class QuerySnippetView extends React.PureComponent< if (templatedVariables.length) { renderedQuery = await renderTemplatedQuery( context, - this.state.templatedQueryForm, + Object.entries(this.state.templatedQueryForm).map( + ([key, value]) => ({ name: key, value }) + ), engineId ); } diff --git a/querybook/webapp/components/TemplateQueryView/TemplatedQueryView.tsx b/querybook/webapp/components/TemplateQueryView/TemplatedQueryView.tsx index 70e7f15d5..ed51ac417 100644 --- a/querybook/webapp/components/TemplateQueryView/TemplatedQueryView.tsx +++ b/querybook/webapp/components/TemplateQueryView/TemplatedQueryView.tsx @@ -1,5 +1,6 @@ import React from 'react'; +import { TDataDocMetaVariables } from 'const/datadoc'; import { useResource } from 'hooks/useResource'; import { formatError } from 'lib/utils/error'; import { TemplatedQueryResource } from 'resource/queryExecution'; @@ -13,7 +14,7 @@ import './TemplatedQueryView.scss'; export interface ITemplatedQueryViewProps { query: string; - templatedVariables: Record; + templatedVariables: TDataDocMetaVariables; engineId: number; onRunQueryClick?: () => void; } @@ -36,7 +37,7 @@ export const TemplatedQueryView: React.FC = ({ templatedVariables, engineId ), - [query, templatedVariables] + [query, templatedVariables, engineId] ) ); diff --git a/querybook/webapp/const/adhocQuery.ts b/querybook/webapp/const/adhocQuery.ts index 7b187d006..d7e53edd6 100644 --- a/querybook/webapp/const/adhocQuery.ts +++ b/querybook/webapp/const/adhocQuery.ts @@ -1,6 +1,8 @@ +import { IDataDocMetaVariable } from './datadoc'; + export interface IAdhocQuery { query?: string; - templatedVariables?: Record; + templatedVariables?: IDataDocMetaVariable[]; engineId?: number; executionId?: number; rowLimit?: number; diff --git a/querybook/webapp/const/datadoc.ts b/querybook/webapp/const/datadoc.ts index d8124d008..19aaf7405 100644 --- a/querybook/webapp/const/datadoc.ts +++ b/querybook/webapp/const/datadoc.ts @@ -1,4 +1,5 @@ import { ContentState } from 'draft-js'; +import { WithOptional } from 'lib/typescript'; import { Edge, Node } from 'reactflow'; import { IChartConfig } from './dataDocChart'; @@ -67,6 +68,10 @@ export interface IDataDocMetaVariable { value: any; type: TDataDocMetaVariableType; } +export type TDataDocMetaVariables = Array< + WithOptional +>; + export interface IDataDocMeta { variables: IDataDocMetaVariable[]; } @@ -82,7 +87,6 @@ export interface IDataDoc { updated_at: number; meta: IDataDocMeta; - meta_variables: TDataDocMetaVariableDict; title: string; cells?: number[]; diff --git a/querybook/webapp/lib/templated-query/index.ts b/querybook/webapp/lib/templated-query/index.ts index 8fcae255e..79e1fea30 100644 --- a/querybook/webapp/lib/templated-query/index.ts +++ b/querybook/webapp/lib/templated-query/index.ts @@ -1,3 +1,4 @@ +import { TDataDocMetaVariables } from 'const/datadoc'; import { TemplatedQueryResource } from 'resource/queryExecution'; export async function getTemplatedQueryVariables(query: string) { @@ -11,7 +12,7 @@ export async function getTemplatedQueryVariables(query: string) { export async function renderTemplatedQuery( query: string, - variables: Record, + variables: TDataDocMetaVariables, engineId: number ) { const { data } = await TemplatedQueryResource.renderTemplatedQuery( diff --git a/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts b/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts index b84ed1a79..e22c73831 100644 --- a/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts +++ b/querybook/webapp/redux/dataDocWebsocket/dataDocWebsocket.ts @@ -3,7 +3,6 @@ import { IDataDocEditor } from 'const/datadoc'; import dataDocSocket, { IDataDocSocketEvent, } from 'lib/data-doc/datadoc-socketio'; -import { isEmpty, isEqual } from 'lodash'; import { deserializeCell, fetchDataDoc, @@ -59,38 +58,6 @@ export function openDataDoc(docId: number): ThunkResult> { dataDoc, }, }); - } else { - // Even if it is sameOrigin, dervied fields also needs to be updated - const derviedFields = ['meta_variables']; - const docId = rawDataDoc.id; - - dispatch((thunkDispatch, getState) => { - const state = getState(); - const oldDoc = - state.dataDoc.dataDocById[docId] ?? {}; - - // make a partial dataDoc with all the dervied fields - const newDocFields = {}; - for (const field of derviedFields) { - if ( - !isEqual(oldDoc[field], rawDataDoc[field]) - ) { - newDocFields[field] = rawDataDoc[field]; - } - } - - if (!isEmpty(newDocFields)) { - thunkDispatch({ - type: '@@dataDoc/RECEIVE_DATA_DOC_UPDATE', - payload: { - dataDoc: { - id: docId, - ...newDocFields, - }, - }, - }); - } - }); } }, }, diff --git a/querybook/webapp/resource/queryExecution.ts b/querybook/webapp/resource/queryExecution.ts index de1c0f76b..775d638d0 100644 --- a/querybook/webapp/resource/queryExecution.ts +++ b/querybook/webapp/resource/queryExecution.ts @@ -1,4 +1,5 @@ import type { IAccessRequest } from 'const/accessRequest'; +import { TDataDocMetaVariables } from 'const/datadoc'; import { IQueryTranspiler, ITranspiledQuery } from 'const/queryEngine'; import { IQueryError, @@ -149,14 +150,14 @@ export const TemplatedQueryResource = { }), renderTemplatedQuery: ( query: string, - variables: Record, + varConfig: TDataDocMetaVariables, engineId: number ) => ds.save( '/query_execution/templated_query/', { query, - variables, + var_config: varConfig, engine_id: engineId, }, false From cb3a152fc139b4c4cd0dc197e2969b2cb3c7ccb1 Mon Sep 17 00:00:00 2001 From: cgu Date: Thu, 22 Dec 2022 21:53:55 -0800 Subject: [PATCH 5/5] meta.variables should always be defined --- querybook/server/models/datadoc.py | 3 +-- querybook/webapp/components/DataDoc/DataDoc.tsx | 2 +- .../DataDocTemplateButton/DataDocTemplateButton.tsx | 2 +- .../DataDocTemplateButton/DataDocTemplateCell.tsx | 4 ++-- .../webapp/components/QueryComposer/QueryComposer.tsx | 7 ++++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/querybook/server/models/datadoc.py b/querybook/server/models/datadoc.py index 6667d183c..c9260a461 100644 --- a/querybook/server/models/datadoc.py +++ b/querybook/server/models/datadoc.py @@ -71,8 +71,7 @@ def meta(self, new_meta: DataDocMeta): def meta_variables(self) -> dict: """ The field is used to generate a dictionary of templated variables. - It is used in scheduled data docs and passed to frontend to generate - templated queries + It is used in scheduled data docs """ return var_config_to_var_dict(self.meta.get("variables", [])) diff --git a/querybook/webapp/components/DataDoc/DataDoc.tsx b/querybook/webapp/components/DataDoc/DataDoc.tsx index 913df9036..7f230e7fc 100644 --- a/querybook/webapp/components/DataDoc/DataDoc.tsx +++ b/querybook/webapp/components/DataDoc/DataDoc.tsx @@ -633,7 +633,7 @@ class DataDocComponent extends React.PureComponent { key={cell.id} docId={dataDoc.id} numberOfCells={dataDoc.dataDocCells.length} - templatedVariables={dataDoc.meta?.variables ?? []} + templatedVariables={dataDoc.meta.variables} cell={cell} index={index} queryIndexInDoc={queryIndexInDoc} diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx index 36214bfd4..290ad5bf5 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateButton.tsx @@ -30,7 +30,7 @@ export const DataDocTemplateButton: React.FunctionComponent = ({ > { setShowTemplateForm(false); return changeDataDocMeta(dataDoc.id, { diff --git a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx index 23ba3116e..60eb033a8 100644 --- a/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx +++ b/querybook/webapp/components/DataDocTemplateButton/DataDocTemplateCell.tsx @@ -19,7 +19,7 @@ export const DataDocTemplateCell: React.FunctionComponent = ({ isEditable, }) => { const hasMeta = useMemo( - () => dataDoc.meta?.variables?.length > 0, + () => dataDoc.meta.variables.length > 0, [dataDoc.meta] ); const [showFacade, setShowFacade] = useState(!hasMeta && isEditable); @@ -61,7 +61,7 @@ export const DataDocTemplateCell: React.FunctionComponent = ({ { if (newVariables.length === 0) { setShowFacade(true); diff --git a/querybook/webapp/components/QueryComposer/QueryComposer.tsx b/querybook/webapp/components/QueryComposer/QueryComposer.tsx index 7ddae9b72..a987176cd 100644 --- a/querybook/webapp/components/QueryComposer/QueryComposer.tsx +++ b/querybook/webapp/components/QueryComposer/QueryComposer.tsx @@ -160,9 +160,9 @@ const useTemplatedVariables = (dispatch: Dispatch, environmentId: number) => { const templatedVariables = useSelector((state: IStoreState) => { const templatedVariablesInState = state.adhocQuery[environmentId]?.templatedVariables ?? []; - if (Array.isArray(templatedVariablesInState)) { - return templatedVariablesInState; - } else { + + if (!Array.isArray(templatedVariablesInState)) { + // This whole block is only here for legacy reason // In the older version, we are storing it as a dictionary // so we need to convert to the new format const oldTemplatedVarConfig: Record = @@ -177,6 +177,7 @@ const useTemplatedVariables = (dispatch: Dispatch, environmentId: number) => { }); return newConfig; } + return templatedVariablesInState; }); const setTemplatedVariables = useCallback( (newVariables: IDataDocMetaVariable[]) =>