From f2a79f5ba6bf50e9f151dd4c610a087021a8ae17 Mon Sep 17 00:00:00 2001 From: Levko Kravets Date: Fri, 27 Dec 2019 15:03:29 +0200 Subject: [PATCH 1/9] Refine Query Source: extract query flags, data sources and schema to hooks --- .../components/queries/QueryEditor/index.jsx | 40 ++--- client/app/pages/queries/QuerySource.jsx | 142 +++++------------- .../queries/components/QueryPageHeader.jsx | 55 ++++--- .../queries/utils/useDataSourceSchema.js | 63 ++++++++ .../queries/utils/useQueryDataSources.js | 29 ++++ .../app/pages/queries/utils/useQueryFlags.js | 41 +++++ 6 files changed, 219 insertions(+), 151 deletions(-) create mode 100644 client/app/pages/queries/utils/useDataSourceSchema.js create mode 100644 client/app/pages/queries/utils/useQueryDataSources.js create mode 100644 client/app/pages/queries/utils/useQueryFlags.js diff --git a/client/app/components/queries/QueryEditor/index.jsx b/client/app/components/queries/QueryEditor/index.jsx index 869e913334..fd58cc9c0b 100644 --- a/client/app/components/queries/QueryEditor/index.jsx +++ b/client/app/components/queries/QueryEditor/index.jsx @@ -1,4 +1,4 @@ -import React, { useEffect, useMemo, useRef, useState, useCallback, useImperativeHandle } from "react"; +import React, { useEffect, useMemo, useState, useCallback, useImperativeHandle } from "react"; import PropTypes from "prop-types"; import cx from "classnames"; import { AceEditor, snippetsModule, updateSchemaCompleter } from "./ace"; @@ -15,7 +15,7 @@ const QueryEditor = React.forwardRef(function( ref ) { const [container, setContainer] = useState(null); - const editorRef = useRef(null); + const [editorRef, setEditorRef] = useState(null); // For some reason, value for AceEditor should be managed in this way - otherwise it goes berserk when selecting text const [currentValue, setCurrentValue] = useState(value); @@ -44,17 +44,19 @@ const QueryEditor = React.forwardRef(function( ); useEffect(() => { - if (editorRef.current) { - const { editor } = editorRef.current; - updateSchemaCompleter(editor.id, schema); // TODO: cleanup? + if (editorRef) { + const editorId = editorRef.editor.id; + updateSchemaCompleter(editorId, schema); + return () => { + updateSchemaCompleter(editorId, null); + }; } - }, [schema]); + }, [schema, editorRef]); useEffect(() => { function resize() { - if (editorRef.current) { - const { editor } = editorRef.current; - editor.resize(); + if (editorRef) { + editorRef.editor.resize(); } } @@ -63,16 +65,15 @@ const QueryEditor = React.forwardRef(function( const unwatch = resizeObserver(container, resize); return unwatch; } - }, [container]); + }, [container, editorRef]); const handleSelectionChange = useCallback( selection => { - const { editor } = editorRef.current; - const rawSelectedQueryText = editor.session.doc.getTextRange(selection.getRange()); + const rawSelectedQueryText = editorRef.editor.session.doc.getTextRange(selection.getRange()); const selectedQueryText = rawSelectedQueryText.length > 1 ? rawSelectedQueryText : null; onSelectionChange(selectedQueryText); }, - [onSelectionChange] + [editorRef, onSelectionChange] ); const initEditor = useCallback(editor => { @@ -113,8 +114,8 @@ const QueryEditor = React.forwardRef(function( ref, () => ({ paste: text => { - if (editorRef.current) { - const { editor } = editorRef.current; + if (editorRef) { + const { editor } = editorRef; editor.session.doc.replace(editor.selection.getRange(), text); const range = editor.selection.getRange(); onChange(editor.session.getValue()); @@ -122,19 +123,18 @@ const QueryEditor = React.forwardRef(function( } }, focus: () => { - if (editorRef.current) { - const { editor } = editorRef.current; - editor.focus(); + if (editorRef) { + editorRef.editor.focus(); } }, }), - [onChange] + [editorRef, onChange] ); return (
{ - if (data.schema) { - return data.schema; - } else if (data.error.code === SCHEMA_NOT_SUPPORTED) { - return []; - } - return Promise.reject(new Error("Schema refresh failed.")); - }) - .catch(() => { - notification.error("Schema refresh failed.", "Please try again later."); - return Promise.resolve([]); - }); -} - function chooseDataSourceId(dataSourceIds, availableDataSources) { dataSourceIds = map(dataSourceIds, v => parseInt(v, 10)); availableDataSources = map(availableDataSources, ds => ds.id); @@ -74,16 +54,10 @@ function chooseDataSourceId(dataSourceIds, availableDataSources) { function QuerySource(props) { const [query, setQuery] = useState(props.query); const [originalQuerySource, setOriginalQuerySource] = useState(props.query.query); - const [allDataSources, setAllDataSources] = useState([]); - const [dataSourcesLoaded, setDataSourcesLoaded] = useState(false); - const dataSources = useMemo(() => filter(allDataSources, ds => !ds.view_only || ds.id === query.data_source_id), [ - allDataSources, - query.data_source_id, - ]); - const dataSource = useMemo(() => find(dataSources, { id: query.data_source_id }) || null, [query, dataSources]); - const [schema, setSchema] = useState([]); - const refreshSchemaTokenRef = useRef(null); - const [selectedTab, setSelectedTab] = useVisualizationTabHandler(query.visualizations); + const { dataSourcesLoaded, dataSources, dataSource } = useQueryDataSources(query); + const [schema, refreshSchema] = useDataSourceSchema(dataSource); + const queryFlags = useQueryFlags(query, dataSource); + const [selectedVisualization, setSelectedVisualization] = useVisualizationTabHandler(query.visualizations); const parameters = useMemo(() => query.getParametersDefs(), [query]); const [dirtyParameters, setDirtyParameters] = useState(query.getParameters().hasPendingValues()); @@ -98,10 +72,7 @@ function QuerySource(props) { } = useQueryExecute(query); const editorRef = useRef(null); - const autocompleteAvailable = useMemo(() => { - const tokensCount = reduce(schema, (totalLength, table) => totalLength + table.columns.length, 0); - return tokensCount <= 5000; - }, [schema]); + const autocompleteAvailable = useMemo(() => schema.tokensCount <= 5000, [schema]); const [autocompleteEnabled, setAutocompleteEnabled] = useState(localOptions.get("liveAutocomplete", true)); const [selectedText, setSelectedText] = useState(null); @@ -121,37 +92,6 @@ function QuerySource(props) { } }, [dirtyParameters, parameters, query]); - useEffect(() => { - let cancelDataSourceLoading = false; - DataSource.query().$promise.then(data => { - if (!cancelDataSourceLoading) { - setDataSourcesLoaded(true); - setAllDataSources(data); - } - }); - - return () => { - // cancel pending operations - cancelDataSourceLoading = true; - refreshSchemaTokenRef.current = null; - }; - }, []); - - const reloadSchema = useCallback( - (refresh = undefined) => { - const refreshToken = Math.random() - .toString(36) - .substr(2); - refreshSchemaTokenRef.current = refreshToken; - getSchema(dataSource, refresh).then(data => { - if (refreshSchemaTokenRef.current === refreshToken) { - setSchema(data); - } - }); - }, - [dataSource] - ); - const [handleQueryEditorChange] = useDebouncedCallback(queryText => { setQuery(extend(query.clone(), { query: queryText })); }, 200); @@ -164,10 +104,6 @@ function QuerySource(props) { .catch(error => notification.error(error)); }, [dataSource, query]); - useEffect(() => { - reloadSchema(); - }, [reloadSchema]); - useEffect(() => { recordEvent("view_source", "query", query.id); }, [query.id]); @@ -218,7 +154,7 @@ function QuerySource(props) { useEffect(() => { // choose data source id for new queries - if (dataSourcesLoaded && query.isNew()) { + if (dataSourcesLoaded && queryFlags.isNew) { const firstDataSourceId = dataSources.length > 0 ? dataSources[0].id : null; handleDataSourceChange( chooseDataSourceId( @@ -227,7 +163,7 @@ function QuerySource(props) { ) ); } - }, [query, dataSourcesLoaded, dataSources, handleDataSourceChange]); + }, [query, queryFlags, dataSourcesLoaded, dataSources, handleDataSourceChange]); const openAddToDashboardDialog = useCallback( visualizationId => { @@ -246,8 +182,7 @@ function QuerySource(props) { ); const editSchedule = useCallback(() => { - const canScheduleQuery = true; // TODO: Use real value - if (!query.can_edit || !canScheduleQuery) { + if (!queryFlags.canEdit || !queryFlags.canSchedule) { return; } @@ -261,7 +196,7 @@ function QuerySource(props) { }).result.then(schedule => { updateQuerySchedule(query, schedule).then(setQuery); }); - }, [query]); + }, [query, queryFlags]); const doUpdateQueryDescription = useCallback( description => { @@ -296,14 +231,11 @@ function QuerySource(props) { } }, []); - const canExecuteQuery = useMemo( - () => - !isEmpty(query.query) && - !isQueryExecuting && - !dirtyParameters && - (query.is_safe || (currentUser.hasPermission("execute_query") && dataSource && !dataSource.view_only)), - [isQueryExecuting, dirtyParameters, query, dataSource] - ); + const canExecuteQuery = useMemo(() => queryFlags.canExecute && !isQueryExecuting && !dirtyParameters, [ + isQueryExecuting, + dirtyParameters, + queryFlags, + ]); const isDirty = query.query !== originalQuerySource; const doExecuteQuery = useCallback(() => { @@ -320,7 +252,7 @@ function QuerySource(props) { return (
- +
- reloadSchema(true)} onItemSelect={handleSchemaItemSelect} /> + refreshSchema(true)} + onItemSelect={handleSchemaItemSelect} + />
{!query.isNew() && (
Save @@ -418,7 +354,7 @@ function QuerySource(props) { dataSourceSelectorProps={ dataSource ? { - disabled: !query.can_edit, + disabled: !queryFlags.canEdit, value: dataSource.id, onChange: handleDataSourceChange, options: map(dataSources, ds => ({ value: ds.id, label: ds.name })), @@ -429,7 +365,7 @@ function QuerySource(props) {
- {!query.isNew() && } + {!queryFlags.isNew && }
setDirtyParameters(query.getParameters().hasPendingValues())} onValuesChange={() => { @@ -486,14 +422,14 @@ function QuerySource(props) { addQueryVisualization(query, queryResult).then(({ query, visualization }) => { setQuery(query); - setSelectedTab(visualization.id); + setSelectedVisualization(visualization.id); }) } onDeleteVisualization={visualization => @@ -510,7 +446,7 @@ function QuerySource(props) { {queryResultData.status === "done" && (
- {!query.isNew() && query.can_edit && ( + {!queryFlags.isNew && queryFlags.canEdit && ( editQueryVisualization( @@ -519,7 +455,7 @@ function QuerySource(props) { find(query.visualizations, { id: visId }) ).then(({ query }) => setQuery(query)) } - selectedTab={selectedTab} + selectedTab={selectedVisualization} /> )} diff --git a/client/app/pages/queries/components/QueryPageHeader.jsx b/client/app/pages/queries/components/QueryPageHeader.jsx index cf43f69ddf..f5d8c8abf7 100644 --- a/client/app/pages/queries/components/QueryPageHeader.jsx +++ b/client/app/pages/queries/components/QueryPageHeader.jsx @@ -12,6 +12,8 @@ import { QueryTagsControl } from "@/components/tags-control/TagsControl"; import PermissionsEditorDialog from "@/components/PermissionsEditorDialog"; import ApiKeyDialog from "@/components/queries/ApiKeyDialog"; import getTags from "@/services/getTags"; +import { clientConfig } from "@/services/auth"; +import useQueryFlags from "../utils/useQueryFlags"; import { updateQuery, publishQuery, unpublishQuery, renameQuery, archiveQuery, duplicateQuery } from "../utils"; @@ -53,7 +55,9 @@ function createMenu(menu) { ); } -export default function QueryPageHeader({ query, sourceMode, onChange }) { +export default function QueryPageHeader({ query, sourceMode, selectedVisualization, onChange }) { + const queryFlags = useQueryFlags(query); + const saveName = useCallback( name => { renameQuery(query, name).then(onChange); @@ -68,17 +72,14 @@ export default function QueryPageHeader({ query, sourceMode, onChange }) { [query, onChange] ); - const selectedTab = null; // TODO: replace with actual value - const canViewSource = true; // TODO: replace with actual value - const canForkQuery = () => true; // TODO: replace with actual value - const showPermissionsControl = true; // TODO: replace with actual value + const showPermissionsControl = clientConfig.showPermissionsControl; const moreActionsMenu = useMemo( () => createMenu([ { fork: { - isEnabled: !query.isNew() && canForkQuery(), + isEnabled: !queryFlags.isNew && queryFlags.canFork, title: ( Fork @@ -92,14 +93,14 @@ export default function QueryPageHeader({ query, sourceMode, onChange }) { }, { archive: { - isAvailable: !query.isNew() && query.can_edit && !query.is_archived, + isAvailable: !queryFlags.isNew && queryFlags.canEdit && !queryFlags.isArchived, title: "Archive", onClick: () => { archiveQuery(query).then(onChange); }, }, managePermissions: { - isAvailable: !query.isNew() && query.can_edit && !query.is_archived && showPermissionsControl, + isAvailable: !queryFlags.isNew && queryFlags.canEdit && !queryFlags.isArchived && showPermissionsControl, title: "Manage Permissions", onClick: () => { const aclUrl = `api/queries/${query.id}/acl`; @@ -111,7 +112,7 @@ export default function QueryPageHeader({ query, sourceMode, onChange }) { }, }, unpublish: { - isAvailable: !query.isNew() && query.can_edit && !query.is_draft, + isAvailable: !queryFlags.isNew && queryFlags.canEdit && !queryFlags.isDraft, title: "Unpublish", onClick: () => { unpublishQuery(query).then(onChange); @@ -120,7 +121,7 @@ export default function QueryPageHeader({ query, sourceMode, onChange }) { }, { showAPIKey: { - isAvailable: !query.isNew(), + isAvailable: !queryFlags.isNew, title: "Show API Key", onClick: () => { ApiKeyDialog.showModal({ query }).result.then(onChange); @@ -128,33 +129,33 @@ export default function QueryPageHeader({ query, sourceMode, onChange }) { }, }, ]), - [showPermissionsControl, query, onChange] + [showPermissionsControl, query, queryFlags, onChange] ); return (
- {!query.isNew() && ( + {!queryFlags.isNew && ( )}

- +

- {query.is_draft && !query.is_archived && !query.isNew() && query.can_edit && ( + {queryFlags.isDraft && !queryFlags.isArchived && !queryFlags.isNew && queryFlags.canEdit && ( )} - {!query.isNew() && canViewSource && ( + {!queryFlags.isNew && queryFlags.canViewSource && ( {!sourceMode && ( -