Feature/review mode (#310)

* Show diffs only in Review Mode

* Rename to Review Mode and add reject message

* Show Undo Redo only when task not in progress

* Fix Undo/Redo in Review Mode

* Do not ask for confirmation if no change in sql

Author: ppsreejith

apps/src/metabase/appController.ts

@@ -161,12 +161,15 @@ export class MetabaseController extends AppController<MetabaseAppState> {
       return {text: `Using ${Object.keys(template_tags || {}).length} template tags and ${(parameters || []).length} parameters`, code: sql, oldCode: sqlQuery}
     }
   })
-  async updateSQLQueryWithParams({ sql, template_tags = {}, parameters = [], parameterValues = [], executeImmediately = true, _type = "markdown", ctes = [] }: { sql: string, template_tags?: object, parameters?: any[], parameterValues?: Array<{id: string, value: string[]}>, executeImmediately?: boolean, _type?: string, ctes: CTE[] }) {
+  async updateSQLQueryWithParams({ sql, template_tags = {}, parameters = [], parameterValues = [], executeImmediately = true, _type = "markdown", ctes = [], skipConfirmation=false }: { sql: string, template_tags?: object, parameters?: any[], parameterValues?: Array<{id: string, value: string[]}>, executeImmediately?: boolean, _type?: string, ctes: CTE[], skipConfirmation?: boolean }) {
     const actionContent: BlankMessageContent = {
       type: "BLANK",
     };
     const state = (await this.app.getState()) as MetabaseAppStateSQLEditor;
-    const { userApproved, userFeedback } = await RPCs.getUserConfirmation({content: sql, contentTitle: "Approve SQL Query?", oldContent: state.currentCard?.dataset_query?.native?.query || state.sqlQuery || ''});
+    const oldContent = state.currentCard?.dataset_query?.native?.query || state.sqlQuery || ''
+    const isContentUnchanged = sql == oldContent
+    const override = skipConfirmation || isContentUnchanged? false : undefined
+    const { userApproved, userFeedback } = await RPCs.getUserConfirmation({content: sql, contentTitle: "Approve SQL Query?", oldContent, override});
     if (!userApproved) {
       actionContent.content = '<UserCancelled>Reason: ' + (userFeedback === '' ?  'No particular reason given' : userFeedback) + '</UserCancelled>';
       return actionContent;
@@ -411,7 +414,7 @@ export class MetabaseController extends AppController<MetabaseAppState> {
       return {text: `${explanation}${paramValuesInfo}`, code: sql, oldCode: currentQuery, language: "sql", extraArgs: {old: {template_tags: currentTemplateTags, parameters: currentParameters}, new: {template_tags, parameters, parameterValues}}}
     }
   })
-  async ExecuteQuery({ sql, _ctes = [], explanation = "", template_tags={}, parameters=[], parameterValues=[] }: { sql: string, _ctes?: CTE[], explanation?: string, template_tags?: object, parameters?: any[], parameterValues?: Array<{id: string, value: string[]}> }) {
+  async ExecuteQuery({ sql, _ctes = [], explanation = "", template_tags={}, parameters=[], parameterValues=[], skipConfirmation=false }: { sql: string, _ctes?: CTE[], explanation?: string, template_tags?: object, parameters?: any[], parameterValues?: Array<{id: string, value: string[]}>, skipConfirmation?: boolean }) {
     // console.log('Template tags are', template_tags)
     // console.log('Parameters are', parameters)
     // Try parsing template_tags and parameters if they are strings
@@ -440,7 +443,7 @@ export class MetabaseController extends AppController<MetabaseAppState> {
     const pageType = metabaseState.useStore().getState().toolContext?.pageType;
 
     if (pageType === 'sql') {
-        return await this.updateSQLQueryWithParams({ sql, template_tags, parameters, parameterValues, executeImmediately: true, _type: "markdown", ctes: _ctes });
+        return await this.updateSQLQueryWithParams({ sql, template_tags, parameters, parameterValues, executeImmediately: true, _type: "markdown", ctes: _ctes, skipConfirmation });
     }
     else if ((pageType === 'dashboard') || (pageType === 'unknown')) {
         return await this.runSQLQueryWithParams({ sql, template_tags, parameters, ctes: _ctes });

web/src/app/userConfirmation.ts

@@ -3,22 +3,21 @@ import { getState } from "../state/store"
 import { sleep } from "../helpers/utils"
 import { toggleUserConfirmation } from "../state/chat/reducer"
 import { abortPlan } from '../state/chat/reducer'
+import { isUndefined } from "lodash"
 
-export async function getUserConfirmation({content, contentTitle, oldContent, override = false}: {content: string, contentTitle: string, oldContent: string | undefined, override?: boolean}) {
+export async function getUserConfirmation({content, contentTitle, oldContent, override}: {content: string, contentTitle: string, oldContent: string | undefined, override?: boolean}) {
   const state = getState()
-  const isEnabled = state.settings.confirmChanges || override
+  const isEnabled = isUndefined(override) ? state.settings.confirmChanges : override
   if (!isEnabled) return { userApproved: true, userFeedback: '' }
   const thread = state.chat.activeThread
   dispatch(toggleUserConfirmation({show: true, content: content, contentTitle: contentTitle, oldContent: oldContent}))
 
   while (true){
     const state = getState()
     const userConfirmation = state.chat.threads[thread].userConfirmation
-    console.log('Polling user confirmation:', userConfirmation.show, userConfirmation.content, content)
     if (userConfirmation.show && userConfirmation.content === content && userConfirmation.userInput != 'NULL'){
       const userApproved = userConfirmation.userInput == 'APPROVE'
       const userFeedback = userConfirmation.userFeedback || ''
-      console.log('User approved:', userApproved, 'feedback:', userConfirmation.userFeedback || 'No feedback')
       dispatch(toggleUserConfirmation({show: false, content: '', contentTitle: '', oldContent: ''}))
       return { userApproved, userFeedback }
     }

web/src/components/common/ActionStack.tsx

@@ -59,6 +59,8 @@ export const ActionStack: React.FC<{status: string, actions: Array<ActionStatusV
   const url = useAppStore((state) => state.toolContext.url) || '';
   const origin = url ? new URL(url).origin : '';
   const thread = useSelector((state: RootState) => state.chat.activeThread)
+  const activeThread = useSelector((state: RootState) => state.chat.threads[thread])
+  const taskInProgress = !(activeThread.status == 'FINISHED')
   const totalThreads = useSelector((state: RootState) => state.chat.threads.length)
   const embedConfigs = useSelector((state: RootState) => state.configs.embed);
 
@@ -105,7 +107,7 @@ export const ActionStack: React.FC<{status: string, actions: Array<ActionStatusV
             executeAction({
                 index: -1,
                 function: 'ExecuteQuery',
-                args: {sql: sql, template_tags: extraArgs?.template_tags || {}, parameters: extraArgs?.parameters || []},
+                args: {sql: sql, template_tags: extraArgs?.template_tags || {}, parameters: extraArgs?.parameters || [], skipConfirmation: true},
             });
         };
 
@@ -186,7 +188,7 @@ export const ActionStack: React.FC<{status: string, actions: Array<ActionStatusV
                 return (
                     <Box aria-label="thinking-content" borderBottomWidth={1} mb={1} borderBottomColor={'minusxGreen.800'} w={"100%"}>
                         <Markdown content={text}></Markdown>
-                        {pageType && pageType == 'sql' && undoRedoArr[i]}
+                        {pageType && pageType == 'sql' && !taskInProgress && undoRedoArr[i]}
                     </Box>
                 )
             })}

web/src/components/common/ChatInputArea.tsx

@@ -2,7 +2,8 @@ import {
   HStack,
   Stack,
   Checkbox,
-  Text
+  Text,
+  Badge
 } from '@chakra-ui/react'
 import React, { forwardRef, useState, useEffect, useCallback } from 'react'
 import { useDispatch, useSelector } from 'react-redux'
@@ -108,7 +109,10 @@ const ChatInputArea = forwardRef<HTMLTextAreaElement, ChatInputAreaProps>(
                       isChecked={confirmChanges}
                       onChange={(e) => updateConfirmChanges(e.target.checked)}
                     >
-                      <Text fontSize="xs">Review SQL Edits</Text>
+                      <HStack spacing={1} align="start">
+                        <Text fontSize="xs">Review Mode</Text>
+                        <Badge size="sm" colorScheme="blue" fontSize="xs">NEW</Badge>
+                      </HStack>
                     </Checkbox>
                   )}
                 </HStack>

web/src/components/common/UserConfirmation.tsx

@@ -130,7 +130,7 @@ export const UserConfirmation = () => {
               useDarkTheme={true}
               leftTitle={<Text fontSize="sm" color="minusxBW.100" textAlign={"center"}>SQL in the Editor</Text>}
               rightTitle={<Text fontSize="sm" color="minusxBW.100" textAlign={"center"}>SQL to be executed</Text>}
-              showDiffOnly={false}
+              showDiffOnly={true}
             />
           </div>
 
@@ -193,7 +193,12 @@ export const UserConfirmation = () => {
                 colorScheme="red"
                 minWidth={"120px"}
               >
-                {showRejectFeedback ? 'Submit' : 'Reject'}
+                <HStack spacing={1} align="end">
+                  <Text>{showRejectFeedback ? 'Submit' : 'Reject'}</Text>
+                  {!showRejectFeedback && (
+                    <Text fontSize="xs" opacity={0.6}>(with feedback)</Text>
+                  )}
+                </HStack>
               </Button>
 
               {showRejectFeedback && (