Skip to content

Commit b3cec5a

Browse files
authored
big conf window, nicer diff (#308)
* big conf window, nicer diff * style changes * sql edit checkbox
1 parent 274b221 commit b3cec5a

File tree

10 files changed

+351
-103
lines changed

10 files changed

+351
-103
lines changed

apps/src/metabase/appController.ts

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,6 @@ export class MetabaseController extends AppController<MetabaseAppState> {
117117
const allSnippetsDict = await getSnippets() as MetabaseStateSnippetsDict;
118118
const allTemplateTags = getAllTemplateTagsInQuery(sql, allSnippetsDict)
119119
const state = (await this.app.getState()) as MetabaseAppStateSQLEditor;
120-
const userApproved = await RPCs.getUserConfirmation({content: sql, contentTitle: "Update SQL query?", oldContent: state.sqlQuery});
121-
if (!userApproved) {
122-
throw new Error("Action (and subsequent plan) cancelled!");
123-
}
124120
if (state.sqlEditorState == "closed") {
125121
await this.toggleSQLEditor("open");
126122
}
@@ -169,15 +165,11 @@ export class MetabaseController extends AppController<MetabaseAppState> {
169165
const actionContent: BlankMessageContent = {
170166
type: "BLANK",
171167
};
172-
// sql = processSQLWithCtesOrModels(sql, ctes);
173-
// const metabaseState = this.app as App<MetabaseAppState>;
174-
// const allModels = metabaseState.useStore().getState().toolContext?.dbInfo?.models || [];
175-
// use allModels for this replacement
176-
// sql = replaceLLMFriendlyIdentifiersInSqlWithModels(sql, allModels)
177168
const state = (await this.app.getState()) as MetabaseAppStateSQLEditor;
178-
const userApproved = await RPCs.getUserConfirmation({content: sql, contentTitle: "Update SQL query with parameters?", oldContent: state.sqlQuery});
169+
const { userApproved, userFeedback } = await RPCs.getUserConfirmation({content: sql, contentTitle: "Approve SQL Query?", oldContent: state.currentCard?.dataset_query?.native?.query || state.sqlQuery || ''});
179170
if (!userApproved) {
180-
throw new Error("Action (and subsequent plan) cancelled!");
171+
actionContent.content = '<UserCancelled>Reason: ' + (userFeedback === '' ? 'No particular reason given' : userFeedback) + '</UserCancelled>';
172+
return actionContent;
181173
}
182174
if (state.sqlEditorState == "closed") {
183175
await this.toggleSQLEditor("open");
@@ -363,7 +355,7 @@ export class MetabaseController extends AppController<MetabaseAppState> {
363355
const actionContent: BlankMessageContent = {
364356
type: "BLANK",
365357
};
366-
const userApproved = await RPCs.getUserConfirmation({content: memory, contentTitle: "Shall I add this to memory?", oldContent: undefined, override: true});
358+
const { userApproved, userFeedback } = await RPCs.getUserConfirmation({content: memory, contentTitle: "Shall I add this to memory?", oldContent: undefined, override: true});
367359
if (userApproved) {
368360
dispatch(addMemory(memory));
369361
dispatch(updateIsDevToolsOpen(true))
@@ -387,10 +379,6 @@ export class MetabaseController extends AppController<MetabaseAppState> {
387379
}
388380
})
389381
async executeSQLQuery() {
390-
const userApproved = await RPCs.getUserConfirmation({content: "Execute query", contentTitle: "Accept below action?", oldContent: undefined});
391-
if (!userApproved) {
392-
throw new Error("Action (and subsequent plan) cancelled!");
393-
}
394382
return await this._executeSQLQueryInternal();
395383
}
396384

@@ -451,22 +439,11 @@ export class MetabaseController extends AppController<MetabaseAppState> {
451439
const metabaseState = this.app as App<MetabaseAppState>;
452440
const pageType = metabaseState.useStore().getState().toolContext?.pageType;
453441

454-
// Check if template_tags or parameters are provided and non-empty
455-
const hasTemplateTagsOrParams = (template_tags && Object.keys(template_tags).length > 0) || (parameters && Array.isArray(parameters) && parameters.length > 0);
456-
457442
if (pageType === 'sql') {
458-
// if (hasTemplateTagsOrParams) {
459-
return await this.updateSQLQueryWithParams({ sql, template_tags, parameters, parameterValues, executeImmediately: true, _type: "markdown", ctes: _ctes });
460-
// } else {
461-
// return await this.updateSQLQuery({ sql, executeImmediately: true, _type: "csv", ctes: _ctes });
462-
// }
443+
return await this.updateSQLQueryWithParams({ sql, template_tags, parameters, parameterValues, executeImmediately: true, _type: "markdown", ctes: _ctes });
463444
}
464445
else if ((pageType === 'dashboard') || (pageType === 'unknown')) {
465-
// if (hasTemplateTagsOrParams) {
466-
return await this.runSQLQueryWithParams({ sql, template_tags, parameters, ctes: _ctes });
467-
// } else {
468-
// return await this.runSQLQuery({ sql, ctes: _ctes });
469-
// }
446+
return await this.runSQLQueryWithParams({ sql, template_tags, parameters, ctes: _ctes });
470447
}
471448
}
472449

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
},
1717
"packageManager": "yarn@4.3.1",
1818
"dependencies": {
19+
"react-diff-viewer-continued": "^3.4.0",
1920
"react-syntax-highlighter": "^15.5.0",
2021
"react-window": "^1.8.11"
2122
}

web/src/app/userConfirmation.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,20 @@ import { abortPlan } from '../state/chat/reducer'
77
export async function getUserConfirmation({content, contentTitle, oldContent, override = false}: {content: string, contentTitle: string, oldContent: string | undefined, override?: boolean}) {
88
const state = getState()
99
const isEnabled = state.settings.confirmChanges || override
10-
if (!isEnabled) return true
10+
if (!isEnabled) return { userApproved: true, userFeedback: '' }
1111
const thread = state.chat.activeThread
1212
dispatch(toggleUserConfirmation({show: true, content: content, contentTitle: contentTitle, oldContent: oldContent}))
1313

1414
while (true){
1515
const state = getState()
1616
const userConfirmation = state.chat.threads[thread].userConfirmation
17+
console.log('Polling user confirmation:', userConfirmation.show, userConfirmation.content, content)
1718
if (userConfirmation.show && userConfirmation.content === content && userConfirmation.userInput != 'NULL'){
1819
const userApproved = userConfirmation.userInput == 'APPROVE'
19-
console.log('User approved:', userApproved)
20+
const userFeedback = userConfirmation.userFeedback || ''
21+
console.log('User approved:', userApproved, 'feedback:', userConfirmation.userFeedback || 'No feedback')
2022
dispatch(toggleUserConfirmation({show: false, content: '', contentTitle: '', oldContent: ''}))
21-
return userApproved
23+
return { userApproved, userFeedback }
2224
}
2325
await sleep(100)
2426
}

web/src/components/common/ChatInputArea.tsx

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,11 @@ import AbortTaskButton from './AbortTaskButton'
1212
import AutosizeTextarea from './AutosizeTextarea'
1313
import { abortPlan } from '../../state/chat/reducer'
1414
import { setInstructions as setTaskInstructions } from '../../state/thumbnails/reducer'
15-
import { setDemoMode } from '../../state/settings/reducer'
15+
import { setConfirmChanges } from '../../state/settings/reducer'
1616
import { configs } from '../../constants'
1717
import _ from 'lodash'
18+
import { MetabaseContext } from 'apps/types'
19+
import { getApp } from '../../helpers/app'
1820

1921
interface ChatInputAreaProps {
2022
isRecording: boolean
@@ -27,17 +29,22 @@ interface ChatInputAreaProps {
2729
}
2830
}
2931

32+
const app = getApp()
33+
const useAppStore = app.useStore()
34+
3035
const ChatInputArea = forwardRef<HTMLTextAreaElement, ChatInputAreaProps>(
3136
({ isRecording, runTask, appEnabledStatus }, ref) => {
3237
const dispatch = useDispatch()
3338
const reduxInstructions = useSelector((state: RootState) => state.thumbnails.instructions)
3439
const activeThread = useSelector((state: RootState) => state.chat.threads[state.chat.activeThread])
35-
const demoMode = useSelector((state: RootState) => state.settings.demoMode)
40+
const confirmChanges = useSelector((state: RootState) => state.settings.confirmChanges)
3641
const taskInProgress = !(activeThread.status == 'FINISHED')
3742
const [instructions, setInstructions] = useState<string>(reduxInstructions)
3843

39-
const updateDemoMode = (value: boolean) => {
40-
dispatch(setDemoMode(value))
44+
// Tool context access for SQL page
45+
const toolContext: MetabaseContext = useAppStore((state) => state.toolContext)
46+
const updateConfirmChanges = (value: boolean) => {
47+
dispatch(setConfirmChanges(value))
4148
}
4249

4350
// Sync with redux instructions when they change
@@ -80,7 +87,7 @@ const ChatInputArea = forwardRef<HTMLTextAreaElement, ChatInputAreaProps>(
8087
<HStack aria-label="chat-controls" position={"absolute"} bottom={0} width={"100%"} p={2}>
8188
<HStack justify={"space-between"} width={"100%"}>
8289
<HStack gap={0}>
83-
{configs.IS_DEV && false && (
90+
{(toolContext.pageType === 'sql') && (
8491
<Checkbox
8592
sx={{
8693
'& input:not(:checked) + span': {
@@ -98,10 +105,10 @@ const ChatInputArea = forwardRef<HTMLTextAreaElement, ChatInputAreaProps>(
98105
marginLeft: 1,
99106
}
100107
}}
101-
isChecked={demoMode}
102-
onChange={(e) => updateDemoMode(e.target.checked)}
108+
isChecked={confirmChanges}
109+
onChange={(e) => updateConfirmChanges(e.target.checked)}
103110
>
104-
<Text fontSize="xs">Advanced Mode</Text>
111+
<Text fontSize="xs">Review SQL Edits</Text>
105112
</Checkbox>
106113
)}
107114
</HStack>

web/src/components/common/Markdown.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,11 +131,12 @@ function getTextFromChildren(children: any): string {
131131
function ModifiedBlockquote(props: any) {
132132
const textContent = getTextFromChildren(props.children);
133133
const isError = textContent.toLowerCase().includes('error');
134-
134+
const isWarning = textContent.toLowerCase().includes('user cancellation');
135+
135136
return (
136137
<blockquote style={{
137138
borderLeft: '4px solid',
138-
borderLeftColor: isError ? '#e53e3e' : '#14a085',
139+
borderLeftColor: isError ? '#e53e3e' : isWarning ? '#d69e2e' : '#14a085',
139140
borderRadius: '2px 0 0 2px',
140141
paddingLeft: '16px',
141142
margin: '0px',

0 commit comments

Comments
 (0)