Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: notebook updates #1485

Merged
merged 7 commits into from
Nov 6, 2024
Merged

fix: notebook updates #1485

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
44947e2
fix: show compiled query in notebook query results
saravmajestic Oct 28, 2024
46e51ea
fix: handle markdown execution
saravmajestic Oct 28, 2024
ee2c227
Merge branch 'master' into fix/notebook-bugs-v2
saravmajestic Oct 29, 2024
2e9a616
Merge branch 'master' into fix/notebook-bugs-v2
saravmajestic Nov 4, 2024
298b940
fix: ai-2180 remove teammates flag in extension
saravmajestic Nov 4, 2024
8ba65b6
fix: AI-2188 use citations component from lib
saravmajestic Nov 4, 2024
10912ec
fix: AI-2188 use citations component from lib
saravmajestic Nov 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2497,6 +2497,8 @@ project_name="${e.getProjectName()}"
a.clearOutput(),
e.document.languageId)
) {
case "markdown":
break;
case "python":
this.telemetry.startTelemetryEvent(
u.TelemetryEvents["Notebook/Execute"],
Expand Down
8 changes: 4 additions & 4 deletions webview_panels/src/modules/app/appSlice.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const initialState = {
frontendUrl: null,
currency: "USD",
// This is tenant level global setting
teammatesEnabled: true
teammatesEnabled: false,
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be synced on extension load from saas

},
} as AppStateProps;

Expand All @@ -22,7 +22,7 @@ const appSlice = createSlice({
reducers: {
setTenantInfo: (
state,
action: PayloadAction<AppStateProps["tenantInfo"]>
action: PayloadAction<AppStateProps["tenantInfo"]>,
) => {
state.tenantInfo = action.payload;
},
Expand All @@ -34,7 +34,7 @@ const appSlice = createSlice({
},
updateIsComponentsApiInitialized: (
state,
action: PayloadAction<boolean>
action: PayloadAction<boolean>,
) => {
state.isComponentsApiInitialized = action.payload;
},
Expand All @@ -52,7 +52,7 @@ const appSlice = createSlice({
},
setCurrentUser: (
state,
action: PayloadAction<AppStateProps["currentUser"]>
action: PayloadAction<AppStateProps["currentUser"]>,
) => {
state.currentUser = action.payload;
},
Expand Down
1 change: 1 addition & 0 deletions webview_panels/src/modules/queryPanel/context/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export interface QueryPanelStateProps {
columnNames: string[];
columnTypes: string[];
raw_sql: string;
compiled_sql: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Property usage needs to be updated in components

The verification reveals that while compiled_sql is being handled in some parts of the codebase (like useQueryPanelListeners.ts and BookmarkButton.tsx), it's not being properly utilized in the main query results display component (QueryPanelContent.tsx). The component currently only uses data, columnNames, and columnTypes from queryResults, but doesn't handle the new compiled_sql property.

Key locations requiring updates:

  • webview_panels/src/modules/queryPanel/components/QueryPanelContents/QueryPanelContent.tsx: Should handle the new compiled_sql property from queryResults
  • webview_panels/src/modules/queryPanel/useQueryPanelState.ts: Consider exposing compiled_sql alongside other queryResults data
🔗 Analysis chain

LGTM! Verify usage in components.

The addition of compiled_sql to queryResults maintains consistency with the existing QueryBookmark interface and aligns with the PR's objective to support compiled SQL outputs.

Let's verify the property usage in components:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if components are properly handling the new compiled_sql property
# Expected: Components should destructure or access this property from queryResults

# Search for components accessing queryResults
rg -t tsx -t ts "queryResults[.?]" webview_panels/src/modules/queryPanel

# Search for specific compiled_sql usage
rg -t tsx -t ts "compiled_sql" webview_panels/src/modules/queryPanel

Length of output: 199

Script:

#!/bin/bash
# Let's try with correct ripgrep file type options and more specific paths
# Search for queryResults usage
rg --type-add 'ts:*.{ts,tsx}' -t ts "queryResults[.?]" webview_panels/src/modules/queryPanel/

# Search for compiled_sql usage
rg --type-add 'ts:*.{ts,tsx}' -t ts "compiled_sql" webview_panels/src/modules/queryPanel/

# Also search for any QueryResults type usage to understand the context better
rg --type-add 'ts:*.{ts,tsx}' -t ts "QueryResults" webview_panels/src/modules/queryPanel/

Length of output: 3565

};
queryExecutionInfo?: { elapsedTime: number };
queryResultsError?: {
Expand Down
62 changes: 54 additions & 8 deletions webview_panels/src/notebook/renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,66 @@
import PerspectiveViewer from "@modules/queryPanel/components/perspective/PerspectiveViewer";
import type { QueryPanelStateProps } from "@modules/queryPanel/context/types";
import classes from "./renderer.module.scss";
import PreTag from "@modules/markdown/PreTag";
import { CodeBlock, Nav, NavItem, NavLink, Stack } from "@uicore";
import PerspectiveErrorBoundary from "@modules/queryPanel/components/perspective/PerspectiveErrorBoundary";
import { QueryPanelTitleTabState } from "@modules/queryPanel/components/QueryPanelContents/types";
import { useState } from "react";

interface Props {
items: NonNullable<QueryPanelStateProps["queryResults"]>;
}
const AllItems = ({ items }: Props): JSX.Element => {
const [tabState, setTabState] = useState(QueryPanelTitleTabState.Preview);
const compiledCodeMarkup = items.compiled_sql;
return (
<div className={classes.perspectiveWrapper}>
<PerspectiveViewer
columnNames={items.columnNames}
columnTypes={items.columnTypes}
data={items.data}
styles={{ minHeight: 100, height: 400 }}
/>
</div>
<Stack direction="column" className="mt-2">
<Nav tabs>
<NavItem>
<NavLink
active={QueryPanelTitleTabState.Preview === tabState}
onClick={() => setTabState(QueryPanelTitleTabState.Preview)}
className="cursor-pointer"
>
Query results{" "}
</NavLink>
</NavItem>
{compiledCodeMarkup ? (
<NavItem>
<NavLink
active={QueryPanelTitleTabState.Sql === tabState}
onClick={() => setTabState(QueryPanelTitleTabState.Sql)}
className="cursor-pointer"
>
SQL
</NavLink>
</NavItem>
) : null}
</Nav>
{tabState === QueryPanelTitleTabState.Preview ? (
<div className={classes.perspectiveWrapper}>
<PerspectiveErrorBoundary>
<PerspectiveViewer
columnNames={items.columnNames}
columnTypes={items.columnTypes}
data={items.data}
styles={{ minHeight: 100, height: 400 }}
/>
</PerspectiveErrorBoundary>
</div>
) : null}
{tabState === QueryPanelTitleTabState.Sql && compiledCodeMarkup ? (
<div style={{ width: "fit-content" }}>
<PreTag text={compiledCodeMarkup}>
<CodeBlock
code={compiledCodeMarkup}
language="sql"
showLineNumbers
/>
</PreTag>
</div>
) : null}
</Stack>
);
};

Expand Down