Alerting: Fix simplified query step (#97046)

* Fix simplified query step

* remove is advancedMode and use editorSettings.simplifiedQueryEditor

* remove unnecessary Boolean conversion

* fix when feature toggle is disabled

* fix test

* simplify code

* fix when not ff is not enabled

public/app/features/alerting/unified/components/rule-editor/QueryWrapper.tsx

@@ -14,6 +14,7 @@ import {
   RelativeTimeRange,
   ThresholdsConfig,
 } from '@grafana/data';
+import { config } from '@grafana/runtime';
 import { DataQuery } from '@grafana/schema';
 import { GraphThresholdsStyleMode, Icon, InlineField, Input, Stack, Tooltip, useStyles2 } from '@grafana/ui';
 import { logInfo } from 'app/features/alerting/unified/Analytics';
@@ -81,7 +82,8 @@ export const QueryWrapper = ({
   const defaults = dsInstance?.getDefaultQuery ? dsInstance.getDefaultQuery(CoreApp.UnifiedAlerting) : {};
 
   const { getValues } = useFormContext<RuleFormValues>();
-  const isAdvancedMode = getValues('editorSettings.simplifiedQueryEditor') !== true;
+  const isSwitchModeEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false;
+  const isAdvancedMode = isSwitchModeEnabled ? getValues('editorSettings.simplifiedQueryEditor') !== true : true;
 
   const queryWithDefaults = {
     ...defaults,

public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx

@@ -67,7 +67,11 @@ import { GrafanaFolderAndLabelsStep } from '../GrafanaFolderAndLabelsStep';
 import { NotificationsStep } from '../NotificationsStep';
 import { RecordingRulesNameSpaceAndGroupStep } from '../RecordingRulesNameSpaceAndGroupStep';
 import { RuleInspector } from '../RuleInspector';
-import { QueryAndExpressionsStep } from '../query-and-alert-condition/QueryAndExpressionsStep';
+import {
+  areQueriesTransformableToSimpleCondition,
+  isExpressionQueryInAlert,
+  QueryAndExpressionsStep,
+} from '../query-and-alert-condition/QueryAndExpressionsStep';
 import { translateRouteParamToRuleType } from '../util';
 
 type Props = {
@@ -386,15 +390,17 @@ function formValuesFromQueryParams(ruleDefinition: string, type: RuleFormType):
     };
   }
 
-  return setInstantOrRange(
+  return setQueryEditorSettings(
-    ignoreHiddenQueries({
+    setInstantOrRange(
-      ...getDefaultFormValues(),
+      ignoreHiddenQueries({
-      ...ruleFromQueryParams,
+        ...getDefaultFormValues(),
-      annotations: normalizeDefaultAnnotations(ruleFromQueryParams.annotations ?? []),
+        ...ruleFromQueryParams,
-      queries: ruleFromQueryParams.queries ?? getDefaultQueries(),
+        annotations: normalizeDefaultAnnotations(ruleFromQueryParams.annotations ?? []),
-      type: type || RuleFormType.grafana,
+        queries: ruleFromQueryParams.queries ?? getDefaultQueries(),
-      evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL,
+        type: type || RuleFormType.grafana,
-    })
+        evaluateEvery: DEFAULT_GROUP_EVALUATION_INTERVAL,
+      })
+    )
   );
 }
 
@@ -405,6 +411,35 @@ function formValuesFromPrefill(rule: Partial<RuleFormValues>): RuleFormValues {
   });
 }
 
+function setQueryEditorSettings(values: RuleFormValues): RuleFormValues {
+  const isQuerySwitchModeEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false;
+
+  if (!isQuerySwitchModeEnabled) {
+    return {
+      ...values,
+      editorSettings: {
+        simplifiedQueryEditor: false,
+        simplifiedNotificationEditor: true, // actually it doesn't matter in this case
+      },
+    };
+  }
+
+  // data queries only
+  const dataQueries = values.queries.filter((query) => !isExpressionQuery(query.model));
+
+  // expression queries only
+  const expressionQueries = values.queries.filter((query) => isExpressionQueryInAlert(query));
+
+  const queryParamsAreTransformable = areQueriesTransformableToSimpleCondition(dataQueries, expressionQueries);
+  return {
+    ...values,
+    editorSettings: {
+      simplifiedQueryEditor: queryParamsAreTransformable,
+      simplifiedNotificationEditor: true,
+    },
+  };
+}
+
 function setInstantOrRange(values: RuleFormValues): RuleFormValues {
   return {
     ...values,

public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/QueryAndExpressionsStep.tsx

@@ -32,7 +32,6 @@ import {
 import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto';
 
 import { useRulesSourcesWithRuler } from '../../../hooks/useRuleSourcesWithRuler';
-import { useURLSearchParams } from '../../../hooks/useURLSearchParams';
 import { RuleFormType, RuleFormValues } from '../../../types/rule-form';
 import { getDefaultOrFirstCompatibleDataSource } from '../../../utils/datasource';
 import { isPromOrLokiQuery, PromOrLokiQuery } from '../../../utils/rule-form';
@@ -134,9 +133,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
   } = useFormContext<RuleFormValues>();
 
   const { queryPreviewData, runQueries, cancelQueries, isPreviewLoading, clearPreviewData } = useAlertQueryRunner();
-  const [queryParams] = useURLSearchParams();
   const isSwitchModeEnabled = config.featureToggles.alertingQueryAndExpressionsStepMode ?? false;
-  const isNewFromQueryParams = queryParams.has('defaults') && !editingExistingRule;
 
   const initialState = {
     queries: getValues('queries'),
@@ -167,20 +164,23 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
   const isCloudAlertRuleType = isCloudAlertingRuleByType(type);
   const [showResetModeModal, setShowResetModal] = useState(false);
 
-  const { isAdvancedMode, simpleCondition, setSimpleCondition } = useAdvancedMode(
+  const simplifiedQueryInForm = editorSettings?.simplifiedQueryEditor;
-    editorSettings,
+
+  const { simpleCondition, setSimpleCondition } = useAdvancedMode(
+    simplifiedQueryInForm,
     isGrafanaAlertingType,
-    isNewFromQueryParams,
     dataQueries,
     expressionQueries
   );
 
+  const simplifiedQueryStep = isSwitchModeEnabled ? getValues('editorSettings.simplifiedQueryEditor') : false;
+
   // If we switch to simple mode we need to update the simple condition with the data in the queries reducer
   useEffect(() => {
-    if (!isAdvancedMode && isGrafanaAlertingType) {
+    if (simplifiedQueryStep && isGrafanaAlertingType) {
       setSimpleCondition(getSimpleConditionFromExpressions(expressionQueries));
     }
-  }, [isAdvancedMode, expressionQueries, isGrafanaAlertingType, setSimpleCondition]);
+  }, [simplifiedQueryStep, expressionQueries, isGrafanaAlertingType, setSimpleCondition]);
 
   const { rulesSourcesWithRuler } = useRulesSourcesWithRuler();
 
@@ -192,14 +192,14 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
         return;
       }
       // we need to be sure the condition is set once we switch to simple mode
-      if (!isAdvancedMode) {
+      if (simplifiedQueryStep) {
         setValue('condition', SimpleConditionIdentifier.thresholdId);
         runQueries(getValues('queries'), SimpleConditionIdentifier.thresholdId);
       } else {
         runQueries(getValues('queries'), condition || (getValues('condition') ?? ''));
       }
     },
-    [isCloudAlertRuleType, runQueries, getValues, isAdvancedMode, setValue]
+    [isCloudAlertRuleType, runQueries, getValues, simplifiedQueryStep, setValue]
   );
 
   // whenever we update the queries we have to update the form too
@@ -472,13 +472,12 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
   if (!type) {
     return null;
   }
-
   const switchMode =
     isGrafanaAlertingType && isSwitchModeEnabled
       ? {
-          isAdvancedMode,
+          isAdvancedMode: !simplifiedQueryStep,
           setAdvancedMode: (isAdvanced: boolean) => {
-            if (!isAdvanced) {
+            if (!getValues('editorSettings.simplifiedQueryEditor')) {
               if (!areQueriesTransformableToSimpleCondition(dataQueries, expressionQueries)) {
                 setShowResetModal(true);
                 return;
@@ -573,7 +572,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
               condition={condition}
               onSetCondition={handleSetCondition}
             />
-            {isAdvancedMode && (
+            {!simplifiedQueryStep && (
               <Tooltip content={'You appear to have no compatible data sources'} show={noCompatibleDataSources}>
                 <Button
                   type="button"
@@ -590,7 +589,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
               </Tooltip>
             )}
             {/* We only show Switch for Grafana managed alerts */}
-            {isGrafanaAlertingType && isAdvancedMode && (
+            {isGrafanaAlertingType && !simplifiedQueryStep && (
               <SmartAlertTypeDetector
                 editingExistingRule={editingExistingRule}
                 rulesSourcesWithRuler={rulesSourcesWithRuler}
@@ -599,7 +598,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
               />
             )}
             {/* Expression Queries */}
-            {isAdvancedMode && (
+            {!simplifiedQueryStep && (
               <>
                 <Stack direction="column" gap={0}>
                   <Text element="h5">Expressions</Text>
@@ -628,7 +627,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
             )}
             {/* action buttons */}
             <Stack direction="column">
-              {!isAdvancedMode && (
+              {simplifiedQueryStep && (
                 <SimpleConditionEditor
                   simpleCondition={simpleCondition}
                   onChange={setSimpleCondition}
@@ -638,7 +637,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
                 />
               )}
               <Stack direction="row">
-                {isAdvancedMode && config.expressionsEnabled && <TypeSelectorButton onClickType={onClickType} />}
+                {!simplifiedQueryStep && config.expressionsEnabled && <TypeSelectorButton onClickType={onClickType} />}
 
                 {isPreviewLoading && (
                   <Button icon="spinner" type="button" variant="destructive" onClick={cancelQueries}>
@@ -653,7 +652,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
                     onClick={() => runQueriesPreview()}
                     disabled={emptyQueries}
                   >
-                    {isAdvancedMode
+                    {!simplifiedQueryStep
                       ? t('alerting.queryAndExpressionsStep.preview', 'Preview')
                       : t('alerting.queryAndExpressionsStep.previewCondition', 'Preview alert rule condition')}
                   </Button>
@@ -688,7 +687,7 @@ export const QueryAndExpressionsStep = ({ editingExistingRule, onDataChange }: P
         confirmText="Deactivate"
         icon="exclamation-triangle"
         onConfirm={() => {
-          setValue('editorSettings.simplifiedNotificationEditor', true);
+          setValue('editorSettings.simplifiedQueryEditor', true);
           setShowResetModal(false);
           dispatch(resetToSimpleCondition());
         }}
@@ -768,7 +767,7 @@ const useSetExpressionAndDataSource = () => {
   };
 };
 
-function isExpressionQueryInAlert(
+export function isExpressionQueryInAlert(
   query: AlertQuery<AlertDataQuery | ExpressionQuery>
 ): query is AlertQuery<ExpressionQuery> {
   return isExpressionQuery(query.model);

public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/determineAdvancedMode.test.ts

@@ -1,76 +1,26 @@
-import { produce } from 'immer';
-
-import { dataQuery, reduceExpression, thresholdExpression } from '../../../mocks';
-
 import { determineAdvancedMode } from './useAdvancedMode';
 
-const dataQueries = [dataQuery];
-const expressionQueries = [reduceExpression, thresholdExpression];
-
 describe('determineAdvancedMode', () => {
   it('should return true if simplifiedQueryEditor is false', () => {
-    const editorSettings = { simplifiedQueryEditor: false, simplifiedNotificationEditor: true };
     const isGrafanaAlertingType = true;
-    const isNewFromQueryParams = false;
 
-    const result = determineAdvancedMode(
+    const result = determineAdvancedMode(false, isGrafanaAlertingType);
-      editorSettings,
-      isGrafanaAlertingType,
-      isNewFromQueryParams,
-      dataQueries,
-      expressionQueries
-    );
 
     expect(result).toBe(true);
   });
 
   it('should return true if isGrafanaAlertingType is false', () => {
-    const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true };
     const isGrafanaAlertingType = false;
-    const isNewFromQueryParams = false;
 
-    const result = determineAdvancedMode(
+    const result = determineAdvancedMode(true, isGrafanaAlertingType);
-      editorSettings,
-      isGrafanaAlertingType,
-      isNewFromQueryParams,
-      dataQueries,
-      expressionQueries
-    );
-
-    expect(result).toBe(true);
-  });
-
-  const editorSettings = { simplifiedQueryEditor: true, simplifiedNotificationEditor: true };
-  it('should return true if isNewFromQueryParams is true and queries are not transformable', () => {
-    const isGrafanaAlertingType = true;
-    const isNewFromQueryParams = true;
-
-    const newQuery = produce(dataQuery, (draft) => {
-      draft.refId = 'whatever';
-    });
-
-    const result = determineAdvancedMode(
-      editorSettings,
-      isGrafanaAlertingType,
-      isNewFromQueryParams,
-      [newQuery],
-      expressionQueries
-    );
 
     expect(result).toBe(true);
   });
 
   it('should return false if all conditions are false', () => {
     const isGrafanaAlertingType = true;
-    const isNewFromQueryParams = false;
 
-    const result = determineAdvancedMode(
+    const result = determineAdvancedMode(true, isGrafanaAlertingType);
-      editorSettings,
-      isGrafanaAlertingType,
-      isNewFromQueryParams,
-      dataQueries,
-      expressionQueries
-    );
 
     expect(result).toBe(false);
   });

public/app/features/alerting/unified/components/rule-editor/query-and-alert-condition/useAdvancedMode.ts

@@ -5,8 +5,6 @@ import { EvalFunction } from 'app/features/alerting/state/alertDef';
 import { ExpressionQuery } from 'app/features/expressions/types';
 import { AlertDataQuery, AlertQuery } from 'app/types/unified-alerting-dto';
 
-import { SimplifiedEditor } from '../../../types/rule-form';
-
 import { areQueriesTransformableToSimpleCondition } from './QueryAndExpressionsStep';
 import { getSimpleConditionFromExpressions, SimpleCondition } from './SimpleCondition';
 
@@ -27,19 +25,8 @@ function initializeSimpleCondition(
     };
   }
 }
-export function determineAdvancedMode(
+export function determineAdvancedMode(simplifiedQueryEditor: boolean | undefined, isGrafanaAlertingType: boolean) {
-  editorSettings: SimplifiedEditor | undefined,
+  return simplifiedQueryEditor === false || !isGrafanaAlertingType;
-  isGrafanaAlertingType: boolean,
-  isNewFromQueryParams: boolean,
-  dataQueries: Array<AlertQuery<ExpressionQuery | AlertDataQuery>>,
-  expressionQueries: Array<AlertQuery<ExpressionQuery>>
-) {
-  const queryParamsAreTransformable = areQueriesTransformableToSimpleCondition(dataQueries, expressionQueries);
-  return (
-    Boolean(editorSettings?.simplifiedQueryEditor) === false ||
-    !isGrafanaAlertingType ||
-    (isNewFromQueryParams && !queryParamsAreTransformable)
-  );
 }
 
 /*
@@ -47,29 +34,22 @@ export function determineAdvancedMode(
   depending on the editor settings, the alert type, and the queries.
    */
 export const useAdvancedMode = (
-  editorSettings: SimplifiedEditor | undefined,
+  simplifiedQueryEditor: boolean | undefined,
   isGrafanaAlertingType: boolean,
-  isNewFromQueryParams: boolean,
   dataQueries: Array<AlertQuery<ExpressionQuery | AlertDataQuery>>,
   expressionQueries: Array<AlertQuery<ExpressionQuery>>
 ) => {
-  const isAdvancedMode = determineAdvancedMode(
+  const isAdvancedMode = determineAdvancedMode(simplifiedQueryEditor, isGrafanaAlertingType);
-    editorSettings,
-    isGrafanaAlertingType,
-    isNewFromQueryParams,
-    dataQueries,
-    expressionQueries
-  );
 
   const [simpleCondition, setSimpleCondition] = useState<SimpleCondition>(
     initializeSimpleCondition(isGrafanaAlertingType, dataQueries, expressionQueries)
   );
 
   useEffect(() => {
-    if (!isAdvancedMode && isGrafanaAlertingType) {
+    if (isGrafanaAlertingType && !isAdvancedMode) {
       setSimpleCondition(getSimpleConditionFromExpressions(expressionQueries));
     }
   }, [isAdvancedMode, expressionQueries, isGrafanaAlertingType]);
 
-  return { isAdvancedMode, simpleCondition, setSimpleCondition };
+  return { simpleCondition, setSimpleCondition };
 };